Merge pull request #1129 from Shpectator/admission-webhooks-status-re…

…sponse ⚠️ admission responses with raw Status
kubernetes-sigs · Aug 27, 2020 · be59d64 · be59d64
2 parents ba987e4 + 9f235ae
commit be59d64
Show file tree

Hide file tree

Showing 3 changed files with 279 additions and 0 deletions.
diff --git a/pkg/webhook/admission/response.go b/pkg/webhook/admission/response.go
@@ -96,3 +96,14 @@ func PatchResponseFromRaw(original, current []byte) Response {
 		},
 	}
 }
+
+// validationResponseFromStatus returns a response for admitting a request with provided Status object.
+func validationResponseFromStatus(allowed bool, status metav1.Status) Response {
+	resp := Response{
+		AdmissionResponse: admissionv1beta1.AdmissionResponse{
+			Allowed: allowed,
+			Result:  &status,
+		},
+	}
+	return resp
+}
diff --git a/pkg/webhook/admission/validator.go b/pkg/webhook/admission/validator.go
@@ -20,7 +20,10 @@ import (
 	"context"
 	"net/http"
 
+	goerrors "errors"
+
 	"k8s.io/api/admission/v1beta1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
@@ -68,6 +71,10 @@ func (h *validatingHandler) Handle(ctx context.Context, req Request) Response {
 
 		err = obj.ValidateCreate()
 		if err != nil {
+			var apiStatus errors.APIStatus
+			if goerrors.As(err, &apiStatus) {
+				return validationResponseFromStatus(false, apiStatus.Status())
+			}
 			return Denied(err.Error())
 		}
 	}
@@ -86,6 +93,10 @@ func (h *validatingHandler) Handle(ctx context.Context, req Request) Response {
 
 		err = obj.ValidateUpdate(oldObj)
 		if err != nil {
+			var apiStatus errors.APIStatus
+			if goerrors.As(err, &apiStatus) {
+				return validationResponseFromStatus(false, apiStatus.Status())
+			}
 			return Denied(err.Error())
 		}
 	}
@@ -100,6 +111,10 @@ func (h *validatingHandler) Handle(ctx context.Context, req Request) Response {
 
 		err = obj.ValidateDelete()
 		if err != nil {
+			var apiStatus errors.APIStatus
+			if goerrors.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
@@ -0,0 +1,253 @@
+package admission
+
+import (
+	"context"
+	goerrors "errors"
+	"net/http"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	"k8s.io/api/admission/v1beta1"
+	apierrs "k8s.io/apimachinery/pkg/api/errors"
+	v1 "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 _ = Describe("validatingHandler", func() {
+
+	decoder, _ := NewDecoder(scheme.Scheme)
+
+	Context("when dealing with successful results", func() {
+
+		f := &fakeValidator{ErrorToReturn: nil}
+		handler := validatingHandler{validator: f, decoder: decoder}
+
+		It("should return 200 in response when create succeeds", func() {
+
+			response := handler.Handle(context.TODO(), Request{
+				AdmissionRequest: v1beta1.AdmissionRequest{
+					Operation: v1beta1.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: v1beta1.AdmissionRequest{
+					Operation: v1beta1.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: v1beta1.AdmissionRequest{
+					Operation: v1beta1.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 := &apierrs.StatusError{
+			ErrStatus: v1.Status{
+				Message: "some message",
+				Code:    http.StatusUnprocessableEntity,
+			},
+		}
+		f := &fakeValidator{ErrorToReturn: expectedError}
+		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: v1beta1.AdmissionRequest{
+					Operation: v1beta1.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: v1beta1.AdmissionRequest{
+					Operation: v1beta1.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: v1beta1.AdmissionRequest{
+					Operation: v1beta1.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 := goerrors.New("some error")
+		f := &fakeValidator{ErrorToReturn: expectedError}
+		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: v1beta1.AdmissionRequest{
+					Operation: v1beta1.Create,
+					Object: runtime.RawExtension{
+						Raw:    []byte("{}"),
+						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()))
+
+		})
+
+		It("should return 403 response when ValidateUpdate returns non-APIStatus error", func() {
+
+			response := handler.Handle(context.TODO(), Request{
+				AdmissionRequest: v1beta1.AdmissionRequest{
+					Operation: v1beta1.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(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: v1beta1.AdmissionRequest{
+					Operation: v1beta1.Delete,
+					OldObject: runtime.RawExtension{
+						Raw:    []byte("{}"),
+						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()))
+
+		})
+
+	})
+
+	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() {})
+
+})
+
+type fakeValidator struct {
+	ErrorToReturn error `json:"ErrorToReturn,omitempty"`
+}
+
+var _ Validator = &fakeValidator{}
+
+var fakeValidatorVK = schema.GroupVersionKind{Group: "foo.test.org", Version: "v1", Kind: "fakeValidator"}
+
+func (v *fakeValidator) ValidateCreate() error {
+	return v.ErrorToReturn
+}
+
+func (v *fakeValidator) ValidateUpdate(old runtime.Object) error {
+	return v.ErrorToReturn
+}
+
+func (v *fakeValidator) ValidateDelete() error {
+	return v.ErrorToReturn
+}
+
+func (v *fakeValidator) GetObjectKind() schema.ObjectKind { return v }
+
+func (v *fakeValidator) DeepCopyObject() runtime.Object {
+	return &fakeValidator{ErrorToReturn: v.ErrorToReturn}
+}
+
+func (v *fakeValidator) GroupVersionKind() schema.GroupVersionKind {
+	return fakeValidatorVK
+}
+
+func (v *fakeValidator) SetGroupVersionKind(gvk schema.GroupVersionKind) {}