Skip to content

⚠️ fix: rename owner label and clarify variable naming #2349

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

Merged
merged 1 commit into from
Nov 19, 2025
Merged

⚠️ fix: rename owner label and clarify variable naming #2349

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
7 changes: 3 additions & 4 deletions internal/operator-controller/applier/boxcutter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
helmclient "github.com/operator-framework/helm-operator-plugins/pkg/client"

ocv1 "github.com/operator-framework/operator-controller/api/v1"
"github.com/operator-framework/operator-controller/internal/operator-controller/controllers"
"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
)

Expand Down Expand Up @@ -183,7 +182,7 @@ func (r *SimpleRevisionGenerator) buildClusterExtensionRevision(
ObjectMeta: metav1.ObjectMeta{
Annotations: annotations,
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand Down Expand Up @@ -217,7 +216,7 @@ type boxcutterStorageMigratorClient interface {
func (m *BoxcutterStorageMigrator) Migrate(ctx context.Context, ext *ocv1.ClusterExtension, objectLabels map[string]string) error {
existingRevisionList := ocv1.ClusterExtensionRevisionList{}
if err := m.Client.List(ctx, &existingRevisionList, client.MatchingLabels{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
}); err != nil {
return fmt.Errorf("listing ClusterExtensionRevisions before attempting migration: %w", err)
}
Expand Down Expand Up @@ -429,7 +428,7 @@ func (bc *Boxcutter) garbageCollectOldRevisions(ctx context.Context, revisionLis
func (bc *Boxcutter) getExistingRevisions(ctx context.Context, extName string) ([]ocv1.ClusterExtensionRevision, error) {
existingRevisionList := &ocv1.ClusterExtensionRevisionList{}
if err := bc.Client.List(ctx, existingRevisionList, client.MatchingLabels{
controllers.ClusterExtensionRevisionOwnerLabel: extName,
labels.OwnerNameKey: extName,
}); err != nil {
return nil, fmt.Errorf("listing revisions: %w", err)
}
Expand Down
63 changes: 31 additions & 32 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 @@ -28,7 +28,6 @@ import (

ocv1 "github.com/operator-framework/operator-controller/api/v1"
"github.com/operator-framework/operator-controller/internal/operator-controller/applier"
"github.com/operator-framework/operator-controller/internal/operator-controller/controllers"
"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
)

Expand Down Expand Up @@ -86,7 +85,7 @@ func Test_SimpleRevisionGenerator_GenerateRevisionFromHelmRelease(t *testing.T)
"olm.operatorframework.io/package-name": "my-package",
},
Labels: map[string]string{
"olm.operatorframework.io/owner": "test-123",
labels.OwnerNameKey: "test-123",
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand Down Expand Up @@ -178,9 +177,9 @@ func Test_SimpleRevisionGenerator_GenerateRevision(t *testing.T) {
rev, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, ext, map[string]string{}, map[string]string{})
require.NoError(t, err)

t.Log("by checking the olm.operatorframework.io/owner label is set to the name of the ClusterExtension")
t.Log("by checking the olm.operatorframework.io/owner-name label is set to the name of the ClusterExtension")
require.Equal(t, map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: "test-extension",
labels.OwnerNameKey: "test-extension",
}, rev.Labels)
t.Log("by checking the revision number is 0")
require.Equal(t, int64(0), rev.Spec.Revision)
Expand Down Expand Up @@ -344,7 +343,7 @@ func TestBoxcutter_Apply(t *testing.T) {
Name: "test-ext-1",
UID: "rev-uid-1",
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand Down Expand Up @@ -402,7 +401,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Annotations: revisionAnnotations,
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand Down Expand Up @@ -430,7 +429,7 @@ func TestBoxcutter_Apply(t *testing.T) {
},
validate: func(t *testing.T, c client.Client) {
revList := &ocv1.ClusterExtensionRevisionList{}
err := c.List(t.Context(), revList, client.MatchingLabels{controllers.ClusterExtensionRevisionOwnerLabel: ext.Name})
err := c.List(t.Context(), revList, client.MatchingLabels{labels.OwnerNameKey: ext.Name})
require.NoError(t, err)
require.Len(t, revList.Items, 1)

Expand All @@ -450,7 +449,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Annotations: revisionAnnotations,
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand Down Expand Up @@ -481,7 +480,7 @@ func TestBoxcutter_Apply(t *testing.T) {
},
validate: func(t *testing.T, c client.Client) {
revList := &ocv1.ClusterExtensionRevisionList{}
err := c.List(context.Background(), revList, client.MatchingLabels{controllers.ClusterExtensionRevisionOwnerLabel: ext.Name})
err := c.List(context.Background(), revList, client.MatchingLabels{labels.OwnerNameKey: ext.Name})
require.NoError(t, err)
// No new revision should be created
require.Len(t, revList.Items, 1)
Expand All @@ -496,7 +495,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Annotations: revisionAnnotations,
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand Down Expand Up @@ -528,7 +527,7 @@ func TestBoxcutter_Apply(t *testing.T) {
},
validate: func(t *testing.T, c client.Client) {
revList := &ocv1.ClusterExtensionRevisionList{}
err := c.List(context.Background(), revList, client.MatchingLabels{controllers.ClusterExtensionRevisionOwnerLabel: ext.Name})
err := c.List(context.Background(), revList, client.MatchingLabels{labels.OwnerNameKey: ext.Name})
require.NoError(t, err)
require.Len(t, revList.Items, 2)

Expand Down Expand Up @@ -557,7 +556,7 @@ func TestBoxcutter_Apply(t *testing.T) {
validate: func(t *testing.T, c client.Client) {
// Ensure no revisions were created
revList := &ocv1.ClusterExtensionRevisionList{}
err := c.List(context.Background(), revList, client.MatchingLabels{controllers.ClusterExtensionRevisionOwnerLabel: ext.Name})
err := c.List(context.Background(), revList, client.MatchingLabels{labels.OwnerNameKey: ext.Name})
require.NoError(t, err)
assert.Empty(t, revList.Items)
},
Expand All @@ -570,7 +569,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Annotations: revisionAnnotations,
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{},
Expand All @@ -582,7 +581,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-1",
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand All @@ -594,7 +593,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-2",
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand All @@ -606,7 +605,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-3",
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand All @@ -618,7 +617,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-4",
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand All @@ -630,7 +629,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-5",
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand All @@ -642,7 +641,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-6",
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand Down Expand Up @@ -674,7 +673,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Annotations: revisionAnnotations,
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{},
Expand All @@ -686,7 +685,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-1",
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand All @@ -698,7 +697,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-2",
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand All @@ -711,7 +710,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-3",
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand All @@ -723,7 +722,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-4",
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand All @@ -736,7 +735,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-5",
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand All @@ -748,7 +747,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-6",
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand All @@ -760,7 +759,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Name: "rev-7",
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand Down Expand Up @@ -794,7 +793,7 @@ func TestBoxcutter_Apply(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{
Annotations: revisionAnnotations,
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand Down Expand Up @@ -830,7 +829,7 @@ func TestBoxcutter_Apply(t *testing.T) {
labels.PackageNameKey: "test-package",
},
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{
Expand Down Expand Up @@ -858,7 +857,7 @@ func TestBoxcutter_Apply(t *testing.T) {
},
validate: func(t *testing.T, c client.Client) {
revList := &ocv1.ClusterExtensionRevisionList{}
err := c.List(context.Background(), revList, client.MatchingLabels{controllers.ClusterExtensionRevisionOwnerLabel: ext.Name})
err := c.List(context.Background(), revList, client.MatchingLabels{labels.OwnerNameKey: ext.Name})
require.NoError(t, err)
// Should still be only 1 revision (in-place update, not new revision)
require.Len(t, revList.Items, 1)
Expand All @@ -870,7 +869,7 @@ func TestBoxcutter_Apply(t *testing.T) {
assert.Equal(t, "1.0.1", rev.Annotations[labels.BundleVersionKey])
assert.Equal(t, "test-package", rev.Annotations[labels.PackageNameKey])
// Verify owner label is still present
assert.Equal(t, ext.Name, rev.Labels[controllers.ClusterExtensionRevisionOwnerLabel])
assert.Equal(t, ext.Name, rev.Labels[labels.OwnerNameKey])
},
},
}
Expand Down Expand Up @@ -1061,7 +1060,7 @@ func (m *mockBundleRevisionBuilder) GenerateRevisionFromHelmRelease(
ObjectMeta: metav1.ObjectMeta{
Name: "test-revision",
Labels: map[string]string{
controllers.ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
},
},
Spec: ocv1.ClusterExtensionRevisionSpec{},
Expand Down
7 changes: 4 additions & 3 deletions internal/operator-controller/controllers/clusterextension_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,8 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1.Cl
return ctrl.Result{}, err
}

storeLbls := map[string]string{
// The following values will be stored as annotations and not labels
revisionAnnotations := map[string]string{
labels.BundleNameKey: resolvedRevisionMetadata.Name,
labels.PackageNameKey: resolvedRevisionMetadata.Package,
labels.BundleVersionKey: resolvedRevisionMetadata.Version,
Expand All @@ -297,7 +298,7 @@ func (r *ClusterExtensionReconciler) reconcile(ctx context.Context, ext *ocv1.Cl
// to ensure exponential backoff can occur:
// - Permission errors (it is not possible to watch changes to permissions.
// The only way to eventually recover from permission errors is to keep retrying).
rolloutSucceeded, rolloutStatus, err := r.Applier.Apply(ctx, imageFS, ext, objLbls, storeLbls)
rolloutSucceeded, rolloutStatus, err := r.Applier.Apply(ctx, imageFS, ext, objLbls, revisionAnnotations)

// Set installed status
if rolloutSucceeded {
Expand Down Expand Up @@ -531,7 +532,7 @@ func (d *BoxcutterRevisionStatesGetter) GetRevisionStates(ctx context.Context, e
// recent revisions. We should consolidate to avoid code duplication.
existingRevisionList := &ocv1.ClusterExtensionRevisionList{}
if err := d.Reader.List(ctx, existingRevisionList, client.MatchingLabels{
ClusterExtensionRevisionOwnerLabel: ext.Name,
labels.OwnerNameKey: ext.Name,
}); err != nil {
return nil, fmt.Errorf("listing revisions: %w", err)
}
Expand Down
16 changes: 8 additions & 8 deletions internal/operator-controller/controllers/clusterextensionrevision_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ import (
"sigs.k8s.io/controller-runtime/pkg/source"

ocv1 "github.com/operator-framework/operator-controller/api/v1"
"github.com/operator-framework/operator-controller/internal/operator-controller/labels"
)

const (
ClusterExtensionRevisionOwnerLabel = "olm.operatorframework.io/owner"
clusterExtensionRevisionTeardownFinalizer = "olm.operatorframework.io/teardown"
)

Expand Down Expand Up @@ -458,15 +458,15 @@ func (c *ClusterExtensionRevisionReconciler) removeFinalizer(ctx context.Context
// listPreviousRevisions returns active revisions belonging to the same ClusterExtension with lower revision numbers.
// Filters out the current revision, archived revisions, deleting revisions, and revisions with equal or higher numbers.
func (c *ClusterExtensionRevisionReconciler) listPreviousRevisions(ctx context.Context, rev *ocv1.ClusterExtensionRevision) ([]*ocv1.ClusterExtensionRevision, error) {
ownerLabel, ok := rev.Labels[ClusterExtensionRevisionOwnerLabel]
ownerLabel, ok := rev.Labels[labels.OwnerNameKey]
if !ok {
// No owner label means this revision isn't properly labeled - return empty list
return nil, nil
}

revList := &ocv1.ClusterExtensionRevisionList{}
if err := c.TrackingCache.List(ctx, revList, client.MatchingLabels{
ClusterExtensionRevisionOwnerLabel: ownerLabel,
labels.OwnerNameKey: ownerLabel,
}); err != nil {
return nil, fmt.Errorf("listing revisions: %w", err)
}
Expand Down Expand Up @@ -521,12 +521,12 @@ func (c *ClusterExtensionRevisionReconciler) toBoxcutterRevision(ctx context.Con
for _, specObj := range specPhase.Objects {
obj := specObj.Object.DeepCopy()

labels := obj.GetLabels()
if labels == nil {
labels = map[string]string{}
objLabels := obj.GetLabels()
if objLabels == nil {
objLabels = map[string]string{}
}
labels[ClusterExtensionRevisionOwnerLabel] = rev.Labels[ClusterExtensionRevisionOwnerLabel]
obj.SetLabels(labels)
objLabels[labels.OwnerNameKey] = rev.Labels[labels.OwnerNameKey]
obj.SetLabels(objLabels)

switch specObj.CollisionProtection {
case ocv1.CollisionProtectionIfNoController, ocv1.CollisionProtectionNone:
Expand Down
Loading
Loading