Skip to content

Commit b42689d

Browse files
authored
feat(helm): derive chart appVersion from the release, and lint with ct (#7643)
* 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. * feat(helm): derive chart appVersion from the release instead of gating on it appVersion is what the chart's image tags default to, so a stale one publishes a chart that installs an older Sim than the release it ships with, and images.yaml -- the list an operator mirrors into a disconnected registry -- names the wrong tags with it. Published chart versions are immutable, so each stale value is frozen the moment it ships. It was bumped by hand, drifted forty releases, and drifted twice more after a check started catching it. That is the tell: the check could refuse to publish but could not supply the value, so the only thing it reliably produced was a red build on every release and a chart that never shipped. Only 1.11.0 was ever published for exactly this reason. The publish jobs now derive it. On a release merge the tag does not exist yet -- this commit is what cuts it -- so the version comes from the commit subject, and from the latest release for every other push. Both publish paths run the same script before packaging, so the OCI artifact and the HTTP repo cannot disagree. Removes the post-merge check, which is now unreachable by construction, and does not replace it with the pre-merge title gate that was considered: that one guarded ground truth with a heuristic, and its failure mode was passing silently. Precedent is cert-manager, whose chart also lives in its application repo and which injects the version at package time. The projects that commit the value and bump by hand -- argo-cd, ingress-nginx, prometheus-community -- all keep the chart in a separate repository, where a human is already editing Chart.yaml as the unit of change. We are the former shape. Verified the resolution across six subjects: release commit, multi-line body carrying a decoy version, ordinary push, leading whitespace, shell metacharacters (no expansion), and a version that is not at the start. Also verified the script's exit codes directly rather than through a pipe, since a gate that cannot fail is the thing being replaced here. Sets appVersion to v0.8.26, two releases ahead of where it was stuck, and regenerates images.yaml with it. * fix(helm): match ci.yml's release predicate and resolve the version once Two review findings, both real. The leading-whitespace tolerance was wrong here. detect-version in ci.yml is what actually cuts the tag and builds the images, and it anchors the version at the first character. Accepting a subject it rejects meant the chart could publish naming a release that was never created, pointing at images that do not exist. The pattern is now identical to ci.yml's, with a comment saying it has to stay that way. The tolerance came from a PR-title parsing problem in an earlier design; commit subjects have a different authority. The two publish jobs also resolved the version independently, and they run in parallel. A release becoming public between those two API calls would package the same immutable chart version with different appVersions, so the OCI artifact and the HTTP repo would install different Sims. Resolution moved to one job both consume, which also removes the duplicated logic. Separately, --check only inspected Chart.yaml while the script writes both that and the inventory derived from it, so it could report success over a stale inventory. It now verifies both halves; confirmed it exits 1 when the inventory is stale and 0 once regenerated.
1 parent 6954859 commit b42689d

7 files changed

Lines changed: 303 additions & 161 deletions

File tree

.github/workflows/helm.yml

Lines changed: 106 additions & 76 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
@@ -195,6 +193,49 @@ jobs:
195193
- name: Run helm test
196194
run: helm test sim --namespace sim --timeout 5m
197195

196+
# Resolved once and shared, so the two publish paths cannot package the same
197+
# immutable chart version with different appVersions -- they run in parallel,
198+
# and a release becoming public between two independent API calls would be
199+
# enough to make the OCI artifact and the HTTP repo install different Sims.
200+
release-version:
201+
name: Resolve the app release
202+
if: github.event_name == 'push' && github.ref == 'refs/heads/main' && github.repository == 'simstudioai/sim'
203+
runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-2vcpu-ubuntu-2404' || 'ubuntu-latest' }}
204+
timeout-minutes: 5
205+
permissions:
206+
contents: read # Read the release list.
207+
outputs:
208+
version: ${{ steps.resolve.outputs.version }}
209+
steps:
210+
# On a release merge the tag does not exist yet -- this commit is what cuts
211+
# it -- so the subject is the only source available, and the latest release
212+
# is the source of truth for every other push.
213+
#
214+
# The subject pattern MUST stay identical to detect-version in ci.yml, which
215+
# is what actually creates the tag and builds the images. If this one
216+
# matched a subject that one rejects, the chart would publish naming a
217+
# release that was never cut.
218+
- name: Resolve the app release
219+
id: resolve
220+
env:
221+
HEAD_COMMIT_MESSAGE: ${{ github.event.head_commit.message }}
222+
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
223+
run: |
224+
set -euo pipefail
225+
subject=${HEAD_COMMIT_MESSAGE%%$'\n'*}
226+
if [[ "$subject" =~ ^(v[0-9]+\.[0-9]+\.[0-9]+): ]]; then
227+
resolved="${BASH_REMATCH[1]}"
228+
echo "Release commit; shipping the chart with ${resolved}."
229+
else
230+
resolved=$(gh api "repos/${GITHUB_REPOSITORY}/releases/latest" --jq .tag_name)
231+
echo "Not a release commit; shipping the chart with the latest release ${resolved}."
232+
fi
233+
if [ -z "$resolved" ]; then
234+
echo "::error::Could not resolve an app release to ship this chart with."
235+
exit 1
236+
fi
237+
echo "version=${resolved}" >> "$GITHUB_OUTPUT"
238+
198239
# Publishes the chart to GHCR as an OCI artifact. Self-hosters cannot admit a
199240
# chart pulled from a git checkout — they need an immutable, versioned artifact
200241
# they can pin by digest and mirror into an internal registry — so shipping the
@@ -206,7 +247,7 @@ jobs:
206247
# A separate workflow would race those instead of waiting for them.
207248
publish:
208249
name: Publish chart to GHCR
209-
needs: [chart, install]
250+
needs: [chart, install, release-version]
210251
if: github.event_name == 'push' && github.ref == 'refs/heads/main' && github.repository == 'simstudioai/sim'
211252
runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-2vcpu-ubuntu-2404' || 'ubuntu-latest' }}
212253
timeout-minutes: 15
@@ -234,6 +275,19 @@ jobs:
234275
username: ${{ github.repository_owner }}
235276
password: ${{ secrets.GITHUB_TOKEN }}
236277

278+
- name: Setup Bun
279+
uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2
280+
with:
281+
bun-version: 1.4.1
282+
283+
# Derives appVersion rather than checking it, so the published chart cannot
284+
# be pinned to an older Sim than the release it ships with. The committed
285+
# value is kept current too, but nothing depends on a human remembering.
286+
- name: Sync chart appVersion to the release
287+
env:
288+
APP_VERSION: ${{ needs.release-version.outputs.version }}
289+
run: bun run scripts/sync-chart-appversion.ts --version "${APP_VERSION}"
290+
237291
- name: Package chart
238292
id: package
239293
run: |
@@ -249,44 +303,6 @@ jobs:
249303
echo "repository=ghcr.io/${GITHUB_REPOSITORY_OWNER}/charts/${name}"
250304
} >> "$GITHUB_OUTPUT"
251305
252-
# `appVersion` is what the image tags default to, so a stale one publishes
253-
# a chart that silently installs an old Sim -- and because published chart
254-
# versions are immutable, every stale value is frozen forever. It sat six
255-
# releases behind before this check existed, bumped only by hand.
256-
#
257-
# BEHIND is the failure. AHEAD is normal and must not be blocked: a
258-
# version tag is cut by the main-branch merge commit that releases it
259-
# (detect-version in ci.yml), so appVersion legitimately names a release
260-
# that does not exist yet while that release is still being built. Failing
261-
# 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
263-
# circularity, and it is why appVersion went unbumped for so long.
264-
#
265-
# Compares against the latest GitHub release rather than a hardcoded value
266-
# so the check cannot go stale itself. Prereleases and drafts are excluded:
267-
# the `/releases/latest` endpoint already returns neither.
268-
- name: appVersion does not lag the app release
269-
env:
270-
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
271-
run: |
272-
set -euo pipefail
273-
app_version=$(helm show chart helm/sim | awk '/^appVersion:/ {print $2}' | tr -d '"')
274-
latest=$(gh api "repos/${GITHUB_REPOSITORY}/releases/latest" --jq .tag_name)
275-
if [ -z "$latest" ]; then
276-
echo "::error::Could not resolve the latest release; refusing to publish unverified."
277-
exit 1
278-
fi
279-
if [ "$app_version" = "$latest" ]; then
280-
echo "appVersion ${app_version} matches the latest release."
281-
exit 0
282-
fi
283-
oldest=$(printf '%s\n%s\n' "$app_version" "$latest" | sort -V | head -1)
284-
if [ "$oldest" = "$app_version" ]; then
285-
echo "::error::Chart.yaml appVersion is ${app_version} but the latest release is ${latest}. Bump appVersion (and the chart version) so the chart does not publish an install pinned to an older Sim."
286-
exit 1
287-
fi
288-
echo "::notice::appVersion ${app_version} is ahead of the latest release ${latest}, which is expected while that release is still being cut."
289-
290306
# Chart versions are immutable once published: whoever pinned a version
291307
# must keep resolving the same bytes forever. The PR gate above already
292308
# forces a version bump on every chart change, so a version that is
@@ -424,7 +440,7 @@ jobs:
424440
# the job holding the signing identity.
425441
publish-http:
426442
name: Publish chart to the Helm repo
427-
needs: [chart, install]
443+
needs: [chart, install, release-version]
428444
if: github.event_name == 'push' && github.ref == 'refs/heads/main' && github.repository == 'simstudioai/sim'
429445
runs-on: ${{ (vars.CI_PROVIDER == '' || vars.CI_PROVIDER == 'blacksmith') && 'blacksmith-2vcpu-ubuntu-2404' || 'ubuntu-latest' }}
430446
timeout-minutes: 15
@@ -454,6 +470,20 @@ jobs:
454470
echo "::warning::No gh-pages branch, so the HTTP chart repo was not updated. Create it and point GitHub Pages at it to activate this job. The OCI publish is unaffected."
455471
fi
456472
473+
- name: Setup Bun
474+
uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2
475+
with:
476+
bun-version: 1.4.1
477+
478+
# Derives appVersion rather than checking it, so the published chart cannot
479+
# be pinned to an older Sim than the release it ships with. The committed
480+
# value is kept current too, but nothing depends on a human remembering.
481+
- name: Sync chart appVersion to the release
482+
if: steps.pages.outputs.exists == 'true'
483+
env:
484+
APP_VERSION: ${{ needs.release-version.outputs.version }}
485+
run: bun run scripts/sync-chart-appversion.ts --version "${APP_VERSION}"
486+
457487
- name: Configure Git
458488
if: steps.pages.outputs.exists == 'true'
459489
env:

