Unpack job security updates (operator-framework#2805)

* Remove unneeded 'get' verb from bundle unpacker role

Signed-off-by: perdasilva <perdasilva@redhat.com>

* Move bundle unpacker policy rule creation to its own method

Signed-off-by: perdasilva <perdasilva@redhat.com>

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",