Add URL beside service

pkg/apis/admissionregistration/types.go

@@ -266,26 +266,60 @@ const (
 // WebhookClientConfig contains the information to make a TLS
 // connection with the webhook
 type WebhookClientConfig struct {
-	// Service is a reference to the service for this webhook. If there is only
-	// one port open for the service, that port will be used. If there are multiple
-	// ports open, port 443 will be used if it is open, otherwise it is an error.
-	// Required
-	Service ServiceReference
+	// `url` gives the location of the webhook, in standard URL form
+	// (`[scheme://]host:port/path`). Exactly one of `url` or `service`
+	// must be specified.
+	//
+	// The `host` should not refer to a service running in the cluster; use
+	// the `service` field instead. The host might be resolved via external
+	// DNS in some apiservers (e.g., `kube-apiserver` cannot resolve
+	// in-cluster DNS as that would be a layering violation). `host` may
+	// also be an IP address.
+	//
+	// Please note that using `localhost` or `127.0.0.1` as a `host` is
+	// risky unless you take great care to run this webhook on all hosts
+	// which run an apiserver which might need to make calls to this
+	// webhook. Such installs are likely to be non-portable, i.e., not easy
+	// to turn up in a new cluster.
+	//
+	// If the scheme is present, it must be "https://".
+	//
+	// A path is optional, and if present may be any string permissible in
+	// a URL. You may use the path to pass an arbitrary string to the
+	// webhook, for example, a cluster identifier.
+	//
+	// +optional
+	URL *string
 
-	// URLPath is an optional field that specifies the URL path to use when posting the AdmissionReview object.
-	URLPath string
+	// `service` is a reference to the service for this webhook. Either
+	// `service` or `url` must be specified.
+	//
+	// If the webhook is running within the cluster, then you should use `service`.
+	//
+	// If there is only one port open for the service, that port will be
+	// used. If there are multiple ports open, port 443 will be used if it
+	// is open, otherwise it is an error.
+	//
+	// +optional
+	Service *ServiceReference
 
-	// CABundle is a PEM encoded CA bundle which will be used to validate webhook's server certificate.
-	// Required
+	// `caBundle` is a PEM encoded CA bundle which will be used to validate
+	// the webhook's server certificate.
+	// Required.
 	CABundle []byte
 }
 
 // ServiceReference holds a reference to Service.legacy.k8s.io
 type ServiceReference struct {
-	// Namespace is the namespace of the service
+	// `namespace` is the namespace of the service.
 	// Required
 	Namespace string
-	// Name is the name of the service
+	// `name` is the name of the service.
 	// Required
 	Name string
+
+	// `path` is an optional URL path which will be sent in any request to
+	// this service.
+	// +optional
+	Path *string
 }

pkg/apis/admissionregistration/validation/validation.go

@@ -18,6 +18,7 @@ package validation
 
 import (
 	"fmt"
+	"net/url"
 	"strings"
 
 	genericvalidation "k8s.io/apimachinery/pkg/api/validation"
@@ -192,29 +193,69 @@ func validateWebhook(hook *admissionregistration.Webhook, fldPath *field.Path) f
 		allErrors = append(allErrors, field.NotSupported(fldPath.Child("failurePolicy"), *hook.FailurePolicy, supportedFailurePolicies.List()))
 	}
 
-	if len(hook.ClientConfig.URLPath) != 0 {
-		allErrors = append(allErrors, validateURLPath(fldPath.Child("clientConfig", "urlPath"), hook.ClientConfig.URLPath)...)
-	}
-
 	if hook.NamespaceSelector != nil {
 		allErrors = append(allErrors, metav1validation.ValidateLabelSelector(hook.NamespaceSelector, fldPath.Child("namespaceSelector"))...)
 	}
 
+	allErrors = append(allErrors, validateWebhookClientConfig(fldPath.Child("clientConfig"), &hook.ClientConfig)...)
+
+	return allErrors
+}
+
+func validateWebhookClientConfig(fldPath *field.Path, cc *admissionregistration.WebhookClientConfig) field.ErrorList {
+	var allErrors field.ErrorList
+	if (cc.URL == nil) == (cc.Service == nil) {
+		allErrors = append(allErrors, field.Required(fldPath.Child("url"), "exactly one of url or service is required"))
+	}
+
+	if cc.URL != nil {
+		const form = "; desired format: https://host[/path]"
+		if u, err := url.Parse(*cc.URL); err != nil {
+			allErrors = append(allErrors, field.Required(fldPath.Child("url"), "url must be a valid URL: "+err.Error()+form))
+		} else {
+			if u.Scheme != "" && u.Scheme != "https" {
+				allErrors = append(allErrors, field.Required(fldPath.Child("url"), "'https' is the only allowed URL scheme"+form))
+			}
+			if len(u.Host) == 0 {
+				allErrors = append(allErrors, field.Required(fldPath.Child("url"), "host must be provided"+form))
+			}
+		}
+	}
+
+	if cc.Service != nil {
+		allErrors = append(allErrors, validateWebhookService(fldPath.Child("service"), cc.Service)...)
+	}
 	return allErrors
 }
 
