Skip to content

Commit d57c077

Browse files
pedjakclaude
andauthored
🐛 Add bundle-version and package-name annotations to CER phase objects (#2580)
* Add bundle-version and package-name annotations to CER phase objects When upgrading between bundle versions that produce identical Kubernetes manifests, the installed version status was not updated because CER phases were identical across versions, causing in-place patches instead of new revision creation. Propagate bundle-version and package-name annotations onto each rendered object within CER phases so that different bundle versions always produce distinct phases, triggering new revision creation via phase immutability. As a side benefit, every applied bundle resource now carries two annotations that immediately tell an observer which package and version it belongs to. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Address PR review feedback for CER phase annotations - Rename mergeLabelMaps to mergeStringMaps since it handles both labels and annotations - Only set bundle-version/package-name annotations when values are non-empty - Update unit test to use realistic revisionAnnotations values Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 1d2a4f7 commit d57c077

4 files changed

Lines changed: 91 additions & 8 deletions

File tree

internal/operator-controller/applier/boxcutter.go

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,24 @@ func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease(
6868
if err := yaml.Unmarshal([]byte(doc), &obj); err != nil {
6969
return nil, err
7070
}
71-
obj.SetLabels(mergeLabelMaps(obj.GetLabels(), objectLabels))
71+
obj.SetLabels(mergeStringMaps(obj.GetLabels(), objectLabels))
7272

7373
// Memory optimization: strip large annotations
7474
// Note: ApplyStripTransform never returns an error in practice
7575
_ = cache.ApplyStripAnnotationsTransform(&obj)
7676
sanitizedUnstructured(ctx, &obj)
7777

78+
annotationUpdates := map[string]string{}
79+
if v := helmRelease.Labels[labels.BundleVersionKey]; v != "" {
80+
annotationUpdates[labels.BundleVersionKey] = v
81+
}
82+
if v := helmRelease.Labels[labels.PackageNameKey]; v != "" {
83+
annotationUpdates[labels.PackageNameKey] = v
84+
}
85+
if len(annotationUpdates) > 0 {
86+
obj.SetAnnotations(mergeStringMaps(obj.GetAnnotations(), annotationUpdates))
87+
}
88+
7889
objs = append(objs, *ocv1ac.ClusterExtensionRevisionObject().
7990
WithObject(obj))
8091
}
@@ -126,7 +137,7 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
126137
// objectLabels
127138
objs := make([]ocv1ac.ClusterExtensionRevisionObjectApplyConfiguration, 0, len(plain))
128139
for _, obj := range plain {
129-
obj.SetLabels(mergeLabelMaps(obj.GetLabels(), objectLabels))
140+
obj.SetLabels(mergeStringMaps(obj.GetLabels(), objectLabels))
130141

131142
gvk, err := apiutil.GVKForObject(obj, r.Scheme)
132143
if err != nil {
@@ -146,6 +157,17 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
146157
}
147158
sanitizedUnstructured(ctx, &unstr)
148159

160+
annotationUpdates := map[string]string{}
161+
if v := revisionAnnotations[labels.BundleVersionKey]; v != "" {
162+
annotationUpdates[labels.BundleVersionKey] = v
163+
}
164+
if v := revisionAnnotations[labels.PackageNameKey]; v != "" {
165+
annotationUpdates[labels.PackageNameKey] = v
166+
}
167+
if len(annotationUpdates) > 0 {
168+
unstr.SetAnnotations(mergeStringMaps(unstr.GetAnnotations(), annotationUpdates))
169+
}
170+
149171
objs = append(objs, *ocv1ac.ClusterExtensionRevisionObject().
150172
WithObject(unstr))
151173
}
@@ -671,9 +693,9 @@ func revisionManagementPerms(rev *ocv1ac.ClusterExtensionRevisionApplyConfigurat
671693
}
672694
}
673695

674-
func mergeLabelMaps(m1, m2 map[string]string) map[string]string {
675-
mergedLabels := make(map[string]string, len(m1)+len(m2))
676-
maps.Copy(mergedLabels, m1)
677-
maps.Copy(mergedLabels, m2)
678-
return mergedLabels
696+
func mergeStringMaps(m1, m2 map[string]string) map[string]string {
697+
merged := make(map[string]string, len(m1)+len(m2))
698+
maps.Copy(merged, m1)
699+
maps.Copy(merged, m2)
700+
return merged
679701
}

internal/operator-controller/applier/boxcutter_test.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,10 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
128128
"labels": map[string]interface{}{
129129
"my-label": "my-value",
130130
},
131+
"annotations": map[string]interface{}{
132+
"olm.operatorframework.io/bundle-version": "1.2.0",
133+
"olm.operatorframework.io/package-name": "my-package",
134+
},
131135
},
132136
},
133137
}),
@@ -140,6 +144,10 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
140144
"labels": map[string]interface{}{
141145
"my-label": "my-value",
142146
},
147+
"annotations": map[string]interface{}{
148+
"olm.operatorframework.io/bundle-version": "1.2.0",
149+
"olm.operatorframework.io/package-name": "my-package",
150+
},
143151
},
144152
},
145153
}),
@@ -199,7 +207,10 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
199207
},
200208
}
201209

