fix docs and validation

pkg/apis/admissionregistration/types.go

@@ -282,12 +282,16 @@ type WebhookClientConfig struct {
 	// 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://".
+	// The scheme must be "https"; the URL must begin with "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.
 	//
+	// Attempting to use a user or basic auth e.g. "user:password@" is not
+	// allowed. Fragments ("#...") and query parameters ("?...") are not
+	// allowed, either.
+	//
 	// +optional
 	URL *string
 

pkg/apis/admissionregistration/validation/validation.go

@@ -213,11 +213,20 @@ func validateWebhookClientConfig(fldPath *field.Path, cc *admissionregistration.
 		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 u.Scheme != "https" {
+				allErrors = append(allErrors, field.Invalid(fldPath.Child("url"), u.Scheme, "'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))
+				allErrors = append(allErrors, field.Invalid(fldPath.Child("url"), u.Host, "host must be provided"+form))
+			}
+			if u.User != nil {
+				allErrors = append(allErrors, field.Invalid(fldPath.Child("url"), u.User.String(), "user information is not permitted in the URL"))
+			}
+			if len(u.Fragment) != 0 {
+				allErrors = append(allErrors, field.Invalid(fldPath.Child("url"), u.Fragment, "fragments are not permitted in the URL"))
+			}
+			if len(u.RawQuery) != 0 {
+				allErrors = append(allErrors, field.Invalid(fldPath.Child("url"), u.RawQuery, "query parameters are not permitted in the URL"))
 			}
 		}
 	}

pkg/apis/admissionregistration/validation/validation_test.go

@@ -538,7 +538,7 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
 						},
 					},
 				}),
-			expectedError: `[0].clientConfig.url: Required value: host must be provided`,
+			expectedError: `[0].clientConfig.url: Invalid value: "": host must be provided`,
 		},
 		{
 			name: "wrong scheme",
@@ -567,6 +567,45 @@ func TestValidateValidatingWebhookConfiguration(t *testing.T) {
 			expectedError: `host must be provided`,
 		},
 		{
+			name: "fragment",
+			config: newValidatingWebhookConfiguration(
+				[]admissionregistration.Webhook{
+					{
+						Name: "webhook.k8s.io",
+						ClientConfig: admissionregistration.WebhookClientConfig{
+							URL: strPtr("https://example.com/#bookmark"),
+						},
+					},
+				}),
+			expectedError: `"bookmark": fragments are not permitted`,
+		},
+		{
+			name: "query",
+			config: newValidatingWebhookConfiguration(
+				[]admissionregistration.Webhook{
+					{
+						Name: "webhook.k8s.io",
+						ClientConfig: admissionregistration.WebhookClientConfig{
+							URL: strPtr("https://example.com?arg=value"),
+						},
+					},
+				}),
+			expectedError: `"arg=value": query parameters are not permitted`,
+		},
+		{
+			name: "user",
+			config: newValidatingWebhookConfiguration(
+				[]admissionregistration.Webhook{
+					{
+						Name: "webhook.k8s.io",
+						ClientConfig: admissionregistration.WebhookClientConfig{
+							URL: strPtr("https://harry.potter@example.com/"),
+						},
+					},
+				}),
+			expectedError: `"harry.potter": user information is not permitted`,
+		},
+		{
 			name: "just totally wrong",
 			config: newValidatingWebhookConfiguration(
 				[]admissionregistration.Webhook{

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

@@ -288,12 +288,16 @@ type WebhookClientConfig struct {
 	// 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://".
+	// The scheme must be "https"; the URL must begin with "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.
 	//
+	// Attempting to use a user or basic auth e.g. "user:password@" is not
+	// allowed. Fragments ("#...") and query parameters ("?...") are not
+	// allowed, either.
+	//
 	// +optional
 	URL *string `json:"url,omitempty" protobuf:"bytes,3,opt,name=url"`
 

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

@@ -400,6 +400,9 @@ 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
 	}
@@ -464,7 +467,6 @@ func (a *GenericAdmissionWebhook) hookClient(h *v1alpha1.Webhook) (*rest.RESTCli
 	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)
 }