:seedling: Add e2e regression test for cross-CE collision protection by perdasilva · Pull Request #2783 · operator-framework/operator-controller

perdasilva · 2026-06-25T10:06:39Z

Summary

Add e2e regression test verifying that a second ClusterExtension targeting an already-installed bundle cannot take over resources from the original owner, even with a higher-revision ClusterObjectSet. The single merged scenario covers both initial collision detection after an upgrade and collision persistence across revisions.
Use ${NAME} / ${NAME}-dup naming convention in the collision scenario so CE names stay abstract. Stop overwriting sc.clusterExtensionName in ResourceIsApplied so ${NAME} remains constant throughout a scenario — the overwrite was always a no-op for existing tests since they all use name: ${NAME}.
Auto-track every applied ClusterExtension for cleanup via addedResources in ResourceIsApplied, eliminating the manual TrackCurrentClusterExtensionForCleanup step and the redundant sc.clusterExtensionName cleanup in ScenarioCleanup.
Add named CE condition steps (NamedClusterExtensionReportsCondition, NamedClusterExtensionReportsConditionWithMessageFragment) so step descriptions explicitly identify which CE is being asserted.
Add ClusterExtensionOwnsClusterObjectSets step to verify the number of ClusterObjectSets owned by a named CE.
Extract messageFragmentComparison helper to deduplicate the fragment-matching closure across condition steps.
Preserve unresolved template variables as literal ${KEY} instead of silently substituting empty string.

Note: ClusterObjectSets still use the legacy sc.clusterObjectSetName cleanup path rather than addedResources tracking. The same consolidation can be done in a follow-up.

Test plan

go vet passes
New scenario passes against a kind cluster with BoxcutterRuntime and DeploymentConfig enabled
CI e2e suite passes

🤖 Generated with Claude Code

netlify · 2026-06-25T10:06:45Z

✅ Deploy Preview for olmv1 ready!

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

openshift-ci · 2026-06-25T10:06:45Z

