feat: Add ServiceAccount validation to ClusterExtension reconciliation

Add a validation stage at the start of the ClusterExtension reconciliation
pipeline that checks whether the ServiceAccount specified in .spec.serviceAccount
exists on the cluster before proceeding with bundle installation.

Key changes:
- Add ValidateClusterExtension orchestrator with open/closed architecture
  for extensible validation
- Add ServiceAccountValidator using authentication.TokenGetter
- Use cached tokens instead of creating new ones on each validation
- Collect and return all validation errors (fail-fast disabled)
- Set appropriate status conditions on validation failure
- Add comprehensive unit tests (10 test cases)
- Add e2e test scenario for ServiceAccount validation

Benefits:
- Early failure detection for missing ServiceAccounts
- No unnecessary token creation (reuses cached tokens)
- Reduced API server load through token caching
- Same token used for validation and actual bundle operations
- Automatic token rotation via TokenGetter
- Clear error messages for operators

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>

Author: Per Goncalves da Silva

cmd/operator-controller/main.go

@@ -625,8 +625,12 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl
 		ActionClientGetter: acg,
 		RevisionGenerator:  rg,
 	}
+	tokenGetter := authentication.NewTokenGetter(coreClient, authentication.WithExpirationDuration(1*time.Hour))
 	ceReconciler.ReconcileSteps = []controllers.ReconcileStepFunc{
 		controllers.HandleFinalizers(c.finalizers),
+		controllers.ValidateClusterExtension(
+			controllers.ServiceAccountValidator(tokenGetter),
+		),
 		controllers.MigrateStorage(storageMigrator),
 		controllers.RetrieveRevisionStates(revisionStatesGetter),
 		controllers.ResolveBundle(c.resolver, c.mgr.GetClient()),
@@ -656,20 +660,14 @@ 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, err := controllers.NewDefaultRevisionEngineFactory(
 		c.mgr.GetScheme(),
 		trackingCache,
 		discoveryClient,
 		c.mgr.GetRESTMapper(),
 		fieldOwnerPrefix,
 		c.mgr.GetConfig(),
-		cerTokenGetter,
+		tokenGetter,
 	)
 	if err != nil {
 		return fmt.Errorf("unable to create revision engine factory: %w", err)
@@ -747,6 +745,9 @@ func (c *helmReconcilerConfigurator) Configure(ceReconciler *controllers.Cluster
 	revisionStatesGetter := &controllers.HelmRevisionStatesGetter{ActionClientGetter: acg}
 	ceReconciler.ReconcileSteps = []controllers.ReconcileStepFunc{
 		controllers.HandleFinalizers(c.finalizers),
+		controllers.ValidateClusterExtension(
+			controllers.ServiceAccountValidator(tokenGetter),
+		),
 		controllers.RetrieveRevisionStates(revisionStatesGetter),
 		controllers.ResolveBundle(c.resolver, c.mgr.GetClient()),
 		controllers.UnpackBundle(c.imagePuller, c.imageCache),

internal/operator-controller/controllers/clusterextension_controller_test.go

@@ -15,11 +15,14 @@ import (
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/storage/driver"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/rand"
+	"k8s.io/client-go/kubernetes/fake"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	crfinalizer "sigs.k8s.io/controller-runtime/pkg/finalizer"
@@ -768,14 +771,26 @@ func TestClusterExtensionBoxcutterApplierFailsDoesNotLeakDeprecationErrors(t *te
 }
 
 func TestClusterExtensionServiceAccountNotFound(t *testing.T) {
+	// Create fake Kubernetes clientset (without creating the service account)
+	fakeClientset := fake.NewClientset()
+
+	// Create concrete TokenGetter with the fake client
+	tokenGetter := authentication.NewTokenGetter(fakeClientset.CoreV1())
+
 	cl, reconciler := newClientAndReconciler(t, func(d *deps) {
 		d.RevisionStatesGetter = &MockRevisionStatesGetter{
-			Err: &authentication.ServiceAccountNotFoundError{
-				ServiceAccountName:      "missing-sa",
-				ServiceAccountNamespace: "default",
-			}}
+			RevisionStates: &controllers.RevisionStates{},
+		}
 	})
 
+	// Add validation step to the beginning of the reconcile steps
+	reconciler.ReconcileSteps = append(
+		[]controllers.ReconcileStepFunc{controllers.ValidateClusterExtension(
+			controllers.ServiceAccountValidator(tokenGetter),
+		)},
+		reconciler.ReconcileSteps...,
+	)
+
 	ctx := context.Background()
 	extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
 
@@ -803,26 +818,201 @@ func TestClusterExtensionServiceAccountNotFound(t *testing.T) {
 
 	require.Equal(t, ctrl.Result{}, res)
 	require.Error(t, err)
-	var saErr *authentication.ServiceAccountNotFoundError
-	require.ErrorAs(t, err, &saErr)
 	t.Log("By fetching updated cluster extension after reconcile")
 	require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
 
 	t.Log("By checking the status conditions")
 	installedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeInstalled)
 	require.NotNil(t, installedCond)
 	require.Equal(t, metav1.ConditionUnknown, installedCond.Status)
-	require.Contains(t, installedCond.Message, fmt.Sprintf("service account %q not found in namespace %q: unable to authenticate with the Kubernetes cluster.",
-		"missing-sa", "default"))
+	require.Contains(t, installedCond.Message, "installation cannot proceed due to the following validation error(s)")
+	require.Contains(t, installedCond.Message, "service account \"missing-sa\" not found")
 
 	progressingCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeProgressing)
 	require.NotNil(t, progressingCond)
 	require.Equal(t, metav1.ConditionTrue, progressingCond.Status)
 	require.Equal(t, ocv1.ReasonRetrying, progressingCond.Reason)
-	require.Contains(t, progressingCond.Message, "installation cannot proceed due to missing ServiceAccount")
+	require.Contains(t, progressingCond.Message, "installation cannot proceed due to the following validation error(s)")
+	require.Contains(t, progressingCond.Message, "service account \"missing-sa\" not found")
 	require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
 }
 
+func TestValidateClusterExtension(t *testing.T) {
+	tests := []struct {
+		name                 string
+		validators           []controllers.ClusterExtensionValidator
+		expectError          bool
+		errorMessageIncludes string
+	}{
+		{
+			name: "all validators pass",
+			validators: []controllers.ClusterExtensionValidator{
+				// Validator that always passes
+				func(_ context.Context, _ *ocv1.ClusterExtension) error {
+					return nil
+				},
+			},
+			expectError: false,
+		},
+		{
+			name: "validator fails - sets Progressing condition",
+			validators: []controllers.ClusterExtensionValidator{
+				func(_ context.Context, _ *ocv1.ClusterExtension) error {
+					return errors.New("generic validation error")
+				},
+			},
+			expectError:          true,
+			errorMessageIncludes: "generic validation error",
+		},
+		{
+			name: "multiple validators - collects all failures",
+			validators: []controllers.ClusterExtensionValidator{
+				// First validator fails
+				func(_ context.Context, _ *ocv1.ClusterExtension) error {
+					return errors.New("first validator failed")
+				},
+				// Second validator also fails
+				func(_ context.Context, _ *ocv1.ClusterExtension) error {
+					return errors.New("second validator failed")
+				},
+				// Third validator fails too
+				func(_ context.Context, _ *ocv1.ClusterExtension) error {
+					return errors.New("third validator failed")
+				},
+			},
+			expectError:          true,
+			errorMessageIncludes: "first validator failed\nsecond validator failed\nthird validator failed",
+		},
+		{
+			name: "multiple validators - all pass",
+			validators: []controllers.ClusterExtensionValidator{
+				func(_ context.Context, _ *ocv1.ClusterExtension) error {
+					return nil
+				},
+				func(_ context.Context, _ *ocv1.ClusterExtension) error {
+					return nil
+				},
+				func(_ context.Context, _ *ocv1.ClusterExtension) error {
+					return nil
+				},
+			},
+			expectError: false,
+		},
+		{
+			name: "multiple validators - some pass, some fail",
+			validators: []controllers.ClusterExtensionValidator{
+				// First validator passes
+				func(_ context.Context, _ *ocv1.ClusterExtension) error {
+					return nil
+				},
+				// Second validator fails
+				func(_ context.Context, _ *ocv1.ClusterExtension) error {
+					return errors.New("validation error 1")
+				},
+				// Third validator passes
+				func(_ context.Context, _ *ocv1.ClusterExtension) error {
+					return nil
+				},
+				// Fourth validator fails
+				func(_ context.Context, _ *ocv1.ClusterExtension) error {
+					return errors.New("validation error 2")
+				},
+			},
+			expectError:          true,
+			errorMessageIncludes: "validation error 1\nvalidation error 2",
+		},
+		{
+			name: "service account not found",
+			validators: []controllers.ClusterExtensionValidator{
+				newServiceAccountValidator(),
+			},
+			expectError:          true,
+			errorMessageIncludes: "service account \"missing-sa\" not found",
+		},
+		{
+			name: "service account found",
+			validators: []controllers.ClusterExtensionValidator{
+				newServiceAccountValidator(&corev1.ServiceAccount{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "test-sa",
+						Namespace: "test-namespace",
+					},
+				}),
+			},
+			expectError: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := context.Background()
+
+			cl, reconciler := newClientAndReconciler(t, func(d *deps) {
+				d.RevisionStatesGetter = &MockRevisionStatesGetter{
+					RevisionStates: &controllers.RevisionStates{},
+				}
+			})
+
+			reconciler.ReconcileSteps = append(
+				[]controllers.ReconcileStepFunc{controllers.ValidateClusterExtension(tt.validators...)},
+				reconciler.ReconcileSteps...,
+			)
+
+			extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
+
+			t.Log("Given a cluster extension with a missing service account")
+			clusterExtension := &ocv1.ClusterExtension{
+				ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
+				Spec: ocv1.ClusterExtensionSpec{
+					Source: ocv1.SourceConfig{
+						SourceType: "Catalog",
+						Catalog: &ocv1.CatalogFilter{
+							PackageName: "test-package",
+						},
+					},
+					Namespace: "default",
+					ServiceAccount: ocv1.ServiceAccountReference{
+						Name: "missing-sa",
+					},
+				},
+			}
+
+			require.NoError(t, cl.Create(ctx, clusterExtension))
+
+			t.Log("When reconciling the cluster extension")
+			res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
+			require.Equal(t, ctrl.Result{}, res)
+			if tt.expectError {
+				require.Error(t, err)
+				t.Log("By fetching updated cluster extension after reconcile")
+				require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
+
+				t.Log("By checking the status conditions")
+				installedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeInstalled)
+				require.NotNil(t, installedCond)
+				require.Equal(t, metav1.ConditionUnknown, installedCond.Status)
+				require.Contains(t, installedCond.Message, "installation cannot proceed due to the following validation error(s)")
+				require.Contains(t, installedCond.Message, tt.errorMessageIncludes)
+
+				progressingCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeProgressing)
+				require.NotNil(t, progressingCond)
+				require.Equal(t, metav1.ConditionTrue, progressingCond.Status)
+				require.Equal(t, ocv1.ReasonRetrying, progressingCond.Reason)
+				require.Contains(t, progressingCond.Message, "installation cannot proceed due to the following validation error(s)")
+				require.Contains(t, progressingCond.Message, tt.errorMessageIncludes)
+				require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
+			}
+		})
+	}
+}
+
+func newServiceAccountValidator(objs ...runtime.Object) controllers.ClusterExtensionValidator {
+	fakeClientset := fake.NewClientset(objs...)
+	tokenGetter := authentication.NewTokenGetter(fakeClientset.CoreV1())
+
+	return controllers.ServiceAccountValidator(tokenGetter)
+}
+
 func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) {
 	mockApplier := &MockApplier{
 		installCompleted: true,

internal/operator-controller/controllers/clusterextension_reconcile_steps.go

@@ -23,6 +23,7 @@ import (
 
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/finalizer"
@@ -63,19 +64,70 @@ func HandleFinalizers(f finalizer.Finalizer) ReconcileStepFunc {
 	}
 }
 
+// ClusterExtensionValidator is a function that validates a ClusterExtension.
+// It returns an error if validation fails. Validators are executed sequentially
+// in the order they are registered.
+type ClusterExtensionValidator func(context.Context, *ocv1.ClusterExtension) error
+
+// ValidateClusterExtension returns a ReconcileStepFunc that executes all
+// validators sequentially. All validators are executed even if some fail,
+// and all errors are collected and returned as a joined error.
+// This provides complete validation feedback in a single reconciliation cycle.
+func ValidateClusterExtension(validators ...ClusterExtensionValidator) ReconcileStepFunc {
+	return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) {
+		l := log.FromContext(ctx)
+
+		l.Info("validating cluster extension")
+		var validationErrors []error
+		for _, validator := range validators {
+			if err := validator(ctx, ext); err != nil {
+				validationErrors = append(validationErrors, err)
+			}
+		}
+
+		// If there are no validation errors, continue reconciliation
+		if len(validationErrors) == 0 {
+			return nil, nil
+		}
+
+		// Set status condition with the validation error for other validation failures
+		err := fmt.Errorf("installation cannot proceed due to the following validation error(s): %w", errors.Join(validationErrors...))
+		setInstalledStatusConditionUnknown(ext, err.Error())
+		setStatusProgressing(ext, err)
+		return nil, err
+	}
+}
+
+// ServiceAccountValidator returns a validator that checks if the specified
+// ServiceAccount exists in the cluster by using the TokenGetter to fetch a token.
+// The TokenGetter maintains an internal cache, so this validation reuses tokens
+// instead of creating new ones on every validation attempt. The same token will
+// be reused for actual bundle operations.
+func ServiceAccountValidator(tokenGetter *authentication.TokenGetter) ClusterExtensionValidator {
+	return func(ctx context.Context, ext *ocv1.ClusterExtension) error {
+		saKey := types.NamespacedName{
+			Name:      ext.Spec.ServiceAccount.Name,
+			Namespace: ext.Spec.Namespace,
+		}
+		_, err := tokenGetter.Get(ctx, saKey)
+		if err != nil {
+			var saErr *authentication.ServiceAccountNotFoundError
+			if errors.As(err, &saErr) {
+				return fmt.Errorf("service account %q not found in namespace %q", saKey.Name, saKey.Namespace)
+			}
+			return fmt.Errorf("error validating service account: %w", err)
+		}
+		return nil
+	}
+}
+
 func RetrieveRevisionStates(r RevisionStatesGetter) ReconcileStepFunc {
 	return func(ctx context.Context, state *reconcileState, ext *ocv1.ClusterExtension) (*ctrl.Result, error) {
 		l := log.FromContext(ctx)
 		l.Info("getting installed bundle")
 		revisionStates, err := r.GetRevisionStates(ctx, ext)
 		if err != nil {
 			setInstallStatus(ext, nil)
-			var saerr *authentication.ServiceAccountNotFoundError
-			if errors.As(err, &saerr) {
-				setInstalledStatusConditionUnknown(ext, saerr.Error())
-				setStatusProgressing(ext, errors.New("installation cannot proceed due to missing ServiceAccount"))
-				return nil, err
-			}
 			setInstalledStatusConditionUnknown(ext, err.Error())
 			setStatusProgressing(ext, errors.New("retrying to get installed bundle"))
 			return nil, err