From c4b3fb7dfc849be58aa22491b4a1553f8a9e4f07 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Wed, 28 Oct 2026 11:46:37 +0100 Subject: [PATCH] feat(ci): validate combined Helm post-render streams --- .github/workflows/ci.yaml | 5 +- AGENTS.md | 2 +- scripts/guard-helm-post-renderers.sh | 36 +++++++---- scripts/guard-pdb-selector-match.sh | 42 +++++++++---- scripts/prepare-helm-render-input-capture.sh | 26 ++++++++ .../tests/test-guard-helm-post-renderers.sh | 61 ++++++++++++++++++- .../tests/test-guard-pdb-selector-match.sh | 55 ++++++++++++++++- 7 files changed, 195 insertions(+), 32 deletions(-) create mode 100755 scripts/prepare-helm-render-input-capture.sh diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index a99e630858..c5dde52b3c 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -641,6 +641,7 @@ jobs: - 'scripts/tests/test-kubescape-storage-placement.sh' - 'scripts/tests/test-kubescape-runtime-rules-switch.sh' - 'scripts/build-controller-helm.sh' + - 'scripts/prepare-helm-render-input-capture.sh' - 'scripts/render-helm-chart/**' # Both halves and the reviewed exception list, for the same reasons. The # guard renders charts with the renderer the two entries above build. @@ -3826,7 +3827,7 @@ jobs: - name: 🧩 Validate the post-renderer guard run: | - shellcheck scripts/build-controller-helm.sh scripts/guard-helm-post-renderers.sh scripts/tests/test-guard-helm-post-renderers.sh + shellcheck scripts/build-controller-helm.sh scripts/prepare-helm-render-input-capture.sh scripts/guard-helm-post-renderers.sh scripts/tests/test-guard-helm-post-renderers.sh controller_helm="$RUNNER_TEMP/controller-helm" bash scripts/build-controller-helm.sh "$controller_helm" echo "CONTROLLER_HELM=$controller_helm" >> "$GITHUB_ENV" @@ -3981,7 +3982,7 @@ jobs: # The fixture suite pulls no chart: a stand-in packages its fixture charts, and the # renderer built here renders them. run: | - shellcheck scripts/guard-pdb-selector-match.sh scripts/tests/test-guard-pdb-selector-match.sh + shellcheck scripts/prepare-helm-render-input-capture.sh scripts/guard-pdb-selector-match.sh scripts/tests/test-guard-pdb-selector-match.sh controller_helm="$RUNNER_TEMP/controller-helm" bash scripts/build-controller-helm.sh "$controller_helm" echo "CONTROLLER_HELM=$controller_helm" >> "$GITHUB_ENV" diff --git a/AGENTS.md b/AGENTS.md index 477b0eeb40..df99178574 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -504,7 +504,7 @@ After making changes, validate at the appropriate level. **For maintenance, only 1. **Kustomize build** — the cluster overlays and every layer listed under [Validation](#validation) build; the overlays alone cover only the Flux wiring. 2. **YAML / schema** — `kubectl apply --dry-run=client -f ` on changed manifests (no Flux variable substitution). 3. **Coverage** — each changed file appears in the validator's output (its path for `ksail workload validate`, its resource for a layer build); a zero exit alone proves nothing. -4. **Post-renderers** — no build above executes a HelmRelease's `postRenderers`; only helm-controller does, against the chart's rendered output. When a change touches a release that carries them (its post-renderers, chart version, values, substitution variables or Kubernetes pin), run `bash scripts/guard-helm-post-renderers.sh --flux-version 2.9.6 --base origin/main --base-kube-version "$(git show origin/main:ksail.prod.yaml | yq -er '.spec.cluster.kubernetesVersion')" --kube-version "$(yq -er '.spec.cluster.kubernetesVersion' ksail.prod.yaml)" k8s` with Helm v4.2.4: it pulls each changed release's chart, renders it with the release's own values as an install and an upgrade, and applies its post-renderers, failing where one cannot apply (#3581). The audited offline profile refuses unsupported historical or SDK capability inputs. CI runs the same check in the `validate-helm-post-renderers` job. A failure that only appears on the cluster, in an upgrade triggered by nothing but a post-renderer or common-metadata change, is reported once and then hidden behind `Ready=True`; [the runbook](docs/operations/helm-release-ready-after-failed-upgrade.md) has the check for it (#4376). Every release with `postRenderers` sets `postRenderStrategy: nohooks` (Flux 2.9 otherwise post-renders hooks too); the guard refuses any other value or none. +4. **Post-renderers** — no build above executes a HelmRelease's `postRenderers`; only helm-controller does, against the chart's rendered output. When a change touches a release that carries them (its post-renderers, chart version, values, substitution variables or Kubernetes pin), run `bash scripts/guard-helm-post-renderers.sh --flux-version 2.9.6 --base origin/main --base-kube-version "$(git show origin/main:ksail.prod.yaml | yq -er '.spec.cluster.kubernetesVersion')" --kube-version "$(yq -er '.spec.cluster.kubernetesVersion' ksail.prod.yaml)" k8s` with Helm v4.2.4: it pulls each changed release's chart, renders it with the release's own values as an install and an upgrade, and applies its post-renderers, failing where one cannot apply (#3581). The audited offline profile refuses unsupported historical or SDK capability inputs. CI runs the same check in the `validate-helm-post-renderers` job. A failure that only appears on the cluster, in an upgrade triggered by nothing but a post-renderer or common-metadata change, is reported once and then hidden behind `Ready=True`; [the runbook](docs/operations/helm-release-ready-after-failed-upgrade.md) has the check for it (#4376). Every release with `postRenderers` explicitly selects `postRenderStrategy: nohooks` for ordinary manifests or `combined` for ordinary manifests and hooks together. The guard evaluates the selected stream for both install and upgrade; an omitted strategy, `separate`, or any other unsupported value remains unknown. 5. **PodDisruptionBudget selectors** — a budget whose selector matches no pod is valid YAML and passes every build and scan above; its only symptom is `expectedPods: 0` on the live cluster. When a change touches a budget, the pod labels of a workload one guards, a chart version or values of a release one guards, or a Flagger `Canary`, run `bash scripts/guard-pdb-selector-match.sh --kube-version "$(yq -er '.spec.cluster.kubernetesVersion' ksail.prod.yaml)" k8s`. It needs Go, to build the chart renderer once (`CONTROLLER_HELM` names one already built by `scripts/build-controller-helm.sh`), and network access to pull charts. It renders both clusters and requires each budget to select a workload rendered into its namespace, rendering the HelmReleases installed beside a budget the manifests do not satisfy (with their values and post-renderers, as an install and an upgrade) and deriving the `-primary` workload Flagger serves from, whose pod labels differ from the target's, for a `Canary` in the manifests or in a rendered chart (#3596, #4365). Replica counts are not judged. A budget whose pods nothing here renders (an operator building its workload at runtime, the Talos-installed CoreDNS) takes a reviewed row in [`scripts/pdb-selector-exceptions.tsv`](scripts/pdb-selector-exceptions.tsv) naming the one cluster it applies to, what builds the pods, the version pin their labels were verified at, and where. The guard does not check an excepted selector against its pods; what it enforces on a row is narrower: it applies to no other cluster; it is refused while that cluster renders, into the budget's namespace, a workload carrying part of what the selector names, unless the row lists that workload as reviewed and unrelated; it is refused when it names a workload or a HelmRelease the guard can render as the producer; and it fails when its pin (a chart version, or the Talos version in `ksail.prod.yaml`) no longer equals the verified version, so a bump cannot merge until the labels are verified again. A stale row fails. A budget it cannot decide is exit 2, never clean, and each such entry leads with the cause; a chart pull is tried three times before it counts as failed. CI runs it on pull requests in the `validate-pdb-selectors` job. ### Cluster scenarios (CI / full local dev only) diff --git a/scripts/guard-helm-post-renderers.sh b/scripts/guard-helm-post-renderers.sh index 177e5aaa93..910113bc49 100755 --- a/scripts/guard-helm-post-renderers.sh +++ b/scripts/guard-helm-post-renderers.sh @@ -35,9 +35,10 @@ # # CONTROLLER PROFILE. --flux-version is an explicit, audited offline profile, not live version # discovery. Flux 2.9.6 (helm-controller 1.6.5) embeds Helm 4.2.4. Its default postRenderStrategy -# is `combined`, which also post-renders hooks; this guard renders regular manifests only, so every -# post-rendered release must set `postRenderStrategy: nohooks` explicitly and any other value, or none, -# is refused. Other profiles are UNKNOWN until audited. A matching +# is `combined`, which post-renders hooks and ordinary manifests in one stream. +# This guard models explicit `nohooks` and `combined` strategies; an omitted +# strategy or the separate-stream strategy is UNKNOWN until explicitly modeled. +# Other profiles are UNKNOWN until audited. A matching # 2.9.x selector does not prove the running patch: production delivery still needs OIDC readback. # # ⚠️ WHAT THIS DOES NOT SEE. The chart renders without a cluster, so a template that branches on @@ -167,6 +168,10 @@ ensure_controller_renderer() { die 'cannot read the controller SDK renderer metadata' [ "$metadata" = 'version.BuildInfo{Version:"v4.2", GitCommit:"", GitTreeState:"", GoVersion:"go1.26.8", KubeClientVersion:"v1.36"}' ] || die 'the controller SDK renderer has unaudited capability metadata' + # Capture Helm's real input to a post-renderer. The final `helm template` + # output has already lost transient annotations that a patch may inspect. + bash "$(dirname "${BASH_SOURCE[0]}")/prepare-helm-render-input-capture.sh" "$HELM_PLUGINS" || + die 'cannot prepare the private capture plugin' controller_helm_checked=1 } @@ -377,7 +382,7 @@ collect() { # *) die "cluster '$cluster' selector '$selector' does not admit audited Flux $flux_version" ;; esac profile="$(jq -n --arg flux "$flux_version" --arg helm "$helm_version" --arg kube "$kube_version" --arg selector "$selector" \ - '{flux: $flux, helm: $helm, kube: $kube, selector: $selector, hooks: "nohooks"}')" + '{flux: $flux, helm: $helm, kube: $kube, selector: $selector, hooks: ["nohooks", "combined"]}')" while IFS="$tab" read -r doc rendered path; do releases="$(yq ea -o=json -I=0 "[. | $post_rendered] | map(select(. != null))" "$rendered.resolved.yaml" 2>"$rendered.releases.err")" || die "cannot parse the substituted HelmReleases rendered from '$path': $(head -c 500 "$rendered.releases.err")" @@ -451,7 +456,7 @@ apply_post_renderers() { # # Flux does. check_release() { # local record="$1" work="$2" label pull ref repo version chart_desc tgz release namespace - local mode error rendered value_ref target value hash dependency pending inspected=0 + local mode error rendered wire_input value_ref target value hash dependency pending hook_strategy inspected=0 local value_args=() label="$(jq -r '"\(.cluster): HelmRelease \(.release.metadata.namespace)/\(.release.metadata.name)"' "$record")" mkdir -p "$work" || die "cannot create '$work'" @@ -461,8 +466,9 @@ check_release() { # (((.kustomize // {}).patches // []) | all(.[]; keys - ["patch", "target"] | length == 0)))' \ "$record" >/dev/null || die "$label: a post-renderer is not a kustomize patches/images renderer, which is all Flux's Kustomize post-renderer applies — this guard cannot execute it" - jq -e '.release.spec.postRenderStrategy == "nohooks"' "$record" >/dev/null || - die "$label: spec.postRenderStrategy must be nohooks; Flux $flux_version otherwise post-renders hooks too, which this guard does not model" + jq -e '.release.spec.postRenderStrategy | IN("nohooks", "combined")' "$record" >/dev/null || + die "$label: spec.postRenderStrategy must be explicit nohooks or combined; omitted and separate-stream strategies are not modeled" + hook_strategy="$(jq -r '.release.spec.postRenderStrategy' "$record")" || die "$label: cannot read its post-render strategy" jq -e '.release.spec.upgrade.preserveValues != true' "$record" >/dev/null || die "$label: spec.upgrade.preserveValues needs historical release values this offline guard cannot read" jq -e '((.release.spec.chart.spec.valuesFiles // []) | length == 0) and (.release.spec.chart.spec.valuesFile == null)' \ @@ -573,6 +579,7 @@ check_release() { # for mode in install upgrade; do rendered="$work/$mode.yaml" + wire_input="$work/$mode.postrender-input.yaml" if [ "$mode" = upgrade ]; then set -- --is-upgrade else @@ -581,17 +588,24 @@ check_release() { # if [ -n "$kube_version" ]; then set -- "$@" --kube-version "$kube_version" fi - "$controller_helm" template "$release" "$tgz" --namespace "$namespace" "${value_args[@]}" "$@" \ + HELM_GUARD_CAPTURE="$wire_input" "$controller_helm" template "$release" "$tgz" --namespace "$namespace" "${value_args[@]}" "$@" \ + --post-renderer guard-input-capture \ >"$rendered" 2>"$rendered.err" || die "$label: cannot assemble its values or render $chart_desc as an $mode: $(head -c 500 "$rendered.err")" - yq 'select(tag == "!!map") | select(.metadata.annotations["helm.sh/hook"] == null)' "$rendered" >"$work/$mode.manifests.yaml" || - die "$label: cannot split the $mode render of $chart_desc into hooks and manifests" + [ -s "$wire_input" ] || die "$label: the SDK did not capture the $mode post-renderer input" + if [ "$hook_strategy" = nohooks ]; then + yq 'select(tag == "!!map") | select(.metadata.annotations["helm.sh/hook"] == null)' "$wire_input" >"$work/$mode.manifests.yaml" || + die "$label: cannot split the $mode render of $chart_desc into hooks and manifests" + else + yq 'select(tag == "!!map")' "$wire_input" >"$work/$mode.manifests.yaml" || + die "$label: cannot collect the combined $mode render of $chart_desc" + fi error="$(apply_post_renderers "$record" "$work/$mode.manifests.yaml" "$work/$mode.manifests")" case $? in 0) ;; 1) printf '%s: its post-renderers do not apply to the %s render of %s (%s): %s\n' \ - "$label" "$mode" "$chart_desc" manifests "$error" >>"$scratch/findings" + "$label" "$mode" "$chart_desc" "$hook_strategy" "$error" >>"$scratch/findings" return 0 ;; *) exit 2 ;; diff --git a/scripts/guard-pdb-selector-match.sh b/scripts/guard-pdb-selector-match.sh index 41947d92af..3ae64cdd2e 100755 --- a/scripts/guard-pdb-selector-match.sh +++ b/scripts/guard-pdb-selector-match.sh @@ -52,7 +52,7 @@ # ⚠️ CANNOT-CHECK IS NEVER CLEAN. A budget nothing rendered satisfies, in a namespace holding a # HelmRelease this guard cannot render the way Flux does, is exit 2: a chart source that needs # credentials or is not a HelmRepository or OCIRepository, a `valuesFrom` entry without a -# `targetPath`, `valuesFiles`, `upgrade.preserveValues`, a `postRenderStrategy` other than `nohooks` +# `targetPath`, `valuesFiles`, `upgrade.preserveValues`, an unsupported post-render strategy # (or none on a post-rendered release), a post-renderer that is not a kustomize `patches`/`images` # renderer or does not apply, or a chart that fails to pull or render. A chart pull is tried # PDB_GUARD_PULL_ATTEMPTS times (default 3), waiting @@ -162,6 +162,8 @@ ensure_renderer() { fi [ -x "$controller_helm" ] || die "the controller renderer '$controller_helm' is not executable" "$controller_helm" version >/dev/null 2>&1 || die "the controller renderer '$controller_helm' does not run" + bash "$script_dir/prepare-helm-render-input-capture.sh" "$HELM_PLUGINS" || + die 'cannot prepare the private capture plugin' controller_helm_checked=1 } @@ -531,7 +533,7 @@ pull_chart() { # # /reason, when the release cannot be rendered the way Flux does. render_release() { # local resolved="$1" ns="$2" name="$3" work="$4" record pull error ref repo version tgz release namespace - local mode value_ref target value hash count index + local mode value_ref target value hash count index hook_strategy local value_args=() mkdir -p "$work" || die "cannot create '$work'" record="$work/record.json" @@ -546,14 +548,16 @@ render_release() { # cannot "a post-renderer is not a kustomize patches/images renderer" return 1 fi - # Only regular manifests are post-rendered here. Flux 2.9 post-renders hooks too unless a - # post-rendered release sets nohooks, so any other strategy, or none, is not modelled. - if ! jq -e '(.release.spec.postRenderStrategy == "nohooks") or + # Combined evaluates hooks in the same stream, then excludes lifecycle hooks + # from the ordinary workloads a budget can protect. Missing strategies stay + # unknown when a post-renderer exists; separate streams are not modeled. + if ! jq -e '(.release.spec.postRenderStrategy | IN("nohooks", "combined")) or (.release.spec.postRenderStrategy == null and ((.release.spec.postRenderers // []) | length == 0))' \ "$record" >/dev/null; then - cannot "spec.postRenderStrategy is not modelled (a post-rendered release must set nohooks)" + cannot "spec.postRenderStrategy is not modelled (a post-rendered release must set explicit nohooks or combined)" return 1 fi + hook_strategy="$(jq -r '.release.spec.postRenderStrategy // "nohooks"' "$record")" || die 'cannot read the post-render strategy' if ! jq -e '.release.spec.upgrade.preserveValues != true' "$record" >/dev/null; then cannot "spec.upgrade.preserveValues needs historical release values" return 1 @@ -644,16 +648,25 @@ render_release() { # if [ -n "$kube_version" ]; then set -- "$@" --kube-version "$kube_version" fi - if ! "$controller_helm" template "$release" "$tgz" --namespace "$namespace" "${value_args[@]}" "$@" \ + if ! HELM_GUARD_CAPTURE="$work/$mode.postrender-input.yaml" "$controller_helm" template "$release" "$tgz" --namespace "$namespace" "${value_args[@]}" "$@" \ + --post-renderer guard-input-capture \ >"$work/$mode.rendered.yaml" 2>"$work/$mode.err"; then cannot "its chart does not render as an $mode: $(tr '\n' ' ' <"$work/$mode.err" | head -c 300)" return 1 fi - # Hooks are not part of the release's manifest, and Flux does not post-render them under nohooks. + if [ ! -s "$work/$mode.postrender-input.yaml" ]; then + cannot "the SDK did not capture its $mode post-renderer input" + return 1 + fi mkdir -p "$work/$mode" || die "cannot create '$work/$mode'" - yq 'select(tag == "!!map") | select(.metadata.annotations["helm.sh/hook"] == null)' "$work/$mode.rendered.yaml" \ - >"$work/$mode/resources.yaml" 2>"$work/$mode.err" || - die "HelmRelease $ns/$name: cannot read its $mode render: $(head -c 500 "$work/$mode.err")" + if [ "$hook_strategy" = combined ]; then + yq 'select(tag == "!!map")' "$work/$mode.postrender-input.yaml" >"$work/$mode/resources.yaml" || + die "HelmRelease $ns/$name: cannot collect its combined $mode render" + else + yq 'select(tag == "!!map") | select(.metadata.annotations["helm.sh/hook"] == null)' "$work/$mode.postrender-input.yaml" \ + >"$work/$mode/resources.yaml" 2>"$work/$mode.err" || + die "HelmRelease $ns/$name: cannot read its $mode render: $(head -c 500 "$work/$mode.err")" + fi index=0 while [ "$index" -lt "$count" ]; do jq --argjson i "$index" '{apiVersion: "kustomize.config.k8s.io/v1beta1", kind: "Kustomization", @@ -666,7 +679,12 @@ render_release() { # mv "$work/$mode.next.yaml" "$work/$mode/resources.yaml" || die "cannot stage the output of post-renderer $((index + 1))" index=$((index + 1)) done - cp "$work/$mode/resources.yaml" "$work/$mode.yaml" || die "cannot keep the $mode render of HelmRelease $ns/$name" + if [ "$hook_strategy" = combined ]; then + yq 'select(tag == "!!map") | select(.metadata.annotations["helm.sh/hook"] == null)' "$work/$mode/resources.yaml" >"$work/$mode.yaml" || + die "cannot retain the ordinary $mode workloads of HelmRelease $ns/$name" + else + cp "$work/$mode/resources.yaml" "$work/$mode.yaml" || die "cannot keep the $mode render of HelmRelease $ns/$name" + fi done } diff --git a/scripts/prepare-helm-render-input-capture.sh b/scripts/prepare-helm-render-input-capture.sh new file mode 100755 index 0000000000..49bacca92c --- /dev/null +++ b/scripts/prepare-helm-render-input-capture.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash +# Prepare an identity post-renderer inside a caller-owned private Helm directory. +# The caller supplies HELM_GUARD_CAPTURE for each audited SDK template invocation. +set -euo pipefail +if [[ $# != 1 || -z ${1:-} || $1 != /* ]]; then + printf '%s\n' 'usage: prepare-helm-render-input-capture.sh ' >&2 + exit 2 +fi +capture_plugin="$1/guard-input-capture" +mkdir -p "$capture_plugin" +cat >"$capture_plugin/plugin.yaml" <<'YAML' +apiVersion: v1 +type: postrenderer/v1 +name: guard-input-capture +version: 0.1.0 +runtime: subprocess +runtimeConfig: + platformCommand: + - command: ${HELM_PLUGIN_DIR}/capture.sh +YAML +cat >"$capture_plugin/capture.sh" <<'BASH' +#!/usr/bin/env bash +set -euo pipefail +tee "${HELM_GUARD_CAPTURE:?capture path required}" +BASH +chmod +x "$capture_plugin/capture.sh" diff --git a/scripts/tests/test-guard-helm-post-renderers.sh b/scripts/tests/test-guard-helm-post-renderers.sh index 3954e8a1ad..ebad2c1a35 100755 --- a/scripts/tests/test-guard-helm-post-renderers.sh +++ b/scripts/tests/test-guard-helm-post-renderers.sh @@ -381,6 +381,41 @@ pr_job_leaf_add() { EOF } +# A lifecycle patch on an exact hook Job, with its existing annotation map. +pr_job_hook_cleanup() { + cat <<'EOF' + postRenderers: + - kustomize: + patches: + - target: + group: batch + version: v1 + kind: Job + name: flux-operator-migrate + patch: | + - op: add + path: /metadata/annotations/helm.sh~1hook-delete-policy + value: before-hook-creation,hook-succeeded +EOF +} + +# Helm adds this transient annotation before invoking a post-renderer and +# removes it afterward. A guard must evaluate the actual input to the patch. +pr_renderer_filename_is() { + cat <<'EOF' + postRenderers: + - kustomize: + patches: + - target: + kind: Deployment + name: flux-operator + patch: | + - op: test + path: /metadata/annotations/postrenderer.helm.sh~1postrender-filename + value: flux-operator/templates/deployment.yaml +EOF +} + # A JSON-6902 test that the rendered replica count is the given number. pr_replicas_are() { # cat <