feat: Add validation framework with ServiceAccount validator to ClusterExtension

Introduces an extensible validation framework that runs early in the
ClusterExtension reconciliation pipeline (after finalizer handling,
before revision state retrieval) to catch configuration errors before
expensive operations begin.

Key changes:
- New ClusterExtensionValidator type and ValidateClusterExtension
  reconcile step that executes all validators and collects all errors
  (no fail-fast behavior)
- ServiceAccountValidator checks SA existence via direct CoreV1 API
  Get call, providing clear "not found" feedback
- Sets Installed=Unknown and Progressing=Retrying conditions on
  validation failure
- Removed SA-specific error handling from RetrieveRevisionStates,
  since validation now catches these errors earlier
- Added RBAC permission (get serviceaccounts) to the operator-controller
  manager ClusterRole and regenerated manifests

Testing:
- Table-driven unit tests covering 7 scenarios (all pass, single
  fail, multiple fail, mixed, SA not found, SA found)
- E2E scenario for missing ServiceAccount validation error

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

Author: Per Goncalves da Silva

cmd/operator-controller/main.go

@@ -627,6 +627,9 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl
 	}
 	ceReconciler.ReconcileSteps = []controllers.ReconcileStepFunc{
 		controllers.HandleFinalizers(c.finalizers),
+		controllers.ValidateClusterExtension(
+			controllers.ServiceAccountValidator(coreClient),
+		),
 		controllers.MigrateStorage(storageMigrator),
 		controllers.RetrieveRevisionStates(revisionStatesGetter),
 		controllers.ResolveBundle(c.resolver, c.mgr.GetClient()),
@@ -747,6 +750,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(coreClient),
+		),
 		controllers.RetrieveRevisionStates(revisionStatesGetter),
 		controllers.ResolveBundle(c.resolver, c.mgr.GetClient()),
 		controllers.UnpackBundle(c.imagePuller, c.imageCache),

helm/olmv1/templates/rbac/clusterrole-operator-controller-manager-role.yml

@@ -15,6 +15,12 @@ rules:
       - serviceaccounts/token
     verbs:
       - create
+  - apiGroups:
+      - ""
+    resources:
+      - serviceaccounts
+    verbs:
+      - get
   - apiGroups:
       - apiextensions.k8s.io
     resources:

internal/operator-controller/controllers/clusterextension_controller_test.go

@@ -15,11 +15,13 @@ 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/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"
@@ -29,7 +31,6 @@ import (
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
-	"github.com/operator-framework/operator-controller/internal/operator-controller/authentication"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/bundle"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/conditionsets"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/controllers"
@@ -767,60 +768,177 @@ func TestClusterExtensionBoxcutterApplierFailsDoesNotLeakDeprecationErrors(t *te
 	require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
 }
 
-func TestClusterExtensionServiceAccountNotFound(t *testing.T) {
-	cl, reconciler := newClientAndReconciler(t, func(d *deps) {
-		d.RevisionStatesGetter = &MockRevisionStatesGetter{
-			Err: &authentication.ServiceAccountNotFoundError{
-				ServiceAccountName:      "missing-sa",
-				ServiceAccountNamespace: "default",
-			}}
-	})
-
-	ctx := context.Background()
-	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",
+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
 				},
 			},
-			Namespace: "default",
-			ServiceAccount: ocv1.ServiceAccountReference{
-				Name: "missing-sa",
+			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{
+				serviceAccountValidatorWithFakeClient(&corev1.ServiceAccount{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "missing-sa",
+						Namespace: "test-ns",
+					},
+				}),
+			},
+			expectError:          true,
+			errorMessageIncludes: `service account "test-sa" not found in namespace "test-ns"`,
+		},
+		{
+			name: "service account found",
+			validators: []controllers.ClusterExtensionValidator{
+				serviceAccountValidatorWithFakeClient(&corev1.ServiceAccount{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "test-sa",
+						Namespace: "test-ns",
+					},
+				}),
 			},
+			expectError: false,
 		},
 	}
 
