Skip to content

ci: publish released charts to GHCR as OCI artifacts - #297

Open
jordan-simonovski wants to merge 2 commits into
mainfrom
jordansimonovski/oci-publish
Open

jordan-simonovski wants to merge 2 commits into
mainfrom
jordansimonovski/oci-publish

Conversation

@jordan-simonovski

Copy link
Copy Markdown
Contributor

Released charts are now also pushed to oci://ghcr.io/clickhouse/charts, so users can run helm install my-clickstack oci://ghcr.io/clickhouse/charts/clickstack without adding the GitHub Pages repo. The path matches other public ClickHouse charts on GHCR, such as charts/clickhouse-ruler; the operator chart this repo depends on (clickhouse-operator-helm) is also pulled from GHCR.

Backwards compatibility

  • The GitHub Pages repo and helm repo add keep working unchanged.
  • The first release run after merge pushes the current versions, clickstack 3.5.0 and clickstack-operators 1.1.0. Older versions are not backfilled.

Key decisions

  • GHCR gets the .tgz attached to each GitHub release, not a fresh package of the current commit. Both charts have changed on main since their last release without a version bump, so repackaging would publish different contents under an existing version.
  • Every run checks each chart's current version against GHCR and pushes it if missing, so a push that fails on one run is retried on the next.

Other impact

  • New GHCR packages start private. After the first release, an org admin needs to make charts/clickstack and charts/clickstack-operators public, or anonymous pulls return 403.
  • README install instructions stay on the Pages repo until the packages are public; the OCI command goes in a follow-up.
  • The release workflow gains packages: write.
Implementation detail
  • Runs after chart-releaser, only on runs that publish (no pending changesets).
  • Skips a chart whose <name>-<version> GitHub release does not exist yet.
  • If the GHCR existence check fails for a reason other than the version being absent, such as a registry outage, the step pushes again. The chart bytes are identical, but the manifest digest changes.
  • Tested against a local registry:2: the first run pushed both charts, the second skipped both, and the pulled chart was byte-identical to the release asset. Not yet run on GitHub Actions.

@jordan-simonovski
jordan-simonovski requested a review from a team as a code owner November 5, 2026 22:49
@changeset-bot

changeset-bot Bot commented Nov 5, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 95e871c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Deep Review

Single-file CI change: a new Push charts to GHCR step in .github/workflows/release.yml that publishes already-released chart .tgz assets to oci://ghcr.io/clickhouse/charts, guarded by the same hasChangesets == 'false' publish gate as chart-releaser. No chart templates, values.yaml, or tests are touched.

Verified against ground truth: the tag="$name-$version" and -p "$tag.tgz" naming matches helm/chart-releaser-action defaults; the glob charts/*/Chart.yaml matches exactly the two top-level charts; the token is piped via --password-stdin (never in argv); and the GitHub Actions default run: shell already supplies -e -o pipefail. No secret leak, no render/parse break, no auth bypass.

✅ No critical issues found. No P0/P1 findings.

🟡 P2 — recommended

  • .github/workflows/release.yml:77 — The GHCR existence probe helm show chart ... --version returns non-zero both when a version is genuinely absent and on transient failures (registry outage, rate limit, auth blip), so the else branch re-downloads and re-pushes an already-published version, minting a new OCI manifest digest for identical chart bytes.
    • Fix: Distinguish "version absent" from "probe errored" (e.g. inspect the failure, or query tag existence separately) and skip the push unless the version is confirmed missing.
🔵 P3 nitpicks (5)
  • .github/workflows/release.yml:68 — The inline run: block omits set -euo pipefail; the runner default already enforces -e -o pipefail, leaving only -u (nounset) absent versus the AGENTS.md "Shell Script Conventions" bar.
    • Fix: Add set -euo pipefail as the first line of the block; the step relies only on runner-provided vars so -u is safe.
    • project-standards
  • .github/workflows/release.yml:70 — pkgs=$(mktemp -d) is never removed; harmless on an ephemeral runner but leaves an untracked temp dir for the rest of the job.
    • Fix: Add trap 'rm -rf "$pkgs"' EXIT after the mktemp.
  • .github/workflows/release.yml:14 — packages: write is granted workflow-wide; functionally equivalent today since the workflow has a single job, but any job added later inherits the write scope unnecessarily.
    • Fix: Move the packages: write grant to the release job's permissions: block.
    • security
  • .github/workflows/release.yml:64 — The if: steps.changesets.outputs.hasChangesets == 'false' guard is now repeated across three sequential publish steps.
    • Fix: Consider grouping the publish steps or extracting the gate to reduce drift risk if the condition ever changes.
  • .github/workflows/release.yml:63 — The PR carries no changeset (flagged by changeset-bot); confirm that is intentional for a CI-only change per the AGENTS.md "Add a changeset for user-facing changes" convention, since adding an OCI install channel is arguably user-facing.
    • Fix: Either add a changeset or confirm the CI-only change is intentionally exempt.
    • previous-comments

Reviewers (7): correctness, security, reliability, maintainability, testing, project-standards, previous-comments. Security and project-standards returned from subagents; the other five dimensions were analyzed directly by the orchestrator against ground truth when their subagents did not return within the review window.

Testing gaps: No automated coverage for the new release shell step — helm-unittest does not apply to workflow YAML, and the repo has no workflow-test harness, so correctness rests on the author's manual registry:2 run (not yet exercised on GitHub Actions). This is an inherent gap for CI-only changes, not a blocker.

No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR.

This branch has not been deployed

No deployments
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