Skip to content

EN-11853: expose standard Helm chart configuration options #152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
thefirstofthe300 merged 16 commits into from
Sep 23, 2026
Merged

EN-11853: expose standard Helm chart configuration options #152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
3bb62aa
chore: onboard Serena for this repository
thefirstofthe300 Sep 17, 2026
b9ffaba
docs: record reserved-name collision convention for chart extension v…
thefirstofthe300 Sep 17, 2026
feca15e
docs(EN-11853): add assented requirements and accepted design
thefirstofthe300 Sep 17, 2026
e7f28dc
test(EN-11853): add contract probes for standard Helm config values
thefirstofthe300 Sep 17, 2026
2169813
feat(EN-11853): seal contract at Gate 2
thefirstofthe300 Sep 18, 2026
d21b483
feat(EN-11853): add extension values and reserved-name validation
thefirstofthe300 Sep 18, 2026
8f67ca7
feat(EN-11853): implement gremlin DaemonSet config placements
thefirstofthe300 Sep 18, 2026
95a5997
feat(EN-11853): implement chao Deployment config placements
thefirstofthe300 Sep 18, 2026
73fe881
fix(EN-11853): correct seal.txt format and add validation dossier
thefirstofthe300 Sep 18, 2026
f62e409
fix(EN-11853): address review findings
thefirstofthe300 Sep 18, 2026
dd122cb
docs(EN-11853): re-validate against post-review commit
thefirstofthe300 Sep 18, 2026
76afd1f
test: rename test specs to generic, ticket-independent names
thefirstofthe300 Sep 22, 2026
7e9a0b2
merge: bring in master (PR #151 default-k8squery)
thefirstofthe300 Sep 22, 2026
95d15f4
docs: record test-naming convention in CLAUDE.md
thefirstofthe300 Sep 22, 2026
9f97e52
docs: record convention against internal ticket/design references
thefirstofthe300 Sep 23, 2026
32fd3d3
fix: address round-2 review findings (Copilot + human)
thefirstofthe300 Sep 23, 2026
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
3 changes: 3 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,6 @@
.iml

**/tests/__snapshot__/

.serena/cache/
reviews/
89 changes: 89 additions & 0 deletions CLAUDE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
# Gremlin Helm Charts — Repo Conventions

## Reserved-name collisions must fail fast

Any *new* chart value that lets a user **define** a named Kubernetes fragment alongside
chart-managed resources of the same kind — `extraVolumes`, `extraVolumeMounts`, `initContainers`,
and any future value of this shape — must fail the template render (via a `_validation.tpl`-style
`fail` check, included at the top of the resource template) when the user-supplied name collides
with a name the chart itself already owns.

This does not apply retroactively to `extraEnv`. It's pre-existing, and `extraEnv` renders last in
both containers' `env:` lists, so a collision there is Kubernetes' documented last-wins override
behavior — a real, working use case (e.g. overriding `GREMLIN_SERVICE_URL` or `https_proxy`), not
a bug. Retrofitting a hard failure onto it would break existing installs on their next
`helm upgrade` with no opt-in.

This also does not apply to `envFrom`: it *references* an existing ConfigMap/Secret by name rather
than defining a new object, and the chart cannot see that object's keys at render time, so there
is nothing to statically collide-check.

**The DaemonSet and Chao Deployment each get their own reserved list, not one shared list.** They
render different chart-managed resources, and checking a workload's `extraVolumes`/`initContainers`
against the *other* workload's names produces false rejections (Chao doesn't render `gremlin-state`,
the DaemonSet doesn't render `chao-tls-identity`, and so on).

Reserved names as of this writing:

- DaemonSet volumes: `gremlin-state`, `gremlin-executions`, `gremlin-logs`, `cgroup-root`,
`seccomp-root`, `seccomp-profile`, `gremlin-cert`, `ssl-cert-file`, `gremlin-tls-identity`,
`gremlin-opencl-icd`, `kfd`, `dri`, `opencl-vendors`, plus the container-driver socket volumes
(derived from `.Values.containerDrivers.*.name` at render time, not hardcoded — currently
`docker-sock`, `containerd-sock`, `crio-sock`).
- DaemonSet container names (for `initContainers`): `seccomp-init`, `gremlin`.
- Chao volumes: `gremlin-cert`, `ssl-cert-file`, `chao-tls-identity`.
- Chao container names (for `initContainers`): `chao`.
- Env vars (not currently checked — see `envFrom`/`extraEnv` above): `GREMLIN_TEAM_ID`,
`GREMLIN_TEAM_SECRET`, `GREMLIN_TEAM_CERTIFICATE_OR_FILE`, `GREMLIN_TEAM_PRIVATE_KEY_OR_FILE`,
`GREMLIN_IDENTIFIER`, `GREMLIN_CLIENT_TAGS`, `GREMLIN_COLLECT_DNS`, `GREMLIN_SERVICE_URL`,
`GREMLIN_PUSH_POD_CIDR_TAGS`, `GREMLIN_PUSH_ZONE_CIDR_TAGS`, `https_proxy`, `no_proxy`,
`SSL_CERT_FILE`, `SSL_CERT_DIR`, `GREMLIN_TLS_IDENTITY_CERTIFICATE`,
`GREMLIN_TLS_IDENTITY_PRIVATE_KEY`, `GREMLIN_CLUSTER_ID`, and any GPU vendor env vars (e.g.
`NVIDIA_VISIBLE_DEVICES`, `NVIDIA_DRIVER_CAPABILITIES`).

**Why:** a silent name collision either gets rejected by the Kubernetes API with an opaque error,
or silently shadows/duplicates a chart-managed resource — both are worse than a clear failure at
`helm template`/`helm install` time naming the conflicting value path.

**How to apply:** whenever a new chart value accepts user-supplied names in this shape, add its
name to the correct workload's reserved list (`gremlin.reservedNames.daemonset*` /
`.chao*` in `_validation.tpl`) and to this file's list above. Prefer deriving a reserved name from
its source value (as the container-driver sockets do) over hardcoding it, when the source is a
small, fixed-shape values block — hardcoding only the GPU vendor gap below because that block's
shape is genuinely open-ended.

