Skip to content

Conversation

@perdasilva
Copy link
Contributor

Description

The Helm applier currently adds the CSV annotations including the olm.properties annotation, which aggregates the properties present in the metadata/properties.yaml file in the bundle, to the Helm release annotations.

The olm.properties is especially important because it contains properties of interest for other operators on the cluster such as olm.maxOpenShiftVersion, which is used by OpenShift to detect whether there are any operators currently installed that might block an upgrade.

In this PR we add the "olm.properties" annotation to the ClusterExtensionRevision annotations. I've avoided adding all of the CSV annotations because it may contains many that are a bit confusing (e.g. createdAt and capabilities). This can be modified in the future after the appropriate discussions. But, because its so visible now (as opposed to hidden with the Helm release secret), it might be prudent to add only was is necessary for now.

We don't have an e2e test that checks the Helm release within the release secret for the appropriate annotations. So, I've added one only for the BoxcutterRuntime feature.

Reviewer Checklist

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

Copilot AI review requested due to automatic review settings January 30, 2026 16:04
@netlify
Copy link

netlify bot commented Jan 30, 2026

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit 651e1c8
🔍 Latest deploy log https://app.netlify.com/projects/olmv1/deploys/69808a597021ea0008b2c7ae
😎 Deploy Preview https://deploy-preview-2471--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds the olm.properties annotation from bundle metadata to ClusterExtensionRevision annotations. This annotation aggregates properties from the bundle's metadata/properties.yaml file and is important for cluster operators (e.g., OpenShift) to detect compatibility information like olm.maxOpenShiftVersion that might affect cluster upgrades. The implementation selectively adds only the olm.properties annotation to avoid cluttering the revision with potentially confusing CSV annotations.

Changes:

  • Added logic to extract and copy the olm.properties annotation from bundle CSV to ClusterExtensionRevision annotations
  • Introduced a new E2E test step to verify annotation presence on revisions
  • Added comprehensive unit tests for the annotation copying behavior

Reviewed changes

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

Show a summary per file
File Description
internal/operator-controller/rukpak/bundle/source/source.go Added constant for olm.properties key and nil check for CSV annotations
internal/operator-controller/applier/provider.go Added getBundleAnnotations helper function to extract CSV annotations from bundle filesystem
internal/operator-controller/applier/boxcutter.go Implemented logic to selectively copy olm.properties annotation to revision annotations during revision generation
internal/operator-controller/applier/boxcutter_test.go Added unit tests for bundle annotation copying, including edge cases for missing properties and CSV annotations
test/e2e/steps/steps.go Added E2E test step function to verify ClusterExtensionRevision annotations
test/e2e/features/install.feature Added E2E test scenario to validate olm.properties annotation appears on revisions

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

@perdasilva perdasilva force-pushed the revision-csv-annotations branch from 6657401 to 4070119 Compare January 30, 2026 16:17
Copy link
Contributor

@pedjak pedjak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 30, 2026
@tmshort
Copy link
Contributor

tmshort commented Jan 30, 2026

/approve

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 30, 2026
@tmshort
Copy link
Contributor

tmshort commented Jan 30, 2026

Unfortunately, the experimental-e2e test is failing (3 times out of 3).

Copilot AI review requested due to automatic review settings February 2, 2026 07:48
@perdasilva perdasilva force-pushed the revision-csv-annotations branch from 4070119 to 53472e8 Compare February 2, 2026 07:48
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Feb 2, 2026
@perdasilva perdasilva force-pushed the revision-csv-annotations branch from 53472e8 to 52db1fe Compare February 2, 2026 07:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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


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

@perdasilva perdasilva force-pushed the revision-csv-annotations branch from 52db1fe to d071ac4 Compare February 2, 2026 09:57
@codecov
Copy link

codecov bot commented Feb 2, 2026

Codecov Report

❌ Patch coverage is 77.77778% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.55%. Comparing base (2bdb702) to head (651e1c8).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/operator-controller/applier/boxcutter.go 80.00% 1 Missing and 1 partial ⚠️
...operator-controller/rukpak/bundle/source/source.go 33.33% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2471      +/-   ##
==========================================
+ Coverage   69.52%   69.55%   +0.03%     
==========================================
  Files         102      102              
  Lines        8339     8354      +15     
==========================================
+ Hits         5798     5811      +13     
- Misses       2078     2079       +1     
- Partials      463      464       +1     
Flag Coverage Δ
e2e 46.53% <5.55%> (-0.34%) ⬇️
experimental-e2e 13.20% <0.00%> (+0.02%) ⬆️
unit 57.48% <77.77%> (+0.04%) ⬆️

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.

Copilot AI review requested due to automatic review settings February 2, 2026 10:19
@perdasilva perdasilva force-pushed the revision-csv-annotations branch from d071ac4 to 150f12c Compare February 2, 2026 10:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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


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

@perdasilva perdasilva force-pushed the revision-csv-annotations branch from 150f12c to 681e933 Compare February 2, 2026 10:39
Copilot AI review requested due to automatic review settings February 2, 2026 11:17
@perdasilva perdasilva force-pushed the revision-csv-annotations branch from 681e933 to 1bbc64a Compare February 2, 2026 11:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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


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

@perdasilva perdasilva force-pushed the revision-csv-annotations branch from 1bbc64a to 651e1c8 Compare February 2, 2026 11:28
Copy link
Contributor

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 2, 2026
@openshift-ci
Copy link

openshift-ci bot commented Feb 2, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, 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 [camilamacedo86,tmshort]

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

@openshift-merge-bot openshift-merge-bot bot merged commit e617310 into operator-framework:main Feb 2, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants