Skip to content

🐛 Add bundle-version and package-name annotations to CER phase objects #2580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pedjak wants to merge 1 commit into
base: main
Choose a base branch
from
+68 −0
Open

🐛 Add bundle-version and package-name annotations to CER phase objects #2580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions internal/operator-controller/applier/boxcutter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease(
_ = cache.ApplyStripAnnotationsTransform(&obj)
sanitizedUnstructured(ctx, &obj)

Copy link
Contributor

@camilamacedo86 camilamacedo86 Mar 20, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I think you are trying to address: https://github.com/operator-framework/operator-controller/pull/2578/changes

I used the ownerRef to do so, but the PackageName and Version I think might be a better choice.

So @pedjak

Can you please just address Copilot comments?
Then, I think we are OK with this one to get merged

obj.SetAnnotations(mergeLabelMaps(obj.GetAnnotations(), map[string]string{
labels.BundleVersionKey: helmRelease.Labels[labels.BundleVersionKey],
labels.PackageNameKey: helmRelease.Labels[labels.PackageNameKey],
}))
Comment on lines +78 to +81
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Minor readability: mergeLabelMaps is now used to merge both labels and annotations. Consider renaming it to something generic (e.g., mergeStringMaps) to avoid implying it’s label-specific.

Copilot uses AI. Check for mistakes.
Comment on lines +78 to +81
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

GenerateRevisionFromHelmRelease blindly reads helmRelease.Labels[...] and writes those values into per-object annotations. If the Helm release is missing these labels (e.g., older releases or unexpected callers), this will set empty-string annotations, which is both misleading and defeats the goal of making phases distinct across versions. Consider (a) validating the required label keys are present and returning an error if not, or (b) only merging these annotations when the values are non-empty / present.

Copilot uses AI. Check for mistakes.

objs = append(objs, *ocv1ac.ClusterExtensionRevisionObject().
WithObject(obj))
}
Expand Down Expand Up @@ -146,6 +151,11 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
}
sanitizedUnstructured(ctx, &unstr)

unstr.SetAnnotations(mergeLabelMaps(unstr.GetAnnotations(), map[string]string{
labels.BundleVersionKey: revisionAnnotations[labels.BundleVersionKey],
labels.PackageNameKey: revisionAnnotations[labels.PackageNameKey],
}))

Comment on lines +154 to +158
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

GenerateRevision pulls revisionAnnotations[labels.BundleVersionKey] / [labels.PackageNameKey] without checking presence, and then unconditionally writes the resulting (possibly empty) values into every rendered object annotation. This can produce resources annotated with empty bundle metadata and—if the caller ever forgets to pass these keys—won’t actually guarantee phase immutability across bundle versions. Consider requiring these keys to be set (return an error if missing) or only applying them when non-empty.

Suggested change
unstr.SetAnnotations(mergeLabelMaps(unstr.GetAnnotations(), map[string]string{
labels.BundleVersionKey: revisionAnnotations[labels.BundleVersionKey],
labels.PackageNameKey: revisionAnnotations[labels.PackageNameKey],
}))
annotationUpdates := map[string]string{}
if v := revisionAnnotations[labels.BundleVersionKey]; v != "" {
annotationUpdates[labels.BundleVersionKey] = v
}
if v := revisionAnnotations[labels.PackageNameKey]; v != "" {
annotationUpdates[labels.PackageNameKey] = v
}
if len(annotationUpdates) > 0 {
unstr.SetAnnotations(mergeLabelMaps(unstr.GetAnnotations(), annotationUpdates))
}

Copilot uses AI. Check for mistakes.
objs = append(objs, *ocv1ac.ClusterExtensionRevisionObject().
WithObject(unstr))
}
Expand Down
14 changes: 14 additions & 0 deletions internal/operator-controller/applier/boxcutter_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
"labels": map[string]interface{}{
"my-label": "my-value",
},
"annotations": map[string]interface{}{
"olm.operatorframework.io/bundle-version": "1.2.0",
"olm.operatorframework.io/package-name": "my-package",
},
},
},
}),
Expand All @@ -140,6 +144,10 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
"labels": map[string]interface{}{
"my-label": "my-value",
},
"annotations": map[string]interface{}{
"olm.operatorframework.io/bundle-version": "1.2.0",
"olm.operatorframework.io/package-name": "my-package",
},
},
},
}),
Expand Down Expand Up @@ -223,6 +231,10 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
"kind": "Service",
"metadata": map[string]interface{}{
"name": "test-service",
"annotations": map[string]interface{}{
"olm.operatorframework.io/bundle-version": "",
"olm.operatorframework.io/package-name": "",
},
},
"spec": map[string]interface{}{},
},
Expand All @@ -244,6 +256,8 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
},
"annotations": map[string]interface{}{
"my-annotation": "my-annotation-value",
"olm.operatorframework.io/bundle-version": "",
"olm.operatorframework.io/package-name": "",
},
Comment on lines 234 to 261
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

