Skip to content

fix(ci): stop the release from publishing charts with no subcharts - #40

Open
pigri wants to merge 1 commit into
mainfrom
fix/release-vendors-no-subcharts
Open

pigri wants to merge 1 commit into
mainfrom
fix/release-vendors-no-subcharts

Conversation

@pigri

@pigri pigri commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Found while preparing the v2026.09.18 tag. Tagging main as it stands
would have published broken charts, silently.

Packaging at the merge commit shows the synapse artifact contains zero
subcharts, and installing it with cert-manager.enabled: true renders
nothing. Dragonfly the same. Nothing errors, because a gated subchart that
is simply absent is indistinguishable from one that is turned off — which is
the same silent-failure shape #39 was written to guard against.

Two independent failures, both swallowed by one || true

The release job ran:

helm dependency build "$chart" || true

1. charts/synapse/Chart.lock was stale. It was generated in #38 before
the condition was renamed to cert-manager.enabled. The lock digest covers the
dependency entries, so helm dependency build refuses it on any clean
checkout:

Error: the lock file (Chart.lock) is out of sync with the dependencies file

2. The release job never added the jetstack repo. It added only
valkey — a repository nothing has depended on since Dragonfly replaced it.
Without jetstack, cert-manager cannot be resolved even from a correct lock:

Error: no repository definition for https://charts.jetstack.io

Either one alone produces an empty charts/. || true turned both into
silence, and chart-releaser then packaged whatever was on disk.

Why CI never caught it

validate-charts.yaml ran helm dependency build || helm dependency update.
The fallback regenerated the stale lock in place, so every check — including
the ones on #38 and #39 — ran against a repaired tree, not the tree being
merged. Reviewing a repaired tree tells you nothing about what ships.

Both workflows now add jetstack and build strictly, with no fallback and no
|| true, so drift fails at review time and can never reach a tag.

Verification

Reproduced the release environment exactly — clean checkout at the merge
commit, isolated HELM_REPOSITORY_CONFIG, only the repos the workflow adds:

before after
helm dependency build fails (both causes) builds all 3 charts
subcharts in synapse-*.tgz 0 cert-manager, dragonfly
published artifact, cert-manager.enabled=true renders nothing renders
published artifact, dragonfly.enabled=true renders nothing renders

helm lint passes on all three charts, and #39's guards still fire (acme +
cert-manager refused, both aliases refused, defaults render).

synapse 0.10.1 -> 0.10.2, synapse-stack 0.12.1 -> 0.12.2 with both pins.

Tagging main as it stands would have published synapse with an EMPTY charts/
directory. Verified by packaging at the merge commit: the artifact contains
zero subcharts, and installing it with `cert-manager.enabled: true` renders
nothing at all. A gated subchart that is simply absent is indistinguishable
from one that is turned off, so nothing anywhere reports a problem.

Two independent failures, both swallowed by the same `|| true`:

1. charts/synapse/Chart.lock was stale. It was generated before the
   cert-manager condition was renamed to `cert-manager.enabled`, and the lock
   digest covers the dependency entries, so `helm dependency build` refused it
   on any clean checkout. Regenerated here.

2. The release job only ran `helm repo add valkey` - a repository nothing has
   depended on since Dragonfly replaced it - and never added jetstack. Without
   it cert-manager cannot be resolved at all, even from a correct lock.

The release job now adds jetstack and builds strictly, so either failure stops
the release instead of quietly shrinking the artifact.

PR CI gets the same treatment, and for the same reason it stayed green through
both: its `helm dependency build || helm dependency update` fallback rewrote
the stale lock in place, so every check ran against a repaired tree rather than
the one being merged.

synapse 0.10.1 -> 0.10.2, synapse-stack 0.12.1 -> 0.12.2 with both pins.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant