Fix duplicate ClusterExtensionRevisions during Helm-to-Boxcutter migration

This commit fixes OCPBUGS-62943 where two ClusterExtensionRevisions were
being created during Helm-to-Boxcutter migration instead of just one.

Root causes:
1. Manifest ordering inconsistency: CRDs from Helm release manifest and
   bundle manifest appeared in different orders, causing PhaseSort to
   produce different phase structures even though they contained the same
   objects.

2. CollisionProtection mismatch: The migrated revision had
   collisionProtection=None (needed to adopt Helm-managed resources) while
   the bundle-generated revision had collisionProtection=Prevent (the
   default value).

Solution:

1. Added deterministic sorting in PhaseSort (phase.go):
   - Sort objects within each phase by Group, Version, Kind, Namespace, Name
   - Ensures consistent phase structure regardless of input order
   - Critical for comparing revisions from different sources

2. Added CollisionProtection preservation (boxcutter.go):
   - New preserveCollisionProtection() function copies CollisionProtection
     values from current revision to desired revision for matching objects
   - New objectKey() helper generates unique keys based on GVK+namespace+name
   - Called before patching to ensure CollisionProtection values match

With these changes, only a single ClusterExtensionRevision is created
during Helm-to-Boxcutter migration, as expected.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Todd Short <tshort@redhat.com>

Author: tmshort

internal/operator-controller/applier/boxcutter.go

@@ -308,6 +308,12 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
 		desiredRevision.Spec.Revision = currentRevision.Spec.Revision
 		desiredRevision.Name = currentRevision.Name
 
+		// Preserve CollisionProtection settings from the current revision to avoid
+		// creating a new revision when the only difference is the CollisionProtection value.
+		// This is critical during Helm-to-Boxcutter migration where the migrated revision
+		// has CollisionProtection=None to adopt Helm-managed resources.
+		preserveCollisionProtection(desiredRevision, currentRevision)
+
 		err := bc.createOrUpdate(ctx, desiredRevision)
 		switch {
 		case apierrors.IsInvalid(err):
@@ -380,6 +386,38 @@ func (bc *Boxcutter) apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
 	return true, "", nil
 }
 
+// preserveCollisionProtection copies the CollisionProtection settings from the current revision to the desired revision
+// for objects that have matching GVK, namespace, and name. This ensures that when patching an existing revision,
+// we don't inadvertently change the CollisionProtection value, which would cause the patch to fail since
+// CollisionProtection is an immutable field.
+func preserveCollisionProtection(desired, current *ocv1.ClusterExtensionRevision) {
+	// Build a map of objects in the current revision by their identity (GVK + namespace + name)
+	currentObjects := make(map[string]ocv1.CollisionProtection)
+	for _, phase := range current.Spec.Phases {
+		for _, obj := range phase.Objects {
+			key := objectKey(&obj.Object)
+			currentObjects[key] = obj.CollisionProtection
+		}
+	}
+
+	// Update desired revision objects to use the same CollisionProtection as current revision
+	for i := range desired.Spec.Phases {
+		for j := range desired.Spec.Phases[i].Objects {
+			desiredObj := &desired.Spec.Phases[i].Objects[j]
+			key := objectKey(&desiredObj.Object)
+			if collisionProtection, found := currentObjects[key]; found {
+				desiredObj.CollisionProtection = collisionProtection
+			}
+		}
+	}
+}
+
+// objectKey generates a unique key for an object based on its GVK, namespace, and name
+func objectKey(obj *unstructured.Unstructured) string {
+	gvk := obj.GroupVersionKind()
+	return fmt.Sprintf("%s/%s/%s/%s/%s", gvk.Group, gvk.Version, gvk.Kind, obj.GetNamespace(), obj.GetName())
+}
+
 // setPreviousRevisions populates spec.previous of latestRevision, trimming the list of previous _archived_ revisions down to
 // ClusterExtensionRevisionPreviousLimit or to the first _active_ revision and deletes trimmed revisions from the cluster.
 // NOTE: revisionList must be sorted in chronographical order, from oldest to latest.

internal/operator-controller/applier/phase.go

@@ -1,6 +1,8 @@
 package applier
 
 import (
+	"slices"
+
 	"k8s.io/apimachinery/pkg/runtime/schema"
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
@@ -111,6 +113,57 @@ func init() {
 	}
 }
 
+// Sort objects within the phase deterministically by Group, Version, Kind, Namespace, Name
+// to ensure consistent ordering regardless of input order. This is critical for
+// Helm-to-Boxcutter migration where the same resources may come from different sources
+// (Helm release manifest vs bundle manifest) and need to produce identical phases.
+func compareClusterExtensionRevision(a, b ocv1.ClusterExtensionRevisionObject) int {
+	aGVK := a.Object.GroupVersionKind()
+	bGVK := b.Object.GroupVersionKind()
+
+	// Compare Group
+	if aGVK.Group < bGVK.Group {
+		return -1
+	} else if aGVK.Group > bGVK.Group {
+		return 1
+	}
+
+	// Compare Version
+	if aGVK.Version < bGVK.Version {
+		return -1
+	} else if aGVK.Version > bGVK.Version {
+		return 1
+	}
+
+	// Compare Kind
+	if aGVK.Kind < bGVK.Kind {
+		return -1
+	} else if aGVK.Kind > bGVK.Kind {
+		return 1
+	}
+
+	// Compare Namespace
+	aNs := a.Object.GetNamespace()
+	bNs := b.Object.GetNamespace()
+	if aNs < bNs {
+		return -1
+	} else if aNs > bNs {
+		return 1
+	}
+
+	// Compare Name
+	aName := a.Object.GetName()
+	bName := b.Object.GetName()
+	if aName < bName {
+		return -1
+	}
+	if aName > bName {
+		return 1
+	}
+
+	return 0
+}
+
 // PhaseSort takes an unsorted list of objects and organizes them into sorted phases.
 // Each phase will be applied in order according to DefaultPhaseOrder. Objects
 // within a single phase are applied simultaneously.
@@ -125,6 +178,9 @@ func PhaseSort(unsortedObjs []ocv1.ClusterExtensionRevisionObject) []ocv1.Cluste
 
 	for _, phaseName := range defaultPhaseOrder {
 		if objs, ok := phaseMap[phaseName]; ok {
+			// Sort objects within the phase deterministically
+			slices.SortFunc(objs, compareClusterExtensionRevision)
+
 			phasesSorted = append(phasesSorted, ocv1.ClusterExtensionRevisionPhase{
 				Name:    string(phaseName),
 				Objects: objs,

internal/operator-controller/applier/phase_test.go

@@ -262,16 +262,16 @@ func Test_PhaseSort(t *testing.T) {
 						{
 							Object: unstructured.Unstructured{
 								Object: map[string]interface{}{
-									"apiVersion": "apps/v1",
-									"kind":       "Deployment",
+									"apiVersion": "v1",
+									"kind":       "ConfigMap",
 								},
 							},
 						},
 						{
 							Object: unstructured.Unstructured{
 								Object: map[string]interface{}{
-									"apiVersion": "v1",
-									"kind":       "ConfigMap",
+									"apiVersion": "apps/v1",
+									"kind":       "Deployment",
 								},
 							},
 						},