diff --git a/examples/builtins/validatingwebhook.go b/examples/builtins/validatingwebhook.go index e6094598bb..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,34 +33,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) (admission.Warnings, 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) (admission.Warnings, 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) (admission.Warnings, 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) (admission.Warnings, error) { return v.validate(ctx, obj) } diff --git a/examples/crd/pkg/resource.go b/examples/crd/pkg/resource.go index 9c3d4c72bc..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,41 +67,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() (admission.Warnings, 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) (admission.Warnings, 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() (admission.Warnings, 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..fee86562bc 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() (admission.Warnings, 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) (admission.Warnings, 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() (admission.Warnings, 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() (admission.Warnings, 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) (admission.Warnings, 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() (admission.Warnings, 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) (admission.Warnings, 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) (admission.Warnings, 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) (admission.Warnings, 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/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/admissiontest/util.go deleted file mode 100644 index 6e35a73907..0000000000 --- a/pkg/webhook/admission/admissiontest/util.go +++ /dev/null @@ -1,66 +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 - -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. -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 -} diff --git a/pkg/webhook/admission/validator.go b/pkg/webhook/admission/validator.go index 43ea3ee65f..00bda8a4ce 100644 --- a/pkg/webhook/admission/validator.go +++ b/pkg/webhook/admission/validator.go @@ -18,7 +18,8 @@ package admission import ( "context" - goerrors "errors" + "errors" + "fmt" "net/http" v1 "k8s.io/api/admission/v1" @@ -26,12 +27,29 @@ 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. type Validator interface { runtime.Object - ValidateCreate() error - ValidateUpdate(old runtime.Object) error - ValidateDelete() 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 Warnings, err error) + + // ValidateUpdate validates the object on update. The oldObj is the object before the update. + // The optional warnings will be added to the response as warning messages. + // Return an error if the object is invalid. + ValidateUpdate(old runtime.Object) (warnings Warnings, err error) + + // ValidateDelete validates the object on deletion. + // The optional warnings will be added to the response as warning messages. + // Return an error if the object is invalid. + ValidateDelete() (warnings Warnings, err error) } // ValidatingWebhookFor creates a new Webhook for validating the provided type. @@ -54,29 +72,26 @@ func (h *validatingHandler) Handle(ctx context.Context, req Request) Response { if h.validator == nil { panic("validator should never be nil") } - // 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() - if err != nil { - var apiStatus apierrors.APIStatus - if goerrors.As(err, &apiStatus) { - return validationResponseFromStatus(false, apiStatus.Status()) - } - return Denied(err.Error()) + var err error + var warnings []string + + switch req.Operation { + case v1.Connect: + // No validation for connect requests. + // TODO(vincepri): Should we validate CONNECT requests? In what cases? + case v1.Create: + if err = h.decoder.Decode(req, obj); err != nil { + return Errored(http.StatusBadRequest, err) } - } - 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 +100,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 goerrors.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 goerrors.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 %q", req.Operation)) } - return Allowed("") + if err != nil { + var apiStatus apierrors.APIStatus + if errors.As(err, &apiStatus) { + return validationResponseFromStatus(false, apiStatus.Status()).WithWarnings(warnings...) + } + return Denied(err.Error()).WithWarnings(warnings...) + } + return Allowed("").WithWarnings(warnings...) } diff --git a/pkg/webhook/admission/validator_custom.go b/pkg/webhook/admission/validator_custom.go index 755e464a91..e99fbd8a85 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) error - ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) error - ValidateDelete(ctx context.Context, obj runtime.Object) 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 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 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 Warnings, err error) } // WithCustomValidator creates a new Webhook for validating the provided type. @@ -65,6 +78,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 +89,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 +99,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,20 +107,20 @@ 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)) + return Errored(http.StatusBadRequest, fmt.Errorf("unknown operation %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 validationResponseFromStatus(false, apiStatus.Status()).WithWarnings(warnings...) } - 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 index 3da8f0b46a..404fad9016 100644 --- a/pkg/webhook/admission/validator_test.go +++ b/pkg/webhook/admission/validator_test.go @@ -18,13 +18,11 @@ package admission import ( "context" - goerrors "errors" + "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" @@ -39,9 +37,8 @@ var _ = Describe("validatingHandler", func() { decoder := NewDecoder(scheme.Scheme) - Context("when dealing with successful results", func() { - - f := &admissiontest.FakeValidator{ErrorToReturn: nil, GVKToReturn: fakeValidatorVK} + Context("when dealing with successful results without warning", func() { + f := &fakeValidator{ErrorToReturn: nil, GVKToReturn: fakeValidatorVK, WarningsToReturn: nil} handler := validatingHandler{validator: f, decoder: decoder} It("should return 200 in response when create succeeds", func() { @@ -93,10 +90,151 @@ var _ = Describe("validatingHandler", func() { 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 := &fakeValidator{ErrorToReturn: nil, GVKToReturn: fakeValidatorVK, WarningsToReturn: []string{ + warningMessage, + anotherWarningMessage, + }} + 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{ + 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))) + 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.validator, + }, + OldObject: runtime.RawExtension{ + Raw: []byte("{}"), + Object: handler.validator, + }, + }, + }) + 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.validator, + }, + }, + }) + 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() { + 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 := &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(ContainElements(warningMessage)) + Expect(response.AdmissionResponse.Warnings).Should(ContainElements(anotherWarningMessage)) + + }) + + 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(ContainElements(warningMessage)) + Expect(response.AdmissionResponse.Warnings).Should(ContainElements(anotherWarningMessage)) + + }) + + 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(ContainElements(warningMessage)) + Expect(response.AdmissionResponse.Warnings).Should(ContainElements(anotherWarningMessage)) + + }) + + }) + + Context("when dealing with Status errors, without warning messages", func() { expectedError := &apierrors.StatusError{ ErrStatus: metav1.Status{ @@ -104,7 +242,7 @@ var _ = Describe("validatingHandler", func() { Code: http.StatusUnprocessableEntity, }, } - f := &admissiontest.FakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK} + 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() { @@ -166,10 +304,11 @@ var _ = Describe("validatingHandler", func() { }) }) - Context("when dealing with non-status errors", func() { - expectedError := goerrors.New("some error") - f := &admissiontest.FakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK} + Context("when dealing with non-status errors, without warning messages", func() { + + expectedError := errors.New("some error") + f := &fakeValidator{ErrorToReturn: expectedError, GVKToReturn: fakeValidatorVK} handler := validatingHandler{validator: f, decoder: decoder} It("should return 403 response when ValidateCreate with error message embedded", func() { @@ -185,6 +324,7 @@ var _ = Describe("validatingHandler", func() { }) 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())) }) @@ -212,7 +352,6 @@ var _ = Describe("validatingHandler", func() { }) It("should return 403 response when ValidateDelete returns non-APIStatus error", func() { - response := handler.Handle(context.TODO(), Request{ AdmissionRequest: admissionv1.AdmissionRequest{ Operation: admissionv1.Delete, @@ -227,7 +366,75 @@ var _ = Describe("validatingHandler", func() { 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 := &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() { + + 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.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)) + }) + + 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())) + 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.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())) + 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() {}) @@ -239,3 +446,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, + } +}