Fix deprecation conditions and make YAML output cleaner

Deprecation conditions now only show what matters:
- When deprecated: condition shows "True" with deprecation message
- When not deprecated: condition is absent (no clutter!)
- When we can't check: condition shows "Unknown"

This fixes three problems:
1. Install errors were leaking into deprecation conditions
2. Catalog unavailable showed "False" instead of "Unknown"
3. BundleDeprecated was checking the wrong bundle (resolved vs installed)

Also improved the code:
- Simpler logic (clear all, then add only what's needed)
- Better reason values (Deprecated, DeprecationStatusUnknown, Absent)
- Comprehensive test coverage for all scenarios

Assisted-by: Cursor

Author: camilamacedo86

api/v1/clusterextension_types.go

@@ -489,12 +489,13 @@ type ClusterExtensionStatus struct {
 	// When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterExtensionRevisions in active roll out.
 	// </opcon:experimental:description>
 	//
-	// When the ClusterExtension is sourced from a catalog, it may also communicate a deprecation condition.
-	// These are indications from a package owner to guide users away from a particular package, channel, or bundle:
-	//   - BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
-	//   - ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
-	//   - PackageDeprecated is set if the requested package is marked deprecated in the catalog.
-	//   - Deprecated is a rollup condition that is present when any of the deprecated conditions are present.
+	// When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
+	// These are indications from a package owner to guide users away from a particular package, channel, or bundle.
+	// Deprecation conditions are only present when there's something to report - absence means "not deprecated".
+	//   - BundleDeprecated is set to True if the installed bundle is marked as deprecated in the catalog, or Unknown if no bundle is installed yet.
+	//   - ChannelDeprecated is set to True if any requested channel is marked as deprecated in the catalog, or Unknown if the channel is not found.
+	//   - PackageDeprecated is set to True if the requested package is marked as deprecated in the catalog, or Unknown if the package is not found.
+	//   - Deprecated is a rollup condition that is present only when at least one deprecation exists (True) or when catalog information is unavailable (Unknown).
 	//
 	// +listType=map
 	// +listMapKey=type

api/v1/common_types.go

@@ -29,7 +29,8 @@ const (
 	ReasonBlocked    = "Blocked"
 
 	// Deprecation reasons
-	ReasonDeprecated = "Deprecated"
+	ReasonDeprecated               = "Deprecated"
+	ReasonDeprecationStatusUnknown = "DeprecationStatusUnknown"
 
 	// Common reasons
 	ReasonSucceeded = "Succeeded"

docs/api-reference/olmv1-api-reference.md

@@ -359,7 +359,7 @@ _Appears in:_
 
 | Field | Description | Default | Validation |
 | --- | --- | --- | --- |
-| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | conditions represents the current state of the ClusterExtension.<br />The set of condition types which apply to all spec.source variations are Installed and Progressing.<br />The Installed condition represents whether the bundle has been installed for this ClusterExtension:<br />  - When Installed is True and the Reason is Succeeded, the bundle has been successfully installed.<br />  - When Installed is False and the Reason is Failed, the bundle has failed to install.<br />The Progressing condition represents whether or not the ClusterExtension is advancing towards a new state.<br />When Progressing is True and the Reason is Succeeded, the ClusterExtension is making progress towards a new state.<br />When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.<br />When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.<br /><opcon:experimental:description><br />When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterExtensionRevisions in active roll out.<br /></opcon:experimental:description><br />When the ClusterExtension is sourced from a catalog, it may also communicate a deprecation condition.<br />These are indications from a package owner to guide users away from a particular package, channel, or bundle:<br />  - BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.<br />  - ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.<br />  - PackageDeprecated is set if the requested package is marked deprecated in the catalog.<br />  - Deprecated is a rollup condition that is present when any of the deprecated conditions are present. |  |  |
+| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.31/#condition-v1-meta) array_ | conditions represents the current state of the ClusterExtension.<br />The set of condition types which apply to all spec.source variations are Installed and Progressing.<br />The Installed condition represents whether the bundle has been installed for this ClusterExtension:<br />  - When Installed is True and the Reason is Succeeded, the bundle has been successfully installed.<br />  - When Installed is False and the Reason is Failed, the bundle has failed to install.<br />The Progressing condition represents whether or not the ClusterExtension is advancing towards a new state.<br />When Progressing is True and the Reason is Succeeded, the ClusterExtension is making progress towards a new state.<br />When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.<br />When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.<br /><opcon:experimental:description><br />When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterExtensionRevisions in active roll out.<br /></opcon:experimental:description><br />When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.<br />These are indications from a package owner to guide users away from a particular package, channel, or bundle.<br />Deprecation conditions are only present when there's something to report - absence means "not deprecated".<br />  - BundleDeprecated is set to True if the installed bundle is marked as deprecated in the catalog, or Unknown if no bundle is installed yet.<br />  - ChannelDeprecated is set to True if any requested channel is marked as deprecated in the catalog, or Unknown if the channel is not found.<br />  - PackageDeprecated is set to True if the requested package is marked as deprecated in the catalog, or Unknown if the package is not found.<br />  - Deprecated is a rollup condition that is present only when at least one deprecation exists (True) or when catalog information is unavailable (Unknown). |  |  |
 | `install` _[ClusterExtensionInstallStatus](#clusterextensioninstallstatus)_ | install is a representation of the current installation status for this ClusterExtension. |  |  |
 | `activeRevisions` _[RevisionStatus](#revisionstatus) array_ | activeRevisions holds a list of currently active (non-archived) ClusterExtensionRevisions,<br />including both installed and rolling out revisions.<br /><opcon:experimental> |  |  |
 

helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml

@@ -591,12 +591,13 @@ spec:
 
                   When Progressing is True and Reason is RollingOut, the ClusterExtension has one or more ClusterExtensionRevisions in active roll out.
 
-                  When the ClusterExtension is sourced from a catalog, it may also communicate a deprecation condition.
-                  These are indications from a package owner to guide users away from a particular package, channel, or bundle:
-                    - BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
-                    - ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
-                    - PackageDeprecated is set if the requested package is marked deprecated in the catalog.
-                    - Deprecated is a rollup condition that is present when any of the deprecated conditions are present.
+                  When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
+                  These are indications from a package owner to guide users away from a particular package, channel, or bundle.
+                  Deprecation conditions are only present when there's something to report - absence means "not deprecated".
+                    - BundleDeprecated is set to True if the installed bundle is marked as deprecated in the catalog, or Unknown if no bundle is installed yet.
+                    - ChannelDeprecated is set to True if any requested channel is marked as deprecated in the catalog, or Unknown if the channel is not found.
+                    - PackageDeprecated is set to True if the requested package is marked as deprecated in the catalog, or Unknown if the package is not found.
+                    - Deprecated is a rollup condition that is present only when at least one deprecation exists (True) or when catalog information is unavailable (Unknown).
                 items:
                   description: Condition contains details for one aspect of the current
                     state of this API Resource.

helm/olmv1/base/operator-controller/crd/standard/olm.operatorframework.io_clusterextensions.yaml

@@ -507,12 +507,13 @@ spec:
                   When Progressing is True and the Reason is Retrying, the ClusterExtension has encountered an error that could be resolved on subsequent reconciliation attempts.
                   When Progressing is False and the Reason is Blocked, the ClusterExtension has encountered an error that requires manual intervention for recovery.
 
-                  When the ClusterExtension is sourced from a catalog, it may also communicate a deprecation condition.
-                  These are indications from a package owner to guide users away from a particular package, channel, or bundle:
-                    - BundleDeprecated is set if the requested bundle version is marked deprecated in the catalog.
-                    - ChannelDeprecated is set if the requested channel is marked deprecated in the catalog.
-                    - PackageDeprecated is set if the requested package is marked deprecated in the catalog.
-                    - Deprecated is a rollup condition that is present when any of the deprecated conditions are present.
+                  When the ClusterExtension is sourced from a catalog, it may surface deprecation conditions based on catalog metadata.
+                  These are indications from a package owner to guide users away from a particular package, channel, or bundle.
+                  Deprecation conditions are only present when there's something to report - absence means "not deprecated".
+                    - BundleDeprecated is set to True if the installed bundle is marked as deprecated in the catalog, or Unknown if no bundle is installed yet.
+                    - ChannelDeprecated is set to True if any requested channel is marked as deprecated in the catalog, or Unknown if the channel is not found.
+                    - PackageDeprecated is set to True if the requested package is marked as deprecated in the catalog, or Unknown if the package is not found.
+                    - Deprecated is a rollup condition that is present only when at least one deprecation exists (True) or when catalog information is unavailable (Unknown).
                 items:
                   description: Condition contains details for one aspect of the current
                     state of this API Resource.

internal/operator-controller/conditionsets/conditionsets.go

@@ -36,6 +36,7 @@ var ConditionTypes = []string{
 var ConditionReasons = []string{
 	ocv1.ReasonSucceeded,
 	ocv1.ReasonDeprecated,
+	ocv1.ReasonDeprecationStatusUnknown,
 	ocv1.ReasonFailed,
 	ocv1.ReasonBlocked,
 	ocv1.ReasonRetrying,

internal/operator-controller/controllers/clusterextension_admission_test.go

@@ -13,20 +13,20 @@ import (
 )
 
 func TestClusterExtensionSourceConfig(t *testing.T) {
-	sourceTypeEmptyError := "Invalid value: null"
+	sourceTypeEmptyErrors := []string{"Invalid value: \"null\"", "Invalid value: null"}
 	sourceTypeMismatchError := "spec.source.sourceType: Unsupported value"
 	sourceConfigInvalidError := "spec.source: Invalid value"
 	// unionField represents the required Catalog or (future) Bundle field required by SourceConfig
 	testCases := []struct {
 		name       string
 		sourceType string
 		unionField string
-		errMsg     string
+		errMsgs    []string
 	}{
-		{"sourceType is null", "", "Catalog", sourceTypeEmptyError},
-		{"sourceType is invalid", "Invalid", "Catalog", sourceTypeMismatchError},
-		{"catalog field does not exist", "Catalog", "", sourceConfigInvalidError},
-		{"sourceConfig has required fields", "Catalog", "Catalog", ""},
+		{"sourceType is null", "", "Catalog", sourceTypeEmptyErrors},
+		{"sourceType is invalid", "Invalid", "Catalog", []string{sourceTypeMismatchError}},
+		{"catalog field does not exist", "Catalog", "", []string{sourceConfigInvalidError}},
+		{"sourceConfig has required fields", "Catalog", "Catalog", nil},
 	}
 
 	t.Parallel()
@@ -62,12 +62,20 @@ func TestClusterExtensionSourceConfig(t *testing.T) {
 				}))
 			}
 
-			if tc.errMsg == "" {
+			if len(tc.errMsgs) == 0 {
 				require.NoError(t, err, "unexpected error for sourceType %q: %w", tc.sourceType, err)
-			} else {
-				require.Error(t, err)
-				require.Contains(t, err.Error(), tc.errMsg)
+				return
+			}
+
+			require.Error(t, err)
+			matched := false
+			for _, msg := range tc.errMsgs {
+				if strings.Contains(err.Error(), msg) {
+					matched = true
+					break
+				}
 			}
+			require.True(t, matched, "expected one of %v in error %q", tc.errMsgs, err)
 		})
 	}
 }