[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

Adds an end-to-end regression scenario to ensure cross-ClusterExtension “collision protection” works (a second ClusterExtension targeting an already-installed bundle cannot take over resources), and extends the godog step library to make multi-CE assertions readable and robust.

Changes:

Adds a new e2e scenario covering duplicate ClusterExtension installs and verifies original ownership is retained.
Extends step definitions with named ClusterExtension condition assertions, a ClusterObjectSet ownership-count assertion, and a ${PREV_NAME} template variable to reference a previously-applied CE.
Refactors message-fragment matching into a shared helper and changes template expansion to preserve unknown ${KEY} variables literally.

Reviewed changes

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

File	Description
test/e2e/steps/steps.go	Adds named CE steps, COS ownership-count step, `${PREV_NAME}` support, message-fragment helper, and preserves unresolved template variables.
test/e2e/steps/hooks.go	Extends scenario context to track `previousClusterExtensionName`.
test/e2e/features/update.feature	Adds a new regression scenario validating cross-CE collision protection behavior.

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

codecov · 2026-06-25T10:36:35Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.44%. Comparing base (cf0266c) to head (430c5d6).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2783      +/-   ##
==========================================
+ Coverage   70.42%   70.44%   +0.01%     
==========================================
  Files         143      143              
  Lines       10625    10625              
==========================================
+ Hits         7483     7485       +2     
+ Misses       2580     2579       -1     
+ Partials      562      561       -1

Flag	Coverage Δ
e2e	`35.11% <ø> (-0.03%)`	⬇️
experimental-e2e	`52.46% <ø> (+0.17%)`	⬆️
unit	`59.50% <ø> (-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.

Copilot

Pull request overview

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

Copilot

Pull request overview

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

pedjak · 2026-06-29T10:49:09Z

      kind: ClusterExtension
      metadata:
-        name: ${NAME}
+        name: ce-orig-${SCENARIO_ID}


I would prefer that we keep using ${NAME} - it is a very handy way to avoid leaking how we actually craft it under the hood.

But once you have 2 resources, how do you disambiguate?

But once you have 2 resources, how do you disambiguate?

By name, we should track inside ScenarioContext resource kinds and their names

The problem is we track the CE last CE name as a way to drive unnamed steps, e.g. ClusterExtension is Available. Maybe we could remove the unnamed steps, keep NAME static for the execution and let templating produce the CE name. This would require we always provide the CE name in those steps. I'm not entirely against that - I don't think it hurts readability (just makes things more precise, and anyway, if the idea if to in some sense reproduce what the user would do, every kubectl command would take the CE name anyway). Wdyt?

I'd probably do this in a separate PR and rebase this one.

The problem is we track the CE last CE name as a way to drive unnamed steps, e.g. ClusterExtension is Available

such steps should work as-is if there is just single CE in the context, then it fully clear which one we are talking about.

True, but then order becomes a dependency and the mechanics become obscure for the developer. They need to dig in to the code to understand why they can't reference a previous CE in a particular step. What do we lose by adding the additional clarity in each step?

It doesn't impact readability and is inline with user behavior w.r.t input

It reduces state tracking as much as a possible - simplifying each step (reducing complexity)

Not even the extra key strokes are a concern anymore since it will all probably be generated XD

pedjak · 2026-06-29T10:50:07Z

      kind: ClusterExtension
      metadata:
-        name: ${NAME}-dup
+        name: ce-dup-${SCENARIO_ID}


same here, what is an advantage of changing it to ce-dup-${SCENARIO_ID} ?

pedjak · 2026-06-29T11:06:02Z

            version: 1.0.1
      """
-    Then ClusterExtension reports Progressing as True with Reason Retrying and Message includes:
+    Then ClusterExtension "ce-dup-${SCENARIO_ID}" reports Progressing as True with Reason Retrying and Message includes:


Suggested change

Then ClusterExtension "ce-dup-${SCENARIO_ID}" reports Progressing as True with Reason Retrying and Message includes:

Then ClusterExtension "${NAME}-dup" reports Progressing as True with Reason Retrying and Message includes:

pedjak · 2026-06-29T11:07:00Z

+
+  @BoxcutterRuntime
+  @DeploymentConfig
+  Scenario: Cannot install a ClusterExtension that refers to an already installed bundle


given that we have the previous test, do we really need this one at all? What is the added value?

I think the main differentiation is that in this one we ensure that when the conflicting CE has a higher revision COS it doesn't take over the resources (the regression). I think it would be ok to just merge these two since the first just checks same version collisions.

Copilot

Pull request overview

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

- Add e2e test verifying that a second ClusterExtension targeting an already-installed bundle cannot take over resources from the original owner, even with a higher-revision ClusterObjectSet. The single merged scenario covers both initial collision detection after an upgrade and collision persistence across revisions. - Use ${NAME} / ${NAME}-dup naming convention in the collision scenario so CE names stay abstract. Stop overwriting sc.clusterExtensionName in ResourceIsApplied so ${NAME} remains constant throughout a scenario — the overwrite was always a no-op for existing tests since they all use name: ${NAME}. - Auto-track every applied ClusterExtension for cleanup via addedResources in ResourceIsApplied, eliminating the manual TrackCurrentClusterExtensionForCleanup step and the redundant sc.clusterExtensionName cleanup in ScenarioCleanup. - Add named CE condition steps (NamedClusterExtensionReportsCondition, NamedClusterExtensionReportsConditionWithMessageFragment) so step descriptions explicitly identify which CE is being asserted. - Add ClusterExtensionOwnsClusterObjectSets step to verify the number of ClusterObjectSets owned by a named CE. - Extract messageFragmentComparison helper to deduplicate the fragment-matching closure across condition steps. - Preserve unresolved template variables as literal ${KEY} instead of silently substituting empty string. 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 3 out of 3 changed files in this pull request and generated no new comments.

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

openshift-ci Bot requested review from oceanc80 and tmshort June 25, 2026 10:06

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

perdasilva mentioned this pull request Jun 25, 2026

🌱 Upgrade boxcutter to v0.14.0 and adapt sibling owners API #2764

Merged

4 tasks

Copilot AI reviewed Jun 25, 2026

View reviewed changes

Comment thread test/e2e/steps/steps.go

perdasilva force-pushed the e2e-collision-regression-test branch from 943273a to a29cdef Compare June 25, 2026 10:14

pedjak reviewed Jun 26, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings June 26, 2026 14:58

perdasilva force-pushed the e2e-collision-regression-test branch from a29cdef to f79695c Compare June 26, 2026 14:58

Copilot started reviewing on behalf of perdasilva June 26, 2026 14:58 View session

Copilot AI reviewed Jun 26, 2026

View reviewed changes

Comment thread test/e2e/steps/steps.go

perdasilva force-pushed the e2e-collision-regression-test branch from f79695c to b48aef1 Compare June 29, 2026 08:10

Copilot AI review requested due to automatic review settings June 29, 2026 08:14

perdasilva force-pushed the e2e-collision-regression-test branch from b48aef1 to cdaa226 Compare June 29, 2026 08:14

Copilot started reviewing on behalf of perdasilva June 29, 2026 08:15 View session

Copilot AI reviewed Jun 29, 2026

View reviewed changes

Comment thread test/e2e/steps/steps.go Outdated

perdasilva force-pushed the e2e-collision-regression-test branch from cdaa226 to b0c5c59 Compare June 29, 2026 08:40

pedjak reviewed Jun 29, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings June 29, 2026 15:13

perdasilva force-pushed the e2e-collision-regression-test branch from b0c5c59 to 9797be3 Compare June 29, 2026 15:13

Copilot started reviewing on behalf of perdasilva June 29, 2026 15:13 View session

Copilot AI reviewed Jun 29, 2026

View reviewed changes

perdasilva force-pushed the e2e-collision-regression-test branch from 9797be3 to d1a9612 Compare June 29, 2026 15:40

perdasilva force-pushed the e2e-collision-regression-test branch from d1a9612 to 430c5d6 Compare June 30, 2026 07:39

Copilot AI review requested due to automatic review settings June 30, 2026 07:39

Copilot started reviewing on behalf of perdasilva June 30, 2026 07:39 View session

Copilot AI reviewed Jun 30, 2026

View reviewed changes

	Then ClusterExtension "ce-dup-${SCENARIO_ID}" reports Progressing as True with Reason Retrying and Message includes:
	Then ClusterExtension "${NAME}-dup" reports Progressing as True with Reason Retrying and Message includes:

Uh oh!

Conversation

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

Uh oh!

Summary

Test plan

Uh oh!

netlify Bot commented Jun 25, 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 Jun 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

perdasilva commented Jun 25, 2026 •

edited

Loading

netlify Bot commented Jun 25, 2026 •

edited

Loading

codecov Bot commented Jun 25, 2026 •

edited

Loading

perdasilva Jun 29, 2026 •

edited

Loading