Fix non-cached client

pkg/bundle/bundle.go

@@ -54,10 +54,10 @@ type Options struct {
 // bundle is a controller-runtime controller. Implements the actual controller
 // logic by reconciling over Bundles.
 type bundle struct {
-	// client is a Kubernetes client that makes calls to the API for every request.
+	// directClient is a Kubernetes client that makes calls to the API for every request.
 	// Should be used for updating, deleting, and when requesting data from
 	// resources whose informer only caches metadata.
-	client client.Client
+	directClient client.Client
 
 	// lister makes requests to the informer cache. Beware that resources whose
 	// informer only caches metadata, will not return underlying data of that
@@ -126,7 +126,7 @@ func (b *bundle) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result,
 				},
 			}
 
-			err := b.client.Get(ctx, client.ObjectKeyFromObject(configMap), configMap)
+			err := b.directClient.Get(ctx, client.ObjectKeyFromObject(configMap), configMap)
 
 			// Ignore ConfigMaps that have not been created yet, as they will be
 			// created later on in the sync.
@@ -142,7 +142,7 @@ func (b *bundle) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result,
 
 			delete(configMap.Data, bundle.Status.Target.ConfigMap.Key)
 
-			if err := b.client.Update(ctx, configMap); err != nil {
+			if err := b.directClient.Update(ctx, configMap); err != nil {
 				log.Error(err, "failed to delete old ConfigMap target key")
 				b.recorder.Eventf(&bundle, corev1.EventTypeWarning, "TargetUpdateError", "Failed to remove old key from ConfigMap target: %s", err)
 				return ctrl.Result{}, fmt.Errorf("failed to delete old ConfigMap target key: %w", err)
@@ -153,7 +153,7 @@ func (b *bundle) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result,
 
 		// Return with update here, so targets are synced on the next Reconcile.
 		bundle.Status.Target = &bundle.Spec.Target
-		return ctrl.Result{}, b.client.Status().Update(ctx, &bundle)
+		return ctrl.Result{}, b.directClient.Status().Update(ctx, &bundle)
 	}
 
 	resolvedBundle, err := b.buildSourceBundle(ctx, &bundle)
@@ -169,7 +169,7 @@ func (b *bundle) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result,
 		})
 
 		b.recorder.Eventf(&bundle, corev1.EventTypeWarning, "SourceNotFound", "Bundle source was not found: %s", err)
-		return ctrl.Result{}, b.client.Status().Update(ctx, &bundle)
+		return ctrl.Result{}, b.directClient.Status().Update(ctx, &bundle)
 	}
 
 	if err != nil {
@@ -200,7 +200,7 @@ func (b *bundle) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result,
 				Message: fmt.Sprintf("Failed to sync bundle to namespace %q: %s", namespace.Name, err),
 			})
 
-			return ctrl.Result{Requeue: true}, b.client.Status().Update(ctx, &bundle)
+			return ctrl.Result{Requeue: true}, b.directClient.Status().Update(ctx, &bundle)
 		}
 
 		if synced {
@@ -241,5 +241,5 @@ func (b *bundle) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result,
 
 	b.recorder.Eventf(&bundle, corev1.EventTypeNormal, "Synced", message)
 
-	return ctrl.Result{}, b.client.Status().Update(ctx, &bundle)
+	return ctrl.Result{}, b.directClient.Status().Update(ctx, &bundle)
 }

pkg/bundle/bundle_test.go

