node-exporter: only render clusterrole if PSP is enabled (#203)

Author: ggilmore

charts/sourcegraph/README.md

@@ -192,7 +192,7 @@ In addition to the documented values, all services also support the following va
 | nodeExporter.podSecurityContext | object | `{"fsGroup":65534,"runAsGroup":65534,"runAsNonRoot":true,"runAsUser":65534}` | Security context for the `node-exporter` pod, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-pod) |
 | nodeExporter.podSecurityPolicy.enabled | bool | `false` | Enable [PodSecurityPolicy](https://kubernetes.io/docs/concepts/policy/pod-security-policy/) for `node-exporter` pods |
 | nodeExporter.resources | object | `{"limits":{"cpu":"1","memory":"1Gi"},"requests":{"cpu":".2","memory":"100Mi"}}` | Resource requests & limits for the `node-exporter` container, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/) |
-| nodeExporter.serviceAccount.create | bool | `true` | Enable creation of ServiceAccount for `node-exporter` |
+| nodeExporter.serviceAccount.create | bool | `false` | Enable creation of ServiceAccount for `node-exporter` |
 | nodeExporter.serviceAccount.name | string | `"node-exporter"` | Name of the ServiceAccount to be created or an existing ServiceAccount |
 | openTelemetry.agent.name | string | `"otel-agent"` | Name used by resources. Does not affect service names or PVCs. |
 | openTelemetry.agent.resources | object | `{"limits":{"cpu":"500m","memory":"500Mi"},"requests":{"cpu":"100m","memory":"100Mi"}}` | Resource requests & limits for the `otel-agent` container, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/) |

charts/sourcegraph/templates/node-exporter/node-exporter.ClusterRole.yaml

@@ -1,4 +1,4 @@
-{{- if .Values.nodeExporter.enabled -}}
+{{- if and .Values.nodeExporter.enabled .Values.nodeExporter.podSecurityPolicy.enabled  -}}
 kind: ClusterRole
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:

charts/sourcegraph/templates/node-exporter/node-exporter.ClusterRoleBinding.yaml

@@ -1,4 +1,4 @@
-{{- if .Values.nodeExporter.enabled -}}
+{{- if and .Values.nodeExporter.enabled .Values.nodeExporter.podSecurityPolicy.enabled  -}}
 kind: ClusterRoleBinding
 apiVersion: rbac.authorization.k8s.io/v1
 metadata:

charts/sourcegraph/templates/node-exporter/node-exporter.DaemonSet.yaml

@@ -41,7 +41,9 @@ spec:
         deploy: sourcegraph
         app: node-exporter
     spec:
+      {{- if .Values.nodeExporter.serviceAccount.create }}
       {{- include "sourcegraph.renderServiceAccountName" (list . "nodeExporter") | trim | nindent 6 }}
+      {{- end }}
       containers:
       - name: node-exporter
         image: {{ include "sourcegraph.image" (list . "nodeExporter" ) }}

charts/sourcegraph/templates/node-exporter/node-exporter.PodSecurityPolicy.yaml

@@ -1,4 +1,9 @@
 {{- if and .Values.nodeExporter.enabled .Values.nodeExporter.podSecurityPolicy.enabled  -}}
+
+{{- if not .Values.nodeExporter.serviceAccount.create -}}
+{{ fail "Node Exporter's service account must be enabled in order to use its pod security policy (set 'nodeExporter.ServiceAccount.create' to true)" }}
+{{- end -}}
+
 apiVersion: policy/v1beta1
 kind: PodSecurityPolicy
 metadata:

charts/sourcegraph/tests/nodeExporter_test.yaml

@@ -8,7 +8,7 @@ templates:
   - node-exporter/node-exporter.Service.yaml
   - node-exporter/node-exporter.ServiceAccount.yaml
 tests:
-  - it: should render the DaemonSet, Service, and ClusterRoles if node-exporter is enabled
+  - it: should render the DaemonSet and Service if node-exporter is enabled
     set: 
       nodeExporter:
         enabled: true
@@ -23,16 +23,6 @@ tests:
           apiVersion: v1
           name: node-exporter
         template: node-exporter/node-exporter.Service.yaml
-      - containsDocument:
-          kind: ClusterRole
-          apiVersion: rbac.authorization.k8s.io/v1
-          name: node-exporter
-        template: node-exporter/node-exporter.ClusterRole.yaml
-      - containsDocument:
-          kind: ClusterRoleBinding
-          apiVersion: rbac.authorization.k8s.io/v1
-          name: node-exporter
-        template: node-exporter/node-exporter.ClusterRoleBinding.yaml
 
   - it: should not render any resources if node-exporter is disabled
     set: 
@@ -49,9 +39,26 @@ tests:
       - node-exporter/node-exporter.Service.yaml
       - node-exporter/node-exporter.ServiceAccount.yaml
 
