Remove SetFields from Manager and Controller

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

Author: vincepri

pkg/builder/controller.go

@@ -22,6 +22,7 @@ import (
 	"strings"
 
 	"github.com/go-logr/logr"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/klog/v2"
 
@@ -33,6 +34,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
+	internalsource "sigs.k8s.io/controller-runtime/pkg/source/internal"
 )
 
 // Supporting mocking out functions for testing.
@@ -196,16 +198,18 @@ func (blder *Builder) Build(r reconcile.Reconciler) (controller.Controller, erro
 	return blder.ctrl, nil
 }
 
-func (blder *Builder) project(obj client.Object, proj objectProjection) (source.Source, error) {
-	src := source.Kind(blder.mgr.GetCache(), obj)
+func (blder *Builder) project(obj client.Object, proj objectProjection) (client.Object, error) {
 	switch proj {
 	case projectAsNormal:
-		return src, nil
+		return obj, nil
 	case projectAsMetadata:
-		if err := source.KindAsPartialMetadata(src, blder.mgr.GetScheme()); err != nil {
-			return nil, err
+		metaObj := &metav1.PartialObjectMetadata{}
+		gvk, err := getGvk(obj, blder.mgr.GetScheme())
+		if err != nil {
+			return nil, fmt.Errorf("unable to determine GVK of %T for a metadata-only watch: %w", obj, err)
 		}
-		return src, nil
+		metaObj.SetGroupVersionKind(gvk)
+		return metaObj, nil
 	default:
 		panic(fmt.Sprintf("unexpected projection type %v on type %T, should not be possible since this is an internal field", proj, obj))
 	}
@@ -214,10 +218,11 @@ func (blder *Builder) project(obj client.Object, proj objectProjection) (source.
 func (blder *Builder) doWatch() error {
 	// Reconcile type
 	if blder.forInput.object != nil {
-		src, err := blder.project(blder.forInput.object, blder.forInput.objectProjection)
+		obj, err := blder.project(blder.forInput.object, blder.forInput.objectProjection)
 		if err != nil {
 			return err
 		}
+		src := source.Kind(blder.mgr.GetCache(), obj)
 		hdler := &handler.EnqueueRequestForObject{}
 		allPredicates := append(blder.globalPredicates, blder.forInput.predicates...)
 		if err := blder.ctrl.Watch(src, hdler, allPredicates...); err != nil {
@@ -230,10 +235,11 @@ func (blder *Builder) doWatch() error {
 		return errors.New("Owns() can only be used together with For()")
 	}
 	for _, own := range blder.ownsInput {
-		src, err := blder.project(own.object, own.objectProjection)
+		obj, err := blder.project(own.object, own.objectProjection)
 		if err != nil {
 			return err
 		}
+		src := source.Kind(blder.mgr.GetCache(), obj)
 		hdler := handler.EnqueueRequestForOwner(
 			blder.mgr.GetScheme(), blder.mgr.GetRESTMapper(),
 			blder.forInput.object,
@@ -254,13 +260,13 @@ func (blder *Builder) doWatch() error {
 		allPredicates := append([]predicate.Predicate(nil), blder.globalPredicates...)
 		allPredicates = append(allPredicates, w.predicates...)
 
-		// If the source of this watch is of type *source.Kind, project it.
-		if srckind, ok := w.src.(source.SyncingSource); ok {
-			if w.objectProjection == projectAsMetadata {
-				if err := source.KindAsPartialMetadata(srckind, blder.mgr.GetScheme()); err != nil {
-					return err
-				}
+		// If the source of this watch is of type Kind, project it.
+		if srckind, ok := w.src.(*internalsource.Kind); ok {
+			typeForSrc, err := blder.project(srckind.Type, w.objectProjection)
+			if err != nil {
+				return err
 			}
+			srckind.Type = typeForSrc
 		}
 
 		if err := blder.ctrl.Watch(w.src, w.eventhandler, allPredicates...); err != nil {

pkg/cluster/cluster.go

@@ -37,11 +37,6 @@ import (
 
 // Cluster provides various methods to interact with a cluster.
 type Cluster interface {
-	// SetFields will set any dependencies on an object for which the object has implemented the inject
-	// interface - e.g. inject.Client.
-	// Deprecated: use the equivalent Options field to set a field. This method will be removed in v0.10.
-	SetFields(interface{}) error
-
 	// GetConfig returns an initialized Config
 	GetConfig() *rest.Config
 

pkg/cluster/cluster_test.go

@@ -29,11 +29,8 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
-	"sigs.k8s.io/controller-runtime/pkg/cache/informertest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	logf "sigs.k8s.io/controller-runtime/pkg/internal/log"
 	intrec "sigs.k8s.io/controller-runtime/pkg/internal/recorder"
-	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 )
 
 var _ = Describe("cluster.Cluster", func() {
@@ -111,47 +108,6 @@ var _ = Describe("cluster.Cluster", func() {
 		})
 	})
 
-	Describe("SetFields", func() {
-		It("should inject field values", func() {
-			c, err := New(cfg, func(o *Options) {
-				o.NewCache = func(_ *rest.Config, _ cache.Options) (cache.Cache, error) {
-					return &informertest.FakeInformers{}, nil
-				}
-			})
-			Expect(err).NotTo(HaveOccurred())
-
-			By("Injecting the dependencies")
-			err = c.SetFields(&injectable{
-				scheme: func(scheme *runtime.Scheme) error {
-					defer GinkgoRecover()
-					Expect(scheme).To(Equal(c.GetScheme()))
-					return nil
-				},
-				client: func(client client.Client) error {
-					defer GinkgoRecover()
-					Expect(client).To(Equal(c.GetClient()))
-					return nil
-				},
-				log: func(logger logr.Logger) error {
-					defer GinkgoRecover()
-					Expect(logger).To(Equal(logf.RuntimeLog.WithName("cluster")))
-					return nil
-				},
-			})
-			Expect(err).NotTo(HaveOccurred())
-
-			By("Returning an error if dependency injection fails")
-
-			expected := fmt.Errorf("expected error")
-			err = c.SetFields(&injectable{
-				scheme: func(scheme *runtime.Scheme) error {
-					return expected
-				},
-			})
-			Expect(err).To(Equal(expected))
-		})
-	})
-
 	It("should not leak goroutines when stopped", func() {
 		currentGRs := goleak.IgnoreCurrent()
 
@@ -211,37 +167,3 @@ var _ = Describe("cluster.Cluster", func() {
 		Expect(c.GetAPIReader()).NotTo(BeNil())
 	})
 })
-
-var _ inject.Scheme = &injectable{}
-var _ inject.Logger = &injectable{}
-
-type injectable struct {
-	scheme func(scheme *runtime.Scheme) error
-	client func(client.Client) error
-	log    func(logger logr.Logger) error
-}
-
-func (i *injectable) InjectClient(c client.Client) error {
-	if i.client == nil {
-		return nil
-	}
-	return i.client(c)
-}
-
-func (i *injectable) InjectScheme(scheme *runtime.Scheme) error {
-	if i.scheme == nil {
-		return nil
-	}
-	return i.scheme(scheme)
-}
-
-func (i *injectable) InjectLogger(log logr.Logger) error {
-	if i.log == nil {
-		return nil
-	}
-	return i.log(log)
-}
-
-func (i *injectable) Start(<-chan struct{}) error {
-	return nil
-}

pkg/cluster/internal.go

@@ -28,7 +28,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	intrec "sigs.k8s.io/controller-runtime/pkg/internal/recorder"
-	"sigs.k8s.io/controller-runtime/pkg/runtime/inject"
 )
 
 type cluster struct {
@@ -64,13 +63,6 @@ type cluster struct {
 	logger logr.Logger
 }
 
-func (c *cluster) SetFields(i interface{}) error {
-	if _, err := inject.SchemeInto(c.scheme, i); err != nil {
-		return err
-	}
-	return nil
-}
-
 func (c *cluster) GetConfig() *rest.Config {
 	return c.config
 }

pkg/controller/controller.go

@@ -139,11 +139,6 @@ func NewUnmanaged(name string, mgr manager.Manager, options Options) (Controller
 		options.RateLimiter = workqueue.DefaultControllerRateLimiter()
 	}
 
-	// Inject dependencies into Reconciler
-	if err := mgr.SetFields(options.Reconciler); err != nil {
-		return nil, err
-	}
-
 	if options.RecoverPanic == nil {
 		options.RecoverPanic = mgr.GetControllerOptions().RecoverPanic
 	}

pkg/controller/controller_test.go

@@ -18,14 +18,12 @@ package controller_test
 
 import (
 	"context"
-	"fmt"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"go.uber.org/goleak"
 	corev1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/utils/pointer"
 
 	"sigs.k8s.io/controller-runtime/pkg/config/v1alpha1"
@@ -61,16 +59,6 @@ var _ = Describe("controller.Controller", func() {
 			Expect(err.Error()).To(ContainSubstring("must specify Reconciler"))
 		})
 
-		It("NewController should return an error if injecting Reconciler fails", func() {
-			m, err := manager.New(cfg, manager.Options{})
-			Expect(err).NotTo(HaveOccurred())
-
-			c, err := controller.New("foo", m, controller.Options{Reconciler: &failRec{}})
-			Expect(c).To(BeNil())
-			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(ContainSubstring("expected error"))
-		})
-
 		It("should not return an error if two controllers are registered with different names", func() {
 			m, err := manager.New(cfg, manager.Options{})
 			Expect(err).NotTo(HaveOccurred())
@@ -223,15 +211,3 @@ var _ = Describe("controller.Controller", func() {
 		})
 	})
 })
-
-var _ reconcile.Reconciler = &failRec{}
-
-type failRec struct{}
-
-func (*failRec) Reconcile(context.Context, reconcile.Request) (reconcile.Result, error) {
-	return reconcile.Result{}, nil
-}
-
-func (*failRec) InjectScheme(*runtime.Scheme) error {
-	return fmt.Errorf("expected error")
-}

pkg/handler/enqueue_owner.go

@@ -46,9 +46,9 @@ type OwnerOption func(e *enqueueRequestForOwner)
 // - a source.Kind Source with Type of Pod.
 //
 // - a handler.enqueueRequestForOwner EventHandler with an OwnerType of ReplicaSet and OnlyControllerOwner set to true.
-func EnqueueRequestForOwner(scheme *runtime.Scheme, mapper meta.RESTMapper, owner client.Object, opts ...OwnerOption) EventHandler {
+func EnqueueRequestForOwner(scheme *runtime.Scheme, mapper meta.RESTMapper, ownerType client.Object, opts ...OwnerOption) EventHandler {
 	e := &enqueueRequestForOwner{
-		ownerType: owner,
+		ownerType: ownerType,
 		mapper:    mapper,
 	}
 	if err := e.parseOwnerTypeGroupKind(scheme); err != nil {

pkg/manager/internal.go

@@ -192,18 +192,18 @@ 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 {
+	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 := cm.cluster.SetFields(i); err != nil {
+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 {
+	if _, err := inject.InjectorInto(cm.setFields, i); err != nil {
 		return err
 	}
 	if _, err := inject.LoggerInto(cm.logger, i); err != nil {