fix: stale deprecation conditions after ClusterExtension upgrade

When upgrading from a deprecated bundle to a non-deprecated one,
deprecation conditions (Deprecated, BundleDeprecated) remained True
with stale messages until a second reconciliation cycle. Fix by
refreshing deprecation status after a successful apply using the
newly installed bundle name. Also adds a Valid condition type to
ClusterExtension status, set by the shared ValidateClusterExtension
step for both Helm and Boxcutter applier pipelines.

Generated-by: Cursor/Claude

Author: camilamacedo86

api/v1/clusterextension_types.go

@@ -487,7 +487,11 @@ type RevisionStatus struct {
 type ClusterExtensionStatus struct {
 	// conditions represents the current state of the ClusterExtension.
 	//
-	// The set of condition types which apply to all spec.source variations are Installed and Progressing.
+	// The set of condition types which apply to all spec.source variations are Installed, Progressing, and Valid.
+	//
+	// The Valid condition represents whether the ClusterExtension's configuration passes all validation checks:
+	//   - When Valid is True and the Reason is Succeeded, the ClusterExtension configuration is valid (e.g., referenced ServiceAccount exists).
+	//   - When Valid is False and the Reason is Failed, the ClusterExtension configuration has validation errors.
 	//
 	// The Installed condition represents whether the bundle has been installed for this ClusterExtension:
 	//   - When Installed is True and the Reason is Succeeded, the bundle has been successfully installed.

api/v1/common_types.go

@@ -19,6 +19,7 @@ package v1
 const (
 	TypeInstalled   = "Installed"
 	TypeProgressing = "Progressing"
+	TypeValid       = "Valid"
 
 	// Installed reasons
 	ReasonAbsent = "Absent"

internal/operator-controller/conditionsets/conditionsets.go

@@ -31,6 +31,7 @@ var ConditionTypes = []string{
 	ocv1.TypeChannelDeprecated,
 	ocv1.TypeBundleDeprecated,
 	ocv1.TypeProgressing,
+	ocv1.TypeValid,
 }
 
 var ConditionReasons = []string{

internal/operator-controller/controllers/clusterextension_controller.go

@@ -63,6 +63,8 @@ type reconcileState struct {
 	revisionStates           *RevisionStates
 	resolvedRevisionMetadata *RevisionMetadata
 	imageFS                  fs.FS
+	resolvedDeprecation      *declcfg.Deprecation
+	hasCatalogData           bool
 }
 
 // ReconcileStepFunc represents a single step in the ClusterExtension reconciliation process.
@@ -175,7 +177,7 @@ func (r *ClusterExtensionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 //nolint:unparam // reason parameter is designed to be flexible, even if current callers use the same value
 func ensureFailureConditionsWithReason(ext *ocv1.ClusterExtension, reason v1alpha1.ConditionReason, message string) {
 	for _, condType := range conditionsets.ConditionTypes {
-		if isDeprecationCondition(condType) {
+		if isDeprecationCondition(condType) || isValidationCondition(condType) {
 			continue
 		}
 		cond := apimeta.FindStatusCondition(ext.Status.Conditions, condType)
@@ -353,6 +355,12 @@ func isDeprecationCondition(condType string) bool {
 	}
 }
 
+// isValidationCondition reports whether the given type is a validation
+// condition managed by the ValidateClusterExtension step.
+func isValidationCondition(condType string) bool {
+	return condType == ocv1.TypeValid
+}
+
 // deprecationInfo captures the deprecation data needed to update condition status.
 type deprecationInfo struct {
 	PackageEntries []declcfg.DeprecationEntry

internal/operator-controller/controllers/clusterextension_controller_test.go

@@ -344,6 +344,108 @@ func TestClusterExtensionUpgradeShowsInstalledBundleDeprecation(t *testing.T) {
 	require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
 }
 
+// TestClusterExtensionUpgradeFromDeprecatedBundleClearsDeprecation verifies that after
+// a successful upgrade from a deprecated bundle to a non-deprecated bundle, the deprecation
+// conditions are updated in the SAME reconciliation cycle (no stale conditions).
+//
+// Scenario:
+//   - Bundle v1.0.1 is installed and deprecated in the catalog
+//   - Bundle v1.0.3 is resolved (not deprecated) and the applier succeeds (rolloutSucceeded=true)
+//   - After the apply, BundleDeprecated should be False (reflecting the newly installed v1.0.3)
+//   - Deprecated rollup should also be False
+//
+// This is the regression test for the bug where deprecation conditions remained stale
+// (showing the old deprecated bundle) after a successful upgrade until the next reconciliation.
+func TestClusterExtensionUpgradeFromDeprecatedBundleClearsDeprecation(t *testing.T) {
+	ctx := context.Background()
+	pkgName := fmt.Sprintf("upgrade-clear-%s", rand.String(6))
+	installedBundleName := fmt.Sprintf("%s.v1.0.1", pkgName)
+	resolvedBundleName := fmt.Sprintf("%s.v1.0.3", pkgName)
+	deprecationMessage := fmt.Sprintf("%s is deprecated. Uninstall and install v1.0.3 for support.", installedBundleName)
+
+	cl, reconciler := newClientAndReconciler(t, func(d *deps) {
+		d.Resolver = resolve.Func(func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {
+			v := bundle.VersionRelease{
+				Version: bsemver.MustParse("1.0.3"),
+			}
+			return &declcfg.Bundle{
+					Name:    resolvedBundleName,
+					Package: pkgName,
+					Image:   fmt.Sprintf("quay.io/example/%s@sha256:resolved103", pkgName),
+				}, &v, &declcfg.Deprecation{
+					Entries: []declcfg.DeprecationEntry{{
+						Reference: declcfg.PackageScopedReference{
+							Schema: declcfg.SchemaBundle,
+							Name:   installedBundleName,
+						},
+						Message: deprecationMessage,
+					}},
+				}, nil
+		})
+		d.RevisionStatesGetter = &MockRevisionStatesGetter{
+			RevisionStates: &controllers.RevisionStates{
+				Installed: &controllers.RevisionMetadata{
+					Package: pkgName,
+					BundleMetadata: ocv1.BundleMetadata{
+						Name:    installedBundleName,
+						Version: "1.0.1",
+					},
+					Image: fmt.Sprintf("quay.io/example/%s@sha256:installed101", pkgName),
+				},
+			},
+		}
+		d.ImagePuller = &imageutil.MockPuller{ImageFS: fstest.MapFS{}}
+		d.Applier = &MockApplier{installCompleted: true}
+	})
+
+	extKey := types.NamespacedName{Name: fmt.Sprintf("cluster-extension-test-%s", rand.String(8))}
+	clusterExtension := &ocv1.ClusterExtension{
+		ObjectMeta: metav1.ObjectMeta{Name: extKey.Name},
+		Spec: ocv1.ClusterExtensionSpec{
+			Source: ocv1.SourceConfig{
+				SourceType: "Catalog",
+				Catalog:    &ocv1.CatalogFilter{PackageName: pkgName},
+			},
+			Namespace:      "default",
+			ServiceAccount: ocv1.ServiceAccountReference{Name: "default"},
+		},
+	}
+	require.NoError(t, cl.Create(ctx, clusterExtension))
+
+	res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
+	require.Equal(t, ctrl.Result{}, res)
+	require.NoError(t, err)
+
+	require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
+
+	// After a successful upgrade to v1.0.3, deprecation should reflect the NEW bundle
+	bundleCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeBundleDeprecated)
+	require.NotNil(t, bundleCond)
+	require.Equal(t, metav1.ConditionFalse, bundleCond.Status, "newly installed bundle v1.0.3 is NOT deprecated")
+	require.Equal(t, ocv1.ReasonNotDeprecated, bundleCond.Reason)
+
+	deprecatedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeDeprecated)
+	require.NotNil(t, deprecatedCond)
+	require.Equal(t, metav1.ConditionFalse, deprecatedCond.Status, "no deprecation exists after upgrade")
+	require.Equal(t, ocv1.ReasonNotDeprecated, deprecatedCond.Reason)
+
+	pkgCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypePackageDeprecated)
+	require.NotNil(t, pkgCond)
+	require.Equal(t, metav1.ConditionFalse, pkgCond.Status)
+
+	channelCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeChannelDeprecated)
+	require.NotNil(t, channelCond)
+	require.Equal(t, metav1.ConditionFalse, channelCond.Status)
+
+	// Verify the installed bundle IS v1.0.3
+	require.NotNil(t, clusterExtension.Status.Install)
+	require.Equal(t, resolvedBundleName, clusterExtension.Status.Install.Bundle.Name)
+	require.Equal(t, "1.0.3", clusterExtension.Status.Install.Bundle.Version)
+
+	verifyInvariants(ctx, t, reconciler.Client, clusterExtension)
+	require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
+}
+
 // TestClusterExtensionResolutionFailsWithoutCatalogDeprecationData verifies deprecation status handling when catalog data is unavailable.
 //
 // Scenario:
@@ -920,6 +1022,13 @@ func TestValidateClusterExtension(t *testing.T) {
 				require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
 
 				t.Log("By checking the status conditions")
+				validCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeValid)
+				require.NotNil(t, validCond)
+				require.Equal(t, metav1.ConditionFalse, validCond.Status)
+				require.Equal(t, ocv1.ReasonFailed, validCond.Reason)
+				require.Contains(t, validCond.Message, "operation cannot proceed due to the following validation error(s)")
+				require.Contains(t, validCond.Message, tt.errorMessageIncludes)
+
 				installedCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeInstalled)
 				require.NotNil(t, installedCond)
 				require.Equal(t, metav1.ConditionUnknown, installedCond.Status)
@@ -935,7 +1044,11 @@ func TestValidateClusterExtension(t *testing.T) {
 			} else {
 				require.NoError(t, err)
 				require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
-				require.Empty(t, clusterExtension.Status.Conditions)
+
+				validCond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1.TypeValid)
+				require.NotNil(t, validCond)
+				require.Equal(t, metav1.ConditionTrue, validCond.Status)
+				require.Equal(t, ocv1.ReasonSucceeded, validCond.Reason)
 			}
 			require.NoError(t, cl.DeleteAllOf(ctx, &ocv1.ClusterExtension{}))
 		})

