NET-10871 Remove unnecessary resource permissions in connect-inject C…

…lusterRole (#4307) * Remove unnecessary resource permissions in connect-inject ClusterRole * Add changelog entry * Update unit test coverage * Update broken unit test * Update broken unit test * Update broken unit test
hashicorp · Sep 6, 2024 · 8bfb601 · 8bfb601
1 parent 82ed2e7
commit 8bfb601
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 11 deletions.
diff --git a/.changelog/4307.txt b/.changelog/4307.txt
@@ -0,0 +1,3 @@
+```release-note:improvement
+connect-inject: remove unnecessary resource permissions from connect-inject ClusterRole
+```
diff --git a/charts/consul/templates/connect-inject-clusterrole.yaml b/charts/consul/templates/connect-inject-clusterrole.yaml
@@ -156,15 +156,21 @@ rules:
     - update
     - watch
 {{- end }}
+- apiGroups: [""]
+  resources: ["secrets", "serviceaccounts", "services"]
+  verbs:
+    - get
+    - list
+    - watch
+    - delete
+    - create
+    - update
 - apiGroups: [ "" ]
-  resources: [ "secrets", "serviceaccounts", "endpoints", "services", "namespaces", "nodes" ]
+  resources: ["endpoints", "namespaces", "nodes"]
   verbs:
-  - create
   - get
   - list
   - watch
-  - delete
-  - update
 - apiGroups: [ "rbac.authorization.k8s.io" ]
   resources: [ "roles", "rolebindings" ]
   verbs:

diff --git a/charts/consul/test/unit/connect-inject-clusterrole.bats b/charts/consul/test/unit/connect-inject-clusterrole.bats
@@ -34,7 +34,7 @@ load _helpers
 #--------------------------------------------------------------------
 # rules
 
-@test "connectInject/ClusterRole: sets get, list, and watch access to endpoints, services, namespaces and nodes in all api groups" {
+@test "connectInject/ClusterRole: sets get, list, watch, delete, create, and update access to secrets, serviceaccounts and services in core api group" {
   cd `chart_dir`
   local object=$(helm template \
       -s templates/connect-inject-clusterrole.yaml  \
@@ -44,12 +44,50 @@ load _helpers
       . | tee /dev/stderr |
       yq -r '.rules[2]' | tee /dev/stderr)
 
-  local actual=$(echo $object | yq -r '.resources[| index("endpoints")' | tee /dev/stderr)
+  local actual=$(echo $object | yq -r '.resources[| index("secrets")' | tee /dev/stderr)
+  [ "${actual}" != null ]
+
+  local actual=$(echo $object | yq -r '.resources[| index("serviceaccounts")' | tee /dev/stderr)
   [ "${actual}" != null ]
 
   local actual=$(echo $object | yq -r '.resources[| index("services")' | tee /dev/stderr)
   [ "${actual}" != null ]
 
+  local actual=$(echo $object | yq -r '.apiGroups[0]' | tee /dev/stderr)
+  [ "${actual}" = "" ]
+
+  local actual=$(echo $object | yq -r '.verbs | index("get")' | tee /dev/stderr)
+  [ "${actual}" != null ]
+
+  local actual=$(echo $object | yq -r '.verbs | index("list")' | tee /dev/stderr)
+  [ "${actual}" != null ]
+
+  local actual=$(echo $object | yq -r '.verbs | index("watch")' | tee /dev/stderr)
+  [ "${actual}" != null ]
+
+  local actual=$(echo $object | yq -r '.verbs | index("delete")' | tee /dev/stderr)
+  [ "${actual}" != null ]
+
+  local actual=$(echo $object | yq -r '.verbs | index("create")' | tee /dev/stderr)
+  [ "${actual}" != null ]
+
+  local actual=$(echo $object | yq -r '.verbs | index("update")' | tee /dev/stderr)
+  [ "${actual}" != null ]
+}
+
+@test "connectInject/ClusterRole: sets get, list, and watch access to endpoints, namespaces and nodes in core api group" {
+  cd `chart_dir`
+  local object=$(helm template \
+      -s templates/connect-inject-clusterrole.yaml  \
+      --set 'global.enabled=false' \
+      --set 'client.enabled=true' \
+      --set 'connectInject.enabled=true' \
+      . | tee /dev/stderr |
+      yq -r '.rules[3]' | tee /dev/stderr)
+
+  local actual=$(echo $object | yq -r '.resources[| index("endpoints")' | tee /dev/stderr)
+  [ "${actual}" != null ]
+
   local actual=$(echo $object | yq -r '.resources[| index("namespaces")' | tee /dev/stderr)
   [ "${actual}" != null ]
 
@@ -77,7 +115,7 @@ load _helpers
       --set 'client.enabled=true' \
       --set 'connectInject.enabled=true' \
       . | tee /dev/stderr |
-      yq -r '.rules[4]' | tee /dev/stderr)
+      yq -r '.rules[5]' | tee /dev/stderr)
 
   local actual=$(echo $object | yq -r '.resources[| index("pods")' | tee /dev/stderr)
   [ "${actual}" != null ]
@@ -106,7 +144,7 @@ load _helpers
       --set 'client.enabled=true' \
       --set 'connectInject.enabled=true' \
       . | tee /dev/stderr |
-      yq -r '.rules[5]' | tee /dev/stderr)
+      yq -r '.rules[6]' | tee /dev/stderr)
 
   local actual=$(echo $object | yq -r '.resources[| index("leases")' | tee /dev/stderr)
   [ "${actual}" != null ]
@@ -197,7 +235,7 @@ load _helpers
       --set 'global.secretsBackend.vault.consulServerRole=bar' \
       --set 'global.secretsBackend.vault.consulCARole=test2' \
       . | tee /dev/stderr |
-      yq -r '.rules[6]' | tee /dev/stderr)
+      yq -r '.rules[7]' | tee /dev/stderr)
 
   local actual=$(echo $object | yq -r '.resources[0]' | tee /dev/stderr)
   [ "${actual}" = "mutatingwebhookconfigurations" ]
@@ -227,7 +265,7 @@ load _helpers
       -s templates/connect-inject-clusterrole.yaml  \
       --set 'global.openshift.enabled=true' \
       . | tee /dev/stderr |
-      yq '.rules[13].resourceNames | index("restricted-v2")' | tee /dev/stderr)
+      yq '.rules[14].resourceNames | index("restricted-v2")' | tee /dev/stderr)
   [ "${object}" == 0 ]
 }
 
@@ -238,7 +276,7 @@ load _helpers
       --set 'global.openshift.enabled=true' \
       --set 'connectInject.apiGateway.managedGatewayClass.openshiftSCCName=fakescc' \
       . | tee /dev/stderr |
-       yq '.rules[13].resourceNames | index("fakescc")' | tee /dev/stderr)
+       yq '.rules[14].resourceNames | index("fakescc")' | tee /dev/stderr)
    [ "${object}" == 0 ]
 }