Skip to content

Commit 15b9378

Browse files
michaellzcmichaellzc
authored
feat(executors-k8s): improve handling for security context (#835)
ref PLAT-463 unfortunatelly `executor.securityConext.privieleged` (defaults to `false` now) is impossible to be backward compatible, so we make sure the same default is carryover to `executor.containerSecurityContext.` which has a default of `privileged=false`. ### Checklist - [x] Follow the [manual testing process](https://github.com/sourcegraph/deploy-sourcegraph-helm/blob/main/TEST.md) - [x] Update [changelog](https://github.com/sourcegraph/deploy-sourcegraph-helm/blob/main/charts/sourcegraph/CHANGELOG.md) - [x] Update [Kubernetes update doc](https://docs.sourcegraph.com/admin/updates/kubernetes)
1 parent 73050a9 commit 15b9378

File tree

5 files changed

+132
-22
lines changed

5 files changed

+132
-22
lines changed

‎charts/sourcegraph-executor/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@
55

66
* Added new chart `sourcegraph-executor-k8s` to deploy Sourcegraph executors that use Kubernetes jobs.
77
* **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.
8+
- **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‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ In addition to the documented values, the `executor` and `private-docker-registr
5454
|-----|------|---------|-------------|
5555
| executor.affinity | object | `{}` | Affinity, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/scheduling-eviction/assign-pod-node/#affinity-and-anti-affinity) |
5656
| executor.configureRbac | bool | `true` | Whether to configure the necessary RBAC resources. Required only once for all executor deployments. |
57+
| 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) |
5758
| 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. |
5859
| executor.debug.keepWorkspaces | string | `"false"` | |
5960
| 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
8687
| executor.maximumRuntimePerJob | string | `"30m"` | |
8788
| executor.namespace | string | `"default"` | The namespace in which jobs are generated by the executor. |
8889
| executor.nodeSelector | object | `{}` | NodeSelector, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/configuration/assign-pod-node/#nodeselector) |
90+
| 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) |
8991
| executor.queueName | string | `""` | The name of the queue to pull jobs from to. Possible values: batches and codeintel. **Either this or queueNames is required.** |
9092
| executor.queueNames | list | `[]` | The names of multiple queues to pull jobs from to. Possible values: batches and codeintel. **Either this or queueName is required.** |
9193
| executor.replicas | int | `1` | |
9294
| executor.resources.limits.cpu | string | `"1"` | |
9395
| executor.resources.limits.memory | string | `"1Gi"` | |
9496
| executor.resources.requests.cpu | string | `"500m"` | |
9597
| executor.resources.requests.memory | string | `"200Mi"` | |
96-
| executor.securityContext | object | `{"fsGroup":null,"privileged":false,"runAsGroup":null,"runAsUser":null}` | The containerSecurityContext for the executor image |
98+
| executor.securityContext | object | `{"fsGroup":null,"privileged":false,"runAsGroup":null,"runAsUser":null}` | DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead. |
99+
| executor.securityContext.fsGroup | string | `nil` | DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead. |
100+
| executor.securityContext.privileged | bool | `false` | DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead. |
101+
| executor.securityContext.runAsGroup | string | `nil` | DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead. |
102+
| executor.securityContext.runAsUser | string | `nil` | DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead. |
97103
| executor.storageSize | string | `"10Gi"` | The storage size of the PVC attached to the executor deployment. |
98104
| executor.tolerations | list | `[]` | Tolerations, learn more from the [Kubernetes documentation](https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/) |
99105
| 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‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,16 +44,26 @@ spec:
4444
{{- include "executor.labels" . | nindent 8 }}
4545
spec:
4646
securityContext:
47-
fsGroup: {{ .Values.executor.securityContext.fsGroup }}
48-
runAsUser: {{ .Values.executor.securityContext.runAsUser }}
49-
runAsGroup: {{ .Values.executor.securityContext.runAsGroup }}
47+
{{- if .Values.executor.podSecurityContext }}
48+
{{- toYaml .Values.executor.podSecurityContext | nindent 8 }}
49+
{{- else }}
50+
{{- with .Values.executor.securityContext.fsGroup }}
51+
fsGroup: {{ . }}
52+
{{- end }}
53+
{{- with .Values.executor.securityContext.runAsUser }}
54+
runAsUser: {{ . }}
55+
{{- end }}
56+
{{- with .Values.executor.securityContext.runAsGroup }}
57+
runAsGroup: {{ . }}
58+
{{- end }}
59+
{{- end }}
5060
serviceAccountName: sg-executor
5161
containers:
5262
- name: executor
5363
image: {{ include "sourcegraph.image" (list . "executor") }}
5464
imagePullPolicy: {{ .Values.sourcegraph.image.pullPolicy }}
5565
securityContext:
56-
privileged: {{ .Values.executor.securityContext.privileged }}
66+
{{- toYaml .Values.executor.containerSecurityContext | nindent 12 }}
5767
ports:
5868
- name: http-debug
5969
containerPort: 6060

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

Lines changed: 97 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,7 @@ tests:
88
- it: should render the Deployment, Service, ConfigMap, PVC if executor is enabled
99
set:
1010
executor:
11-
enabled: true
1211
queueName: "test"
13-
rbac:
14-
enabled: true
1512
asserts:
1613
- containsDocument:
1714
kind: Deployment
@@ -34,15 +31,103 @@ tests:
3431
name: sg-executor-test
3532
template: executor.PersistentVolumeClaim.yaml
3633

37-
- it: should not render any resources if executor is disabled
34+
- it: should render default containerSecurityContext with privileged false
35+
template: executor.Deployment.yaml
3836
set:
3937
executor:
40-
enabled: false
41-
rbac:
42-
enabled: false
38+
queueName: "test"
39+
asserts:
40+
- equal:
41+
path: spec.template.spec.containers[0].securityContext.privileged
42+
value: false
43+
44+
- it: should render custom containerSecurityContext
45+
template: executor.Deployment.yaml
46+
set:
47+
executor:
48+
queueName: "test"
49+
containerSecurityContext:
50+
privileged: true
51+
runAsUser: 1000
52+
runAsNonRoot: true
53+
allowPrivilegeEscalation: false
54+
asserts:
55+
- equal:
56+
path: spec.template.spec.containers[0].securityContext
57+
value:
58+
privileged: true
59+
runAsUser: 1000
60+
runAsNonRoot: true
61+
allowPrivilegeEscalation: false
62+
63+
- it: should render podSecurityContext when set
64+
template: executor.Deployment.yaml
65+
set:
66+
executor:
67+
queueName: "test"
68+
podSecurityContext:
69+
fsGroup: 2000
70+
runAsUser: 1000
71+
runAsGroup: 3000
72+
asserts:
73+
- equal:
74+
path: spec.template.spec.securityContext
75+
value:
76+
fsGroup: 2000
77+
runAsUser: 1000
78+
runAsGroup: 3000
79+
80+
- it: should fall back to legacy securityContext fields when podSecurityContext is empty
81+
template: executor.Deployment.yaml
82+
set:
83+
executor:
84+
queueName: "test"
85+
podSecurityContext: {}
86+
securityContext:
87+
fsGroup: 1001
88+
runAsUser: 1001
89+
runAsGroup: 1001
90+
asserts:
91+
- equal:
92+
path: spec.template.spec.securityContext
93+
value:
94+
fsGroup: 1001
95+
runAsUser: 1001
96+
runAsGroup: 1001
97+
98+
- it: should not render legacy securityContext fields when podSecurityContext is set
99+
template: executor.Deployment.yaml
100+
set:
101+
executor:
102+
queueName: "test"
103+
podSecurityContext:
104+
fsGroup: 2000
105+
securityContext:
106+
fsGroup: 1001
107+
runAsUser: 1001
108+
runAsGroup: 1001
109+
asserts:
110+
- equal:
111+
path: spec.template.spec.securityContext.fsGroup
112+
value: 2000
113+
- isNull:
114+
path: spec.template.spec.securityContext.runAsUser
115+
- isNull:
116+
path: spec.template.spec.securityContext.runAsGroup
117+
118+
- it: should omit pod securityContext fields not set in legacy securityContext
119+
template: executor.Deployment.yaml
120+
set:
121+
executor:
122+
queueName: "test"
123+
podSecurityContext: {}
124+
securityContext:
125+
fsGroup: 500
43126
asserts:
44-
- hasDocuments:
45-
count: 0
46-
templates:
47-
- executor.Deployment.yaml
48-
- executor.Service.yaml
127+
- equal:
128+
path: spec.template.spec.securityContext.fsGroup
129+
value: 500
130+
- isNull:
131+
path: spec.template.spec.securityContext.runAsUser
132+
- isNull:
133+
path: spec.template.spec.securityContext.runAsGroup

‎charts/sourcegraph-executor/k8s/values.yaml‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,15 +101,15 @@ executor:
101101
namespace: "default"
102102
# -- The path to the kubeconfig file. If not specified, the in-cluster config is used.
103103
kubeconfigPath: ""
104-
# -- The containerSecurityContext for the executor image
104+
# -- DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead.
105105
securityContext:
106-
# @default -- nil; accepts [0, 2147483647]
106+
# -- DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead.
107107
runAsUser:
108-
# @default -- nil; accepts [0, 2147483647]
108+
# -- DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead.
109109
runAsGroup:
110-
# @default -- nil; accepts [0, 2147483647]
110+
# -- DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead.
111111
fsGroup:
112-
# @default -- false; accepts [true, false]
112+
# -- DEPRECATED: Use `executor.containerSecurityContext` or `executor.podSecurityContext` instead.
113113
privileged: false
114114

115115
kubernetesJob:
@@ -177,3 +177,11 @@ executor:
177177

178178
# -- For local deployments the host is 'host.docker.internal' and this needs to be true
179179
dockerAddHostGateway: "false"
180+
181+
# -- Security context for the container,
182+
# learn more from the [Kubernetes documentation](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-container)
183+
containerSecurityContext:
184+
privileged: false
185+
# -- Security context for the pod,
186+
# learn more from the [Kubernetes documentation](https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-pod)
187+
podSecurityContext: {}

0 commit comments

Comments
 (0)