Add custom TerminalError with granular Reason support

Previously, all terminal errors used the generic "Blocked" reason in
the Progressing condition. This wasn't semantically correct - "Blocked"
was intended for external blockers like resource collisions, not for
user configuration errors.

This commit introduces a custom TerminalError type that can carry a
specific Reason field, allowing more precise categorization of terminal
errors in status conditions.

Changes:
- Add ReasonInvalidConfiguration constant for configuration errors
- Create NewTerminalError(reason, err) to wrap errors with a reason
- Add ExtractTerminalReason(err) to retrieve the reason from errors
- Update setStatusProgressing to use extracted reason when available
- Fall back to ReasonBlocked for backward compatibility with plain
  reconcile.TerminalError() usage
- Update config validation to use ReasonInvalidConfiguration
- Update tests and e2e expectations accordingly

Result:
- Config errors now show: Progressing: False, Reason: InvalidConfiguration
- Generic terminal errors show: Progressing: False, Reason: Blocked
- More helpful and semantically correct status reporting

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

Author: camilamacedo86

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

@@ -9,14 +9,14 @@ import (
 	"helm.sh/helm/v3/pkg/chart"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"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
@@ -78,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, reconcile.TerminalError(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/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/clusterextension_controller.go

@@ -50,7 +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"
-	ocerror "github.com/operator-framework/operator-controller/internal/shared/util/error"
+	errorutil "github.com/operator-framework/operator-controller/internal/shared/util/error"
 	k8sutil "github.com/operator-framework/operator-controller/internal/shared/util/k8s"
 )
 
@@ -456,11 +456,17 @@ func (r *ClusterExtensionReconciler) SetupWithManager(mgr ctrl.Manager, opts ...
 }
 
 func wrapErrorWithResolutionInfo(resolved ocv1.BundleMetadata, err error) error {
-	// Preserve TerminalError type through wrapping by re-wrapping after adding context
+	// Preserve TerminalError type and reason through wrapping
 	if errors.Is(err, reconcile.TerminalError(nil)) {
-		// Unwrap to get the inner error, add context, then re-wrap as TerminalError
-		innerErr := ocerror.UnwrapTerminal(err)
+		// 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,7 +25,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
-	ocerror "github.com/operator-framework/operator-controller/internal/shared/util/error"
+	errorutil "github.com/operator-framework/operator-controller/internal/shared/util/error"
 )
 
 const (
@@ -157,12 +157,19 @@ func setStatusProgressing(ext *ocv1.ClusterExtension, err error) {
 	if err != nil {
 		progressingCond.Reason = ocv1.ReasonRetrying
 		// Unwrap TerminalError to avoid "terminal error:" prefix in message
-		progressingCond.Message = ocerror.UnwrapTerminal(err).Error()
+		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)

internal/operator-controller/controllers/common_controller_test.go

@@ -14,6 +14,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"
 )
 
 func TestSetStatusProgressing(t *testing.T) {
@@ -46,7 +47,7 @@ func TestSetStatusProgressing(t *testing.T) {
 			},
 		},
 		{
-			name:             "non-nil ClusterExtension, terminal error, Progressing condition has status False with reason Blocked",
+			name:             "non-nil ClusterExtension, terminal error without reason, Progressing condition has status False with reason Blocked",
 			err:              reconcile.TerminalError(errors.New("boom")),
 			clusterExtension: &ocv1.ClusterExtension{},
 			expected: metav1.Condition{
@@ -56,6 +57,17 @@ func TestSetStatusProgressing(t *testing.T) {
 				Message: "boom",
 			},
 		},
+		{
+			name:             "non-nil ClusterExtension, terminal error with InvalidConfiguration reason, Progressing condition has status False with that reason",
+			err:              errorutil.NewTerminalError(ocv1.ReasonInvalidConfiguration, errors.New("missing required field")),
+			clusterExtension: &ocv1.ClusterExtension{},
+			expected: metav1.Condition{
+				Type:    ocv1.TypeProgressing,
+				Status:  metav1.ConditionFalse,
+				Reason:  ocv1.ReasonInvalidConfiguration,
+				Message: "missing required field",
+			},
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			setStatusProgressing(tc.clusterExtension, tc.err)

internal/shared/util/error/terminal.go

@@ -6,6 +6,57 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
+// terminalErrorWithReason is an internal error type that carries a Reason field
+// to provide more granular categorization of terminal errors for status conditions.
+type terminalErrorWithReason struct {
+	reason string
+	err    error
+}
+
+func (e *terminalErrorWithReason) Error() string {
+	return e.err.Error()
+}
+
+func (e *terminalErrorWithReason) Unwrap() error {
+	return e.err
+}
+
+// NewTerminalError creates a terminal error with a specific reason.
+// The error is wrapped with reconcile.TerminalError so controller-runtime
+// recognizes it as terminal, while preserving the reason for status reporting.
+//
+// Example usage:
+//
+//	return error.NewTerminalError(ocv1.ReasonInvalidConfiguration, fmt.Errorf("missing required field"))
+//
+// The reason can be extracted later using ExtractTerminalReason() when setting
+// status conditions to provide more specific feedback than just "Blocked".
+func NewTerminalError(reason string, err error) error {
+	return reconcile.TerminalError(&terminalErrorWithReason{
+		reason: reason,
+		err:    err,
+	})
+}
+
+// ExtractTerminalReason extracts the reason from a terminal error created with
+// NewTerminalError. Returns the reason and true if found, or empty string and
+// false if the error was not created with NewTerminalError.
+//
+// This allows setStatusProgressing to use specific reasons like "InvalidConfiguration"
+// instead of the generic "Blocked" for all terminal errors.
+func ExtractTerminalReason(err error) (string, bool) {
+	if err == nil {
+		return "", false
+	}
+	// Unwrap the reconcile.TerminalError wrapper first
+	unwrapped := errors.Unwrap(err)
+	var terr *terminalErrorWithReason
+	if errors.As(unwrapped, &terr) {
+		return terr.reason, true
+	}
+	return "", false
+}
+
 func WrapTerminal(err error, isTerminal bool) error {
 	if !isTerminal || err == nil {
 		return err

test/e2e/features/install.feature

@@ -117,7 +117,7 @@ Feature: Install ClusterExtension
               matchLabels:
                 "olm.operatorframework.io/metadata.name": test-catalog
       """
-    And ClusterExtension reports Progressing as False with Reason Blocked and Message:
+    And ClusterExtension reports Progressing as False with Reason InvalidConfiguration and Message:
       """
       error for resolved bundle "single-namespace-operator.1.0.0" with version "1.0.0":
       invalid ClusterExtension configuration: invalid configuration: required field "watchNamespace" is missing
@@ -169,7 +169,7 @@ Feature: Install ClusterExtension
               matchLabels:
                 "olm.operatorframework.io/metadata.name": test-catalog
       """
-    And ClusterExtension reports Progressing as False with Reason Blocked and Message:
+    And ClusterExtension reports Progressing as False with Reason InvalidConfiguration and Message:
       """
       error for resolved bundle "own-namespace-operator.1.0.0" with version
       "1.0.0": invalid ClusterExtension configuration: invalid configuration: required
@@ -197,7 +197,7 @@ Feature: Install ClusterExtension
               matchLabels:
                 "olm.operatorframework.io/metadata.name": test-catalog
       """
-    And ClusterExtension reports Progressing as False with Reason Blocked and Message:
+    And ClusterExtension reports Progressing as False with Reason InvalidConfiguration and Message:
       """
       error for resolved bundle "own-namespace-operator.1.0.0" with version
       "1.0.0": invalid ClusterExtension configuration: invalid configuration: 'some-ns'