(feat): [Boxcutter] Use serviceAccount for ClusterExtensionRevision operations

ClusterExtensionRevision controller now uses the serviceAccount from
ClusterExtension spec instead of admin client. This applies objects
with proper RBAC permissions via a factory pattern that creates
scoped clients per revision.

Assisted-by: CLAUDE

Author: camilamacedo86

cmd/operator-controller/main.go

@@ -42,10 +42,7 @@ import (
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 	"k8s.io/klog/v2"
 	"k8s.io/utils/ptr"
-	"pkg.package-operator.run/boxcutter/machinery"
 	"pkg.package-operator.run/boxcutter/managedcache"
-	"pkg.package-operator.run/boxcutter/ownerhandling"
-	"pkg.package-operator.run/boxcutter/validation"
 	ctrl "sigs.k8s.io/controller-runtime"
 	crcache "sigs.k8s.io/controller-runtime/pkg/cache"
 	"sigs.k8s.io/controller-runtime/pkg/certwatcher"
@@ -653,21 +650,31 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl
 		return fmt.Errorf("unable to add tracking cache to manager: %v", err)
 	}
 
+	cerCoreClient, err := corev1client.NewForConfig(c.mgr.GetConfig())
+	if err != nil {
+		return fmt.Errorf("unable to create client for ClusterExtensionRevision controller: %w", err)
+	}
+	cerTokenGetter := authentication.NewTokenGetter(cerCoreClient, authentication.WithExpirationDuration(1*time.Hour))
+
+	revisionEngineFactory := &controllers.DefaultRevisionEngineFactory{
+		Scheme:           c.mgr.GetScheme(),
+		TrackingCache:    trackingCache,
+		DiscoveryClient:  discoveryClient,
+		RESTMapper:       c.mgr.GetRESTMapper(),
+		FieldOwnerPrefix: fieldOwnerPrefix,
+	}
+
+	scopedClientFactory := &controllers.DefaultScopedClientFactory{
+		BaseConfig:  c.mgr.GetConfig(),
+		Scheme:      c.mgr.GetScheme(),
+		TokenGetter: cerTokenGetter,
+	}
+
 	if err = (&controllers.ClusterExtensionRevisionReconciler{
-		Client: c.mgr.GetClient(),
-		RevisionEngine: machinery.NewRevisionEngine(
-			machinery.NewPhaseEngine(
-				machinery.NewObjectEngine(
-					c.mgr.GetScheme(), trackingCache, c.mgr.GetClient(),
-					ownerhandling.NewNative(c.mgr.GetScheme()),
-					machinery.NewComparator(ownerhandling.NewNative(c.mgr.GetScheme()), discoveryClient, c.mgr.GetScheme(), fieldOwnerPrefix),
-					fieldOwnerPrefix, fieldOwnerPrefix,
-				),
-				validation.NewClusterPhaseValidator(c.mgr.GetRESTMapper(), c.mgr.GetClient()),
-			),
-			validation.NewRevisionValidator(), c.mgr.GetClient(),
-		),
-		TrackingCache: trackingCache,
+		Client:                c.mgr.GetClient(),
+		RevisionEngineFactory: revisionEngineFactory,
+		TrackingCache:         trackingCache,
+		ScopedClientFactory:   scopedClientFactory,
 	}).SetupWithManager(c.mgr); err != nil {
 		return fmt.Errorf("unable to setup ClusterExtensionRevision controller: %w", err)
 	}

internal/operator-controller/applier/boxcutter.go

