Skip to content

Commit affff42

Browse files
waleedlatif1claude
andcommitted
improvement(helm): lint the chart with ct instead of hand-rolled checks
`ct lint` (helm/chart-testing) is what the large charts standardize on -- ingress-nginx, prometheus-community and external-secrets all gate on it -- and it subsumes both of the checks this workflow was doing by hand. Its --check-version-increment defaults to true, so the bespoke version-bump job is now redundant and is deleted; on top of the bare `helm lint` it replaces, it adds yamllint over Chart.yaml and every values file, yamale schema validation of Chart.yaml, and maintainer-account validation. Two invocations, because ct couples change detection to the version check. --charts lints unconditionally but DISABLES the version check, so it is used only on pushes, where a merged branch leaves nothing to diff. PRs use --chart-dirs (this chart is at helm/sim, not ct's default charts/) with --target-branch, which is the combination that actually fails a chart change carrying no version bump. Verified both directions against a scratch repo: no bump fails with "Needs a version bump!", bump passes. Adopting it required the chart to satisfy the standard linter: - values.yaml had 66 lines of trailing whitespace and one comment missing its second leading space. Only the comment line is not blank; no value changed. - ci/full-values.yaml used `[{ path: /, ... }]`, which yamllint rejects for the spaces inside braces. Rendered output is byte-identical either way. - maintainers[].name is validated as a real account on the forge, and "Sim Team" is not one. Now the GitHub org, matching how every chart cited above lists its maintainers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017TknQyteeUDn6xi94sH71Y
1 parent 8c73508 commit affff42

4 files changed

Lines changed: 109 additions & 106 deletions

File tree

‎.github/workflows/helm.yml‎

Lines changed: 38 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ jobs:
3939
steps:
4040
- uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4
4141
with:
42+
# ct diffs the chart against the target branch to check the version
43+
# increment, so a shallow clone would have nothing to compare.
44+
fetch-depth: 0
4245
persist-credentials: false
4346

4447
- name: Set up Helm
@@ -64,8 +67,41 @@ jobs:
6467
- name: Image inventory is current
6568
run: bun run images:check
6669

67-
- name: Helm lint
68-
run: helm lint helm/sim --values helm/sim/ci/default-values.yaml
70+
# `ct lint` is the CNCF chart linter (helm/chart-testing) that the large
71+
# charts standardize on. It subsumes `helm lint` and adds what a bare
72+
# helm lint does not check: yamllint over Chart.yaml and every values
73+
# file, a yamale schema validation of Chart.yaml, maintainer-account
74+
# validation, and the chart version increment. It also picks up
75+
# `helm/sim/ci/*-values.yaml` on its own -- that directory name is ct's
76+
# convention, which is why no --values flag is passed here.
77+
- name: Set up chart-testing
78+
uses: helm/chart-testing-action@6ec842c01de15ebb84c8627d2744a0c2f2755c9f # v2.8.0
79+
80+
# Two modes, because ct couples change detection to the version check.
81+
#
82+
# On a PR: --chart-dirs (this chart lives at helm/sim, not ct's default
83+
# `charts/`) plus --target-branch, so ct diffs the chart against the base
84+
# and fails it with "Needs a version bump!" when chart content moved and
85+
# `version:` did not. This is what replaced the hand-rolled gate.
86+
#
87+
# On a push: --charts, which lints unconditionally. Change detection has
88+
# nothing to diff once the branch is merged, so without it a push run
89+
# would report "No chart changes detected" and lint nothing at all.
90+
# --charts also DISABLES the version check -- that is ct's behavior, not
91+
# an oversight, and it is why it cannot be the flag used on PRs.
92+
- name: Chart lint (ct)
93+
env:
94+
EVENT_NAME: ${{ github.event_name }}
95+
BASE_REF: ${{ github.base_ref }}
96+
run: |
97+
set -euo pipefail
98+
args=(--validate-maintainers=true)
99+
if [ "$EVENT_NAME" = "pull_request" ]; then
100+
args+=(--chart-dirs helm --target-branch "$BASE_REF")
101+
else
102+
args+=(--charts helm/sim)
103+
fi
104+
ct lint "${args[@]}"
69105
70106
- name: Helm unit tests
71107
run: |
@@ -122,39 +158,6 @@ jobs:
122158
--set externalDatabase.password=ci-dummy-password > /dev/null
123159
done
124160
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-
158161
install:
159162
name: Install on kind and run helm test
160163
needs: chart

‎helm/sim/Chart.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ icon: https://raw.githubusercontent.com/simstudioai/sim/main/apps/sim/public/log
1010
sources:
1111
- https://github.com/simstudioai/sim
1212
maintainers:
13-
- name: Sim Team
13+
- name: simstudioai
1414
email: help@sim.ai
15-
url: https://sim.ai
15+
url: https://github.com/simstudioai
1616
keywords:
1717
- ai
1818
- workflow

‎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

0 commit comments

Comments
 (0)