🐛 OCPBUGS-78455: fix(Boxcutter-Collision): Fix collision detection bypass after ClusterExtension upgrade by camilamacedo86 · Pull Request #2578 · operator-framework/operator-controller

camilamacedo86 · 2026-03-20T09:22:50Z

When a ClusterExtension was upgraded (e.g., v1.0.0 to v1.0.1), the upgrade set a higher revision number on managed objects. A second

ClusterExtension installing the same package was then allowed because boxcutter's revision linearity check returned ActionProgressed instead of ActionCollision, silently skipping the conflict.

Fix: after boxcutter reconcile, check ActionProgressed results for objects whose controller ownerReference belongs to a revision from a different ClusterExtension, and treat those as collisions.

Motivated by: https://redhat.atlassian.net/browse/OCPBUGS-78455

netlify · 2026-03-20T09:22:55Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`2b87748`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/69bd56217e058f0008779e3c
😎 Deploy Preview	https://deploy-preview-2578--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

openshift-ci · 2026-03-20T09:22:56Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign grokspawn for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot

Pull request overview

Fixes a Boxcutter collision-detection bypass that could allow a second ClusterExtension to install the same package after the first ClusterExtension upgraded and bumped managed-object revision numbers.

Changes:

Treat ActionProgressed results as collisions when the reconciled object is controller-owned by a different ClusterExtensionRevision (foreign ownerRef).
Add an E2E scenario covering “upgrade then duplicate install” and a new step to ensure both ClusterExtension resources are cleaned up.
Add unit tests validating the new “foreign revision ownerRef” collision behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
`internal/operator-controller/controllers/clusterextensionrevision_controller.go`	Adds foreign-ownerRef collision detection for `ActionProgressed` objects and tracks sibling CER names during reconcile.
`internal/operator-controller/controllers/clusterextensionrevision_controller_test.go`	Adds unit coverage for foreign/sibling/non-CER controller ownerRef cases.
`test/e2e/steps/steps.go`	Adds a step to track the currently-applied `ClusterExtension` for cleanup when a scenario applies a second CE.
`test/e2e/features/update.feature`	Adds an E2E scenario asserting collisions are detected after an upgrade when installing a duplicate CE.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/operator-controller/controllers/clusterextensionrevision_controller.go

codecov · 2026-03-20T10:00:06Z

Codecov Report

❌ Patch coverage is 91.66667% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.89%. Comparing base (0db26d7) to head (2b87748).

Files with missing lines	Patch %	Lines
...controllers/clusterextensionrevision_controller.go	91.66%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2578      +/-   ##
==========================================
+ Coverage   67.74%   67.89%   +0.15%     
==========================================
  Files         137      137              
  Lines        9560     9586      +26     
==========================================
+ Hits         6476     6508      +32     
+ Misses       2585     2581       -4     
+ Partials      499      497       -2

Flag	Coverage Δ
e2e	`38.04% <0.00%> (-0.12%)`	⬇️
experimental-e2e	`51.24% <80.55%> (+0.17%)`	⬆️
unit	`52.95% <86.11%> (+0.14%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

grokspawn · 2026-03-20T13:56:20Z

internal/operator-controller/controllers/clusterextensionrevision_controller.go

+// nil when the controller is a sibling revision (same ClusterExtension) or not a CER at all.
+func getForeignRevisionController(obj metav1.Object, siblingRevisionNames sets.Set[string]) *metav1.OwnerReference {
+	for i := range obj.GetOwnerReferences() {
+		ref := obj.GetOwnerReferences()[i]


why are we avoiding setting this to a local and iterating over it? If we're concerned that the contents can change over the course of this call then couldn't we end up in a situation where the first call determines a bounds which exceeds the returned results later in the loop?
Like if L551 says i := 12, but then obj.GetOwnerReferences() is updated to hold fewer than 12 items.

Why not

refs := obj.GetOwnerReferences() for i := range refs { ref := refs[i]

I think we can. Goog Catcher 🥇

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/operator-controller/controllers/clusterextensionrevision_controller_test.go

…rExtension upgrade When a ClusterExtension was upgraded (e.g., v1.0.0 to v1.0.1), the upgrade set a higher revision number on managed objects. A second ClusterExtension installing the same package was then allowed because boxcutter's revision linearity check returned ActionProgressed instead of ActionCollision, silently skipping the conflict. Fix: after boxcutter reconcile, check ActionProgressed results for objects whose controller ownerReference belongs to a revision from a different ClusterExtension, and treat those as collisions. Generated-by: Claude/Cursor

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-20T14:32:32Z

internal/operator-controller/controllers/clusterextensionrevision_controller.go

 			}
+			if ores.Action() == machinery.ActionProgressed {
+				if controllerRef := getForeignRevisionController(ores.Object(), siblingRevisionNames); controllerRef != nil {
+					collidingObjs = append(collidingObjs, ores.String()+fmt.Sprintf("Conflicting Owner: %s\n", controllerRef.String()))


The collision message is built by concatenating ores.String() directly with "Conflicting Owner: ..." without any separator. Unless ores.String() always ends with a newline, this will produce a hard-to-read merged string. Consider adding an explicit delimiter (e.g., "\n" or " ") between the two parts and avoid embedding an extra trailing newline in the appended fragment, since the final message already uses strings.Join(..., "\n\n").

Suggested change

collidingObjs = append(collidingObjs, ores.String()+fmt.Sprintf("Conflicting Owner: %s\n", controllerRef.String()))

collidingObjs = append(collidingObjs, ores.String()+fmt.Sprintf("\nConflicting Owner: %s", controllerRef.String()))

Copilot · 2026-03-20T14:32:32Z

internal/operator-controller/controllers/clusterextensionrevision_controller.go

+		ref := refs[i]
+		if ref.Controller != nil && *ref.Controller &&
+			ref.Kind == ocv1.ClusterExtensionRevisionKind &&
+			ref.APIVersion == ocv1.GroupVersion.String() {
+			if !siblingRevisionNames.Has(ref.Name) {
+				return &ref


getForeignRevisionController returns &ref where ref is a copy of the slice element. This forces an escape allocation and can be confusing to readers expecting a pointer into the original ownerReferences slice. Returning &refs[i] (or copying the fields you need into a new struct explicitly) would be clearer and avoid the extra allocation.

Suggested change

ref := refs[i]

if ref.Controller != nil && *ref.Controller &&

ref.Kind == ocv1.ClusterExtensionRevisionKind &&

ref.APIVersion == ocv1.GroupVersion.String() {

if !siblingRevisionNames.Has(ref.Name) {

return &ref

if refs[i].Controller != nil && *refs[i].Controller &&

refs[i].Kind == ocv1.ClusterExtensionRevisionKind &&

refs[i].APIVersion == ocv1.GroupVersion.String() {

if !siblingRevisionNames.Has(refs[i].Name) {

return &refs[i]

Copilot · 2026-03-20T14:32:33Z

internal/operator-controller/controllers/clusterextensionrevision_controller_test.go

+
+				rev := &ocv1.ClusterExtensionRevision{}
+				require.NoError(t, testClient.Get(t.Context(), client.ObjectKey{Name: tc.reconcilingRevisionName}, rev))
+				cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.TypeProgressing)


This test looks up the ClusterExtensionRevision Progressing condition using ocv1.TypeProgressing (the generic/common constant). Using the CER-specific constant ocv1.ClusterExtensionRevisionTypeProgressing would make the intent clearer and would catch any future divergence between common and CER condition-type constants.

Suggested change

cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.TypeProgressing)

cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing)

Copilot AI review requested due to automatic review settings March 20, 2026 09:22

openshift-ci bot requested review from OchiengEd and pedjak March 20, 2026 09:22

camilamacedo86 changed the title ~~🐛 fix(Boxcutter-Collision): Fix collision detection bypass after Cluste…~~ 🐛 fix(Boxcutter-Collision): Fix collision detection bypass after ClusterExtension upgrade Mar 20, 2026

Copilot started reviewing on behalf of camilamacedo86 March 20, 2026 09:23 View session

camilamacedo86 requested a review from perdasilva March 20, 2026 09:25

Copilot AI reviewed Mar 20, 2026

View reviewed changes

internal/operator-controller/controllers/clusterextensionrevision_controller.go Show resolved Hide resolved

internal/operator-controller/controllers/clusterextensionrevision_controller.go Outdated Show resolved Hide resolved

camilamacedo86 force-pushed the fix-issu-install-package-upgrade-same-box branch from a73cf6d to 8b504c8 Compare March 20, 2026 09:39

grokspawn reviewed Mar 20, 2026

View reviewed changes

camilamacedo86 force-pushed the fix-issu-install-package-upgrade-same-box branch from 8b504c8 to 36f661c Compare March 20, 2026 14:00

Copilot AI review requested due to automatic review settings March 20, 2026 14:00

Copilot started reviewing on behalf of camilamacedo86 March 20, 2026 14:00 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

internal/operator-controller/controllers/clusterextensionrevision_controller_test.go Outdated Show resolved Hide resolved

camilamacedo86 force-pushed the fix-issu-install-package-upgrade-same-box branch from 36f661c to 2b87748 Compare March 20, 2026 14:13

camilamacedo86 requested review from Copilot and grokspawn March 20, 2026 14:28

Copilot started reviewing on behalf of camilamacedo86 March 20, 2026 14:28 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

camilamacedo86 changed the title ~~🐛 fix(Boxcutter-Collision): Fix collision detection bypass after ClusterExtension upgrade~~ 🐛 OCPBUGS-78455: fix(Boxcutter-Collision): Fix collision detection bypass after ClusterExtension upgrade Mar 20, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🐛 OCPBUGS-78455: fix(Boxcutter-Collision): Fix collision detection bypass after ClusterExtension upgrade#2578

🐛 OCPBUGS-78455: fix(Boxcutter-Collision): Fix collision detection bypass after ClusterExtension upgrade#2578
camilamacedo86 wants to merge 1 commit intooperator-framework:mainfrom
camilamacedo86:fix-issu-install-package-upgrade-same-box

camilamacedo86 commented Mar 20, 2026 •

edited

Loading

Uh oh!

netlify bot commented Mar 20, 2026 •

edited

Loading

Uh oh!

openshift-ci bot commented Mar 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Mar 20, 2026 •

edited

Loading

Uh oh!

grokspawn Mar 20, 2026

Uh oh!

camilamacedo86 Mar 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	collidingObjs = append(collidingObjs, ores.String()+fmt.Sprintf("Conflicting Owner: %s\n", controllerRef.String()))
	collidingObjs = append(collidingObjs, ores.String()+fmt.Sprintf("\nConflicting Owner: %s", controllerRef.String()))

	cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.TypeProgressing)
	cond := meta.FindStatusCondition(rev.Status.Conditions, ocv1.ClusterExtensionRevisionTypeProgressing)

Conversation

camilamacedo86 commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify bot commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

openshift-ci bot commented Mar 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

grokspawn Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

camilamacedo86 Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

camilamacedo86 commented Mar 20, 2026 •

edited

Loading

netlify bot commented Mar 20, 2026 •

edited

Loading

codecov bot commented Mar 20, 2026 •

edited

Loading