From ad9b00b9e5bccb03ce7f0745db9fa64be88690f0 Mon Sep 17 00:00:00 2001 From: Nishchay Date: Wed, 17 Jul 2019 18:40:17 -0700 Subject: [PATCH] :warning: support delete validation in validator interface - Currently validator interface supports create and update - Add support for delete as well, which could be a breaking change - Users need to implement ValidateDelete since interface is updated --- examples/crd/pkg/resource.go | 10 +++ pkg/builder/webhook.go | 58 ++++++++------- pkg/builder/webhook_test.go | 111 +++++++++++++++++++++++++++++ pkg/webhook/admission/validator.go | 15 ++++ 4 files changed, 168 insertions(+), 26 deletions(-) diff --git a/examples/crd/pkg/resource.go b/examples/crd/pkg/resource.go index 6637303f5b..9c3d4c72bc 100644 --- a/examples/crd/pkg/resource.go +++ b/examples/crd/pkg/resource.go @@ -93,6 +93,16 @@ func (c *ChaosPod) ValidateUpdate(old runtime.Object) error { return nil } +// ValidateDelete implements webhookutil.validator so a webhook will be registered for the type +func (c *ChaosPod) ValidateDelete() error { + log.Info("validate delete", "name", c.Name) + + if c.Spec.NextStop.Before(&metav1.Time{Time: time.Now()}) { + return fmt.Errorf(".spec.nextStop must be later than current time") + } + return nil +} + // +kubebuilder:webhook:path=/mutate-chaosapps-metamagical-io-v1-chaospod,mutating=true,failurePolicy=fail,groups=chaosapps.metamagical.io,resources=chaospods,verbs=create;update,versions=v1,name=mchaospod.kb.io var _ webhook.Defaulter = &ChaosPod{} diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go index 5208a1414a..89fb62c71b 100644 --- a/pkg/builder/webhook.go +++ b/pkg/builder/webhook.go @@ -96,37 +96,43 @@ func (blder *WebhookBuilder) registerWebhooks() error { // registerDefaultingWebhook registers a defaulting webhook if th func (blder *WebhookBuilder) registerDefaultingWebhook() { - if defaulter, isDefaulter := blder.apiType.(admission.Defaulter); isDefaulter { - mwh := admission.DefaultingWebhookFor(defaulter) - if mwh != nil { - path := generateMutatePath(blder.gvk) - - // Checking if the path is already registered. - // If so, just skip it. - if !blder.isAlreadyHandled(path) { - log.Info("Registering a mutating webhook", - "GVK", blder.gvk, - "path", path) - blder.mgr.GetWebhookServer().Register(path, mwh) - } + 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) + if mwh != nil { + path := generateMutatePath(blder.gvk) + + // Checking if the path is already registered. + // If so, just skip it. + if !blder.isAlreadyHandled(path) { + log.Info("Registering a mutating webhook", + "GVK", blder.gvk, + "path", path) + blder.mgr.GetWebhookServer().Register(path, mwh) } } } func (blder *WebhookBuilder) registerValidatingWebhook() { - if validator, isValidator := blder.apiType.(admission.Validator); isValidator { - vwh := admission.ValidatingWebhookFor(validator) - if vwh != nil { - path := generateValidatePath(blder.gvk) - - // Checking if the path is already registered. - // If so, just skip it. - if !blder.isAlreadyHandled(path) { - log.Info("Registering a validating webhook", - "GVK", blder.gvk, - "path", path) - blder.mgr.GetWebhookServer().Register(path, vwh) - } + 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 + } + vwh := admission.ValidatingWebhookFor(validator) + if vwh != nil { + path := generateValidatePath(blder.gvk) + + // Checking if the path is already registered. + // If so, just skip it. + if !blder.isAlreadyHandled(path) { + log.Info("Registering a validating webhook", + "GVK", blder.gvk, + "path", path) + blder.mgr.GetWebhookServer().Register(path, vwh) } } } diff --git a/pkg/builder/webhook_test.go b/pkg/builder/webhook_test.go index 79f0e79d90..47dd101ce1 100644 --- a/pkg/builder/webhook_test.go +++ b/pkg/builder/webhook_test.go @@ -269,6 +269,103 @@ var _ = Describe("application", func() { Expect(w.Body).To(ContainSubstring(`"allowed":true`)) Expect(w.Body).To(ContainSubstring(`"code":200`)) }) + + It("should scaffold a validating webhook if the type implements the Validator interface to validate deletes", func() { + By("creating a controller manager") + m, err := manager.New(cfg, manager.Options{}) + Expect(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()) + Expect(err).NotTo(HaveOccurred()) + + err = WebhookManagedBy(m). + For(&TestValidator{}). + Complete() + Expect(err).NotTo(HaveOccurred()) + svr := m.GetWebhookServer() + Expect(svr).NotTo(BeNil()) + + reader := strings.NewReader(`{ + "kind":"AdmissionReview", + "apiVersion":"admission.k8s.io/v1beta1", + "request":{ + "uid":"07e52e8d-4513-11e9-a716-42010a800270", + "kind":{ + "group":"", + "version":"v1", + "kind":"TestValidator" + }, + "resource":{ + "group":"", + "version":"v1", + "resource":"testvalidator" + }, + "namespace":"default", + "operation":"DELETE", + "object":null, + "oldObject":{ + "replica":1 + } + } +}`) + stopCh := make(chan struct{}) + close(stopCh) + // 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(stopCh) + if err != nil && !os.IsNotExist(err) { + Expect(err).NotTo(HaveOccurred()) + } + + By("sending a request to a validating webhook path to check for failed delete") + path := generateValidatePath(testValidatorGVK) + req := httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader) + req.Header.Add(http.CanonicalHeaderKey("Content-Type"), "application/json") + w := httptest.NewRecorder() + svr.WebhookMux.ServeHTTP(w, req) + Expect(w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable field") + Expect(w.Body).To(ContainSubstring(`"allowed":false`)) + Expect(w.Body).To(ContainSubstring(`"code":403`)) + + reader = strings.NewReader(`{ + "kind":"AdmissionReview", + "apiVersion":"admission.k8s.io/v1beta1", + "request":{ + "uid":"07e52e8d-4513-11e9-a716-42010a800270", + "kind":{ + "group":"", + "version":"v1", + "kind":"TestValidator" + }, + "resource":{ + "group":"", + "version":"v1", + "resource":"testvalidator" + }, + "namespace":"default", + "operation":"DELETE", + "object":null, + "oldObject":{ + "replica":0 + } + } +}`) + By("sending a request to a validating webhook path with correct request") + path = generateValidatePath(testValidatorGVK) + req = httptest.NewRequest("POST", "http://svc-name.svc-ns.svc"+path, reader) + req.Header.Add(http.CanonicalHeaderKey("Content-Type"), "application/json") + w = httptest.NewRecorder() + svr.WebhookMux.ServeHTTP(w, req) + Expect(w.Code).To(Equal(http.StatusOK)) + By("sanity checking the response contains reasonable field") + Expect(w.Body).To(ContainSubstring(`"allowed":true`)) + Expect(w.Body).To(ContainSubstring(`"code":200`)) + + }) }) }) @@ -357,6 +454,13 @@ func (v *TestValidator) ValidateUpdate(old runtime.Object) error { return nil } +func (v *TestValidator) ValidateDelete() error { + if v.Replica > 0 { + return errors.New("number of replica should be less than or equal to 0 to delete") + } + return nil +} + // TestDefaultValidator var _ runtime.Object = &TestDefaultValidator{} @@ -407,3 +511,10 @@ func (dv *TestDefaultValidator) ValidateUpdate(old runtime.Object) error { } return nil } + +func (dv *TestDefaultValidator) ValidateDelete() error { + if dv.Replica > 0 { + return errors.New("number of replica should be less than or equal to 0 to delete") + } + return nil +} diff --git a/pkg/webhook/admission/validator.go b/pkg/webhook/admission/validator.go index 282bb9b47c..6defcfe92f 100644 --- a/pkg/webhook/admission/validator.go +++ b/pkg/webhook/admission/validator.go @@ -29,6 +29,7 @@ type Validator interface { runtime.Object ValidateCreate() error ValidateUpdate(old runtime.Object) error + ValidateDelete() error } // ValidatingWebhookFor creates a new Webhook for validating the provided type. @@ -89,5 +90,19 @@ func (h *validatingHandler) Handle(ctx context.Context, req Request) Response { } } + if req.Operation == v1beta1.Delete { + // In reference to PR: https://github.com/kubernetes/kubernetes/pull/76346 + // OldObject contains the object being deleted + err := h.decoder.DecodeRaw(req.OldObject, obj) + if err != nil { + return Errored(http.StatusBadRequest, err) + } + + err = obj.ValidateDelete() + if err != nil { + return Denied(err.Error()) + } + } + return Allowed("") }