Construct and inject decoder into webhook handlers

This ensures that decoders (which are no longer constructed in the
manager) are properly injected into webhook handlers.  The webhook
itself receives a scheme, which it uses to construct a decoder.

Author: DirectXMan12

example/main.go

@@ -85,8 +85,8 @@ func main() {
 	mgr.Add(hookServer)
 
 	entryLog.Info("registering webhooks to the webhook server")
-	hookServer.Register("/mutate-pods", webhook.Admission{&podAnnotator{}})
-	hookServer.Register("/validate-pods", webhook.Admission{&podValidator{}})
+	hookServer.Register("/mutate-pods", webhook.Admission{Handler: &podAnnotator{}})
+	hookServer.Register("/validate-pods", webhook.Admission{Handler: &podValidator{}})
 
 	entryLog.Info("starting manager")
 	if err := mgr.Start(signals.SetupSignalHandler()); err != nil {

example/mutatingwebhook.go

@@ -29,7 +29,7 @@ import (
 // podAnnotator annotates Pods
 type podAnnotator struct {
 	client  client.Client
-	decoder admission.Decoder
+	decoder *admission.Decoder
 }
 
 // podAnnotator adds an annotation to every incoming pods.
@@ -67,7 +67,7 @@ func (a *podAnnotator) InjectClient(c client.Client) error {
 // A decoder will be automatically injected.
 
 // InjectDecoder injects the decoder.
-func (a *podAnnotator) InjectDecoder(d admission.Decoder) error {
+func (a *podAnnotator) InjectDecoder(d *admission.Decoder) error {
 	a.decoder = d
 	return nil
 }

example/validatingwebhook.go

@@ -29,7 +29,7 @@ import (
 // podValidator validates Pods
 type podValidator struct {
 	client  client.Client
-	decoder admission.Decoder
+	decoder *admission.Decoder
 }
 
 // podValidator admits a pod iff a specific annotation exists.
@@ -66,7 +66,7 @@ func (v *podValidator) InjectClient(c client.Client) error {
 // A decoder will be automatically injected.
 
 // InjectDecoder injects the decoder.
-func (v *podValidator) InjectDecoder(d admission.Decoder) error {
+func (v *podValidator) InjectDecoder(d *admission.Decoder) error {
 	v.decoder = d
 	return nil
 }

pkg/webhook/admission/http_test.go

@@ -100,8 +100,20 @@ type nopCloser struct {
 func (nopCloser) Close() error { return nil }
 
 type fakeHandler struct {
-	invoked bool
-	fn      func(context.Context, Request) Response
+	invoked        bool
+	fn             func(context.Context, Request) Response
+	decoder        *Decoder
+	injectedString string
+}
+
+func (h *fakeHandler) InjectDecoder(d *Decoder) error {
+	h.decoder = d
+	return nil
+}
+
+func (h *fakeHandler) InjectString(s string) error {
+	h.injectedString = s
+	return nil
 }
 
 func (h *fakeHandler) Handle(ctx context.Context, req Request) Response {

pkg/webhook/admission/inject.go

@@ -18,12 +18,12 @@ package admission
 
 // DecoderInjector is used by the ControllerManager to inject decoder into webhook handlers.
 type DecoderInjector interface {
-	InjectDecoder(Decoder) error
+	InjectDecoder(*Decoder) error
 }
 
 // InjectDecoderInto will set decoder on i and return the result if it implements Decoder.  Returns
 // false if i does not implement Decoder.
-func InjectDecoderInto(decoder Decoder, i interface{}) (bool, error) {
+func InjectDecoderInto(decoder *Decoder, i interface{}) (bool, error) {
 	if s, ok := i.(DecoderInjector); ok {
 		return true, s.InjectDecoder(decoder)
 	}

pkg/webhook/admission/webhook.go

@@ -24,6 +24,7 @@ import (
 	"github.com/appscode/jsonpatch"
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/json"
 
 	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
@@ -108,6 +109,9 @@ type Webhook struct {
 	// Handler actually processes an admission request returning whether it was allowed or denied,
 	// and potentially patches to apply to the handler.
 	Handler Handler
+
+	// scheme is used to construct the Decoder
+	decoder *Decoder
 }
 
 // Handle processes AdmissionRequest.
@@ -124,11 +128,60 @@ func (w *Webhook) Handle(ctx context.Context, req Request) Response {
 	return resp
 }
 
+// InjectScheme injects a scheme into the webhook, in order to construct a Decoder.
+func (w *Webhook) InjectScheme(s *runtime.Scheme) error {
+	// TODO(directxman12): we should have a better way to pass this down
+
+	var err error
+	w.decoder, err = NewDecoder(s)
+	if err != nil {
+		return err
+	}
+
+	// inject the decoder here too, just in case the order of calling this is not
+	// scheme first, then inject func
+	if w.Handler != nil {
+		if _, err := InjectDecoderInto(w.GetDecoder(), w.Handler); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// GetDecoder returns a decoder to decode the objects embedded in admission requests.
+// It may be nil if we haven't received a scheme to use to determine object types yet.
+func (w *Webhook) GetDecoder() *Decoder {
+	return w.decoder
+}
+
 // InjectFunc injects the field setter into the webhook.
 func (w *Webhook) InjectFunc(f inject.Func) error {
 	// inject directly into the handlers.  It would be more correct
 	// to do this in a sync.Once in Handle (since we don't have some
 	// other start/finalize-type method), but it's more efficient to
 	// do it here, presumably.
-	return f(w.Handler)
+
+	// also inject a decoder, and wrap this so that we get a setFields
+	// that injects a decoder (hopefully things don't ignore the duplicate
+	// InjectorInto call).
+
+	var setFields inject.Func
+	setFields = func(target interface{}) error {
+		if err := f(target); err != nil {
+			return err
+		}
+
+		if _, err := inject.InjectorInto(setFields, target); err != nil {
+			return err
+		}
+
+		if _, err := InjectDecoderInto(w.GetDecoder(), target); err != nil {
+			return err
+		}
+
+		return nil
+	}
+
+	return setFields(w.Handler)
 }

pkg/webhook/admission/webhook_test.go

@@ -26,7 +26,10 @@ import (
 	"github.com/appscode/jsonpatch"
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	machinerytypes "k8s.io/apimachinery/pkg/types"
+
+	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 )
 
 var _ = Describe("Admission Webhooks", func() {
@@ -117,4 +120,92 @@ var _ = Describe("Admission Webhooks", func() {
 		Expect(resp.PatchType).To(Equal(&patchType))
 		Expect(resp.Patch).To(Equal([]byte(`[{"op":"add","path":"/a","value":2},{"op":"replace","path":"/b","value":4}]`)))
 	})
+
+	Describe("dependency injection", func() {
+		It("should set dependencies passed in on the handler", func() {
+			By("setting up a webhook and injecting it with a injection func that injects a string")
+			setFields := func(target interface{}) error {
+				inj, ok := target.(stringInjector)
+				if !ok {
+					return nil
+				}
+
+				return inj.InjectString("something")
+			}
+			handler := &fakeHandler{}
+			webhook := &Webhook{
+				Handler: handler,
+			}
+			Expect(setFields(webhook)).To(Succeed())
+			Expect(inject.InjectorInto(setFields, webhook)).To(BeTrue())
+
+			By("checking that the string was injected")
+			Expect(handler.injectedString).To(Equal("something"))
+		})
+
+		It("should inject a decoder into the handler", func() {
+			By("setting up a webhook and injecting it with a injection func that injects a scheme")
+			setFields := func(target interface{}) error {
+				if _, err := inject.SchemeInto(runtime.NewScheme(), target); err != nil {
+					return err
+				}
+				return nil
+			}
+			handler := &fakeHandler{}
+			webhook := &Webhook{
+				Handler: handler,
+			}
+			Expect(setFields(webhook)).To(Succeed())
+			Expect(inject.InjectorInto(setFields, webhook)).To(BeTrue())
+
+			By("checking that the decoder was injected")
+			Expect(handler.decoder).NotTo(BeNil())
+		})
+
+		It("should pass a setFields that also injects a decoder into sub-dependencies", func() {
+			By("setting up a webhook and injecting it with a injection func that injects a scheme")
+			setFields := func(target interface{}) error {
+				if _, err := inject.SchemeInto(runtime.NewScheme(), target); err != nil {
+					return err
+				}
+				return nil
+			}
+			handler := &handlerWithSubDependencies{
+				Handler: HandlerFunc(func(ctx context.Context, req Request) Response {
+					return Response{}
+				}),
+				dep: &subDep{},
+			}
+			webhook := &Webhook{
+				Handler: handler,
+			}
+			Expect(setFields(webhook)).To(Succeed())
+			Expect(inject.InjectorInto(setFields, webhook)).To(BeTrue())
+
+			By("checking that setFields sets the decoder as well")
+			Expect(handler.dep.decoder).NotTo(BeNil())
+		})
+	})
 })
+
+type stringInjector interface {
+	InjectString(s string) error
+}
+
+type handlerWithSubDependencies struct {
+	Handler
+	dep *subDep
+}
+
+func (h *handlerWithSubDependencies) InjectFunc(f inject.Func) error {
+	return f(h.dep)
+}
+
+type subDep struct {
+	decoder *Decoder
+}
+
+func (d *subDep) InjectDecoder(dec *Decoder) error {
+	d.decoder = dec
+	return nil
+}