Pkotest

cmd/reference/internal/reference.go

@@ -112,7 +112,7 @@
 	mc := managedcache.NewObjectBoundAccessManager[*corev1.ConfigMap](
 		ctrl.Log,
 		mapper, r.restConfig, cache.Options{
-			Scheme: r.scheme, Mapper: mgr.GetRESTMapper(),
+			Mapper: mgr.GetRESTMapper(),
 		})
 	if err := mgr.Add(mc); err != nil {
 		return fmt.Errorf("adding managedcache: %w", err)

managedcache/gvk.go

@@ -0,0 +1,21 @@
+package managedcache
+
+import (
+	"errors"
+	"fmt"
+
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+)
+
+// ErrNonEmptyKindOrVersion is returned when either kind or version of the given object are empty.
+var ErrNonEmptyKindOrVersion = errors.New("object must have non-empty kind and version")
+
+func gvkForObject(obj runtime.Object) (schema.GroupVersionKind, error) {
+	gvk := obj.GetObjectKind().GroupVersionKind()
+	if gvk.Kind == "" || gvk.Version == "" {
+		return schema.GroupVersionKind{}, fmt.Errorf("%w: %s", ErrNonEmptyKindOrVersion, gvk)
+	}
+
+	return gvk, nil
+}

managedcache/objectboundaccess.go

@@ -8,14 +8,12 @@
 
 	"github.com/go-logr/logr"
 	"k8s.io/apimachinery/pkg/api/meta"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
@@ -24,11 +22,11 @@
 
 // ObjectBoundAccessManager manages caches and clients bound to objects.
 // Each object instance will receive it's own cache and client instance.
-type ObjectBoundAccessManager[T refType] interface {
+type ObjectBoundAccessManager[T RefType] interface {
 	manager.Runnable
 	// Get returns a TrackingCache for the provided object if one exists.
 	// If one does not exist, a new Cache is created and returned.
-	Get(context.Context, T) (Accessor, error)
+	Get(ctx context.Context, owner T) (Accessor, error)
 
 	// GetWithUser returns a TrackingCache for the provided object if one exist.
 	// If one does not exist, a new Cache is created and returned.
@@ -42,14 +40,14 @@
 
 	// Free will stop and remove a TrackingCache for
 	// the provided object, if one exists.
-	Free(context.Context, T) error
+	Free(ctx context.Context, owner T) error
 
 	// FreeWithUser informs the manager that the given user no longer needs
 	// a cache scoped to owner T. If the cache has no active users, it will be stopped.
 	FreeWithUser(ctx context.Context, owner T, user client.Object) error
 
 	// Source returns a controller-runtime source to watch from a controller.
-	Source(handler.EventHandler, ...predicate.Predicate) source.Source
+	Source(handler handler.EventHandler, predicates ...predicate.Predicate) source.Source
 }
 
 // Accessor provides write and cached read access to the cluster.
@@ -59,15 +57,14 @@
 }
 
 // NewObjectBoundAccessManager returns a new ObjectBoundAccessManager for T.
-func NewObjectBoundAccessManager[T refType](
+func NewObjectBoundAccessManager[T RefType](
 	log logr.Logger,
 	mapConfig ConfigMapperFunc[T],
 	baseRestConfig *rest.Config,
 	baseCacheOptions cache.Options,
 ) ObjectBoundAccessManager[T] {
 	return &objectBoundAccessManagerImpl[T]{
 		log:              log.WithName("ObjectBoundAccessManager"),
-		scheme:           baseCacheOptions.Scheme,
 		restMapper:       baseCacheOptions.Mapper,
 		mapConfig:        mapConfig,
 		baseRestConfig:   baseRestConfig,
@@ -82,22 +79,22 @@
 	}
 }
 
-type refType interface {
+// RefType constrains the owner type of an ObjectBoundAccessManager.
+type RefType interface {
 	client.Object
 	comparable
 }
 
 // ConfigMapperFunc applies changes to rest.Config and cache.Options based on the given object.
-type ConfigMapperFunc[T refType] func(
+type ConfigMapperFunc[T RefType] func(
 	context.Context, T, *rest.Config, cache.Options) (*rest.Config, cache.Options, error)
 
 type newClientFunc func(config *rest.Config, opts client.Options) (client.Client, error)
 
 var _ ObjectBoundAccessManager[client.Object] = (*objectBoundAccessManagerImpl[client.Object])(nil)
 
-type objectBoundAccessManagerImpl[T refType] struct {
+type objectBoundAccessManagerImpl[T RefType] struct {
 	log              logr.Logger
-	scheme           *runtime.Scheme
 	restMapper       meta.RESTMapper
 	mapConfig        ConfigMapperFunc[T]
 	baseRestConfig   *rest.Config
@@ -117,7 +114,7 @@
 	cancel   func()
 }
 
-type accessorRequest[T refType] struct {
+type accessorRequest[T RefType] struct {
 	owner      T
 	user       client.Object
 	gvks       sets.Set[schema.GroupVersionKind]
@@ -250,7 +247,7 @@
 		log.V(-1).Info("reusing cache for owner")
 
 		if req.user != nil {
-			entry.users[req.owner.GetUID()] = req.gvks
+			entry.users[req.user.GetUID()] = req.gvks
 		}
 
 		return entry.accessor, m.gcCache(ctx, req.owner)
@@ -268,7 +265,6 @@
 	}
 
 	client, err := m.newClient(restConfig, client.Options{
-		Scheme:     m.baseCacheOptions.Scheme,
 		Mapper:     m.baseCacheOptions.Mapper,
 		HTTPClient: m.baseCacheOptions.HTTPClient,
 	})
@@ -355,7 +351,7 @@
 	gvks := sets.Set[schema.GroupVersionKind]{}
 
 	for _, obj := range usedFor {
-		gvk, err := apiutil.GVKForObject(obj, m.scheme)
+		gvk, err := gvkForObject(obj)
 		if err != nil {
 			return nil, err
 		}

managedcache/trackingcache.go

@@ -3,21 +3,20 @@
 import (
 	"context"
 	"errors"
+	"fmt"
 	"strings"
 	"sync"
 
 	"github.com/go-logr/logr"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/rest"
 	toolscache "k8s.io/client-go/tools/cache"
 	"sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/source"
@@ -45,7 +44,6 @@
 type trackingCache struct {
 	cache.Cache
 	log          logr.Logger
-	scheme       *runtime.Scheme
 	restMapper   meta.RESTMapper
 	cacheSourcer cacheSourcer
 
@@ -81,7 +79,6 @@
 ) (TrackingCache, error) {
 	wehc := &trackingCache{
 		log:          log.WithName("TrackingCache"),
-		scheme:       opts.Scheme,
 		restMapper:   opts.Mapper,
 		cacheSourcer: cacheSourcer,
 
@@ -196,23 +193,31 @@
 }
 
 func (c *trackingCache) ensureCacheSync(ctx context.Context, obj client.Object) error {
-	gvk, err := apiutil.GVKForObject(obj, c.scheme)
+	gvk, err := gvkForObject(obj)
 	if err != nil {
 		return err
 	}
 
-	return c.ensureCacheSyncForGVK(ctx, gvk)
+	if err := c.ensureCacheSyncForGVK(ctx, gvk); err != nil {
+		return fmt.Errorf("ensuring cache sync for GVK: %w", err)
+	}
+
+	return nil
 }
 
 func (c *trackingCache) ensureCacheSyncList(ctx context.Context, list client.ObjectList) error {
-	gvk, err := apiutil.GVKForObject(list, c.scheme)
+	gvk, err := gvkForObject(list)
 	if err != nil {
 		return err
 	}
 	// We need the non-list GVK, so chop off the "List" from the end of the kind.
 	gvk.Kind = strings.TrimSuffix(gvk.Kind, "List")
 
-	return c.ensureCacheSyncForGVK(ctx, gvk)
+	if err := c.ensureCacheSyncForGVK(ctx, gvk); err != nil {
+		return fmt.Errorf("ensuring cache sync for (list) GVK: %w", err)
+	}
+
+	return nil
 }
 
 func (c *trackingCache) ensureCacheSyncForGVK(ctx context.Context, gvk schema.GroupVersionKind) error {
@@ -229,7 +234,9 @@
 				return
 			}
 
-			i, err := c.Cache.GetInformerForKind(ctx, gvk, cache.BlockUntilSynced(false))
+			obj := &unstructured.Unstructured{}
+			obj.SetGroupVersionKind(gvk)
+			i, err := c.Cache.GetInformer(ctx, obj, cache.BlockUntilSynced(false))
 			if err != nil {
 				errCh <- err
 
@@ -284,7 +291,12 @@
 		return err
 	}
 
-	return c.Cache.Get(ctx, key, obj, opts...)
+	err := c.Cache.Get(ctx, key, obj, opts...)
+	if err != nil {
+		return fmt.Errorf("getting object: %w", err)
+	}
+
+	return nil
 }
 
 func (c *trackingCache) List(
@@ -333,7 +345,7 @@
 	c.accessLock.Lock()
 	defer c.accessLock.Unlock()
 
-	gvk, err := apiutil.GVKForObject(obj, c.scheme)
+	gvk, err := gvkForObject(obj)
 	if err != nil {
 		return err
 	}