Skip to content

charts: Keep chart READMEs unchanged across releases - #963

Merged
michaellzc merged 2 commits into
mainfrom
wg/plat-876-release-invariant-helm-docs
Oct 21, 2026
Merged

michaellzc merged 2 commits into
mainfrom
wg/plat-876-release-invariant-helm-docs

Conversation

@DaedalusG

@DaedalusG DaedalusG commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Every release rewrites the image digests and registry in values.yaml and the chart version in Chart.yaml, and all three show up in the generated READMEs. That forced the release worker to regenerate the READMEs with its own copy of helm-docs. When that copy drifted from scripts/helm-docs.sh, internal/release-v8.1.0 failed ":book: Verify helm-docs is up-to-date" and had to be fixed by hand. The regeneration also doesn't reach customers. The publish Action packages the chart from the first push to promote/release-*, before the docs commit lands, so the published sourcegraph-8.0.0.tgz README lists the internal registry as the default.

This PR makes the READMEs independent of everything a release changes, so the release worker can drop its helm-docs step entirely (sourcegraph/sourcegraph#17056):

  • Every pinned image defaultTag gets # @default -- `<appVersion>@sha256:<digest>`, pinned by each release, and sourcegraph.image.repository gets # @default -- Sourcegraph's public Docker Hub registry, in the four charts the release updates. The two executor images also get the # -- description helm-docs needs before it honors @default. The registry annotation deliberately avoids the literal registry string, because the release replaces that string everywhere in values.yaml, comments included.
  • The migrator README uses a <chart-version> placeholder instead of {{ template "chart.version" . }}.
  • A new CI step fails if any chart README renders a pinned digest or index.docker.io/sourcegraph. A new image added without an annotation then fails on its own PR rather than on the next release branch. It is a separate step so it doesn't conflict with CI: Make helm-docs.sh portable #954, which rewrites the existing helm-docs step.
  • Published charts still list the exact values. Before helm package, the release and insiders publish workflows run the new scripts/ci/render-published-readmes.sh. It deletes the release-value @default annotations and reruns ./scripts/helm-docs.sh, so each published README lists the same digests and registry as the values.yaml packaged with it. Nothing is committed. Because the docs are built from the commit being published, the README no longer depends on a later docs commit landing first, which was the cause of the wrong v8.0.0 README. The step is continue-on-error: if it fails (for example, the helm-docs download from GitHub fails), the chart still publishes with the committed placeholder README.

The tradeoff is that READMEs viewed in this repository show placeholders instead of exact digests. The branch's values.yaml still has them, and the README in the published chart (helm show readme) shows them in full. The migrator's helm upgrade examples keep the <chart-version> placeholder in the published chart too.

How this was tested

  • Simulated release. CI can't exercise a release, so I reproduced one on a scratch worktree. A temporary test in the sourcegraph repo ran the release worker's own values.yaml rewrite (dev/images.UpdateHelmManifestContent) over the four charts the release touches. It first rewrote them to the internal registry with new tags and digests (the internal phase), then back to the public registry (the promote phase). I bumped Chart.yaml version/appVersion with the same regex as UpdateChartVersions, and ran ./scripts/helm-docs.sh after each phase.

    README lines changed after internal phase after promote phase
    main 68 60
    this branch 0 0

    values.yaml and Chart.yaml changed as expected in both runs. Zero drift on this branch also shows that the release rewrite leaves the # @default -- comments intact.

  • Annotation coverage. After regenerating, no chart README contains a 64-character @sha256: digest or index.docker.io/sourcegraph. A second ./scripts/helm-docs.sh run leaves the tree clean.

  • New CI guard. I ran the new step's command against both trees. It exits 0 on this branch. On main it exits 1 and lists the offending lines, for example sourcegraph.image.repository in charts/sourcegraph-executor/dind/README.md.

  • Publish rendering, end to end. In a scratch worktree I rebuilt a promote branch the way a release would: internal phase, Chart.yaml bump, promote phase. Buildkite's helm-docs check stays clean on that tree. I then ran the publish job's steps with Helm v3.4.0, the version the workflows pin: the new script, then both helm package loops. I extracted each .tgz and compared its README with its packaged values.yaml. The expected values were taken from values.yaml, not from helm-docs output.

    Scenario Digests in packaged values.yaml Missing from packaged README Placeholders left Registry row
    Release publish (4 charts) 24 / 1 / 1 / 1 0 0 "index.docker.io/sourcegraph"
    Insiders publish (4 charts) 24 / 1 / 1 / 1 0 0 "index.docker.io/sourcegraph"
    Render step forced to fail 24 / 1 / 1 / 1 all all placeholder; packaging still succeeds

    For every chart, in all three scenarios, helm template of the packaged chart matches the unmodified chart, after normalizing the migrator Job's random name suffix. The script only touches comments and READMEs. The sourcegraph chart has 25 annotated images but 24 distinct digests, because codeIntelDB and pgsql share one. shellcheck is clean, and actionlint reports the same three pre-existing actions/checkout@v3 findings as on main.

  • CI. On the first commit, Buildkite build 3049 passed, including ":book: Verify helm-docs is up-to-date" and the new ":book: Verify READMEs do not render release-managed values" step. "Lint and Test charts" also passed. The changes to values.yaml are comments only, so rendered manifests are unchanged. The publish workflows only run on pushes to main, release/** and promote/release-**, so this PR's CI can't exercise them. The end-to-end run above stands in for that.

The first real check of the full flow will be the next release once both PRs are deployed.

Merge this, and backport it to every X.Y.x branch that still gets patch releases (at least 8.0.x and 8.1.x), before the release worker change deploys. Patch releases branch from X.Y.x, and a branch without these annotations would fail the helm-docs check once the worker stops regenerating READMEs.

Part of PLAT-876.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

@DaedalusG
DaedalusG marked this pull request as ready for review October 20, 2026 19:52
@DaedalusG
DaedalusG requested a review from a team October 20, 2026 19:52
@michaellzc michaellzc added the backport 8.1.x Backport to 8.1.x release branch label Oct 21, 2026
@michaellzc
michaellzc enabled auto-merge (squash) October 21, 2026 16:49
Every release rewrote image digests, the registry, and the chart version in
the chart READMEs, so the release worker had to regenerate them with a copy
of helm-docs. When that copy drifted from scripts/helm-docs.sh, the release
branch failed the helm-docs check (v8.1.0, PLAT-876).

Annotate the release-managed values in values.yaml with `# @default --`, and
use a `<chart-version>` placeholder in the migrator README. The generated
READMEs no longer depend on anything the release changes, so the release
worker can stop regenerating them. A new CI step fails if a README renders a
pinned digest or the public registry again.

Amp-Thread-ID: https://ampcode.com/threads/T-01a13c8e-c52e-7584-ab27-1a7fb05bc3b7
The committed READMEs show placeholders for release-managed values, so a
release no longer changes them. Published charts should still document the
exact image digests and registry their values.yaml uses.

Before packaging, the release and insiders publish workflows now delete the
release-value `# @default --` annotations and rerun helm-docs, so each
published README lists the same defaults as its packaged values.yaml. The
step is continue-on-error: if it fails, the chart publishes with the
committed placeholder README rather than not at all.

Amp-Thread-ID: https://ampcode.com/threads/T-01a13c8e-c52e-7584-ab27-1a7fb05bc3b7
@michaellzc
michaellzc force-pushed the wg/plat-876-release-invariant-helm-docs branch from 76ec16f to 8315d79 Compare October 21, 2026 16:49
@michaellzc
michaellzc merged commit c36d89e into main Oct 21, 2026
6 checks passed
@michaellzc
michaellzc deleted the wg/plat-876-release-invariant-helm-docs branch October 21, 2026 16:51
@sourcegraph-release-bot

Copy link
Copy Markdown
Collaborator

The backport to 8.1.x failed at https://github.com/sourcegraph/deploy-sourcegraph-helm/actions/runs/39638482689:

Reviews may only be requested from collaborators. One or more of the teams you specified is not a collaborator of the sourcegraph/deploy-sourcegraph-helm repository.

To backport this PR manually, you can either:

Via the sg tool

Use the sg backport command to backport your commit to the release branch.

sg backport -r 8.1.x -p 963
Via your terminal

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-8.1.x 8.1.x
# Navigate to the new working tree
cd .worktrees/backport-8.1.x
# Create a new branch
git switch --create backport-963-to-8.1.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c36d89e37179d08eb682543650602f0fa5ca748a
# Push it to GitHub
git push --set-upstream origin backport-963-to-8.1.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-8.1.x

If you encouter conflict, first resolve the conflict and stage all files, then run the commands below:

git cherry-pick --continue
# Push it to GitHub
git push --set-upstream origin backport-963-to-8.1.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-8.1.x
  • Follow above instructions to backport the commit.
  • Create a pull request where the base branch is 8.1.x and the compare/head branch is backport-963-to-8.1.x., click here to create the pull request.

Once the pull request has been created, please ensure the following:

  • Make sure to tag @sourcegraph/release in the pull request description.

  • kindly remove the release-blocker from this pull request.

michaellzc pushed a commit that referenced this pull request Oct 21, 2026
…#973)

Backport c36d89e from #963

Every release rewrites the image digests and registry in `values.yaml`
and the chart version in `Chart.yaml`, and all three show up in the
generated READMEs. That forced the release worker to regenerate the
READMEs with its own copy of helm-docs. When that copy drifted from
`scripts/helm-docs.sh`, `internal/release-v8.1.0` failed ":book: Verify
helm-docs is up-to-date" and had to be fixed by hand. The regeneration
also doesn't reach customers. The publish Action packages the chart from
the first push to `promote/release-*`, before the docs commit lands, so
the published `sourcegraph-8.0.0.tgz` README lists the internal registry
as the default.

This PR makes the READMEs independent of everything a release changes,
so the release worker can drop its helm-docs step entirely
(sourcegraph/sourcegraph#17056):

- Every pinned image `defaultTag` gets `` # @default --
`<appVersion>@sha256:<digest>`, pinned by each release ``, and
`sourcegraph.image.repository` gets `# @default -- Sourcegraph's public
Docker Hub registry`, in the four charts the release updates. The two
executor images also get the `# -- ` description helm-docs needs before
it honors `@default`. The registry annotation deliberately avoids the
literal registry string, because the release replaces that string
everywhere in `values.yaml`, comments included.
- The migrator README uses a `<chart-version>` placeholder instead of
`{{ template "chart.version" . }}`.
- A new CI step fails if any chart README renders a pinned digest or
`index.docker.io/sourcegraph`. A new image added without an annotation
then fails on its own PR rather than on the next release branch. It is a
separate step so it doesn't conflict with #954, which rewrites the
existing helm-docs step.
- Published charts still list the exact values. Before `helm package`,
the release and insiders publish workflows run the new
`scripts/ci/render-published-readmes.sh`. It deletes the release-value
`@default` annotations and reruns `./scripts/helm-docs.sh`, so each
published README lists the same digests and registry as the
`values.yaml` packaged with it. Nothing is committed. Because the docs
are built from the commit being published, the README no longer depends
on a later docs commit landing first, which was the cause of the wrong
v8.0.0 README. The step is `continue-on-error`: if it fails (for
example, the helm-docs download from GitHub fails), the chart still
publishes with the committed placeholder README.

The tradeoff is that READMEs viewed in this repository show placeholders
instead of exact digests. The branch's `values.yaml` still has them, and
the README in the published chart (`helm show readme`) shows them in
full. The migrator's `helm upgrade` examples keep the `<chart-version>`
placeholder in the published chart too.

## How this was tested

- **Simulated release.** CI can't exercise a release, so I reproduced
one on a scratch worktree. A temporary test in the sourcegraph repo ran
the release worker's own `values.yaml` rewrite
(`dev/images.UpdateHelmManifestContent`) over the four charts the
release touches. It first rewrote them to the internal registry with new
tags and digests (the internal phase), then back to the public registry
(the promote phase). I bumped `Chart.yaml` `version`/`appVersion` with
the same regex as `UpdateChartVersions`, and ran
`./scripts/helm-docs.sh` after each phase.

  | | README lines changed after internal phase | after promote phase |
  |---|---|---|
  | `main` | 68 | 60 |
  | this branch | 0 | 0 |

`values.yaml` and `Chart.yaml` changed as expected in both runs. Zero
drift on this branch also shows that the release rewrite leaves the `#
@default --` comments intact.
- **Annotation coverage.** After regenerating, no chart README contains
a 64-character `@sha256:` digest or `index.docker.io/sourcegraph`. A
second `./scripts/helm-docs.sh` run leaves the tree clean.
- **New CI guard.** I ran the new step's command against both trees. It
exits 0 on this branch. On `main` it exits 1 and lists the offending
lines, for example `sourcegraph.image.repository` in
`charts/sourcegraph-executor/dind/README.md`.
- **Publish rendering, end to end.** In a scratch worktree I rebuilt a
promote branch the way a release would: internal phase, `Chart.yaml`
bump, promote phase. Buildkite's helm-docs check stays clean on that
tree. I then ran the publish job's steps with Helm v3.4.0, the version
the workflows pin: the new script, then both `helm package` loops. I
extracted each `.tgz` and compared its README with its packaged
`values.yaml`. The expected values were taken from `values.yaml`, not
from helm-docs output.

| Scenario | Digests in packaged `values.yaml` | Missing from packaged
README | Placeholders left | Registry row |
  |---|---|---|---|---|
| Release publish (4 charts) | 24 / 1 / 1 / 1 | 0 | 0 |
`"index.docker.io/sourcegraph"` |
| Insiders publish (4 charts) | 24 / 1 / 1 / 1 | 0 | 0 |
`"index.docker.io/sourcegraph"` |
| Render step forced to fail | 24 / 1 / 1 / 1 | all | all | placeholder;
packaging still succeeds |

For every chart, in all three scenarios, `helm template` of the packaged
chart matches the unmodified chart, after normalizing the migrator Job's
random name suffix. The script only touches comments and READMEs. The
`sourcegraph` chart has 25 annotated images but 24 distinct digests,
because `codeIntelDB` and `pgsql` share one. `shellcheck` is clean, and
`actionlint` reports the same three pre-existing `actions/checkout@v3`
findings as on `main`.
- **CI.** On the first commit, [Buildkite build
3049](https://buildkite.com/sourcegraph/deploy-sourcegraph-helm/builds/3049)
passed, including ":book: Verify helm-docs is up-to-date" and the new
":book: Verify READMEs do not render release-managed values" step. "Lint
and Test charts" also passed. The changes to `values.yaml` are comments
only, so rendered manifests are unchanged. The publish workflows only
run on pushes to `main`, `release/**` and `promote/release-**`, so this
PR's CI can't exercise them. The end-to-end run above stands in for
that.

The first real check of the full flow will be the next release once both
PRs are deployed.

Merge this, and backport it to every `X.Y.x` branch that still gets
patch releases (at least `8.0.x` and `8.1.x`), before the release worker
change deploys. Patch releases branch from `X.Y.x`, and a branch without
these annotations would fail the helm-docs check once the worker stops
regenerating READMEs.

Part of
[PLAT-876](https://linear.app/sourcegraph/issue/PLAT-876/find-a-permanent-fix-for-helm-docs-release-failures).

Co-authored-by: Warren Gifford <warrenbruceg@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants