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 verify that the cr/crb variables are not nil before attempting to access those object's labels. The testing fake client may need to be updated in the future to handle returning these resources properly.

Signed-off-by: timflannagan <[email protected]>

Author: timflannagan

pkg/controller/operators/olm/operatorgroup.go

@@ -532,11 +532,12 @@ func (a *Operator) ensureSingletonRBAC(operatorNamespace string, csv *v1alpha1.C
 			}
 			// TODO: this should do something smarter if the cluster role already exists
 			if cr, err := a.opClient.CreateClusterRole(clusterRole); err != nil {
+				if !k8serrors.IsAlreadyExists(err) {
+					return err
+				}
 				// if the CR already exists, but the label is correct, the cache is just behind
-				if k8serrors.IsAlreadyExists(err) && ownerutil.IsOwnedByLabel(cr, csv) {
+				if cr != nil && ownerutil.IsOwnedByLabel(cr, csv) {
 					continue
-				} else {
-					return err
 				}
 			}
 			a.logger.Debug("created cluster role")
@@ -572,12 +573,14 @@ func (a *Operator) ensureSingletonRBAC(operatorNamespace string, csv *v1alpha1.C
 			}
 			// TODO: this should do something smarter if the cluster role binding already exists
 			if crb, err := a.opClient.CreateClusterRoleBinding(clusterRoleBinding); err != nil {
-				// if the CR already exists, but the label is correct, the cache is just behind
-				if k8serrors.IsAlreadyExists(err) && ownerutil.IsOwnedByLabel(crb, csv) {
-					continue
-				} else {
+				if !k8serrors.IsAlreadyExists(err) {
 					return err
 				}
+				// if the CRB already exists, but the label is correct, the cache is just behind
+				if crb != nil && ownerutil.IsOwnedByLabel(crb, csv) {
+					continue
+				}
+				return err
 			}
 		}
 	}