ci: automate helm chart release alongside package release#3439
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.
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.