feat(helm): expand guidelines for namespace agnosticism and values hygiene
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.<instance>.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.<instance>.yaml
This commit is contained in:
moilanik
parent
cb402009fe
commit
0af84c24c7
@ -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.
|
||||
|
||||
---
|
||||
|
||||
## <20> 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`.
|
||||
|
||||
---
|
||||
|
||||
## <20>🔍 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.<instance>.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.<key>`
|
||||
- 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.<instance>.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.<key>` — 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.<instance>.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.<installation>.yaml
|
||||
host: {{ required "global.domain is required — set it in values.<installation>.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.<key>` 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.<instance>.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.<instance>.yaml, not values.yaml
|
||||
resources:
|
||||
requests:
|
||||
cpu: {{ required "resources.requests.cpu is required — set it in values.<instance>.yaml" .Values.resources.requests.cpu }}
|
||||
memory: {{ required "resources.requests.memory is required — set it in values.<instance>.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
|
||||
|
||||
Loading…
Reference in New Issue
Block a user