-	require.NoError(t, cl.Create(ctx, clusterExtension))
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := context.Background()
 
-	t.Log("When reconciling the cluster extension")
-	res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
+			cl, reconciler := newClientAndReconciler(t, func(d *deps) {
+				d.RevisionStatesGetter = &MockRevisionStatesGetter{
+					RevisionStates: &controllers.RevisionStates{},
+				}
+				d.Validators = tt.validators
+			})
 
-	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))
+			extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
 
-	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"))
+			clusterExtension := &ocv1.ClusterExtension{
+				ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
+				Spec: ocv1.ClusterExtensionSpec{
+					Source: ocv1.SourceConfig{
+						SourceType: "Catalog",
+						Catalog: &ocv1.CatalogFilter{
+							PackageName: "test-package",
+						},
+					},
+					Namespace: "test-ns",
+					ServiceAccount: ocv1.ServiceAccountReference{
+						Name: "test-sa",
+					},
+				},
+			}
 
-	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.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
+			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)
+			} else {
+				require.NoError(t, err)
+				require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
+				require.Empty(t, clusterExtension.Status.Conditions)
+			}
+			require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
+		})
+	}
 }
 
 func TestClusterExtensionApplierFailsWithBundleInstalled(t *testing.T) {
@@ -2878,3 +2996,10 @@ func TestCheckCatalogsExist(t *testing.T) {
 		require.False(t, exists)
 	})
 }
+
+func serviceAccountValidatorWithFakeClient(serviceAccount *corev1.ServiceAccount) controllers.ClusterExtensionValidator {
+	if serviceAccount == nil {
+		return controllers.ServiceAccountValidator(fake.NewClientset().CoreV1())
+	}
+	return controllers.ServiceAccountValidator(fake.NewClientset(serviceAccount).CoreV1())
+}

internal/operator-controller/controllers/clusterextension_reconcile_steps.go

@@ -21,15 +21,16 @@ import (
 	"errors"
 	"fmt"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/finalizer"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
-	"github.com/operator-framework/operator-controller/internal/operator-controller/authentication"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/bundleutil"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/resolve"
@@ -63,19 +64,62 @@ 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 performing a direct Get call.
+func ServiceAccountValidator(saClient corev1client.ServiceAccountsGetter) ClusterExtensionValidator {
+	return func(ctx context.Context, ext *ocv1.ClusterExtension) error {
+		_, err := saClient.ServiceAccounts(ext.Spec.Namespace).Get(ctx, ext.Spec.ServiceAccount.Name, metav1.GetOptions{})
+		if err != nil {
+			if apierrors.IsNotFound(err) {
+				return fmt.Errorf("service account %q not found in namespace %q", ext.Spec.ServiceAccount.Name, ext.Spec.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

internal/operator-controller/controllers/suite_test.go

@@ -88,6 +88,7 @@ type deps struct {
 	ImagePuller          image.Puller
 	ImageCache           image.Cache
 	Applier              controllers.Applier
+	Validators           []controllers.ClusterExtensionValidator
 }
 
 func newClientAndReconciler(t *testing.T, opts ...reconcilerOption) (client.Client, *controllers.ClusterExtensionReconciler) {
@@ -105,7 +106,11 @@ func newClientAndReconciler(t *testing.T, opts ...reconcilerOption) (client.Clie
 	for _, opt := range opts {
 		opt(d)
 	}
-	reconciler.ReconcileSteps = []controllers.ReconcileStepFunc{controllers.HandleFinalizers(d.Finalizers), controllers.RetrieveRevisionStates(d.RevisionStatesGetter)}
+	reconciler.ReconcileSteps = []controllers.ReconcileStepFunc{
+		controllers.HandleFinalizers(d.Finalizers),
+		controllers.ValidateClusterExtension(d.Validators...),
+		controllers.RetrieveRevisionStates(d.RevisionStatesGetter),
+	}
 	if r := d.Resolver; r != nil {
 		reconciler.ReconcileSteps = append(reconciler.ReconcileSteps, controllers.ResolveBundle(r, cl))
 	}