@@ -808,10 +808,10 @@ func Test_Reconcile(t *testing.T) {
 			fakerecorder := record.NewFakeRecorder(1)
 
 			b := &bundle{
-				client:   fakeclient,
-				lister:   fakeclient,
-				recorder: fakerecorder,
-				clock:    fixedclock,
+				directClient: fakeclient,
+				lister:       fakeclient,
+				recorder:     fakerecorder,
+				clock:        fixedclock,
 				Options: Options{
 					Log:       klogr.New(),
 					Namespace: trustNamespace,

pkg/bundle/cache/cache.go

@@ -27,6 +27,7 @@ import (
 	"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"
 )
 
 var _ cache.Cache = &multiScopedCache{}
@@ -83,11 +84,12 @@ func NewMultiScopedCache(namespace string, namespacedInformers []schema.GroupKin
 
 // GetInformer returns the underlying cache's GetInformer based on resource type.
 func (b *multiScopedCache) GetInformer(ctx context.Context, obj client.Object) (cache.Informer, error) {
-	if err := setGroupVersionKind(b.scheme, obj); err != nil {
+	gvk, err := apiutil.GVKForObject(obj, b.scheme)
+	if err != nil {
 		return nil, err
 	}
 
-	cache, err := b.cacheFromGVK(obj.GetObjectKind().GroupVersionKind())
+	cache, err := b.cacheFromGVK(gvk)
 	if err != nil {
 		return nil, err
 	}
@@ -97,7 +99,6 @@ func (b *multiScopedCache) GetInformer(ctx context.Context, obj client.Object) (
 // GetInformerForKind returns the underlying cache's GetInformerForKind based
 // on resource type.
 func (b *multiScopedCache) GetInformerForKind(ctx context.Context, gvk schema.GroupVersionKind) (cache.Informer, error) {
-
 	cache, err := b.cacheFromGVK(gvk)
 	if err != nil {
 		return nil, err
@@ -145,11 +146,12 @@ func (b *multiScopedCache) WaitForCacheSync(ctx context.Context) bool {
 
 // IndexField returns the underlying cache's IndexField based on resource type.
 func (b *multiScopedCache) IndexField(ctx context.Context, obj client.Object, field string, extractValue client.IndexerFunc) error {
-	if err := setGroupVersionKind(b.scheme, obj); err != nil {
+	gvk, err := apiutil.GVKForObject(obj, b.scheme)
+	if err != nil {
 		return err
 	}
 
-	cache, err := b.cacheFromGVK(obj.GetObjectKind().GroupVersionKind())
+	cache, err := b.cacheFromGVK(gvk)
 	if err != nil {
 		return err
 	}
@@ -158,11 +160,12 @@ func (b *multiScopedCache) IndexField(ctx context.Context, obj client.Object, fi
 
 // Get returns the underlying cache's Get based on resource type.
 func (b *multiScopedCache) Get(ctx context.Context, key client.ObjectKey, obj client.Object) error {
-	if err := setGroupVersionKind(b.scheme, obj); err != nil {
+	gvk, err := apiutil.GVKForObject(obj, b.scheme)
+	if err != nil {
 		return err
 	}
 
-	cache, err := b.cacheFromGVK(obj.GetObjectKind().GroupVersionKind())
+	cache, err := b.cacheFromGVK(gvk)
 	if err != nil {
 		return err
 	}
@@ -171,11 +174,12 @@ func (b *multiScopedCache) Get(ctx context.Context, key client.ObjectKey, obj cl
 
 // List returns the underlying cache's List based on resource type.
 func (b *multiScopedCache) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error {
-	if err := setGroupVersionKind(b.scheme, list); err != nil {
+	gvk, err := apiutil.GVKForObject(list, b.scheme)
+	if err != nil {
 		return err
 	}
 
-	cache, err := b.cacheFromGVK(list.GetObjectKind().GroupVersionKind())
+	cache, err := b.cacheFromGVK(gvk)
 	if err != nil {
 		return err
 	}
@@ -196,26 +200,3 @@ func (b *multiScopedCache) cacheFromGVK(gvk schema.GroupVersionKind) (cache.Cach
 	}
 	return b.clusterCache, nil
 }
-
-// setGroupVersionKind populates the Group and Kind fields of obj using the
-// scheme type registry.
-// Inspired by https://github.com/kubernetes-sigs/controller-runtime/issues/1735#issuecomment-984763173
-func setGroupVersionKind(scheme *runtime.Scheme, obj runtime.Object) error {
-	gvk := obj.GetObjectKind().GroupVersionKind()
-	if gvk.Group != "" || gvk.Kind != "" || scheme == nil {
-		return nil // eg. in case of PartialMetadata, we don't want to overwrite the Group/ Kind
-	}
-
-	gvks, unversioned, err := scheme.ObjectKinds(obj)
-	if err != nil {
-		return err
-	}
-	if unversioned {
-		return fmt.Errorf("ObjectKinds unexpectedly returned unversioned: %#v", unversioned)
-	}
-	if len(gvks) != 1 {
-		return fmt.Errorf("ObjectKinds unexpectedly returned zero or multiple gvks: %#v", gvks)
-	}
-	obj.GetObjectKind().SetGroupVersionKind(gvks[0])
-	return nil
-}

pkg/bundle/controller.go

@@ -47,12 +47,20 @@ import (
 // when any related resource event in the Bundle source and target.
 // The controller will only cache metadata for ConfigMaps and Secrets.
 func AddBundleController(ctx context.Context, mgr manager.Manager, opts Options) error {
+	directClient, err := client.New(mgr.GetConfig(), client.Options{
+		Scheme: mgr.GetScheme(),
+		Mapper: mgr.GetRESTMapper(),
+	})
+	if err != nil {
+		return fmt.Errorf("failed to create client: %w", err)
+	}
+
 	b := &bundle{
-		client:   mgr.GetClient(),
-		lister:   mgr.GetCache(),
-		recorder: mgr.GetEventRecorderFor("bundles"),
-		clock:    clock.RealClock{},
-		Options:  opts,
+		directClient: directClient,
+		lister:       mgr.GetCache(),
+		recorder:     mgr.GetEventRecorderFor("bundles"),
+		clock:        clock.RealClock{},
+		Options:      opts,
 	}
 
 	if b.Options.DefaultPackageLocation != "" {

pkg/bundle/sync.go

@@ -105,7 +105,7 @@ func (b *bundle) buildSourceBundle(ctx context.Context, bundle *trustapi.Bundle)
 // configMapBundle returns the data in the target ConfigMap within the trust Namespace.
 func (b *bundle) configMapBundle(ctx context.Context, ref *trustapi.SourceObjectKeySelector) (string, error) {
 	var configMap corev1.ConfigMap
-	err := b.client.Get(ctx, client.ObjectKey{Namespace: b.Namespace, Name: ref.Name}, &configMap)
+	err := b.directClient.Get(ctx, client.ObjectKey{Namespace: b.Namespace, Name: ref.Name}, &configMap)
 	if apierrors.IsNotFound(err) {
 		return "", notFoundError{err}
 	}
@@ -125,7 +125,7 @@ func (b *bundle) configMapBundle(ctx context.Context, ref *trustapi.SourceObject
 // secretBundle returns the data in the target Secret within the trust Namespace.
 func (b *bundle) secretBundle(ctx context.Context, ref *trustapi.SourceObjectKeySelector) (string, error) {
 	var secret corev1.Secret
-	err := b.client.Get(ctx, client.ObjectKey{Namespace: b.Namespace, Name: ref.Name}, &secret)
+	err := b.directClient.Get(ctx, client.ObjectKey{Namespace: b.Namespace, Name: ref.Name}, &secret)
 	if apierrors.IsNotFound(err) {
 		return "", notFoundError{err}
 	}
@@ -160,7 +160,7 @@ func (b *bundle) syncTarget(ctx context.Context, log logr.Logger,
 	matchNamespace := namespaceSelector.Matches(labels.Set(namespace.Labels))
 
 	var configMap corev1.ConfigMap
-	err := b.client.Get(ctx, client.ObjectKey{Namespace: namespace.Name, Name: bundle.Name}, &configMap)
+	err := b.directClient.Get(ctx, client.ObjectKey{Namespace: namespace.Name, Name: bundle.Name}, &configMap)
 
 	// If the ConfigMap doesn't exist yet, create it.
 	if apierrors.IsNotFound(err) {
@@ -182,7 +182,7 @@ func (b *bundle) syncTarget(ctx context.Context, log logr.Logger,
 			},
 		}
 
-		return true, b.client.Create(ctx, &configMap)
+		return true, b.directClient.Create(ctx, &configMap)
 	}
 
 	if err != nil {
@@ -194,7 +194,7 @@ func (b *bundle) syncTarget(ctx context.Context, log logr.Logger,
 		// The ConfigMap is owned by this controller- delete it.
 		if metav1.IsControlledBy(&configMap, bundle) {
 			log.V(2).Info("deleting bundle from Namespace since namespaceSelector does not match")
-			return true, b.client.Delete(ctx, &configMap)
+			return true, b.directClient.Delete(ctx, &configMap)
 		}
 		// The ConfigMap isn't owned by us, so we shouldn't delete it. Return that
 		// we did nothing.
@@ -223,7 +223,7 @@ func (b *bundle) syncTarget(ctx context.Context, log logr.Logger,
 		return false, nil
 	}
 
-	if err := b.client.Update(ctx, &configMap); err != nil {
+	if err := b.directClient.Update(ctx, &configMap); err != nil {
 		return true, fmt.Errorf("failed to update configmap %s/%s with bundle: %w", namespace, bundle.Name, err)
 	}
 

pkg/bundle/sync_test.go

@@ -333,7 +333,7 @@ func Test_syncTarget(t *testing.T) {
 			fakeclient := clientBuilder.Build()
 			fakerecorder := record.NewFakeRecorder(1)
 
-			b := &bundle{client: fakeclient, recorder: fakerecorder}
+			b := &bundle{directClient: fakeclient, recorder: fakerecorder}
 
 			needsUpdate, err := b.syncTarget(context.TODO(), klogr.New(), &trustapi.Bundle{
 				ObjectMeta: metav1.ObjectMeta{Name: bundleName},
@@ -551,7 +551,7 @@ func Test_buildSourceBundle(t *testing.T) {
 				Build()
 
 			b := &bundle{
-				client: fakeclient,
+				directClient: fakeclient,
 				defaultPackage: &fspkg.Package{
 					Name:    "testpkg",
 					Version: "123",