This unit test passes an empty revisionAnnotations map into GenerateRevision, which now results in per-object annotations with empty bundle-version / package-name values. Since production callers populate these annotations (and empty values are misleading), consider updating the test to pass realistic revisionAnnotations values and assert those propagated values instead of empty strings.

Copilot uses AI. Check for mistakes.
},
"spec": map[string]interface{}{
Expand Down
30 changes: 30 additions & 0 deletions test/e2e/features/update.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,36 @@ Feature: Update ClusterExtension
When ClusterCatalog "test" image version "v2" is also tagged as "latest"
Then bundle "test-operator.1.3.0" is installed in version "1.3.0"

@BoxcutterRuntime
Scenario: Update to a version with identical bundle content creates a new revision
Given ClusterExtension is applied
"""
apiVersion: olm.operatorframework.io/v1
kind: ClusterExtension
metadata:
name: ${NAME}
spec:
namespace: ${TEST_NAMESPACE}
serviceAccount:
name: olm-sa
source:
sourceType: Catalog
catalog:
packageName: test
selector:
matchLabels:
"olm.operatorframework.io/metadata.name": test-catalog
version: 1.0.0
upgradeConstraintPolicy: SelfCertified
"""
And ClusterExtension is rolled out
And ClusterExtension is available
And bundle "test-operator.1.0.0" is installed in version "1.0.0"
When ClusterExtension is updated to version "1.0.4"
Then ClusterExtension is rolled out
And ClusterExtension is available
And bundle "test-operator.1.0.4" is installed in version "1.0.4"
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

This e2e scenario is titled as creating a new revision, but it only asserts the installed bundle/version. To actually validate the regression (phase immutability triggers new revision creation), add assertions that the active revision name increments (e.g., ${NAME}-2) and/or that the previous revision is archived after updating to 1.0.4.

Suggested change
And bundle "test-operator.1.0.4" is installed in version "1.0.4"
And bundle "test-operator.1.0.4" is installed in version "1.0.4"
And ClusterExtension reports "${NAME}-2" as active revision
And ClusterExtensionRevision "${NAME}-2" reports Progressing as True with Reason Succeeded
And ClusterExtensionRevision "${NAME}-2" reports Available as True with Reason ProbesSucceeded
And ClusterExtensionRevision "${NAME}-1" is archived

Copilot uses AI. Check for mistakes.

@BoxcutterRuntime
Scenario: Each update creates a new revision and resources not present in the new revision are removed from the cluster
Given ClusterExtension is applied
Expand Down
14 changes: 14 additions & 0 deletions testdata/images/catalogs/test-catalog/v1/configs/catalog.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ entries:
- name: test-operator.1.0.0
- name: test-operator.1.0.1
replaces: test-operator.1.0.0
- name: test-operator.1.0.4
- name: test-operator.1.2.0
replaces: test-operator.1.0.1
---
Expand All @@ -40,6 +41,19 @@ properties:
packageName: test
version: 1.0.1
---
# Bundle with identical rendered content as v1.0.0 (same image).
# Used to test that upgrading between versions with identical manifests
# correctly updates the installed version status (OCPBUGS-78311).
schema: olm.bundle
name: test-operator.1.0.4
package: test
image: docker-registry.operator-controller-e2e.svc.cluster.local:5000/bundles/registry-v1/test-operator:v1.0.0
properties:
- type: olm.package
value:
packageName: test
version: 1.0.4
---
# Bundle with a wrong image ref causing image pull failure
schema: olm.bundle
name: test-operator.1.0.2
Expand Down
Loading