Skip to content

Commit b5757ad

Browse files
committed
improvement(helm): lint the chart with ct instead of hand-rolled checks
Replaces `helm lint` and the bespoke version-bump job with chart-testing, the CNCF chart linter that ingress-nginx, prometheus-community and external-secrets all gate on. It subsumes both: `helm lint` plus yamllint over Chart.yaml and every values file, Chart.yaml schema validation, and --check-version-increment, which is on by default and is exactly what the 33 lines of bash were reimplementing. One trap worth recording. Passing `--charts` silently DISABLES the version check -- it prints "Version increment checking disabled." and still exits 0, so wiring it that way would have swapped a working gate for one that can never fail. The PR path uses `--chart-dirs helm --target-branch <base>`; a push has no base to diff, so `--charts` is correct there. Verified both directions in a scratch repo with a real remote: content changed without a bump gives "chart version not ok. Needs a version bump!" and exit 1, the same change with a bump gives "Chart version ok." Three chart fixes ct's yamllint required: 66 lines of trailing whitespace in values.yaml, one inline comment a space short of the two yamllint wants, and brace spacing in ci/full-values.yaml. Nothing but whitespace -- no non-comment line changed, and both rendered manifest sets are byte-identical before and after. Renames ci/kind-values.yaml to ci/kind-overlay.yaml. ct treats every ci/*-values.yaml as a standalone values set, but that file is a partial layered on default-values.yaml, so linting it alone tripped the chart's own required-secret guards. The new name is outside the glob and the header says why. Maintainer validation stays off: it resolves maintainers[].name against real forge accounts and ours is the display name "Sim Team", so enabling it would change what Artifact Hub shows.
1 parent fbcf13e commit b5757ad

4 files changed

Lines changed: 111 additions & 109 deletions

File tree

.github/workflows/helm.yml

Lines changed: 35 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,19 @@ jobs:
3939
steps:
4040
- uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4
4141
with:
42+
# ct diffs the chart against the PR base to decide whether the version
43+
# was bumped, so a shallow clone would leave it nothing to compare.
44+
fetch-depth: 0
4245
persist-credentials: false
4346

4447
- name: Set up Helm
4548
uses: azure/setup-helm@1a275c3b69536ee54be43f2070a358922e12c8d4 # v4
4649
with:
4750
version: v3.16.4
4851

52+
- name: Set up chart-testing
53+
uses: helm/chart-testing-action@6ec842c01de15ebb84c8627d2744a0c2f2755c9f # v2.8.0
54+
4955
- name: Setup Bun
5056
uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2
5157
with:
@@ -64,8 +70,33 @@ jobs:
6470
- name: Image inventory is current
6571
run: bun run images:check
6672

