NO-ISSUE: Synchronize From Upstream Repositories

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"

commitchecker.yaml

@@ -1,4 +1,4 @@
-expectedMergeBase: fbe909f7ba35a9f771da6ec0431bbde2ac45d5fb
+expectedMergeBase: ee4bab558cc0f7f46ff480bfa8917ffe4650a210
 upstreamBranch: main
 upstreamOrg: operator-framework
 upstreamRepo: operator-controller

go.mod

@@ -17,7 +17,7 @@ require (
 	github.com/google/go-containerregistry v0.20.7
 	github.com/google/renameio/v2 v2.0.2
 	github.com/gorilla/handlers v1.5.2
-	github.com/klauspost/compress v1.18.2
+	github.com/klauspost/compress v1.18.3
 	github.com/opencontainers/go-digest v1.0.0
 	github.com/opencontainers/image-spec v1.1.1
 	github.com/operator-framework/api v0.37.0

go.sum

@@ -323,8 +323,8 @@ github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnr
 github.com/json-iterator/go v1.1.12/go.mod h1:e30LSqwooZae/UwlEbR2852Gd8hjQvJoHmT4TnhNGBo=
 github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8=
 github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck=
-github.com/klauspost/compress v1.18.2 h1:iiPHWW0YrcFgpBYhsA6D1+fqHssJscY/Tm/y2Uqnapk=
-github.com/klauspost/compress v1.18.2/go.mod h1:R0h/fSBs8DE4ENlcrlib3PsXS61voFxhIs2DeRhCvJ4=
+github.com/klauspost/compress v1.18.3 h1:9PJRvfbmTabkOX8moIpXPbMMbYN60bWImDDU7L+/6zw=
+github.com/klauspost/compress v1.18.3/go.mod h1:R0h/fSBs8DE4ENlcrlib3PsXS61voFxhIs2DeRhCvJ4=
 github.com/klauspost/pgzip v1.2.6 h1:8RXeL5crjEUFnR2/Sn6GJNWtSQ3Dk8pq4CL3jvdDyjU=
 github.com/klauspost/pgzip v1.2.6/go.mod h1:Ch1tH69qFZu15pkjo5kYi6mth2Zzwzt50oCQKQE9RUs=
 github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=

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,
@@ -393,7 +425,7 @@ func Test_RegistryV1ManifestProvider_SingleOwnNamespaceSupport(t *testing.T) {
 		})
 		require.Error(t, err)
 		require.Contains(t, err.Error(), "invalid ClusterExtension configuration:")
-		require.Contains(t, err.Error(), "watchNamespace must be")
+		require.Contains(t, err.Error(), "must be")
 		require.Contains(t, err.Error(), "install-namespace")
 	})
 

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/config/config.go

@@ -31,6 +31,7 @@ import (
 	"strings"
 
 	"github.com/santhosh-tekuri/jsonschema/v6"
+	"github.com/santhosh-tekuri/jsonschema/v6/kind"
 	"sigs.k8s.io/yaml"
 )
 
@@ -208,7 +209,7 @@ func validateConfigWithSchema(configBytes []byte, schema map[string]any, install
 				return fmt.Errorf("value must be a string")
 			}
 			if str != installNamespace {
-				return fmt.Errorf("invalid value %q: watchNamespace must be %q (the namespace where the operator is installed) because this operator only supports OwnNamespace install mode", str, installNamespace)
+				return fmt.Errorf("invalid value %q: must be %q (the namespace where the operator is installed) because this operator only supports OwnNamespace install mode", str, installNamespace)
 			}
 			return nil
 		},
@@ -228,7 +229,7 @@ func validateConfigWithSchema(configBytes []byte, schema map[string]any, install
 				return fmt.Errorf("value must be a string")
 			}
 			if str == installNamespace {
-				return fmt.Errorf("invalid value %q: watchNamespace must be different from %q (the install namespace) because this operator uses SingleNamespace install mode to watch a different namespace", str, installNamespace)
+				return fmt.Errorf("invalid value %q: must be different from %q (the install namespace) because this operator uses SingleNamespace install mode to watch a different namespace", str, installNamespace)
 			}
 			return nil
 		},
