From e13341355d00121b828ee4915ddee9e183540aa0 Mon Sep 17 00:00:00 2001 From: Vince Prignano Date: Mon, 27 Sep 2021 11:58:34 -0700 Subject: [PATCH] :sparkles: Allow webhooks to register custom validators/defaulter types This changeset allows our webhook builder to take in a handler any other struct other than a runtime.Object. Today having an object as the primary source of truth for both Defaulting and Validators makes API types carry a lot of information and business logic alongside their definitions. Moreover, lots of folks in the past have asked for ways to have an external type to handle these operations and use a controller runtime client for validations. This change brings a new way to register webhooks, which admission.For handler any type (struct) can be a defaulting or validating handler for a runtime Object. Signed-off-by: Vince Prignano --- pkg/builder/webhook.go | 78 +++++++-- pkg/builder/webhook_test.go | 199 ++++++++++++++++++++++ pkg/webhook/admission/defaulter_custom.go | 85 +++++++++ pkg/webhook/admission/validator_custom.go | 111 ++++++++++++ pkg/webhook/alias.go | 6 + 5 files changed, 461 insertions(+), 18 deletions(-) create mode 100644 pkg/webhook/admission/defaulter_custom.go create mode 100644 pkg/webhook/admission/validator_custom.go diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go index d24877d303..dcfa787bb2 100644 --- a/pkg/builder/webhook.go +++ b/pkg/builder/webhook.go @@ -17,6 +17,7 @@ limitations under the License. package builder import ( + "errors" "net/http" "net/url" "strings" @@ -32,10 +33,12 @@ import ( // WebhookBuilder builds a Webhook. type WebhookBuilder struct { - apiType runtime.Object - gvk schema.GroupVersionKind - mgr manager.Manager - config *rest.Config + apiType runtime.Object + withDefaulter admission.CustomDefaulter + withValidator admission.CustomValidator + gvk schema.GroupVersionKind + mgr manager.Manager + config *rest.Config } // WebhookManagedBy allows inform its manager.Manager. @@ -53,6 +56,18 @@ func (blder *WebhookBuilder) For(apiType runtime.Object) *WebhookBuilder { return blder } +// WithDefaulter takes a admission.WithDefaulter interface, a MutatingWebhook will be wired for this type. +func (blder *WebhookBuilder) WithDefaulter(defaulter admission.CustomDefaulter) *WebhookBuilder { + blder.withDefaulter = defaulter + return blder +} + +// WithValidator takes a admission.WithValidator interface, a ValidatingWebhook will be wired for this type. +func (blder *WebhookBuilder) WithValidator(validator admission.CustomValidator) *WebhookBuilder { + blder.withValidator = validator + return blder +} + // Complete builds the webhook. func (blder *WebhookBuilder) Complete() error { // Set the Config @@ -69,9 +84,13 @@ func (blder *WebhookBuilder) loadRestConfig() { } func (blder *WebhookBuilder) registerWebhooks() error { + typ, err := blder.getType() + if err != nil { + return err + } + // Create webhook(s) for each type - var err error - blder.gvk, err = apiutil.GVKForObject(blder.apiType, blder.mgr.GetScheme()) + blder.gvk, err = apiutil.GVKForObject(typ, blder.mgr.GetScheme()) if err != nil { return err } @@ -88,12 +107,7 @@ func (blder *WebhookBuilder) registerWebhooks() error { // registerDefaultingWebhook registers a defaulting webhook if th. func (blder *WebhookBuilder) registerDefaultingWebhook() { - defaulter, isDefaulter := blder.apiType.(admission.Defaulter) - if !isDefaulter { - log.Info("skip registering a mutating webhook, admission.Defaulter interface is not implemented", "GVK", blder.gvk) - return - } - mwh := admission.DefaultingWebhookFor(defaulter) + mwh := blder.getDefaultingWebhook() if mwh != nil { path := generateMutatePath(blder.gvk) @@ -108,13 +122,21 @@ func (blder *WebhookBuilder) registerDefaultingWebhook() { } } -func (blder *WebhookBuilder) registerValidatingWebhook() { - validator, isValidator := blder.apiType.(admission.Validator) - if !isValidator { - log.Info("skip registering a validating webhook, admission.Validator interface is not implemented", "GVK", blder.gvk) - return +func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook { + if defaulter := blder.withDefaulter; defaulter != nil { + return admission.WithCustomDefaulter(blder.apiType, defaulter) + } + if defaulter, ok := blder.apiType.(admission.Defaulter); ok { + return admission.DefaultingWebhookFor(defaulter) } - vwh := admission.ValidatingWebhookFor(validator) + log.Info( + "skip registering a mutating webhook, object does not implement admission.Defaulter or WithDefaulter wasn't called", + "GVK", blder.gvk) + return nil +} + +func (blder *WebhookBuilder) registerValidatingWebhook() { + vwh := blder.getValidatingWebhook() if vwh != nil { path := generateValidatePath(blder.gvk) @@ -129,6 +151,19 @@ func (blder *WebhookBuilder) registerValidatingWebhook() { } } +func (blder *WebhookBuilder) getValidatingWebhook() *admission.Webhook { + if validator := blder.withValidator; validator != nil { + return admission.WithCustomValidator(blder.apiType, validator) + } + if validator, ok := blder.apiType.(admission.Validator); ok { + return admission.ValidatingWebhookFor(validator) + } + log.Info( + "skip registering a validating webhook, object does not implement admission.Validator or WithValidator wasn't called", + "GVK", blder.gvk) + return nil +} + func (blder *WebhookBuilder) registerConversionWebhook() error { ok, err := conversion.IsConvertible(blder.mgr.GetScheme(), blder.apiType) if err != nil { @@ -145,6 +180,13 @@ func (blder *WebhookBuilder) registerConversionWebhook() error { return nil } +func (blder *WebhookBuilder) getType() (runtime.Object, error) { + if blder.apiType != nil { + return blder.apiType, nil + } + return nil, errors.New("For() must be called with a valid object") +} + func (blder *WebhookBuilder) isAlreadyHandled(path string) bool { if blder.mgr.GetWebhookServer().WebhookMux == nil { return false diff --git a/pkg/builder/webhook_test.go b/pkg/builder/webhook_test.go index 53fc95468d..80703a38ff 100644 --- a/pkg/builder/webhook_test.go +++ b/pkg/builder/webhook_test.go @@ -134,6 +134,81 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) }) + It("should scaffold a defaulting webhook with a custom defaulter", func() { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testDefaulterGVK.GroupVersion()} + builder.Register(&TestDefaulter{}, &TestDefaulterList{}) + err = builder.AddToScheme(m.GetScheme()) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + + err = WebhookManagedBy(m). + WithDefaulter(&TestCustomDefaulter{}). + For(&TestDefaulter{}). + Complete() + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + svr := m.GetWebhookServer() + ExpectWithOffset(1, svr).NotTo(BeNil()) + + reader := strings.NewReader(`{ + "kind":"AdmissionReview", + "apiVersion":"admission.k8s.io/` + admissionReviewVersion + `", + "request":{ + "uid":"07e52e8d-4513-11e9-a716-42010a800270", + "kind":{ + "group":"", + "version":"v1", + "kind":"TestDefaulter" + }, + "resource":{ + "group":"", + "version":"v1", + "resource":"testdefaulter" + }, + "namespace":"default", + "operation":"CREATE", + "object":{ + "replica":1 + }, + "oldObject":null + } +}`) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + // TODO: we may want to improve it to make it be able to inject dependencies, + // but not always try to load certs and return not found error. + err = svr.Start(ctx) + if err != nil && !os.IsNotExist(err) { + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + } + + By("sending a request to a mutating webhook path") + path := generateMutatePath(testDefaulterGVK) + req := httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader) + req.Header.Add("Content-Type", "application/json") + w := httptest.NewRecorder() + svr.WebhookMux.ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable fields") + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":true`)) + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"patch":`)) + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`)) + + By("sending a request to a validating webhook path that doesn't exist") + path = generateValidatePath(testDefaulterGVK) + _, err = reader.Seek(0, 0) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + req = httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader) + req.Header.Add("Content-Type", "application/json") + w = httptest.NewRecorder() + svr.WebhookMux.ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) + }) + It("should scaffold a validating webhook if the type implements the Validator interface", func() { By("creating a controller manager") m, err := manager.New(cfg, manager.Options{}) @@ -209,6 +284,82 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`)) }) + It("should scaffold a validating webhook with a custom validator", func() { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + + By("registering the type in the Scheme") + builder := scheme.Builder{GroupVersion: testValidatorGVK.GroupVersion()} + builder.Register(&TestValidator{}, &TestValidatorList{}) + err = builder.AddToScheme(m.GetScheme()) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + + err = WebhookManagedBy(m). + WithValidator(&TestCustomValidator{}). + For(&TestValidator{}). + Complete() + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + svr := m.GetWebhookServer() + ExpectWithOffset(1, svr).NotTo(BeNil()) + + reader := strings.NewReader(`{ + "kind":"AdmissionReview", + "apiVersion":"admission.k8s.io/` + admissionReviewVersion + `", + "request":{ + "uid":"07e52e8d-4513-11e9-a716-42010a800270", + "kind":{ + "group":"", + "version":"v1", + "kind":"TestValidator" + }, + "resource":{ + "group":"", + "version":"v1", + "resource":"testvalidator" + }, + "namespace":"default", + "operation":"UPDATE", + "object":{ + "replica":1 + }, + "oldObject":{ + "replica":2 + } + } +}`) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + // TODO: we may want to improve it to make it be able to inject dependencies, + // but not always try to load certs and return not found error. + err = svr.Start(ctx) + if err != nil && !os.IsNotExist(err) { + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + } + + By("sending a request to a mutating webhook path that doesn't exist") + path := generateMutatePath(testValidatorGVK) + req := httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader) + req.Header.Add("Content-Type", "application/json") + w := httptest.NewRecorder() + svr.WebhookMux.ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) + + By("sending a request to a validating webhook path") + path = generateValidatePath(testValidatorGVK) + _, err = reader.Seek(0, 0) + ExpectWithOffset(1, err).NotTo(HaveOccurred()) + req = httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader) + req.Header.Add("Content-Type", "application/json") + w = httptest.NewRecorder() + svr.WebhookMux.ServeHTTP(w, req) + ExpectWithOffset(1, w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable field") + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`)) + ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`)) + }) + It("should scaffold defaulting and validating webhooks if the type implements both Defaulter and Validator interfaces", func() { By("creating a controller manager") m, err := manager.New(cfg, manager.Options{}) @@ -537,3 +688,51 @@ func (dv *TestDefaultValidator) ValidateDelete() error { } return nil } + +// TestCustomDefaulter. + +type TestCustomDefaulter struct{} + +func (*TestCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error { + d := obj.(*TestDefaulter) //nolint:ifshort + if d.Replica < 2 { + d.Replica = 2 + } + return nil +} + +var _ admission.CustomDefaulter = &TestCustomDefaulter{} + +// TestCustomValidator. + +type TestCustomValidator struct{} + +func (*TestCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) error { + v := obj.(*TestValidator) //nolint:ifshort + if v.Replica < 0 { + return errors.New("number of replica should be greater than or equal to 0") + } + return nil +} + +func (*TestCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) error { + v := newObj.(*TestValidator) + old := oldObj.(*TestValidator) //nolint:ifshort + if v.Replica < 0 { + return errors.New("number of replica should be greater than or equal to 0") + } + if v.Replica < old.Replica { + return fmt.Errorf("new replica %v should not be fewer than old replica %v", v.Replica, old.Replica) + } + return nil +} + +func (*TestCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) error { + v := obj.(*TestValidator) //nolint:ifshort + if v.Replica > 0 { + return errors.New("number of replica should be less than or equal to 0 to delete") + } + return nil +} + +var _ admission.CustomValidator = &TestCustomValidator{} diff --git a/pkg/webhook/admission/defaulter_custom.go b/pkg/webhook/admission/defaulter_custom.go new file mode 100644 index 0000000000..a012784e43 --- /dev/null +++ b/pkg/webhook/admission/defaulter_custom.go @@ -0,0 +1,85 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package admission + +import ( + "context" + "encoding/json" + + "errors" + "net/http" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" +) + +// CustomDefaulter defines functions for setting defaults on resources. +type CustomDefaulter interface { + Default(ctx context.Context, obj runtime.Object) error +} + +// WithCustomDefaulter creates a new Webhook for a CustomDefaulter interface. +func WithCustomDefaulter(obj runtime.Object, defaulter CustomDefaulter) *Webhook { + return &Webhook{ + Handler: &defaulterForType{object: obj, defaulter: defaulter}, + } +} + +type defaulterForType struct { + defaulter CustomDefaulter + object runtime.Object + decoder *Decoder +} + +var _ DecoderInjector = &defaulterForType{} + +func (h *defaulterForType) InjectDecoder(d *Decoder) error { + h.decoder = d + return nil +} + +// Handle handles admission requests. +func (h *defaulterForType) Handle(ctx context.Context, req Request) Response { + if h.defaulter == nil { + panic("defaulter should never be nil") + } + if h.object == nil { + panic("object should never be nil") + } + + // Get the object in the request + obj := h.object.DeepCopyObject() + if err := h.decoder.Decode(req, obj); err != nil { + return Errored(http.StatusBadRequest, err) + } + + // Default the object + if err := h.defaulter.Default(ctx, obj); err != nil { + var apiStatus apierrors.APIStatus + if errors.As(err, &apiStatus) { + return validationResponseFromStatus(false, apiStatus.Status()) + } + return Denied(err.Error()) + } + + // Create the patch + marshalled, err := json.Marshal(obj) + if err != nil { + return Errored(http.StatusInternalServerError, err) + } + return PatchResponseFromRaw(req.Object.Raw, marshalled) +} diff --git a/pkg/webhook/admission/validator_custom.go b/pkg/webhook/admission/validator_custom.go new file mode 100644 index 0000000000..38d5565111 --- /dev/null +++ b/pkg/webhook/admission/validator_custom.go @@ -0,0 +1,111 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package admission + +import ( + "context" + "errors" + "fmt" + "net/http" + + v1 "k8s.io/api/admission/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" +) + +// CustomValidator defines functions for validating an operation. +type CustomValidator interface { + ValidateCreate(ctx context.Context, obj runtime.Object) error + ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) error + ValidateDelete(ctx context.Context, obj runtime.Object) error +} + +// WithCustomValidator creates a new Webhook for validating the provided type. +func WithCustomValidator(obj runtime.Object, validator CustomValidator) *Webhook { + return &Webhook{ + Handler: &validatorForType{object: obj, validator: validator}, + } +} + +type validatorForType struct { + validator CustomValidator + object runtime.Object + decoder *Decoder +} + +var _ DecoderInjector = &validatorForType{} + +// InjectDecoder injects the decoder into a validatingHandler. +func (h *validatorForType) InjectDecoder(d *Decoder) error { + h.decoder = d + return nil +} + +// Handle handles admission requests. +func (h *validatorForType) Handle(ctx context.Context, req Request) Response { + if h.validator == nil { + panic("validator should never be nil") + } + if h.object == nil { + panic("object should never be nil") + } + + // Get the object in the request + obj := h.object.DeepCopyObject() + + var err error + switch req.Operation { + case v1.Create: + if err := h.decoder.Decode(req, obj); err != nil { + return Errored(http.StatusBadRequest, err) + } + + err = h.validator.ValidateCreate(ctx, obj) + case v1.Update: + oldObj := obj.DeepCopyObject() + if err := h.decoder.DecodeRaw(req.Object, obj); err != nil { + return Errored(http.StatusBadRequest, err) + } + if err := h.decoder.DecodeRaw(req.OldObject, oldObj); err != nil { + return Errored(http.StatusBadRequest, err) + } + + err = h.validator.ValidateUpdate(ctx, oldObj, obj) + case v1.Delete: + // In reference to PR: https://github.com/kubernetes/kubernetes/pull/76346 + // OldObject contains the object being deleted + if err := h.decoder.DecodeRaw(req.OldObject, obj); err != nil { + return Errored(http.StatusBadRequest, err) + } + + err = h.validator.ValidateDelete(ctx, obj) + default: + return Errored(http.StatusBadRequest, fmt.Errorf("unknown operation request %q", req.Operation)) + } + + // Check the error message first. + if err != nil { + var apiStatus apierrors.APIStatus + if errors.As(err, &apiStatus) { + return validationResponseFromStatus(false, apiStatus.Status()) + } + return Denied(err.Error()) + } + + // Return allowed if everything succeeded. + return Allowed("") +} diff --git a/pkg/webhook/alias.go b/pkg/webhook/alias.go index 1a831016af..293137db49 100644 --- a/pkg/webhook/alias.go +++ b/pkg/webhook/alias.go @@ -29,6 +29,12 @@ type Defaulter = admission.Defaulter // Validator defines functions for validating an operation. type Validator = admission.Validator +// CustomDefaulter defines functions for setting defaults on resources. +type CustomDefaulter = admission.CustomDefaulter + +// CustomValidator defines functions for validating an operation. +type CustomValidator = admission.CustomValidator + // AdmissionRequest defines the input for an admission handler. // It contains information to identify the object in // question (group, version, kind, resource, subresource,