Skip to content

Commit bf99831

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 bf99831

File tree

1 file changed

+6
-8
lines changed

1 file changed

+6
-8
lines changed

pkg/controller/operators/olm/operatorgroup.go

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -531,13 +531,12 @@ 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 {
534+
if _, err := a.opClient.CreateClusterRole(clusterRole); err != nil {
535535
// if the CR already exists, but the label is correct, the cache is just behind
536-
if k8serrors.IsAlreadyExists(err) && ownerutil.IsOwnedByLabel(cr, csv) {
536+
if k8serrors.IsAlreadyExists(err) && ownerutil.IsOwnedByLabel(clusterRole, csv) {
537537
continue
538-
} else {
539-
return err
540538
}
539+
return err
541540
}
542541
a.logger.Debug("created cluster role")
543542
}
@@ -571,13 +570,12 @@ func (a *Operator) ensureSingletonRBAC(operatorNamespace string, csv *v1alpha1.C
571570
},
572571
}
573572
// TODO: this should do something smarter if the cluster role binding already exists
574-
if crb, err := a.opClient.CreateClusterRoleBinding(clusterRoleBinding); err != nil {
573+
if _, err := a.opClient.CreateClusterRoleBinding(clusterRoleBinding); err != nil {
575574
// if the CR already exists, but the label is correct, the cache is just behind
576-
if k8serrors.IsAlreadyExists(err) && ownerutil.IsOwnedByLabel(crb, csv) {
575+
if k8serrors.IsAlreadyExists(err) && ownerutil.IsOwnedByLabel(clusterRoleBinding, csv) {
577576
continue
578-
} else {
579-
return err
580577
}
578+
return err
581579
}
582580
}
583581
}

0 commit comments

Comments
 (0)