Skip to content

Commit 76c749f

Browse files
perdasilvaPer Goncalves da Silvaclaude
authored
Set owner-kind label on ClusterExtensionRevision resources (#2480)
Add the owner-kind label to ClusterExtensionRevision resources to ensure they are properly labeled with both owner-kind and owner-name. Previously, only the owner-name label was set on the ClusterExtensionRevision resource itself. The owner-kind label was only applied to objects within the revision, but not to the revision resource. This change updates the buildClusterExtensionRevision function to set both labels.OwnerKindKey (ClusterExtension) and labels.OwnerNameKey on the ClusterExtensionRevision metadata. Also fixed variable name collision where local variable 'labels' shadowed the imported 'labels' package by renaming it to 'mergedLabels'. Signed-off-by: Per Goncalves da Silva <pegoncal@redhat.com> Co-authored-by: Per Goncalves da Silva <pegoncal@redhat.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 2582cf1 commit 76c749f

4 files changed

Lines changed: 58 additions & 12 deletions

File tree

internal/operator-controller/applier/boxcutter.go

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -67,12 +67,7 @@ func (r *SimpleRevisionGenerator) GenerateRevisionFromHelmRelease(
6767
if err := yaml.Unmarshal([]byte(doc), &obj); err != nil {
6868
return nil, err
6969
}
70-
71-
existingLabels := obj.GetLabels()
72-
labels := make(map[string]string, len(existingLabels)+len(objectLabels))
73-
maps.Copy(labels, existingLabels)
74-
maps.Copy(labels, objectLabels)
75-
obj.SetLabels(labels)
70+
obj.SetLabels(mergeLabelMaps(obj.GetLabels(), objectLabels))
7671

7772
// Memory optimization: strip large annotations
7873
// Note: ApplyStripTransform never returns an error in practice
@@ -131,11 +126,7 @@ func (r *SimpleRevisionGenerator) GenerateRevision(
131126
// objectLabels
132127
objs := make([]ocv1.ClusterExtensionRevisionObject, 0, len(plain))
133128
for _, obj := range plain {
134-
existingLabels := obj.GetLabels()
135-
labels := make(map[string]string, len(existingLabels)+len(objectLabels))
136-
maps.Copy(labels, existingLabels)
137-
maps.Copy(labels, objectLabels)
138-
obj.SetLabels(labels)
129+
obj.SetLabels(mergeLabelMaps(obj.GetLabels(), objectLabels))
139130

140131
gvk, err := apiutil.GVKForObject(obj, r.Scheme)
141132
if err != nil {
@@ -219,6 +210,7 @@ func (r *SimpleRevisionGenerator) buildClusterExtensionRevision(
219210
ObjectMeta: metav1.ObjectMeta{
220211
Annotations: annotations,
221212
Labels: map[string]string{
213+
labels.OwnerKindKey: ocv1.ClusterExtensionKind,
222214
labels.OwnerNameKey: ext.Name,
223215
},
224216
},
@@ -666,3 +658,10 @@ func revisionManagementPerms(rev *ocv1.ClusterExtensionRevision) func(user.Info)
666658
}
667659
}
668660
}
661+
662+
func mergeLabelMaps(m1, m2 map[string]string) map[string]string {
663+
mergedLabels := make(map[string]string, len(m1)+len(m2))
664+
maps.Copy(mergedLabels, m1)
665+
maps.Copy(mergedLabels, m2)
666+
return mergedLabels
667+
}

internal/operator-controller/applier/boxcutter_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
109109
"olm.operatorframework.io/service-account-namespace": "test-namespace",
110110
},
111111
Labels: map[string]string{
112+
labels.OwnerKindKey: ocv1.ClusterExtensionKind,
112113
labels.OwnerNameKey: "test-123",
113114
},
114115
},
@@ -207,8 +208,9 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
207208
rev, err := b.GenerateRevision(t.Context(), dummyBundle, ext, map[string]string{}, map[string]string{})
208209
require.NoError(t, err)
209210

210-
t.Log("by checking the olm.operatorframework.io/owner-name label is set to the name of the ClusterExtension")
211+
t.Log("by checking the olm.operatorframework.io/owner-name and owner-kind labels are set")
211212
require.Equal(t, map[string]string{
213+
labels.OwnerKindKey: ocv1.ClusterExtensionKind,
212214
labels.OwnerNameKey: "test-extension",
213215
}, rev.Labels)
214216
t.Log("by checking the revision number is 0")

test/e2e/features/install.feature

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,3 +417,29 @@ Feature: Install ClusterExtension
417417
"""
418418
[{"type":"olm.test-property","value":"some-value"}]
419419
"""
420+
421+
@BoxcutterRuntime
422+
Scenario: ClusterExtensionRevision is labeled with owner information
423+
When ClusterExtension is applied
424+
"""
425+
apiVersion: olm.operatorframework.io/v1
426+
kind: ClusterExtension
427+
metadata:
428+
name: ${NAME}
429+
spec:
430+
namespace: ${TEST_NAMESPACE}
431+
serviceAccount:
432+
name: olm-sa
433+
source:
434+
sourceType: Catalog
435+
catalog:
436+
packageName: test
437+
version: 1.2.0
438+
selector:
439+
matchLabels:
440+
"olm.operatorframework.io/metadata.name": test-catalog
441+
"""
442+
Then ClusterExtension is rolled out
443+
And ClusterExtension is available
444+
And ClusterExtensionRevision "${NAME}-1" has label "olm.operatorframework.io/owner-kind" with value "ClusterExtension"
445+
And ClusterExtensionRevision "${NAME}-1" has label "olm.operatorframework.io/owner-name" with value "${NAME}"

test/e2e/steps/steps.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ func RegisterSteps(sc *godog.ScenarioContext) {
7474
sc.Step(`^(?i)ClusterExtension reports ([[:alnum:]]+) transition between (\d+) and (\d+) minutes since its creation$`, ClusterExtensionReportsConditionTransitionTime)
7575
sc.Step(`^(?i)ClusterExtensionRevision "([^"]+)" is archived$`, ClusterExtensionRevisionIsArchived)
7676
sc.Step(`^(?i)ClusterExtensionRevision "([^"]+)" contains annotation "([^"]+)" with value$`, ClusterExtensionRevisionHasAnnotationWithValue)
77+
sc.Step(`^(?i)ClusterExtensionRevision "([^"]+)" has label "([^"]+)" with value "([^"]+)"$`, ClusterExtensionRevisionHasLabelWithValue)
7778

7879
sc.Step(`^(?i)resource "([^"]+)" is installed$`, ResourceAvailable)
7980
sc.Step(`^(?i)resource "([^"]+)" is available$`, ResourceAvailable)
@@ -507,6 +508,24 @@ func ClusterExtensionRevisionHasAnnotationWithValue(ctx context.Context, revisio
507508
return nil
508509
}
509510

511+
func ClusterExtensionRevisionHasLabelWithValue(ctx context.Context, revisionName, labelKey, labelValue string) error {
512+
sc := scenarioCtx(ctx)
513+
revisionName = substituteScenarioVars(strings.TrimSpace(revisionName), sc)
514+
labelValue = substituteScenarioVars(labelValue, sc)
515+
waitFor(ctx, func() bool {
516+
obj, err := getResource("clusterextensionrevision", revisionName, "")
517+
if err != nil {
518+
logger.V(1).Error(err, "failed to get clusterextensionrevision", "name", revisionName)
519+
return false
520+
}
521+
if obj.GetLabels() == nil {
522+
return false
523+
}
524+
return obj.GetLabels()[labelKey] == labelValue
525+
})
526+
return nil
527+
}
528+
510529
func ResourceAvailable(ctx context.Context, resource string) error {
511530
sc := scenarioCtx(ctx)
512531
resource = substituteScenarioVars(resource, sc)

0 commit comments

Comments
 (0)