Merge pull request #12 from bakito/main

keep separate variables for validator and mutator in handler
snorwin · Sep 10, 2021 · 2b718ac · 2b718ac
2 parents 5b3ee97 + 8aca827
commit 2b718ac
Show file tree

Hide file tree

Showing 5 changed files with 73 additions and 67 deletions.
diff --git a/pkg/webhook/handler.go b/pkg/webhook/handler.go
@@ -20,10 +20,26 @@ type Handler interface {
 	inject.Client
 }
 
+// withValidationHandler create a validation handler instance
+func withValidationHandler(validator Validator, object runtime.Object) Handler {
+	return &handler{validator: validator, injector: validator, Object: object}
+}
+
+// withMutationHandler create a mutation handler instance
+func withMutationHandler(mutator Mutator, object runtime.Object) Handler {
+	return &handler{mutator: mutator, injector: mutator, Object: object}
+}
+
 // handler is wrapper type for Validator and Mutator, implements the Handler interface.
 type handler struct {
-	Handler interface{}
-	Object  runtime.Object
+	// injector keep this reference for dependency injection
+	injector interface{}
+	// validator instance, should be nil if mutator is set
+	validator Validator
+	// mutator instance, should be nil if validator is set
+	mutator Mutator
+
+	Object runtime.Object
 
 	decoder *admission.Decoder
 }
@@ -59,21 +75,21 @@ func (h *handler) Handle(ctx context.Context, req admission.Request) admission.R
 	}
 
 	// invoke validator
