🐛 fix: Configuration validation errors now show "Failed" status instead of "Absent" (#2465)

* Make config errors permanent instead of retrying

When configuration is invalid, mark it as a permanent error so the
system doesn't keep trying to install it over and over. This saves
resources and makes it clear the user needs to fix their config.

Assisted-by: Claude <noreply@anthropic.com>

* Show "Failed" status when config is wrong

Before: Invalid config showed status as "Absent" which was confusing
After: Shows "Failed" so users know something is wrong

Only checks the latest attempt, not old ones that don't matter anymore.

Assisted-by: Claude <noreply@anthropic.com>

* Fix status not updating when BOXCUTTER config fails

BOXCUTTER runtime was returning early on config errors without
setting the status. Now it updates the status first so users can
see what went wrong.

Assisted-by: Claude <noreply@anthropic.com>

* Add tests for config error handling

Tests check:
- Config errors are permanent (not retried)
- Status shows "Failed" when config is wrong
- Namespace names are validated
- AllNamespaces operators reject watchNamespace config

Includes both unit tests and end-to-end tests.

Assisted-by: Claude <noreply@anthropic.com>

* Remove 'terminal error:' prefix from status messages

When TerminalError is used, unwrap it before showing the error
message in status conditions. This prevents the 'terminal error:'
prefix from appearing in user-facing messages.

Changes:
- Add UnwrapTerminal() helper function to cleanly unwrap terminal errors
- Use the helper in setStatusProgressing and wrapErrorWithResolutionInfo
- Don't set Installed condition when Progressing is Blocked (terminal error)
  since the Progressing condition already provides all necessary information

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

* 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>

* Update internal/operator-controller/controllers/boxcutter_reconcile_steps.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

---------

Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Author: 3 people

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)