Remove controller-runtime's dependecy injections

Author: snorwin

pkg/mocks/manager/mock.go

@@ -7,33 +7,22 @@ import (
 
 	admissionv1 "k8s.io/api/admission/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
-// Handler is an interface that combines various interfaces.
-type Handler interface {
-	admission.Handler
-	admission.DecoderInjector
-	inject.Client
-}
-
 // withValidationHandler create a validation handler instance
-func withValidationHandler(validator Validator, object runtime.Object) Handler {
-	return &handler{validator: validator, injector: validator, Object: object}
+func withValidationHandler(validator Validator, object runtime.Object, decoder *admission.Decoder) admission.Handler {
+	return &handler{validator: validator, Object: object, decoder: decoder}
 }
 
 // withMutationHandler create a mutation handler instance
-func withMutationHandler(mutator Mutator, object runtime.Object) Handler {
-	return &handler{mutator: mutator, injector: mutator, Object: object}
+func withMutationHandler(mutator Mutator, object runtime.Object, decoder *admission.Decoder) admission.Handler {
+	return &handler{mutator: mutator, Object: object, decoder: decoder}
 }
 
 // handler is wrapper type for Validator and Mutator, implements the Handler interface.
 type handler struct {
-	// 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
@@ -108,25 +97,3 @@ func (h *handler) Handle(ctx context.Context, req admission.Request) admission.R
 
 	return admission.Denied("")
 }
-
-// InjectDecoder implements the admission.DecoderInjector interface.
-func (h *handler) InjectDecoder(decoder *admission.Decoder) error {
-	h.decoder = decoder
-
-	// pass decoder to the underlying handler
-	if injector, ok := h.injector.(admission.DecoderInjector); ok {
-		return injector.InjectDecoder(decoder)
-	}
-
-	return nil
-}
-
-// InjectClient implements the inject.Client interface.
-func (h *handler) InjectClient(client client.Client) error {
-	// pass client to the underlying handler
-	if injector, ok := h.injector.(inject.Client); ok {
-		return injector.InjectClient(client)
-	}
-
-	return nil
-}

pkg/webhook/handler.go

@@ -12,8 +12,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
@@ -26,7 +24,7 @@ var _ = Describe("Handler", func() {
 			scheme := runtime.NewScheme()
 			err := corev1.AddToScheme(scheme)
 			Ω(err).ShouldNot(HaveOccurred())
-			decoder, err = admission.NewDecoder(scheme)
+			decoder = admission.NewDecoder(scheme)
 			Ω(err).ShouldNot(HaveOccurred())
 
 		})
@@ -50,9 +48,7 @@ var _ = Describe("Handler", func() {
 					pod.Name = "bar"
 					return admission.Allowed("")
 				},
-			}, &corev1.Pod{})
-			err = h.InjectDecoder(decoder)
-			Ω(err).ShouldNot(HaveOccurred())
+			}, &corev1.Pod{}, decoder)
 			result := h.Handle(context.TODO(), admission.Request{
 				AdmissionRequest: admissionv1.AdmissionRequest{
 					Object: runtime.RawExtension{
@@ -87,10 +83,7 @@ var _ = Describe("Handler", func() {
 						Patches: []jsonpatch.JsonPatchOperation{},
 					}
 				},
-			}, &corev1.Pod{})
-
-			err = h.InjectDecoder(decoder)
-			Ω(err).ShouldNot(HaveOccurred())
+			}, &corev1.Pod{}, decoder)
 			result := h.Handle(context.TODO(), admission.Request{
 				AdmissionRequest: admissionv1.AdmissionRequest{
 					Object: runtime.RawExtension{
@@ -122,10 +115,7 @@ var _ = Describe("Handler", func() {
 				DeleteFunc: func(_ context.Context, _ admission.Request, _ runtime.Object) admission.Response {
 					return admission.Denied("")
 				},
-			}, &corev1.Pod{})
-
-			err = h.InjectDecoder(decoder)
-			Ω(err).ShouldNot(HaveOccurred())
+			}, &corev1.Pod{}, decoder)
 
 			result := h.Handle(context.TODO(), admission.Request{
 				AdmissionRequest: admissionv1.AdmissionRequest{
@@ -174,10 +164,7 @@ var _ = Describe("Handler", func() {
 					Ω(object).Should(Equal(pod))
 					return admission.Allowed("")
 				},
-			}, &corev1.Pod{})
-
-			err = h.InjectDecoder(decoder)
-			Ω(err).ShouldNot(HaveOccurred())
+			}, &corev1.Pod{}, decoder)
 
 			result := h.Handle(context.TODO(), admission.Request{
 				AdmissionRequest: admissionv1.AdmissionRequest{
@@ -205,9 +192,7 @@ var _ = Describe("Handler", func() {
 				Func: func(_ context.Context, _ admission.Request, _ runtime.Object) admission.Response {
 					return admission.Allowed("")
 				},
-			}, &corev1.Pod{})
-			err := h.InjectDecoder(decoder)
-			Ω(err).ShouldNot(HaveOccurred())
+			}, &corev1.Pod{}, decoder)
 
 			result := h.Handle(context.TODO(), admission.Request{
 				AdmissionRequest: admissionv1.AdmissionRequest{
@@ -228,46 +213,4 @@ var _ = Describe("Handler", func() {
 			Ω(result.Allowed).Should(BeFalse())
 		})
 	})
-	Context("InjectDecoder", func() {
-		var (
-			decoder *admission.Decoder
-		)
-		BeforeEach(func() {
-			decoder = &admission.Decoder{}
-		})
-		It("should pass decoder to validating webhook", func() {
-			webhook := ValidatingWebhook{}
-			Ω((&handler{injector: &webhook}).InjectDecoder(decoder)).ShouldNot(HaveOccurred())
-			Ω(webhook.Decoder).Should(Equal(decoder))
-		})
-		It("should pass decoder to mutating webhook", func() {
-			webhook := MutatingWebhook{}
-			Ω((&handler{injector: &webhook}).InjectDecoder(decoder)).ShouldNot(HaveOccurred())
-			Ω(webhook.Decoder).Should(Equal(decoder))
-		})
-		It("should never fail if handler not set", func() {
-			Ω((&handler{}).InjectDecoder(decoder)).ShouldNot(HaveOccurred())
-		})
-	})
-	Context("InjectClient", func() {
-		var (
-			client client.Client
-		)
-		BeforeEach(func() {
-			client = fake.NewClientBuilder().Build()
-		})
-		It("should pass client to validating webhook", func() {
-			webhook := ValidatingWebhook{}
-			Ω((&handler{injector: &webhook}).InjectClient(client)).ShouldNot(HaveOccurred())
-			Ω(webhook.Client).Should(Equal(client))
-		})
-		It("should pass client to mutating webhook", func() {
-			webhook := MutatingWebhook{}
-			Ω((&handler{injector: &webhook}).InjectClient(client)).ShouldNot(HaveOccurred())
-			Ω(webhook.Client).Should(Equal(client))
-		})
-		It("should never fail if handler not set", func() {
-			Ω((&handler{}).InjectClient(client)).ShouldNot(HaveOccurred())
-		})
-	})
 })

pkg/webhook/handler_test.go

@@ -11,7 +11,6 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
-	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 )
 
@@ -80,11 +79,12 @@ func (blder *Builder) Complete(i interface{}) error {
 		return fmt.Errorf("validating prefix %q must start with '/'", blder.prefixValidate)
 	}
 
+	decoder := admission.NewDecoder(blder.mgr.GetScheme())
+
 	isWebhook := false
 	if validator, ok := i.(Validator); ok {
-		w, err := blder.createAdmissionWebhook(withValidationHandler(validator, blder.apiType))
-		if err != nil {
-			return err
+		w := &admission.Webhook{
+			Handler: withValidationHandler(validator, blder.apiType, decoder),
 		}
 
 		if err := blder.registerValidatingWebhook(w); err != nil {
@@ -94,9 +94,8 @@ func (blder *Builder) Complete(i interface{}) error {
 	}
 
 	if mutator, ok := i.(Mutator); ok {
-		w, err := blder.createAdmissionWebhook(withMutationHandler(mutator, blder.apiType))
-		if err != nil {
-			return err
+		w := &admission.Webhook{
+			Handler: withMutationHandler(mutator, blder.apiType, decoder),
 		}
 
 		if err := blder.registerMutatingWebhook(w); err != nil {
@@ -109,31 +108,19 @@ func (blder *Builder) Complete(i interface{}) error {
 		return fmt.Errorf("webhook instance %v does implement neither Mutator nor Validator interface", i)
 	}
 
-	return nil
-}
-
-func (blder *Builder) createAdmissionWebhook(handler Handler) (*admission.Webhook, error) {
-	w := &admission.Webhook{
-		Handler: handler,
-	}
-
-	// inject scheme for decoder
-	if err := w.InjectScheme(blder.mgr.GetScheme()); err != nil {
-		return nil, err
+	if injector, ok := i.(InjectedClient); ok {
+		if err := injector.InjectClient(blder.mgr.GetClient()); err != nil {
+			return err
+		}
 	}
 
-	// inject client
-	if err := w.InjectFunc(func(i interface{}) error {
-		if injector, ok := i.(inject.Client); ok {
-			return injector.InjectClient(blder.mgr.GetClient())
+	if injector, ok := i.(InjectedDecoder); ok {
+		if err := injector.InjectDecoder(decoder); err != nil {
+			return err
 		}
-
-		return nil
-	}); err != nil {
-		return nil, err
 	}
 
-	return w, nil
+	return nil
 }
 
 func (blder *Builder) registerValidatingWebhook(w *admission.Webhook) error {
@@ -171,11 +158,11 @@ func (blder *Builder) registerMutatingWebhook(w *admission.Webhook) error {
 }
 
 func isAlreadyHandled(mgr ctrl.Manager, path string) bool {
-	if mgr.GetWebhookServer().WebhookMux == nil {
+	if mgr.GetWebhookServer().WebhookMux() == nil {
 		return false
 	}
 
-	h, p := mgr.GetWebhookServer().WebhookMux.Handler(&http.Request{URL: &url.URL{Path: path}})
+	h, p := mgr.GetWebhookServer().WebhookMux().Handler(&http.Request{URL: &url.URL{Path: path}})
 	if p == path && h != nil {
 		return true
 	}

pkg/webhook/webhook.go

@@ -21,7 +21,7 @@ var _ = Describe("Webhook", func() {
 			mock *gomock.Controller
 			mgr  *manager.MockManager
 
-			server *webhook2.Server
+			server webhook2.Server
 		)
 		BeforeEach(func() {
 			mock = gomock.NewController(GinkgoT())
@@ -40,7 +40,7 @@ var _ = Describe("Webhook", func() {
 				Return(fake.NewClientBuilder().Build()).
 				AnyTimes()
 
-			server = &webhook2.Server{}
+			server = &webhook2.DefaultServer{}
 			mgr.EXPECT().
 				GetWebhookServer().
 				Return(server).AnyTimes()