Skip to content

Commit 58f3e62

Browse files
ecordellecordell
committed
fix(operatorgroup): set labels on copied resources correctly
1 parent 37bccd5 commit 58f3e62

File tree

2 files changed

+50
-13
lines changed

2 files changed

+50
-13
lines changed

‎pkg/controller/operators/olm/operator_test.go

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"fmt"
1111
"math"
1212
"math/big"
13+
"sort"
1314
"strings"
1415
"testing"
1516
"time"
@@ -422,6 +423,15 @@ func withAnnotations(obj runtime.Object, annotations map[string]string) runtime.
422423
return meta.(runtime.Object)
423424
}
424425

426+
func withLabels(obj runtime.Object, labels map[string]string) runtime.Object {
427+
meta, ok := obj.(metav1.Object)
428+
if !ok {
429+
panic("could not find metadata on object")
430+
}
431+
meta.SetLabels(labels)
432+
return meta.(runtime.Object)
433+
}
434+
425435
func csvWithAnnotations(csv *v1alpha1.ClusterServiceVersion, annotations map[string]string) *v1alpha1.ClusterServiceVersion {
426436
return withAnnotations(csv, annotations).(*v1alpha1.ClusterServiceVersion)
427437
}
@@ -3089,7 +3099,10 @@ func TestSyncOperatorGroups(t *testing.T) {
30893099
annotatedDeployment,
30903100
},
30913101
targetNamespace: {
3092-
withAnnotations(targetCSV.DeepCopy(), map[string]string{v1alpha2.OperatorGroupAnnotationKey: "operator-group-1", v1alpha2.OperatorGroupNamespaceAnnotationKey: operatorNamespace}),
3102+
withLabels(
3103+
withAnnotations(targetCSV.DeepCopy(), map[string]string{v1alpha2.OperatorGroupAnnotationKey: "operator-group-1", v1alpha2.OperatorGroupNamespaceAnnotationKey: operatorNamespace}),
3104+
map[string]string{v1alpha1.CopiedLabelKey: operatorNamespace},
3105+
),
30933106
&rbacv1.Role{
30943107
TypeMeta: metav1.TypeMeta{
30953108
Kind: "Role",
@@ -3099,8 +3112,9 @@ func TestSyncOperatorGroups(t *testing.T) {
30993112
Name: "csv-role",
31003113
Namespace: targetNamespace,
31013114
Labels: map[string]string{
3115+
"olm.copiedFrom": "operator-ns",
31023116
"olm.owner": "csv1",
3103-
"olm.owner.namespace": "operator-ns",
3117+
"olm.owner.namespace": "target-ns",
31043118
"olm.owner.kind": "ClusterServiceVersion",
31053119
},
31063120
OwnerReferences: []metav1.OwnerReference{
@@ -3118,8 +3132,9 @@ func TestSyncOperatorGroups(t *testing.T) {
31183132
Name: "csv-rolebinding",
31193133
Namespace: targetNamespace,
31203134
Labels: map[string]string{
3135+
"olm.copiedFrom": "operator-ns",
31213136
"olm.owner": "csv1",
3122-
"olm.owner.namespace": "operator-ns",
3137+
"olm.owner.namespace": "target-ns",
31233138
"olm.owner.kind": "ClusterServiceVersion",
31243139
},
31253140
OwnerReferences: []metav1.OwnerReference{
@@ -3184,7 +3199,10 @@ func TestSyncOperatorGroups(t *testing.T) {
31843199
annotatedDeployment,
31853200
},
31863201
targetNamespace: {
3187-
withAnnotations(targetCSV.DeepCopy(), map[string]string{v1alpha2.OperatorGroupAnnotationKey: "operator-group-1", v1alpha2.OperatorGroupNamespaceAnnotationKey: operatorNamespace}),
3202+
withLabels(
3203+
withAnnotations(targetCSV.DeepCopy(), map[string]string{v1alpha2.OperatorGroupAnnotationKey: "operator-group-1", v1alpha2.OperatorGroupNamespaceAnnotationKey: operatorNamespace}),
3204+
map[string]string{v1alpha1.CopiedLabelKey: operatorNamespace},
3205+
),
31883206
&rbacv1.Role{
31893207
TypeMeta: metav1.TypeMeta{
31903208
Kind: "Role",
@@ -3194,8 +3212,9 @@ func TestSyncOperatorGroups(t *testing.T) {
31943212
Name: "csv-role",
31953213
Namespace: targetNamespace,
31963214
Labels: map[string]string{
3215+
"olm.copiedFrom": "operator-ns",
31973216
"olm.owner": "csv1",
3198-
"olm.owner.namespace": "operator-ns",
3217+
"olm.owner.namespace": "target-ns",
31993218
"olm.owner.kind": "ClusterServiceVersion",
32003219
},
32013220
OwnerReferences: []metav1.OwnerReference{
@@ -3213,8 +3232,9 @@ func TestSyncOperatorGroups(t *testing.T) {
32133232
Name: "csv-rolebinding",
32143233
Namespace: targetNamespace,
32153234
Labels: map[string]string{
3235+
"olm.copiedFrom": "operator-ns",
32163236
"olm.owner": "csv1",
3217-
"olm.owner.namespace": "operator-ns",
3237+
"olm.owner.namespace": "target-ns",
32183238
"olm.owner.kind": "ClusterServiceVersion",
32193239
},
32203240
OwnerReferences: []metav1.OwnerReference{
@@ -3325,7 +3345,10 @@ func TestSyncOperatorGroups(t *testing.T) {
33253345
},
33263346
},
33273347
targetNamespace: {
3328-
withAnnotations(targetCSV.DeepCopy(), map[string]string{v1alpha2.OperatorGroupAnnotationKey: "operator-group-1", v1alpha2.OperatorGroupNamespaceAnnotationKey: operatorNamespace}),
3348+
withLabels(
3349+
withAnnotations(targetCSV.DeepCopy(), map[string]string{v1alpha2.OperatorGroupAnnotationKey: "operator-group-1", v1alpha2.OperatorGroupNamespaceAnnotationKey: operatorNamespace}),
3350+
map[string]string{v1alpha1.CopiedLabelKey: operatorNamespace},
3351+
),
33293352
},
33303353
}},
33313354
},
@@ -3435,7 +3458,10 @@ func TestSyncOperatorGroups(t *testing.T) {
34353458
},
34363459
},
34373460
targetNamespace: {
3438-
withAnnotations(targetCSV.DeepCopy(), map[string]string{v1alpha2.OperatorGroupAnnotationKey: "operator-group-1", v1alpha2.OperatorGroupNamespaceAnnotationKey: operatorNamespace}),
3461+
withLabels(
3462+
withAnnotations(targetCSV.DeepCopy(), map[string]string{v1alpha2.OperatorGroupAnnotationKey: "operator-group-1", v1alpha2.OperatorGroupNamespaceAnnotationKey: operatorNamespace}),
3463+
map[string]string{v1alpha1.CopiedLabelKey: operatorNamespace},
3464+
),
34393465
},
34403466
}},
34413467
},
@@ -3604,6 +3630,8 @@ func TestSyncOperatorGroups(t *testing.T) {
36043630

36053631
operatorGroup, err := op.GetClient().OperatorsV1alpha2().OperatorGroups(tt.initial.operatorGroup.GetNamespace()).Get(tt.initial.operatorGroup.GetName(), metav1.GetOptions{})
36063632
require.NoError(t, err)
3633+
sort.Strings(tt.expectedStatus.Namespaces)
3634+
sort.Strings(operatorGroup.Status.Namespaces)
36073635
assert.Equal(t, tt.expectedStatus, operatorGroup.Status)
36083636

36093637
for namespace, objects := range tt.final.objects {

‎pkg/controller/operators/olm/operatorgroup.go

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ func (a *Operator) ensureTenantRBAC(operatorNamespace, targetNamespace string, c
397397
return err
398398
}
399399

400-
targetRoles, err := a.lister.RbacV1().RoleLister().List(ownerutil.CSVOwnerSelector(targetCSV))
400+
targetRoles, err := a.lister.RbacV1().RoleLister().Roles(targetNamespace).List(ownerutil.CSVOwnerSelector(targetCSV))
401401
if err != nil {
402402
return err
403403
}
@@ -409,6 +409,7 @@ func (a *Operator) ensureTenantRBAC(operatorNamespace, targetNamespace string, c
409409

410410
for _, ownedRole := range ownedRoles {
411411
// don't trust the owner label
412+
// TODO: this can skip objects that have owner labels but different ownerreferences
412413
if !ownerutil.IsOwnedBy(ownedRole, csv) {
413414
continue
414415
}
@@ -425,8 +426,11 @@ func (a *Operator) ensureTenantRBAC(operatorNamespace, targetNamespace string, c
425426
}
426427

427428
// role doesn't exist, create it
429+
// TODO: we can work around error cases here; if there's an un-owned role with a matching name we should generate instead
428430
ownedRole.SetNamespace(targetNamespace)
429431
ownedRole.SetOwnerReferences([]metav1.OwnerReference{ownerutil.NonBlockingOwner(targetCSV)})
432+
ownerutil.AddOwnerLabels(ownedRole, targetCSV)
433+
ownedRole.SetLabels(utillabels.AddLabel(ownedRole.GetLabels(), v1alpha1.CopiedLabelKey, operatorNamespace))
430434
if _, err := a.OpClient.CreateRole(ownedRole); err != nil {
431435
return err
432436
}
@@ -437,7 +441,7 @@ func (a *Operator) ensureTenantRBAC(operatorNamespace, targetNamespace string, c
437441
return err
438442
}
439443

440-
targetRoleBindings, err := a.lister.RbacV1().RoleBindingLister().List(ownerutil.CSVOwnerSelector(targetCSV))
444+
targetRoleBindings, err := a.lister.RbacV1().RoleBindingLister().RoleBindings(targetNamespace).List(ownerutil.CSVOwnerSelector(targetCSV))
441445
if err != nil {
442446
return err
443447
}
@@ -457,13 +461,16 @@ func (a *Operator) ensureTenantRBAC(operatorNamespace, targetNamespace string, c
457461

458462
// role binding exists
459463
if ok {
460-
// TODO: should we check if SA/role has changed?
464+
// TODO: we should check if SA/role has changed
461465
continue
462466
}
463467

464468
// role binding doesn't exist
469+
// TODO: we can work around error cases here; if there's an un-owned role with a matching name we should generate instead
465470
ownedRoleBinding.SetNamespace(targetNamespace)
466471
ownedRoleBinding.SetOwnerReferences([]metav1.OwnerReference{ownerutil.NonBlockingOwner(targetCSV)})
472+
ownerutil.AddOwnerLabels(ownedRoleBinding, targetCSV)
473+
ownedRoleBinding.SetLabels(utillabels.AddLabel(ownedRoleBinding.GetLabels(), v1alpha1.CopiedLabelKey, operatorNamespace))
467474
if _, err := a.OpClient.CreateRoleBinding(ownedRoleBinding); err != nil {
468475
return err
469476
}
@@ -504,7 +511,9 @@ func (a *Operator) copyToNamespace(csv *v1alpha1.ClusterServiceVersion, namespac
504511
logger = logger.WithField("csv", csv.GetName())
505512
if fetchedCSV != nil {
506513
logger.Debug("checking annotations")
507-
if !reflect.DeepEqual(fetchedCSV.Annotations, newCSV.Annotations) {
514+
515+
if !reflect.DeepEqual(a.copyOperatorGroupAnnotations(&fetchedCSV.ObjectMeta), a.copyOperatorGroupAnnotations(&newCSV.ObjectMeta)) {
516+
// TODO: only copy over the opgroup annotations, not _all_ annotations
508517
fetchedCSV.Annotations = newCSV.Annotations
509518
fetchedCSV.SetLabels(utillabels.AddLabel(fetchedCSV.GetLabels(), v1alpha1.CopiedLabelKey, csv.GetNamespace()))
510519
// CRs don't support strategic merge patching, but in the future if they do this should be updated to patch
@@ -534,7 +543,7 @@ func (a *Operator) copyToNamespace(csv *v1alpha1.ClusterServiceVersion, namespac
534543
} else if k8serrors.IsNotFound(err) {
535544
newCSV.SetNamespace(namespace)
536545
newCSV.SetResourceVersion("")
537-
newCSV.SetLabels(utillabels.AddLabel(fetchedCSV.GetLabels(), v1alpha1.CopiedLabelKey, csv.GetNamespace()))
546+
newCSV.SetLabels(utillabels.AddLabel(newCSV.GetLabels(), v1alpha1.CopiedLabelKey, csv.GetNamespace()))
538547

539548
logger.Debug("copying CSV to target")
540549
createdCSV, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(namespace).Create(newCSV)

0 commit comments

Comments
 (0)