diff --git a/pkg/bundle/bundle.go b/pkg/bundle/bundle.go index 24170c2a2..3e716b051 100644 --- a/pkg/bundle/bundle.go +++ b/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) } diff --git a/pkg/bundle/bundle_test.go b/pkg/bundle/bundle_test.go index 51cacf7a5..96420dabe 100644 --- a/pkg/bundle/bundle_test.go +++ b/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, diff --git a/pkg/bundle/cache/cache.go b/pkg/bundle/cache/cache.go index bf5956a88..f8e10d475 100644 --- a/pkg/bundle/cache/cache.go +++ b/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 -} diff --git a/pkg/bundle/controller.go b/pkg/bundle/controller.go index c5241a931..e18069a78 100644 --- a/pkg/bundle/controller.go +++ b/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 != "" { diff --git a/pkg/bundle/sync.go b/pkg/bundle/sync.go index b11073f5c..36ae6fd00 100644 --- a/pkg/bundle/sync.go +++ b/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) } diff --git a/pkg/bundle/sync_test.go b/pkg/bundle/sync_test.go index 1583e81e4..97898ca17 100644 --- a/pkg/bundle/sync_test.go +++ b/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",