fix: adopt migrated ClusterObjectSets

Signed-off-by: Todd Short <tshort@redhat.com>

Author: tmshort

internal/operator-controller/applier/boxcutter.go

@@ -478,6 +478,17 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
 		return true, "", nil
 	}
 
+	// The migration tool creates a successful revision 1 before it creates the
+	// ClusterExtension. Do not render that same bundle into a second revision:
+	// claim the pre-existing revision and use it as the initial installed state.
+	// Comparing the resolved bundle metadata keeps normal upgrades intact.
+	if revision := migratedSubscriptionRevision(existingRevisions, revisionAnnotations); revision != nil {
+		if err := bc.adoptRevision(ctx, ext, revision); err != nil {
+			return false, "", fmt.Errorf("adopting migrated revision %s: %w", revision.Name, err)
+		}
+		return true, "", nil
+	}
+
 	// Generate desired revision
 	desiredRevision, err := bc.RevisionGenerator.GenerateRevision(ctx, contentFS, ext, objectLabels, revisionAnnotations)
 	if err != nil {
@@ -574,6 +585,46 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
 	return true, "", nil
 }
 
+func migratedSubscriptionRevision(revisions []ocv1.ClusterObjectSet, desiredAnnotations map[string]string) *ocv1.ClusterObjectSet {
+	for i := range revisions {
+		revision := &revisions[i]
+		if revision.Spec.Revision != 1 ||
+			revision.Annotations[labels.MigratedFromSubscriptionKey] == "" ||
+			!meta.IsStatusConditionTrue(revision.Status.Conditions, ocv1.ClusterObjectSetTypeSucceeded) {
+			continue
+		}
+
+		matchesBundle := true
+		for _, key := range []string{labels.PackageNameKey, labels.BundleNameKey, labels.BundleVersionKey, labels.BundleReferenceKey, labels.BundleReleaseKey} {
+			if revision.Annotations[key] != desiredAnnotations[key] {
+				matchesBundle = false
+				break
+			}
+		}
+		if matchesBundle {
+			return revision
+		}
+	}
+	return nil
+}
+
+func (bc *Boxcutter) adoptRevision(ctx context.Context, ext *ocv1.ClusterExtension, revision *ocv1.ClusterObjectSet) error {
+	for _, ref := range revision.OwnerReferences {
+		if ref.Controller != nil && *ref.Controller && ref.UID != ext.UID {
+			return fmt.Errorf("revision is already controlled by %s %q", ref.Kind, ref.Name)
+		}
+	}
+
+	for _, ref := range revision.OwnerReferences {
+		if ref.Controller != nil && *ref.Controller && ref.UID == ext.UID {
+			return nil
+		}
+	}
+
+	revision.OwnerReferences = append(revision.OwnerReferences, *metav1.NewControllerRef(ext, ocv1.SchemeGroupVersion.WithKind(ocv1.ClusterExtensionKind)))
+	return bc.Client.Update(ctx, revision)
+}
+
 // createExternalizedRevision creates a new COS with all objects externalized to Secrets.
 // It follows a crash-safe three-step sequence: create Secrets, create COS, patch ownerRefs.
 func (bc *Boxcutter) createExternalizedRevision(ctx context.Context, ext *ocv1.ClusterExtension, desiredRevision *ocv1ac.ClusterObjectSetApplyConfiguration, existingRevisions []ocv1.ClusterObjectSet) error {

internal/operator-controller/applier/boxcutter_test.go

@@ -666,6 +666,42 @@ func TestBoxcutter_Apply(t *testing.T) {
 				assert.Equal(t, "test-ext-1", revList.Items[0].Name)
 			},
 		},
+		{
+			name: "adopts matching migration revision without creating a duplicate",
+			mockBuilder: func(t *testing.T) applier.ClusterObjectSetGenerator {
+				ctrl := gomock.NewController(t)
+				return mockapplier.NewMockClusterObjectSetGenerator(ctrl)
+			},
+			existingObjs: []client.Object{
+				&ocv1.ClusterObjectSet{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:   "test-ext-1",
+						Labels: map[string]string{labels.OwnerNameKey: ext.Name},
+						Annotations: map[string]string{
+							labels.MigratedFromSubscriptionKey: "test-namespace/test-subscription",
+							labels.PackageNameKey:              "test-package",
+							labels.BundleNameKey:               "test-package.v1.0.0",
+							labels.BundleVersionKey:            "1.0.0",
+							labels.BundleReferenceKey:          "registry.example/test-package@sha256:123",
+						},
+					},
+					Spec: ocv1.ClusterObjectSetSpec{Revision: 1},
+					Status: ocv1.ClusterObjectSetStatus{Conditions: []metav1.Condition{{
+						Type:   ocv1.ClusterObjectSetTypeSucceeded,
+						Status: metav1.ConditionTrue,
+					}}},
+				},
+			},
+			validate: func(t *testing.T, c client.Client) {
+				revList := &ocv1.ClusterObjectSetList{}
+				require.NoError(t, c.List(t.Context(), revList, client.MatchingLabels{labels.OwnerNameKey: ext.Name}))
+				require.Len(t, revList.Items, 1)
+				assert.Equal(t, "test-ext-1", revList.Items[0].Name)
+				require.Len(t, revList.Items[0].OwnerReferences, 1)
+				assert.Equal(t, ext.Name, revList.Items[0].OwnerReferences[0].Name)
+				assert.Equal(t, ext.UID, revList.Items[0].OwnerReferences[0].UID)
+			},
+		},
 		{
 			name: "new revision created when objects in new revision are different",
 			mockBuilder: func(t *testing.T) applier.ClusterObjectSetGenerator {
@@ -1087,6 +1123,13 @@ func TestBoxcutter_Apply(t *testing.T) {
 					labels.BundleVersionKey: "1.0.1",
 					labels.PackageNameKey:   "test-package",
 				}
+			} else if tc.name == "adopts matching migration revision without creating a duplicate" {
+				revisionAnnotations = map[string]string{
+					labels.PackageNameKey:     "test-package",
+					labels.BundleNameKey:      "test-package.v1.0.0",
+					labels.BundleVersionKey:   "1.0.0",
+					labels.BundleReferenceKey: "registry.example/test-package@sha256:123",
+				}
 			}
 			completed, status, err := boxcutter.Apply(t.Context(), testFS, ext, nil, revisionAnnotations)
 

internal/operator-controller/labels/labels.go

@@ -51,4 +51,10 @@ const (
 	// that were created during migration from Helm releases. This label is used
 	// to distinguish migrated revisions from those created by normal Boxcutter operation.
 	MigratedFromHelmKey = "olm.operatorframework.io/migrated-from-helm"
+
+	// MigratedFromSubscriptionKey is the annotation placed on a revision created
+	// by the OLM v0-to-v1 migration tool. A matching revision 1 is adopted by a
+	// subsequently-created ClusterExtension instead of being replaced by its
+	// initial Boxcutter reconciliation.
+	MigratedFromSubscriptionKey = "olm.operatorframework.io/migrated-from-subscription"
 )