Merge pull request kubernetes#54145 from deads2k/admission-05-url

Automatic merge from submit-queue (batch tested with PRs 54145, 53821). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

add url path for admission webhooks

Fixes kubernetes#53826

Adds an optional field to admission webhook registration that allows a user to specify a path to post to.  This achieves parity with other webhooks.

Each segment is required to be a dns subdomain, which mirrors url rules for groups.

@kubernetes/api-reviewers 

```release-note
admission webhook registration now allows URL paths
```

api/openapi-spec/swagger.json

@@ -62262,6 +62262,7 @@
     "description": "AdmissionHookClientConfig contains the information to make a TLS connection with the webhook",
     "required": [
      "service",
+     "urlPath",
      "caBundle"
     ],
     "properties": {
@@ -62273,6 +62274,10 @@
      "service": {
       "description": "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",
       "$ref": "#/definitions/io.k8s.api.admissionregistration.v1alpha1.ServiceReference"
+     },
+     "urlPath": {
+      "description": "URLPath is an optional field that specifies the URL path to use when posting the AdmissionReview object.",
+      "type": "string"
      }
     }
    },

api/swagger-spec/admissionregistration.k8s.io_v1alpha1.json

@@ -1764,13 +1764,18 @@
     "description": "AdmissionHookClientConfig contains the information to make a TLS connection with the webhook",
     "required": [
      "service",
+     "urlPath",
      "caBundle"
     ],
     "properties": {
      "service": {
       "$ref": "v1alpha1.ServiceReference",
       "description": "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"
      },
+     "urlPath": {
+      "type": "string",
+      "description": "URLPath is an optional field that specifies the URL path to use when posting the AdmissionReview object."
+     },
      "caBundle": {
       "type": "string",
       "description": "CABundle is a PEM encoded CA bundle which will be used to validate webhook's server certificate. Required"

docs/api-reference/admissionregistration.k8s.io/v1alpha1/definitions.html

@@ -720,6 +720,13 @@ <h3 id="_v1alpha1_admissionhookclientconfig">v1alpha1.AdmissionHookClientConfig<
 <td class="tableblock halign-left valign-top"></td>
 </tr>
 <tr>
+<td class="tableblock halign-left valign-top"><p class="tableblock">urlPath</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">URLPath is an optional field that specifies the URL path to use when posting the AdmissionReview object.</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">true</p></td>
+<td class="tableblock halign-left valign-top"><p class="tableblock">string</p></td>
+<td class="tableblock halign-left valign-top"></td>
+</tr>
+<tr>
 <td class="tableblock halign-left valign-top"><p class="tableblock">caBundle</p></td>
 <td class="tableblock halign-left valign-top"><p class="tableblock">CABundle is a PEM encoded CA bundle which will be used to validate webhook&#8217;s server certificate. Required</p></td>
 <td class="tableblock halign-left valign-top"><p class="tableblock">true</p></td>

pkg/apis/admissionregistration/types.go

@@ -199,6 +199,10 @@ type AdmissionHookClientConfig struct {
 	// ports open, port 443 will be used if it is open, otherwise it is an error.
 	// Required
 	Service ServiceReference
+
+	// URLPath is an optional field that specifies the URL path to use when posting the AdmissionReview object.
+	URLPath string
+
 	// CABundle is a PEM encoded CA bundle which will be used to validate webhook's server certificate.
 	// Required
 	CABundle []byte

pkg/apis/admissionregistration/v1alpha1/zz_generated.conversion.go

@@ -63,6 +63,7 @@ func autoConvert_v1alpha1_AdmissionHookClientConfig_To_admissionregistration_Adm
 	if err := Convert_v1alpha1_ServiceReference_To_admissionregistration_ServiceReference(&in.Service, &out.Service, s); err != nil {
 		return err
 	}
+	out.URLPath = in.URLPath
 	out.CABundle = *(*[]byte)(unsafe.Pointer(&in.CABundle))
 	return nil
 }
@@ -76,6 +77,7 @@ func autoConvert_admissionregistration_AdmissionHookClientConfig_To_v1alpha1_Adm
 	if err := Convert_admissionregistration_ServiceReference_To_v1alpha1_ServiceReference(&in.Service, &out.Service, s); err != nil {
 		return err
 	}
+	out.URLPath = in.URLPath
 	out.CABundle = *(*[]byte)(unsafe.Pointer(&in.CABundle))
 	return nil
 }

pkg/apis/admissionregistration/validation/validation.go

@@ -182,6 +182,44 @@ func validateExternalAdmissionHook(hook *admissionregistration.ExternalAdmission
 	if hook.FailurePolicy != nil && !supportedFailurePolicies.Has(string(*hook.FailurePolicy)) {
 		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)...)
+	}
+
+	return allErrors
+}
+
+func validateURLPath(fldPath *field.Path, urlPath string) field.ErrorList {
+	var allErrors field.ErrorList
+	if urlPath == "/" || len(urlPath) == 0 {
+		return allErrors
+	}
+	if urlPath == "//" {
+		allErrors = append(allErrors, field.Invalid(fldPath, urlPath, "segment[0] may not be empty"))
+		return allErrors
+	}
+
+	if !strings.HasPrefix(urlPath, "/") {
+		allErrors = append(allErrors, field.Invalid(fldPath, urlPath, "must start with a '/'"))
+	}
+
+	urlPathToCheck := urlPath[1:]
+	if strings.HasSuffix(urlPathToCheck, "/") {
+		urlPathToCheck = urlPathToCheck[:len(urlPathToCheck)-1]
+	}
+	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)))
+			continue
+		}
+		failures := validation.IsDNS1123Subdomain(step)
+		for _, failure := range failures {
+			allErrors = append(allErrors, field.Invalid(fldPath, urlPath, fmt.Sprintf("segment[%d]: %v", i, failure)))
+		}
+	}
+
 	return allErrors
 }
 

