:seedling: Upgrade boxcutter to v0.14.0 and adapt sibling owners API by perdasilva · Pull Request #2764 · operator-framework/operator-controller

perdasilva · 2026-06-12T11:41:36Z

Description

Adapt the ClusterObjectSet controller to boxcutter v0.14.0's API:

WithPreviousOwners is replaced by WithSiblingOwners, which takes all active sibling revisions (both lower and higher) so boxcutter can distinguish siblings from true collisions during handover.
NewObjectEngine now requires a managedBy parameter.

Changes

listSiblingRevisions — returns all active revisions for the same ClusterExtension, excluding self, archived, and deleting revisions. Used by buildBoxcutterPhases with WithSiblingOwners.
listPreviousRevisions — returns only lower-revision active siblings. Used for archiving after rollout completes.
listOtherActiveRevisions — shared helper with predicate-based filtering used by both methods above.
buildBoxcutterPhases — uses WithSiblingOwners instead of WithPreviousOwners.
NewObjectEngine — passes FieldOwnerPrefix as the new managedBy parameter.
Dependency bumps — boxcutter v0.13.1 → v0.14.0, plus transitive updates.
Unit tests — separate table-driven tests for listSiblingRevisions and listPreviousRevisions with shared callRevisionLister helper.

The cross-CE collision regression e2e test has been split out into #2783.

Reviewer Checklist

API Go Documentation
Tests: Unit Tests (and E2E Tests, if appropriate)
Comprehensive Commit Messages
Links to related GitHub Issue(s)

🤖 Generated with Claude Code

netlify · 2026-06-12T11:41:41Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`cc13c78`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/6a3d0dfcc47c2b000808f444
😎 Deploy Preview	https://deploy-preview-2764--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

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

Copilot

Pull request overview

This PR updates the operator-controller’s Boxcutter integration to treat all active sibling ClusterObjectSet revisions (both lower and higher revision numbers) as relevant “owners”, aiming to avoid false-positive collision reporting during revision handover. It also adds coverage to validate collision behavior when a conflicting ClusterExtension is upgraded.

Changes:

Switch Boxcutter ownership inputs from “previous revisions only” to “all active sibling revisions” and add unit tests for the sibling listing logic.
Add an e2e scenario asserting collisions persist even when a conflicting ClusterExtension upgrades to a different version.
Update Go module dependencies for Boxcutter (and related transitive deps).

Reviewed changes

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

Show a summary per file

File	Description
`test/e2e/features/update.feature`	Adds an e2e scenario for collision persistence across upgrades (but needs step/param fixes).
`internal/operator-controller/controllers/revision_engine_factory.go`	Adjusts Boxcutter object engine factory invocation to match updated dependency API.
`internal/operator-controller/controllers/clusterobjectset_controller.go`	Introduces sibling revision listing and feeds sibling owners into Boxcutter.
`internal/operator-controller/controllers/clusterobjectset_controller_internal_test.go`	Adds unit tests for `listSiblingRevisions`.
`go.mod`	Bumps Boxcutter + deps, but currently includes merge-conflict markers and an invalid local replace.
`go.sum`	Updates dependency sums, but currently includes multiple unresolved merge-conflict blocks.

Comments suppressed due to low confidence (2)

go.mod:7

go.mod still contains unresolved merge-conflict markers (<<<<<<< / ======= / >>>>>>>). This makes the module file invalid and will break go tooling in CI. Resolve the conflict by choosing a single go directive line and removing all conflict markers, then re-run go mod tidy to ensure go.sum is consistent.

go 1.26.3