**Known gap:** `gremlin.gpu.<vendor>` blocks are still static. A hand-configured vendor's
`volumes`/`volumeMounts` entries are not covered by the check — a collision there surfaces only as
a Kubernetes API rejection at apply time, not a chart-level failure. Whoever adds a vendor block
should add its names to the reserved list above.

## Name test files after the behavior, not the ticket

`gremlin/tests/*.yaml` files (and their `suite:` descriptions) describe what they test, not which
ticket introduced them — e.g. `extra_volumes_test.yaml` / `suite: extraVolumes / extraVolumeMounts`,
not `en-12345_extra_volumes_test.yaml` / `suite: EN-12345 - extraVolumes`.

**Why:** a ticket ID tells a future reader nothing about what broke, and outlives its usefulness
the moment the ticket is closed — the test itself is what has to stay legible.

**How to apply:** when adding tests for a ticket, name the file and suite for the behavior under
test, matching the existing style (`daemonset_resources_test.yaml`,
`chao_deployment_namespaces_test.yaml`, ...). Keep ticket context in the commit message and PR
description instead.

## Don't reference internal tickets or design docs in repo-committed files

This repo (and this file) is public-facing. Don't cite a Jira ticket key, an internal contract
slug, or a path like `.claude/contracts/<slug>/design.md` in `CLAUDE.md`, `README.md`, code
comments, or `values.yaml` doc comments — describe the reasoning in prose instead of pointing at
where it was decided.

**Why:** a ticket or internal-tooling reference is meaningless to anyone outside the team, and it
rots the moment that tooling's working state is cleaned up or the ticket is closed — a design
rationale should stand on its own, not depend on a link that may not resolve for the reader, or at
all.

