feat(charts): add decdn-node Helm chart for Kubernetes - #47
Conversation
Adds charts/decdn-node, the Kubernetes counterpart of the decdn_node role: a one-replica StatefulSet (one release = one node identity) on the upstream daemon-only image, with a PVC data dir, operator-provisioned Secrets, a public UDP Service or hostPort, and metrics bound 0.0.0.0 behind a ClusterIP Service and a NetworkPolicy. - node.toml is rendered from a structured `config:` map by a custom TOML renderer (Helm's toToml would emit YAML integers as floats). The chart injects path/port keys, fails on collisions, refuses secret-bearing keys, and ports the role's pull-through derivation. - A `prepare` init container installs keystore.json and node.secret onto the PVC at 0600 (upstream rejects symlinked or group/world-readable key files) and the keystore password into an in-memory volume, never the PVC. - Only named env keys are injected (no envFrom): DECDN_* env overrides node.toml and would bypass the managed keys and schema checks. - Disabling the NetworkPolicy requires networkPolicy.allowUnrestrictedMetrics. Shared schema guard: the molecule schema checker is extracted to check-schema-keys.py and used by both molecule and the chart, and now also checks scalar-array keys and empty tables (it previously skipped every list), with good/bad fixtures that test the checker itself. CI: new `helm` job (`make lint-helm`: strict lint, positive/negative render tests, digest-pinned kubeconform, schema keys); `make security` now also KICS-scans the rendered chart and always runs both scans. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Resolve the metrics-policy probe access, checksum annotation override, and security-context override issues.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds a Kubernetes Helm chart for deCDN nodes with hardened workload configuration, secret references, custom TOML/schema validation, network controls, and CI coverage.
Changes:
- Adds the
decdn-nodeHelm chart and deployment resources. - Adds render tests, schema fixtures, and shared validation.
- Extends documentation, Make targets, security scanning, and CI.
File summaries
| File | Description |
|---|---|
README.md |
Documents Ansible and Helm deployment paths. |
Makefile |
Adds Helm linting and security targets. |
CONTRIBUTING.md |
Documents Helm validation workflows. |
charts/decdn-node/values.yaml |
Defines chart defaults and configuration. |
charts/decdn-node/values.schema.json |
Validates Helm values. |
charts/decdn-node/tests/render-test.sh |
Adds render and invariant tests. |
charts/decdn-node/templates/statefulset.yaml |
Deploys the node workload and storage. |
charts/decdn-node/templates/servicemonitor.yaml |
Adds optional Prometheus integration. |
charts/decdn-node/templates/serviceaccount.yaml |
Manages the service account. |
charts/decdn-node/templates/service.yaml |
Exposes QUIC traffic. |
charts/decdn-node/templates/service-metrics.yaml |
Provides metrics access. |
charts/decdn-node/templates/NOTES.txt |
Provides deployment guidance. |
charts/decdn-node/templates/networkpolicy.yaml |
Restricts network access. |
charts/decdn-node/templates/configmap.yaml |
Renders node.toml. |
charts/decdn-node/templates/_helpers.tpl |
Implements validation and TOML rendering. |
charts/decdn-node/README.md |
Documents chart usage and operations. |
charts/decdn-node/ci/ci-values.yaml |
Exercises broad chart configuration. |
charts/decdn-node/ci/ci-resolve-only.yaml |
Tests resolve-only configuration. |
charts/decdn-node/ci/ci-origins.yaml |
Tests origins and exposure settings. |
charts/decdn-node/Chart.yaml |
Defines chart metadata. |
charts/decdn-node/.helmignore |
Excludes non-package files. |
ansible/molecule/schema/verify.yml |
Uses the shared schema checker. |
ansible/molecule/schema/files/checker-fixtures/good.toml |
Adds valid checker coverage. |
ansible/molecule/schema/files/checker-fixtures/bad.toml |
Adds invalid checker coverage. |
ansible/molecule/schema/files/checker-fixtures/bad.expected |
Defines expected checker failures. |
ansible/molecule/schema/files/check-schema-keys.py |
Shares schema-key validation. |
AGENTS.md |
Documents Helm-specific repository rules. |
.pre-commit-config.yaml |
Excludes Helm templates from YAML parsing. |
.github/workflows/ci.yml |
Adds Helm and chart security CI jobs. |
Review details
Suppressed comments (2)
charts/decdn-node/templates/networkpolicy.yaml:34
- With the default
metrics.networkPolicy.from: [], this policy has no TCP rule for the metrics port, so it denies every metrics ingress connection. The HTTP probes in the StatefulSet are kubelet requests to the pod IP; on CNIs that enforce NetworkPolicy for node-to-pod traffic, startup/readiness/liveness will all fail and the StatefulSet never becomes Ready. Add an explicit, configurable allowance for the kubelet/node CIDRs (or use a probe mechanism not subject to pod ingress policy) and test the default on a supported CNI rather than relying on the comment that common CNIs bypass it.
{{- with .Values.metrics.networkPolicy.from }}
- from:
{{- toYaml . | nindent 8 }}
ports:
- protocol: TCP
port: {{ int $.Values.metrics.port }}
{{- end }}
charts/decdn-node/templates/statefulset.yaml:31
podLabelsis rendered afterdecdn-node.labels, so a value such aspodLabels.app.kubernetes.io/nameorpodLabels.app.kubernetes.io/instanceoverwrites a label required by the StatefulSet selector and Services. Kubernetes then rejects the StatefulSet because its selector no longer matches the pod template (or, depending on the label, the Services have no endpoints). Render the chart-owned selector labels after user labels, or reject these reserved keys.
labels:
{{- include "decdn-node.labels" . | nindent 8 }}
{{- with .Values.podLabels }}
{{- toYaml . | nindent 8 }}
{{- end }}
- Files reviewed: 29/29 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Address PR review: - podLabels may not re-set a chart label (a selector label override would detach the pod from its StatefulSet and Services). - podAnnotations may not re-set checksum/config (config changes would stop rolling the pod). - (pod)securityContext stays overridable (e.g. another non-root uid), but the render fails if the merged result runs as root, allows privilege escalation or privileged mode, has a writable root filesystem, adds capabilities, drops fewer than ALL, or disables seccomp. Also fix the helm CI job: Helm v4.3.0 keeps a `--set key=null` and reports "got null" where v4.0.4 reports "missing property"; the two schema negative tests now accept either wording. Document that kubelet probes are unaffected by the metrics NetworkPolicy (the spec always allows traffic between a pod and its own node). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Responses to the two suppressed review comments, both addressed in b8f20ae:
The failing |
Summary
Adds
charts/decdn-node, a Helm chart that deploys the deCDN node on Kubernetes with the same guarantees as the Ansibledecdn_noderole: no secrets in git or values, fail-loud config, one public hole (QUIC udp/4433), and no baked-in protocol facts.ghcr.io/decdn/decdn-node. Upstream hasn't published it yet, soimage.tagorimage.digestis required. It runs non-root with a read-only rootfs and all capabilities dropped, with a 300s drain window and/metricsstartup/readiness/liveness probes.prepareinit container installskeystore.jsonandnode.secretonto the PVC at 0600. Upstream rejects symlinked or group/world-readable key files.DECDN_RPC_URLpluspassthroughKeys.DECDN_*env would overridenode.toml, soDECDN_*names are refused.values.configmirrorsnode.tomland is rendered by a custom TOML renderer, because Helm'stoTomlemits YAML integers as floats.origins, and integers of 2^53 or more.hostPort.0.0.0.0in the pod, behind a ClusterIP Service and a default-on NetworkPolicy.networkPolicy.allowUnrestrictedMetrics: true.ansible/molecule/schema/files/check-schema-keys.py, used by both molecule and the chart, so one key list covers both deploy paths.relay_urlsordenied_hashespassed. It also now flags unknown empty tables and rejects empty input.helmjob runs oncharts/**, the shared checker, theMakefileorci.yml, viamake lint-helm: strict lint, 3 positive renders with port/exposure checks, 29 must-fail renders, digest-pinned kubeconform, and the schema keys.make securitynow also KICS-scans the rendered chart and always runs both scans.Test plan
DECDN_CLI=<decdn> make lint-helm: 43 checks pass, including the realdecdn config validateon all three CI renders. CI has no decdn binary, so there it printsSKIPPED.from, a hardcodedbind_port); the new invariants caught both.molecule test -s schema(the extracted checker is unchanged for the Ansible path),make -C ansible lintmake security: 0 HIGH/CRITICAL for bothansible/and the rendered chartKnown limitations
kubectl rollout restart; only config changes roll the pod.https://user:pass@…); the README warns about this.automountServiceAccountToken: falsehasn't been tested in a cluster.bookworm-slim(glibc 2.36), so a binary built on a newer host won't start in it.🤖 Generated with Claude Code