67-
- name: Helm lint
68-
run: helm lint helm/sim --values helm/sim/ci/default-values.yaml
73+
# ct is the CNCF chart linter (ingress-nginx, prometheus-community and
74+
# external-secrets all gate on it). Beyond `helm lint` it runs yamllint over
75+
# Chart.yaml and every values file, validates Chart.yaml against a schema,
76+
# and — the reason the hand-rolled version-bump job is gone — enforces that
77+
# the chart version increases whenever chart content changes.
78+
#
79+
# `--chart-dirs helm --target-branch` is load-bearing. Passing `--charts`
80+
# instead silently DISABLES the version-increment check ("Version increment
81+
# checking disabled.") and still exits 0, which would leave a gate that
82+
# never fails. On a push there is no base to diff, so `--charts` is correct
83+
# there and the version check simply does not apply.
84+
#
85+
# Maintainer validation is off because it resolves `maintainers[].name`
86+
# against real forge accounts, and ours is the display name "Sim Team".
87+
# Turning it on means changing what Artifact Hub shows.
88+
- name: Chart lint (ct)
89+
env:
90+
BASE_REF: ${{ github.base_ref }}
91+
run: |
92+
set -euo pipefail
93+
args=(--validate-maintainers=false)
94+
if [ "${GITHUB_EVENT_NAME}" = "pull_request" ]; then
95+
args+=(--chart-dirs helm --target-branch "${BASE_REF}")
96+
else
97+
args+=(--charts helm/sim)
98+
fi
99+
ct lint "${args[@]}"
69100
70101
- name: Helm unit tests
71102
run: |
@@ -122,39 +153,6 @@ jobs:
122153
--set externalDatabase.password=ci-dummy-password > /dev/null
123154
done
124155
125-
version-bump:
126-
name: Chart version bumped
127-
if: github.event_name == 'pull_request'
128-
runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-2vcpu-ubuntu-2404' || 'ubuntu-latest' }}
129-
timeout-minutes: 5
130-
steps:
131-
- uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4
132-
with:
133-
fetch-depth: 0
134-
# The version gate only reads history and fetches a public branch, so
135-
# it never needs the token left behind in .git/config.
136-
persist-credentials: false
137-
- name: Require a Chart.yaml version bump when chart content changes
138-
env:
139-
BASE_REF: ${{ github.base_ref }}
140-
run: |
141-
set -euo pipefail
142-
base="origin/${BASE_REF}"
143-
git fetch origin "${BASE_REF}"
144-
merge_base=$(git merge-base "$base" HEAD)
145-
changed=$(git diff --name-only "$merge_base" HEAD)
146-
if echo "$changed" | grep -q '^helm/sim/'; then
147-
base_version=$(git show "$merge_base:helm/sim/Chart.yaml" | awk '/^version:/ {print $2}')
148-
head_version=$(awk '/^version:/ {print $2}' helm/sim/Chart.yaml)
149-
echo "base=$base_version head=$head_version"
150-
if [ "$base_version" = "$head_version" ]; then
151-
echo "::error::helm/sim/** changed but Chart.yaml version did not (still $head_version). Bump it per SemVer."
152-
exit 1
153-
fi
154-
else
155-
echo "No chart changes; skipping."
156-
fi
157-
158156
install:
159157
name: Install on kind and run helm test
160158
needs: chart
@@ -180,7 +178,7 @@ jobs:
180178
helm install sim helm/sim \
181179
--namespace sim --create-namespace \
182180
--values helm/sim/ci/default-values.yaml \
183-
--values helm/sim/ci/kind-values.yaml \
181+
--values helm/sim/ci/kind-overlay.yaml \
184182
--wait --timeout 15m
185183
186184
- name: Diagnostics on failure
@@ -259,7 +257,7 @@ jobs:
259257
# (detect-version in ci.yml), so appVersion legitimately names a release
260258
# that does not exist yet while that release is still being built. Failing
261259
# on any mismatch would race that workflow and block the very publish the
262-
# bump was for. `helm/sim/ci/kind-values.yaml` documents the same
260+
# bump was for. `helm/sim/ci/kind-overlay.yaml` documents the same
263261
# circularity, and it is why appVersion went unbumped for so long.
264262
#
265263
# Compares against the latest GitHub release rather than a hardcoded value

helm/sim/ci/full-values.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ ingress:
2121
enabled: true
2222
app:
2323
host: ci.example.com
24-
paths: [{ path: /, pathType: Prefix }]
24+
paths: [{path: /, pathType: Prefix}]
2525
realtime:
2626
host: ci-ws.example.com
27-
paths: [{ path: /, pathType: Prefix }]
27+
paths: [{path: /, pathType: Prefix}]
2828
tls:
2929
enabled: true
3030

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
1-
# CI-only overlay for the kind install test: shrink resource requests so the
2-
# default configuration schedules on a small CI runner. Layered on top of
3-
# ci/default-values.yaml. Dummy sizing — never use in a deployment.
1+
# CI-only OVERLAY for the kind install test. The name deliberately avoids the
2+
# `ci/*-values.yaml` suffix: `ct lint` treats every file matching that glob as a
3+
# standalone values set, and this one is a partial layered on default-values.yaml.
4+
#
5+
# Shrinks resource requests so the default configuration schedules on a small CI
6+
# runner. Layered on top of ci/default-values.yaml. Dummy sizing — never use in
7+
# a deployment.
48

59
# Pin every first-party image to the published :latest rather than letting the
610
# tag default to Chart.AppVersion. appVersion names the release the chart ships

0 commit comments

Comments
 (0)