Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found while preparing the
v2026.09.18tag. Taggingmainas it standswould have published broken charts, silently.
Packaging at the merge commit shows the
synapseartifact contains zerosubcharts, and installing it with
cert-manager.enabled: truerendersnothing. 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
|| trueThe release job ran:
1.
charts/synapse/Chart.lockwas stale. It was generated in #38 beforethe condition was renamed to
cert-manager.enabled. The lock digest covers thedependency entries, so
helm dependency buildrefuses it on any cleancheckout:
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:
Either one alone produces an empty
charts/.|| trueturned both intosilence, and chart-releaser then packaged whatever was on disk.
Why CI never caught it
validate-charts.yamlranhelm 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:helm dependency buildsynapse-*.tgzcert-manager,dragonflycert-manager.enabled=truedragonfly.enabled=truehelm lintpasses on all three charts, and #39's guards still fire (acme +cert-manager refused, both aliases refused, defaults render).
synapse0.10.1 -> 0.10.2,synapse-stack0.12.1 -> 0.12.2 with both pins.