⚠️ Remove pkg/inject

Signed-off-by: Vince Prignano <vincepri@redhat.com>

.golangci.yml

@@ -80,8 +80,6 @@ issues:
   - Subprocess launch(ed with variable|ing should be audited)
   - (G204|G104|G307)
   - "ST1000: at least one file in a package should have a package comment"
-  - "SA1019: \"sigs.k8s.io/controller-runtime/pkg/runtime/inject\""
-  - "SA1019: inject.*"
   exclude-rules:
   - linters:
     - gosec

alias.go

@@ -139,7 +139,7 @@ var (
 	// The logger, when used with controllers, can be expected to contain basic information about the object
 	// that's being reconciled like:
 	// - `reconciler group` and `reconciler kind` coming from the For(...) object passed in when building a controller.
-	// - `name` and `namespace` injected from the reconciliation request.
+	// - `name` and `namespace` from the reconciliation request.
 	//
 	// This is meant to be used with the context supplied in a struct that satisfies the Reconciler interface.
 	LoggerFrom = log.FromContext

doc.go

@@ -52,7 +52,7 @@ limitations under the License.
 //
 // Every controller and webhook is ultimately run by a Manager (pkg/manager). A
 // manager is responsible for running controllers and webhooks, and setting up
-// common dependencies (pkg/runtime/inject), like shared caches and clients, as
+// common dependencies, like shared caches and clients, as
 // well as managing leader election (pkg/leaderelection).  Managers are
 // generally configured to gracefully shut down controllers on pod termination
 // by wiring up a signal handler (pkg/manager/signals).

example_test.go

@@ -37,7 +37,6 @@ import (
 // ReplicaSetReconciler.
 //
 // * Start the application.
-// TODO(pwittrock): Update this example when we have better dependency injection support.
 func Example() {
 	var log = ctrl.Log.WithName("builder-examples")
 
@@ -75,7 +74,6 @@ func Example() {
 // ReplicaSetReconciler.
 //
 // * Start the application.
-// TODO(pwittrock): Update this example when we have better dependency injection support.
 func Example_updateLeaderElectionDurations() {
 	var log = ctrl.Log.WithName("builder-examples")
 	leaseDuration := 100 * time.Second

examples/builtins/main.go

@@ -22,6 +22,7 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
+	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
@@ -30,7 +31,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/manager/signals"
 	"sigs.k8s.io/controller-runtime/pkg/source"
-	"sigs.k8s.io/controller-runtime/pkg/webhook"
 )
 
 func init() {
@@ -71,13 +71,14 @@ func main() {
 		os.Exit(1)
 	}
 
-	// Setup webhooks
-	entryLog.Info("setting up webhook server")
-	hookServer := mgr.GetWebhookServer()
-
-	entryLog.Info("registering webhooks to the webhook server")
-	hookServer.Register("/mutate-v1-pod", &webhook.Admission{Handler: &podAnnotator{Client: mgr.GetClient()}})
-	hookServer.Register("/validate-v1-pod", &webhook.Admission{Handler: &podValidator{Client: mgr.GetClient()}})
+	if err := builder.WebhookManagedBy(mgr).
+		For(&corev1.Pod{}).
+		WithDefaulter(&podAnnotator{}).
+		WithValidator(&podValidator{}).
+		Complete(); err != nil {
+		entryLog.Error(err, "unable to create webhook", "webhook", "Pod")
+		os.Exit(1)
+	}
 
 	entryLog.Info("starting manager")
 	if err := mgr.Start(signals.SetupSignalHandler()); err != nil {

examples/builtins/mutatingwebhook.go

@@ -18,10 +18,10 @@ package main
 
 import (
 	"context"
-	"encoding/json"
-	"net/http"
+	"fmt"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -36,36 +36,18 @@ type podAnnotator struct {
 	decoder *admission.Decoder
 }
 
-// podAnnotator adds an annotation to every incoming pods.
-func (a *podAnnotator) Handle(ctx context.Context, req admission.Request) admission.Response {
-	// set up a convenient log object so we don't have to type request over and over again
+func (a *podAnnotator) Default(ctx context.Context, obj runtime.Object) error {
 	log := logf.FromContext(ctx)
-
-	pod := &corev1.Pod{}
-	err := a.decoder.Decode(req, pod)
-	if err != nil {
-		return admission.Errored(http.StatusBadRequest, err)
+	pod, ok := obj.(*corev1.Pod)
+	if !ok {
+		return fmt.Errorf("expected a Pod but got a %T", obj)
 	}
 
 	if pod.Annotations == nil {
 		pod.Annotations = map[string]string{}
 	}
 	pod.Annotations["example-mutating-admission-webhook"] = "foo"
+	log.Info("Annotated Pod")
 
-	marshaledPod, err := json.Marshal(pod)
-	if err != nil {
-		return admission.Errored(http.StatusInternalServerError, err)
-	}
-	log.Info("Annotating Pod")
-
-	return admission.PatchResponseFromRaw(req.Object.Raw, marshaledPod)
-}
-
-// podAnnotator implements admission.DecoderInjector.
-// A decoder will be automatically injected.
-
-// InjectDecoder injects the decoder.
-func (a *podAnnotator) InjectDecoder(d *admission.Decoder) error {
-	a.decoder = d
 	return nil
 }

examples/builtins/validatingwebhook.go

@@ -19,9 +19,9 @@ package main
 import (
 	"context"
 	"fmt"
-	"net/http"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -36,36 +36,35 @@ type podValidator struct {
 	decoder *admission.Decoder
 }
 
-// podValidator admits a pod if a specific annotation exists.
-func (v *podValidator) Handle(ctx context.Context, req admission.Request) admission.Response {
-	// set up a convenient log object so we don't have to type request over and over again
+// validate admits a pod if a specific annotation exists.
+func (v *podValidator) validate(ctx context.Context, obj runtime.Object) error {
 	log := logf.FromContext(ctx)
-
-	pod := &corev1.Pod{}
-	err := v.decoder.Decode(req, pod)
-	if err != nil {
-		return admission.Errored(http.StatusBadRequest, err)
+	pod, ok := obj.(*corev1.Pod)
+	if !ok {
+		return 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 admission.Denied(fmt.Sprintf("missing annotation %s", key))
+		return fmt.Errorf("missing annotation %s", key)
 	}
 	if anno != "foo" {
-		return admission.Denied(fmt.Sprintf("annotation %s did not have value %q", key, "foo"))
+		return fmt.Errorf("annotation %s did not have value %q", key, "foo")
 	}
 
-	return admission.Allowed("")
+	return nil
 }
 
-// podValidator implements admission.DecoderInjector.
-// A decoder will be automatically injected.
+func (v *podValidator) ValidateCreate(ctx context.Context, obj runtime.Object) error {
+	return v.validate(ctx, obj)
+}
 
-// InjectDecoder injects the decoder.
-func (v *podValidator) InjectDecoder(d *admission.Decoder) error {
-	v.decoder = d
-	return nil
+func (v *podValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) error {
+	return v.validate(ctx, newObj)
+}
+
+func (v *podValidator) ValidateDelete(ctx context.Context, obj runtime.Object) error {
+	return v.validate(ctx, obj)
 }

pkg/builder/example_test.go

@@ -107,7 +107,9 @@ func ExampleBuilder() {
 		ControllerManagedBy(mgr).  // Create the ControllerManagedBy
 		For(&appsv1.ReplicaSet{}). // ReplicaSet is the Application API
 		Owns(&corev1.Pod{}).       // ReplicaSet owns Pods created by it
-		Complete(&ReplicaSetReconciler{})
+		Complete(&ReplicaSetReconciler{
+			Client: mgr.GetClient(),
+		})
 	if err != nil {
 		log.Error(err, "could not create controller")
 		os.Exit(1)
@@ -155,8 +157,3 @@ func (a *ReplicaSetReconciler) Reconcile(ctx context.Context, req reconcile.Requ
 
 	return reconcile.Result{}, nil
 }
-
-func (a *ReplicaSetReconciler) InjectClient(c client.Client) error {
-	a.Client = c
-	return nil
-}

pkg/builder/webhook.go

@@ -164,10 +164,10 @@ func (blder *WebhookBuilder) registerDefaultingWebhook() {
 
 func (blder *WebhookBuilder) getDefaultingWebhook() *admission.Webhook {
 	if defaulter := blder.withDefaulter; defaulter != nil {
-		return admission.WithCustomDefaulter(blder.apiType, defaulter).WithRecoverPanic(blder.recoverPanic)
+		return admission.WithCustomDefaulter(blder.mgr.GetScheme(), blder.apiType, defaulter).WithRecoverPanic(blder.recoverPanic)
 	}
 	if defaulter, ok := blder.apiType.(admission.Defaulter); ok {
-		return admission.DefaultingWebhookFor(defaulter).WithRecoverPanic(blder.recoverPanic)
+		return admission.DefaultingWebhookFor(blder.mgr.GetScheme(), defaulter).WithRecoverPanic(blder.recoverPanic)
 	}
 	log.Info(
 		"skip registering a mutating webhook, object does not implement admission.Defaulter or WithDefaulter wasn't called",
@@ -194,10 +194,10 @@ func (blder *WebhookBuilder) registerValidatingWebhook() {
 
 func (blder *WebhookBuilder) getValidatingWebhook() *admission.Webhook {
 	if validator := blder.withValidator; validator != nil {
-		return admission.WithCustomValidator(blder.apiType, validator).WithRecoverPanic(blder.recoverPanic)
+		return admission.WithCustomValidator(blder.mgr.GetScheme(), blder.apiType, validator).WithRecoverPanic(blder.recoverPanic)
 	}
 	if validator, ok := blder.apiType.(admission.Validator); ok {
-		return admission.ValidatingWebhookFor(validator).WithRecoverPanic(blder.recoverPanic)
+		return admission.ValidatingWebhookFor(blder.mgr.GetScheme(), validator).WithRecoverPanic(blder.recoverPanic)
 	}
 	log.Info(
 		"skip registering a validating webhook, object does not implement admission.Validator or WithValidator wasn't called",

pkg/builder/webhook_test.go

@@ -115,8 +115,6 @@ func runTests(admissionReviewVersion string) {
 
 		ctx, cancel := context.WithCancel(context.Background())
 		cancel()
-		// TODO: we may want to improve it to make it be able to inject dependencies,
-		// but not always try to load certs and return not found error.
 		err = svr.Start(ctx)
 		if err != nil && !os.IsNotExist(err) {
 			ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -191,8 +189,6 @@ func runTests(admissionReviewVersion string) {
 
 		ctx, cancel := context.WithCancel(context.Background())
 		cancel()
-		// TODO: we may want to improve it to make it be able to inject dependencies,
-		// but not always try to load certs and return not found error.
 		err = svr.Start(ctx)
 		if err != nil && !os.IsNotExist(err) {
 			ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -208,7 +204,7 @@ func runTests(admissionReviewVersion string) {
 		By("sanity checking the response contains reasonable fields")
 		ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`))
 		ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":500`))
-		ExpectWithOffset(1, w.Body).To(ContainSubstring(`"message":"panic: injected panic [recovered]`))
+		ExpectWithOffset(1, w.Body).To(ContainSubstring(`"message":"panic: fake panic test [recovered]`))
 	})
 
 	It("should scaffold a defaulting webhook with a custom defaulter", func() {
@@ -260,8 +256,6 @@ func runTests(admissionReviewVersion string) {
 
 		ctx, cancel := context.WithCancel(context.Background())
 		cancel()
-		// TODO: we may want to improve it to make it be able to inject dependencies,
-		// but not always try to load certs and return not found error.
 		err = svr.Start(ctx)
 		if err != nil && !os.IsNotExist(err) {
 			ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -337,8 +331,6 @@ func runTests(admissionReviewVersion string) {
 
 		ctx, cancel := context.WithCancel(context.Background())
 		cancel()
-		// TODO: we may want to improve it to make it be able to inject dependencies,
-		// but not always try to load certs and return not found error.
 		err = svr.Start(ctx)
 		if err != nil && !os.IsNotExist(err) {
 			ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -411,8 +403,6 @@ func runTests(admissionReviewVersion string) {
 
 		ctx, cancel := context.WithCancel(context.Background())
 		cancel()
-		// TODO: we may want to improve it to make it be able to inject dependencies,
-		// but not always try to load certs and return not found error.
 		err = svr.Start(ctx)
 		if err != nil && !os.IsNotExist(err) {
 			ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -430,7 +420,7 @@ func runTests(admissionReviewVersion string) {
 		By("sanity checking the response contains reasonable field")
 		ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`))
 		ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":500`))
-		ExpectWithOffset(1, w.Body).To(ContainSubstring(`"message":"panic: injected panic [recovered]`))
+		ExpectWithOffset(1, w.Body).To(ContainSubstring(`"message":"panic: fake panic test [recovered]`))
 	})
 
 	It("should scaffold a validating webhook with a custom validator", func() {
@@ -463,12 +453,12 @@ func runTests(admissionReviewVersion string) {
     "kind":{
       "group":"foo.test.org",
       "version":"v1",
-      "kind":"TestDefaulter"
+      "kind":"TestValidator"
     },
     "resource":{
       "group":"foo.test.org",
       "version":"v1",
-      "resource":"testdefaulter"
+      "resource":"testvalidator"
     },
     "namespace":"default",
     "name":"foo",
@@ -484,8 +474,6 @@ func runTests(admissionReviewVersion string) {
 
 		ctx, cancel := context.WithCancel(context.Background())
 		cancel()
-		// TODO: we may want to improve it to make it be able to inject dependencies,
-		// but not always try to load certs and return not found error.
 		err = svr.Start(ctx)
 		if err != nil && !os.IsNotExist(err) {
 			ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -511,7 +499,7 @@ func runTests(admissionReviewVersion string) {
 		By("sanity checking the response contains reasonable field")
 		ExpectWithOffset(1, w.Body).To(ContainSubstring(`"allowed":false`))
 		ExpectWithOffset(1, w.Body).To(ContainSubstring(`"code":403`))
-		EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testdefaulter"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`))
+		EventuallyWithOffset(1, logBuffer).Should(gbytes.Say(`"msg":"Validating object","object":{"name":"foo","namespace":"default"},"namespace":"default","name":"foo","resource":{"group":"foo.test.org","version":"v1","resource":"testvalidator"},"user":"","requestID":"07e52e8d-4513-11e9-a716-42010a800270"`))
 	})
 
 	It("should scaffold defaulting and validating webhooks if the type implements both Defaulter and Validator interfaces", func() {
@@ -558,8 +546,6 @@ func runTests(admissionReviewVersion string) {
 
 		ctx, cancel := context.WithCancel(context.Background())
 		cancel()
-		// TODO: we may want to improve it to make it be able to inject dependencies,
-		// but not always try to load certs and return not found error.
 		err = svr.Start(ctx)
 		if err != nil && !os.IsNotExist(err) {
 			ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -636,8 +622,6 @@ func runTests(admissionReviewVersion string) {
 }`)
 
 		cancel()
-		// TODO: we may want to improve it to make it be able to inject dependencies,
-		// but not always try to load certs and return not found error.
 		err = svr.Start(ctx)
 		if err != nil && !os.IsNotExist(err) {
 			ExpectWithOffset(1, err).NotTo(HaveOccurred())
@@ -724,7 +708,7 @@ func (*TestDefaulterList) DeepCopyObject() runtime.Object   { return nil }
 
 func (d *TestDefaulter) Default() {
 	if d.Panic {
-		panic("injected panic")
+		panic("fake panic test")
 	}
 	if d.Replica < 2 {
 		d.Replica = 2
@@ -767,7 +751,7 @@ var _ admission.Validator = &TestValidator{}
 
 func (v *TestValidator) ValidateCreate() error {
 	if v.Panic {
-		panic("injected panic")
+		panic("fake panic test")
 	}
 	if v.Replica < 0 {
 		return errors.New("number of replica should be greater than or equal to 0")
@@ -777,7 +761,7 @@ func (v *TestValidator) ValidateCreate() error {
 
 func (v *TestValidator) ValidateUpdate(old runtime.Object) error {
 	if v.Panic {
-		panic("injected panic")
+		panic("fake panic test")
 	}
 	if v.Replica < 0 {
 		return errors.New("number of replica should be greater than or equal to 0")
@@ -792,7 +776,7 @@ func (v *TestValidator) ValidateUpdate(old runtime.Object) error {
 
 func (v *TestValidator) ValidateDelete() error {
 	if v.Panic {
-		panic("injected 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")