From 33deaedaf6244bc31b1a4496fa546ed41f62d73e Mon Sep 17 00:00:00 2001 From: David Eads Date: Wed, 18 Oct 2017 10:57:39 -0400 Subject: [PATCH] add url path for admission webhooks --- pkg/apis/admissionregistration/types.go | 4 ++ .../v1alpha1/zz_generated.conversion.go | 2 + .../validation/validation.go | 18 +++++++ .../validation/validation_test.go | 52 ++++++++++++++++++ plugin/pkg/admission/webhook/admission.go | 4 +- .../pkg/admission/webhook/admission_test.go | 53 +++++++++---------- .../admissionregistration/v1alpha1/types.go | 4 ++ 7 files changed, 107 insertions(+), 30 deletions(-) diff --git a/pkg/apis/admissionregistration/types.go b/pkg/apis/admissionregistration/types.go index b4676560037..092f790c491 100644 --- a/pkg/apis/admissionregistration/types.go +++ b/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 diff --git a/pkg/apis/admissionregistration/v1alpha1/zz_generated.conversion.go b/pkg/apis/admissionregistration/v1alpha1/zz_generated.conversion.go index 86ef143f0ae..792254ba656 100644 --- a/pkg/apis/admissionregistration/v1alpha1/zz_generated.conversion.go +++ b/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 } diff --git a/pkg/apis/admissionregistration/validation/validation.go b/pkg/apis/admissionregistration/validation/validation.go index 3c4068f24c7..8f6c71980b0 100644 --- a/pkg/apis/admissionregistration/validation/validation.go +++ b/pkg/apis/admissionregistration/validation/validation.go @@ -182,6 +182,24 @@ 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 { + if !strings.HasPrefix(hook.ClientConfig.URLPath, "/") || !strings.HasSuffix(hook.ClientConfig.URLPath, "/") { + allErrors = append(allErrors, field.Invalid(fldPath.Child("clientConfig", "urlPath"), hook.ClientConfig.URLPath, "must start and end with a '/'")) + } + steps := strings.Split(hook.ClientConfig.URLPath[1:len(hook.ClientConfig.URLPath)-1], "/") + for i, step := range steps { + if len(step) == 0 { + allErrors = append(allErrors, field.Invalid(fldPath.Child("clientConfig", "urlPath"), hook.ClientConfig.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.Child("clientConfig", "urlPath"), hook.ClientConfig.URLPath, fmt.Sprintf("segment[%d]: %v", i, failure))) + } + } + } + return allErrors } diff --git a/pkg/apis/admissionregistration/validation/validation_test.go b/pkg/apis/admissionregistration/validation/validation_test.go index a842c472818..3de314fd28e 100644 --- a/pkg/apis/admissionregistration/validation/validation_test.go +++ b/pkg/apis/admissionregistration/validation/validation_test.go @@ -482,6 +482,58 @@ 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 and end with a '/'`, + }, + { + name: "URLPath must end with slash", + config: getExternalAdmissionHookConfiguration( + []admissionregistration.ExternalAdmissionHook{ + { + Name: "webhook.k8s.io", + ClientConfig: admissionregistration.AdmissionHookClientConfig{ + URLPath: "/foo", + }, + }, + }), + expectedError: `clientConfig.urlPath: Invalid value: "/foo": must start and end with a '/'`, + }, + { + 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 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) diff --git a/plugin/pkg/admission/webhook/admission.go b/plugin/pkg/admission/webhook/admission.go index f7155b5a658..4cb9828dd04 100644 --- a/plugin/pkg/admission/webhook/admission.go +++ b/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, diff --git a/plugin/pkg/admission/webhook/admission_test.go b/plugin/pkg/admission/webhook/admission_test.go index 71076d0a813..b4e13c3e6b7 100644 --- a/plugin/pkg/admission/webhook/admission_test.go +++ b/plugin/pkg/admission/webhook/admission_test.go @@ -54,7 +54,6 @@ func (f *fakeHookSource) Run(stopCh <-chan struct{}) {} type fakeServiceResolver struct { base url.URL - path string } func (f fakeServiceResolver) ResolveEndpoint(namespace, name string) (*url.URL, error) { @@ -62,7 +61,6 @@ func (f fakeServiceResolver) ResolveEndpoint(namespace, name string) (*url.URL, return nil, fmt.Errorf("couldn't resolve service location") } u := f.base - u.Path = f.path return &u, nil } @@ -128,13 +126,17 @@ func TestAdmit(t *testing.T) { expectAllow bool errorContains string } - ccfg := registrationv1alpha1.AdmissionHookClientConfig{ - Service: registrationv1alpha1.ServiceReference{ - Name: "webhook-test", - Namespace: "default", - }, - CABundle: caCert, + ccfg := func(urlPath string) registrationv1alpha1.AdmissionHookClientConfig { + return registrationv1alpha1.AdmissionHookClientConfig{ + Service: registrationv1alpha1.ServiceReference{ + Name: "webhook-test", + Namespace: "default", + }, + URLPath: urlPath, + CABundle: caCert, + } } + matchEverythingRules := []registrationv1alpha1.RuleWithOperations{{ Operations: []registrationv1alpha1.OperationType{registrationv1alpha1.OperationAll}, Rule: registrationv1alpha1.Rule{ @@ -152,109 +154,102 @@ func TestAdmit(t *testing.T) { hookSource: fakeHookSource{ hooks: []registrationv1alpha1.ExternalAdmissionHook{{ Name: "nomatch", - ClientConfig: ccfg, + ClientConfig: ccfg("disallow"), Rules: []registrationv1alpha1.RuleWithOperations{{ Operations: []registrationv1alpha1.OperationType{registrationv1alpha1.Create}, }}, }}, }, - path: "disallow", expectAllow: true, }, "match & allow": { hookSource: fakeHookSource{ hooks: []registrationv1alpha1.ExternalAdmissionHook{{ Name: "allow", - ClientConfig: ccfg, + ClientConfig: ccfg("allow"), Rules: matchEverythingRules, }}, }, - path: "allow", expectAllow: true, }, "match & disallow": { hookSource: fakeHookSource{ hooks: []registrationv1alpha1.ExternalAdmissionHook{{ Name: "disallow", - ClientConfig: ccfg, + ClientConfig: ccfg("disallow"), Rules: matchEverythingRules, }}, }, - path: "disallow", errorContains: "without explanation", }, "match & disallow ii": { hookSource: fakeHookSource{ hooks: []registrationv1alpha1.ExternalAdmissionHook{{ Name: "disallowReason", - ClientConfig: ccfg, + ClientConfig: ccfg("disallowReason"), Rules: matchEverythingRules, }}, }, - path: "disallowReason", errorContains: "you shall not pass", }, "match & fail (but allow because fail open)": { hookSource: fakeHookSource{ hooks: []registrationv1alpha1.ExternalAdmissionHook{{ Name: "internalErr A", - ClientConfig: ccfg, + ClientConfig: ccfg("internalErr"), Rules: matchEverythingRules, FailurePolicy: &policyIgnore, }, { Name: "internalErr B", - ClientConfig: ccfg, + ClientConfig: ccfg("internalErr"), Rules: matchEverythingRules, FailurePolicy: &policyIgnore, }, { Name: "internalErr C", - ClientConfig: ccfg, + ClientConfig: ccfg("internalErr"), Rules: matchEverythingRules, FailurePolicy: &policyIgnore, }}, }, - path: "internalErr", expectAllow: true, }, "match & fail (but allow because fail open on nil)": { hookSource: fakeHookSource{ hooks: []registrationv1alpha1.ExternalAdmissionHook{{ Name: "internalErr A", - ClientConfig: ccfg, + ClientConfig: ccfg("internalErr"), Rules: matchEverythingRules, }, { Name: "internalErr B", - ClientConfig: ccfg, + ClientConfig: ccfg("internalErr"), Rules: matchEverythingRules, }, { Name: "internalErr C", - ClientConfig: ccfg, + ClientConfig: ccfg("internalErr"), Rules: matchEverythingRules, }}, }, - path: "internalErr", expectAllow: true, }, "match & fail (but fail because fail closed)": { hookSource: fakeHookSource{ hooks: []registrationv1alpha1.ExternalAdmissionHook{{ Name: "internalErr A", - ClientConfig: ccfg, + ClientConfig: ccfg("internalErr"), Rules: matchEverythingRules, FailurePolicy: &policyFail, }, { Name: "internalErr B", - ClientConfig: ccfg, + ClientConfig: ccfg("internalErr"), Rules: matchEverythingRules, FailurePolicy: &policyFail, }, { Name: "internalErr C", - ClientConfig: ccfg, + ClientConfig: ccfg("internalErr"), Rules: matchEverythingRules, FailurePolicy: &policyFail, }}, }, - path: "internalErr", expectAllow: false, }, } @@ -262,7 +257,7 @@ func TestAdmit(t *testing.T) { for name, tt := range table { t.Run(name, func(t *testing.T) { wh.hookSource = &tt.hookSource - wh.serviceResolver = fakeServiceResolver{base: *serverURL, path: tt.path} + wh.serviceResolver = fakeServiceResolver{base: *serverURL} wh.SetScheme(legacyscheme.Scheme) err = wh.Admit(admission.NewAttributesRecord(&object, &oldObject, kind, namespace, name, resource, subResource, operation, &userInfo)) diff --git a/staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go b/staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go index d4827e59d33..5e8541f4fa5 100644 --- a/staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go +++ b/staging/src/k8s.io/api/admissionregistration/v1alpha1/types.go @@ -203,6 +203,10 @@ type AdmissionHookClientConfig struct { // 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"` + + // 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"` + // CABundle is a PEM encoded CA bundle which will be used to validate webhook's server certificate. // Required CABundle []byte `json:"caBundle" protobuf:"bytes,2,opt,name=caBundle"`