-	if validator, ok := h.Handler.(Validator); ok {
+	if h.validator != nil {
 		switch req.Operation {
 		case admissionv1.Create:
-			return validator.ValidateCreate(ctx, req, req.Object.Object)
+			return h.validator.ValidateCreate(ctx, req, req.Object.Object)
 		case admissionv1.Update:
-			return validator.ValidateUpdate(ctx, req, req.Object.Object, req.OldObject.Object)
+			return h.validator.ValidateUpdate(ctx, req, req.Object.Object, req.OldObject.Object)
 		case admissionv1.Delete:
-			return validator.ValidateDelete(ctx, req, req.OldObject.Object)
+			return h.validator.ValidateDelete(ctx, req, req.OldObject.Object)
 		}
 	}
 
 	// invoke mutator
-	if mutator, ok := h.Handler.(Mutator); ok {
+	if h.mutator != nil {
 		if req.Object.Object != nil {
-			resp := mutator.Mutate(ctx, req, req.Object.Object)
+			resp := h.mutator.Mutate(ctx, req, req.Object.Object)
 			if resp.Allowed && resp.Patches == nil {
 				// generate patches
 				marshalled, err := json.Marshal(req.Object.Object)
@@ -98,7 +114,7 @@ func (h *handler) InjectDecoder(decoder *admission.Decoder) error {
 	h.decoder = decoder
 
 	// pass decoder to the underlying handler
-	if injector, ok := h.Handler.(admission.DecoderInjector); ok {
+	if injector, ok := h.injector.(admission.DecoderInjector); ok {
 		return injector.InjectDecoder(decoder)
 	}
 
@@ -108,7 +124,7 @@ func (h *handler) InjectDecoder(decoder *admission.Decoder) error {
 // InjectClient implements the inject.Client interface.
 func (h *handler) InjectClient(client client.Client) error {
 	// pass client to the underlying handler
-	if injector, ok := h.Handler.(inject.Client); ok {
+	if injector, ok := h.injector.(inject.Client); ok {
 		return injector.InjectClient(client)
 	}
 

diff --git a/pkg/webhook/handler_test.go b/pkg/webhook/handler_test.go
@@ -44,16 +44,13 @@ var _ = Describe("Handler", func() {
 			raw, err := json.Marshal(pod)
 			Ω(err).ShouldNot(HaveOccurred())
 
-			h := handler{
-				Handler: &MutateFunc{
-					Func: func(_ context.Context, _ admission.Request, obj runtime.Object) admission.Response {
-						pod := obj.(*corev1.Pod)
-						pod.Name = "bar"
-						return admission.Allowed("")
-					},
+			h := withMutationHandler(&MutateFunc{
+				Func: func(_ context.Context, _ admission.Request, obj runtime.Object) admission.Response {
+					pod := obj.(*corev1.Pod)
+					pod.Name = "bar"
+					return admission.Allowed("")
 				},
-				Object: &corev1.Pod{},
-			}
+			}, &corev1.Pod{})
 			err = h.InjectDecoder(decoder)
 			Ω(err).ShouldNot(HaveOccurred())
 			result := h.Handle(context.TODO(), admission.Request{
@@ -79,21 +76,19 @@ var _ = Describe("Handler", func() {
 			raw, err := json.Marshal(pod)
 			Ω(err).ShouldNot(HaveOccurred())
 
-			h := handler{
-				Handler: &MutateFunc{
-					Func: func(_ context.Context, _ admission.Request, obj runtime.Object) admission.Response {
-						pod := obj.(*corev1.Pod)
-						pod.Name = "bar"
-						return admission.Response{
-							AdmissionResponse: admissionv1.AdmissionResponse{
-								Allowed: true,
-							},
-							Patches: []jsonpatch.JsonPatchOperation{},
-						}
-					},
+			h := withMutationHandler(&MutateFunc{
+				Func: func(_ context.Context, _ admission.Request, obj runtime.Object) admission.Response {
+					pod := obj.(*corev1.Pod)
+					pod.Name = "bar"
+					return admission.Response{
+						AdmissionResponse: admissionv1.AdmissionResponse{
+							Allowed: true,
+						},
+						Patches: []jsonpatch.JsonPatchOperation{},
+					}
 				},
-				Object: &corev1.Pod{},
-			}
+			}, &corev1.Pod{})
+
 			err = h.InjectDecoder(decoder)
 			Ω(err).ShouldNot(HaveOccurred())
 			result := h.Handle(context.TODO(), admission.Request{
@@ -117,20 +112,18 @@ var _ = Describe("Handler", func() {
 			raw, err := json.Marshal(pod)
 			Ω(err).ShouldNot(HaveOccurred())
 
-			h := handler{
-				Handler: &ValidateFuncs{
-					CreateFunc: func(_ context.Context, _ admission.Request, _ runtime.Object) admission.Response {
-						return admission.Allowed("")
-					},
-					UpdateFunc: func(_ context.Context, _ admission.Request, _ runtime.Object, _ runtime.Object) admission.Response {
-						return admission.Denied("")
-					},
-					DeleteFunc: func(_ context.Context, _ admission.Request, _ runtime.Object) admission.Response {
-						return admission.Denied("")
-					},
+			h := withValidationHandler(&ValidateFuncs{
+				CreateFunc: func(_ context.Context, _ admission.Request, _ runtime.Object) admission.Response {
+					return admission.Allowed("")
 				},
-				Object: &corev1.Pod{},
-			}
+				UpdateFunc: func(_ context.Context, _ admission.Request, _ runtime.Object, _ runtime.Object) admission.Response {
+					return admission.Denied("")
+				},
+				DeleteFunc: func(_ context.Context, _ admission.Request, _ runtime.Object) admission.Response {
+					return admission.Denied("")
+				},
+			}, &corev1.Pod{})
+
 			err = h.InjectDecoder(decoder)
 			Ω(err).ShouldNot(HaveOccurred())
 
@@ -176,15 +169,13 @@ var _ = Describe("Handler", func() {
 			raw, err := json.Marshal(pod)
 			Ω(err).ShouldNot(HaveOccurred())
 
-			h := handler{
-				Handler: &MutateFunc{
-					Func: func(_ context.Context, request admission.Request, object runtime.Object) admission.Response {
-						Ω(object).Should(Equal(pod))
-						return admission.Allowed("")
-					},
+			h := withMutationHandler(&MutateFunc{
+				Func: func(_ context.Context, request admission.Request, object runtime.Object) admission.Response {
+					Ω(object).Should(Equal(pod))
+					return admission.Allowed("")
 				},
-				Object: &corev1.Pod{},
-			}
+			}, &corev1.Pod{})
+
 			err = h.InjectDecoder(decoder)
 			Ω(err).ShouldNot(HaveOccurred())
 
@@ -210,14 +201,11 @@ var _ = Describe("Handler", func() {
 			Ω(result.Allowed).Should(BeTrue())
 		})
 		It("should not decode invalid object", func() {
-			h := handler{
-				Handler: &MutateFunc{
-					Func: func(_ context.Context, _ admission.Request, _ runtime.Object) admission.Response {
-						return admission.Allowed("")
-					},
+			h := withMutationHandler(&MutateFunc{
+				Func: func(_ context.Context, _ admission.Request, _ runtime.Object) admission.Response {
+					return admission.Allowed("")
 				},
-				Object: &corev1.Pod{},
-			}
+			}, &corev1.Pod{})
 			err := h.InjectDecoder(decoder)
 			Ω(err).ShouldNot(HaveOccurred())
 
@@ -249,12 +237,12 @@ var _ = Describe("Handler", func() {
 		})
 		It("should pass decoder to validating webhook", func() {
 			webhook := ValidatingWebhook{}
-			Ω((&handler{Handler: &webhook}).InjectDecoder(decoder)).ShouldNot(HaveOccurred())
+			Ω((&handler{injector: &webhook}).InjectDecoder(decoder)).ShouldNot(HaveOccurred())
 			Ω(webhook.Decoder).Should(Equal(decoder))
 		})
 		It("should pass decoder to mutating webhook", func() {
 			webhook := MutatingWebhook{}
-			Ω((&handler{Handler: &webhook}).InjectDecoder(decoder)).ShouldNot(HaveOccurred())
+			Ω((&handler{injector: &webhook}).InjectDecoder(decoder)).ShouldNot(HaveOccurred())
 			Ω(webhook.Decoder).Should(Equal(decoder))
 		})
 		It("should never fail if handler not set", func() {
@@ -270,12 +258,12 @@ var _ = Describe("Handler", func() {
 		})
 		It("should pass client to validating webhook", func() {
 			webhook := ValidatingWebhook{}
-			Ω((&handler{Handler: &webhook}).InjectClient(client)).ShouldNot(HaveOccurred())
+			Ω((&handler{injector: &webhook}).InjectClient(client)).ShouldNot(HaveOccurred())
 			Ω(webhook.Client).Should(Equal(client))
 		})
 		It("should pass client to mutating webhook", func() {
 			webhook := MutatingWebhook{}
-			Ω((&handler{Handler: &webhook}).InjectClient(client)).ShouldNot(HaveOccurred())
+			Ω((&handler{injector: &webhook}).InjectClient(client)).ShouldNot(HaveOccurred())
 			Ω(webhook.Client).Should(Equal(client))
 		})
 		It("should never fail if handler not set", func() {

diff --git a/pkg/webhook/mutating_webhook.go b/pkg/webhook/mutating_webhook.go
@@ -2,6 +2,7 @@ package webhook
 
 import (
 	"context"
+
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )

diff --git a/pkg/webhook/mutating_webhook_test.go b/pkg/webhook/mutating_webhook_test.go
@@ -2,6 +2,7 @@ package webhook_test
 
 import (
 	"context"
+
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/snorwin/k8s-generic-webhook/pkg/webhook"

diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go
@@ -81,7 +81,7 @@ func (blder *Builder) Complete(i interface{}) error {
 	}
 
 	if validator, ok := i.(Validator); ok {
-		w, err := blder.createAdmissionWebhook(&handler{Handler: validator, Object: blder.apiType})
+		w, err := blder.createAdmissionWebhook(withValidationHandler(validator, blder.apiType))
 		if err != nil {
 			return err
 		}
@@ -92,7 +92,7 @@ func (blder *Builder) Complete(i interface{}) error {
 	}
 
 	if mutator, ok := i.(Mutator); ok {
-		w, err := blder.createAdmissionWebhook(&handler{Handler: mutator, Object: blder.apiType})
+		w, err := blder.createAdmissionWebhook(withMutationHandler(mutator, blder.apiType))
 		if err != nil {
 			return err
 		}