-func validateURLPath(fldPath *field.Path, urlPath string) field.ErrorList {
+func validateWebhookService(fldPath *field.Path, svc *admissionregistration.ServiceReference) field.ErrorList {
 	var allErrors field.ErrorList
+
+	if len(svc.Name) == 0 {
+		allErrors = append(allErrors, field.Required(fldPath.Child("name"), "service name is required"))
+	}
+
+	if len(svc.Namespace) == 0 {
+		allErrors = append(allErrors, field.Required(fldPath.Child("namespace"), "service namespace is required"))
+	}
+
+	if svc.Path == nil {
+		return allErrors
+	}
+
+	// TODO: replace below with url.Parse + verifying that host is empty?
+
+	urlPath := *svc.Path
 	if urlPath == "/" || len(urlPath) == 0 {
 		return allErrors
 	}
 	if urlPath == "//" {
-		allErrors = append(allErrors, field.Invalid(fldPath, urlPath, "segment[0] may not be empty"))
+		allErrors = append(allErrors, field.Invalid(fldPath.Child("path"), urlPath, "segment[0] may not be empty"))
 		return allErrors
 	}
 
 	if !strings.HasPrefix(urlPath, "/") {
-		allErrors = append(allErrors, field.Invalid(fldPath, urlPath, "must start with a '/'"))
+		allErrors = append(allErrors, field.Invalid(fldPath.Child("path"), urlPath, "must start with a '/'"))
 	}
 
 	urlPathToCheck := urlPath[1:]
@@ -224,12 +265,12 @@ func validateURLPath(fldPath *field.Path, urlPath string) field.ErrorList {
 	steps := strings.Split(urlPathToCheck, "/")
 	for i, step := range steps {
 		if len(step) == 0 {
-			allErrors = append(allErrors, field.Invalid(fldPath, urlPath, fmt.Sprintf("segment[%d] may not be empty", i)))
+			allErrors = append(allErrors, field.Invalid(fldPath.Child("path"), urlPath, fmt.Sprintf("segment[%d] may not be empty", i)))
 			continue
 		}
 		failures := validation.IsDNS1123Subdomain(step)
 		for _, failure := range failures {
-			allErrors = append(allErrors, field.Invalid(fldPath, urlPath, fmt.Sprintf("segment[%d]: %v", i, failure)))
+			allErrors = append(allErrors, field.Invalid(fldPath.Child("path"), urlPath, fmt.Sprintf("segment[%d]: %v", i, failure)))
 		}
 	}
 

staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go

@@ -272,26 +272,60 @@ const (
 // WebhookClientConfig contains the information to make a TLS
 // connection with the webhook
 type WebhookClientConfig struct {
-	// Service is a reference to the service for this webhook. If there is only
-	// one port open for the service, that port will be used. If there are multiple
-	// ports open, port 443 will be used if it is open, otherwise it is an error.
-	// Required
-	Service ServiceReference `json:"service" protobuf:"bytes,1,opt,name=service"`
+	// `url` gives the location of the webhook, in standard URL form
+	// (`[scheme://]host:port/path`). Exactly one of `url` or `service`
+	// must be specified.
+	//
+	// The `host` should not refer to a service running in the cluster; use
+	// the `service` field instead. The host might be resolved via external
+	// DNS in some apiservers (e.g., `kube-apiserver` cannot resolve
+	// in-cluster DNS as that would be a layering violation). `host` may
+	// also be an IP address.
+	//
+	// Please note that using `localhost` or `127.0.0.1` as a `host` is
+	// risky unless you take great care to run this webhook on all hosts
+	// which run an apiserver which might need to make calls to this
+	// webhook. Such installs are likely to be non-portable, i.e., not easy
+	// to turn up in a new cluster.
+	//
+	// If the scheme is present, it must be "https://".
+	//
+	// A path is optional, and if present may be any string permissible in
+	// a URL. You may use the path to pass an arbitrary string to the
+	// webhook, for example, a cluster identifier.
+	//
+	// +optional
+	URL *string `json:"url,omitempty"`
 
-	// URLPath is an optional field that specifies the URL path to use when posting the AdmissionReview object.
-	URLPath string `json:"urlPath" protobuf:"bytes,3,opt,name=urlPath"`
+	// `service` is a reference to the service for this webhook. Either
+	// `service` or `url` must be specified.
+	//
+	// If the webhook is running within the cluster, then you should use `service`.
+	//
+	// If there is only one port open for the service, that port will be
+	// used. If there are multiple ports open, port 443 will be used if it
+	// is open, otherwise it is an error.
+	//
+	// +optional
+	Service *ServiceReference `json:"service" protobuf:"bytes,1,opt,name=service"`
 
-	// CABundle is a PEM encoded CA bundle which will be used to validate webhook's server certificate.
-	// Required
+	// `caBundle` is a PEM encoded CA bundle which will be used to validate
+	// the webhook's server certificate.
+	// Required.
 	CABundle []byte `json:"caBundle" protobuf:"bytes,2,opt,name=caBundle"`
 }
 
 // ServiceReference holds a reference to Service.legacy.k8s.io
 type ServiceReference struct {
-	// Namespace is the namespace of the service
+	// `namespace` is the namespace of the service.
 	// Required
 	Namespace string `json:"namespace" protobuf:"bytes,1,opt,name=namespace"`
-	// Name is the name of the service
+	// `name` is the name of the service.
 	// Required
 	Name string `json:"name" protobuf:"bytes,2,opt,name=name"`
+
+	// `path` is an optional URL path which will be sent in any request to
+	// this service.
+	// +optional
+	Path *string `json:"path,omitempty"`
 }

staging/src/k8s.io/apiserver/pkg/admission/plugin/webhook/admission.go

@@ -20,6 +20,7 @@ package webhook
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net"
@@ -55,6 +56,10 @@ const (
 	defaultCacheSize = 200
 )
 
+var (
+	ErrNeedServiceOrURL = errors.New("webhook configuration must have either service or URL")
+)
+
 type ErrCallingWebhook struct {
 	WebhookName string
 	Reason      error
@@ -395,47 +400,71 @@ func toStatusErr(name string, result *metav1.Status) *apierrors.StatusError {
 
 func (a *GenericAdmissionWebhook) hookClient(h *v1alpha1.Webhook) (*rest.RESTClient, error) {
 	cacheKey, err := json.Marshal(h.ClientConfig)
-	if err != nil {
-		return nil, err
-	}
 	if client, ok := a.cache.Get(string(cacheKey)); ok {
 		return client.(*rest.RESTClient), nil
 	}
 
-	serverName := h.ClientConfig.Service.Name + "." + h.ClientConfig.Service.Namespace + ".svc"
-	restConfig, err := a.authInfoResolver.ClientConfigFor(serverName)
-	if err != nil {
-		return nil, err
+	complete := func(cfg *rest.Config) (*rest.RESTClient, error) {
+		cfg.TLSClientConfig.CAData = h.ClientConfig.CABundle
+		cfg.ContentConfig.NegotiatedSerializer = a.negotiatedSerializer
+		cfg.ContentConfig.ContentType = runtime.ContentTypeJSON
+		client, err := rest.UnversionedRESTClientFor(cfg)
+		if err == nil {
+			a.cache.Add(string(cacheKey), client)
+		}
+		return client, err
 	}
 
-	cfg := rest.CopyConfig(restConfig)
-	host := serverName + ":443"
-	cfg.Host = "https://" + host
-	cfg.APIPath = h.ClientConfig.URLPath
-	cfg.TLSClientConfig.ServerName = serverName
-	cfg.TLSClientConfig.CAData = h.ClientConfig.CABundle
-	cfg.ContentConfig.NegotiatedSerializer = a.negotiatedSerializer
-	cfg.ContentConfig.ContentType = runtime.ContentTypeJSON
-
-	delegateDialer := cfg.Dial
-	if delegateDialer == nil {
-		delegateDialer = net.Dial
-	}
-
-	cfg.Dial = func(network, addr string) (net.Conn, error) {
-		if addr == host {
-			u, err := a.serviceResolver.ResolveEndpoint(h.ClientConfig.Service.Namespace, h.ClientConfig.Service.Name)
-			if err != nil {
-				return nil, err
+	if svc := h.ClientConfig.Service; svc != nil {
+		serverName := svc.Name + "." + svc.Namespace + ".svc"
+		restConfig, err := a.authInfoResolver.ClientConfigFor(serverName)
+		if err != nil {
+			return nil, err
+		}
+		cfg := rest.CopyConfig(restConfig)
+		host := serverName + ":443"
+		cfg.Host = "https://" + host
+		if svc.Path != nil {
+			cfg.APIPath = *svc.Path
+		}
+		cfg.TLSClientConfig.ServerName = serverName
+
+		delegateDialer := cfg.Dial
+		if delegateDialer == nil {
+			delegateDialer = net.Dial
+		}
+		cfg.Dial = func(network, addr string) (net.Conn, error) {
+			if addr == host {
+				u, err := a.serviceResolver.ResolveEndpoint(svc.Namespace, svc.Name)
+				if err != nil {
+					return nil, err
+				}
+				addr = u.Host
 			}
-			addr = u.Host
+			return delegateDialer(network, addr)
 		}
-		return delegateDialer(network, addr)
+
+		return complete(cfg)
+	}
+
+	if h.ClientConfig.URL == nil {
+		return nil, &ErrCallingWebhook{WebhookName: h.Name, Reason: ErrNeedServiceOrURL}
+	}
+
+	u, err := url.Parse(*h.ClientConfig.URL)
+	if err != nil {
+		return nil, &ErrCallingWebhook{WebhookName: h.Name, Reason: fmt.Errorf("Unparsable URL: %v", err)}
 	}
 
-	client, err := rest.UnversionedRESTClientFor(cfg)
-	if err == nil {
-		a.cache.Add(string(cacheKey), client)
+	restConfig, err := a.authInfoResolver.ClientConfigFor(u.Host)
+	if err != nil {
+		return nil, err
 	}
-	return client, err
+
+	cfg := rest.CopyConfig(restConfig)
+	cfg.Host = u.Host
+	cfg.APIPath = u.Path
+	// TODO: test if this is needed: cfg.TLSClientConfig.ServerName = u.Host
+
+	return complete(cfg)
 }

test/e2e/apimachinery/webhook.go

@@ -218,6 +218,8 @@ func deployWebhookAndService(f *framework.Framework, image string, context *cert
 	framework.ExpectNoError(err, "waiting for service %s/%s have %d endpoint", namespace, serviceName, 1)
 }
 
+func strPtr(s string) *string { return &s }
+
 func registerWebhook(f *framework.Framework, context *certContext) {
 	client := f.ClientSet
 	By("Registering the webhook via the AdmissionRegistration API")
@@ -239,11 +241,11 @@ func registerWebhook(f *framework.Framework, context *certContext) {
 					},
 				}},
 				ClientConfig: v1alpha1.WebhookClientConfig{
-					Service: v1alpha1.ServiceReference{
+					Service: &v1alpha1.ServiceReference{
 						Namespace: namespace,
 						Name:      serviceName,
+						Path:      strPtr("/pods"),
 					},
-					URLPath:  "/pods",
 					CABundle: context.signingCert,
 				},
 			},
@@ -268,11 +270,11 @@ func registerWebhook(f *framework.Framework, context *certContext) {
 					},
 				},
 				ClientConfig: v1alpha1.WebhookClientConfig{
-					Service: v1alpha1.ServiceReference{
+					Service: &v1alpha1.ServiceReference{
 						Namespace: namespace,
 						Name:      serviceName,
+						Path:      strPtr("/configmaps"),
 					},
-					URLPath:  "/configmaps",
 					CABundle: context.signingCert,
 				},
 			},

test/integration/etcd/etcd_storage_path_test.go

@@ -382,11 +382,11 @@ var etcdStorageData = map[schema.GroupVersionResource]struct {
 		expectedEtcdPath: "/registry/initializerconfigurations/ic1",
 	},
 	gvr("admissionregistration.k8s.io", "v1alpha1", "validatingwebhookconfigurations"): {
-		stub:             `{"metadata":{"name":"hook1","creationTimestamp":null},"webhooks":[{"name":"externaladmissionhook.k8s.io","clientConfig":{"service":{"namespace":"","name":""},"caBundle":null},"rules":[{"operations":["CREATE"],"apiGroups":["group"],"apiVersions":["version"],"resources":["resource"]}],"failurePolicy":"Ignore"}]}`,
+		stub:             `{"metadata":{"name":"hook1","creationTimestamp":null},"webhooks":[{"name":"externaladmissionhook.k8s.io","clientConfig":{"service":{"namespace":"ns","name":"n"},"caBundle":null},"rules":[{"operations":["CREATE"],"apiGroups":["group"],"apiVersions":["version"],"resources":["resource"]}],"failurePolicy":"Ignore"}]}`,
 		expectedEtcdPath: "/registry/validatingwebhookconfigurations/hook1",
 	},
 	gvr("admissionregistration.k8s.io", "v1alpha1", "mutatingwebhookconfigurations"): {
-		stub:             `{"metadata":{"name":"hook1","creationTimestamp":null},"webhooks":[{"name":"externaladmissionhook.k8s.io","clientConfig":{"service":{"namespace":"","name":""},"caBundle":null},"rules":[{"operations":["CREATE"],"apiGroups":["group"],"apiVersions":["version"],"resources":["resource"]}],"failurePolicy":"Ignore"}]}`,
+		stub:             `{"metadata":{"name":"hook1","creationTimestamp":null},"webhooks":[{"name":"externaladmissionhook.k8s.io","clientConfig":{"service":{"namespace":"ns","name":"n"},"caBundle":null},"rules":[{"operations":["CREATE"],"apiGroups":["group"],"apiVersions":["version"],"resources":["resource"]}],"failurePolicy":"Ignore"}]}`,
 		expectedEtcdPath: "/registry/mutatingwebhookconfigurations/hook1",
 	},
 	// --