(feat): [Boxcutter] Use ClusterExtension ServiceAccount for revision operations

Implement serviceAccount-scoped operations for ClusterExtensionRevision
controller. Changes include:

- Add RevisionEngineFactory to create engines with scoped clients
- CER controller reads ServiceAccount from parent ClusterExtension
- Factory pattern produces RevisionEngine per ServiceAccount
- Scoped client enforces RBAC for all resource operations
- ServiceAccount stored as annotation for observability
- Fallback to impersonation when ServiceAccount not configured
- TrackingCache continues using global client for caching/cleanup
- Nil check for TokenGetter to prevent panics
- Comprehensive tests for error paths and factory behavior

This ensures extension installations respect ServiceAccount RBAC
instead of using admin privileges. Supports both token-based auth
(when SA configured) and impersonation (olm:clusterextensions group)
as fallback for simple-install mode.

Assisted-by: Cursor

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/applier/boxcutter_test.go

@@ -66,6 +66,12 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "test-123",
 		},
+		Spec: ocv1.ClusterExtensionSpec{
+			Namespace: "test-namespace",
+			ServiceAccount: ocv1.ServiceAccountReference{
+				Name: "test-sa",
+			},
+		},
 	}
 
 	objectLabels := map[string]string{
@@ -79,10 +85,11 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "test-123-1",
 			Annotations: map[string]string{
-				"olm.operatorframework.io/bundle-name":      "my-bundle",
-				"olm.operatorframework.io/bundle-reference": "bundle-ref",
-				"olm.operatorframework.io/bundle-version":   "1.2.0",
-				"olm.operatorframework.io/package-name":     "my-package",
+				"olm.operatorframework.io/bundle-name":          "my-bundle",
+				"olm.operatorframework.io/bundle-reference":     "bundle-ref",
+				"olm.operatorframework.io/bundle-version":       "1.2.0",
+				"olm.operatorframework.io/package-name":         "my-package",
+				"olm.operatorframework.io/service-account-name": "test-sa",
 			},
 			Labels: map[string]string{
 				labels.OwnerNameKey: "test-123",
@@ -172,6 +179,12 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "test-extension",
 		},
+		Spec: ocv1.ClusterExtensionSpec{
+			Namespace: "test-namespace",
+			ServiceAccount: ocv1.ServiceAccountReference{
+				Name: "test-sa",
+			},
+		},
 	}
 
 	rev, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, ext, map[string]string{}, map[string]string{})
@@ -291,7 +304,12 @@ func Test_SimpleRevisionGenerator_AppliesObjectLabelsAndRevisionAnnotations(t *t
 		"other": "value",
 	}
 
-	rev, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, &ocv1.ClusterExtension{}, map[string]string{
+	rev, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, &ocv1.ClusterExtension{
+		Spec: ocv1.ClusterExtensionSpec{
+			Namespace:      "test-namespace",
+			ServiceAccount: ocv1.ServiceAccountReference{Name: "test-sa"},
+		},
+	}, map[string]string{
 		"some": "value",
 	}, revAnnotations)
 	require.NoError(t, err)
@@ -319,7 +337,12 @@ func Test_SimpleRevisionGenerator_Failure(t *testing.T) {
 		ManifestProvider: r,
 	}
 
-	rev, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, &ocv1.ClusterExtension{}, map[string]string{}, map[string]string{})
+	rev, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, &ocv1.ClusterExtension{
+		Spec: ocv1.ClusterExtensionSpec{
+			Namespace:      "test-namespace",
+			ServiceAccount: ocv1.ServiceAccountReference{Name: "test-sa"},
+		},
+	}, map[string]string{}, map[string]string{})
 	require.Nil(t, rev)
 	t.Log("by checking rendering errors are propagated")
 	require.Error(t, err)

internal/operator-controller/controllers/clusterextensionrevision_controller.go

@@ -43,9 +43,16 @@ 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)
+	CreateImpersonatedClient(ctx context.Context, ext *ocv1.ClusterExtension) (client.Client, error)
 }
 
 type trackingCache interface {
@@ -60,6 +67,11 @@ 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.
+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,30 @@ func (c *ClusterExtensionRevisionReconciler) toBoxcutterRevision(ctx context.Con
 	return r, opts, nil
 }
 
+// getScopedClient gets the ServiceAccount from the parent ClusterExtension
+// and creates a client scoped to that ServiceAccount. If ServiceAccount is not
+// configured, it falls back to impersonation using synthetic users.
+func (c *ClusterExtensionRevisionReconciler) getScopedClient(ctx context.Context, rev *ocv1.ClusterExtensionRevision) (client.Client, error) {
+	ownerName := rev.Labels[labels.OwnerNameKey]
+	if ownerName == "" {
+		return nil, fmt.Errorf("revision %q is missing owner label", rev.Name)
+	}
+
+	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)
+	}
+
+	// ServiceAccount is currently required by API validation, but handle optional case
+	// for future flexibility or when validation might be relaxed
+	if ext.Spec.ServiceAccount.Name == "" {
+		// Fallback to impersonation if ServiceAccount not configured
+		return c.ScopedClientFactory.CreateImpersonatedClient(ctx, ext)
+	}
+
+	return c.ScopedClientFactory.CreateScopedClient(ctx, ext.Spec.Namespace, ext.Spec.ServiceAccount.Name)
+}
+
 var (
 	deploymentProbe = &probing.GroupKindSelector{
 		GroupKind: schema.GroupKind{Group: appsv1.GroupName, Kind: "Deployment"},