🐛 fix: Configuration validation errors now show "Failed" status instead of "Absent"

api/v1/common_types.go

@@ -24,9 +24,10 @@ const (
 	ReasonAbsent = "Absent"
 
 	// Progressing reasons
-	ReasonRollingOut = "RollingOut"
-	ReasonRetrying   = "Retrying"
-	ReasonBlocked    = "Blocked"
+	ReasonRollingOut           = "RollingOut"
+	ReasonRetrying             = "Retrying"
+	ReasonBlocked              = "Blocked"
+	ReasonInvalidConfiguration = "InvalidConfiguration"
 
 	// Deprecation reasons
 	ReasonDeprecated               = "Deprecated"

internal/operator-controller/applier/provider.go

@@ -16,6 +16,7 @@ import (
 	"github.com/operator-framework/operator-controller/internal/operator-controller/config"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/bundle/source"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/rukpak/render"
+	errorutil "github.com/operator-framework/operator-controller/internal/shared/util/error"
 )
 
 // ManifestProvider returns the manifests that should be applied by OLM given a bundle and its associated ClusterExtension
@@ -77,7 +78,7 @@ func (r *RegistryV1ManifestProvider) Get(bundleFS fs.FS, ext *ocv1.ClusterExtens
 		bundleConfigBytes := extensionConfigBytes(ext)
 		bundleConfig, err := config.UnmarshalConfig(bundleConfigBytes, schema, ext.Spec.Namespace)
 		if err != nil {
-			return nil, fmt.Errorf("invalid ClusterExtension configuration: %w", err)
+			return nil, errorutil.NewTerminalError(ocv1.ReasonInvalidConfiguration, fmt.Errorf("invalid ClusterExtension configuration: %w", err))
 		}
 
 		if watchNS := bundleConfig.GetWatchNamespace(); watchNS != nil {

internal/operator-controller/applier/provider_test.go

@@ -11,6 +11,7 @@ import (
 	apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 
@@ -100,6 +101,37 @@ func Test_RegistryV1ManifestProvider_Integration(t *testing.T) {
 		require.Contains(t, err.Error(), "invalid ClusterExtension configuration")
 	})
 
+	t.Run("returns terminal error for invalid config", func(t *testing.T) {
+		provider := applier.RegistryV1ManifestProvider{
+			BundleRenderer: render.BundleRenderer{
+				ResourceGenerators: []render.ResourceGenerator{
+					func(rv1 *bundle.RegistryV1, opts render.Options) ([]client.Object, error) {
+						return nil, nil
+					},
+				},
+			},
+			IsSingleOwnNamespaceEnabled: true,
+		}
+
+		// Bundle with SingleNamespace install mode requiring watchNamespace config
+		bundleFS := bundlefs.Builder().WithPackageName("test").
+			WithCSV(clusterserviceversion.Builder().WithInstallModeSupportFor(v1alpha1.InstallModeTypeSingleNamespace).Build()).Build()
+
+		// ClusterExtension without required config
+		ext := &ocv1.ClusterExtension{
+			Spec: ocv1.ClusterExtensionSpec{
+				Namespace: "install-namespace",
+				// No config provided - should fail validation
+			},
+		}
+
+		_, err := provider.Get(bundleFS, ext)
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "invalid ClusterExtension configuration")
+		// Assert that config validation errors are terminal (not retriable)
+		require.ErrorIs(t, err, reconcile.TerminalError(nil), "config validation errors should be terminal")
+	})
+
 	t.Run("returns rendered manifests", func(t *testing.T) {
 		provider := applier.RegistryV1ManifestProvider{
 			BundleRenderer: registryv1.Renderer,

internal/operator-controller/conditionsets/conditionsets.go

@@ -40,6 +40,7 @@ var ConditionReasons = []string{
 	ocv1.ReasonDeprecationStatusUnknown,
 	ocv1.ReasonFailed,
 	ocv1.ReasonBlocked,
+	ocv1.ReasonInvalidConfiguration,
 	ocv1.ReasonRetrying,
 	ocv1.ReasonAbsent,
 	ocv1.ReasonRollingOut,

internal/operator-controller/controllers/boxcutter_reconcile_steps.go

@@ -19,6 +19,7 @@ package controllers
 import (
 	"cmp"
 	"context"
+	"errors"
 	"fmt"
 	"io/fs"
 	"slices"
@@ -27,6 +28,7 @@ import (
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
@@ -114,6 +116,12 @@ func ApplyBundleWithBoxcutter(apply func(ctx context.Context, contentFS fs.FS, e
 			// If there was an error applying the resolved bundle,
 			// report the error via the Progressing condition.
 			setStatusProgressing(ext, wrapErrorWithResolutionInfo(state.resolvedRevisionMetadata.BundleMetadata, err))
+			// Only set Installed condition for retryable errors.
+			// For terminal errors (Progressing: False with a terminal reason such as Blocked or InvalidConfiguration),
+			// the Progressing condition already provides all necessary information about the failure.
+			if !errors.Is(err, reconcile.TerminalError(nil)) {
+				setInstalledStatusFromRevisionStates(ext, state.revisionStates)
+			}
 			return nil, err
 		}
 

internal/operator-controller/controllers/clusterextension_controller.go

@@ -50,6 +50,7 @@ import (
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/conditionsets"
 	"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
+	errorutil "github.com/operator-framework/operator-controller/internal/shared/util/error"
 	k8sutil "github.com/operator-framework/operator-controller/internal/shared/util/k8s"
 )
 
@@ -455,6 +456,19 @@ func (r *ClusterExtensionReconciler) SetupWithManager(mgr ctrl.Manager, opts ...
 }
 
 func wrapErrorWithResolutionInfo(resolved ocv1.BundleMetadata, err error) error {
+	// Preserve TerminalError type and reason through wrapping
+	if errors.Is(err, reconcile.TerminalError(nil)) {
+		// Extract the reason if one was provided
+		reason, hasReason := errorutil.ExtractTerminalReason(err)
+		// Unwrap to get the inner error, add context
+		innerErr := errorutil.UnwrapTerminal(err)
+		wrappedErr := fmt.Errorf("error for resolved bundle %q with version %q: %w", resolved.Name, resolved.Version, innerErr)
+		// Re-wrap preserving the reason if it existed
+		if hasReason {
+			return errorutil.NewTerminalError(reason, wrappedErr)
+		}
+		return reconcile.TerminalError(wrappedErr)
+	}
 	return fmt.Errorf("error for resolved bundle %q with version %q: %w", resolved.Name, resolved.Version, err)
 }
 

internal/operator-controller/controllers/common_controller.go

@@ -25,6 +25,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
+	errorutil "github.com/operator-framework/operator-controller/internal/shared/util/error"
 )
 
 const (
@@ -56,11 +57,8 @@ func setInstalledStatusFromRevisionStates(ext *ocv1.ClusterExtension, revisionSt
 	// Nothing is installed
 	if revisionStates.Installed == nil {
 		setInstallStatus(ext, nil)
-		if len(revisionStates.RollingOut) == 0 {
-			setInstalledStatusConditionFalse(ext, ocv1.ReasonFailed, "No bundle installed")
-		} else {
-			setInstalledStatusConditionFalse(ext, ocv1.ReasonAbsent, "No bundle installed")
-		}
+		reason := determineFailureReason(revisionStates.RollingOut)
+		setInstalledStatusConditionFalse(ext, reason, "No bundle installed")
 		return
 	}
 	// Something is installed
@@ -71,6 +69,45 @@ func setInstalledStatusFromRevisionStates(ext *ocv1.ClusterExtension, revisionSt
 	setInstalledStatusConditionSuccess(ext, fmt.Sprintf("Installed bundle %s successfully", revisionStates.Installed.Image))
 }
 
+// determineFailureReason determines the appropriate reason for the Installed condition
+// when no bundle is installed (Installed: False).
+//
+// Returns Failed when:
+//   - No rolling revisions exist (nothing to install)
+//   - The latest rolling revision has Reason: Retrying (indicates an error occurred)
+//
+// Returns Absent when:
+//   - Rolling revisions exist with the latest having Reason: RollingOut (healthy phased rollout in progress)
+//
+// Rationale:
+//   - Failed: Semantically indicates an error prevented installation
+//   - Absent: Semantically indicates "not there yet" (neutral state, e.g., during healthy rollout)
+//   - Retrying reason indicates an error (config validation, apply failure, etc.)
+//   - RollingOut reason indicates healthy progress (not an error)
+//   - Only the LATEST revision matters - old errors superseded by newer healthy revisions should not cause Failed
+//
+// Note: rollingRevisions are sorted in ascending order by Spec.Revision (oldest to newest),
+// so the latest revision is the LAST element in the array.
+func determineFailureReason(rollingRevisions []*RevisionMetadata) string {
+	if len(rollingRevisions) == 0 {
+		return ocv1.ReasonFailed
+	}
+
+	// Check if the LATEST rolling revision indicates an error (Retrying reason)
+	// Latest revision is the last element in the array (sorted ascending by Spec.Revision)
+	latestRevision := rollingRevisions[len(rollingRevisions)-1]
+	progressingCond := apimeta.FindStatusCondition(latestRevision.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing)
+	if progressingCond != nil && progressingCond.Reason == string(ocv1.ClusterExtensionRevisionReasonRetrying) {
+		// Retrying indicates an error occurred (config, apply, validation, etc.)
+		// Use Failed for semantic correctness: installation failed due to error
+		return ocv1.ReasonFailed
+	}
+
+	// No error detected in latest revision - it's progressing healthily (RollingOut) or no conditions set
+	// Use Absent for neutral "not installed yet" state
+	return ocv1.ReasonAbsent
+}
+
 // setInstalledStatusConditionSuccess sets the installed status condition to success.
 func setInstalledStatusConditionSuccess(ext *ocv1.ClusterExtension, message string) {
 	SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
@@ -119,12 +156,20 @@ func setStatusProgressing(ext *ocv1.ClusterExtension, err error) {
 
 	if err != nil {
 		progressingCond.Reason = ocv1.ReasonRetrying
-		progressingCond.Message = err.Error()
+		// Unwrap TerminalError to avoid "terminal error:" prefix in message
+		progressingCond.Message = errorutil.UnwrapTerminal(err).Error()
 	}
 
 	if errors.Is(err, reconcile.TerminalError(nil)) {
 		progressingCond.Status = metav1.ConditionFalse
-		progressingCond.Reason = ocv1.ReasonBlocked
+		// Try to extract a specific reason from the terminal error.
+		// If the error was created with NewTerminalError(reason, err), use that reason.
+		// Otherwise, fall back to the generic "Blocked" reason.
+		if reason, ok := errorutil.ExtractTerminalReason(err); ok {
+			progressingCond.Reason = reason
+		} else {
+			progressingCond.Reason = ocv1.ReasonBlocked
+		}
 	}
 
 	SetStatusCondition(&ext.Status.Conditions, progressingCond)