⚠️ Remove pkg/inject

This changeset brings in a few more breaking changes to remove the rest
of the dependency injection code.

The webhook code is now a bit simplified and more explicit
in accepting a *runtime.Scheme. The logger is now also not configured in the Standalone mode.
We should revisit in a future PR the webhook standalone configuration,
and potentially always require a manager. From a quick look in GitHub
projects, it seems most folks don't actually use these standalone
webhooks, which makes sense because the Manager makes things easier,
although there might be private code that's using them.

The config loader was using InjectScheme before, although it really
didn't need to.

Signed-off-by: Vince Prignano <[email protected]>

Author: vincepri

.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/mutatingwebhook.go

@@ -60,12 +60,3 @@ func (a *podAnnotator) Handle(ctx context.Context, req admission.Request) admiss
 
 	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

@@ -60,12 +60,3 @@ func (v *podValidator) Handle(ctx context.Context, req admission.Request) admiss
 
 	return admission.Allowed("")
 }
-
-// podValidator implements admission.DecoderInjector.
-// A decoder will be automatically injected.
-
-// InjectDecoder injects the decoder.
-func (v *podValidator) InjectDecoder(d *admission.Decoder) error {
-	v.decoder = d
-	return nil
-}

pkg/builder/example_test.go

@@ -155,8 +155,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")

pkg/cluster/internal.go

@@ -34,14 +34,8 @@ type cluster struct {
 	// config is the rest.config used to talk to the apiserver.  Required.
 	config *rest.Config
 
-	// scheme is the scheme injected into Controllers, EventHandlers, Sources and Predicates.  Defaults
-	// to scheme.scheme.
 	scheme *runtime.Scheme
-
-	cache cache.Cache
-
-	// TODO(directxman12): Provide an escape hatch to get individual indexers
-	// client is the client injected into Controllers (and EventHandlers, Sources and Predicates).
+	cache  cache.Cache
 	client client.Client
 
 	// apiReader is the reader that will make requests to the api server and not the cache.

pkg/config/config.go

@@ -83,12 +83,6 @@ func (d *DeferredFileLoader) OfKind(obj ControllerManagerConfiguration) *Deferre
 	return d
 }
 
-// InjectScheme will configure the scheme to be used for decoding the file.
-func (d *DeferredFileLoader) InjectScheme(scheme *runtime.Scheme) error {
-	d.scheme = scheme
-	return nil
-}
-
 // loadFile is used from the mutex.Once to load the file.
 func (d *DeferredFileLoader) loadFile() {
 	if d.scheme == nil {

pkg/config/example_test.go

@@ -44,41 +44,10 @@ func ExampleFile() {
 }
 
 // This example will load the file from a custom path.
-func ExampleDeferredFileLoader_atPath() {
+func ExampleFile_atPath() {
 	loader := config.File().AtPath("/var/run/controller-runtime/config.yaml")
 	if _, err := loader.Complete(); err != nil {
 		fmt.Println("failed to load config")
 		os.Exit(1)
 	}
 }
-
-// This example sets up loader with a custom scheme.
-func ExampleDeferredFileLoader_injectScheme() {
-	loader := config.File()
-	err := loader.InjectScheme(scheme)
-	if err != nil {
-		fmt.Println("failed to inject scheme")
-		os.Exit(1)
-	}
-
-	_, err = loader.Complete()
-	if err != nil {
-		fmt.Println("failed to load config")
-		os.Exit(1)
-	}
-}
-
-// This example sets up the loader with a custom scheme and custom type.
-func ExampleDeferredFileLoader_ofKind() {
-	loader := config.File().OfKind(&v1alpha1.CustomControllerManagerConfiguration{})
-	err := loader.InjectScheme(scheme)
-	if err != nil {
-		fmt.Println("failed to inject scheme")
-		os.Exit(1)
-	}
-	_, err = loader.Complete()
-	if err != nil {
-		fmt.Println("failed to load config")
-		os.Exit(1)
-	}
-}

pkg/internal/controller/controller_test.go

@@ -212,7 +212,7 @@ var _ = Describe("controller", func() {
 		It("should process events from source.Channel", func() {
 			// channel to be closed when event is processed
 			processed := make(chan struct{})
-			// source channel to be injected
+			// source channel
 			ch := make(chan event.GenericEvent, 1)
 
 			ctx, cancel := context.WithCancel(context.TODO())

pkg/manager/internal.go

@@ -46,7 +46,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/internal/httpserver"
 	intrec "sigs.k8s.io/controller-runtime/pkg/internal/recorder"
 	"sigs.k8s.io/controller-runtime/pkg/metrics"
-	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
 )
 
@@ -191,28 +190,9 @@ func (cm *controllerManager) Add(r Runnable) error {
 }
 
 func (cm *controllerManager) add(r Runnable) error {
-	// Set dependencies on the object
-	if err := cm.setFields(r); err != nil {
-		return err
-	}
 	return cm.runnables.Add(r)
 }
 
-// Deprecated: use the equivalent Options field to set a field. This method will be removed in v0.10.
-func (cm *controllerManager) setFields(i interface{}) error {
-	if _, err := inject.SchemeInto(cm.cluster.GetScheme(), i); err != nil {
-		return err
-	}
-	if _, err := inject.InjectorInto(cm.setFields, i); err != nil {
-		return err
-	}
-	if _, err := inject.LoggerInto(cm.logger, i); err != nil {
-		return err
-	}
-
-	return nil
-}
-
 // AddMetricsExtraHandler adds extra handler served on path to the http server that serves metrics.
 func (cm *controllerManager) AddMetricsExtraHandler(path string, handler http.Handler) error {
 	cm.Lock()