pkg/apis/admissionregistration/validation/validation_test.go

@@ -482,18 +482,111 @@ func TestValidateExternalAdmissionHookConfiguration(t *testing.T) {
 				}),
 			expectedError: `externalAdmissionHooks[0].failurePolicy: Unsupported value: "other": supported values: "Fail", "Ignore"`,
 		},
+		{
+			name: "URLPath must start with slash",
+			config: getExternalAdmissionHookConfiguration(
+				[]admissionregistration.ExternalAdmissionHook{
+					{
+						Name: "webhook.k8s.io",
+						ClientConfig: admissionregistration.AdmissionHookClientConfig{
+							URLPath: "foo/",
+						},
+					},
+				}),
+			expectedError: `clientConfig.urlPath: Invalid value: "foo/": must start with a '/'`,
+		},
+		{
+			name: "URLPath accepts slash",
+			config: getExternalAdmissionHookConfiguration(
+				[]admissionregistration.ExternalAdmissionHook{
+					{
+						Name: "webhook.k8s.io",
+						ClientConfig: admissionregistration.AdmissionHookClientConfig{
+							URLPath: "/",
+						},
+					},
+				}),
+			expectedError: ``,
+		},
+		{
+			name: "URLPath accepts no trailing slash",
+			config: getExternalAdmissionHookConfiguration(
+				[]admissionregistration.ExternalAdmissionHook{
+					{
+						Name: "webhook.k8s.io",
+						ClientConfig: admissionregistration.AdmissionHookClientConfig{
+							URLPath: "/foo",
+						},
+					},
+				}),
+			expectedError: ``,
+		},
+		{
+			name: "URLPath fails //",
+			config: getExternalAdmissionHookConfiguration(
+				[]admissionregistration.ExternalAdmissionHook{
+					{
+						Name: "webhook.k8s.io",
+						ClientConfig: admissionregistration.AdmissionHookClientConfig{
+							URLPath: "//",
+						},
+					},
+				}),
+			expectedError: `clientConfig.urlPath: Invalid value: "//": segment[0] may not be empty`,
+		},
+		{
+			name: "URLPath no empty step",
+			config: getExternalAdmissionHookConfiguration(
+				[]admissionregistration.ExternalAdmissionHook{
+					{
+						Name: "webhook.k8s.io",
+						ClientConfig: admissionregistration.AdmissionHookClientConfig{
+							URLPath: "/foo//bar/",
+						},
+					},
+				}),
+			expectedError: `clientConfig.urlPath: Invalid value: "/foo//bar/": segment[1] may not be empty`,
+		}, {
+			name: "URLPath no empty step 2",
+			config: getExternalAdmissionHookConfiguration(
+				[]admissionregistration.ExternalAdmissionHook{
+					{
+						Name: "webhook.k8s.io",
+						ClientConfig: admissionregistration.AdmissionHookClientConfig{
+							URLPath: "/foo/bar//",
+						},
+					},
+				}),
+			expectedError: `clientConfig.urlPath: Invalid value: "/foo/bar//": segment[2] may not be empty`,
+		},
+		{
+			name: "URLPath no non-subdomain",
+			config: getExternalAdmissionHookConfiguration(
+				[]admissionregistration.ExternalAdmissionHook{
+					{
+						Name: "webhook.k8s.io",
+						ClientConfig: admissionregistration.AdmissionHookClientConfig{
+							URLPath: "/apis/foo.bar/v1alpha1/--bad",
+						},
+					},
+				}),
+			expectedError: `clientConfig.urlPath: Invalid value: "/apis/foo.bar/v1alpha1/--bad": segment[3]: a DNS-1123 subdomain`,
+		},
 	}
 	for _, test := range tests {
-		errs := ValidateExternalAdmissionHookConfiguration(test.config)
-		err := errs.ToAggregate()
-		if err != nil {
-			if e, a := test.expectedError, err.Error(); !strings.Contains(a, e) || e == "" {
-				t.Errorf("test case %s, expected to contain %s, got %s", test.name, e, a)
-			}
-		} else {
-			if test.expectedError != "" {
-				t.Errorf("test case %s, unexpected no error, expected to contain %s", test.name, test.expectedError)
+		t.Run(test.name, func(t *testing.T) {
+			errs := ValidateExternalAdmissionHookConfiguration(test.config)
+			err := errs.ToAggregate()
+			if err != nil {
+				if e, a := test.expectedError, err.Error(); !strings.Contains(a, e) || e == "" {
+					t.Errorf("expected to contain %s, got %s", e, a)
+				}
+			} else {
+				if test.expectedError != "" {
+					t.Errorf("unexpected no error, expected to contain %s", test.expectedError)
+				}
 			}
-		}
+		})
+
 	}
 }

plugin/pkg/admission/webhook/admission.go

@@ -45,6 +45,8 @@ import (
 	admissioninit "k8s.io/kubernetes/pkg/kubeapiserver/admission"
 
 	// install the clientgo admission API for use with api registry
+	"path"
+
 	_ "k8s.io/kubernetes/pkg/apis/admission/install"
 )
 
@@ -286,7 +288,7 @@ func (a *GenericAdmissionWebhook) hookClient(h *v1alpha1.ExternalAdmissionHook)
 	// TODO: cache these instead of constructing one each time
 	cfg := &rest.Config{
 		Host:    u.Host,
-		APIPath: u.Path,
+		APIPath: path.Join(u.Path, h.ClientConfig.URLPath),
 		TLSClientConfig: rest.TLSClientConfig{
 			ServerName: h.ClientConfig.Service.Name + "." + h.ClientConfig.Service.Namespace + ".svc",
 			CAData:     h.ClientConfig.CABundle,