Skip to content

Commit 09d1e58

Browse files
committed
pkg/controller: Fix panic when creating cluster-scoped RBAC in OG controller
Fixes [#2091](#2091). This is a follow-up to [#2309](#2309) that attempted to fix the original issue. When checking whether the ClusterRole/ClusterRoleBinding resources already exist, we're also checking whether the existing labels are owned by the CSV we're currently handling. When accessing the "cr" or "crb" variables that the Create calls output, a panic is produced as we're attempting to access the meta.Labels key from those resources, except those resources themselves are nil. Update the check to instead use the static type instead to avoid a panic. Signed-off-by: timflannagan <[email protected]>
1 parent 864b58d commit 09d1e58

File tree

1 file changed

+6
-9
lines changed

1 file changed

+6
-9
lines changed

pkg/controller/operators/olm/operatorgroup.go

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -531,13 +531,11 @@ func (a *Operator) ensureSingletonRBAC(operatorNamespace string, csv *v1alpha1.C
531531
}),
532532
}
533533
// TODO: this should do something smarter if the cluster role already exists
534-
if cr, err := a.opClient.CreateClusterRole(clusterRole); err != nil {
535-
// if the CR already exists, but the label is correct, the cache is just behind
536-
if k8serrors.IsAlreadyExists(err) && ownerutil.IsOwnedByLabel(cr, csv) {
534+
if _, err := a.opClient.CreateClusterRole(clusterRole); err != nil {
535+
if k8serrors.IsAlreadyExists(err) && ownerutil.IsOwnedByLabel(clusterRole, csv) {
537536
continue
538-
} else {
539-
return err
540537
}
538+
return err
541539
}
542540
a.logger.Debug("created cluster role")
543541
}
@@ -571,13 +569,12 @@ func (a *Operator) ensureSingletonRBAC(operatorNamespace string, csv *v1alpha1.C
571569
},
572570
}
573571
// TODO: this should do something smarter if the cluster role binding already exists
574-
if crb, err := a.opClient.CreateClusterRoleBinding(clusterRoleBinding); err != nil {
572+
if _, err := a.opClient.CreateClusterRoleBinding(clusterRoleBinding); err != nil {
575573
// if the CR already exists, but the label is correct, the cache is just behind
576-
if k8serrors.IsAlreadyExists(err) && ownerutil.IsOwnedByLabel(crb, csv) {
574+
if k8serrors.IsAlreadyExists(err) && ownerutil.IsOwnedByLabel(clusterRoleBinding, csv) {
577575
continue
578-
} else {
579-
return err
580576
}
577+
return err
581578
}
582579
}
583580
}

0 commit comments

Comments
 (0)