🐛 Restrict ClusterObjectSet retention to the current controller owner by fgiudici · Pull Request #3040 · operator-framework/operator-controller

fgiudici · 2026-10-20T16:38:30Z

Description

When ClusterObjectSet revisions from different controller owners share a spec.group, retention counts all of them and can delete archived revisions belonging to another owner. Filter revisions by the current ClusterExtension controller owner UID before calculating the retention allowance.

Keep group-based discovery, handoff, and revision numbering, along with the existing archived-only deletion policy. Keep the original retention cases in TestBoxcutter_Apply in boxcutter_test.go, correcting their controller owner references. Add three explicit cases in the same table for owner exclusion, pruning among mixed owners, and counting active and deleting revisions. Also assert handoff for a foreign-owned revision without an owner label.

Validation:

make lint passes with zero issues.
make test-unit passes.
make verify passes.
Applier tests pass.
Both explicit ownership regression cases fail against the parent implementation, confirming they catch the original retention bug.

Reviewer Checklist

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

Summary by CodeRabbit

Bug Fixes
- Revision retention is now scoped to the current extension owner.
- Revisions belonging to other owners are preserved during cleanup.
- Active and deleting revisions continue to count toward retention limits but are not removed.
- Applying unchanged content can transfer ownership of an existing revision to the current extension.

openshift-ci · 2026-10-20T16:38:36Z

[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 perdasilva 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

netlify · 2026-10-20T16:38:41Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`47535e4`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/6ad7a51d2bdabdb6385b9407
😎 Deploy Preview	https://deploy-preview-3040--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.

coderabbitai · 2026-10-20T16:38:45Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 8c8fc450-45e5-4837-824f-057033aefac4

📥 Commits

Reviewing files that changed from the base of the PR and between e2b424a and 9f202f1.

📒 Files selected for processing: 3

internal/operator-controller/applier/boxcutter.go
internal/operator-controller/applier/boxcutter_retention_test.go
internal/operator-controller/applier/boxcutter_test.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Walkthrough

Boxcutter revision garbage collection now counts only revisions controlled by the current extension owner toward the five-revision limit. Tests cover deletion eligibility, active and deleting revisions, and ownership handoff.

Changes: Revision retention

Layer / File(s)	Summary
Owner-scoped garbage collection `internal/operator-controller/applier/boxcutter.go`	Garbage collection receives the extension owner and applies the five-revision limit only to revisions controlled by that owner.
Retention and ownership tests `internal/operator-controller/applier/boxcutter_retention_test.go`, `internal/operator-controller/applier/boxcutter_test.go`	The new table-driven test covers revision eligibility, numbering, object contents, and completion status. Existing tests cover handoff of an unchanged revision from a foreign owner; prior cross-owner retention tests were removed.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: perdasilva

Merge Risk: ⚪ Minimal · up to 9f202

Revision cleanup now counts and prunes only the revisions owned by the current ClusterExtension. Revisions belonging to another owner are no longer deleted. Retention for owned revisions behaves as before. No concrete merge-blocking risk remains.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies the bug fix and the main change: limiting ClusterObjectSet retention to the current controller owner.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description explains the bug, intended behavior, scope, and validation results. It includes the required Description and Reviewer Checklist sections. It does not provide related issue links, and t…

Full details: Description check

Explanation

The description explains the bug, intended behavior, scope, and validation results. It includes the required Description and Reviewer Checklist sections. It does not provide related issue links, and the checklist items remain unchecked, but these omissions are non-critical.

Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

joelanford · 2026-10-20T16:47:43Z

@fgiudici , wdyt about holding off on this until we implement https://redhat.atlassian.net/browse/OPRUN-4747?

Group-based discovery can include revisions controlled by another owner. Filter revisions by controller owner UID before calculating the retention allowance so those revisions are neither counted nor deleted. Preserve group-based discovery, handoff, revision numbering, and the existing archived-only deletion policy. Keep the existing retention cases in the Apply test table with corrected controller owner references. Add explicit cases for owner identity, mixed revision ordering, and active and deleting revisions. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: Francesco Giudici <fgiudici@redhat.com>

fgiudici · 2026-10-20T17:40:08Z

@fgiudici , wdyt about holding off on this until we implement https://redhat.atlassian.net/browse/OPRUN-4747?

Yep, makes sense. I could probably just close this for now.

fgiudici · 2026-10-21T07:56:28Z

Added a comment to https://redhat.atlassian.net/browse/OPRUN-4747 to not forget.
Closing this PR.

openshift-ci Bot requested review from grokspawn and rashmigottipati October 20, 2026 16:38

fgiudici marked this pull request as draft October 20, 2026 16:40

openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 20, 2026

fgiudici force-pushed the owner-scoped-cos-retention branch from 9f202f1 to 47535e4 Compare October 20, 2026 17:30

fgiudici closed this Oct 21, 2026

Navigation Menu

Repository navigation

🐛 Restrict ClusterObjectSet retention to the current controller owner - #3040

🐛 Restrict ClusterObjectSet retention to the current controller owner#3040
fgiudici wants to merge 1 commit into
operator-framework:mainfrom
fgiudici:owner-scoped-cos-retention

fgiudici commented Oct 20, 2026 •

edited

Loading

Uh oh!

openshift-ci Bot commented Oct 20, 2026

Uh oh!

netlify Bot commented Oct 20, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 20, 2026 •

edited

Loading

Uh oh!

joelanford commented Oct 20, 2026

Uh oh!

fgiudici commented Oct 20, 2026

Uh oh!

fgiudici commented Oct 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

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

Uh oh!

Description

Reviewer Checklist

Summary by CodeRabbit

Uh oh!

openshift-ci Bot commented Oct 20, 2026

Uh oh!

netlify Bot commented Oct 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!

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

Uh oh!

Uh oh!

joelanford commented Oct 20, 2026

Uh oh!

fgiudici commented Oct 20, 2026

Uh oh!

fgiudici commented Oct 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fgiudici commented Oct 20, 2026 •

edited

Loading

netlify Bot commented Oct 20, 2026 •

edited

Loading

coderabbitai Bot commented Oct 20, 2026 •

edited

Loading