internal/operator-controller/controllers/clusterextension_reconcile_steps.go

@@ -87,11 +87,13 @@ func ValidateClusterExtension(validators ...ClusterExtensionValidator) Reconcile
 
 		// If there are no validation errors, continue reconciliation
 		if len(validationErrors) == 0 {
+			setValidStatusConditionTrue(ext, "ClusterExtension passes all validations")
 			return nil, nil
 		}
 
 		// Set status conditions with the validation errors
 		err := fmt.Errorf("operation cannot proceed due to the following validation error(s): %w", errors.Join(validationErrors...))
+		setValidStatusConditionFalse(ext, err.Error())
 		setInstalledStatusConditionUnknown(ext, err.Error())
 		setStatusProgressing(ext, err)
 		return nil, err
@@ -180,6 +182,8 @@ func ResolveBundle(r resolve.Resolver, c client.Client) ReconcileStepFunc {
 		//   the deprecation status to unknown? Or perhaps we somehow combine the deprecation information from
 		//   all catalogs? This needs a follow-up discussion and PR.
 		hasCatalogData := err == nil || resolvedDeprecation != nil
+		state.resolvedDeprecation = resolvedDeprecation
+		state.hasCatalogData = hasCatalogData
 		SetDeprecationStatus(ext, installedBundleName, resolvedDeprecation, hasCatalogData)
 
 		if err != nil {
@@ -437,6 +441,15 @@ func ApplyBundle(a Applier) ReconcileStepFunc {
 		}
 		setInstalledStatusFromRevisionStates(ext, state.revisionStates)
 
+		// After a successful rollout the installed bundle may have changed
+		// (e.g. upgrade from a deprecated to a non-deprecated version).
+		// Refresh deprecation conditions so they reflect the newly running
+		// bundle instead of the pre-upgrade bundle that was used during
+		// resolution.
+		if rolloutSucceeded && state.revisionStates.Installed != nil {
+			SetDeprecationStatus(ext, state.revisionStates.Installed.Name, state.resolvedDeprecation, state.hasCatalogData)
+		}
+
 		// If there was an error applying the resolved bundle,
 		// report the error via the Progressing condition.
 		if err != nil {

internal/operator-controller/controllers/common_controller.go

@@ -145,6 +145,26 @@ func setInstallStatus(ext *ocv1.ClusterExtension, installStatus *ocv1.ClusterExt
 	ext.Status.Install = installStatus
 }
 
+func setValidStatusConditionTrue(ext *ocv1.ClusterExtension, message string) {
+	SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
+		Type:               ocv1.TypeValid,
+		Status:             metav1.ConditionTrue,
+		Reason:             ocv1.ReasonSucceeded,
+		Message:            message,
+		ObservedGeneration: ext.GetGeneration(),
+	})
+}
+
+func setValidStatusConditionFalse(ext *ocv1.ClusterExtension, message string) {
+	SetStatusCondition(&ext.Status.Conditions, metav1.Condition{
+		Type:               ocv1.TypeValid,
+		Status:             metav1.ConditionFalse,
+		Reason:             ocv1.ReasonFailed,
+		Message:            message,
+		ObservedGeneration: ext.GetGeneration(),
+	})
+}
+
 func setStatusProgressing(ext *ocv1.ClusterExtension, err error) {
 	progressingCond := metav1.Condition{
 		Type:               ocv1.TypeProgressing,