✨ Adopt ClusterObjectDeployment only when not controlled by another owner

internal/operator-controller/applier/boxcutter.go

@@ -465,6 +465,16 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
 		}
 	}
 
+	// Refuse to adopt a ClusterObjectDeployment that is already controlled by a
+	// different owner. We adopt one that has no controller (stamping our controller
+	// ownerReference via the apply below) or one we already control, but taking over a
+	// deployment controlled by someone else would fight that owner. Returning an
+	// ordinary error lets the ClusterExtension report Progressing=True/Retrying and
+	// use the normal controller retry backoff.
+	if err := bc.checkAdoptable(ctx, ext); err != nil {
+		return false, "", err
+	}
+
 	// Generate the desired ClusterObjectDeployment and the Secrets its template refs.
 	cod, secrets, err := bc.RevisionGenerator.GenerateClusterObjectDeployment(ctx, contentFS, ext, objectLabels, revisionAnnotations)
 	if err != nil {
@@ -522,6 +532,34 @@ func (bc *Boxcutter) Apply(ctx context.Context, contentFS fs.FS, ext *ocv1.Clust
 	return true, "", nil
 }
 
+// checkAdoptable verifies that the applier may own the ClusterObjectDeployment named
+// after the ClusterExtension. It is adoptable when it does not yet exist, has no
+// controller ownerReference, is already controlled by this ClusterExtension, or is
+// controlled by a reference that matches this ClusterExtension by group, kind, and name
+// but with a different UID — the latter indicates the ClusterExtension was deleted and
+// recreated (for example a backup/restore), which comes back with a fresh UID, so the
+// deployment is logically still ours. When it is controlled by a genuinely different
+// owner, it returns an error rather than fighting that owner for control.
+func (bc *Boxcutter) checkAdoptable(ctx context.Context, ext *ocv1.ClusterExtension) error {
+	existing := &ocv1.ClusterObjectDeployment{}
+	switch err := bc.Client.Get(ctx, client.ObjectKey{Name: ext.Name}, existing); {
+	case apierrors.IsNotFound(err):
+		return nil
+	case err != nil:
+		return fmt.Errorf("getting ClusterObjectDeployment %q: %w", ext.Name, err)
+	}
+
+	controller := metav1.GetControllerOf(existing)
+	if controller == nil || controller.UID == ext.UID {
+		return nil
+	}
+
+	return fmt.Errorf(
+		"ClusterObjectDeployment %q is already controlled by %s %q; refusing to adopt it",
+		ext.Name, controller.Kind, controller.Name,
+	)
+}
+
 // runPreflights runs the configured preflight checks for a desired apply. It determines whether
 // this is an install or an upgrade from whether the ClusterObjectDeployment already exists (a
 // direct, controller-owned signal, rather than our own ClusterExtension status which may be stale

internal/operator-controller/applier/boxcutter_test.go

@@ -18,6 +18,7 @@ import (
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 	ocv1ac "github.com/operator-framework/operator-controller/applyconfigurations/api/v1"
@@ -869,6 +870,100 @@ func TestBoxcutterApply_NilContentUsesExistingCOD(t *testing.T) {
 	})
 }
 