@@ -186,6 +186,11 @@ func (r *SimpleRevisionGenerator) buildClusterExtensionRevision(
 	ext *ocv1.ClusterExtension,
 	annotations map[string]string,
 ) *ocv1.ClusterExtensionRevision {
+	if annotations == nil {
+		annotations = make(map[string]string)
+	}
+	annotations[labels.ServiceAccountNameKey] = ext.Spec.ServiceAccount.Name
+
 	return &ocv1.ClusterExtensionRevision{
 		ObjectMeta: metav1.ObjectMeta{
 			Annotations: annotations,

internal/operator-controller/controllers/clusterextensionrevision_controller.go

@@ -43,9 +43,15 @@ const (
 // ClusterExtensionRevisionReconciler actions individual snapshots of ClusterExtensions,
 // as part of the boxcutter integration.
 type ClusterExtensionRevisionReconciler struct {
-	Client         client.Client
-	RevisionEngine RevisionEngine
-	TrackingCache  trackingCache
+	Client                client.Client
+	RevisionEngineFactory RevisionEngineFactory
+	TrackingCache         trackingCache
+	ScopedClientFactory   ScopedClientFactory
+}
+
+// ScopedClientFactory creates a client scoped to a specific serviceAccount
+type ScopedClientFactory interface {
+	CreateScopedClient(ctx context.Context, namespace, serviceAccountName string) (client.Client, error)
 }
 
 type trackingCache interface {
@@ -60,6 +66,12 @@ type RevisionEngine interface {
 	Reconcile(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error)
 }
 
+// RevisionEngineFactory creates a RevisionEngine with a given kube client.
+// This allows each ClusterExtensionRevision to use a client scoped to its serviceAccount.
+type RevisionEngineFactory interface {
+	CreateRevisionEngine(client client.Client) RevisionEngine
+}
+
 //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensionrevisions,verbs=get;list;watch;update;patch;create;delete
 //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensionrevisions/status,verbs=update;patch
 //+kubebuilder:rbac:groups=olm.operatorframework.io,resources=clusterextensionrevisions/finalizers,verbs=update
@@ -139,7 +151,15 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
 		return ctrl.Result{}, werr
 	}
 
-	rres, err := c.RevisionEngine.Reconcile(ctx, *revision, opts...)
+	scopedClient, err := c.getScopedClient(ctx, rev)
+	if err != nil {
+		setRetryingConditions(rev, err.Error())
+		return ctrl.Result{}, fmt.Errorf("failed to create client with serviceAccount permissions: %v", err)
+	}
+
+	revisionEngine := c.RevisionEngineFactory.CreateRevisionEngine(scopedClient)
+
+	rres, err := revisionEngine.Reconcile(ctx, *revision, opts...)
 	if err != nil {
 		if rres != nil {
 			// Log detailed reconcile reports only in debug mode (V(1)) to reduce verbosity.
@@ -253,7 +273,15 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev
 func (c *ClusterExtensionRevisionReconciler) teardown(ctx context.Context, rev *ocv1.ClusterExtensionRevision, revision *boxcutter.Revision) (ctrl.Result, error) {
 	l := log.FromContext(ctx)
 
-	tres, err := c.RevisionEngine.Teardown(ctx, *revision)
+	scopedClient, err := c.getScopedClient(ctx, rev)
+	if err != nil {
+		markAsAvailableUnknown(rev, ocv1.ClusterExtensionRevisionReasonReconciling, err.Error())
+		return ctrl.Result{}, fmt.Errorf("failed to create client for teardown: %v", err)
+	}
+
+	revisionEngine := c.RevisionEngineFactory.CreateRevisionEngine(scopedClient)
+
+	tres, err := revisionEngine.Teardown(ctx, *revision)
 	if err != nil {
 		if tres != nil {
 			l.V(1).Info("teardown failure report", "report", tres.String())
@@ -453,6 +481,36 @@ func (c *ClusterExtensionRevisionReconciler) toBoxcutterRevision(ctx context.Con
 	return r, opts, nil
 }
 
+// getScopedClient extracts serviceAccount information from the ClusterExtensionRevision
+// and creates a client scoped to that serviceAccount. The namespace is derived from the
+// owner ClusterExtension, following the project convention that ServiceAccounts must exist
+// in the ClusterExtension's namespace.
+func (c *ClusterExtensionRevisionReconciler) getScopedClient(ctx context.Context, rev *ocv1.ClusterExtensionRevision) (client.Client, error) {
+	annotations := rev.GetAnnotations()
+	if annotations == nil {
+		return nil, fmt.Errorf("revision %q is missing required annotations", rev.Name)
+	}
+
+	saName := annotations[labels.ServiceAccountNameKey]
+	if saName == "" {
+		return nil, fmt.Errorf("revision %q is missing serviceAccount name configuration", rev.Name)
+	}
+
+	// Get namespace from owner ClusterExtension via label
+	ownerName := rev.Labels[labels.OwnerNameKey]
+	if ownerName == "" {
+		return nil, fmt.Errorf("revision %q is missing owner label", rev.Name)
+	}
+
+	// Fetch the owner ClusterExtension to get its namespace
+	ext := &ocv1.ClusterExtension{}
+	if err := c.Client.Get(ctx, types.NamespacedName{Name: ownerName}, ext); err != nil {
+		return nil, fmt.Errorf("failed to get owner ClusterExtension %q: %w", ownerName, err)
+	}
+
+	return c.ScopedClientFactory.CreateScopedClient(ctx, ext.Spec.Namespace, saName)
+}
+
 var (
 	deploymentProbe = &probing.GroupKindSelector{
 		GroupKind: schema.GroupKind{Group: appsv1.GroupName, Kind: "Deployment"},

internal/operator-controller/controllers/clusterextensionrevision_controller_test.go

@@ -484,14 +484,16 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_RevisionReconciliation(t
 				Build()
 
 			// reconcile cluster extension revision
-			result, err := (&controllers.ClusterExtensionRevisionReconciler{
-				Client: testClient,
-				RevisionEngine: &mockRevisionEngine{
-					reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) {
-						return tc.revisionResult, tc.revisionReconcileErr
-					},
+			mockEngine := &mockRevisionEngine{
+				reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) {
+					return tc.revisionResult, tc.revisionReconcileErr
 				},
-				TrackingCache: &mockTrackingCache{client: testClient},
+			}
+			result, err := (&controllers.ClusterExtensionRevisionReconciler{
+				Client:                testClient,
+				RevisionEngineFactory: &mockRevisionEngineFactory{engine: mockEngine},
+				TrackingCache:         &mockTrackingCache{client: testClient},
+				ScopedClientFactory:   &mockScopedClientFactory{client: testClient},
 			}).Reconcile(t.Context(), ctrl.Request{
 				NamespacedName: types.NamespacedName{
 					Name: tc.reconcilingRevisionName,
@@ -603,14 +605,16 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_ValidationError_Retries(t
 				Build()
 
 			// reconcile cluster extension revision
-			result, err := (&controllers.ClusterExtensionRevisionReconciler{
-				Client: testClient,
-				RevisionEngine: &mockRevisionEngine{
-					reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) {
-						return tc.revisionResult, nil
-					},
+			mockEngine := &mockRevisionEngine{
+				reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) {
+					return tc.revisionResult, nil
 				},
-				TrackingCache: &mockTrackingCache{client: testClient},
+			}
+			result, err := (&controllers.ClusterExtensionRevisionReconciler{
+				Client:                testClient,
+				RevisionEngineFactory: &mockRevisionEngineFactory{engine: mockEngine},
+				TrackingCache:         &mockTrackingCache{client: testClient},
+				ScopedClientFactory:   &mockScopedClientFactory{client: testClient},
 			}).Reconcile(t.Context(), ctrl.Request{
 				NamespacedName: types.NamespacedName{
 					Name: clusterExtensionRevisionName,
@@ -652,7 +656,7 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) {
 				rev1.Finalizers = []string{
 					"olm.operatorframework.io/teardown",
 				}
-				return []client.Object{rev1}
+				return []client.Object{ext, rev1}
 			},
 			validate: func(t *testing.T, c client.Client) {
 				rev := &ocv1.ClusterExtensionRevision{}
@@ -887,18 +891,20 @@ func Test_ClusterExtensionRevisionReconciler_Reconcile_Deletion(t *testing.T) {
 				Build()
 
 			// reconcile cluster extension revision
-			result, err := (&controllers.ClusterExtensionRevisionReconciler{
-				Client: testClient,
-				RevisionEngine: &mockRevisionEngine{
-					reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) {
-						return tc.revisionResult, nil
-					},
-					teardown: tc.revisionEngineTeardownFn(t),
+			mockEngine := &mockRevisionEngine{
+				reconcile: func(ctx context.Context, rev machinerytypes.Revision, opts ...machinerytypes.RevisionReconcileOption) (machinery.RevisionResult, error) {
+					return tc.revisionResult, nil
 				},
+				teardown: tc.revisionEngineTeardownFn(t),
+			}
+			result, err := (&controllers.ClusterExtensionRevisionReconciler{
+				Client:                testClient,
+				RevisionEngineFactory: &mockRevisionEngineFactory{engine: mockEngine},
 				TrackingCache: &mockTrackingCache{
 					client: testClient,
 					freeFn: tc.trackingCacheFreeFn,
 				},
+				ScopedClientFactory: &mockScopedClientFactory{client: testClient},
 			}).Reconcile(t.Context(), ctrl.Request{
 				NamespacedName: types.NamespacedName{
 					Name: clusterExtensionRevisionName,
@@ -952,10 +958,11 @@ func newTestClusterExtensionRevision(t *testing.T, revisionName string, ext *ocv
 			UID:        types.UID(revisionName),
 			Generation: int64(1),
 			Annotations: map[string]string{
-				labels.PackageNameKey:     "some-package",
-				labels.BundleNameKey:      "some-package.v1.0.0",
-				labels.BundleReferenceKey: "registry.io/some-repo/some-package:v1.0.0",
-				labels.BundleVersionKey:   "1.0.0",
+				labels.PackageNameKey:        "some-package",
+				labels.BundleNameKey:         "some-package.v1.0.0",
+				labels.BundleReferenceKey:    "registry.io/some-repo/some-package:v1.0.0",
+				labels.BundleVersionKey:      "1.0.0",
+				labels.ServiceAccountNameKey: ext.Spec.ServiceAccount.Name,
 			},
 			Labels: map[string]string{
 				labels.OwnerNameKey: "test-ext",
@@ -1001,6 +1008,24 @@ func (m mockRevisionEngine) Reconcile(ctx context.Context, rev machinerytypes.Re
 	return m.reconcile(ctx, rev)
 }
 
+// mockRevisionEngineFactory creates mock RevisionEngines for testing
+type mockRevisionEngineFactory struct {
+	engine controllers.RevisionEngine
+}
+
+func (f *mockRevisionEngineFactory) CreateRevisionEngine(c client.Client) controllers.RevisionEngine {
+	return f.engine
+}
+
+// mockScopedClientFactory returns the test client for testing
+type mockScopedClientFactory struct {
+	client client.Client
+}
+
+func (f *mockScopedClientFactory) CreateScopedClient(ctx context.Context, namespace, serviceAccountName string) (client.Client, error) {
+	return f.client, nil
+}
+
 type mockRevisionResult struct {
 	validationError *validation.RevisionValidationError
 	phases          []machinery.PhaseResult

internal/operator-controller/controllers/revision_engine_factory.go

@@ -0,0 +1,81 @@
+//go:build !standard
+
+package controllers
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+
+	"k8s.io/apimachinery/pkg/api/meta"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/discovery"
+	"k8s.io/client-go/rest"
+	"pkg.package-operator.run/boxcutter/machinery"
+	"pkg.package-operator.run/boxcutter/managedcache"
+	"pkg.package-operator.run/boxcutter/ownerhandling"
+	"pkg.package-operator.run/boxcutter/validation"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/operator-framework/operator-controller/internal/operator-controller/authentication"
+)
+
+// DefaultRevisionEngineFactory creates boxcutter RevisionEngines with serviceAccount-scoped clients.
+type DefaultRevisionEngineFactory struct {
+	Scheme           *runtime.Scheme
+	TrackingCache    managedcache.TrackingCache
+	DiscoveryClient  discovery.CachedDiscoveryInterface
+	RESTMapper       meta.RESTMapper
+	FieldOwnerPrefix string
+}
+
+// CreateRevisionEngine constructs a boxcutter RevisionEngine using the provided client.
+// The client should be scoped to the appropriate serviceAccount for permission isolation.
+func (f *DefaultRevisionEngineFactory) CreateRevisionEngine(c client.Client) RevisionEngine {
+	return machinery.NewRevisionEngine(
+		machinery.NewPhaseEngine(
+			machinery.NewObjectEngine(
+				f.Scheme, f.TrackingCache, c,
+				ownerhandling.NewNative(f.Scheme),
+				machinery.NewComparator(ownerhandling.NewNative(f.Scheme), f.DiscoveryClient, f.Scheme, f.FieldOwnerPrefix),
+				f.FieldOwnerPrefix, f.FieldOwnerPrefix,
+			),
+			validation.NewClusterPhaseValidator(f.RESTMapper, c),
+		),
+		validation.NewRevisionValidator(), c,
+	)
+}
+
+// DefaultScopedClientFactory creates Kubernetes clients scoped to specific ServiceAccounts.
+// This ensures that operations are performed with the appropriate RBAC permissions.
+type DefaultScopedClientFactory struct {
+	BaseConfig  *rest.Config
+	Scheme      *runtime.Scheme
+	TokenGetter *authentication.TokenGetter
+}
+
+// CreateScopedClient creates a client authenticated as the specified ServiceAccount.
+// The client uses token authentication via the TokenInjectingRoundTripper.
+func (f *DefaultScopedClientFactory) CreateScopedClient(ctx context.Context, namespace, serviceAccountName string) (client.Client, error) {
+	saConfig := rest.AnonymousClientConfig(f.BaseConfig)
+	saConfig.Wrap(func(rt http.RoundTripper) http.RoundTripper {
+		return &authentication.TokenInjectingRoundTripper{
+			Tripper:     rt,
+			TokenGetter: f.TokenGetter,
+			Key: types.NamespacedName{
+				Name:      serviceAccountName,
+				Namespace: namespace,
+			},
+		}
+	})
+
+	scopedClient, err := client.New(saConfig, client.Options{
+		Scheme: f.Scheme,
+	})
+	if err != nil {
+		return nil, fmt.Errorf("failed to create client for ServiceAccount %s/%s: %w", namespace, serviceAccountName, err)
+	}
+
+	return scopedClient, nil
+}

internal/operator-controller/labels/labels.go

@@ -1,10 +1,11 @@
 package labels
 
 const (
-	OwnerKindKey       = "olm.operatorframework.io/owner-kind"
-	OwnerNameKey       = "olm.operatorframework.io/owner-name"
-	PackageNameKey     = "olm.operatorframework.io/package-name"
-	BundleNameKey      = "olm.operatorframework.io/bundle-name"
-	BundleVersionKey   = "olm.operatorframework.io/bundle-version"
-	BundleReferenceKey = "olm.operatorframework.io/bundle-reference"
+	OwnerKindKey          = "olm.operatorframework.io/owner-kind"
+	OwnerNameKey          = "olm.operatorframework.io/owner-name"
+	PackageNameKey        = "olm.operatorframework.io/package-name"
+	BundleNameKey         = "olm.operatorframework.io/bundle-name"
+	BundleVersionKey      = "olm.operatorframework.io/bundle-version"
+	BundleReferenceKey    = "olm.operatorframework.io/bundle-reference"
+	ServiceAccountNameKey = "olm.operatorframework.io/service-account-name"
 )