ci: automate helm chart release alongside package release#3439
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings🔇 Additional comments (2)
WalkthroughAdds automated Helm chart release steps across three workflows. .github/workflows/changesets-pr.yml gains a Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.github/workflows/release-helm.yml (1)
94-102: Harden against script injection by passing inputs viaenv.
${{ inputs.chart_version }}and${{ github.ref_name }}are expanded directly into the shell script, which is the classic GitHub Actions script injection pattern. Practical risk is low here (workflow_dispatch requires write access and tag pushes require similar), but the mitigation is trivial and matches GitHub's security guidance.🔒 Proposed hardening
- name: Extract version from tag or input id: version + env: + CHART_VERSION_INPUT: ${{ inputs.chart_version }} + REF_NAME: ${{ github.ref_name }} run: | - if [ -n "${{ inputs.chart_version }}" ]; then - VERSION="${{ inputs.chart_version }}" + if [ -n "$CHART_VERSION_INPUT" ]; then + VERSION="$CHART_VERSION_INPUT" else - VERSION="${{ github.ref_name }}" + VERSION="$REF_NAME" VERSION="${VERSION#helm-v}" fi echo "version=$VERSION" >> $GITHUB_OUTPUT echo "Releasing version: $VERSION"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release-helm.yml around lines 94 - 102, The script currently expands `${{ inputs.chart_version }}` and `${{ github.ref_name }}` directly into the shell which risks command injection; change the job step to pass these values via environment variables (e.g., set CHART_VERSION and REF_NAME in env) and reference those env vars inside the run block when setting VERSION and writing to GITHUB_OUTPUT; update occurrences of VERSION, `${{ inputs.chart_version }}` and `${{ github.ref_name }}` in the step so the run script uses safe shell variables like `$CHART_VERSION` / `$REF_NAME` instead of interpolated GitHub expressions..github/workflows/changesets-pr.yml (1)
137-142: Consider usingyqfor YAML edits.The
sedapproach works given current Chart.yaml formatting, but it's brittle to formatting changes (e.g., inline comments likeversion: 1.0.0 # ..., quoted values, or a stray trailing space afterversion:), which would silently be clobbered by the.*match.yqwould preserve structure and be explicit about the fields being mutated.♻️ Optional refactor using mikefarah/yq (pre-installed on ubuntu-latest runners)
- name: Bump Chart.yaml run: | set -e VERSION=$(jq -r '.version' packages/cli-v3/package.json) - sed -i "s/^version:.*/version: ${VERSION}/" ./hosting/k8s/helm/Chart.yaml - sed -i "s/^appVersion:.*/appVersion: v${VERSION}/" ./hosting/k8s/helm/Chart.yaml + yq eval -i ".version = \"${VERSION}\"" ./hosting/k8s/helm/Chart.yaml + yq eval -i ".appVersion = \"v${VERSION}\"" ./hosting/k8s/helm/Chart.yamlNote: pin
yqviamikefarah/yqaction if reproducibility matters.Is mikefarah yq pre-installed on GitHub Actions ubuntu-latest runners in 2026?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/changesets-pr.yml around lines 137 - 142, The current "Bump Chart.yaml" step uses brittle sed replacements for Chart.yaml; replace the two sed -i lines that update version and appVersion with mikefarah/yq invocations that set .version and .appVersion explicitly from the VERSION environment variable (ensuring appVersion is prefixed with "v"), and export VERSION from packages/cli-v3/package.json as before; optionally pin/install mikefarah/yq via the official action to guarantee availability and reproducibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/changesets-pr.yml:
- Around line 137-142: The current "Bump Chart.yaml" step uses brittle sed
replacements for Chart.yaml; replace the two sed -i lines that update version
and appVersion with mikefarah/yq invocations that set .version and .appVersion
explicitly from the VERSION environment variable (ensuring appVersion is
prefixed with "v"), and export VERSION from packages/cli-v3/package.json as
before; optionally pin/install mikefarah/yq via the official action to guarantee
availability and reproducibility.
In @.github/workflows/release-helm.yml:
- Around line 94-102: The script currently expands `${{ inputs.chart_version }}`
and `${{ github.ref_name }}` directly into the shell which risks command
injection; change the job step to pass these values via environment variables
(e.g., set CHART_VERSION and REF_NAME in env) and reference those env vars
inside the run block when setting VERSION and writing to GITHUB_OUTPUT; update
occurrences of VERSION, `${{ inputs.chart_version }}` and `${{ github.ref_name
}}` in the step so the run script uses safe shell variables like
`$CHART_VERSION` / `$REF_NAME` instead of interpolated GitHub expressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2bf7b606-4e4b-4081-a39a-63a1fc9bb5d0
📒 Files selected for processing (3)
.github/workflows/changesets-pr.yml.github/workflows/release-helm.yml.github/workflows/release.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (31)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: audit
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2195
File: hosting/k8s/helm/Chart.yaml:1-18
Timestamp: 2025-06-25T13:18:44.103Z
Learning: For the Trigger.dev Helm chart, the chart name should be "trigger" (not "trigger-v4") since this is the first official chart release. Helper templates should use the actual chart name from .Chart.Name rather than hardcoded prefixes.
📚 Learning: 2026-04-15T15:39:06.868Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-15T15:39:06.868Z
Learning: When modifying any public package (`packages/*` or `integrations/*`), add a changeset using `pnpm run changeset:add`.
Applied to files:
.github/workflows/changesets-pr.yml
🔇 Additional comments (3)
.github/workflows/release.yml (2)
145-150: LGTM — mirrors the existing Docker tag pattern.Tag-push with default
GITHUB_TOKENwon't retriggerrelease-helm.yml'spush: tags: helm-v*trigger, which is the intended behavior per the PR description (Helm runs viaworkflow_call, tag serves as a marker).
163-173: LGTM — dependency chain and gating look correct.
needs: [release, publish-docker]ensures the chart is only published after its referenced Docker images exist, andsecrets: inheritpropagatesGITHUB_TOKENfor both the GHCR OCI push and thesoftprops/action-gh-releasestep inrelease-helm.yml..github/workflows/release-helm.yml (1)
132-132: LGTM — tag format now consistent across all invocation paths.
helm-v{version}matches both thepush: tags: helm-v*trigger and the tag pushed fromrelease.yml, so the draft release attaches to the expected tag regardless of how this workflow is invoked.
|
ready |
Wires up automatic Helm chart releases to ride along with the existing changeset-driven package release flow.
Today
Chart.yamlis bumped by hand andrelease-helm.ymlfires only when a human pushes ahelm-v*tag. With this, the changeset release PR also carries aChart.yamlbump so main always matches the published version, andrelease.ymlinvokesrelease-helm.ymlviaworkflow_callafter Docker images are published.helm-v${VERSION}tag is pushed as a marker (same GITHUB_TOKEN trick asv.docker.*). Manualhelm-v*tag flow still works. Chart.yaml consistency check inrelease-helm.ymlis the safety net if the bump job ever drifts.First rollout: the open
changeset-release/mainPR has stale Chart.yaml. Bump it manually on that branch before merging, otherwise the first automated helm release fails at the consistency check.