+// TestBoxcutterApply_Adoption verifies the applier only adopts an existing
+// ClusterObjectDeployment when it is not already controlled by a different owner.
+func TestBoxcutterApply_Adoption(t *testing.T) {
+	type testCase struct {
+		name                   string
+		ext                    *ocv1.ClusterExtension
+		cod                    *ocv1.ClusterObjectDeployment
+		assert                 func(t *testing.T, cl client.Client, applyCompleted bool, applyStatus string, applyErr error)
+		wantExtControllerOwner bool
+	}
+
+	for _, tc := range []testCase{
+		{
+			name: "adopts an existing deployment with no controller owner reference",
+			ext:  &ocv1.ClusterExtension{ObjectMeta: metav1.ObjectMeta{Name: "test-ext", UID: "test-uid"}},
+			cod: &ocv1.ClusterObjectDeployment{
+				ObjectMeta: metav1.ObjectMeta{Name: "test-ext"},
+			},
+			assert: func(t *testing.T, _ client.Client, applyCompleted bool, applyStatus string, applyErr error) {
+				require.NoError(t, applyErr)
+				require.True(t, applyCompleted)
+				require.Empty(t, applyStatus)
+			},
+			wantExtControllerOwner: true,
+		},
+		{
+			name: "proceeds when the existing deployment is already controlled by this extension",
+			ext:  &ocv1.ClusterExtension{ObjectMeta: metav1.ObjectMeta{Name: "test-ext", UID: "test-uid"}},
+			cod: &ocv1.ClusterObjectDeployment{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            "test-ext",
+					OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(&ocv1.ClusterExtension{ObjectMeta: metav1.ObjectMeta{Name: "test-ext", UID: "test-uid"}}, ocv1.GroupVersion.WithKind(ocv1.ClusterExtensionKind))},
+				},
+			},
+			assert: func(t *testing.T, _ client.Client, applyCompleted bool, applyStatus string, applyErr error) {
+				require.NoError(t, applyErr)
+				require.True(t, applyCompleted)
+				require.Empty(t, applyStatus)
+			},
+			wantExtControllerOwner: true,
+		},
+		{
+			name: "refuses to adopt a deployment controlled by a different owner",
+			ext:  &ocv1.ClusterExtension{ObjectMeta: metav1.ObjectMeta{Name: "test-ext", UID: "test-uid"}},
+			cod: &ocv1.ClusterObjectDeployment{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:            "test-ext",
+					OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(&ocv1.ClusterExtension{ObjectMeta: metav1.ObjectMeta{Name: "other-ext", UID: "other-uid"}}, ocv1.GroupVersion.WithKind(ocv1.ClusterExtensionKind))},
+				},
+			},
+			assert: func(t *testing.T, cl client.Client, applyCompleted bool, applyStatus string, applyErr error) {
+				require.Error(t, applyErr)
+				assert.False(t, applyCompleted)
+				assert.Empty(t, applyStatus)
+
+				t.Log("by surfacing a non-terminal error so retries occur without CE changes")
+				require.NotErrorIs(t, applyErr, reconcile.TerminalError(nil), "error was terminal, but expected non-terminal error so that CE retries occur")
+
+				t.Log("by naming the deployment and the conflicting owner in the message")
+				assert.Contains(t, applyErr.Error(), "test-ext")
+				assert.Contains(t, applyErr.Error(), "other-ext")
+
+				secretList := &corev1.SecretList{}
+				require.NoError(t, cl.List(t.Context(), secretList, client.InNamespace("olmv1-system")))
+				assert.Empty(t, secretList.Items)
+			},
+			wantExtControllerOwner: false,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			scheme := codTestScheme(t)
+			cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(tc.cod).Build()
+			bc := newCODBoxcutter(t, scheme, cl, []client.Object{
+				&corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "test-cm", Namespace: "test-ns"}},
+			})
+
+			actualCompleted, actualStatus, actualErr := bc.Apply(t.Context(), dummyBundle, tc.ext, map[string]string{"some": "label"}, map[string]string{})
+			tc.assert(t, cl, actualCompleted, actualStatus, actualErr)
+
+			cod := &ocv1.ClusterObjectDeployment{}
+			require.NoError(t, cl.Get(t.Context(), client.ObjectKey{Name: tc.ext.Name}, cod))
+			owner := metav1.GetControllerOf(cod)
+			if tc.wantExtControllerOwner {
+				require.NotNil(t, owner)
+				assert.Equal(t, tc.ext.Name, owner.Name)
+				assert.Equal(t, tc.ext.UID, owner.UID)
+				assert.Equal(t, ocv1.ClusterExtensionKind, owner.Kind)
+			} else {
+				require.Equal(t, metav1.GetControllerOf(tc.cod), owner)
+			}
+		})
+	}
+}
+
 // TestBoxcutterApply_Preflights verifies which preflight hook runs for each rollout state
 // and that a preflight failure propagates out of Apply. Each row is a sequence of applies
 // against the same Boxcutter: a step renders its own objects, so identical content between

test/e2e/features/cod_adoption_conflict.feature

