✨ Add bundle olm.properties to revision annotations #2471
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
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.propertiesannotation 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.
Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com>
perdasilva
force-pushed
the
revision-csv-annotations
branch
from
6657401 to
4070119
Compare
| if obj.GetAnnotations() == nil { | ||
| return false | ||
| } | ||
| if obj.GetAnnotations()[annotationKey] != expectedValue { |
There was a problem hiding this comment.
I think the rest of the function could collapsed into
return obj.GetAnnotations()[annotationKey] == expectedValue
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tmshort The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
Unfortunately, the |
3 participants
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.
createdAtandcapabilities). 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