require (
	github.com/BurntSushi/toml v1.6.0
	github.com/Masterminds/semver/v3 v3.5.0

go.mod:329

This replace points to a developer-local filesystem path, which will not exist in CI and will break module resolution for anyone else. Repository go.mod files should not contain machine-specific local replaces; rely on the tagged boxcutter version instead.

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

Copilot

Pull request overview

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

codecov · 2026-06-12T12:47:02Z

Codecov Report

❌ Patch coverage is 94.73684% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 70.42%. Comparing base (7d6a00d) to head (cc13c78).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
...troller/controllers/clusterobjectset_controller.go	93.75%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2764   +/-   ##
=======================================
  Coverage   70.42%   70.42%           
=======================================
  Files         143      143           
  Lines       10617    10625    +8     
=======================================
+ Hits         7477     7483    +6     
- Misses       2579     2580    +1     
- Partials      561      562    +1

Flag	Coverage Δ
e2e	`35.26% <0.00%> (+0.13%)`	⬆️
experimental-e2e	`52.46% <94.73%> (+0.09%)`	⬆️
unit	`59.51% <78.94%> (+0.01%)`	⬆️

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

☔ View full report in Codecov by Harness.
📢 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.

pedjak

question (non-blocking): The Copilot review on an earlier push mentioned an e2e .feature file change for collision persistence across upgrades, but it's absent from the current diff. Is that planned as a follow-up PR? The unit tests cover listSiblingRevisions well, but an e2e test exercising the higher-sibling handover path would add confidence that the behavioral change works end-to-end.

Copilot

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.

pedjak

Review focused on boxcutter library update and coding style.

Copilot

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.

pedjak · 2026-06-24T11:36:00Z

+
+  @BoxcutterRuntime
+  @DeploymentConfig
+  Scenario: A conflicting ClusterExtension with a higher revision does not take over resources from the original owner


I find the scenario title to describe a low-level implementation details - we should focus ourselves here on user visible change, something like "Cannot install further ClusterExtension referring already installed bundle".

Given that we are adding this test in the PR which is about upgrading boxcutter version, I am curious to understand if before this update we did not have such protection? If we did, then I would add this tests in a separate PR. Does boxcutter upgrade gives us this functionality automatically or we need to so something on our side to achieve it?

Renamed to "Cannot install a ClusterExtension that refers to an already installed bundle".

On the scope question: cross-CE collision protection is existing boxcutter behavior — it was there before this PR. This PR changes the sibling owners API from WithSiblingOwners (all active revisions from the same CE) to WithPreviousOwners (only lower-revision ones). That's a same-CE handover concern, not cross-CE. The e2e test was added as a regression test to confirm that cross-CE collision protection still works correctly after the API change, but it's not testing new functionality. Happy to split it out into a separate PR if you prefer.

Happy to split it out into a separate PR if you prefer.

It would make sense to me... and to merge that first, before upgrading the boxcutter.

moving the e2e into another PR

Copilot

Pull request overview

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

Copilot

Pull request overview

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

Copilot

Pull request overview

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

Adapt the ClusterObjectSet controller to boxcutter v0.14.0's API: - WithPreviousOwners is replaced by WithSiblingOwners, which takes all active sibling revisions (both lower and higher) so boxcutter can distinguish siblings from true collisions during handover. - NewObjectEngine now requires a managedBy parameter. Changes: - listSiblingRevisions: returns all active revisions for the same ClusterExtension, excluding self, archived, and deleting revisions. - listPreviousRevisions: returns only lower-revision active siblings, used for archiving after rollout completes. - listOtherActiveRevisions: shared helper with predicate-based filtering used by both methods above. - buildBoxcutterPhases: uses WithSiblingOwners instead of WithPreviousOwners. - NewObjectEngine: passes FieldOwnerPrefix as the new managedBy param. - Unit tests: separate table-driven tests for listSiblingRevisions and listPreviousRevisions with shared callRevisionLister helper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Per G. da Silva <pegoncal@redhat.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 5 changed files in this pull request and generated no new comments.

pedjak

/lgtm

tmshort · 2026-06-26T15:43:45Z

/approve

openshift-ci · 2026-06-26T15:44:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rashmigottipati, tmshort

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [tmshort]

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

Copilot AI review requested due to automatic review settings June 12, 2026 11:41

openshift-ci Bot requested review from rashmigottipati and tmshort June 12, 2026 11:41

Copilot started reviewing on behalf of perdasilva June 12, 2026 11:42 View session

perdasilva force-pushed the update-boxcutter-sibling-owners branch 2 times, most recently from ddb7a76 to fe0acea Compare June 12, 2026 11:43

Copilot AI reviewed Jun 12, 2026

View reviewed changes

perdasilva force-pushed the update-boxcutter-sibling-owners branch from fe0acea to ceb6fee Compare June 12, 2026 11:52

Copilot AI review requested due to automatic review settings June 12, 2026 11:52

Copilot started reviewing on behalf of perdasilva June 12, 2026 11:52 View session

Copilot AI reviewed Jun 12, 2026

View reviewed changes

Comment thread internal/operator-controller/controllers/revision_engine_factory.go

Comment thread go.mod Outdated

perdasilva force-pushed the update-boxcutter-sibling-owners branch 3 times, most recently from d81b767 to 507f415 Compare June 12, 2026 12:23

perdasilva changed the title ~~🌱 Update boxcutter sibling owners~~ 🌱 Update boxcutter integration for sibling owners API Jun 12, 2026

pedjak reviewed Jun 12, 2026

View reviewed changes

Comment thread internal/operator-controller/controllers/clusterobjectset_controller.go

Copilot AI review requested due to automatic review settings June 15, 2026 07:33

perdasilva force-pushed the update-boxcutter-sibling-owners branch from 507f415 to 1dea3d1 Compare June 15, 2026 07:33

Copilot started reviewing on behalf of perdasilva June 15, 2026 07:34 View session

Copilot AI reviewed Jun 15, 2026

View reviewed changes

pedjak reviewed Jun 16, 2026

View reviewed changes

Comment thread internal/operator-controller/controllers/clusterobjectset_controller.go

Comment thread internal/operator-controller/controllers/clusterobjectset_controller.go

pedjak reviewed Jun 16, 2026

View reviewed changes

Comment thread internal/operator-controller/controllers/clusterobjectset_controller_internal_test.go

perdasilva force-pushed the update-boxcutter-sibling-owners branch 2 times, most recently from 1c88b5a to c490544 Compare June 17, 2026 07:31

Copilot AI review requested due to automatic review settings June 18, 2026 07:24

perdasilva force-pushed the update-boxcutter-sibling-owners branch from c490544 to c3a323b Compare June 18, 2026 07:24

Copilot started reviewing on behalf of perdasilva June 18, 2026 07:25 View session

Copilot AI reviewed Jun 18, 2026

View reviewed changes

pedjak reviewed Jun 24, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings June 24, 2026 11:52

Copilot started reviewing on behalf of perdasilva June 24, 2026 11:52 View session

perdasilva force-pushed the update-boxcutter-sibling-owners branch from e9e567f to a2d7a10 Compare June 24, 2026 11:55

Copilot AI reviewed Jun 24, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings June 24, 2026 11:57

Copilot started reviewing on behalf of perdasilva June 24, 2026 11:57 View session

perdasilva changed the title ~~🌱 Update boxcutter integration for sibling owners API~~ 🌱 Update boxcutter integration and add cross-CE collision regression test Jun 24, 2026

Copilot AI reviewed Jun 24, 2026

View reviewed changes

Comment thread internal/operator-controller/controllers/clusterobjectset_controller_internal_test.go Outdated

perdasilva changed the title ~~🌱 Update boxcutter integration and add cross-CE collision regression test~~ 🌱 Upgrade boxcutter to v0.14.0 and adapt sibling owners API Jun 25, 2026

Copilot AI review requested due to automatic review settings June 25, 2026 10:09

perdasilva force-pushed the update-boxcutter-sibling-owners branch from 73cfe87 to 6dca2bb Compare June 25, 2026 10:09

Copilot started reviewing on behalf of perdasilva June 25, 2026 10:09 View session

Copilot AI reviewed Jun 25, 2026

View reviewed changes

Comment thread internal/operator-controller/controllers/clusterobjectset_controller.go

Comment thread internal/operator-controller/controllers/clusterobjectset_controller_internal_test.go Outdated

perdasilva mentioned this pull request Jun 25, 2026

Clean up collision error format and provide context specifc error in the case of duplicate package installation #2784

Open

perdasilva force-pushed the update-boxcutter-sibling-owners branch from 6dca2bb to cc46694 Compare June 25, 2026 10:37

perdasilva force-pushed the update-boxcutter-sibling-owners branch from cc46694 to cc13c78 Compare June 25, 2026 11:16

Copilot AI review requested due to automatic review settings June 25, 2026 11:16

Copilot started reviewing on behalf of perdasilva June 25, 2026 11:16 View session

Copilot AI reviewed Jun 25, 2026

View reviewed changes

pedjak reviewed Jun 25, 2026

View reviewed changes

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Jun 25, 2026

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 26, 2026

rashmigottipati approved these changes Jun 26, 2026

View reviewed changes

openshift-ci Bot assigned rashmigottipati Jun 26, 2026

openshift-merge-bot Bot merged commit cf0266c into operator-framework:main Jun 26, 2026
26 checks passed

coderabbitai Bot mentioned this pull request Sep 16, 2026

test: cover migration rejection paths operator-framework/library-olm#34

Closed

Conversation

perdasilva commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Reviewer Checklist

Uh oh!

netlify Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

pedjak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

pedjak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

pedjak Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

perdasilva Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

pedjak Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

perdasilva Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

perdasilva Jun 25, 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!

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

perdasilva commented Jun 12, 2026 •

edited

Loading

netlify Bot commented Jun 12, 2026 •

edited

Loading

codecov Bot commented Jun 12, 2026 •

edited

Loading