From 2f57272732add6cf1dbf1ea1787c813b6ef06a7b Mon Sep 17 00:00:00 2001 From: STRRL Date: Thu, 29 Sep 2022 14:41:07 +0800 Subject: [PATCH 01/13] feat: new features about support warning with webhook Signed-off-by: STRRL --- pkg/webhook/admission/admissiontest/util.go | 54 +++ pkg/webhook/admission/validator_warn.go | 123 +++++++ .../admission/validator_warn_custom.go | 112 ++++++ pkg/webhook/admission/validator_warn_test.go | 342 ++++++++++++++++++ pkg/webhook/alias.go | 3 + 5 files changed, 634 insertions(+) create mode 100644 pkg/webhook/admission/validator_warn.go create mode 100644 pkg/webhook/admission/validator_warn_custom.go create mode 100644 pkg/webhook/admission/validator_warn_test.go diff --git a/pkg/webhook/admission/admissiontest/util.go b/pkg/webhook/admission/admissiontest/util.go index 6e35a73907..90394389b3 100644 --- a/pkg/webhook/admission/admissiontest/util.go +++ b/pkg/webhook/admission/admissiontest/util.go @@ -19,8 +19,13 @@ package admissiontest import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" + "sigs.k8s.io/controller-runtime/pkg/webhook" ) +var _ runtime.Object = (*FakeValidator)(nil) +var _ schema.ObjectKind = (*FakeValidator)(nil) +var _ webhook.Validator = (*FakeValidator)(nil) + // FakeValidator provides fake validating webhook functionality for testing // It implements the admission.Validator interface and // rejects all requests with the same configured error @@ -64,3 +69,52 @@ func (v *FakeValidator) GroupVersionKind() schema.GroupVersionKind { func (v *FakeValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) { v.GVKToReturn = gvk } + +var _ runtime.Object = (*FakeValidatorWarn)(nil) +var _ schema.ObjectKind = (*FakeValidatorWarn)(nil) +var _ webhook.ValidatorWarn = (*FakeValidatorWarn)(nil) + +// FakeValidatorWarn provides fake validating webhook functionality for testing +// It implements the admission.ValidatorWarn 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 FakeValidatorWarn struct { + // ErrorToReturn is the error for which the FakeValidatorWarn rejects all requests + ErrorToReturn error `json:"ErrorToReturn,omitempty"` + // GVKToReturn is the GroupVersionKind that the webhook operates on + GVKToReturn schema.GroupVersionKind + // WarningsToReturn is the warnings for FakeValidatorWarn returns to all requests + WarningsToReturn []string +} + +func (v *FakeValidatorWarn) ValidateCreate() (err error, warnings []string) { + return v.ErrorToReturn, v.WarningsToReturn +} + +func (v *FakeValidatorWarn) ValidateUpdate(old runtime.Object) (err error, warnings []string) { + return v.ErrorToReturn, v.WarningsToReturn +} + +func (v *FakeValidatorWarn) ValidateDelete() (err error, warnings []string) { + return v.ErrorToReturn, v.WarningsToReturn +} + +func (v *FakeValidatorWarn) SetGroupVersionKind(kind schema.GroupVersionKind) { + v.GVKToReturn = kind +} + +func (v *FakeValidatorWarn) GroupVersionKind() schema.GroupVersionKind { + return v.GVKToReturn +} + +func (v *FakeValidatorWarn) GetObjectKind() schema.ObjectKind { + return v +} + +func (v *FakeValidatorWarn) DeepCopyObject() runtime.Object { + return &FakeValidatorWarn{ErrorToReturn: v.ErrorToReturn, + GVKToReturn: v.GVKToReturn, + WarningsToReturn: v.WarningsToReturn, + } +} diff --git a/pkg/webhook/admission/validator_warn.go b/pkg/webhook/admission/validator_warn.go new file mode 100644 index 0000000000..21bd92636f --- /dev/null +++ b/pkg/webhook/admission/validator_warn.go @@ -0,0 +1,123 @@ +/* +Copyright 2022 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" + goerrors "errors" + "net/http" + + v1 "k8s.io/api/admission/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" +) + +// ValidatorWarn works like Validator, but it allows to return warnings. +type ValidatorWarn interface { + runtime.Object + ValidateCreate() (err error, warnings []string) + ValidateUpdate(old runtime.Object) (err error, warnings []string) + ValidateDelete() (err error, warnings []string) +} + +func ValidatingWebhookWithWarningFor(validatorWarning ValidatorWarn) *Webhook { + return nil +} + +var _ Handler = &validatingWarnHandler{} +var _ DecoderInjector = &validatingWarnHandler{} + +type validatingWarnHandler struct { + validatorWarn ValidatorWarn + decoder *Decoder +} + +// InjectDecoder injects the decoder into a validatingWarnHandler. +func (h *validatingWarnHandler) InjectDecoder(decoder *Decoder) error { + h.decoder = decoder + return nil +} + +// Handle handles admission requests. +func (h *validatingWarnHandler) Handle(ctx context.Context, req Request) Response { + if h.validatorWarn == nil { + panic("validatorWarn should never be nil") + } + + var allWarnings []string + + // Get the object in the request + obj := h.validatorWarn.DeepCopyObject().(ValidatorWarn) + if req.Operation == v1.Create { + err := h.decoder.Decode(req, obj) + if err != nil { + return Errored(http.StatusBadRequest, err) + } + + err, warnings := obj.ValidateCreate() + allWarnings = append(allWarnings, warnings...) + if err != nil { + var apiStatus apierrors.APIStatus + if goerrors.As(err, &apiStatus) { + return validationResponseFromStatus(false, apiStatus.Status()) + } + return Denied(err.Error()).WithWarnings(allWarnings...) + } + } + + if req.Operation == 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) + } + err, warnings := obj.ValidateUpdate(oldObj) + allWarnings = append(allWarnings, warnings...) + if err != nil { + var apiStatus apierrors.APIStatus + if goerrors.As(err, &apiStatus) { + return validationResponseFromStatus(false, apiStatus.Status()) + } + return Denied(err.Error()).WithWarnings(allWarnings...) + } + } + + if req.Operation == 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) + } + + err, warnings := obj.ValidateDelete() + allWarnings = append(allWarnings, warnings...) + if err != nil { + var apiStatus apierrors.APIStatus + if goerrors.As(err, &apiStatus) { + return validationResponseFromStatus(false, apiStatus.Status()) + } + return Denied(err.Error()).WithWarnings(allWarnings...) + } + } + return Allowed("").WithWarnings(allWarnings...) +} diff --git a/pkg/webhook/admission/validator_warn_custom.go b/pkg/webhook/admission/validator_warn_custom.go new file mode 100644 index 0000000000..390fa4d698 --- /dev/null +++ b/pkg/webhook/admission/validator_warn_custom.go @@ -0,0 +1,112 @@ +/* +Copyright 2022 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" +) + +// CustomValidatorWarn works like CustomValidator, but it allows to return warnings. +type CustomValidatorWarn interface { + ValidateCreate(ctx context.Context, obj runtime.Object) (err error, warnings []string) + ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (err error, warnings []string) + ValidateDelete(ctx context.Context, obj runtime.Object) (err error, warnings []string) +} + +// WithCustomValidatorWarn creates a new Webhook for validating the provided type. +func WithCustomValidatorWarn(obj runtime.Object, validatorWarn CustomValidatorWarn) *Webhook { + return &Webhook{ + Handler: &validatorWarnForType{object: obj, validatorWarn: validatorWarn}, + } +} + +var _ Handler = (*validatorWarnForType)(nil) +var _ DecoderInjector = (*validatorWarnForType)(nil) + +type validatorWarnForType struct { + validatorWarn CustomValidatorWarn + object runtime.Object + decoder *Decoder +} + +func (h *validatorWarnForType) InjectDecoder(d *Decoder) error { + h.decoder = d + return nil +} + +func (h *validatorWarnForType) Handle(ctx context.Context, req Request) Response { + if h.validatorWarn == nil { + panic("validatorWarn should never be nil") + } + if h.object == nil { + panic("object should never be nil") + } + + ctx = NewContextWithRequest(ctx, req) + + obj := h.object.DeepCopyObject() + + var err error + var warnings []string + switch req.Operation { + case v1.Create: + if err := h.decoder.Decode(req, obj); err != nil { + return Errored(http.StatusBadRequest, err) + } + + err, warnings = h.validatorWarn.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, warnings = h.validatorWarn.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, warnings = h.validatorWarn.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()).WithWarnings(warnings...) + } + + // Return allowed if everything succeeded. + return Allowed("").WithWarnings(warnings...) +} diff --git a/pkg/webhook/admission/validator_warn_test.go b/pkg/webhook/admission/validator_warn_test.go new file mode 100644 index 0000000000..946a116b1e --- /dev/null +++ b/pkg/webhook/admission/validator_warn_test.go @@ -0,0 +1,342 @@ +package admission + +import ( + "context" + goerrors "errors" + "net/http" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission/admissiontest" + + admissionv1 "k8s.io/api/admission/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + 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 fakeValidatorWarnVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "fakeValidatorWarn"} + +var _ = Describe("validatingWarnHandler", func() { + + decoder, _ := NewDecoder(scheme.Scheme) + + Context("when dealing with successful results without warning", func() { + f := &admissiontest.FakeValidatorWarn{ErrorToReturn: nil, GVKToReturn: fakeValidatorWarnVK, WarningsToReturn: nil} + handler := validatingWarnHandler{validatorWarn: f, decoder: decoder} + + It("should return 200 in response when create succeeds", func() { + response := handler.Handle(context.TODO(), Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + Object: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validatorWarn, + }, + }, + }) + + Expect(response.Allowed).Should(BeTrue()) + Expect(response.Result.Code).Should(Equal(int32(http.StatusOK))) + }) + + It("should return 200 in response when update succeeds", func() { + + response := handler.Handle(context.TODO(), Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Update, + Object: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validatorWarn, + }, + OldObject: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validatorWarn, + }, + }, + }) + Expect(response.Allowed).Should(BeTrue()) + Expect(response.Result.Code).Should(Equal(int32(http.StatusOK))) + }) + + It("should return 200 in response when delete succeeds", func() { + + response := handler.Handle(context.TODO(), Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Delete, + OldObject: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validatorWarn, + }, + }, + }) + Expect(response.Allowed).Should(BeTrue()) + Expect(response.Result.Code).Should(Equal(int32(http.StatusOK))) + }) + }) + + const warningMessage = "warning message" + const anotherWarningMessage = "another warning message" + Context("when dealing with successful results with warning", func() { + f := &admissiontest.FakeValidatorWarn{ErrorToReturn: nil, GVKToReturn: fakeValidatorWarnVK, WarningsToReturn: []string{ + warningMessage, + anotherWarningMessage, + }} + handler := validatingWarnHandler{validatorWarn: f, decoder: decoder} + + 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.validatorWarn, + }, + }, + }) + + Expect(response.Allowed).Should(BeTrue()) + Expect(response.Result.Code).Should(Equal(int32(http.StatusOK))) + Expect(response.AdmissionResponse.Warnings).Should(ContainElement(warningMessage)) + Expect(response.AdmissionResponse.Warnings).Should(ContainElement(anotherWarningMessage)) + }) + + It("should return 200 in response when update succeeds, with warning messages", func() { + + response := handler.Handle(context.TODO(), Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Update, + Object: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validatorWarn, + }, + OldObject: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validatorWarn, + }, + }, + }) + Expect(response.Allowed).Should(BeTrue()) + Expect(response.Result.Code).Should(Equal(int32(http.StatusOK))) + Expect(response.AdmissionResponse.Warnings).Should(ContainElement(warningMessage)) + Expect(response.AdmissionResponse.Warnings).Should(ContainElement(anotherWarningMessage)) + }) + + It("should return 200 in response when delete succeeds, with warning messages", func() { + + response := handler.Handle(context.TODO(), Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Delete, + OldObject: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validatorWarn, + }, + }, + }) + Expect(response.Allowed).Should(BeTrue()) + Expect(response.Result.Code).Should(Equal(int32(http.StatusOK))) + Expect(response.AdmissionResponse.Warnings).Should(ContainElement(warningMessage)) + Expect(response.AdmissionResponse.Warnings).Should(ContainElement(anotherWarningMessage)) + }) + }) + + Context("when dealing with Status errors", func() { + + expectedError := &apierrors.StatusError{ + ErrStatus: metav1.Status{ + Message: "some message", + Code: http.StatusUnprocessableEntity, + }, + } + f := &admissiontest.FakeValidatorWarn{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: nil} + handler := validatingWarnHandler{validatorWarn: f, decoder: decoder} + + 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.validatorWarn, + }, + }, + }) + + Expect(response.Allowed).Should(BeFalse()) + Expect(response.Result.Code).Should(Equal(expectedError.Status().Code)) + Expect(*response.Result).Should(Equal(expectedError.Status())) + + }) + + It("should propagate the Status from ValidateUpdate's return value to the HTTP response", func() { + + response := handler.Handle(context.TODO(), Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Update, + Object: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validatorWarn, + }, + OldObject: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validatorWarn, + }, + }, + }) + + Expect(response.Allowed).Should(BeFalse()) + Expect(response.Result.Code).Should(Equal(expectedError.Status().Code)) + Expect(*response.Result).Should(Equal(expectedError.Status())) + + }) + + It("should propagate the Status from ValidateDelete's return value to the HTTP response", func() { + + response := handler.Handle(context.TODO(), Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Delete, + OldObject: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validatorWarn, + }, + }, + }) + + Expect(response.Allowed).Should(BeFalse()) + Expect(response.Result.Code).Should(Equal(expectedError.Status().Code)) + Expect(*response.Result).Should(Equal(expectedError.Status())) + + }) + + }) + + Context("when dealing with non-status errors, without warning messages", func() { + + expectedError := goerrors.New("some error") + f := &admissiontest.FakeValidatorWarn{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK} + handler := validatingWarnHandler{validatorWarn: f, decoder: decoder} + + 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.validatorWarn, + }, + }, + }) + Expect(response.Allowed).Should(BeFalse()) + Expect(response.Result.Code).Should(Equal(int32(http.StatusForbidden))) + Expect(string(response.Result.Reason)).Should(Equal(expectedError.Error())) + + }) + + 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.validatorWarn, + }, + OldObject: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validatorWarn, + }, + }, + }) + Expect(response.Allowed).Should(BeFalse()) + Expect(response.Result.Code).Should(Equal(int32(http.StatusForbidden))) + Expect(string(response.Result.Reason)).Should(Equal(expectedError.Error())) + + }) + + It("should return 403 response when ValidateDelete returns non-APIStatus error", func() { + response := handler.Handle(context.TODO(), Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Delete, + OldObject: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validatorWarn, + }, + }, + }) + Expect(response.Allowed).Should(BeFalse()) + Expect(response.Result.Code).Should(Equal(int32(http.StatusForbidden))) + Expect(string(response.Result.Reason)).Should(Equal(expectedError.Error())) + }) + }) + + Context("when dealing with non-status errors, with warning messages", func() { + + expectedError := goerrors.New("some error") + f := &admissiontest.FakeValidatorWarn{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{warningMessage, anotherWarningMessage}} + handler := validatingWarnHandler{validatorWarn: f, decoder: decoder} + + 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.validatorWarn, + }, + }, + }) + Expect(response.Allowed).Should(BeFalse()) + Expect(response.Result.Code).Should(Equal(int32(http.StatusForbidden))) + Expect(string(response.Result.Reason)).Should(Equal(expectedError.Error())) + Expect(response.AdmissionResponse.Warnings).Should(ContainElement(warningMessage)) + Expect(response.AdmissionResponse.Warnings).Should(ContainElement(anotherWarningMessage)) + }) + + 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.validatorWarn, + }, + OldObject: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validatorWarn, + }, + }, + }) + Expect(response.Allowed).Should(BeFalse()) + Expect(response.Result.Code).Should(Equal(int32(http.StatusForbidden))) + Expect(string(response.Result.Reason)).Should(Equal(expectedError.Error())) + Expect(response.AdmissionResponse.Warnings).Should(ContainElement(warningMessage)) + Expect(response.AdmissionResponse.Warnings).Should(ContainElement(anotherWarningMessage)) + + }) + + It("should return 403 response when ValidateDelete returns non-APIStatus error", func() { + response := handler.Handle(context.TODO(), Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Delete, + OldObject: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validatorWarn, + }, + }, + }) + Expect(response.Allowed).Should(BeFalse()) + Expect(response.Result.Code).Should(Equal(int32(http.StatusForbidden))) + Expect(string(response.Result.Reason)).Should(Equal(expectedError.Error())) + Expect(response.AdmissionResponse.Warnings).Should(ContainElement(warningMessage)) + Expect(response.AdmissionResponse.Warnings).Should(ContainElement(anotherWarningMessage)) + + }) + }) + +}) diff --git a/pkg/webhook/alias.go b/pkg/webhook/alias.go index 293137db49..c1b1e88228 100644 --- a/pkg/webhook/alias.go +++ b/pkg/webhook/alias.go @@ -29,6 +29,9 @@ type Defaulter = admission.Defaulter // Validator defines functions for validating an operation. type Validator = admission.Validator +// ValidatorWarn like Validator, but could return warnings. +type ValidatorWarn = admission.ValidatorWarn + // CustomDefaulter defines functions for setting defaults on resources. type CustomDefaulter = admission.CustomDefaulter From f09981065a33381be4aa903772392518b4a1a057 Mon Sep 17 00:00:00 2001 From: STRRL Date: Thu, 3 Nov 2022 12:14:26 +0800 Subject: [PATCH 02/13] chore: refine implementing declaration, fix import cycle Signed-off-by: STRRL --- pkg/webhook/admission/admissiontest/util.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/pkg/webhook/admission/admissiontest/util.go b/pkg/webhook/admission/admissiontest/util.go index 90394389b3..3519049b09 100644 --- a/pkg/webhook/admission/admissiontest/util.go +++ b/pkg/webhook/admission/admissiontest/util.go @@ -19,13 +19,8 @@ package admissiontest import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" - "sigs.k8s.io/controller-runtime/pkg/webhook" ) -var _ runtime.Object = (*FakeValidator)(nil) -var _ schema.ObjectKind = (*FakeValidator)(nil) -var _ webhook.Validator = (*FakeValidator)(nil) - // FakeValidator provides fake validating webhook functionality for testing // It implements the admission.Validator interface and // rejects all requests with the same configured error @@ -70,10 +65,6 @@ func (v *FakeValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) { v.GVKToReturn = gvk } -var _ runtime.Object = (*FakeValidatorWarn)(nil) -var _ schema.ObjectKind = (*FakeValidatorWarn)(nil) -var _ webhook.ValidatorWarn = (*FakeValidatorWarn)(nil) - // FakeValidatorWarn provides fake validating webhook functionality for testing // It implements the admission.ValidatorWarn interface and // rejects all requests with the same configured error From 31371cca59e6a7b1593eb6ce3816b47428c9ea13 Mon Sep 17 00:00:00 2001 From: STRRL Date: Thu, 3 Nov 2022 15:23:30 +0800 Subject: [PATCH 03/13] chore: refine interface methods declration Signed-off-by: STRRL --- pkg/webhook/admission/admissiontest/util.go | 12 ++++++------ pkg/webhook/admission/validator_warn.go | 17 ++++++++++------- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/pkg/webhook/admission/admissiontest/util.go b/pkg/webhook/admission/admissiontest/util.go index 3519049b09..acda415871 100644 --- a/pkg/webhook/admission/admissiontest/util.go +++ b/pkg/webhook/admission/admissiontest/util.go @@ -79,16 +79,16 @@ type FakeValidatorWarn struct { WarningsToReturn []string } -func (v *FakeValidatorWarn) ValidateCreate() (err error, warnings []string) { - return v.ErrorToReturn, v.WarningsToReturn +func (v *FakeValidatorWarn) ValidateCreate() (warnings []string, err error) { + return v.WarningsToReturn, v.ErrorToReturn } -func (v *FakeValidatorWarn) ValidateUpdate(old runtime.Object) (err error, warnings []string) { - return v.ErrorToReturn, v.WarningsToReturn +func (v *FakeValidatorWarn) ValidateUpdate(old runtime.Object) (warnings []string, err error) { + return v.WarningsToReturn, v.ErrorToReturn } -func (v *FakeValidatorWarn) ValidateDelete() (err error, warnings []string) { - return v.ErrorToReturn, v.WarningsToReturn +func (v *FakeValidatorWarn) ValidateDelete() (warnings []string, err error) { + return v.WarningsToReturn, v.ErrorToReturn } func (v *FakeValidatorWarn) SetGroupVersionKind(kind schema.GroupVersionKind) { diff --git a/pkg/webhook/admission/validator_warn.go b/pkg/webhook/admission/validator_warn.go index 21bd92636f..c29f25d285 100644 --- a/pkg/webhook/admission/validator_warn.go +++ b/pkg/webhook/admission/validator_warn.go @@ -29,13 +29,16 @@ import ( // ValidatorWarn works like Validator, but it allows to return warnings. type ValidatorWarn interface { runtime.Object - ValidateCreate() (err error, warnings []string) - ValidateUpdate(old runtime.Object) (err error, warnings []string) - ValidateDelete() (err error, warnings []string) + ValidateCreate() (warnings []string, err error) + ValidateUpdate(old runtime.Object) (warnings []string, err error) + ValidateDelete() (warnings []string, err error) } +// ValidatingWebhookWithWarningFor creates a new Webhook for validating the provided type with warning messages. func ValidatingWebhookWithWarningFor(validatorWarning ValidatorWarn) *Webhook { - return nil + return &Webhook{ + Handler: &validatingWarnHandler{validatorWarn: validatorWarning}, + } } var _ Handler = &validatingWarnHandler{} @@ -68,7 +71,7 @@ func (h *validatingWarnHandler) Handle(ctx context.Context, req Request) Respons return Errored(http.StatusBadRequest, err) } - err, warnings := obj.ValidateCreate() + warnings, err := obj.ValidateCreate() allWarnings = append(allWarnings, warnings...) if err != nil { var apiStatus apierrors.APIStatus @@ -90,7 +93,7 @@ func (h *validatingWarnHandler) Handle(ctx context.Context, req Request) Respons if err != nil { return Errored(http.StatusBadRequest, err) } - err, warnings := obj.ValidateUpdate(oldObj) + warnings, err := obj.ValidateUpdate(oldObj) allWarnings = append(allWarnings, warnings...) if err != nil { var apiStatus apierrors.APIStatus @@ -109,7 +112,7 @@ func (h *validatingWarnHandler) Handle(ctx context.Context, req Request) Respons return Errored(http.StatusBadRequest, err) } - err, warnings := obj.ValidateDelete() + warnings, err := obj.ValidateDelete() allWarnings = append(allWarnings, warnings...) if err != nil { var apiStatus apierrors.APIStatus From b37201d02dcc84c5854de9abaf9c97490a2b468d Mon Sep 17 00:00:00 2001 From: STRRL Date: Mon, 5 Dec 2022 21:11:00 +0800 Subject: [PATCH 04/13] chore: move error as the last value returned by CustomValidatorWarn's methods Signed-off-by: STRRL --- pkg/webhook/admission/validator_warn_custom.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/webhook/admission/validator_warn_custom.go b/pkg/webhook/admission/validator_warn_custom.go index 390fa4d698..996b947119 100644 --- a/pkg/webhook/admission/validator_warn_custom.go +++ b/pkg/webhook/admission/validator_warn_custom.go @@ -29,9 +29,9 @@ import ( // CustomValidatorWarn works like CustomValidator, but it allows to return warnings. type CustomValidatorWarn interface { - ValidateCreate(ctx context.Context, obj runtime.Object) (err error, warnings []string) - ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (err error, warnings []string) - ValidateDelete(ctx context.Context, obj runtime.Object) (err error, warnings []string) + ValidateCreate(ctx context.Context, obj runtime.Object) (warnings []string, err error) + ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings []string, err error) + ValidateDelete(ctx context.Context, obj runtime.Object) (warnings []string, err error) } // WithCustomValidatorWarn creates a new Webhook for validating the provided type. @@ -75,7 +75,7 @@ func (h *validatorWarnForType) Handle(ctx context.Context, req Request) Response return Errored(http.StatusBadRequest, err) } - err, warnings = h.validatorWarn.ValidateCreate(ctx, obj) + warnings, err = h.validatorWarn.ValidateCreate(ctx, obj) case v1.Update: oldObj := obj.DeepCopyObject() if err := h.decoder.DecodeRaw(req.Object, obj); err != nil { @@ -85,7 +85,7 @@ func (h *validatorWarnForType) Handle(ctx context.Context, req Request) Response return Errored(http.StatusBadRequest, err) } - err, warnings = h.validatorWarn.ValidateUpdate(ctx, oldObj, obj) + warnings, err = h.validatorWarn.ValidateUpdate(ctx, oldObj, obj) case v1.Delete: // In reference to PR: https://github.com/kubernetes/kubernetes/pull/76346 // OldObject contains the object being deleted @@ -93,7 +93,7 @@ func (h *validatorWarnForType) Handle(ctx context.Context, req Request) Response return Errored(http.StatusBadRequest, err) } - err, warnings = h.validatorWarn.ValidateDelete(ctx, obj) + warnings, err = h.validatorWarn.ValidateDelete(ctx, obj) default: return Errored(http.StatusBadRequest, fmt.Errorf("unknown operation request %q", req.Operation)) } From 7d1e860da9b3fb71e8c59fbacec3a82aae139631 Mon Sep 17 00:00:00 2001 From: STRRL Date: Mon, 5 Dec 2022 21:12:29 +0800 Subject: [PATCH 05/13] chore: stop using goerrors as alias or std errors Signed-off-by: STRRL --- pkg/webhook/admission/validator.go | 8 ++++---- pkg/webhook/admission/validator_test.go | 4 ++-- pkg/webhook/admission/validator_warn.go | 8 ++++---- pkg/webhook/admission/validator_warn_test.go | 6 +++--- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pkg/webhook/admission/validator.go b/pkg/webhook/admission/validator.go index 43ea3ee65f..fc2482d040 100644 --- a/pkg/webhook/admission/validator.go +++ b/pkg/webhook/admission/validator.go @@ -18,7 +18,7 @@ package admission import ( "context" - goerrors "errors" + "errors" "net/http" v1 "k8s.io/api/admission/v1" @@ -66,7 +66,7 @@ func (h *validatingHandler) Handle(ctx context.Context, req Request) Response { err = obj.ValidateCreate() if err != nil { var apiStatus apierrors.APIStatus - if goerrors.As(err, &apiStatus) { + if errors.As(err, &apiStatus) { return validationResponseFromStatus(false, apiStatus.Status()) } return Denied(err.Error()) @@ -88,7 +88,7 @@ func (h *validatingHandler) Handle(ctx context.Context, req Request) Response { err = obj.ValidateUpdate(oldObj) if err != nil { var apiStatus apierrors.APIStatus - if goerrors.As(err, &apiStatus) { + if errors.As(err, &apiStatus) { return validationResponseFromStatus(false, apiStatus.Status()) } return Denied(err.Error()) @@ -106,7 +106,7 @@ func (h *validatingHandler) Handle(ctx context.Context, req Request) Response { err = obj.ValidateDelete() if err != nil { var apiStatus apierrors.APIStatus - if goerrors.As(err, &apiStatus) { + if errors.As(err, &apiStatus) { return validationResponseFromStatus(false, apiStatus.Status()) } return Denied(err.Error()) diff --git a/pkg/webhook/admission/validator_test.go b/pkg/webhook/admission/validator_test.go index 3da8f0b46a..4dcbd0950a 100644 --- a/pkg/webhook/admission/validator_test.go +++ b/pkg/webhook/admission/validator_test.go @@ -18,7 +18,7 @@ package admission import ( "context" - goerrors "errors" + "errors" "net/http" . "github.com/onsi/ginkgo/v2" @@ -168,7 +168,7 @@ var _ = Describe("validatingHandler", func() { }) Context("when dealing with non-status errors", func() { - expectedError := goerrors.New("some error") + expectedError := errors.New("some error") f := &admissiontest.FakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK} handler := validatingHandler{validator: f, decoder: decoder} diff --git a/pkg/webhook/admission/validator_warn.go b/pkg/webhook/admission/validator_warn.go index c29f25d285..ca11d71ed3 100644 --- a/pkg/webhook/admission/validator_warn.go +++ b/pkg/webhook/admission/validator_warn.go @@ -18,7 +18,7 @@ package admission import ( "context" - goerrors "errors" + "errors" "net/http" v1 "k8s.io/api/admission/v1" @@ -75,7 +75,7 @@ func (h *validatingWarnHandler) Handle(ctx context.Context, req Request) Respons allWarnings = append(allWarnings, warnings...) if err != nil { var apiStatus apierrors.APIStatus - if goerrors.As(err, &apiStatus) { + if errors.As(err, &apiStatus) { return validationResponseFromStatus(false, apiStatus.Status()) } return Denied(err.Error()).WithWarnings(allWarnings...) @@ -97,7 +97,7 @@ func (h *validatingWarnHandler) Handle(ctx context.Context, req Request) Respons allWarnings = append(allWarnings, warnings...) if err != nil { var apiStatus apierrors.APIStatus - if goerrors.As(err, &apiStatus) { + if errors.As(err, &apiStatus) { return validationResponseFromStatus(false, apiStatus.Status()) } return Denied(err.Error()).WithWarnings(allWarnings...) @@ -116,7 +116,7 @@ func (h *validatingWarnHandler) Handle(ctx context.Context, req Request) Respons allWarnings = append(allWarnings, warnings...) if err != nil { var apiStatus apierrors.APIStatus - if goerrors.As(err, &apiStatus) { + if errors.As(err, &apiStatus) { return validationResponseFromStatus(false, apiStatus.Status()) } return Denied(err.Error()).WithWarnings(allWarnings...) diff --git a/pkg/webhook/admission/validator_warn_test.go b/pkg/webhook/admission/validator_warn_test.go index 946a116b1e..0405fe9287 100644 --- a/pkg/webhook/admission/validator_warn_test.go +++ b/pkg/webhook/admission/validator_warn_test.go @@ -2,7 +2,7 @@ package admission import ( "context" - goerrors "errors" + "errors" "net/http" . "github.com/onsi/ginkgo/v2" @@ -215,7 +215,7 @@ var _ = Describe("validatingWarnHandler", func() { Context("when dealing with non-status errors, without warning messages", func() { - expectedError := goerrors.New("some error") + expectedError := errors.New("some error") f := &admissiontest.FakeValidatorWarn{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK} handler := validatingWarnHandler{validatorWarn: f, decoder: decoder} @@ -275,7 +275,7 @@ var _ = Describe("validatingWarnHandler", func() { Context("when dealing with non-status errors, with warning messages", func() { - expectedError := goerrors.New("some error") + expectedError := errors.New("some error") f := &admissiontest.FakeValidatorWarn{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{warningMessage, anotherWarningMessage}} handler := validatingWarnHandler{validatorWarn: f, decoder: decoder} From 9205bab0a5f9f827097656fb7fd0ae2c2c45e95d Mon Sep 17 00:00:00 2001 From: STRRL Date: Thu, 9 Feb 2023 09:24:22 +0800 Subject: [PATCH 06/13] refactor(defaulting webhook): breaking the original interface instead of introducing new interface Signed-off-by: STRRL --- pkg/webhook/admission/admissiontest/util.go | 64 +---- pkg/webhook/admission/validator.go | 68 +++-- pkg/webhook/admission/validator_custom.go | 18 +- pkg/webhook/admission/validator_test.go | 241 ------------------ pkg/webhook/admission/validator_warn.go | 126 --------- .../admission/validator_warn_custom.go | 112 -------- pkg/webhook/admission/validator_warn_test.go | 185 +++++++++++--- 7 files changed, 195 insertions(+), 619 deletions(-) delete mode 100644 pkg/webhook/admission/validator_test.go delete mode 100644 pkg/webhook/admission/validator_warn.go delete mode 100644 pkg/webhook/admission/validator_warn_custom.go diff --git a/pkg/webhook/admission/admissiontest/util.go b/pkg/webhook/admission/admissiontest/util.go index acda415871..f63456d34d 100644 --- a/pkg/webhook/admission/admissiontest/util.go +++ b/pkg/webhook/admission/admissiontest/util.go @@ -25,86 +25,42 @@ import ( // It implements the admission.Validator 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 { // 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 GVKToReturn schema.GroupVersionKind -} - -// ValidateCreate implements admission.Validator. -func (v *FakeValidator) ValidateCreate() error { - return v.ErrorToReturn -} - -// ValidateUpdate implements admission.Validator. -func (v *FakeValidator) ValidateUpdate(old runtime.Object) error { - return v.ErrorToReturn -} - -// ValidateDelete implements admission.Validator. -func (v *FakeValidator) ValidateDelete() error { - return v.ErrorToReturn -} - -// GetObjectKind implements admission.Validator. -func (v *FakeValidator) GetObjectKind() schema.ObjectKind { return v } - -// DeepCopyObject implements admission.Validator. -func (v *FakeValidator) DeepCopyObject() runtime.Object { - return &FakeValidator{ErrorToReturn: v.ErrorToReturn, GVKToReturn: v.GVKToReturn} -} - -// GroupVersionKind implements admission.Validator. -func (v *FakeValidator) GroupVersionKind() schema.GroupVersionKind { - return v.GVKToReturn -} - -// SetGroupVersionKind implements admission.Validator. -func (v *FakeValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) { - v.GVKToReturn = gvk -} - -// FakeValidatorWarn provides fake validating webhook functionality for testing -// It implements the admission.ValidatorWarn 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 FakeValidatorWarn struct { - // ErrorToReturn is the error for which the FakeValidatorWarn rejects all requests - ErrorToReturn error `json:"ErrorToReturn,omitempty"` - // GVKToReturn is the GroupVersionKind that the webhook operates on - GVKToReturn schema.GroupVersionKind - // WarningsToReturn is the warnings for FakeValidatorWarn returns to all requests + // WarningsToReturn is the warnings for FakeValidator returns to all requests WarningsToReturn []string } -func (v *FakeValidatorWarn) ValidateCreate() (warnings []string, err error) { +func (v *FakeValidator) ValidateCreate() (warnings []string, err error) { return v.WarningsToReturn, v.ErrorToReturn } -func (v *FakeValidatorWarn) ValidateUpdate(old runtime.Object) (warnings []string, err error) { +func (v *FakeValidator) ValidateUpdate(old runtime.Object) (warnings []string, err error) { return v.WarningsToReturn, v.ErrorToReturn } -func (v *FakeValidatorWarn) ValidateDelete() (warnings []string, err error) { +func (v *FakeValidator) ValidateDelete() (warnings []string, err error) { return v.WarningsToReturn, v.ErrorToReturn } -func (v *FakeValidatorWarn) SetGroupVersionKind(kind schema.GroupVersionKind) { +func (v *FakeValidator) SetGroupVersionKind(kind schema.GroupVersionKind) { v.GVKToReturn = kind } -func (v *FakeValidatorWarn) GroupVersionKind() schema.GroupVersionKind { +func (v *FakeValidator) GroupVersionKind() schema.GroupVersionKind { return v.GVKToReturn } -func (v *FakeValidatorWarn) GetObjectKind() schema.ObjectKind { +func (v *FakeValidator) GetObjectKind() schema.ObjectKind { return v } -func (v *FakeValidatorWarn) DeepCopyObject() runtime.Object { - return &FakeValidatorWarn{ErrorToReturn: v.ErrorToReturn, +func (v *FakeValidator) DeepCopyObject() runtime.Object { + return &FakeValidator{ErrorToReturn: v.ErrorToReturn, GVKToReturn: v.GVKToReturn, WarningsToReturn: v.WarningsToReturn, } diff --git a/pkg/webhook/admission/validator.go b/pkg/webhook/admission/validator.go index fc2482d040..15cdfaf819 100644 --- a/pkg/webhook/admission/validator.go +++ b/pkg/webhook/admission/validator.go @@ -19,6 +19,7 @@ package admission import ( "context" "errors" + "fmt" "net/http" v1 "k8s.io/api/admission/v1" @@ -29,9 +30,9 @@ import ( // Validator defines functions for validating an operation. type Validator interface { runtime.Object - ValidateCreate() error - ValidateUpdate(old runtime.Object) error - ValidateDelete() error + ValidateCreate() ([]string, error) + ValidateUpdate(old runtime.Object) ([]string, error) + ValidateDelete() ([]string, error) } // ValidatingWebhookFor creates a new Webhook for validating the provided type. @@ -55,28 +56,26 @@ func (h *validatingHandler) Handle(ctx context.Context, req Request) Response { panic("validator should never be nil") } + ctx = NewContextWithRequest(ctx, req) + // Get the object in the request obj := h.validator.DeepCopyObject().(Validator) - if req.Operation == v1.Create { - err := h.decoder.Decode(req, obj) - if err != nil { - return Errored(http.StatusBadRequest, err) - } - err = obj.ValidateCreate() + var err error + var warnings []string + + switch req.Operation { + case v1.Create: + err = h.decoder.Decode(req, obj) if err != nil { - var apiStatus apierrors.APIStatus - if errors.As(err, &apiStatus) { - return validationResponseFromStatus(false, apiStatus.Status()) - } - return Denied(err.Error()) + return Errored(http.StatusBadRequest, err) } - } - if req.Operation == v1.Update { + warnings, err = obj.ValidateCreate() + case v1.Update: oldObj := obj.DeepCopyObject() - err := h.decoder.DecodeRaw(req.Object, obj) + err = h.decoder.DecodeRaw(req.Object, obj) if err != nil { return Errored(http.StatusBadRequest, err) } @@ -85,33 +84,26 @@ func (h *validatingHandler) Handle(ctx context.Context, req Request) Response { return Errored(http.StatusBadRequest, err) } - err = obj.ValidateUpdate(oldObj) - if err != nil { - var apiStatus apierrors.APIStatus - if errors.As(err, &apiStatus) { - return validationResponseFromStatus(false, apiStatus.Status()) - } - return Denied(err.Error()) - } - } - - if req.Operation == v1.Delete { + 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) + err = h.decoder.DecodeRaw(req.OldObject, obj) if err != nil { return Errored(http.StatusBadRequest, err) } - err = obj.ValidateDelete() - if err != nil { - var apiStatus apierrors.APIStatus - if errors.As(err, &apiStatus) { - return validationResponseFromStatus(false, apiStatus.Status()) - } - return Denied(err.Error()) - } + warnings, err = obj.ValidateDelete() + default: + return Errored(http.StatusBadRequest, fmt.Errorf("unknown operation request %q", req.Operation)) } - return Allowed("") + if err != nil { + var apiStatus apierrors.APIStatus + if errors.As(err, &apiStatus) { + return validationResponseFromStatus(false, apiStatus.Status()) + } + 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 755e464a91..c966c4abb1 100644 --- a/pkg/webhook/admission/validator_custom.go +++ b/pkg/webhook/admission/validator_custom.go @@ -29,9 +29,9 @@ import ( // 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 + ValidateCreate(ctx context.Context, obj runtime.Object) ([]string, error) + ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) ([]string, error) + ValidateDelete(ctx context.Context, obj runtime.Object) ([]string, error) } // WithCustomValidator creates a new Webhook for validating the provided type. @@ -65,6 +65,8 @@ func (h *validatorForType) Handle(ctx context.Context, req Request) Response { obj := h.object.DeepCopyObject() var err error + var warnings []string + switch req.Operation { case v1.Connect: // No validation for connect requests. @@ -74,7 +76,7 @@ func (h *validatorForType) Handle(ctx context.Context, req Request) Response { return Errored(http.StatusBadRequest, err) } - err = h.validator.ValidateCreate(ctx, obj) + warnings, err = h.validator.ValidateCreate(ctx, obj) case v1.Update: oldObj := obj.DeepCopyObject() if err := h.decoder.DecodeRaw(req.Object, obj); err != nil { @@ -84,7 +86,7 @@ func (h *validatorForType) Handle(ctx context.Context, req Request) Response { return Errored(http.StatusBadRequest, err) } - err = h.validator.ValidateUpdate(ctx, oldObj, obj) + warnings, 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 @@ -92,7 +94,7 @@ func (h *validatorForType) Handle(ctx context.Context, req Request) Response { return Errored(http.StatusBadRequest, err) } - err = h.validator.ValidateDelete(ctx, obj) + warnings, err = h.validator.ValidateDelete(ctx, obj) default: return Errored(http.StatusBadRequest, fmt.Errorf("unknown operation request %q", req.Operation)) } @@ -103,9 +105,9 @@ func (h *validatorForType) Handle(ctx context.Context, req Request) Response { if errors.As(err, &apiStatus) { return validationResponseFromStatus(false, apiStatus.Status()) } - return Denied(err.Error()) + return Denied(err.Error()).WithWarnings(warnings...) } // Return allowed if everything succeeded. - return Allowed("") + return Allowed("").WithWarnings(warnings...) } diff --git a/pkg/webhook/admission/validator_test.go b/pkg/webhook/admission/validator_test.go deleted file mode 100644 index 4dcbd0950a..0000000000 --- a/pkg/webhook/admission/validator_test.go +++ /dev/null @@ -1,241 +0,0 @@ -/* -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" - "net/http" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission/admissiontest" - - admissionv1 "k8s.io/api/admission/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - 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) - - Context("when dealing with successful results", func() { - - f := &admissiontest.FakeValidator{ErrorToReturn: nil, GVKToReturn: fakeValidatorVK} - handler := validatingHandler{validator: f, decoder: decoder} - - It("should return 200 in response when create succeeds", func() { - - response := handler.Handle(context.TODO(), Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Operation: admissionv1.Create, - Object: runtime.RawExtension{ - Raw: []byte("{}"), - Object: handler.validator, - }, - }, - }) - - Expect(response.Allowed).Should(BeTrue()) - Expect(response.Result.Code).Should(Equal(int32(http.StatusOK))) - }) - - It("should return 200 in response when update succeeds", func() { - - response := handler.Handle(context.TODO(), Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Operation: admissionv1.Update, - Object: runtime.RawExtension{ - Raw: []byte("{}"), - Object: handler.validator, - }, - OldObject: runtime.RawExtension{ - Raw: []byte("{}"), - Object: handler.validator, - }, - }, - }) - Expect(response.Allowed).Should(BeTrue()) - Expect(response.Result.Code).Should(Equal(int32(http.StatusOK))) - }) - - It("should return 200 in response when delete succeeds", func() { - - response := handler.Handle(context.TODO(), Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Operation: admissionv1.Delete, - OldObject: runtime.RawExtension{ - Raw: []byte("{}"), - Object: handler.validator, - }, - }, - }) - Expect(response.Allowed).Should(BeTrue()) - Expect(response.Result.Code).Should(Equal(int32(http.StatusOK))) - }) - - }) - - Context("when dealing with Status errors", func() { - - expectedError := &apierrors.StatusError{ - ErrStatus: metav1.Status{ - Message: "some message", - Code: http.StatusUnprocessableEntity, - }, - } - f := &admissiontest.FakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK} - handler := validatingHandler{validator: f, decoder: decoder} - - 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, - }, - }, - }) - - Expect(response.Allowed).Should(BeFalse()) - Expect(response.Result.Code).Should(Equal(expectedError.Status().Code)) - Expect(*response.Result).Should(Equal(expectedError.Status())) - - }) - - It("should propagate the Status from ValidateUpdate's return value to the HTTP response", func() { - - response := handler.Handle(context.TODO(), Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Operation: admissionv1.Update, - Object: runtime.RawExtension{ - Raw: []byte("{}"), - Object: handler.validator, - }, - OldObject: runtime.RawExtension{ - Raw: []byte("{}"), - Object: handler.validator, - }, - }, - }) - - Expect(response.Allowed).Should(BeFalse()) - Expect(response.Result.Code).Should(Equal(expectedError.Status().Code)) - Expect(*response.Result).Should(Equal(expectedError.Status())) - - }) - - It("should propagate the Status from ValidateDelete's return value to the HTTP response", func() { - - response := handler.Handle(context.TODO(), Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Operation: admissionv1.Delete, - OldObject: runtime.RawExtension{ - Raw: []byte("{}"), - Object: handler.validator, - }, - }, - }) - - Expect(response.Allowed).Should(BeFalse()) - Expect(response.Result.Code).Should(Equal(expectedError.Status().Code)) - Expect(*response.Result).Should(Equal(expectedError.Status())) - - }) - - }) - Context("when dealing with non-status errors", func() { - - expectedError := errors.New("some error") - f := &admissiontest.FakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK} - handler := validatingHandler{validator: f, decoder: decoder} - - 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, - }, - }, - }) - Expect(response.Allowed).Should(BeFalse()) - Expect(response.Result.Code).Should(Equal(int32(http.StatusForbidden))) - Expect(response.Result.Message).Should(Equal(expectedError.Error())) - - }) - - 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, - }, - OldObject: runtime.RawExtension{ - Raw: []byte("{}"), - Object: handler.validator, - }, - }, - }) - 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())) - - }) - - It("should return 403 response when ValidateDelete returns non-APIStatus error", func() { - - response := handler.Handle(context.TODO(), Request{ - AdmissionRequest: admissionv1.AdmissionRequest{ - Operation: admissionv1.Delete, - OldObject: runtime.RawExtension{ - Raw: []byte("{}"), - Object: handler.validator, - }, - }, - }) - 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())) - }) - - }) - - PIt("should return 400 in response when create fails on decode", func() {}) - - PIt("should return 400 in response when update fails on decoding new object", func() {}) - - PIt("should return 400 in response when update fails on decoding old object", func() {}) - - PIt("should return 400 in response when delete fails on decode", func() {}) - -}) diff --git a/pkg/webhook/admission/validator_warn.go b/pkg/webhook/admission/validator_warn.go deleted file mode 100644 index ca11d71ed3..0000000000 --- a/pkg/webhook/admission/validator_warn.go +++ /dev/null @@ -1,126 +0,0 @@ -/* -Copyright 2022 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" - "net/http" - - v1 "k8s.io/api/admission/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "k8s.io/apimachinery/pkg/runtime" -) - -// ValidatorWarn works like Validator, but it allows to return warnings. -type ValidatorWarn interface { - runtime.Object - ValidateCreate() (warnings []string, err error) - ValidateUpdate(old runtime.Object) (warnings []string, err error) - ValidateDelete() (warnings []string, err error) -} - -// ValidatingWebhookWithWarningFor creates a new Webhook for validating the provided type with warning messages. -func ValidatingWebhookWithWarningFor(validatorWarning ValidatorWarn) *Webhook { - return &Webhook{ - Handler: &validatingWarnHandler{validatorWarn: validatorWarning}, - } -} - -var _ Handler = &validatingWarnHandler{} -var _ DecoderInjector = &validatingWarnHandler{} - -type validatingWarnHandler struct { - validatorWarn ValidatorWarn - decoder *Decoder -} - -// InjectDecoder injects the decoder into a validatingWarnHandler. -func (h *validatingWarnHandler) InjectDecoder(decoder *Decoder) error { - h.decoder = decoder - return nil -} - -// Handle handles admission requests. -func (h *validatingWarnHandler) Handle(ctx context.Context, req Request) Response { - if h.validatorWarn == nil { - panic("validatorWarn should never be nil") - } - - var allWarnings []string - - // Get the object in the request - obj := h.validatorWarn.DeepCopyObject().(ValidatorWarn) - if req.Operation == v1.Create { - err := h.decoder.Decode(req, obj) - if err != nil { - return Errored(http.StatusBadRequest, err) - } - - warnings, err := obj.ValidateCreate() - allWarnings = append(allWarnings, warnings...) - if err != nil { - var apiStatus apierrors.APIStatus - if errors.As(err, &apiStatus) { - return validationResponseFromStatus(false, apiStatus.Status()) - } - return Denied(err.Error()).WithWarnings(allWarnings...) - } - } - - if req.Operation == 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) - allWarnings = append(allWarnings, warnings...) - if err != nil { - var apiStatus apierrors.APIStatus - if errors.As(err, &apiStatus) { - return validationResponseFromStatus(false, apiStatus.Status()) - } - return Denied(err.Error()).WithWarnings(allWarnings...) - } - } - - if req.Operation == 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() - allWarnings = append(allWarnings, warnings...) - if err != nil { - var apiStatus apierrors.APIStatus - if errors.As(err, &apiStatus) { - return validationResponseFromStatus(false, apiStatus.Status()) - } - return Denied(err.Error()).WithWarnings(allWarnings...) - } - } - return Allowed("").WithWarnings(allWarnings...) -} diff --git a/pkg/webhook/admission/validator_warn_custom.go b/pkg/webhook/admission/validator_warn_custom.go deleted file mode 100644 index 996b947119..0000000000 --- a/pkg/webhook/admission/validator_warn_custom.go +++ /dev/null @@ -1,112 +0,0 @@ -/* -Copyright 2022 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" -) - -// CustomValidatorWarn works like CustomValidator, but it allows to return warnings. -type CustomValidatorWarn interface { - ValidateCreate(ctx context.Context, obj runtime.Object) (warnings []string, err error) - ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings []string, err error) - ValidateDelete(ctx context.Context, obj runtime.Object) (warnings []string, err error) -} - -// WithCustomValidatorWarn creates a new Webhook for validating the provided type. -func WithCustomValidatorWarn(obj runtime.Object, validatorWarn CustomValidatorWarn) *Webhook { - return &Webhook{ - Handler: &validatorWarnForType{object: obj, validatorWarn: validatorWarn}, - } -} - -var _ Handler = (*validatorWarnForType)(nil) -var _ DecoderInjector = (*validatorWarnForType)(nil) - -type validatorWarnForType struct { - validatorWarn CustomValidatorWarn - object runtime.Object - decoder *Decoder -} - -func (h *validatorWarnForType) InjectDecoder(d *Decoder) error { - h.decoder = d - return nil -} - -func (h *validatorWarnForType) Handle(ctx context.Context, req Request) Response { - if h.validatorWarn == nil { - panic("validatorWarn should never be nil") - } - if h.object == nil { - panic("object should never be nil") - } - - ctx = NewContextWithRequest(ctx, req) - - obj := h.object.DeepCopyObject() - - var err error - var warnings []string - switch req.Operation { - case v1.Create: - if err := h.decoder.Decode(req, obj); err != nil { - return Errored(http.StatusBadRequest, err) - } - - warnings, err = h.validatorWarn.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) - } - - warnings, err = h.validatorWarn.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) - } - - warnings, err = h.validatorWarn.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()).WithWarnings(warnings...) - } - - // Return allowed if everything succeeded. - return Allowed("").WithWarnings(warnings...) -} diff --git a/pkg/webhook/admission/validator_warn_test.go b/pkg/webhook/admission/validator_warn_test.go index 0405fe9287..5f8957f9d0 100644 --- a/pkg/webhook/admission/validator_warn_test.go +++ b/pkg/webhook/admission/validator_warn_test.go @@ -1,3 +1,19 @@ +/* +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 ( @@ -17,23 +33,24 @@ import ( "k8s.io/client-go/kubernetes/scheme" ) -var fakeValidatorWarnVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "fakeValidatorWarn"} +var fakeValidatorVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "fakeValidator"} -var _ = Describe("validatingWarnHandler", func() { +var _ = Describe("validatingHandler", func() { - decoder, _ := NewDecoder(scheme.Scheme) + decoder := NewDecoder(scheme.Scheme) Context("when dealing with successful results without warning", func() { - f := &admissiontest.FakeValidatorWarn{ErrorToReturn: nil, GVKToReturn: fakeValidatorWarnVK, WarningsToReturn: nil} - handler := validatingWarnHandler{validatorWarn: f, decoder: decoder} + f := &admissiontest.FakeValidator{ErrorToReturn: nil, GVKToReturn: fakeValidatorVK, WarningsToReturn: nil} + handler := validatingHandler{validator: f, decoder: decoder} It("should return 200 in response when create succeeds", func() { + response := handler.Handle(context.TODO(), Request{ AdmissionRequest: admissionv1.AdmissionRequest{ Operation: admissionv1.Create, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, }, }) @@ -49,11 +66,11 @@ var _ = Describe("validatingWarnHandler", func() { Operation: admissionv1.Update, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, }, }) @@ -68,7 +85,7 @@ var _ = Describe("validatingWarnHandler", func() { Operation: admissionv1.Delete, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, }, }) @@ -80,11 +97,11 @@ var _ = Describe("validatingWarnHandler", func() { const warningMessage = "warning message" const anotherWarningMessage = "another warning message" Context("when dealing with successful results with warning", func() { - f := &admissiontest.FakeValidatorWarn{ErrorToReturn: nil, GVKToReturn: fakeValidatorWarnVK, WarningsToReturn: []string{ + f := &admissiontest.FakeValidator{ErrorToReturn: nil, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{ warningMessage, anotherWarningMessage, }} - handler := validatingWarnHandler{validatorWarn: f, decoder: decoder} + handler := validatingHandler{validator: f, decoder: decoder} It("should return 200 in response when create succeeds, with warning messages", func() { response := handler.Handle(context.TODO(), Request{ @@ -92,7 +109,7 @@ var _ = Describe("validatingWarnHandler", func() { Operation: admissionv1.Create, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, }, }) @@ -110,11 +127,11 @@ var _ = Describe("validatingWarnHandler", func() { Operation: admissionv1.Update, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, }, }) @@ -131,7 +148,7 @@ var _ = Describe("validatingWarnHandler", func() { Operation: admissionv1.Delete, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, }, }) @@ -142,7 +159,81 @@ var _ = Describe("validatingWarnHandler", func() { }) }) - Context("when dealing with Status errors", func() { + Context("when dealing with Status errors, with warning messages", func() { + // Status error would overwrite the warning messages, so no warning messages should be observed. + expectedError := &apierrors.StatusError{ + ErrStatus: metav1.Status{ + Message: "some message", + Code: http.StatusUnprocessableEntity, + }, + } + f := &admissiontest.FakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{warningMessage, anotherWarningMessage}} + handler := validatingHandler{validator: f, decoder: decoder} + + 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, + }, + }, + }) + + Expect(response.Allowed).Should(BeFalse()) + Expect(response.Result.Code).Should(Equal(expectedError.Status().Code)) + Expect(*response.Result).Should(Equal(expectedError.Status())) + Expect(response.AdmissionResponse.Warnings).Should(BeEmpty()) + + }) + + It("should propagate the Status from ValidateUpdate's return value to the HTTP response", func() { + + response := handler.Handle(context.TODO(), Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Update, + Object: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validator, + }, + OldObject: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validator, + }, + }, + }) + + Expect(response.Allowed).Should(BeFalse()) + Expect(response.Result.Code).Should(Equal(expectedError.Status().Code)) + Expect(*response.Result).Should(Equal(expectedError.Status())) + Expect(response.AdmissionResponse.Warnings).Should(BeEmpty()) + + }) + + It("should propagate the Status from ValidateDelete's return value to the HTTP response", func() { + + response := handler.Handle(context.TODO(), Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Delete, + OldObject: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validator, + }, + }, + }) + + Expect(response.Allowed).Should(BeFalse()) + Expect(response.Result.Code).Should(Equal(expectedError.Status().Code)) + Expect(*response.Result).Should(Equal(expectedError.Status())) + Expect(response.AdmissionResponse.Warnings).Should(BeEmpty()) + + }) + + }) + + Context("when dealing with Status errors, without warning messages", func() { expectedError := &apierrors.StatusError{ ErrStatus: metav1.Status{ @@ -150,8 +241,8 @@ var _ = Describe("validatingWarnHandler", func() { Code: http.StatusUnprocessableEntity, }, } - f := &admissiontest.FakeValidatorWarn{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: nil} - handler := validatingWarnHandler{validatorWarn: f, decoder: decoder} + f := &admissiontest.FakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: nil} + handler := validatingHandler{validator: f, decoder: decoder} It("should propagate the Status from ValidateCreate's return value to the HTTP response", func() { @@ -160,7 +251,7 @@ var _ = Describe("validatingWarnHandler", func() { Operation: admissionv1.Create, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, }, }) @@ -178,11 +269,11 @@ var _ = Describe("validatingWarnHandler", func() { Operation: admissionv1.Update, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, }, }) @@ -200,7 +291,7 @@ var _ = Describe("validatingWarnHandler", func() { Operation: admissionv1.Delete, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, }, }) @@ -216,8 +307,8 @@ var _ = Describe("validatingWarnHandler", func() { Context("when dealing with non-status errors, without warning messages", func() { expectedError := errors.New("some error") - f := &admissiontest.FakeValidatorWarn{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK} - handler := validatingWarnHandler{validatorWarn: f, decoder: decoder} + f := &admissiontest.FakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK} + handler := validatingHandler{validator: f, decoder: decoder} It("should return 403 response when ValidateCreate with error message embedded", func() { @@ -226,13 +317,14 @@ var _ = Describe("validatingWarnHandler", func() { Operation: admissionv1.Create, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, }, }) Expect(response.Allowed).Should(BeFalse()) Expect(response.Result.Code).Should(Equal(int32(http.StatusForbidden))) - Expect(string(response.Result.Reason)).Should(Equal(expectedError.Error())) + Expect(response.Result.Reason).Should(Equal(metav1.StatusReasonForbidden)) + Expect(response.Result.Message).Should(Equal(expectedError.Error())) }) @@ -243,17 +335,18 @@ var _ = Describe("validatingWarnHandler", func() { Operation: admissionv1.Update, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, }, }) Expect(response.Allowed).Should(BeFalse()) Expect(response.Result.Code).Should(Equal(int32(http.StatusForbidden))) - Expect(string(response.Result.Reason)).Should(Equal(expectedError.Error())) + Expect(response.Result.Reason).Should(Equal(metav1.StatusReasonForbidden)) + Expect(response.Result.Message).Should(Equal(expectedError.Error())) }) @@ -263,21 +356,22 @@ var _ = Describe("validatingWarnHandler", func() { Operation: admissionv1.Delete, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, }, }) Expect(response.Allowed).Should(BeFalse()) Expect(response.Result.Code).Should(Equal(int32(http.StatusForbidden))) - Expect(string(response.Result.Reason)).Should(Equal(expectedError.Error())) + Expect(response.Result.Reason).Should(Equal(metav1.StatusReasonForbidden)) + Expect(response.Result.Message).Should(Equal(expectedError.Error())) }) }) Context("when dealing with non-status errors, with warning messages", func() { expectedError := errors.New("some error") - f := &admissiontest.FakeValidatorWarn{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{warningMessage, anotherWarningMessage}} - handler := validatingWarnHandler{validatorWarn: f, decoder: decoder} + f := &admissiontest.FakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{warningMessage, anotherWarningMessage}} + handler := validatingHandler{validator: f, decoder: decoder} It("should return 403 response when ValidateCreate with error message embedded", func() { @@ -286,13 +380,14 @@ var _ = Describe("validatingWarnHandler", func() { Operation: admissionv1.Create, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, }, }) Expect(response.Allowed).Should(BeFalse()) Expect(response.Result.Code).Should(Equal(int32(http.StatusForbidden))) - Expect(string(response.Result.Reason)).Should(Equal(expectedError.Error())) + 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)) }) @@ -304,17 +399,18 @@ var _ = Describe("validatingWarnHandler", func() { Operation: admissionv1.Update, Object: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, }, }) Expect(response.Allowed).Should(BeFalse()) Expect(response.Result.Code).Should(Equal(int32(http.StatusForbidden))) - Expect(string(response.Result.Reason)).Should(Equal(expectedError.Error())) + 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)) @@ -326,17 +422,26 @@ var _ = Describe("validatingWarnHandler", func() { Operation: admissionv1.Delete, OldObject: runtime.RawExtension{ Raw: []byte("{}"), - Object: handler.validatorWarn, + Object: handler.validator, }, }, }) Expect(response.Allowed).Should(BeFalse()) Expect(response.Result.Code).Should(Equal(int32(http.StatusForbidden))) - Expect(string(response.Result.Reason)).Should(Equal(expectedError.Error())) + 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)) }) }) + PIt("should return 400 in response when create fails on decode", func() {}) + + PIt("should return 400 in response when update fails on decoding new object", func() {}) + + PIt("should return 400 in response when update fails on decoding old object", func() {}) + + PIt("should return 400 in response when delete fails on decode", func() {}) + }) From 6e2a023e4dd007600dd4797b2defb0a5212f6ac9 Mon Sep 17 00:00:00 2001 From: STRRL Date: Thu, 9 Feb 2023 09:33:12 +0800 Subject: [PATCH 07/13] chore: rename validator_warn_test.go to validator_test.go and fix compilation errors Signed-off-by: STRRL --- examples/builtins/validatingwebhook.go | 16 ++--- examples/crd/pkg/resource.go | 22 +++--- pkg/builder/webhook_test.go | 72 +++++++++---------- pkg/webhook/admission/validator.go | 6 +- ...lidator_warn_test.go => validator_test.go} | 0 pkg/webhook/alias.go | 3 - 6 files changed, 58 insertions(+), 61 deletions(-) rename pkg/webhook/admission/{validator_warn_test.go => validator_test.go} (100%) diff --git a/examples/builtins/validatingwebhook.go b/examples/builtins/validatingwebhook.go index e6094598bb..6e6f642bdd 100644 --- a/examples/builtins/validatingwebhook.go +++ b/examples/builtins/validatingwebhook.go @@ -32,34 +32,34 @@ import ( type podValidator struct{} // validate admits a pod if a specific annotation exists. -func (v *podValidator) validate(ctx context.Context, obj runtime.Object) error { +func (v *podValidator) validate(ctx context.Context, obj runtime.Object) ([]string, error) { log := logf.FromContext(ctx) pod, ok := obj.(*corev1.Pod) if !ok { - return fmt.Errorf("expected a Pod but got a %T", obj) + return nil, fmt.Errorf("expected a Pod but got a %T", obj) } log.Info("Validating Pod") key := "example-mutating-admission-webhook" anno, found := pod.Annotations[key] if !found { - return fmt.Errorf("missing annotation %s", key) + return nil, fmt.Errorf("missing annotation %s", key) } if anno != "foo" { - return fmt.Errorf("annotation %s did not have value %q", key, "foo") + return nil, fmt.Errorf("annotation %s did not have value %q", key, "foo") } - return nil + return nil, nil } -func (v *podValidator) ValidateCreate(ctx context.Context, obj runtime.Object) error { +func (v *podValidator) ValidateCreate(ctx context.Context, obj runtime.Object) ([]string, error) { return v.validate(ctx, obj) } -func (v *podValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) error { +func (v *podValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) ([]string, error) { return v.validate(ctx, newObj) } -func (v *podValidator) ValidateDelete(ctx context.Context, obj runtime.Object) error { +func (v *podValidator) ValidateDelete(ctx context.Context, obj runtime.Object) ([]string, error) { return v.validate(ctx, obj) } diff --git a/examples/crd/pkg/resource.go b/examples/crd/pkg/resource.go index 9c3d4c72bc..0b6581ea70 100644 --- a/examples/crd/pkg/resource.go +++ b/examples/crd/pkg/resource.go @@ -66,41 +66,41 @@ type ChaosPodList struct { var _ webhook.Validator = &ChaosPod{} // ValidateCreate implements webhookutil.validator so a webhook will be registered for the type -func (c *ChaosPod) ValidateCreate() error { +func (c *ChaosPod) ValidateCreate() ([]string, error) { log.Info("validate create", "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, fmt.Errorf(".spec.nextStop must be later than current time") } - return nil + return nil, nil } // ValidateUpdate implements webhookutil.validator so a webhook will be registered for the type -func (c *ChaosPod) ValidateUpdate(old runtime.Object) error { +func (c *ChaosPod) ValidateUpdate(old runtime.Object) ([]string, error) { log.Info("validate update", "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, fmt.Errorf(".spec.nextStop must be later than current time") } oldC, ok := old.(*ChaosPod) if !ok { - return fmt.Errorf("expect old object to be a %T instead of %T", oldC, old) + return nil, fmt.Errorf("expect old object to be a %T instead of %T", oldC, old) } if c.Spec.NextStop.After(oldC.Spec.NextStop.Add(time.Hour)) { - return fmt.Errorf("it is not allowed to delay.spec.nextStop for more than 1 hour") + return nil, fmt.Errorf("it is not allowed to delay.spec.nextStop for more than 1 hour") } - return nil + return nil, nil } // ValidateDelete implements webhookutil.validator so a webhook will be registered for the type -func (c *ChaosPod) ValidateDelete() error { +func (c *ChaosPod) ValidateDelete() ([]string, 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, fmt.Errorf(".spec.nextStop must be later than current time") } - return nil + return nil, 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 diff --git a/pkg/builder/webhook_test.go b/pkg/builder/webhook_test.go index 2ee1e7bfb4..bedb6b0206 100644 --- a/pkg/builder/webhook_test.go +++ b/pkg/builder/webhook_test.go @@ -749,39 +749,39 @@ func (*TestValidatorList) DeepCopyObject() runtime.Object { return nil } var _ admission.Validator = &TestValidator{} -func (v *TestValidator) ValidateCreate() error { +func (v *TestValidator) ValidateCreate() ([]string, error) { if v.Panic { panic("fake panic test") } if v.Replica < 0 { - return errors.New("number of replica should be greater than or equal to 0") + return nil, errors.New("number of replica should be greater than or equal to 0") } - return nil + return nil, nil } -func (v *TestValidator) ValidateUpdate(old runtime.Object) error { +func (v *TestValidator) ValidateUpdate(old runtime.Object) ([]string, error) { if v.Panic { panic("fake panic test") } if v.Replica < 0 { - return errors.New("number of replica should be greater than or equal to 0") + return nil, errors.New("number of replica should be greater than or equal to 0") } if oldObj, ok := old.(*TestValidator); !ok { - return fmt.Errorf("the old object is expected to be %T", oldObj) + return nil, fmt.Errorf("the old object is expected to be %T", oldObj) } else if v.Replica < oldObj.Replica { - return fmt.Errorf("new replica %v should not be fewer than old replica %v", 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 + return nil, nil } -func (v *TestValidator) ValidateDelete() error { +func (v *TestValidator) ValidateDelete() ([]string, error) { if v.Panic { panic("fake panic test") } if v.Replica > 0 { - return errors.New("number of replica should be less than or equal to 0 to delete") + return nil, errors.New("number of replica should be less than or equal to 0 to delete") } - return nil + return nil, nil } // TestDefaultValidator. @@ -824,25 +824,25 @@ func (dv *TestDefaultValidator) Default() { var _ admission.Validator = &TestDefaultValidator{} -func (dv *TestDefaultValidator) ValidateCreate() error { +func (dv *TestDefaultValidator) ValidateCreate() ([]string, error) { if dv.Replica < 0 { - return errors.New("number of replica should be greater than or equal to 0") + return nil, errors.New("number of replica should be greater than or equal to 0") } - return nil + return nil, nil } -func (dv *TestDefaultValidator) ValidateUpdate(old runtime.Object) error { +func (dv *TestDefaultValidator) ValidateUpdate(old runtime.Object) ([]string, error) { if dv.Replica < 0 { - return errors.New("number of replica should be greater than or equal to 0") + return nil, errors.New("number of replica should be greater than or equal to 0") } - return nil + return nil, nil } -func (dv *TestDefaultValidator) ValidateDelete() error { +func (dv *TestDefaultValidator) ValidateDelete() ([]string, error) { if dv.Replica > 0 { - return errors.New("number of replica should be less than or equal to 0 to delete") + return nil, errors.New("number of replica should be less than or equal to 0 to delete") } - return nil + return nil, nil } // TestCustomDefaulter. @@ -872,59 +872,59 @@ var _ admission.CustomDefaulter = &TestCustomDefaulter{} type TestCustomValidator struct{} -func (*TestCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) error { +func (*TestCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) ([]string, error) { logf.FromContext(ctx).Info("Validating object") req, err := admission.RequestFromContext(ctx) if err != nil { - return fmt.Errorf("expected admission.Request in ctx: %w", err) + return nil, fmt.Errorf("expected admission.Request in ctx: %w", err) } if req.Kind.Kind != testValidatorKind { - return fmt.Errorf("expected Kind TestValidator got %q", req.Kind.Kind) + return nil, fmt.Errorf("expected Kind TestValidator got %q", req.Kind.Kind) } 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, errors.New("number of replica should be greater than or equal to 0") } - return nil + return nil, nil } -func (*TestCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) error { +func (*TestCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) ([]string, error) { logf.FromContext(ctx).Info("Validating object") req, err := admission.RequestFromContext(ctx) if err != nil { - return fmt.Errorf("expected admission.Request in ctx: %w", err) + return nil, fmt.Errorf("expected admission.Request in ctx: %w", err) } if req.Kind.Kind != testValidatorKind { - return fmt.Errorf("expected Kind TestValidator got %q", req.Kind.Kind) + return nil, fmt.Errorf("expected Kind TestValidator got %q", req.Kind.Kind) } v := newObj.(*TestValidator) old := oldObj.(*TestValidator) if v.Replica < 0 { - return errors.New("number of replica should be greater than or equal to 0") + return nil, 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, fmt.Errorf("new replica %v should not be fewer than old replica %v", v.Replica, old.Replica) } - return nil + return nil, nil } -func (*TestCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) error { +func (*TestCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) ([]string, error) { logf.FromContext(ctx).Info("Validating object") req, err := admission.RequestFromContext(ctx) if err != nil { - return fmt.Errorf("expected admission.Request in ctx: %w", err) + return nil, fmt.Errorf("expected admission.Request in ctx: %w", err) } if req.Kind.Kind != testValidatorKind { - return fmt.Errorf("expected Kind TestValidator got %q", req.Kind.Kind) + return nil, fmt.Errorf("expected Kind TestValidator got %q", req.Kind.Kind) } 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, errors.New("number of replica should be less than or equal to 0 to delete") } - return nil + return nil, nil } var _ admission.CustomValidator = &TestCustomValidator{} diff --git a/pkg/webhook/admission/validator.go b/pkg/webhook/admission/validator.go index 15cdfaf819..272cc07aa5 100644 --- a/pkg/webhook/admission/validator.go +++ b/pkg/webhook/admission/validator.go @@ -55,9 +55,6 @@ func (h *validatingHandler) Handle(ctx context.Context, req Request) Response { if h.validator == nil { panic("validator should never be nil") } - - ctx = NewContextWithRequest(ctx, req) - // Get the object in the request obj := h.validator.DeepCopyObject().(Validator) @@ -65,6 +62,9 @@ func (h *validatingHandler) Handle(ctx context.Context, req Request) Response { 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: err = h.decoder.Decode(req, obj) if err != nil { diff --git a/pkg/webhook/admission/validator_warn_test.go b/pkg/webhook/admission/validator_test.go similarity index 100% rename from pkg/webhook/admission/validator_warn_test.go rename to pkg/webhook/admission/validator_test.go diff --git a/pkg/webhook/alias.go b/pkg/webhook/alias.go index c1b1e88228..293137db49 100644 --- a/pkg/webhook/alias.go +++ b/pkg/webhook/alias.go @@ -29,9 +29,6 @@ type Defaulter = admission.Defaulter // Validator defines functions for validating an operation. type Validator = admission.Validator -// ValidatorWarn like Validator, but could return warnings. -type ValidatorWarn = admission.ValidatorWarn - // CustomDefaulter defines functions for setting defaults on resources. type CustomDefaulter = admission.CustomDefaulter From 844f97aff94a6d2272fe27f2fef9eb9a0626d55b Mon Sep 17 00:00:00 2001 From: STRRL Date: Fri, 10 Feb 2023 20:10:16 +0800 Subject: [PATCH 08/13] chore: append comments for modified Validator and CustomValidator Signed-off-by: STRRL --- pkg/webhook/admission/validator.go | 20 +++++++++++++++++--- pkg/webhook/admission/validator_custom.go | 19 ++++++++++++++++--- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/pkg/webhook/admission/validator.go b/pkg/webhook/admission/validator.go index 272cc07aa5..9f333dc5cd 100644 --- a/pkg/webhook/admission/validator.go +++ b/pkg/webhook/admission/validator.go @@ -28,11 +28,25 @@ import ( ) // 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. type Validator interface { runtime.Object - ValidateCreate() ([]string, error) - ValidateUpdate(old runtime.Object) ([]string, error) - ValidateDelete() ([]string, error) + + // 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 []string, 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 []string, 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 []string, err error) } // ValidatingWebhookFor creates a new Webhook for validating the provided type. diff --git a/pkg/webhook/admission/validator_custom.go b/pkg/webhook/admission/validator_custom.go index c966c4abb1..cf2b14b36c 100644 --- a/pkg/webhook/admission/validator_custom.go +++ b/pkg/webhook/admission/validator_custom.go @@ -28,10 +28,23 @@ import ( ) // CustomValidator defines functions for validating an operation. +// The object to be validated is passed into methods as a parameter. type CustomValidator interface { - ValidateCreate(ctx context.Context, obj runtime.Object) ([]string, error) - ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) ([]string, error) - ValidateDelete(ctx context.Context, obj runtime.Object) ([]string, error) + + // 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(ctx context.Context, obj runtime.Object) (warnings []string, err error) + + // ValidateUpdate validates the object on update. + // The optional warnings will be added to the response as warning messages. + // Return an error if the object is invalid. + ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings []string, 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(ctx context.Context, obj runtime.Object) (warnings []string, err error) } // WithCustomValidator creates a new Webhook for validating the provided type. From 6fe2ffd0b086ce0bd6951bb4e59caf0727321814 Mon Sep 17 00:00:00 2001 From: STRRL Date: Tue, 21 Feb 2023 22:50:14 +0800 Subject: [PATCH 09/13] chore: address the comments Signed-off-by: STRRL --- pkg/webhook/admission/admissiontest/util.go | 7 ++++--- pkg/webhook/admission/validator.go | 5 ++--- pkg/webhook/admission/validator_custom.go | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/webhook/admission/admissiontest/util.go b/pkg/webhook/admission/admissiontest/util.go index f63456d34d..385a92ec31 100644 --- a/pkg/webhook/admission/admissiontest/util.go +++ b/pkg/webhook/admission/admissiontest/util.go @@ -47,8 +47,8 @@ func (v *FakeValidator) ValidateDelete() (warnings []string, err error) { return v.WarningsToReturn, v.ErrorToReturn } -func (v *FakeValidator) SetGroupVersionKind(kind schema.GroupVersionKind) { - v.GVKToReturn = kind +func (v *FakeValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) { + v.GVKToReturn = gvk } func (v *FakeValidator) GroupVersionKind() schema.GroupVersionKind { @@ -60,7 +60,8 @@ func (v *FakeValidator) GetObjectKind() schema.ObjectKind { } func (v *FakeValidator) DeepCopyObject() runtime.Object { - return &FakeValidator{ErrorToReturn: v.ErrorToReturn, + return &FakeValidator{ + ErrorToReturn: v.ErrorToReturn, GVKToReturn: v.GVKToReturn, WarningsToReturn: v.WarningsToReturn, } diff --git a/pkg/webhook/admission/validator.go b/pkg/webhook/admission/validator.go index 9f333dc5cd..ce5061d0eb 100644 --- a/pkg/webhook/admission/validator.go +++ b/pkg/webhook/admission/validator.go @@ -80,8 +80,7 @@ func (h *validatingHandler) Handle(ctx context.Context, req Request) Response { // No validation for connect requests. // TODO(vincepri): Should we validate CONNECT requests? In what cases? case v1.Create: - err = h.decoder.Decode(req, obj) - if err != nil { + if err = h.decoder.Decode(req, obj); err != nil { return Errored(http.StatusBadRequest, err) } @@ -109,7 +108,7 @@ func (h *validatingHandler) Handle(ctx context.Context, req Request) Response { warnings, err = obj.ValidateDelete() default: - return Errored(http.StatusBadRequest, fmt.Errorf("unknown operation request %q", req.Operation)) + return Errored(http.StatusBadRequest, fmt.Errorf("unknown operation %q", req.Operation)) } if err != nil { diff --git a/pkg/webhook/admission/validator_custom.go b/pkg/webhook/admission/validator_custom.go index cf2b14b36c..835f5dae09 100644 --- a/pkg/webhook/admission/validator_custom.go +++ b/pkg/webhook/admission/validator_custom.go @@ -109,7 +109,7 @@ func (h *validatorForType) Handle(ctx context.Context, req Request) Response { warnings, err = h.validator.ValidateDelete(ctx, obj) default: - return Errored(http.StatusBadRequest, fmt.Errorf("unknown operation request %q", req.Operation)) + return Errored(http.StatusBadRequest, fmt.Errorf("unknown operation %q", req.Operation)) } // Check the error message first. From 1e1dd76fd7c43935e180b9a84098fff8f316dc7d Mon Sep 17 00:00:00 2001 From: STRRL Date: Thu, 13 Apr 2023 22:15:22 +0800 Subject: [PATCH 10/13] refactor: move and unexport FakeValidator for unit tests in admission webhook Signed-off-by: STRRL --- pkg/webhook/admission/admissiontest/doc.go | 18 ------------- .../util.go => fake_validator_test.go} | 26 +++++++++---------- pkg/webhook/admission/validator_test.go | 14 +++++----- 3 files changed, 19 insertions(+), 39 deletions(-) delete mode 100644 pkg/webhook/admission/admissiontest/doc.go rename pkg/webhook/admission/{admissiontest/util.go => fake_validator_test.go} (68%) diff --git a/pkg/webhook/admission/admissiontest/doc.go b/pkg/webhook/admission/admissiontest/doc.go deleted file mode 100644 index b4a7a42191..0000000000 --- a/pkg/webhook/admission/admissiontest/doc.go +++ /dev/null @@ -1,18 +0,0 @@ -/* -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 admissiontest contains fake webhooks for validating admission webhooks -package admissiontest diff --git a/pkg/webhook/admission/admissiontest/util.go b/pkg/webhook/admission/fake_validator_test.go similarity index 68% rename from pkg/webhook/admission/admissiontest/util.go rename to pkg/webhook/admission/fake_validator_test.go index 385a92ec31..f5bb999ace 100644 --- a/pkg/webhook/admission/admissiontest/util.go +++ b/pkg/webhook/admission/fake_validator_test.go @@ -14,53 +14,53 @@ See the License for the specific language governing permissions and limitations under the License. */ -package admissiontest +package admission import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" ) -// FakeValidator provides fake validating webhook functionality for testing +// fakeValidator provides fake validating webhook functionality for testing // It implements the admission.Validator 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 { - // ErrorToReturn is the error for which the FakeValidator rejects all requests +type fakeValidator 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 GVKToReturn schema.GroupVersionKind - // WarningsToReturn is the warnings for FakeValidator returns to all requests + // WarningsToReturn is the warnings for fakeValidator returns to all requests WarningsToReturn []string } -func (v *FakeValidator) ValidateCreate() (warnings []string, err error) { +func (v *fakeValidator) ValidateCreate() (warnings []string, err error) { return v.WarningsToReturn, v.ErrorToReturn } -func (v *FakeValidator) ValidateUpdate(old runtime.Object) (warnings []string, err error) { +func (v *fakeValidator) ValidateUpdate(old runtime.Object) (warnings []string, err error) { return v.WarningsToReturn, v.ErrorToReturn } -func (v *FakeValidator) ValidateDelete() (warnings []string, err error) { +func (v *fakeValidator) ValidateDelete() (warnings []string, err error) { return v.WarningsToReturn, v.ErrorToReturn } -func (v *FakeValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) { +func (v *fakeValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) { v.GVKToReturn = gvk } -func (v *FakeValidator) GroupVersionKind() schema.GroupVersionKind { +func (v *fakeValidator) GroupVersionKind() schema.GroupVersionKind { return v.GVKToReturn } -func (v *FakeValidator) GetObjectKind() schema.ObjectKind { +func (v *fakeValidator) GetObjectKind() schema.ObjectKind { return v } -func (v *FakeValidator) DeepCopyObject() runtime.Object { - return &FakeValidator{ +func (v *fakeValidator) DeepCopyObject() runtime.Object { + return &fakeValidator{ ErrorToReturn: v.ErrorToReturn, GVKToReturn: v.GVKToReturn, WarningsToReturn: v.WarningsToReturn, diff --git a/pkg/webhook/admission/validator_test.go b/pkg/webhook/admission/validator_test.go index 5f8957f9d0..25e719863e 100644 --- a/pkg/webhook/admission/validator_test.go +++ b/pkg/webhook/admission/validator_test.go @@ -23,8 +23,6 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - "sigs.k8s.io/controller-runtime/pkg/webhook/admission/admissiontest" - admissionv1 "k8s.io/api/admission/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -40,7 +38,7 @@ var _ = Describe("validatingHandler", func() { decoder := NewDecoder(scheme.Scheme) Context("when dealing with successful results without warning", func() { - f := &admissiontest.FakeValidator{ErrorToReturn: nil, GVKToReturn: fakeValidatorVK, WarningsToReturn: nil} + f := &fakeValidator{ErrorToReturn: nil, GVKToReturn: fakeValidatorVK, WarningsToReturn: nil} handler := validatingHandler{validator: f, decoder: decoder} It("should return 200 in response when create succeeds", func() { @@ -97,7 +95,7 @@ var _ = Describe("validatingHandler", func() { const warningMessage = "warning message" const anotherWarningMessage = "another warning message" Context("when dealing with successful results with warning", func() { - f := &admissiontest.FakeValidator{ErrorToReturn: nil, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{ + f := &fakeValidator{ErrorToReturn: nil, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{ warningMessage, anotherWarningMessage, }} @@ -167,7 +165,7 @@ var _ = Describe("validatingHandler", func() { Code: http.StatusUnprocessableEntity, }, } - f := &admissiontest.FakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{warningMessage, anotherWarningMessage}} + f := &fakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{warningMessage, anotherWarningMessage}} handler := validatingHandler{validator: f, decoder: decoder} It("should propagate the Status from ValidateCreate's return value to the HTTP response", func() { @@ -241,7 +239,7 @@ var _ = Describe("validatingHandler", func() { Code: http.StatusUnprocessableEntity, }, } - f := &admissiontest.FakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: nil} + f := &fakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: nil} handler := validatingHandler{validator: f, decoder: decoder} It("should propagate the Status from ValidateCreate's return value to the HTTP response", func() { @@ -307,7 +305,7 @@ var _ = Describe("validatingHandler", func() { Context("when dealing with non-status errors, without warning messages", func() { expectedError := errors.New("some error") - f := &admissiontest.FakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK} + f := &fakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK} handler := validatingHandler{validator: f, decoder: decoder} It("should return 403 response when ValidateCreate with error message embedded", func() { @@ -370,7 +368,7 @@ var _ = Describe("validatingHandler", func() { Context("when dealing with non-status errors, with warning messages", func() { expectedError := errors.New("some error") - f := &admissiontest.FakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{warningMessage, anotherWarningMessage}} + f := &fakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{warningMessage, anotherWarningMessage}} handler := validatingHandler{validator: f, decoder: decoder} It("should return 403 response when ValidateCreate with error message embedded", func() { From 8770b4d3b5425b156f48c971ee140ea25f9c1bfa Mon Sep 17 00:00:00 2001 From: STRRL Date: Thu, 13 Apr 2023 22:18:44 +0800 Subject: [PATCH 11/13] refactor: introduce new type as Warnings Signed-off-by: STRRL --- examples/builtins/validatingwebhook.go | 9 +++++---- examples/crd/pkg/resource.go | 7 ++++--- pkg/builder/webhook_test.go | 18 +++++++++--------- pkg/webhook/admission/fake_validator_test.go | 6 +++--- pkg/webhook/admission/validator.go | 9 ++++++--- pkg/webhook/admission/validator_custom.go | 6 +++--- 6 files changed, 30 insertions(+), 25 deletions(-) diff --git a/examples/builtins/validatingwebhook.go b/examples/builtins/validatingwebhook.go index 6e6f642bdd..1fe40f80c4 100644 --- a/examples/builtins/validatingwebhook.go +++ b/examples/builtins/validatingwebhook.go @@ -24,6 +24,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) // +kubebuilder:webhook:path=/validate-v1-pod,mutating=false,failurePolicy=fail,groups="",resources=pods,verbs=create;update,versions=v1,name=vpod.kb.io @@ -32,7 +33,7 @@ import ( type podValidator struct{} // validate admits a pod if a specific annotation exists. -func (v *podValidator) validate(ctx context.Context, obj runtime.Object) ([]string, error) { +func (v *podValidator) validate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { log := logf.FromContext(ctx) pod, ok := obj.(*corev1.Pod) if !ok { @@ -52,14 +53,14 @@ func (v *podValidator) validate(ctx context.Context, obj runtime.Object) ([]stri return nil, nil } -func (v *podValidator) ValidateCreate(ctx context.Context, obj runtime.Object) ([]string, error) { +func (v *podValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { return v.validate(ctx, obj) } -func (v *podValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) ([]string, error) { +func (v *podValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { return v.validate(ctx, newObj) } -func (v *podValidator) ValidateDelete(ctx context.Context, obj runtime.Object) ([]string, error) { +func (v *podValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { return v.validate(ctx, obj) } diff --git a/examples/crd/pkg/resource.go b/examples/crd/pkg/resource.go index 0b6581ea70..555029f5de 100644 --- a/examples/crd/pkg/resource.go +++ b/examples/crd/pkg/resource.go @@ -24,6 +24,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" ) // ChaosPodSpec defines the desired state of ChaosPod @@ -66,7 +67,7 @@ type ChaosPodList struct { var _ webhook.Validator = &ChaosPod{} // ValidateCreate implements webhookutil.validator so a webhook will be registered for the type -func (c *ChaosPod) ValidateCreate() ([]string, error) { +func (c *ChaosPod) ValidateCreate() (admission.Warnings, error) { log.Info("validate create", "name", c.Name) if c.Spec.NextStop.Before(&metav1.Time{Time: time.Now()}) { @@ -76,7 +77,7 @@ func (c *ChaosPod) ValidateCreate() ([]string, error) { } // ValidateUpdate implements webhookutil.validator so a webhook will be registered for the type -func (c *ChaosPod) ValidateUpdate(old runtime.Object) ([]string, error) { +func (c *ChaosPod) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { log.Info("validate update", "name", c.Name) if c.Spec.NextStop.Before(&metav1.Time{Time: time.Now()}) { @@ -94,7 +95,7 @@ func (c *ChaosPod) ValidateUpdate(old runtime.Object) ([]string, error) { } // ValidateDelete implements webhookutil.validator so a webhook will be registered for the type -func (c *ChaosPod) ValidateDelete() ([]string, error) { +func (c *ChaosPod) ValidateDelete() (admission.Warnings, error) { log.Info("validate delete", "name", c.Name) if c.Spec.NextStop.Before(&metav1.Time{Time: time.Now()}) { diff --git a/pkg/builder/webhook_test.go b/pkg/builder/webhook_test.go index bedb6b0206..fee86562bc 100644 --- a/pkg/builder/webhook_test.go +++ b/pkg/builder/webhook_test.go @@ -749,7 +749,7 @@ func (*TestValidatorList) DeepCopyObject() runtime.Object { return nil } var _ admission.Validator = &TestValidator{} -func (v *TestValidator) ValidateCreate() ([]string, error) { +func (v *TestValidator) ValidateCreate() (admission.Warnings, error) { if v.Panic { panic("fake panic test") } @@ -759,7 +759,7 @@ func (v *TestValidator) ValidateCreate() ([]string, error) { return nil, nil } -func (v *TestValidator) ValidateUpdate(old runtime.Object) ([]string, error) { +func (v *TestValidator) ValidateUpdate(old runtime.Object) (admission.Warnings, error) { if v.Panic { panic("fake panic test") } @@ -774,7 +774,7 @@ func (v *TestValidator) ValidateUpdate(old runtime.Object) ([]string, error) { return nil, nil } -func (v *TestValidator) ValidateDelete() ([]string, error) { +func (v *TestValidator) ValidateDelete() (admission.Warnings, error) { if v.Panic { panic("fake panic test") } @@ -824,21 +824,21 @@ func (dv *TestDefaultValidator) Default() { var _ admission.Validator = &TestDefaultValidator{} -func (dv *TestDefaultValidator) ValidateCreate() ([]string, error) { +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) ([]string, error) { +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() ([]string, error) { +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") } @@ -872,7 +872,7 @@ var _ admission.CustomDefaulter = &TestCustomDefaulter{} type TestCustomValidator struct{} -func (*TestCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) ([]string, error) { +func (*TestCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { logf.FromContext(ctx).Info("Validating object") req, err := admission.RequestFromContext(ctx) if err != nil { @@ -889,7 +889,7 @@ func (*TestCustomValidator) ValidateCreate(ctx context.Context, obj runtime.Obje return nil, nil } -func (*TestCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) ([]string, error) { +func (*TestCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { logf.FromContext(ctx).Info("Validating object") req, err := admission.RequestFromContext(ctx) if err != nil { @@ -910,7 +910,7 @@ func (*TestCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj r return nil, nil } -func (*TestCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) ([]string, error) { +func (*TestCustomValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { logf.FromContext(ctx).Info("Validating object") req, err := admission.RequestFromContext(ctx) if err != nil { diff --git a/pkg/webhook/admission/fake_validator_test.go b/pkg/webhook/admission/fake_validator_test.go index f5bb999ace..e4acf63731 100644 --- a/pkg/webhook/admission/fake_validator_test.go +++ b/pkg/webhook/admission/fake_validator_test.go @@ -35,15 +35,15 @@ type fakeValidator struct { WarningsToReturn []string } -func (v *fakeValidator) ValidateCreate() (warnings []string, err error) { +func (v *fakeValidator) ValidateCreate() (warnings Warnings, err error) { return v.WarningsToReturn, v.ErrorToReturn } -func (v *fakeValidator) ValidateUpdate(old runtime.Object) (warnings []string, err error) { +func (v *fakeValidator) ValidateUpdate(old runtime.Object) (warnings Warnings, err error) { return v.WarningsToReturn, v.ErrorToReturn } -func (v *fakeValidator) ValidateDelete() (warnings []string, err error) { +func (v *fakeValidator) ValidateDelete() (warnings Warnings, err error) { return v.WarningsToReturn, v.ErrorToReturn } diff --git a/pkg/webhook/admission/validator.go b/pkg/webhook/admission/validator.go index ce5061d0eb..fae2c94991 100644 --- a/pkg/webhook/admission/validator.go +++ b/pkg/webhook/admission/validator.go @@ -27,6 +27,9 @@ import ( "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. @@ -36,17 +39,17 @@ type Validator interface { // 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 []string, err error) + 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 []string, err error) + 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 []string, err error) + ValidateDelete() (warnings Warnings, err error) } // ValidatingWebhookFor creates a new Webhook for validating the provided type. diff --git a/pkg/webhook/admission/validator_custom.go b/pkg/webhook/admission/validator_custom.go index 835f5dae09..72f76256ea 100644 --- a/pkg/webhook/admission/validator_custom.go +++ b/pkg/webhook/admission/validator_custom.go @@ -34,17 +34,17 @@ type CustomValidator interface { // 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(ctx context.Context, obj runtime.Object) (warnings []string, err error) + ValidateCreate(ctx context.Context, obj runtime.Object) (warnings Warnings, err error) // ValidateUpdate validates the object on update. // The optional warnings will be added to the response as warning messages. // Return an error if the object is invalid. - ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (warnings []string, err error) + ValidateUpdate(ctx context.Context, oldObj, newObj 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(ctx context.Context, obj runtime.Object) (warnings []string, err error) + ValidateDelete(ctx context.Context, obj runtime.Object) (warnings Warnings, err error) } // WithCustomValidator creates a new Webhook for validating the provided type. From 0d248e2d7564cbf9c0a9410f3bc1e50f2b3ae639 Mon Sep 17 00:00:00 2001 From: STRRL Date: Fri, 14 Apr 2023 09:43:23 +0800 Subject: [PATCH 12/13] chore: move fakeValidator from individual file into Validator_test.go Signed-off-by: STRRL --- pkg/webhook/admission/fake_validator_test.go | 68 -------------------- pkg/webhook/admission/validator_test.go | 46 +++++++++++++ 2 files changed, 46 insertions(+), 68 deletions(-) delete mode 100644 pkg/webhook/admission/fake_validator_test.go diff --git a/pkg/webhook/admission/fake_validator_test.go b/pkg/webhook/admission/fake_validator_test.go deleted file mode 100644 index e4acf63731..0000000000 --- a/pkg/webhook/admission/fake_validator_test.go +++ /dev/null @@ -1,68 +0,0 @@ -/* -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 ( - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/runtime/schema" -) - -// fakeValidator provides fake validating webhook functionality for testing -// It implements the admission.Validator 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 { - // 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 - GVKToReturn schema.GroupVersionKind - // WarningsToReturn is the warnings for fakeValidator returns to all requests - WarningsToReturn []string -} - -func (v *fakeValidator) ValidateCreate() (warnings Warnings, err error) { - return v.WarningsToReturn, v.ErrorToReturn -} - -func (v *fakeValidator) ValidateUpdate(old runtime.Object) (warnings Warnings, err error) { - return v.WarningsToReturn, v.ErrorToReturn -} - -func (v *fakeValidator) ValidateDelete() (warnings Warnings, err error) { - return v.WarningsToReturn, v.ErrorToReturn -} - -func (v *fakeValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) { - v.GVKToReturn = gvk -} - -func (v *fakeValidator) GroupVersionKind() schema.GroupVersionKind { - return v.GVKToReturn -} - -func (v *fakeValidator) GetObjectKind() schema.ObjectKind { - return v -} - -func (v *fakeValidator) DeepCopyObject() runtime.Object { - return &fakeValidator{ - ErrorToReturn: v.ErrorToReturn, - GVKToReturn: v.GVKToReturn, - WarningsToReturn: v.WarningsToReturn, - } -} diff --git a/pkg/webhook/admission/validator_test.go b/pkg/webhook/admission/validator_test.go index 25e719863e..a1f1dd18a3 100644 --- a/pkg/webhook/admission/validator_test.go +++ b/pkg/webhook/admission/validator_test.go @@ -443,3 +443,49 @@ var _ = Describe("validatingHandler", func() { PIt("should return 400 in response when delete fails on decode", func() {}) }) + +// fakeValidator provides fake validating webhook functionality for testing +// It implements the admission.Validator 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 { + // 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 + GVKToReturn schema.GroupVersionKind + // WarningsToReturn is the warnings for fakeValidator returns to all requests + WarningsToReturn []string +} + +func (v *fakeValidator) ValidateCreate() (warnings Warnings, err error) { + return v.WarningsToReturn, v.ErrorToReturn +} + +func (v *fakeValidator) ValidateUpdate(old runtime.Object) (warnings Warnings, err error) { + return v.WarningsToReturn, v.ErrorToReturn +} + +func (v *fakeValidator) ValidateDelete() (warnings Warnings, err error) { + return v.WarningsToReturn, v.ErrorToReturn +} + +func (v *fakeValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) { + v.GVKToReturn = gvk +} + +func (v *fakeValidator) GroupVersionKind() schema.GroupVersionKind { + return v.GVKToReturn +} + +func (v *fakeValidator) GetObjectKind() schema.ObjectKind { + return v +} + +func (v *fakeValidator) DeepCopyObject() runtime.Object { + return &fakeValidator{ + ErrorToReturn: v.ErrorToReturn, + GVKToReturn: v.GVKToReturn, + WarningsToReturn: v.WarningsToReturn, + } +} From b2a955209912f188758a1ae2281f8cea691a6126 Mon Sep 17 00:00:00 2001 From: STRRL Date: Mon, 17 Apr 2023 15:24:17 +0800 Subject: [PATCH 13/13] chore: warning message should also be carried with APIStatusError Signed-off-by: STRRL --- pkg/webhook/admission/validator.go | 2 +- pkg/webhook/admission/validator_custom.go | 2 +- pkg/webhook/admission/validator_test.go | 9 ++++++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/webhook/admission/validator.go b/pkg/webhook/admission/validator.go index fae2c94991..00bda8a4ce 100644 --- a/pkg/webhook/admission/validator.go +++ b/pkg/webhook/admission/validator.go @@ -117,7 +117,7 @@ func (h *validatingHandler) Handle(ctx context.Context, req Request) Response { if err != nil { var apiStatus apierrors.APIStatus if errors.As(err, &apiStatus) { - return validationResponseFromStatus(false, apiStatus.Status()) + return validationResponseFromStatus(false, apiStatus.Status()).WithWarnings(warnings...) } return Denied(err.Error()).WithWarnings(warnings...) } diff --git a/pkg/webhook/admission/validator_custom.go b/pkg/webhook/admission/validator_custom.go index 72f76256ea..e99fbd8a85 100644 --- a/pkg/webhook/admission/validator_custom.go +++ b/pkg/webhook/admission/validator_custom.go @@ -116,7 +116,7 @@ func (h *validatorForType) Handle(ctx context.Context, req Request) Response { if err != nil { var apiStatus apierrors.APIStatus if errors.As(err, &apiStatus) { - return validationResponseFromStatus(false, apiStatus.Status()) + return validationResponseFromStatus(false, apiStatus.Status()).WithWarnings(warnings...) } return Denied(err.Error()).WithWarnings(warnings...) } diff --git a/pkg/webhook/admission/validator_test.go b/pkg/webhook/admission/validator_test.go index a1f1dd18a3..404fad9016 100644 --- a/pkg/webhook/admission/validator_test.go +++ b/pkg/webhook/admission/validator_test.go @@ -183,7 +183,8 @@ var _ = Describe("validatingHandler", func() { Expect(response.Allowed).Should(BeFalse()) Expect(response.Result.Code).Should(Equal(expectedError.Status().Code)) Expect(*response.Result).Should(Equal(expectedError.Status())) - Expect(response.AdmissionResponse.Warnings).Should(BeEmpty()) + Expect(response.AdmissionResponse.Warnings).Should(ContainElements(warningMessage)) + Expect(response.AdmissionResponse.Warnings).Should(ContainElements(anotherWarningMessage)) }) @@ -206,7 +207,8 @@ var _ = Describe("validatingHandler", func() { Expect(response.Allowed).Should(BeFalse()) Expect(response.Result.Code).Should(Equal(expectedError.Status().Code)) Expect(*response.Result).Should(Equal(expectedError.Status())) - Expect(response.AdmissionResponse.Warnings).Should(BeEmpty()) + Expect(response.AdmissionResponse.Warnings).Should(ContainElements(warningMessage)) + Expect(response.AdmissionResponse.Warnings).Should(ContainElements(anotherWarningMessage)) }) @@ -225,7 +227,8 @@ var _ = Describe("validatingHandler", func() { Expect(response.Allowed).Should(BeFalse()) Expect(response.Result.Code).Should(Equal(expectedError.Status().Code)) Expect(*response.Result).Should(Equal(expectedError.Status())) - Expect(response.AdmissionResponse.Warnings).Should(BeEmpty()) + Expect(response.AdmissionResponse.Warnings).Should(ContainElements(warningMessage)) + Expect(response.AdmissionResponse.Warnings).Should(ContainElements(anotherWarningMessage)) })