jklein
6ade74db80
A two-replica node group was redundancy on paper: nothing kept both pods off one node, and no PodDisruptionBudget kept a drain from taking both at once. Both objects are now rendered per node group, gated on that group's COMPUTED replica count being >= 2. The distribution is a promise, not an effort: maxSkew 1 on kubernetes.io/hostname with whenUnsatisfiable DoNotSchedule, selecting only this group's own pods, so an "mx" and a "full" pod may share a node. The selector comes from the chart (stalwart.groupLabels), never from the operator - a spread constraint whose selector matches no pod is trivially satisfied, distributes nothing and reports nothing. nodeAffinityPolicy: Ignore is set explicitly, and that is the load-bearing detail. Its API default is Honor, under which a narrowing nodeSelector shrinks the set of counted domains until both replicas fit into one: skew 0, both pods co-located, no Pending, no event, and a selector that is correct - so no check can see it. With Ignore the same narrowing gives skew 2 and the second pod is Pending. A render abort was the alternative and would also have forbidden a nodeSelector spanning five nodes, which is legal. A user entry colliding with the chart's own on (topologyKey, whenUnsatisfiable) aborts the render naming both values, because the API rejects the duplicate with a message about a list index and an apply-based path merges the two over patchMergeKey: topologyKey silently, dropping one selector. Minor rather than major, proved with a diff rather than argued: with the shipped defaults (full: 1, mx: 0) and with ci/stalwart/test-values.yaml, helm template renders byte-for-byte what 1.0.1+upv0.16.24 rendered - both diffs empty. New in CI, over every chart on every PR: ci/run-selector-match-test.sh holds every rendered PodDisruptionBudget selector and every topologySpreadConstraints labelSelector against the labels of the pod templates in the same render. Helm itself is the YAML parser (ci/yaml2json), because the runners have neither pyyaml nor yq and a hand-written parser could read nothing and report nothing. podAffinity/podAntiAffinity are out of scope; matchExpressions-only selectors are reported as not checked rather than counted as passing. Two anti-vacuum guards: a chart with an overlay under ci/selector-match/ that renders no selector fails, and a run that checks no selector at all fails. Fixes #96
288 lines
15 KiB
YAML
288 lines
15 KiB
YAML
name: Validate Helm Charts
|
|
on:
|
|
pull_request:
|
|
|
|
jobs:
|
|
detect-changed-charts:
|
|
name: Detect changed charts
|
|
runs-on: ubuntu-latest
|
|
outputs:
|
|
charts: ${{ steps.detect.outputs.charts }}
|
|
any-chart-changed: ${{ steps.detect.outputs.any-chart-changed }}
|
|
scan-charts: ${{ steps.detect.outputs.scan-charts }}
|
|
any-scan: ${{ steps.detect.outputs.any-scan }}
|
|
install-charts: ${{ steps.detect.outputs.install-charts }}
|
|
any-install: ${{ steps.detect.outputs.any-install }}
|
|
steps:
|
|
- name: "Checkout Repository"
|
|
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
|
|
with:
|
|
fetch-depth: 0
|
|
- name: "Detect changed charts"
|
|
id: detect
|
|
run: |
|
|
CHANGED_FILES=$(git diff --name-only "origin/${{ github.base_ref }}...HEAD")
|
|
CHARTS="[]"
|
|
CHANGED_DIRS=$(echo "$CHANGED_FILES" | cut -d/ -f1 | sort -u)
|
|
for DIR in $CHANGED_DIRS; do
|
|
if [ -f "$DIR/Chart.yaml" ]; then
|
|
CHARTS=$(echo "$CHARTS" | jq -c --arg dir "$DIR" '. + [$dir]')
|
|
fi
|
|
done
|
|
echo "charts=$CHARTS" >> "$GITHUB_OUTPUT"
|
|
if [ "$CHARTS" = "[]" ]; then
|
|
echo "any-chart-changed=false" >> "$GITHUB_OUTPUT"
|
|
echo "No chart directories changed - nothing to build."
|
|
else
|
|
echo "any-chart-changed=true" >> "$GITHUB_OUTPUT"
|
|
echo "Changed charts: $CHARTS"
|
|
fi
|
|
|
|
# The Trivy scan normally follows the changed charts. But a PR that
|
|
# changes the scanner itself touches no chart at all, and would leave
|
|
# the change untested. In that case scan a fixed sample instead, one
|
|
# chart per interesting case: a documented deviation (samba), a fully
|
|
# hardened one (cyberchef), and one with two workloads
|
|
# (matrix-synapse).
|
|
SCAN_CHARTS="$CHARTS"
|
|
if [ "$CHARTS" = "[]" ] && echo "$CHANGED_FILES" | grep -qE '^(ci/(run-trivy-config\.sh|trivy/)|\.github/workflows/validate-charts\.yml)'; then
|
|
SCAN_CHARTS='["samba","cyberchef","matrix-synapse"]'
|
|
echo "Scanner changed without a chart change - scanning the sample: $SCAN_CHARTS"
|
|
fi
|
|
echo "scan-charts=$SCAN_CHARTS" >> "$GITHUB_OUTPUT"
|
|
if [ "$SCAN_CHARTS" = "[]" ]; then
|
|
echo "any-scan=false" >> "$GITHUB_OUTPUT"
|
|
else
|
|
echo "any-scan=true" >> "$GITHUB_OUTPUT"
|
|
fi
|
|
|
|
# --- The install-test matrix: changed charts minus the excluded ones
|
|
# (issue #231). A chart in ci/excluded-charts.txt used to get a job
|
|
# that ran the script, printed SKIP and exited 0 - and GitHub knows
|
|
# only the exit code, so the check list showed a green tick for a
|
|
# chart nothing was ever attempted on. Dropping it here means no job
|
|
# exists for it and nothing claims it passed.
|
|
#
|
|
# The list is NOT parsed here. ci/run-install-test.sh --list-excluded
|
|
# emits "<chart><TAB><reason>", and that script is the one parser;
|
|
# re-implementing the format in YAML would be a second chance to
|
|
# select different charts, silently. The comparison below is jq's
|
|
# "!=" on whole strings, so a chart name is never read as a pattern.
|
|
INSTALL_CHARTS="$CHARTS"
|
|
SUMMARY_ROWS=""
|
|
while IFS=$'\t' read -r EX_NAME EX_REASON; do
|
|
[ -n "$EX_NAME" ] || continue
|
|
if [ "$(echo "$INSTALL_CHARTS" | jq -r --arg c "$EX_NAME" 'index($c) != null')" = "true" ]; then
|
|
# A "|" in the justification would break out of the table cell.
|
|
SUMMARY_ROWS="${SUMMARY_ROWS}| \`$EX_NAME\` | ${EX_REASON//|/\\|} |"$'\n'
|
|
echo "Excluded from the install-test: $EX_NAME - $EX_REASON"
|
|
fi
|
|
INSTALL_CHARTS="$(echo "$INSTALL_CHARTS" | jq -c --arg c "$EX_NAME" 'map(select(. != $c))')"
|
|
done <<< "$(ci/run-install-test.sh --list-excluded)"
|
|
echo "install-charts=$INSTALL_CHARTS" >> "$GITHUB_OUTPUT"
|
|
if [ "$INSTALL_CHARTS" = "[]" ]; then
|
|
echo "any-install=false" >> "$GITHUB_OUTPUT"
|
|
else
|
|
echo "any-install=true" >> "$GITHUB_OUTPUT"
|
|
fi
|
|
|
|
# An absent job says "not run" but never why, and an absence is easy
|
|
# to miss entirely. So every excluded chart of this PR is named here
|
|
# WITH its documented justification - which is the question a review
|
|
# actually asks. This summary is the visible half of the change; the
|
|
# dropped matrix entry is only the half that stops the lie.
|
|
if [ -n "$SUMMARY_ROWS" ]; then
|
|
{
|
|
echo "### Charts excluded from the install-test"
|
|
echo
|
|
echo "These charts are changed by this PR but listed in \`ci/excluded-charts.txt\`, so they get **no** install-test job. There is no green tick for them, because nothing was attempted (issue #231)."
|
|
echo
|
|
echo "| Chart | Documented reason |"
|
|
echo "| --- | --- |"
|
|
printf '%s' "$SUMMARY_ROWS"
|
|
echo
|
|
} >> "$GITHUB_STEP_SUMMARY"
|
|
fi
|
|
echo "Install-test matrix: $INSTALL_CHARTS"
|
|
|
|
build-chart:
|
|
name: "Build ${{ matrix.chart }}"
|
|
needs: detect-changed-charts
|
|
if: needs.detect-changed-charts.outputs.any-chart-changed == 'true'
|
|
runs-on: ubuntu-latest
|
|
strategy:
|
|
fail-fast: false
|
|
matrix:
|
|
chart: ${{ fromJSON(needs.detect-changed-charts.outputs.charts) }}
|
|
steps:
|
|
- name: "Checkout Repository"
|
|
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
|
|
with:
|
|
fetch-depth: 0
|
|
- name: "Install Helm"
|
|
uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 # v5
|
|
# Both steps below refuse to run while a chart declares a dependency that
|
|
# is not in its charts/ directory - lint with "[WARNING] chart directory
|
|
# is missing these dependencies", package with "found in Chart.yaml, but
|
|
# missing in charts/ directory", both exiting 1. Charts without a
|
|
# dependencies: block are skipped without a word, so this costs nothing
|
|
# for the rest of the repo. See ci/helm-dependencies.sh for why it is
|
|
# "update" and not "build".
|
|
- name: "Fetch chart dependencies"
|
|
run: ci/helm-dependencies.sh "${{ matrix.chart }}"
|
|
- name: "Lint Helm Chart"
|
|
run: helm lint --strict "${{ matrix.chart }}"
|
|
- name: "Package Helm Chart"
|
|
run: helm package "${{ matrix.chart }}"
|
|
- name: "Check Chart Version was bumped"
|
|
# template-chart is a scaffold and never published, its version is meaningless
|
|
if: matrix.chart != 'template-chart'
|
|
run: |
|
|
BASE_VERSION=$(git show "origin/${{ github.base_ref }}:${{ matrix.chart }}/Chart.yaml" 2>/dev/null | grep '^version:' | awk '{print $2}' || true)
|
|
NEW_VERSION=$(grep '^version:' "${{ matrix.chart }}/Chart.yaml" | awk '{print $2}')
|
|
if [ -z "$BASE_VERSION" ]; then
|
|
echo "New chart (does not exist on ${{ github.base_ref }}), version: $NEW_VERSION"
|
|
elif [ "$BASE_VERSION" = "$NEW_VERSION" ]; then
|
|
echo "::error file=${{ matrix.chart }}/Chart.yaml::Chart '${{ matrix.chart }}' was modified but its version was not bumped (still $BASE_VERSION). Bump 'version:' in Chart.yaml, otherwise the publish on main will collide with the already published version."
|
|
exit 1
|
|
else
|
|
echo "Version bump OK: $BASE_VERSION -> $NEW_VERSION"
|
|
fi
|
|
|
|
quantity-render:
|
|
name: "Resource quantities"
|
|
runs-on: ubuntu-latest
|
|
# The one job in this workflow that deliberately ignores the changed-chart
|
|
# matrix and runs over *every* chart (issue #157). The bug it guards
|
|
# against - resource quantities formatted with printf "%v", where float64
|
|
# flips to exponential notation above ~1e6 - lives in the resources helper,
|
|
# and that helper is copied into every chart's templates/_helpers.tpl,
|
|
# because sharing it would mean a library chart (README, guideline 14). So
|
|
# it comes back by copy: a new chart started from an older one, or from
|
|
# template-chart before the fix.
|
|
# In that PR the chart is new and its copy of the bug is in no diff anyone
|
|
# reads. Running over everything also means there is no list to maintain -
|
|
# adding a chart enrolls it. Two renders per chart, no cluster, a few
|
|
# seconds in total; it depends on no other job and finishes long before
|
|
# build-chart, so it adds no wall-clock time to the pipeline.
|
|
steps:
|
|
- name: "Checkout Repository"
|
|
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
|
|
- name: "Install Helm"
|
|
uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 # v5
|
|
- name: "Render every chart with bare integer quantities"
|
|
run: ci/run-quantity-render-test.sh
|
|
|
|
selector-match:
|
|
name: "Selector match"
|
|
runs-on: ubuntu-latest
|
|
# The second job that ignores the changed-chart matrix and runs over *every*
|
|
# chart (Gitea #96), and for the same reason as quantity-render above: the
|
|
# mistake arrives by copy into a chart whose own diff nobody reads for it.
|
|
#
|
|
# What it asserts: every PodDisruptionBudget selector and every
|
|
# topologySpreadConstraints labelSelector a chart renders really matches the
|
|
# labels of a pod template in the SAME render. Both objects fail SILENTLY
|
|
# when their selector matches nothing - the spread constraint becomes a
|
|
# condition over zero pods and is trivially satisfied, the budget does its
|
|
# arithmetic over zero pods and never blocks an eviction - so neither a
|
|
# render error nor the install-test can catch it, and with
|
|
# whenUnsatisfiable: DoNotSchedule the object reads like a guarantee while
|
|
# distributing nothing.
|
|
#
|
|
# It needs no cluster and no Trivy: ~14 seconds for the whole repository,
|
|
# depends on no other job, and finishes well inside build-chart. The YAML is
|
|
# parsed by Helm itself through ci/yaml2json, because the runners have
|
|
# neither pyyaml nor yq and a hand-written parser would be able to read
|
|
# nothing and report nothing - see the script's header.
|
|
steps:
|
|
- name: "Checkout Repository"
|
|
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
|
|
- name: "Install Helm"
|
|
uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 # v5
|
|
- name: "Check every rendered selector against the rendered pod labels"
|
|
run: ci/run-selector-match-test.sh
|
|
|
|
trivy-config:
|
|
name: "Trivy config ${{ matrix.chart }}"
|
|
needs: detect-changed-charts
|
|
if: needs.detect-changed-charts.outputs.any-scan == 'true'
|
|
runs-on: ubuntu-latest
|
|
# Reporting only (issue #84): the scan renders each changed chart and lists
|
|
# what `trivy config` finds, but it never fails the job - see
|
|
# TRIVY_CONFIG_MODE below. Deliberately not a gate yet: the repo has several
|
|
# documented deviations, and a check that goes red on those every time is one
|
|
# people learn to scroll past. Depends only on detect-changed-charts, so it
|
|
# runs beside build-chart and adds no wall-clock time to the pipeline.
|
|
strategy:
|
|
fail-fast: false
|
|
matrix:
|
|
chart: ${{ fromJSON(needs.detect-changed-charts.outputs.scan-charts) }}
|
|
env:
|
|
# renovate: datasource=github-releases depName=aquasecurity/trivy
|
|
TRIVY_VERSION: v0.75.0
|
|
# "report" always exits 0. Switch to "gate" to make a finding that no
|
|
# documented exception covers fail the job - the intended step once the
|
|
# seccomp gap (#84) is closed.
|
|
TRIVY_CONFIG_MODE: report
|
|
steps:
|
|
- name: "Checkout Repository"
|
|
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
|
|
- name: "Install Helm"
|
|
uses: azure/setup-helm@9bc31f4ebc9c6b171d7bfbaa5d006ae7abdb4310 # v5
|
|
# A plain release download rather than aquasecurity/setup-trivy, and the
|
|
# reason is measured on the Gitea probe PR of #225, not assumed. That
|
|
# action runs actions/checkout against aquasecurity/trivy on github.com
|
|
# and hands it `github.token` - which on Gitea is a GITEA token.
|
|
# github.com answers 401, checkout retries twice and the job ends on
|
|
# "fatal: could not read Username for 'https://github.com': terminal
|
|
# prompts disabled", an error naming neither the action nor the token.
|
|
# Its actions/cache step is inert on the instance as well ("Cache action
|
|
# is only supported on GHES version >= 3.5"), and fetching the nested
|
|
# actions cost 4m30s before the job even started. One tarball behaves
|
|
# identically on both forges, and the `# renovate:` comment above still
|
|
# keeps TRIVY_VERSION current - verified by dry run: pinned to v0.72.0
|
|
# the lookup offers v0.74.0 as a minor update.
|
|
- name: "Install Trivy"
|
|
run: |
|
|
curl -sSfL -o /tmp/trivy.tar.gz \
|
|
"https://github.com/aquasecurity/trivy/releases/download/${TRIVY_VERSION}/trivy_${TRIVY_VERSION#v}_Linux-64bit.tar.gz"
|
|
mkdir -p "$HOME/.local/bin"
|
|
tar -xzf /tmp/trivy.tar.gz -C "$HOME/.local/bin" trivy
|
|
echo "$HOME/.local/bin" >> "$GITHUB_PATH"
|
|
"$HOME/.local/bin/trivy" --version
|
|
# Findings land in the job summary, not only in the log: exceptions with
|
|
# their reasons come from ci/trivy/trivyignore.yaml, the trusted-registry
|
|
# list from ci/trivy/data/ksv0125.yaml.
|
|
- name: "Scan rendered manifests"
|
|
run: ci/run-trivy-config.sh "${{ matrix.chart }}"
|
|
|
|
install-test:
|
|
name: "Install-test"
|
|
needs: [detect-changed-charts, build-chart]
|
|
if: needs.detect-changed-charts.outputs.any-install == 'true'
|
|
# The job itself lives in .github/workflows/install-test.yml and is called
|
|
# from here and from install-test-all-charts.yml (Gitea #52). Everything
|
|
# that decides HOW a chart is installed moved there unchanged - the runner
|
|
# label expression, the pinned kubectl with its skew check, envsubst, the
|
|
# containerised-job detection, the per-leg cluster name, the freesailarr VPN
|
|
# credentials. What stayed here is WHICH charts are tested and whether the
|
|
# job runs at all. A copied job would be a second chance to test
|
|
# differently, unnoticed, which is the same argument that gives
|
|
# ci/excluded-charts.txt exactly one parser.
|
|
#
|
|
# "install-charts", not "charts": the changed charts minus the ones in
|
|
# ci/excluded-charts.txt, filtered in detect-changed-charts, which also
|
|
# names every excluded chart of this pull request with its documented
|
|
# reason in its job summary (issue #231). The called workflow's matrix
|
|
# comment carries the rest of that lesson, including why the decision
|
|
# cannot be made per matrix leg instead.
|
|
uses: ./.github/workflows/install-test.yml
|
|
with:
|
|
charts: ${{ needs.detect-changed-charts.outputs.install-charts }}
|
|
# As the 24 publish-<chart>.yml hulls do, and measured to work on this
|
|
# instance (README, "Where this repo lives"). The called workflow declares
|
|
# exactly one secret, OP_CONNECT_TOKEN, and only its freesailarr leg reads
|
|
# it.
|
|
secrets: inherit
|