Unpack job security updates (#2805)

* Remove unneeded 'get' verb from bundle unpacker role

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

* Move bundle unpacker policy rule creation to its own method

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

Author: perdasilva

pkg/controller/bundle/bundle_unpacker.go

@@ -435,7 +435,7 @@ func (c *ConfigMapUnpacker) UnpackBundle(lookup *operatorsv1alpha1.BundleLookup,
 		return
 	}
 
-	_, err = c.ensureRole(cmRef)
+	_, err = c.ensureRole(cmRef, c.getRolePolicyRules(cmRef))
 	if err != nil {
 		return
 	}
@@ -610,27 +610,13 @@ func (c *ConfigMapUnpacker) ensureJob(cmRef *corev1.ObjectReference, bundlePath
 	return
 }
 
-func (c *ConfigMapUnpacker) ensureRole(cmRef *corev1.ObjectReference) (role *rbacv1.Role, err error) {
+func (c *ConfigMapUnpacker) ensureRole(cmRef *corev1.ObjectReference, policyRules []rbacv1.PolicyRule) (role *rbacv1.Role, err error) {
 	if cmRef == nil {
 		return nil, fmt.Errorf("configmap reference is nil")
 	}
 
-	rule := rbacv1.PolicyRule{
-		APIGroups: []string{
-			"",
-		},
-		Verbs: []string{
-			"create", "get", "update",
-		},
-		Resources: []string{
-			"configmaps",
-		},
-		ResourceNames: []string{
-			cmRef.Name,
-		},
-	}
 	fresh := &rbacv1.Role{
-		Rules: []rbacv1.PolicyRule{rule},
+		Rules: policyRules,
 	}
 	fresh.SetNamespace(cmRef.Namespace)
 	fresh.SetName(cmRef.Name)
@@ -646,19 +632,43 @@ func (c *ConfigMapUnpacker) ensureRole(cmRef *corev1.ObjectReference) (role *rba
 	}
 
 	// Add the policy rule if necessary
-	for _, existing := range role.Rules {
-		if equality.Semantic.DeepDerivative(rule, existing) {
-			return
+	var ruleDiff []rbacv1.PolicyRule
+	for _, proposed := range policyRules {
+		if !containsRule(role.Rules, proposed) {
+			ruleDiff = append(ruleDiff, proposed)
 		}
 	}
+
 	role = role.DeepCopy()
-	role.Rules = append(role.Rules, rule)
+	role.Rules = append(role.Rules, ruleDiff...)
 
 	role, err = c.client.RbacV1().Roles(role.GetNamespace()).Update(context.TODO(), role, metav1.UpdateOptions{})
 
 	return
 }
 
+// getRolePolicyRules returns the set of policy rules used by the role attached to the
+// bundle unpacker service account. This method lends itself to easier downstream patching when additional
+// policy rules are required, e.g. for Openshift SCC
+func (c *ConfigMapUnpacker) getRolePolicyRules(cmRef *corev1.ObjectReference) []rbacv1.PolicyRule {
+	return []rbacv1.PolicyRule{
+		{
+			APIGroups: []string{
+				"",
+			},
+			Verbs: []string{
+				"get", "update",
+			},
+			Resources: []string{
+				"configmaps",
+			},
+			ResourceNames: []string{
+				cmRef.Name,
+			},
+		},
+	}
+}
+
 func (c *ConfigMapUnpacker) ensureRoleBinding(cmRef *corev1.ObjectReference) (roleBinding *rbacv1.RoleBinding, err error) {
 	fresh := &rbacv1.RoleBinding{
 		Subjects: []rbacv1.Subject{
@@ -738,3 +748,12 @@ func getCondition(job *batchv1.Job, conditionType batchv1.JobConditionType) (con
 	}
 	return
 }
+
+func containsRule(rules []rbacv1.PolicyRule, rule rbacv1.PolicyRule) bool {
+	for _, r := range rules {
+		if equality.Semantic.DeepDerivative(r, rule) {
+			return true
+		}
+	}
+	return false
+}

pkg/controller/bundle/bundle_unpacker_test.go

@@ -353,7 +353,7 @@ func TestConfigMapUnpacker(t *testing.T) {
 									"",
 								},
 								Verbs: []string{
-									"create", "get", "update",
+									"get", "update",
 								},
 								Resources: []string{
 									"configmaps",
@@ -769,7 +769,7 @@ func TestConfigMapUnpacker(t *testing.T) {
 									"",
 								},
 								Verbs: []string{
-									"create", "get", "update",
+									"get", "update",
 								},
 								Resources: []string{
 									"configmaps",