feat(executors-k8s): improve handling for security context

Author: michaellzc

charts/sourcegraph-executor/CHANGELOG.md

@@ -5,3 +5,4 @@
 
 * Added new chart `sourcegraph-executor-k8s` to deploy Sourcegraph executors that use Kubernetes jobs.
 * **BREAKING:** Renamed `sourcegraph-executor` chart to `sourcegraph-executor-dind` to indicate these are Docker in Docker executors. To update to newer versions of this chart, ensure the new Chart name is used.
+- **BREAKING:** The `securityContext` field in the `sourcegraph-executor-k8s` chart is now deprecated. Use `containerSecurityContext` or `podSecurityContext` instead. The `privileged` field has been moved to `containerSecurityContext`. To update to newer versions of this chart, ensure the new fields are used and the deprecated `securityContext` field is removed.

charts/sourcegraph-executor/k8s/README.md

@@ -54,6 +54,7 @@ In addition to the documented values, the `executor` and `private-docker-registr
 |-----|------|---------|-------------|
 | executor.affinity | object | `{}` | Affinity, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#affinity-and-anti-affinity) |
 | executor.configureRbac | bool | `true` | Whether to configure the necessary RBAC resources. Required only once for all executor deployments. |
+| executor.containerSecurityContext | object | `{"privileged":false}` | Security context for the container, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-container) |
 | executor.debug.keepJobs | string | `"false"` | If true, Kubernetes jobs will not be deleted after they complete. Not recommended for production use as it can hit cluster limits. |
 | executor.debug.keepWorkspaces | string | `"false"` |  |
 | executor.dockerAddHostGateway | string | `"false"` | For local deployments the host is 'host.docker.internal' and this needs to be true |
@@ -86,14 +87,19 @@ In addition to the documented values, the `executor` and `private-docker-registr
 | executor.maximumRuntimePerJob | string | `"30m"` |  |
 | executor.namespace | string | `"default"` | The namespace in which jobs are generated by the executor. |
 | executor.nodeSelector | object | `{}` | NodeSelector, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/configuration/assign-pod-node/#nodeselector) |
+| executor.podSecurityContext | object | `{}` | Security context for the pod, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-pod) |
 | executor.queueName | string | `""` | The name of the queue to pull jobs from to. Possible values: batches and codeintel. **Either this or queueNames is required.** |
 | executor.queueNames | list | `[]` | The names of multiple queues to pull jobs from to. Possible values: batches and codeintel. **Either this or queueName is required.** |
 | executor.replicas | int | `1` |  |
 | executor.resources.limits.cpu | string | `"1"` |  |
 | executor.resources.limits.memory | string | `"1Gi"` |  |
 | executor.resources.requests.cpu | string | `"500m"` |  |
 | executor.resources.requests.memory | string | `"200Mi"` |  |
-| executor.securityContext | object | `{"fsGroup":null,"privileged":false,"runAsGroup":null,"runAsUser":null}` | The containerSecurityContext for the executor image |
+| executor.securityContext | object | `{"fsGroup":null,"privileged":false,"runAsGroup":null,"runAsUser":null}` | DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead. |
+| executor.securityContext.fsGroup | string | `nil` | DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead. |
+| executor.securityContext.privileged | bool | `false` | DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead. |
+| executor.securityContext.runAsGroup | string | `nil` | DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead. |
+| executor.securityContext.runAsUser | string | `nil` | DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead. |
 | executor.storageSize | string | `"10Gi"` | The storage size of the PVC attached to the executor deployment. |
 | executor.tolerations | list | `[]` | Tolerations, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/) |
 | sourcegraph.affinity | object | `{}` | Affinity, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#affinity-and-anti-affinity) |

charts/sourcegraph-executor/k8s/templates/executor.Deployment.yaml

@@ -44,16 +44,26 @@ spec:
         {{- include "executor.labels" . | nindent 8 }}
     spec:
       securityContext:
-        fsGroup: {{ .Values.executor.securityContext.fsGroup }}
-        runAsUser: {{ .Values.executor.securityContext.runAsUser }}
-        runAsGroup: {{ .Values.executor.securityContext.runAsGroup }}
+        {{- if .Values.executor.podSecurityContext }}
+        {{- toYaml .Values.executor.podSecurityContext | nindent 8 }}
+        {{- else }}
+        {{- with .Values.executor.securityContext.fsGroup }}
+        fsGroup: {{ . }}
+        {{- end }}
+        {{- with .Values.executor.securityContext.runAsUser }}
+        runAsUser: {{ . }}
+        {{- end }}
+        {{- with .Values.executor.securityContext.runAsGroup }}
+        runAsGroup: {{ . }}
+        {{- end }}
+        {{- end }}
       serviceAccountName: sg-executor
       containers:
         - name: executor
           image: {{ include "sourcegraph.image" (list . "executor") }}
           imagePullPolicy: {{ .Values.sourcegraph.image.pullPolicy }}
           securityContext:
