Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions charts/retool/ci/test-seccomp-install-disabled-option.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Exercises codeExecutor.installSeccompProfile / rr.jsExecutor.installSeccompProfile
# set to false: the chart must render the seccompProfile.localhostProfile reference
# but omit the install-seccomp initContainer, its ConfigMap, and the hostPath volume.
# Overlaid on top of test-install-values.yaml.

# Enable the code-executor (image.tag "latest" in the base does not match the
# workflows semver gate, so enable it explicitly).
workflows:
enabled: true

codeExecutor:
useSeccompProfile: true
installSeccompProfile: false

rr:
jsExecutor:
enabled: true
installSeccompProfile: false
2 changes: 1 addition & 1 deletion charts/retool/templates/configmap_code_executor.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{{- if include "retool.workflows.enabled" . }}
{{- if and (not .Values.codeExecutor.securityContext) .Values.codeExecutor.useSeccompProfile }}
{{- if and (not .Values.codeExecutor.securityContext) .Values.codeExecutor.useSeccompProfile .Values.codeExecutor.installSeccompProfile }}
apiVersion: v1
kind: ConfigMap
metadata:
Expand Down
2 changes: 2 additions & 0 deletions charts/retool/templates/configmap_js_executor.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
{{- if eq (include "retool.rr.componentEnabled" (dict "root" $ "component" "jsExecutor")) "1" }}
{{- if .Values.rr.jsExecutor.installSeccompProfile }}
apiVersion: v1
kind: ConfigMap
metadata:
Expand All @@ -9,3 +10,4 @@ data:
nsjail-seccomp.json: |
{{- .Files.Get "files/nsjail-seccomp.json" | nindent 4 }}
{{- end }}
{{- end }}
8 changes: 5 additions & 3 deletions charts/retool/templates/deployment_code_executor.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
{{- if include "retool.workflows.enabled" . }}
{{- /* Use the less-privileged seccomp sandbox (see codeExecutor.useSeccompProfile in values.yaml) only when it is enabled and no explicit codeExecutor.securityContext is set. */ -}}
{{- $useSecComp := and (not .Values.codeExecutor.securityContext) .Values.codeExecutor.useSeccompProfile -}}
{{- /* Install the seccomp profile via initContainer only when the sandbox is enabled and the admin hasn't opted to manage the profile out of band. */ -}}
{{- $installSeccomp := and $useSecComp .Values.codeExecutor.installSeccompProfile -}}
apiVersion: apps/v1
kind: Deployment
metadata:
Expand Down Expand Up @@ -53,9 +55,9 @@ spec:
{{- if $useSecComp }}
hostUsers: false
{{- end }}
{{- if or $useSecComp .Values.initContainers }}
{{- if or $installSeccomp .Values.initContainers }}
initContainers:
{{- if $useSecComp }}
{{- if $installSeccomp }}
- name: install-seccomp
image: busybox:1.37.0@sha256:b3255e7dfbcd10cb367af0d409747d511aeb66dfac98cf30e97e87e4207dd76f
securityContext:
Expand Down Expand Up @@ -185,7 +187,7 @@ spec:
{{ tpl . $ | indent 6 }}
{{- end }}
volumes:
{{- if $useSecComp }}
{{- if $installSeccomp }}
- name: seccomp-profile
configMap:
name: {{ template "retool.fullname" . }}-code-executor-seccomp
Expand Down
6 changes: 5 additions & 1 deletion charts/retool/templates/deployment_js_executor.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ spec:
{{- if .Values.priorityClassName }}
priorityClassName: "{{ .Values.priorityClassName }}"
{{- end }}
{{- if or .Values.rr.jsExecutor.installSeccompProfile .Values.initContainers }}
initContainers:
{{- if .Values.rr.jsExecutor.installSeccompProfile }}
- name: install-seccomp
image: busybox:1.37.0@sha256:b3255e7dfbcd10cb367af0d409747d511aeb66dfac98cf30e97e87e4207dd76f
securityContext:
Expand Down Expand Up @@ -74,7 +76,7 @@ spec:
mountPath: /seccomp-profile
- name: host-seccomp
mountPath: /host-seccomp
{{- if .Values.initContainers }}
{{- end }}
{{- range $key, $value := .Values.initContainers }}
- name: "{{ $key }}"
{{ toYaml $value | indent 10 }}
Expand Down Expand Up @@ -158,13 +160,15 @@ spec:
{{ tpl . $ | indent 6 }}
{{- end }}
volumes:
{{- if .Values.rr.jsExecutor.installSeccompProfile }}
- name: seccomp-profile
configMap:
name: {{ template "retool.fullname" . }}-js-executor-seccomp
- name: host-seccomp
hostPath:
path: /var/lib/kubelet/seccomp
type: DirectoryOrCreate
{{- end }}
Comment on lines 162 to +171

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 volumes: key can render with no items

