✨ Apply a ClusterObjectDeployment instead of managing ClusterObjectSets

cmd/operator-controller/main.go

@@ -618,6 +618,7 @@ func (c *boxcutterReconcilerConfigurator) Configure(ceReconciler *controllers.Cl
 	rg := &applier.SimpleRevisionGenerator{
 		Scheme:           c.mgr.GetScheme(),
 		ManifestProvider: c.regv1ManifestProvider,
+		SystemNamespace:  cfg.systemNamespace,
 	}
 	fieldOwner := fmt.Sprintf("%s/clusterextension-controller", fieldOwnerPrefix)
 	appl := &applier.Boxcutter{

internal/object-controller/controllers/clusterobjectdeployment_controller.go

@@ -38,6 +38,17 @@ import (
 // defensively for callers (e.g. tests) that bypass API-server defaulting.
 const defaultRevisionHistoryLimit = 5
 
+// bundleAnnotationKeys lists the bundle-metadata annotation keys that are
+// propagated from a ClusterObjectDeployment onto each ClusterObjectSet it creates.
+var bundleAnnotationKeys = []string{
+	labels.PackageNameKey,
+	labels.BundleNameKey,
+	labels.BundleVersionKey,
+	labels.BundleReleaseKey,
+	labels.BundleReferenceKey,
+	labels.OLMPropertiesKey,
+}
+
 // ClusterObjectDeploymentReconciler reconciles ClusterObjectDeployment objects by
 // managing their owned ClusterObjectSet revisions.
 type ClusterObjectDeploymentReconciler struct {
@@ -103,7 +114,7 @@ func (c *ClusterObjectDeploymentReconciler) reconcile(ctx context.Context, cod *
 // changed (or no active revision exists) and reflects the latest revision's
 // rollout progress in the deployment's Available and Progressing conditions.
 func (c *ClusterObjectDeploymentReconciler) reconcileRollout(ctx context.Context, cod *ocv1.ClusterObjectDeployment, revisions []ocv1.ClusterObjectSet) error {
-	hash := templateHash(cod.Spec.Template)
+	hash := TemplateHash(cod.Spec.Template)
 
 	setAvailable(cod, revisions)
 
@@ -126,6 +137,13 @@ func (c *ClusterObjectDeploymentReconciler) reconcileRollout(ctx context.Context
 		return nil
 	}
 
+	// Adopt the latest revision if it's not already owned by this deployment.
+	// This handles the case where a ClusterObjectSet with the correct template hash
+	// exists but doesn't have an owner reference to this ClusterObjectDeployment.
+	if err := c.ensureRevisionOwnership(ctx, cod, latest); err != nil {
+		return err
+	}
+
 	// A revision that has exceeded its progress deadline reports Ready=False with
 	// reason ProgressDeadlineExceeded and never records completedAt. Surface that
 	// terminal rollout state on the deployment's Progressing condition so it is
@@ -225,6 +243,47 @@ func archivedTeardownComplete(rev *ocv1.ClusterObjectSet) bool {
 	return cond != nil && cond.Reason == ocv1.ClusterObjectSetReasonArchived
 }
 
+// ensureRevisionOwnership adopts an orphan revision (one with no controller owner)
+// by setting this deployment as its controller. It is a no-op if the revision is
+// already owned by this deployment, or if it is already controlled by a different
+// controller — a foreign-controlled revision sharing this group is tolerated, not
+// stolen and not treated as an error.
+func (c *ClusterObjectDeploymentReconciler) ensureRevisionOwnership(ctx context.Context, cod *ocv1.ClusterObjectDeployment, rev *ocv1.ClusterObjectSet) error {
+	// Defensive guard: only adopt revisions that belong to this deployment's group.
+	// The group is the deployment name, and siblinghood is defined by spec.group.
+	if rev.Spec.Group != cod.Name {
+		return fmt.Errorf("cannot adopt revision %q: spec.group %q does not match deployment name %q", rev.Name, rev.Spec.Group, cod.Name)
+	}
+
+	// Inspect any existing controller owner reference.
+	for _, ref := range rev.OwnerReferences {
+		if ref.Controller != nil && *ref.Controller {
+			if ref.UID == cod.UID {
+				// Already owned by this deployment.
+				return nil
+			}
+			// Owned by a different controller. Siblinghood is defined by spec.group,
+			// so a foreign-controlled revision may legitimately share this group; we
+			// do not steal it, and we do not treat it as an error (that would wedge
+			// reconciliation). Leave it untouched.
+			return nil
+		}
+	}
+
+	// No controller owner: adopt this orphan revision.
+	updated := rev.DeepCopy()
+	if err := controllerutil.SetControllerReference(cod, updated, c.Scheme); err != nil {
+		return fmt.Errorf("adopting orphan revision %q: %w", rev.Name, err)
+	}
+
+	// Update the revision
+	if err := c.Client.Update(ctx, updated); err != nil {
+		return fmt.Errorf("adopting revision %q: %w", rev.Name, err)
+	}
+
+	return nil
+}
+
 // createRevision builds and creates the next ClusterObjectSet revision owned by
 // the deployment. Any create error, including AlreadyExists, is returned.
 //
@@ -322,13 +381,13 @@ func mapClusterObjectSetToClusterObjectDeployment(_ context.Context, obj client.
 	return []ctrl.Request{{NamespacedName: types.NamespacedName{Name: cos.Spec.Group}}}
 }
 
-// templateHash returns a stable digest of the deployment template. Any change to
+// TemplateHash returns a stable digest of the deployment template. Any change to
 // the template (including changes to fields that are mutable on a ClusterObjectSet)
 // produces a different digest, which is how the controller decides a new revision
 // is required. The shared DeepHashObject helper base36-encodes a sha224 of the
 // template's JSON, which is label-safe and short enough to use as a label value
 // without truncation.
-func templateHash(template ocv1.ClusterObjectDeploymentTemplate) string {
+func TemplateHash(template ocv1.ClusterObjectDeploymentTemplate) string {
 	return hashutil.DeepHashObject(template)
 }
 
@@ -439,11 +498,25 @@ func buildClusterObjectSet(cod *ocv1.ClusterObjectDeployment, revision int64, ha
 	}
 	revLabels[labels.TemplateHashKey] = hash
 
+	revAnnotations := maps.Clone(cod.Spec.Template.Metadata.Annotations)
+	if revAnnotations == nil {
+		revAnnotations = map[string]string{}
+	}
+	// Also copy bundle-metadata annotations from the deployment's own annotations so the
+	// ClusterExtension status reader (which reads these annotations off the COS) keeps
+	// working during the refactor to make operator-controller write CODs instead of
+	// managing COSes directly.
+	for _, key := range bundleAnnotationKeys {
+		if value, ok := cod.Annotations[key]; ok {
+			revAnnotations[key] = value
+		}
+	}
+
 	return &ocv1.ClusterObjectSet{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        fmt.Sprintf("%s-%d", cod.Name, revision),
 			Labels:      revLabels,
-			Annotations: maps.Clone(cod.Spec.Template.Metadata.Annotations),
+			Annotations: revAnnotations,
 		},
 		Spec: ocv1.ClusterObjectSetSpec{
 			Group:                   cod.Name,

internal/object-controller/controllers/clusterobjectdeployment_controller_internal_test.go

@@ -11,12 +11,16 @@ import (
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	"github.com/operator-framework/operator-controller/internal/shared/labels"
+	"github.com/operator-framework/operator-controller/internal/testutil"
 )
 
 func testTemplate() ocv1.ClusterObjectDeploymentTemplate {
@@ -42,26 +46,26 @@ func testTemplate() ocv1.ClusterObjectDeploymentTemplate {
 func Test_templateHash_isStableAndSensitive(t *testing.T) {
 	tmpl := testTemplate()
 
-	h1 := templateHash(tmpl)
+	h1 := TemplateHash(tmpl)
 	require.NotEmpty(t, h1)
 	require.LessOrEqual(t, len(h1), 63, "digest must fit within the label-value length limit")
 	require.Regexp(t, "^[0-9a-z]+$", h1, "digest must be label-value safe")
 
-	h2 := templateHash(tmpl)
+	h2 := TemplateHash(tmpl)
 	require.Equal(t, h1, h2, "same template must hash the same")
 
 	// A change to a phase object changes the hash.
 	changed := testTemplate()
 	changed.Spec.Phases[0].Objects[0].Object.Object["data"] = map[string]interface{}{"k": "v"}
-	require.NotEqual(t, h1, templateHash(changed))
+	require.NotEqual(t, h1, TemplateHash(changed))
 
 	// A change to ONLY progressionProbes (a mutable COS field) still changes the hash.
 	probesChanged := testTemplate()
 	probesChanged.Spec.ProgressionProbes = []ocv1.ProgressionProbe{{
 		Selector:   ocv1.ObjectSelector{Type: ocv1.SelectorTypeGroupKind, GroupKind: metav1.GroupKind{Kind: "ConfigMap"}},
 		Assertions: []ocv1.Assertion{{Type: ocv1.ProbeTypeFieldValue}},
 	}}
-	require.NotEqual(t, h1, templateHash(probesChanged), "a progressionProbes-only change must create a new revision")
+	require.NotEqual(t, h1, TemplateHash(probesChanged), "a progressionProbes-only change must create a new revision")
 }
 
 func Test_nextRevisionNumber(t *testing.T) {
@@ -212,3 +216,95 @@ func Test_mapClusterObjectSetToClusterObjectDeployment(t *testing.T) {
 		})
 	}
 }
+
+func Test_ensureRevisionOwnership(t *testing.T) {
+	scheme := runtime.NewScheme()
+	require.NoError(t, ocv1.AddToScheme(scheme))
+
+	cod := &ocv1.ClusterObjectDeployment{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-cod",
+			UID:  "cod-uid",
+		},
+		Spec: ocv1.ClusterObjectDeploymentSpec{Template: testTemplate()},
+	}
+
+	newClient := func(t *testing.T, objs ...client.Object) client.Client {
+		t.Helper()
+		return testutil.WithIndexes(t, fake.NewClientBuilder().WithScheme(scheme), SetupIndexes).
+			WithStatusSubresource(&ocv1.ClusterObjectDeployment{}, &ocv1.ClusterObjectSet{}).
+			WithObjects(objs...).
+			Build()
+	}
+
+	t.Run("rejects revision with mismatched group", func(t *testing.T) {
+		// A revision from a different group must not be adopted.
+		rev := &ocv1.ClusterObjectSet{
+			ObjectMeta: metav1.ObjectMeta{Name: "other-group-1"},
+			Spec:       ocv1.ClusterObjectSetSpec{Group: "other-group", Revision: 1},
+		}
+		c := newClient(t, cod, rev)
+		reconciler := &ClusterObjectDeploymentReconciler{Client: c, Scheme: scheme}
+
+		err := reconciler.ensureRevisionOwnership(t.Context(), cod, rev)
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "cannot adopt revision")
+		require.Contains(t, err.Error(), "spec.group \"other-group\" does not match deployment name \"test-cod\"")
+
+		// Verify the revision was NOT adopted (no owner reference added)
+		updated := &ocv1.ClusterObjectSet{}
+		require.NoError(t, c.Get(t.Context(), client.ObjectKeyFromObject(rev), updated))
+		require.Empty(t, updated.OwnerReferences)
+	})
+
+	t.Run("adopts revision with matching group", func(t *testing.T) {
+		// A revision in the same group can be adopted.
+		rev := &ocv1.ClusterObjectSet{
+			ObjectMeta: metav1.ObjectMeta{Name: "test-cod-1"},
+			Spec:       ocv1.ClusterObjectSetSpec{Group: "test-cod", Revision: 1},
+		}
+		c := newClient(t, cod, rev)
+		reconciler := &ClusterObjectDeploymentReconciler{Client: c, Scheme: scheme}
+
+		err := reconciler.ensureRevisionOwnership(t.Context(), cod, rev)
+		require.NoError(t, err)
+
+		// Verify the revision was adopted (owner reference set)
+		updated := &ocv1.ClusterObjectSet{}
+		require.NoError(t, c.Get(t.Context(), client.ObjectKeyFromObject(rev), updated))
+		require.Len(t, updated.OwnerReferences, 1)
+		require.Equal(t, cod.Name, updated.OwnerReferences[0].Name)
+		require.Equal(t, cod.UID, updated.OwnerReferences[0].UID)
+		require.Equal(t, ptr.To(true), updated.OwnerReferences[0].Controller)
+	})
+
+	t.Run("no-op when already owned", func(t *testing.T) {
+		// A revision already owned by the deployment should not be updated.
+		rev := &ocv1.ClusterObjectSet{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: "test-cod-1",
+				OwnerReferences: []metav1.OwnerReference{
+					{
+						APIVersion: ocv1.GroupVersion.String(),
+						Kind:       "ClusterObjectDeployment",
+						Name:       cod.Name,
+						UID:        cod.UID,
+						Controller: ptr.To(true),
+					},
+				},
+			},
+			Spec: ocv1.ClusterObjectSetSpec{Group: "test-cod", Revision: 1},
+		}
+		c := newClient(t, cod, rev)
+		reconciler := &ClusterObjectDeploymentReconciler{Client: c, Scheme: scheme}
+
+		err := reconciler.ensureRevisionOwnership(t.Context(), cod, rev)
+		require.NoError(t, err)
+
+		// Verify the revision still has the same owner reference (no update)
+		updated := &ocv1.ClusterObjectSet{}
+		require.NoError(t, c.Get(t.Context(), client.ObjectKeyFromObject(rev), updated))
+		require.Len(t, updated.OwnerReferences, 1)
+		require.Equal(t, cod.UID, updated.OwnerReferences[0].UID)
+	})
+}

internal/object-controller/controllers/clusterobjectdeployment_controller_test.go

@@ -831,3 +831,36 @@ func Test_ClusterObjectDeploymentReconciler_RevisionBounds(t *testing.T) {
 		})
 	}
 }
+
+func Test_ClusterObjectDeploymentReconciler_PropagatesBundleAnnotations(t *testing.T) {
+	cod := codFixture()
+	cod.Annotations = map[string]string{
+		labels.PackageNameKey:     "test-package",
+		labels.BundleNameKey:      "test-bundle",
+		labels.BundleVersionKey:   "1.0.0+2",
+		labels.BundleReferenceKey: "registry.example.com/bundle@sha256:abcdef",
+		labels.BundleReleaseKey:   "2",
+		labels.OLMPropertiesKey:   `[{"type":"olm.package","value":{"packageName":"test-package","version":"1.0.0"}}]`,
+		"example.com/non-bundle":  "should-not-be-copied",
+	}
+
+	c := newCODClient(t, cod)
+
+	_, err := reconcileCOD(t, c)
+	require.NoError(t, err)
+
+	cos, ok := getCOS(t, c, clusterObjectDeploymentName+"-1")
+	require.True(t, ok, "revision 1 should be created")
+
+	// Bundle-metadata annotations should be copied
+	require.Equal(t, "test-package", cos.Annotations[labels.PackageNameKey], "PackageNameKey should be copied")
+	require.Equal(t, "test-bundle", cos.Annotations[labels.BundleNameKey], "BundleNameKey should be copied")
+	require.Equal(t, "1.0.0+2", cos.Annotations[labels.BundleVersionKey], "BundleVersionKey should be copied")
+	require.Equal(t, "registry.example.com/bundle@sha256:abcdef", cos.Annotations[labels.BundleReferenceKey], "BundleReferenceKey should be copied")
+	require.Equal(t, "2", cos.Annotations[labels.BundleReleaseKey], "BundleReleaseKey should be copied")
+	require.JSONEq(t, `[{"type":"olm.package","value":{"packageName":"test-package","version":"1.0.0"}}]`, cos.Annotations[labels.OLMPropertiesKey], "OLMPropertiesKey should be copied")
+
+	// Non-bundle annotation should NOT be copied
+	_, exists := cos.Annotations["example.com/non-bundle"]
+	require.False(t, exists, "non-bundle annotations must not be copied from COD to COS")
+}