helm/sim/Chart.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ apiVersion: v2
22
name: sim
33
description: A Helm chart for Sim - the open-source AI workspace where teams build, deploy, and manage AI agents
44
type: application
5-
version: 1.11.1
6-
appVersion: "v0.8.24"
5+
version: 1.11.2
6+
appVersion: "v0.8.26"
77
kubeVersion: ">=1.25.0-0"
88
home: https://sim.ai
99
icon: https://raw.githubusercontent.com/simstudioai/sim/main/apps/sim/public/logo/primary/primary.svg

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

helm/sim/images.yaml

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,22 +22,22 @@
2222
# render it twice. That override also CHANGES where the chart pulls from, to
2323
# `<your-registry>/nvidia/k8s-device-plugin` — mirror the device plugin there
2424
# instead of to the `mirror` path listed below, or the pull fails.
25-
appVersion: v0.8.24
25+
appVersion: v0.8.26
2626
images:
2727
- source: busybox:1.36
2828
mirror: busybox:1.36
2929
- source: curlimages/curl:8.5.0
3030
mirror: curlimages/curl:8.5.0
31-
- source: ghcr.io/simstudioai/copilot:v0.8.24
32-
mirror: simstudioai/copilot:v0.8.24
33-
- source: ghcr.io/simstudioai/migrations:v0.8.24
34-
mirror: simstudioai/migrations:v0.8.24
35-
- source: ghcr.io/simstudioai/pii:v0.8.24
36-
mirror: simstudioai/pii:v0.8.24
37-
- source: ghcr.io/simstudioai/realtime:v0.8.24
38-
mirror: simstudioai/realtime:v0.8.24
39-
- source: ghcr.io/simstudioai/simstudio:v0.8.24
40-
mirror: simstudioai/simstudio:v0.8.24
31+
- source: ghcr.io/simstudioai/copilot:v0.8.26
32+
mirror: simstudioai/copilot:v0.8.26
33+
- source: ghcr.io/simstudioai/migrations:v0.8.26
34+
mirror: simstudioai/migrations:v0.8.26
35+
- source: ghcr.io/simstudioai/pii:v0.8.26
36+
mirror: simstudioai/pii:v0.8.26
37+
- source: ghcr.io/simstudioai/realtime:v0.8.26
38+
mirror: simstudioai/realtime:v0.8.26
39+
- source: ghcr.io/simstudioai/simstudio:v0.8.26
40+
mirror: simstudioai/simstudio:v0.8.26
4141
- source: nvcr.io/nvidia/k8s-device-plugin:v0.18.2
4242
mirror: nvcr.io/nvidia/k8s-device-plugin:v0.18.2
4343
- source: ollama/ollama:0.23.2

0 commit comments

Comments
 (0)