Before this PR the seccomp-profile and host-seccomp entries were always emitted, so volumes: always had at least two items. With installSeccompProfile: false and no user-defined rr.jsExecutor.volumes, extraConfigMapMounts, or extraVolumes, the volumes: key is now rendered with nothing beneath it (volumes: null in the parsed YAML). The Kubernetes API treats null as an empty list and will accept the pod spec, so this won't break deployments, but it is inconsistent with the explicit-list convention used elsewhere. Consider wrapping the volumes: key itself in an outer conditional, or at minimum emitting volumes: [] in the empty case.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

{{- if .Values.rr.jsExecutor.volumes }}
{{ toYaml .Values.rr.jsExecutor.volumes | indent 8 }}
{{- end }}
Expand Down
16 changes: 16 additions & 0 deletions charts/retool/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,14 @@ codeExecutor:
useSeccompProfile: false
seccompLocalhostProfile: profiles/nsjail-seccomp.json

# Only has an effect when useSeccompProfile is true. Set to false if a cluster
# admin installs the seccomp profile named by seccompLocalhostProfile out of band
# (baked into the node image, a separate DaemonSet, etc.). When false the chart
# skips the install-seccomp initContainer, its ConfigMap, and the hostPath mount,
# but the container still references the profile by that path -- which must already
# exist on every node, or the pod will fail to start.
installSeccompProfile: true

# === RR (Retool agent runtime) =============================================
# Master switch for the whole RR stack. Set `rr.enabled: true` to turn on the
# components nested below — jsExecutor, agent, and agentSandbox — with a
Expand Down Expand Up @@ -861,6 +869,14 @@ rr:

seccompLocalhostProfile: profiles/nsjail-seccomp.json

# Set to false if a cluster admin installs the seccomp profile named by
# seccompLocalhostProfile out of band (baked into the node image, a separate
# DaemonSet, etc.). When false the chart skips the install-seccomp initContainer,
# its ConfigMap, and the hostPath mount, but the container still references the
# profile by that path -- which must already exist on every node, or the pod will
# fail to start.
installSeccompProfile: true

# JS-executor-specific environment; not inherited from the top-level
# .Values.env / .Values.environmentSecrets / .Values.environmentVariables.
env: {}
Expand Down
16 changes: 16 additions & 0 deletions values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,14 @@ codeExecutor:
useSeccompProfile: false
seccompLocalhostProfile: profiles/nsjail-seccomp.json

# Only has an effect when useSeccompProfile is true. Set to false if a cluster
# admin installs the seccomp profile named by seccompLocalhostProfile out of band
# (baked into the node image, a separate DaemonSet, etc.). When false the chart
# skips the install-seccomp initContainer, its ConfigMap, and the hostPath mount,
# but the container still references the profile by that path -- which must already
# exist on every node, or the pod will fail to start.
installSeccompProfile: true

# === RR (Retool agent runtime) =============================================
# Master switch for the whole RR stack. Set `rr.enabled: true` to turn on the
# components nested below — jsExecutor, agent, and agentSandbox — with a
Expand Down Expand Up @@ -861,6 +869,14 @@ rr:

seccompLocalhostProfile: profiles/nsjail-seccomp.json

# Set to false if a cluster admin installs the seccomp profile named by
# seccompLocalhostProfile out of band (baked into the node image, a separate
# DaemonSet, etc.). When false the chart skips the install-seccomp initContainer,
# its ConfigMap, and the hostPath mount, but the container still references the
# profile by that path -- which must already exist on every node, or the pod will
# fail to start.
installSeccompProfile: true

# JS-executor-specific environment; not inherited from the top-level
# .Values.env / .Values.environmentSecrets / .Values.environmentVariables.
env: {}
Expand Down
Loading