-            privileged: {{ .Values.executor.securityContext.privileged }}
+            {{- toYaml .Values.executor.containerSecurityContext | nindent 12 }}
           ports:
             - name: http-debug
               containerPort: 6060

charts/sourcegraph-executor/k8s/tests/executor_test.yaml

@@ -8,10 +8,7 @@ tests:
   - it: should render the Deployment, Service, ConfigMap, PVC if executor is enabled
     set:
       executor:
-        enabled: true
         queueName: "test"
-      rbac:
-        enabled: true
     asserts:
       - containsDocument:
           kind: Deployment
@@ -34,15 +31,103 @@ tests:
           name: sg-executor-test
         template: executor.PersistentVolumeClaim.yaml                
 
-  - it: should not render any resources if executor is disabled
+  - it: should render default containerSecurityContext with privileged false
+    template: executor.Deployment.yaml
     set:
       executor:
-        enabled: false
-      rbac:
-        enabled: false
+        queueName: "test"
+    asserts:
+      - equal:
+          path: spec.template.spec.containers[0].securityContext.privileged
+          value: false
+
+  - it: should render custom containerSecurityContext
+    template: executor.Deployment.yaml
+    set:
+      executor:
+        queueName: "test"
+        containerSecurityContext:
+          privileged: true
+          runAsUser: 1000
+          runAsNonRoot: true
+          allowPrivilegeEscalation: false
+    asserts:
+      - equal:
+          path: spec.template.spec.containers[0].securityContext
+          value:
+            privileged: true
+            runAsUser: 1000
+            runAsNonRoot: true
+            allowPrivilegeEscalation: false
+
+  - it: should render podSecurityContext when set
+    template: executor.Deployment.yaml
+    set:
+      executor:
+        queueName: "test"
+        podSecurityContext:
+          fsGroup: 2000
+          runAsUser: 1000
+          runAsGroup: 3000
+    asserts:
+      - equal:
+          path: spec.template.spec.securityContext
+          value:
+            fsGroup: 2000
+            runAsUser: 1000
+            runAsGroup: 3000
+
+  - it: should fall back to legacy securityContext fields when podSecurityContext is empty
+    template: executor.Deployment.yaml
+    set:
+      executor:
+        queueName: "test"
+        podSecurityContext: {}
+        securityContext:
+          fsGroup: 1001
+          runAsUser: 1001
+          runAsGroup: 1001
+    asserts:
+      - equal:
+          path: spec.template.spec.securityContext
+          value:
+            fsGroup: 1001
+            runAsUser: 1001
+            runAsGroup: 1001
+
+  - it: should not render legacy securityContext fields when podSecurityContext is set
+    template: executor.Deployment.yaml
+    set:
+      executor:
+        queueName: "test"
+        podSecurityContext:
+          fsGroup: 2000
+        securityContext:
+          fsGroup: 1001
+          runAsUser: 1001
+          runAsGroup: 1001
+    asserts:
+      - equal:
+          path: spec.template.spec.securityContext.fsGroup
+          value: 2000
+      - isNull:
+          path: spec.template.spec.securityContext.runAsUser
+      - isNull:
+          path: spec.template.spec.securityContext.runAsGroup
+
+  - it: should omit pod securityContext fields not set in legacy securityContext
+    template: executor.Deployment.yaml
+    set:
+      executor:
+        queueName: "test"
+        podSecurityContext: {}
+        securityContext:
+          fsGroup: 500
     asserts:
-      - hasDocuments:
-          count: 0
-    templates:
-      - executor.Deployment.yaml
-      - executor.Service.yaml
+      - equal:
+          path: spec.template.spec.securityContext.fsGroup
+          value: 500
+      - isNull:
+          path: spec.template.spec.securityContext.runAsUser
+      - isNull:
+          path: spec.template.spec.securityContext.runAsGroup

charts/sourcegraph-executor/k8s/values.yaml

@@ -101,15 +101,15 @@ executor:
   namespace: "default"
   # -- The path to the kubeconfig file. If not specified, the in-cluster config is used.
   kubeconfigPath: ""
-  # -- The containerSecurityContext for the executor image
+  # -- DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead.
   securityContext:
-    # @default -- nil; accepts [0, 2147483647]
+    # -- DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead.
     runAsUser:
-    # @default -- nil; accepts [0, 2147483647]
+    # -- DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead.
     runAsGroup:
-    # @default -- nil; accepts [0, 2147483647]
+    # -- DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead.
     fsGroup:
-    # @default -- false; accepts [true, false]
+    # -- DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead.
     privileged: false
 
   kubernetesJob:
@@ -177,3 +177,11 @@ executor:
 
   # -- For local deployments the host is 'host.docker.internal' and this needs to be true
   dockerAddHostGateway: "false"
+
+  # -- Security context for the container,
+  # learn more from the [Kubernetes documentation](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-container)
+  containerSecurityContext:
+    privileged: false
+  # -- Security context for the pod,
+  # learn more from the [Kubernetes documentation](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-pod)
+  podSecurityContext: {}