diff --git a/pkg/builder/webhook.go b/pkg/builder/webhook.go index 81d8f74056..cfb9f1a69d 100644 --- a/pkg/builder/webhook.go +++ b/pkg/builder/webhook.go @@ -176,16 +176,6 @@ func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook { } return w } - if defaulter, ok := blder.apiType.(admission.Defaulter); ok { - w := admission.DefaultingWebhookFor(blder.mgr.GetScheme(), defaulter) - if blder.recoverPanic != nil { - w = w.WithRecoverPanic(*blder.recoverPanic) - } - return w - } - log.Info( - "skip registering a mutating webhook, object does not implement admission.Defaulter or WithDefaulter wasn't called", - "GVK", blder.gvk) return nil } @@ -215,16 +205,6 @@ func (blder *WebhookBuilder) getValidatingWebhook() *admission.Webhook { } return w } - if validator, ok := blder.apiType.(admission.Validator); ok { - w := admission.ValidatingWebhookFor(blder.mgr.GetScheme(), validator) - if blder.recoverPanic != nil { - w = w.WithRecoverPanic(*blder.recoverPanic) - } - return w - } - log.Info( - "skip registering a validating webhook, object does not implement admission.Validator or WithValidator wasn't called", - "GVK", blder.gvk) return nil } diff --git a/pkg/builder/webhook_test.go b/pkg/builder/webhook_test.go index 4574d5cc77..106825b2d1 100644 --- a/pkg/builder/webhook_test.go +++ b/pkg/builder/webhook_test.go @@ -77,7 +77,7 @@ func runTests(admissionReviewVersion string) { close(stop) }) - It("should scaffold a defaulting webhook if the type implements the Defaulter interface", func() { + It("should scaffold a custom defaulting webhook if the type implements the CustomDefaulter interface", func() { By("creating a controller manager") m, err := manager.New(cfg, manager.Options{}) ExpectWithOffset(1, err).NotTo(HaveOccurred()) @@ -90,6 +90,7 @@ func runTests(admissionReviewVersion string) { err = WebhookManagedBy(m). For(&TestDefaulter{}). + WithDefaulter(&TestCustomDefaulter{}). Complete() ExpectWithOffset(1, err).NotTo(HaveOccurred()) svr := m.GetWebhookServer() @@ -147,7 +148,7 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) }) - It("should scaffold a defaulting webhook which recovers from panics", func() { + It("should scaffold a custom defaulting webhook which recovers from panics", func() { By("creating a controller manager") m, err := manager.New(cfg, manager.Options{}) ExpectWithOffset(1, err).NotTo(HaveOccurred()) @@ -159,7 +160,9 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) err = WebhookManagedBy(m). - For(&TestDefaulter{Panic: true}). + For(&TestDefaulter{}). + WithDefaulter(&TestCustomDefaulter{}). + RecoverPanic(true). // RecoverPanic defaults to true. Complete() ExpectWithOffset(1, err).NotTo(HaveOccurred()) @@ -285,7 +288,7 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, w.Code).To(Equal(http.StatusNotFound)) }) - It("should scaffold a validating webhook if the type implements the Validator interface", func() { + It("should scaffold a custom validating webhook if the type implements the CustomValidator interface", func() { By("creating a controller manager") m, err := manager.New(cfg, manager.Options{}) ExpectWithOffset(1, err).NotTo(HaveOccurred()) @@ -298,6 +301,7 @@ func runTests(admissionReviewVersion string) { err = WebhookManagedBy(m). For(&TestValidator{}). + WithValidator(&TestCustomValidator{}). Complete() ExpectWithOffset(1, err).NotTo(HaveOccurred()) svr := m.GetWebhookServer() @@ -356,7 +360,7 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`)) }) - It("should scaffold a validating webhook which recovers from panics", func() { + It("should scaffold a custom validating webhook which recovers from panics", func() { By("creating a controller manager") m, err := manager.New(cfg, manager.Options{}) ExpectWithOffset(1, err).NotTo(HaveOccurred()) @@ -368,7 +372,8 @@ func runTests(admissionReviewVersion string) { ExpectWithOffset(1, err).NotTo(HaveOccurred()) err = WebhookManagedBy(m). - For(&TestValidator{Panic: true}). + For(&TestValidator{}). + WithValidator(&TestCustomValidator{}). RecoverPanic(true). Complete() ExpectWithOffset(1, err).NotTo(HaveOccurred()) @@ -496,80 +501,7 @@ func runTests(admissionReviewVersion string) { EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`)) }) - 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{}) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - - By("registering the type in the Scheme") - builder := scheme.Builder{GroupVersion: testDefaultValidatorGVK.GroupVersion()} - builder.Register(&TestDefaultValidator{}, &TestDefaultValidatorList{}) - err = builder.AddToScheme(m.GetScheme()) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - - err = WebhookManagedBy(m). - For(&TestDefaultValidator{}). - Complete() - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - svr := m.GetWebhookServer() - ExpectWithOffset(1, svr).NotTo(BeNil()) - - reader := strings.NewReader(admissionReviewGV + admissionReviewVersion + `", - "request":{ - "uid":"07e52e8d-4513-11e9-a716-42010a800270", - "kind":{ - "group":"", - "version":"v1", - "kind":"TestDefaultValidator" - }, - "resource":{ - "group":"", - "version":"v1", - "resource":"testdefaultvalidator" - }, - "namespace":"default", - "operation":"CREATE", - "object":{ - "replica":1 - }, - "oldObject":null - } -}`) - - ctx, cancel := context.WithCancel(context.Background()) - cancel() - 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(testDefaultValidatorGVK) - req := httptest.NewRequest("POST", svcBaseAddr+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":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") - path = generateValidatePath(testDefaultValidatorGVK) - _, err = reader.Seek(0, 0) - ExpectWithOffset(1, err).NotTo(HaveOccurred()) - req = httptest.NewRequest("POST", svcBaseAddr+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":true`)) - ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":200`)) - }) - - It("should scaffold a validating webhook if the type implements the Validator interface to validate deletes", func() { + It("should scaffold a custom validating webhook if the type implements the CustomValidator interface to validate deletes", func() { By("creating a controller manager") ctx, cancel := context.WithCancel(context.Background()) @@ -584,6 +516,7 @@ func runTests(admissionReviewVersion string) { err = WebhookManagedBy(m). For(&TestValidator{}). + WithValidator(&TestCustomValidator{}). Complete() ExpectWithOffset(1, err).NotTo(HaveOccurred()) svr := m.GetWebhookServer() @@ -712,15 +645,6 @@ type TestDefaulterList struct{} func (*TestDefaulterList) GetObjectKind() schema.ObjectKind { return nil } func (*TestDefaulterList) DeepCopyObject() runtime.Object { return nil } -func (d *TestDefaulter) Default() { - if d.Panic { - panic("fake panic test") - } - if d.Replica < 2 { - d.Replica = 2 - } -} - // TestValidator. var _ runtime.Object = &TestValidator{} @@ -753,43 +677,6 @@ type TestValidatorList struct{} func (*TestValidatorList) GetObjectKind() schema.ObjectKind { return nil } func (*TestValidatorList) DeepCopyObject() runtime.Object { return nil } -var _ admission.Validator = &TestValidator{} - -func (v *TestValidator) ValidateCreate() (admission.Warnings, error) { - if v.Panic { - panic("fake panic test") - } - if v.Replica < 0 { - return nil, errors.New("number of replica should be greater than or equal to 0") - } - return nil, nil -} - -func (v *TestValidator) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { - if v.Panic { - panic("fake panic test") - } - if v.Replica < 0 { - return nil, errors.New("number of replica should be greater than or equal to 0") - } - if oldObj, ok := old.(*TestValidator); !ok { - return nil, fmt.Errorf("the old object is expected to be %T", oldObj) - } else if v.Replica < oldObj.Replica { - return nil, fmt.Errorf("new replica %v should not be fewer than old replica %v", v.Replica, oldObj.Replica) - } - return nil, nil -} - -func (v *TestValidator) ValidateDelete() (admission.Warnings, error) { - if v.Panic { - panic("fake panic test") - } - if v.Replica > 0 { - return nil, errors.New("number of replica should be less than or equal to 0 to delete") - } - return nil, nil -} - // TestDefaultValidator. var _ runtime.Object = &TestDefaultValidator{} @@ -822,37 +709,7 @@ type TestDefaultValidatorList struct{} func (*TestDefaultValidatorList) GetObjectKind() schema.ObjectKind { return nil } func (*TestDefaultValidatorList) DeepCopyObject() runtime.Object { return nil } -func (dv *TestDefaultValidator) Default() { - if dv.Replica < 2 { - dv.Replica = 2 - } -} - -var _ admission.Validator = &TestDefaultValidator{} - -func (dv *TestDefaultValidator) ValidateCreate() (admission.Warnings, error) { - if dv.Replica < 0 { - return nil, errors.New("number of replica should be greater than or equal to 0") - } - return nil, nil -} - -func (dv *TestDefaultValidator) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { - if dv.Replica < 0 { - return nil, errors.New("number of replica should be greater than or equal to 0") - } - return nil, nil -} - -func (dv *TestDefaultValidator) ValidateDelete() (admission.Warnings, error) { - if dv.Replica > 0 { - return nil, errors.New("number of replica should be less than or equal to 0 to delete") - } - return nil, nil -} - // TestCustomDefaulter. - type TestCustomDefaulter struct{} func (*TestCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error { @@ -866,6 +723,10 @@ func (*TestCustomDefaulter) Default(ctx context.Context, obj runtime.Object) err } d := obj.(*TestDefaulter) //nolint:ifshort + if d.Panic { + panic("fake panic test") + } + if d.Replica < 2 { d.Replica = 2 } @@ -889,6 +750,9 @@ func (*TestCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Obje } v := obj.(*TestValidator) //nolint:ifshort + if v.Panic { + panic("fake panic test") + } if v.Replica < 0 { return nil, errors.New("number of replica should be greater than or equal to 0") } diff --git a/pkg/webhook/admission/defaulter.go b/pkg/webhook/admission/defaulter.go deleted file mode 100644 index efbbf60282..0000000000 --- a/pkg/webhook/admission/defaulter.go +++ /dev/null @@ -1,84 +0,0 @@ -/* -Copyright 2018 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" - "net/http" - - admissionv1 "k8s.io/api/admission/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" -) - -// Defaulter defines functions for setting defaults on resources. -// Deprecated: Ue CustomDefaulter instead. -type Defaulter interface { - runtime.Object - Default() -} - -// DefaultingWebhookFor creates a new Webhook for Defaulting the provided type. -// Deprecated: Use WithCustomDefaulter instead. -func DefaultingWebhookFor(scheme *runtime.Scheme, defaulter Defaulter) *Webhook { - return &Webhook{ - Handler: &mutatingHandler{defaulter: defaulter, decoder: NewDecoder(scheme)}, - } -} - -type mutatingHandler struct { - defaulter Defaulter - decoder Decoder -} - -// Handle handles admission requests. -func (h *mutatingHandler) Handle(ctx context.Context, req Request) Response { - if h.decoder == nil { - panic("decoder should never be nil") - } - if h.defaulter == nil { - panic("defaulter should never be nil") - } - - // always skip when a DELETE operation received in mutation handler - // describe in https://github.com/kubernetes-sigs/controller-runtime/issues/1762 - if req.Operation == admissionv1.Delete { - return Response{AdmissionResponse: admissionv1.AdmissionResponse{ - Allowed: true, - Result: &metav1.Status{ - Code: http.StatusOK, - }, - }} - } - - // Get the object in the request - obj := h.defaulter.DeepCopyObject().(Defaulter) - if err := h.decoder.Decode(req, obj); err != nil { - return Errored(http.StatusBadRequest, err) - } - - // Default the object - obj.Default() - marshalled, err := json.Marshal(obj) - if err != nil { - return Errored(http.StatusInternalServerError, err) - } - - // Create the patch - return PatchResponseFromRaw(req.Object.Raw, marshalled) -} diff --git a/pkg/webhook/admission/defaulter_test.go b/pkg/webhook/admission/defaulter_custom_test.go similarity index 63% rename from pkg/webhook/admission/defaulter_test.go rename to pkg/webhook/admission/defaulter_custom_test.go index cf7571663c..f1063ffe32 100644 --- a/pkg/webhook/admission/defaulter_test.go +++ b/pkg/webhook/admission/defaulter_custom_test.go @@ -1,3 +1,17 @@ +/* +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 ( @@ -16,8 +30,7 @@ var _ = Describe("Defaulter Handler", func() { It("should return ok if received delete verb in defaulter handler", func() { obj := &TestDefaulter{} - handler := DefaultingWebhookFor(admissionScheme, obj) - + handler := WithCustomDefaulter(admissionScheme, obj, &TestCustomDefaulter{}) resp := handler.Handle(context.TODO(), Request{ AdmissionRequest: admissionv1.AdmissionRequest{ Operation: admissionv1.Delete, @@ -29,7 +42,6 @@ var _ = Describe("Defaulter Handler", func() { Expect(resp.Allowed).Should(BeTrue()) Expect(resp.Result.Code).Should(Equal(int32(http.StatusOK))) }) - }) // TestDefaulter. @@ -61,8 +73,13 @@ type TestDefaulterList struct{} func (*TestDefaulterList) GetObjectKind() schema.ObjectKind { return nil } func (*TestDefaulterList) DeepCopyObject() runtime.Object { return nil } -func (d *TestDefaulter) Default() { - if d.Replica < 2 { - d.Replica = 2 +// TestCustomDefaulter +type TestCustomDefaulter struct{} + +func (d *TestCustomDefaulter) Default(ctx context.Context, obj runtime.Object) error { + o := obj.(*TestDefaulter) + if o.Replica < 2 { + o.Replica = 2 } + return nil } diff --git a/pkg/webhook/admission/validator.go b/pkg/webhook/admission/validator.go deleted file mode 100644 index b28a56eef8..0000000000 --- a/pkg/webhook/admission/validator.go +++ /dev/null @@ -1,127 +0,0 @@ -/* -Copyright 2018 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" -) - -// Warnings represents warning messages. -type Warnings []string - -// Validator defines functions for validating an operation. -// The custom resource kind which implements this interface can validate itself. -// To validate the custom resource with another specific struct, use CustomValidator instead. -// Deprecated: Use CustomValidator instead. -type Validator interface { - runtime.Object - - // ValidateCreate validates the object on creation. - // The optional warnings will be added to the response as warning messages. - // Return an error if the object is invalid. - ValidateCreate() (warnings Warnings, err error) - - // ValidateUpdate validates the object on update. The oldObj is the object before the update. - // The optional warnings will be added to the response as warning messages. - // Return an error if the object is invalid. - ValidateUpdate(old runtime.Object) (warnings Warnings, err error) - - // ValidateDelete validates the object on deletion. - // The optional warnings will be added to the response as warning messages. - // Return an error if the object is invalid. - ValidateDelete() (warnings Warnings, err error) -} - -// ValidatingWebhookFor creates a new Webhook for validating the provided type. -// Deprecated: Use WithCustomValidator instead. -func ValidatingWebhookFor(scheme *runtime.Scheme, validator Validator) *Webhook { - return &Webhook{ - Handler: &validatingHandler{validator: validator, decoder: NewDecoder(scheme)}, - } -} - -type validatingHandler struct { - validator Validator - decoder Decoder -} - -// Handle handles admission requests. -func (h *validatingHandler) Handle(ctx context.Context, req Request) Response { - if h.decoder == nil { - panic("decoder should never be nil") - } - if h.validator == nil { - panic("validator should never be nil") - } - // Get the object in the request - obj := h.validator.DeepCopyObject().(Validator) - - var err error - var warnings []string - - switch req.Operation { - case v1.Connect: - // No validation for connect requests. - // TODO(vincepri): Should we validate CONNECT requests? In what cases? - case v1.Create: - if err = h.decoder.Decode(req, obj); err != nil { - return Errored(http.StatusBadRequest, err) - } - - warnings, err = obj.ValidateCreate() - case v1.Update: - oldObj := obj.DeepCopyObject() - - err = h.decoder.DecodeRaw(req.Object, obj) - if err != nil { - return Errored(http.StatusBadRequest, err) - } - err = h.decoder.DecodeRaw(req.OldObject, oldObj) - if err != nil { - return Errored(http.StatusBadRequest, err) - } - - warnings, err = obj.ValidateUpdate(oldObj) - case v1.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) - } - - warnings, err = obj.ValidateDelete() - default: - return Errored(http.StatusBadRequest, fmt.Errorf("unknown operation %q", req.Operation)) - } - - if err != nil { - var apiStatus apierrors.APIStatus - if errors.As(err, &apiStatus) { - return validationResponseFromStatus(false, apiStatus.Status()).WithWarnings(warnings...) - } - return Denied(err.Error()).WithWarnings(warnings...) - } - return Allowed("").WithWarnings(warnings...) -} diff --git a/pkg/webhook/admission/validator_custom.go b/pkg/webhook/admission/validator_custom.go index b8f194401e..ef1be52a8f 100644 --- a/pkg/webhook/admission/validator_custom.go +++ b/pkg/webhook/admission/validator_custom.go @@ -27,6 +27,9 @@ import ( "k8s.io/apimachinery/pkg/runtime" ) +// Warnings represents warning messages. +type Warnings []string + // CustomValidator defines functions for validating an operation. // The object to be validated is passed into methods as a parameter. type CustomValidator interface { diff --git a/pkg/webhook/admission/validator_test.go b/pkg/webhook/admission/validator_custom_test.go similarity index 85% rename from pkg/webhook/admission/validator_test.go rename to pkg/webhook/admission/validator_custom_test.go index 404fad9016..0e783560a1 100644 --- a/pkg/webhook/admission/validator_test.go +++ b/pkg/webhook/admission/validator_custom_test.go @@ -1,10 +1,8 @@ /* 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 @@ -28,18 +26,16 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/client-go/kubernetes/scheme" ) var fakeValidatorVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "fakeValidator"} -var _ = Describe("validatingHandler", func() { - - decoder := NewDecoder(scheme.Scheme) +var _ = Describe("customValidatingHandler", func() { Context("when dealing with successful results without warning", func() { - f := &fakeValidator{ErrorToReturn: nil, GVKToReturn: fakeValidatorVK, WarningsToReturn: nil} - handler := validatingHandler{validator: f, decoder: decoder} + val := &fakeCustomValidator{ErrorToReturn: nil, GVKToReturn: fakeValidatorVK, WarningsToReturn: nil} + f := &fakeValidator{} + handler := WithCustomValidator(admissionScheme, f, val) It("should return 200 in response when create succeeds", func() { @@ -48,7 +44,7 @@ var _ = Describe("validatingHandler", func() { Operation: admissionv1.Create, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, }, }) @@ -64,11 +60,11 @@ var _ = Describe("validatingHandler", func() { Operation: admissionv1.Update, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, }, }) @@ -83,7 +79,7 @@ var _ = Describe("validatingHandler", func() { Operation: admissionv1.Delete, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, }, }) @@ -95,19 +91,19 @@ var _ = Describe("validatingHandler", func() { const warningMessage = "warning message" const anotherWarningMessage = "another warning message" Context("when dealing with successful results with warning", func() { - f := &fakeValidator{ErrorToReturn: nil, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{ + f := &fakeValidator{} + val := &fakeCustomValidator{ErrorToReturn: nil, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{ warningMessage, anotherWarningMessage, }} - handler := validatingHandler{validator: f, decoder: decoder} - + handler := WithCustomValidator(admissionScheme, f, val) It("should return 200 in response when create succeeds, with warning messages", func() { response := handler.Handle(context.TODO(), Request{ AdmissionRequest: admissionv1.AdmissionRequest{ Operation: admissionv1.Create, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, }, }) @@ -125,11 +121,11 @@ var _ = Describe("validatingHandler", func() { Operation: admissionv1.Update, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, }, }) @@ -145,8 +141,9 @@ var _ = Describe("validatingHandler", func() { AdmissionRequest: admissionv1.AdmissionRequest{ Operation: admissionv1.Delete, OldObject: runtime.RawExtension{ - Raw: []byte("{}"), - Object: handler.validator, + Raw: []byte("{}"), + + Object: f, }, }, }) @@ -165,8 +162,9 @@ var _ = Describe("validatingHandler", func() { Code: http.StatusUnprocessableEntity, }, } - f := &fakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{warningMessage, anotherWarningMessage}} - handler := validatingHandler{validator: f, decoder: decoder} + f := &fakeValidator{} + val := &fakeCustomValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{warningMessage, anotherWarningMessage}} + handler := WithCustomValidator(admissionScheme, f, val) It("should propagate the Status from ValidateCreate's return value to the HTTP response", func() { @@ -175,7 +173,7 @@ var _ = Describe("validatingHandler", func() { Operation: admissionv1.Create, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, }, }) @@ -195,11 +193,12 @@ var _ = Describe("validatingHandler", func() { Operation: admissionv1.Update, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, OldObject: runtime.RawExtension{ - Raw: []byte("{}"), - Object: handler.validator, + Raw: []byte("{}"), + + Object: f, }, }, }) @@ -217,9 +216,10 @@ var _ = Describe("validatingHandler", func() { response := handler.Handle(context.TODO(), Request{ AdmissionRequest: admissionv1.AdmissionRequest{ Operation: admissionv1.Delete, + OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, }, }) @@ -242,17 +242,19 @@ var _ = Describe("validatingHandler", func() { Code: http.StatusUnprocessableEntity, }, } - f := &fakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: nil} - handler := validatingHandler{validator: f, decoder: decoder} + f := &fakeValidator{} + val := &fakeCustomValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: nil} + handler := WithCustomValidator(admissionScheme, f, val) It("should propagate the Status from ValidateCreate's return value to the HTTP response", func() { response := handler.Handle(context.TODO(), Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ Operation: admissionv1.Create, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, }, }) @@ -270,11 +272,11 @@ var _ = Describe("validatingHandler", func() { Operation: admissionv1.Update, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, }, }) @@ -292,13 +294,14 @@ var _ = Describe("validatingHandler", func() { Operation: admissionv1.Delete, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, }, }) Expect(response.Allowed).Should(BeFalse()) Expect(response.Result.Code).Should(Equal(expectedError.Status().Code)) + Expect(*response.Result).Should(Equal(expectedError.Status())) }) @@ -308,17 +311,19 @@ var _ = Describe("validatingHandler", func() { Context("when dealing with non-status errors, without warning messages", func() { expectedError := errors.New("some error") - f := &fakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK} - handler := validatingHandler{validator: f, decoder: decoder} + f := &fakeValidator{} + val := &fakeCustomValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK} + handler := WithCustomValidator(admissionScheme, f, val) It("should return 403 response when ValidateCreate with error message embedded", func() { response := handler.Handle(context.TODO(), Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ Operation: admissionv1.Create, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, }, }) @@ -332,15 +337,16 @@ var _ = Describe("validatingHandler", func() { It("should return 403 response when ValidateUpdate returns non-APIStatus error", func() { response := handler.Handle(context.TODO(), Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ Operation: admissionv1.Update, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, }, }) @@ -357,7 +363,7 @@ var _ = Describe("validatingHandler", func() { Operation: admissionv1.Delete, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, }, }) @@ -371,21 +377,24 @@ var _ = Describe("validatingHandler", func() { Context("when dealing with non-status errors, with warning messages", func() { expectedError := errors.New("some error") - f := &fakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{warningMessage, anotherWarningMessage}} - handler := validatingHandler{validator: f, decoder: decoder} + f := &fakeValidator{} + val := &fakeCustomValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{warningMessage, anotherWarningMessage}} + handler := WithCustomValidator(admissionScheme, f, val) It("should return 403 response when ValidateCreate with error message embedded", func() { response := handler.Handle(context.TODO(), Request{ AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, }, }) Expect(response.Allowed).Should(BeFalse()) + Expect(response.Result.Code).Should(Equal(int32(http.StatusForbidden))) Expect(response.Result.Reason).Should(Equal(metav1.StatusReasonForbidden)) Expect(response.Result.Message).Should(Equal(expectedError.Error())) @@ -400,11 +409,12 @@ var _ = Describe("validatingHandler", func() { Operation: admissionv1.Update, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, OldObject: runtime.RawExtension{ + Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, }, }) @@ -412,6 +422,7 @@ var _ = Describe("validatingHandler", func() { Expect(response.Result.Code).Should(Equal(int32(http.StatusForbidden))) Expect(response.Result.Reason).Should(Equal(metav1.StatusReasonForbidden)) Expect(response.Result.Message).Should(Equal(expectedError.Error())) + Expect(response.AdmissionResponse.Warnings).Should(ContainElement(warningMessage)) Expect(response.AdmissionResponse.Warnings).Should(ContainElement(anotherWarningMessage)) @@ -423,7 +434,7 @@ var _ = Describe("validatingHandler", func() { Operation: admissionv1.Delete, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validator, + Object: f, }, }, }) @@ -447,12 +458,12 @@ var _ = Describe("validatingHandler", func() { }) -// fakeValidator provides fake validating webhook functionality for testing -// It implements the admission.Validator interface and +// fakeCustomValidator provides fake validating webhook functionality for testing +// It implements the admission.CustomValidator interface and // rejects all requests with the same configured error // or passes if ErrorToReturn is nil. // And it would always return configured warning messages WarningsToReturn. -type fakeValidator struct { +type fakeCustomValidator struct { // ErrorToReturn is the error for which the fakeValidator rejects all requests ErrorToReturn error `json:"errorToReturn,omitempty"` // GVKToReturn is the GroupVersionKind that the webhook operates on @@ -461,18 +472,23 @@ type fakeValidator struct { WarningsToReturn []string } -func (v *fakeValidator) ValidateCreate() (warnings Warnings, err error) { +func (v *fakeCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (warnings Warnings, err error) { return v.WarningsToReturn, v.ErrorToReturn } -func (v *fakeValidator) ValidateUpdate(old runtime.Object) (warnings Warnings, err error) { +func (v *fakeCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings Warnings, err error) { return v.WarningsToReturn, v.ErrorToReturn } -func (v *fakeValidator) ValidateDelete() (warnings Warnings, err error) { +func (v *fakeCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (warnings Warnings, err error) { return v.WarningsToReturn, v.ErrorToReturn } +type fakeValidator struct { + // GVKToReturn is the GroupVersionKind that the webhook operates on + GVKToReturn schema.GroupVersionKind +} + func (v *fakeValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) { v.GVKToReturn = gvk } @@ -487,8 +503,6 @@ func (v *fakeValidator) GetObjectKind() schema.ObjectKind { func (v *fakeValidator) DeepCopyObject() runtime.Object { return &fakeValidator{ - ErrorToReturn: v.ErrorToReturn, - GVKToReturn: v.GVKToReturn, - WarningsToReturn: v.WarningsToReturn, + GVKToReturn: v.GVKToReturn, } } diff --git a/pkg/webhook/alias.go b/pkg/webhook/alias.go index e8439e2ea2..2882e7bab3 100644 --- a/pkg/webhook/alias.go +++ b/pkg/webhook/alias.go @@ -23,14 +23,6 @@ import ( // define some aliases for common bits of the webhook functionality -// Defaulter defines functions for setting defaults on resources. -// Deprecated: Use CustomDefaulter instead. -type Defaulter = admission.Defaulter - -// Validator defines functions for validating an operation. -// Deprecated: Use CustomValidator instead. -type Validator = admission.Validator - // CustomDefaulter defines functions for setting defaults on resources. type CustomDefaulter = admission.CustomDefaulter