**How to apply:** when explaining *why* a decision was made, restate the reasoning directly (as
this file already does above for the `extraEnv` exemption). Ticket numbers and links to
in-progress design artifacts belong in the commit message and PR description, not in
repo-committed files.
24 changes: 24 additions & 0 deletions gremlin/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ their default values. See values.yaml for all available options.
| `nodeSelector` | Map of node labels for pod assignment for the `gremlin` container | `{}` |
| `tolerations` | List of node taints to tolerate for the `gremlin` container | `[]` |
| `affinity` | Map of node/pod affinities for the `gremlin` container | `{}` |
| `livenessProbe` | A Kubernetes `Probe` for the `gremlin` container | `{}` |
| `readinessProbe` | A Kubernetes `Probe` for the `gremlin` container | `{}` |
| `startupProbe` | A Kubernetes `Probe` for the `gremlin` container | `{}` |
| `initContainers` | Additional init containers for the `gremlin` DaemonSet, appended after `seccomp-init` when enabled. A name colliding with one the chart manages fails the install | `[]` |
| `extraVolumes` | Additional volumes for the `gremlin` DaemonSet. A name colliding with one the chart manages fails the install | `[]` |
| `extraVolumeMounts` | Additional volume mounts for the `gremlin` container. A name colliding with one the chart manages fails the install | `[]` |
| `lifecycle` | A Kubernetes `Lifecycle` object (`preStop`/`postStart`) for the `gremlin` container | `{}` |
| `envFrom` | Additional `ConfigMap`/`Secret` sources for the `gremlin` container's environment. Unlike `gremlin.extraEnv`, does not override chart-managed env vars (Kubernetes gives `env:` precedence over `envFrom:`) | `[]` |
| `terminationGracePeriodSeconds` | Termination grace period, in seconds, for the `gremlin` DaemonSet's pods | (Kubernetes default: `30`) |
| `dnsConfig` | A Kubernetes `PodDNSConfig` for the `gremlin` DaemonSet's pod spec | `{}` |
| `hostAliases` | A list of Kubernetes `HostAlias` entries for the `gremlin` DaemonSet's pod spec. No effect while `gremlin.hostNetwork` is `true` (the default) | `[]` |
| `podSecurityContext` | A Kubernetes pod-level `SecurityContext` for the `gremlin` DaemonSet, alongside the existing container-level `gremlin.podSecurity.*` settings | `{}` |
| `chao.podLabels` | Kubernetes labels applied to the chao deployment and it's Pods | `{}` |
| `chao.priorityClassName` | The name of the priority class to use for the Chao deployment | `""` |
| `chao.nodeSelector` | Map of node labels for pod assignment for the `chao` container | `{}` |
Expand All @@ -43,6 +55,18 @@ their default values. See values.yaml for all available options.
| `chao.create` | Enable kubernetes targeting by installing k8s client | true |
| `chao.resources` | Set resource requests and limits for the chao deployment | `{}` |
| `chao.extraEnv` | Specify any arbitrary environment variables to pass to the Chao deployment. | `[]` |
| `chao.livenessProbe` | A Kubernetes `Probe` for the `chao` container | `{}` |
| `chao.readinessProbe` | A Kubernetes `Probe` for the `chao` container | `{}` |
| `chao.startupProbe` | A Kubernetes `Probe` for the `chao` container | `{}` |
| `chao.initContainers` | Additional init containers for the `chao` deployment. A name colliding with one the chart manages fails the install | `[]` |
| `chao.extraVolumes` | Additional volumes for the `chao` deployment. A name colliding with one the chart manages fails the install | `[]` |
| `chao.extraVolumeMounts` | Additional volume mounts for the `chao` container. A name colliding with one the chart manages fails the install | `[]` |
| `chao.lifecycle` | A Kubernetes `Lifecycle` object (`preStop`/`postStart`) for the `chao` container | `{}` |
| `chao.envFrom` | Additional `ConfigMap`/`Secret` sources for the `chao` container's environment. Unlike `chao.extraEnv`, does not override chart-managed env vars (Kubernetes gives `env:` precedence over `envFrom:`) | `[]` |
| `chao.terminationGracePeriodSeconds` | Termination grace period, in seconds, for the `chao` deployment's pods | (Kubernetes default: `30`) |
| `chao.dnsConfig` | A Kubernetes `PodDNSConfig` for the `chao` deployment's pod spec | `{}` |
| `chao.hostAliases` | A list of Kubernetes `HostAlias` entries for the `chao` deployment's pod spec | `[]` |
| `chao.podSecurityContext` | A Kubernetes pod-level `SecurityContext` for the `chao` deployment's pod spec | `{}` |
| `chao.namespaces` | List of namespaces for Gremlin to watch for attacking | `[]` |
| `chao.excludedNamespaces` | List of namespaces Gremlin should never report or attack (mutually exclusive with `chao.namespaces`) | `[]` |
| `chao.features.dynamicQuery.enabled` | Let Gremlin query Kubernetes resources beyond the fixed set Chao watches by default. [See below](#chao-dynamic-queries) | `false` |
Expand Down
41 changes: 40 additions & 1 deletion gremlin/templates/_daemonset.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Context: dict
{{- /* CDI device: global gremlin.gpu.cdiDevice, optionally overridden per vendor block */ -}}
{{- $cdiDevice := "" -}}
{{- with $gpu -}}{{- $cdiDevice = default $root.Values.gremlin.gpu.cdiDevice .cdiDevice -}}{{- end -}}
{{- $seccompInit := and $root.Values.gremlin.podSecurity.seccomp.enabled (eq "localhost/gremlin" $root.Values.gremlin.podSecurity.seccomp.profile) -}}
apiVersion: apps/v1
kind: DaemonSet
metadata:
Expand Down Expand Up @@ -100,12 +101,25 @@ spec:
dnsPolicy: {{ $root.Values.gremlin.dnsPolicy }}
hostPID: {{ $root.Values.gremlin.hostPID }}
hostNetwork: {{ $root.Values.gremlin.hostNetwork }}
{{- if not (kindIs "invalid" $root.Values.terminationGracePeriodSeconds) }}
terminationGracePeriodSeconds: {{ $root.Values.terminationGracePeriodSeconds | toJson }}
{{- end }}
{{- with $root.Values.dnsConfig }}
dnsConfig: {{ toYaml . | nindent 8 }}
{{- end }}
{{- with $root.Values.hostAliases }}
hostAliases: {{ toYaml . | nindent 8 }}
{{- end }}
{{- with $root.Values.podSecurityContext }}
securityContext: {{ toYaml . | nindent 8 }}
{{- end }}
{{- if $root.Values.image.pullSecret }}
imagePullSecrets:
- name: {{ $root.Values.image.pullSecret }}
{{- end }}
{{- if and $root.Values.gremlin.podSecurity.seccomp.enabled (eq "localhost/gremlin" $root.Values.gremlin.podSecurity.seccomp.profile) }}
{{- if or $seccompInit $root.Values.initContainers }}
initContainers:
{{- if $seccompInit }}
- name: seccomp-init
image: {{ $root.Values.image.repository }}:{{ $root.Values.image.tag }}
imagePullPolicy: {{ $root.Values.image.pullPolicy }}
Expand All @@ -118,6 +132,10 @@ spec:
- cp
- /gremlin/seccomp.json
- {{ $root.Values.gremlin.podSecurity.seccomp.root }}/gremlin
{{- end }}
{{- with $root.Values.initContainers }}
{{- toYaml . | nindent 8 }}
{{- end }}
{{- end }}
containers:
- name: {{ $root.Chart.Name }}
Expand Down Expand Up @@ -207,6 +225,21 @@ spec:
{{- with $root.Values.gremlin.extraEnv }}
{{- toYaml . | nindent 10 }}
{{- end }}
{{- with $root.Values.envFrom }}
envFrom: {{ toYaml . | nindent 10 }}
{{- end }}
{{- with $root.Values.livenessProbe }}
livenessProbe: {{ toYaml . | nindent 10 }}
{{- end }}
{{- with $root.Values.readinessProbe }}
readinessProbe: {{ toYaml . | nindent 10 }}
{{- end }}
{{- with $root.Values.startupProbe }}
startupProbe: {{ toYaml . | nindent 10 }}
{{- end }}
{{- with $root.Values.lifecycle }}
lifecycle: {{ toYaml . | nindent 10 }}
{{- end }}
volumeMounts:
- name: gremlin-state
mountPath: /var/lib/gremlin
Expand Down Expand Up @@ -245,6 +278,9 @@ spec:
subPath: {{ $gpu.openclIcd.filename }}
readOnly: true
{{- end }}
{{- with $root.Values.extraVolumeMounts }}
{{- toYaml . | nindent 10 }}
{{- end }}
volumes:
- name: cgroup-root
hostPath:
Expand Down Expand Up @@ -293,6 +329,9 @@ spec:
configMap:
name: {{ include "gremlin.fullname" $root }}-opencl-icd
{{- end }}
{{- with $root.Values.extraVolumes }}
{{- toYaml . | nindent 8 }}
{{- end }}
{{- if $root.Values.gremlin.priorityClassName }}
priorityClassName: {{ $root.Values.gremlin.priorityClassName }}
{{- end }}
Expand Down
Loading
Loading