@@ -294,25 +295,29 @@ func formatSchemaError(err error) error {
 
 // formatSingleError formats a single validation error from the schema library.
 func formatSingleError(errUnit jsonschema.OutputUnit) string {
+	if errUnit.Error == nil {
+		return ""
+	}
+
 	// Check the keyword location to identify the error type
-	switch {
-	case strings.Contains(errUnit.KeywordLocation, "/required"):
+	switch errKind := errUnit.Error.Kind.(type) {
+	case *kind.Required:
 		// Missing required field
 		fieldName := extractFieldNameFromMessage(errUnit.Error)
 		if fieldName != "" {
 			return fmt.Sprintf("required field %q is missing", fieldName)
 		}
 		return "required field is missing"
 
-	case strings.Contains(errUnit.KeywordLocation, "/additionalProperties"):
+	case *kind.AdditionalProperties:
 		// Unknown/additional field
 		fieldName := extractFieldNameFromMessage(errUnit.Error)
 		if fieldName != "" {
 			return fmt.Sprintf("unknown field %q", fieldName)
 		}
 		return "unknown field"
 
-	case strings.Contains(errUnit.KeywordLocation, "/type"):
+	case *kind.Type:
 		// Type mismatch (e.g., got null, want string)
 		fieldPath := buildFieldPath(errUnit.InstanceLocation)
 		if fieldPath != "" {
@@ -324,16 +329,14 @@ func formatSingleError(errUnit jsonschema.OutputUnit) string {
 		}
 		return fmt.Sprintf("invalid type: %s", errUnit.Error.String())
 
-	case strings.Contains(errUnit.KeywordLocation, "/format"):
-		// Custom format validation (e.g., OwnNamespace, SingleNamespace constraints)
-		// These already have good error messages from our custom validators
-		if errUnit.Error != nil {
-			return errUnit.Error.String()
-		}
+	case *kind.Format:
 		fieldPath := buildFieldPath(errUnit.InstanceLocation)
-		return fmt.Sprintf("invalid format for field %q", fieldPath)
+		if fieldPath != "" {
+			return fmt.Sprintf("invalid format for field %q: %s", fieldPath, errUnit.Error.String())
+		}
+		return fmt.Sprintf("invalid format: %s", errUnit.Error.String())
 
-	case strings.Contains(errUnit.KeywordLocation, "/anyOf"):
+	case *kind.AnyOf:
 		// anyOf validation failed - could be null or wrong type
 		// This happens when a field accepts [null, string] but got something else
 		fieldPath := buildFieldPath(errUnit.InstanceLocation)
@@ -342,13 +345,31 @@ func formatSingleError(errUnit jsonschema.OutputUnit) string {
 		}
 		return "invalid value"
 
+	case *kind.MaxLength:
+		fieldPath := buildFieldPath(errUnit.InstanceLocation)
+		if fieldPath != "" {
+			return fmt.Sprintf("field %q must have maximum length of %d (len=%d)", fieldPath, errKind.Want, errKind.Got)
+		}
+		return errUnit.Error.String()
+
+	case *kind.MinLength:
+		fieldPath := buildFieldPath(errUnit.InstanceLocation)
+		if fieldPath != "" {
+			return fmt.Sprintf("field %q must have minimum length of %d (len=%d)", fieldPath, errKind.Want, errKind.Got)
+		}
+		return errUnit.Error.String()
+
+	case *kind.Pattern:
+		fieldPath := buildFieldPath(errUnit.InstanceLocation)
+		if fieldPath != "" {
+			return fmt.Sprintf("field %q must match pattern %q", fieldPath, errKind.Want)
+		}
+		return errUnit.Error.String()
+
 	default:
-		// Unknown error type - return the library's error message
+		// Unhandled error type - return the library's error message
 		// This serves as a fallback for future schema features we haven't customized yet
-		if errUnit.Error != nil {
-			return errUnit.Error.String()
-		}
-		return ""
+		return errUnit.Error.String()
 	}
 }
 

internal/operator-controller/config/error_formatting_test.go

@@ -1,6 +1,7 @@
 package config_test
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -72,9 +73,9 @@ func Test_ErrorFormatting_SchemaLibraryVersion(t *testing.T) {
 			supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeOwnNamespace},
 			installNamespace:      "correct-namespace",
 			expectedErrSubstrings: []string{
+				"invalid format for field \"watchNamespace\"",
 				"invalid value",
 				"wrong-namespace",
-				"watchNamespace must be",
 				"correct-namespace",
 				"the namespace where the operator is installed",
 				"OwnNamespace install mode",
@@ -86,14 +87,38 @@ func Test_ErrorFormatting_SchemaLibraryVersion(t *testing.T) {
 			supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace},
 			installNamespace:      "install-ns",
 			expectedErrSubstrings: []string{
+				"invalid format for field \"watchNamespace\"",
+				"not valid singleNamespaceInstallMode",
 				"invalid value",
 				"install-ns",
-				"watchNamespace must be different from",
+				"must be different from",
 				"the install namespace",
 				"SingleNamespace install mode",
 				"watch a different namespace",
 			},
 		},
+		{
+			name:                  "SingleNamespace constraint error bad namespace format",
+			rawConfig:             []byte(`{"watchNamespace": "---AAAA-BBBB-super-long-namespace-that-that-is-waaaaaaaaayyy-longer-than-sixty-three-characters"}`),
+			supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace},
+			installNamespace:      "install-ns",
+			expectedErrSubstrings: []string{
+				"field \"watchNamespace\"",
+				"must match pattern \"^[a-z0-9]([-a-z0-9]*[a-z0-9])?$\"",
+			},
+		},
+		{
+			name:                  "Single- and OwnNamespace constraint error bad namespace format",
+			rawConfig:             []byte(`{"watchNamespace": ` + strings.Repeat("A", 63) + `"}`),
+			supportedInstallModes: []v1alpha1.InstallModeType{v1alpha1.InstallModeTypeSingleNamespace, v1alpha1.InstallModeTypeOwnNamespace},
+			installNamespace:      "install-ns",
+			expectedErrSubstrings: []string{
+				"invalid configuration",
+				"multiple errors found",
+				"must have maximum length of 63 (len=64)",
+				"must match pattern \"^[a-z0-9]([-a-z0-9]*[a-z0-9])?$\"",
+			},
+		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			rv1 := bundle.RegistryV1{

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)
 }