From 0af84c24c71a090211da8120d7ccc7191df3afb5 Mon Sep 17 00:00:00 2001 From: moilanik Date: Tue, 10 Mar 2026 09:29:21 +0200 Subject: [PATCH] feat(helm): expand guidelines for namespace agnosticism and values hygiene MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit checklist: - add explicit grep-based checks for hardcoded namespaces, names, required+default conflicts, unused values keys - scope name checks to templates/ only — hardcoded names in values.yaml are legitimate - fix required error message example to include what to set and where New sections: - values.yaml Is Not Rendered — fixed names in values.yaml for subchart coordination are correct - Never Hardcode Namespace / Release Name — hard rule with IaC principle rationale - Glue Configuration pattern — documented exception for subchart coordination (e.g. Tempo + MinIO) values.yaml hygiene rewrite: - two purposes: static baseline + umbrella glue (wiring subcharts into one unit) - goal is a short, minimal file identical across all installations - installation-specific values belong exclusively in values..yaml - forbidden: empty placeholders, defaults for rarely-changed values, required-value entries required vs defaults rewrite: - sensible default → hardcode in template, not in values.yaml - must-vary value → required with descriptive message, no entry in values.yaml - | default forbidden - required error messages must say exactly what to set and where KISS addition: - hardcoded values are the starting point, not a compromise - parameterize only when developer asks, and into values..yaml --- .ai/instructions/skills/helm.instructions.md | 196 +++++++++++++++++-- 1 file changed, 176 insertions(+), 20 deletions(-) diff --git a/.ai/instructions/skills/helm.instructions.md b/.ai/instructions/skills/helm.instructions.md index 1e0a7cc..559fa82 100644 --- a/.ai/instructions/skills/helm.instructions.md +++ b/.ai/instructions/skills/helm.instructions.md @@ -1,6 +1,81 @@ # Helm Chart Development Guidelines -## 🔍 Before Creating Anything New +--- + +## 🚨 Audit Checklist — Violations to Find When Reviewing + +When asked to review or analyze a Helm project, **actively search for each of these**. Do not rely on a quick read — scan template files for the literal patterns below. A violation must be reported as an issue, not ignored. + +### Hardcoded namespaces +Search all files under `templates/` for a literal `namespace:` key. Flag any value that is not `{{ .Release.Namespace }}` or a `required`/`.Values` expression. + +``` +grep -r "namespace:" templates/ +``` + +Every hit that is not: +```yaml +namespace: {{ .Release.Namespace }} +namespace: {{ required "..." .Values.something }} +namespace: {{ .Values.something }} +``` +is a violation. + +### Hardcoded resource names +Search `templates/` for name values that are plain strings — not using `.Release.Name` or `include`: + +``` +grep -r "name:" templates/ +``` + +Flag names like `name: my-app`, `name: myapp-ingress` — anything that does not derive from `.Release.Name` or a fullname helper. The only allowed exception is `fullnameOverride` in the glue configuration pattern (documented below), which must be explicitly commented. + +**Do not flag hardcoded names in `values.yaml`.** `values.yaml` is not rendered through the template engine — `{{ .Release.Name }}` expressions are not evaluated there. Fixed names in values.yaml for subchart coordination are a legitimate pattern (see Glue Configuration below). + +### Required values with empty defaults +Search for `required` calls paired with a fallback default in values.yaml: + +``` +grep -r "required" templates/ +``` + +For each hit, check if values.yaml contains an entry for that key. If it does with a non-empty or placeholder value, it is a violation — `required` values must have no entry in values.yaml. + +### values.yaml keys not used in any template + +``` +grep -r "\.Values\." templates/ +``` + +Compare the set of `.Values.*` references in templates against all keys defined in values.yaml. Keys in values.yaml that no template references are dead weight — flag them. + +--- + +## � values.yaml Is Not Rendered + +`values.yaml` is static YAML — it is **not** processed by Helm's template engine. Template expressions like `{{ .Release.Name }}` or `{{ .Values.foo }}` do not work in `values.yaml` and must never be placed there. + +Consequences: + +- Resource names passed to subcharts via values.yaml must be literal strings +- When a subchart needs to reference a resource created by the parent (e.g. a ConfigMap name), that name must be hardcoded in values.yaml and matched exactly in the parent template +- This is the correct pattern — not a violation + +```yaml +# values.yaml — correct: fixed name because values.yaml cannot template +configMap: + name: custom-alloy-config # matches the name in templates/alloy-config.yaml + +# templates/alloy-config.yaml — the parent creates the ConfigMap with this exact name +metadata: + name: custom-alloy-config +``` + +Document fixed names with a comment explaining why they are fixed. The audit checklist only flags hardcoded names in `templates/` files — never in `values.yaml`. + +--- + +## �🔍 Before Creating Anything New **ALWAYS search the workspace for existing implementations first.** @@ -37,7 +112,7 @@ If a resource must reference another namespace (e.g. a NetworkPolicy peer), expo ```yaml metadata: - namespace: {{ required "global.namespace is required" .Values.global.targetNamespace }} + namespace: {{ required "global.targetNamespace is required — set the target namespace in values..yaml" .Values.global.targetNamespace }} ``` ### Release Name @@ -80,38 +155,63 @@ The chart is infrastructure code. It must be deployable to any cluster, any name --- -## 🧹 values.yaml Hygiene +## 🧹 values.yaml — Keep It Short and Identical Across Installations -**values.yaml must only contain values that templates actually use.** +**values.yaml serves two purposes:** -Before adding a value: -- Verify at least one template references `.Values.` -- If no template uses it → don't add it -- Empty string defaults (`key: ""`) that only serve as documentation are forbidden — use README instead +1. **Static baseline** — identical for every installation. If a value varies between installations, it does not belong here. +2. **Umbrella glue** — wires subcharts into a coherent whole. Shared values like `global.domain` are defined once here and consumed by multiple subcharts, so the umbrella behaves as a single unit rather than a collection of independent charts. -**Umbrella chart values.yaml:** -- Contains only subchart defaults that override upstream chart defaults -- All environment-specific values belong in the deployment repo -- Must NOT contain empty placeholders +Installation-specific values go exclusively in `values..yaml` in the IaC/deployment repo. -**Subchart values.yaml:** -- Contains only values the subchart's own templates reference -- Required values (validated with `required`) must NOT have empty defaults — absence should trigger the error +**The goal is a short, minimal values.yaml.** The longer it gets, the harder it is to see what actually varies. When in doubt whether a value belongs in values.yaml, it probably doesn't. + +**A value may only appear in values.yaml if:** +- At least one template references `.Values.` — and +- It is either shared glue across subcharts, or overrides an upstream subchart default that would otherwise be wrong for all installations + +**Forbidden:** +- Empty string placeholders (`key: ""`) — these are documentation pretending to be config; use README instead +- Defaults for values that are hardcoded in templates — they create a false impression the value is configurable +- Installation-specific data of any kind — hostnames, credentials, sizing, feature flags +- Required values (validated with `required`) — absence must trigger the error, not a silent empty default + +**Two-file layering in IaC:** + +| File | Location | Content | +|---|---|---| +| `values.yaml` | chart repo | minimal static baseline, same for all installations | +| `values..yaml` | IaC/deployment repo | everything that varies per installation | --- ## ✅ required vs defaults -Use `required` for values that MUST come from the deployer. Do NOT put empty defaults in values.yaml for these. +When a template needs a value, there are exactly two valid patterns: + +**1. The value has a sensible default** — hardcode it directly in the template. Do not add it to values.yaml. + +**2. The value must come from the deployer** — use `required` with a descriptive error message. Do not put any entry in values.yaml for this key. + +`| default` in templates is forbidden. Adding defaults to values.yaml for values that rarely change is also forbidden — it makes values.yaml long and hard to maintain. Hardcoded defaults in templates are easy to find and easy to parameterize later if needed. ```yaml -# Good: fails loudly at install if not provided -name: {{ required "ingress.className is required" .Values.ingress.className }} +# ✅ Sensible default — hardcoded in template, not in values.yaml +replicas: 1 -# Bad: silently passes empty string, fails later in obscure way -name: {{ .Values.ingress.className | default "" }} +# ✅ Must vary per installation — required, set in values..yaml +host: {{ required "global.domain is required — set it in values..yaml" .Values.global.domain }} + +# ❌ Forbidden — default in values.yaml adds noise, bloats values.yaml +replicas: {{ .Values.replicas }} # values.yaml: replicas: 1 + +# ❌ Forbidden — hides missing config, fails later in an obscure way +host: {{ .Values.global.domain | default "" }} +replicas: {{ .Values.replicas | default 1 }} ``` +The error message in `required` must tell the deployer exactly what to set and where. "X is required" alone is not enough — say what value is expected and in which file. + --- ## 📦 Subchart Dependency Conditions @@ -235,6 +335,36 @@ my-umbrella/ --- +## 🔗 Glue Configuration: Coordinating Subchart Dependencies + +When a subchart cannot use Helm template expressions (e.g. reads `values.` directly instead of templating), the parent chart may coordinate resource names as "glue configuration." + +**When this pattern is appropriate:** +- Subchart hardcodes a config path or resource name lookup (cannot be changed) +- Parent chart must create that resource with the exact name the subchart expects +- This is documented in both parent and subchart values.yaml with clear reasoning +- There is no other way to make the subchart work + +**Example: Tempo + MinIO** + +Tempo chart cannot use `{{ .Release.Name }}` in values.yaml. It reads the MinIO hostname directly from config. + +```yaml +# values.yaml — MinIO +minio: + fullnameOverride: "minio" # ← Forces service name to "minio" + # Documented trade-off: prevents multiple releases in same namespace + # but enables Tempo to connect + +# values.yaml — Tempo +tempo: + storage: + s3: + endpoint: "http://minio:9000" # ← Hardcoded to match MinIO fullnameOverride +``` + +--- + ## ⚡ KISS — Start Minimal, Add Only When Asked **Do not add knobs, flags, or options that aren’t needed right now.** @@ -254,6 +384,32 @@ features: **When in doubt, leave it out.** A value that exists but is never used is noise. A template condition that is never toggled is dead code. Raise the question with the developer instead of silently implementing it. +### Hardcoded values in templates are the starting point + +Not every value needs to be configurable. Making everything a parameter is an anti-pattern — it creates noise and makes charts harder to understand. + +Start with hardcoded values in the template. When the developer asks for a value to be parameterized, move it to `values..yaml` — not to `values.yaml`, which is shared across all installations. + +```yaml +# ✅ Start here — hardcoded in template +resources: + requests: + cpu: "100m" + memory: "128Mi" + +# ✅ When developer asks to parameterize — goes in values..yaml, not values.yaml +resources: + requests: + cpu: {{ required "resources.requests.cpu is required — set it in values..yaml" .Values.resources.requests.cpu }} + memory: {{ required "resources.requests.memory is required — set it in values..yaml" .Values.resources.requests.memory }} + +# ❌ Anti-pattern — parameterising everything preemptively +resources: + requests: + cpu: {{ .Values.resources.requests.cpu }} # nobody asked for this yet + memory: {{ .Values.resources.requests.memory }} # adds noise without benefit +``` + --- ## 🧪 Testing Templates