-  - it: should render the podSecurityPolicy if enabled
+  - it: should not render the PodSecurityPolicy, Service Accounts, and ClusterRoles by default
+    asserts:
+      - hasDocuments: 
+          count: 0
+        template: node-exporter/node-exporter.PodSecurityPolicy.yaml
+      - hasDocuments: 
+          count: 0
+        template: node-exporter/node-exporter.ClusterRole.yaml
+      - hasDocuments: 
+            count: 0
+        template: node-exporter/node-exporter.ClusterRoleBinding.yaml
+      - hasDocuments: 
+            count: 0
+        template: node-exporter/node-exporter.ServiceAccount.yaml
+
+  - it: should render the PodSecurityPolicy, ServiceAccounts, and ClusterRoles if PodSecurityPolicy + Service Accounts are enabled
     set: 
       nodeExporter:
+        serviceAccount:
+          create: true
         podSecurityPolicy: 
           enabled: true
     asserts:
@@ -60,6 +67,56 @@ tests:
           apiVersion: policy/v1beta1
           name: node-exporter
         template: node-exporter/node-exporter.PodSecurityPolicy.yaml
+      - containsDocument:
+          kind: ServiceAccount
+          apiVersion: v1
+          name: node-exporter
+        template: node-exporter/node-exporter.ServiceAccount.yaml
+      - containsDocument:
+          kind: ClusterRole
+          apiVersion: rbac.authorization.k8s.io/v1
+          name: node-exporter
+        template: node-exporter/node-exporter.ClusterRole.yaml
+      - containsDocument:
+          kind: ClusterRoleBinding
+          apiVersion: rbac.authorization.k8s.io/v1
+          name: node-exporter
+        template: node-exporter/node-exporter.ClusterRoleBinding.yaml
+  
+  - it: should add the ServiceAccount name to the DaemonSet spec if the ServiceAccount is enabled
+    set: 
+      nodeExporter:
+        serviceAccount:
+          create: true
+          name: "test-service-account-name"
+    asserts:
+      - equal:
+          path: spec.template.spec.serviceAccountName
+          value: "test-service-account-name"
+        template: node-exporter/node-exporter.DaemonSet.yaml
+    
+  - it: should add not the ServiceAccount name to the DaemonSet spec if the ServiceAccount is disabled
+    set: 
+      nodeExporter:
+        serviceAccount:
+          create: false
+          name: "test-service-account-name"
+    asserts:
+      - isEmpty:
+          path: spec.template.spec.serviceAccountName
+        template: node-exporter/node-exporter.DaemonSet.yaml
+  
+  - it: should fail to render if the PodSecurityPolicy is enabled, but the ServiceAccount isn't
+    set: 
+      nodeExporter:
+        serviceAccount:
+          create: false
+        podSecurityPolicy: 
+          enabled: true
+    asserts:
+      - failedTemplate:
+          errorMessage: "Node Exporter's service account must be enabled in order to use its pod security policy (set 'nodeExporter.ServiceAccount.create' to true)"
+        template: node-exporter/node-exporter.PodSecurityPolicy.yaml
 
   - it: should not render the podSecurityPolicy if disabled 
     set: 
@@ -74,6 +131,12 @@ tests:
   - it: should ensure that the namespace is properly propagated to the cluster role binding 
     release:
       namespace: "my-test-namespace"
+    set:
+      nodeExporter:
+        serviceAccount:
+          create: true
+        podSecurityPolicy:
+          enabled: true
     asserts:
       - equal:
           path: subjects[0].namespace
@@ -83,7 +146,10 @@ tests:
   - it: should have host Network and PID enabled by default
     set:
       nodeExporter:
-        podSecurityPolicy: # (unrelated to host network/pid defaults, just for ensuring that PodSecurityPolicy gets rendered so that we can check them in same test)
+        # (these settings are unrelated to host network/pid defaults, they're just for ensuring that PodSecurityPolicy gets rendered so that we can check them in same test)
+        serviceAccount:
+          create: true
+        podSecurityPolicy:
           enabled: true
     asserts:  
       - equal: 
@@ -108,7 +174,10 @@ tests:
       nodeExporter:
         hostNetwork: false 
         hostPID: false
-        podSecurityPolicy: # (unrelated to host network/pid settings, just for ensuring that PodSecurityPolicy gets rendered so that we can check them in same test)
+        # (these settings are unrelated to host network/pid defaults, they're just for ensuring that PodSecurityPolicy gets rendered so that we can check them in same test)
+        serviceAccount:
+          create: true
+        podSecurityPolicy:
           enabled: true
     asserts:  
       - equal: 

charts/sourcegraph/values.yaml

@@ -686,7 +686,7 @@ nodeExporter:
       memory: 100Mi
   serviceAccount:
     # -- Enable creation of ServiceAccount for `node-exporter`
-    create: true
+    create: false
     # -- Name of the ServiceAccount to be created or an existing ServiceAccount
     name: node-exporter
   # Share the host process ID namespace.