202-
rev, err := b.GenerateRevision(t.Context(), dummyBundle, ext, map[string]string{}, map[string]string{})
210+
rev, err := b.GenerateRevision(t.Context(), dummyBundle, ext, map[string]string{}, map[string]string{
211+
labels.BundleVersionKey: "1.0.0",
212+
labels.PackageNameKey: "test-package",
213+
})
203214
require.NoError(t, err)
204215

205216
t.Log("by checking the olm.operatorframework.io/owner-name and owner-kind labels are set")
@@ -223,6 +234,10 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
223234
"kind": "Service",
224235
"metadata": map[string]interface{}{
225236
"name": "test-service",
237+
"annotations": map[string]interface{}{
238+
"olm.operatorframework.io/bundle-version": "1.0.0",
239+
"olm.operatorframework.io/package-name": "test-package",
240+
},
226241
},
227242
"spec": map[string]interface{}{},
228243
},
@@ -244,6 +259,8 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
244259
},
245260
"annotations": map[string]interface{}{
246261
"my-annotation": "my-annotation-value",
262+
"olm.operatorframework.io/bundle-version": "1.0.0",
263+
"olm.operatorframework.io/package-name": "test-package",
247264
},
248265
},
249266
"spec": map[string]interface{}{

test/e2e/features/update.feature

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,36 @@ Feature: Update ClusterExtension
180180
When ClusterCatalog "test" image version "v2" is also tagged as "latest"
181181
Then bundle "test-operator.1.3.0" is installed in version "1.3.0"
182182

183+
@BoxcutterRuntime
184+
Scenario: Update to a version with identical bundle content creates a new revision
185+
Given ClusterExtension is applied
186+
"""
187+
apiVersion: olm.operatorframework.io/v1
188+
kind: ClusterExtension
189+
metadata:
190+
name: ${NAME}
191+
spec:
192+
namespace: ${TEST_NAMESPACE}
193+
serviceAccount:
194+
name: olm-sa
195+
source:
196+
sourceType: Catalog
197+
catalog:
198+
packageName: test
199+
selector:
200+
matchLabels:
201+
"olm.operatorframework.io/metadata.name": test-catalog
202+
version: 1.0.0
203+
upgradeConstraintPolicy: SelfCertified
204+
"""
205+
And ClusterExtension is rolled out
206+
And ClusterExtension is available
207+
And bundle "test-operator.1.0.0" is installed in version "1.0.0"
208+
When ClusterExtension is updated to version "1.0.4"
209+
Then ClusterExtension is rolled out
210+
And ClusterExtension is available
211+
And bundle "test-operator.1.0.4" is installed in version "1.0.4"
212+
183213
@BoxcutterRuntime
184214
Scenario: Each update creates a new revision and resources not present in the new revision are removed from the cluster
185215
Given ClusterExtension is applied

testdata/images/catalogs/test-catalog/v1/configs/catalog.yaml

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ entries:
1717
- name: test-operator.1.0.0
1818
- name: test-operator.1.0.1
1919
replaces: test-operator.1.0.0
20+
- name: test-operator.1.0.4
2021
- name: test-operator.1.2.0
2122
replaces: test-operator.1.0.1
2223
---
@@ -40,6 +41,19 @@ properties:
4041
packageName: test
4142
version: 1.0.1
4243
---
44+
# Bundle with identical rendered content as v1.0.0 (same image).
45+
# Used to test that upgrading between versions with identical manifests
46+
# correctly updates the installed version status (OCPBUGS-78311).
47+
schema: olm.bundle
48+
name: test-operator.1.0.4
49+
package: test
50+
image: docker-registry.operator-controller-e2e.svc.cluster.local:5000/bundles/registry-v1/test-operator:v1.0.0
51+
properties:
52+
- type: olm.package
53+
value:
54+
packageName: test
55+
version: 1.0.4
56+
---
4357
# Bundle with a wrong image ref causing image pull failure
4458
schema: olm.bundle
4559
name: test-operator.1.0.2

0 commit comments

Comments
 (0)