@@ -0,0 +1,55 @@
+@BoxcutterRuntime
+Feature: ClusterExtension refuses to adopt a ClusterObjectDeployment owned by another resource
+
+  As an OLM user, I want a ClusterExtension to report and retry an adoption conflict,
+  so that it does not take over a ClusterObjectDeployment controlled by another owner
+  and can proceed after that deployment is removed.
+
+  Background:
+    Given OLM is available
+    And an image registry is available
+    And namespace "${TEST_NAMESPACE}" is available
+    And a catalog "test" with packages:
+      | package | version | channel | replaces | contents                   |
+      | test    | 1.2.0   | beta    |          | CRD, Deployment, ConfigMap |
+
+  Scenario: ClusterExtension retries after a foreign-owned deployment is removed
+    Given ClusterObjectDeployment with Namespace owner "${TEST_NAMESPACE}" is applied
+      """
+      apiVersion: olm.operatorframework.io/v1
+      kind: ClusterObjectDeployment
+      metadata:
+        name: ${NAME}
+      spec:
+        template:
+          spec:
+            collisionProtection: Prevent
+            phases:
+            - name: resources
+              objects: []
+      """
+    When ClusterExtension is applied
+      """
+      apiVersion: olm.operatorframework.io/v1
+      kind: ClusterExtension
+      metadata:
+        name: ${NAME}
+      spec:
+        namespace: ${TEST_NAMESPACE}
+        source:
+          sourceType: Catalog
+          catalog:
+            packageName: ${PACKAGE:test}
+            version: 1.2.0
+            selector:
+              matchLabels:
+                "olm.operatorframework.io/metadata.name": ${CATALOG:test}
+      """
+    Then ClusterExtension reports Progressing as True with Reason Retrying and Message includes:
+      """
+      ClusterObjectDeployment "${NAME}" is already controlled by Namespace "${TEST_NAMESPACE}"
+      """
+    When resource "clusterobjectdeployment/${NAME}" is removed
+    Then ClusterExtension is rolled out
+    And ClusterExtension is available
+    And ClusterObjectDeployment "${NAME}" is owned by ClusterExtension "${NAME}"

test/e2e/steps/group_steps.go

@@ -17,9 +17,9 @@ import (
 	ocv1 "github.com/operator-framework/operator-controller/api/v1"
 )
 
-// An empty owner name creates an ownerless revision. Resolve a real cluster-scoped
-// owner before creation so garbage collection and reconciliation see valid metadata.
-func ClusterObjectSetWithNamespaceOwnerApplied(ctx context.Context, ownerName string, template *godog.DocString) error {
+// ResourceWithNamespaceOwnerApplied resolves a real cluster-scoped Namespace owner
+// before applying the resource so garbage collection sees valid owner metadata.
+func ResourceWithNamespaceOwnerApplied(ctx context.Context, ownerName string, template *godog.DocString) error {
 	sc := scenarioCtx(ctx)
 	obj, err := toUnstructured(substituteScenarioVars(template.Content, sc))
 	if err != nil {

test/e2e/steps/steps.go

@@ -134,7 +134,8 @@ func RegisterSteps(sc *godog.ScenarioContext) {
 	sc.Step(`^(?i)ClusterObjectDeployment "([^"]+)" consistently reports ([[:alnum:]]+) as ([[:alnum:]]+) with Reason ([[:alnum:]]+) for "([^"]+)"$`, ClusterObjectDeploymentConsistentlyReportsCondition)
 	sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) transition between (\d+) and (\d+) minutes since ClusterObjectSet "([^"]+)" was created$`, ClusterExtensionReportsConditionTransitionTime)
 	sc.Step(`^(?i)ClusterObjectSet is applied(?:\s+.*)?$`, ResourceIsApplied)
-	sc.Step(`^(?i)ClusterObjectSet with Namespace owner "([^"]*)" is applied$`, ClusterObjectSetWithNamespaceOwnerApplied)
+	sc.Step(`^(?i)ClusterObjectSet with Namespace owner "([^"]*)" is applied$`, ResourceWithNamespaceOwnerApplied)
+	sc.Step(`^(?i)ClusterObjectDeployment with Namespace owner "([^"]*)" is applied$`, ResourceWithNamespaceOwnerApplied)
 	sc.Step(`^(?i)ClusterObjectSet "([^"]+)" reconciliation is triggered$`, TriggerClusterObjectSetReconciliation)
 	sc.Step(`^(?i)ClusterObjectSet "([^"]+)" has observed phase "([^"]+)" with a non-empty digest$`, ClusterObjectSetHasObservedPhase)
 	sc.Step(`^(?i)ClusterObjectSet "([^"]+)" is archived$`, ClusterObjectSetIsArchived)