fix(helm): support secrets.existingSecret for core secrets #2860
Conversation
When users set secrets.enabled=false to use an external secret via secrets.existingSecret, the environment variables SESSION_SECRET, MAGIC_LINK_SECRET, ENCRYPTION_KEY, and MANAGED_WORKER_SECRET were not being populated from the secret. The templates only checked `if .Values.secrets.enabled` before setting these env vars. This change updates the condition to also check for `secrets.existingSecret`, so users can reference an existing Kubernetes Secret instead of having the chart create one. Fixes #2859 Co-authored-by: nicktrn <[email protected]>
|
WalkthroughTwo Helm templates were changed to broaden the condition that injects secret-related environment variables: both supervisor.yaml (MANAGED_WORKER_SECRET) and webapp.yaml (SESSION_SECRET and related envs) now include these vars when either Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
PR Review: fix(helm): support secrets.existingSecret for core secretsSummaryThis PR fixes a critical bug where users who set Code Quality and Best PracticesExcellent aspects:
Validation:
Potential Bugs or IssuesNone identified. The logic is sound:
One minor documentation consideration: The Performance ConsiderationsNo performance impact. This is a template-time condition evaluation that doesn't affect runtime behavior. Security ConcernsNone identified. The fix actually improves security posture by enabling users to:
Test CoverageRecommendation: While there are no automated Helm template tests visible in the PR, manual testing should verify:
Overall AssessmentApprove - This is a well-implemented bug fix that:
The only suggestion would be to consider adding Helm template tests (e.g., using |
Review CompleteYour review story is ready! Comment !reviewfast on this PR to re-generate the story. |
|
@claude check if we could easily add helm chart prereleases for prs like this one. i want to avoid having to manually trigger releases. desired outcome: pr that touches helm chart files -> helm prerelease. note that semver app version selectors like ~4.0.0 should not match prereleases |
|
Claude finished @nicktrn's task in 1m 54s —— View job Helm Chart Prerelease Automation Analysis
SummaryYes, this can be easily implemented. Here's my analysis: Current State
Semver Prerelease Behavior (Good News)Per the semver spec, version constraints like
Users with Recommended ImplementationCreate a new workflow name: 🧭 Helm Chart PR Prerelease
on:
pull_request:
types: [opened, synchronize, reopened]
paths:
- 'hosting/k8s/helm/**'
env:
REGISTRY: ghcr.io
CHART_NAME: trigger
jobs:
lint-and-test:
runs-on: ubuntu-latest
permissions:
contents: read
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Set up Helm
uses: azure/setup-helm@v4
with:
version: "3.18.3"
- name: Build dependencies
run: helm dependency build ./hosting/k8s/helm/
- name: Extract dependency charts
run: |
cd ./hosting/k8s/helm/
for file in ./charts/*.tgz; do echo "Extracting $file"; tar -xzf "$file" -C ./charts; done
- name: Lint Helm Chart
run: helm lint ./hosting/k8s/helm/
- name: Render templates
run: |
helm template test-release ./hosting/k8s/helm/ \
--values ./hosting/k8s/helm/values.yaml \
--output-dir ./helm-output
- name: Validate manifests
uses: docker://ghcr.io/yannh/kubeconform:v0.7.0
with:
entrypoint: '/kubeconform'
args: "-summary -output json ./helm-output"
prerelease:
needs: lint-and-test
runs-on: ubuntu-latest
permissions:
contents: read
packages: write
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Set up Helm
uses: azure/setup-helm@v4
with:
version: "3.18.3"
- name: Build dependencies
run: helm dependency build ./hosting/k8s/helm/
- name: Extract dependency charts
run: |
cd ./hosting/k8s/helm/
for file in ./charts/*.tgz; do echo "Extracting $file"; tar -xzf "$file" -C ./charts; done
- name: Log in to Container Registry
uses: docker/login-action@v3
with:
registry: ${{ env.REGISTRY }}
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}
- name: Generate prerelease version
id: version
run: |
BASE_VERSION=$(grep '^version:' ./hosting/k8s/helm/Chart.yaml | awk '{print $2}')
PR_NUMBER=${{ github.event.pull_request.number }}
RUN_NUMBER=${{ github.run_number }}
PRERELEASE_VERSION="${BASE_VERSION}-pr${PR_NUMBER}.${RUN_NUMBER}"
echo "version=$PRERELEASE_VERSION" >> $GITHUB_OUTPUT
echo "Prerelease version: $PRERELEASE_VERSION"
- name: Update Chart.yaml with prerelease version
run: |
sed -i "s/^version:.*/version: ${{ steps.version.outputs.version }}/" ./hosting/k8s/helm/Chart.yaml
- name: Package Helm Chart
run: |
helm package ./hosting/k8s/helm/ --destination /tmp/
- name: Push Helm Chart to GHCR
run: |
VERSION="${{ steps.version.outputs.version }}"
CHART_PACKAGE="/tmp/${{ env.CHART_NAME }}-${VERSION}.tgz"
helm push "$CHART_PACKAGE" "oci://${{ env.REGISTRY }}/${{ github.repository_owner }}/charts"
- name: Add PR comment with install instructions
uses: actions/github-script@v7
with:
script: |
const version = '${{ steps.version.outputs.version }}';
const body = `### 🧭 Helm Chart Prerelease Published
**Version:** \`${version}\`
**Install:**
\`\`\`bash
helm upgrade --install trigger \\
oci://ghcr.io/${{ github.repository_owner }}/charts/trigger \\
--version "${version}"
\`\`\`
> ⚠️ This is a prerelease for testing. Do not use in production.`;
github.rest.issues.createComment({
issue_number: context.issue.number,
owner: context.repo.owner,
repo: context.repo.repo,
body: body
});Key Features
Alternative: Simpler ApproachIf you want to minimize CI complexity, you could instead just add a Helm lint job to Let me know if you'd like me to implement this workflow! |
|
@claude looks great, can you add that workflow to this pr to test? also, make sure this is based on our existing helm workflow as we know it works. something we'll have to figure out: when we keep pushing to the same branch / pr we shouldn't keep adding comments each time but instead just update the existing comment. don't create a gh release btw, no need |
|
Claude finished @nicktrn's task in 1m 46s —— View job Adding Helm Chart Prerelease Workflow
SummaryI created the workflow file but cannot push it due to GitHub App permissions. The GitHub App does not have The workflow file is ready at Key Features
To apply this workflow, you have two options:Option 1: Add the file manually with this content: Click to expand workflow filename: 🧭 Helm Chart PR Prerelease
on:
pull_request:
types: [opened, synchronize, reopened]
paths:
- "hosting/k8s/helm/**"
env:
REGISTRY: ghcr.io
CHART_NAME: trigger
jobs:
lint-and-test:
runs-on: ubuntu-latest
permissions:
contents: read
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Set up Helm
uses: azure/setup-helm@v4
with:
version: "3.18.3"
- name: Build dependencies
run: helm dependency build ./hosting/k8s/helm/
- name: Extract dependency charts
run: |
cd ./hosting/k8s/helm/
for file in ./charts/*.tgz; do echo "Extracting $file"; tar -xzf "$file" -C ./charts; done
- name: Lint Helm Chart
run: |
helm lint ./hosting/k8s/helm/
- name: Render templates
run: |
helm template test-release ./hosting/k8s/helm/ \
--values ./hosting/k8s/helm/values.yaml \
--output-dir ./helm-output
- name: Validate manifests
uses: docker://ghcr.io/yannh/kubeconform:v0.7.0
with:
entrypoint: "/kubeconform"
args: "-summary -output json ./helm-output"
prerelease:
needs: lint-and-test
runs-on: ubuntu-latest
permissions:
contents: read
packages: write
pull-requests: write
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Set up Helm
uses: azure/setup-helm@v4
with:
version: "3.18.3"
- name: Build dependencies
run: helm dependency build ./hosting/k8s/helm/
- name: Extract dependency charts
run: |
cd ./hosting/k8s/helm/
for file in ./charts/*.tgz; do echo "Extracting $file"; tar -xzf "$file" -C ./charts; done
- name: Log in to Container Registry
uses: docker/login-action@v3
with:
registry: ${{ env.REGISTRY }}
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}
- name: Generate prerelease version
id: version
run: |
BASE_VERSION=$(grep '^version:' ./hosting/k8s/helm/Chart.yaml | awk '{print $2}')
PR_NUMBER=${{ github.event.pull_request.number }}
SHORT_SHA=$(echo "${{ github.event.pull_request.head.sha }}" | cut -c1-7)
PRERELEASE_VERSION="${BASE_VERSION}-pr${PR_NUMBER}.${SHORT_SHA}"
echo "version=$PRERELEASE_VERSION" >> $GITHUB_OUTPUT
echo "Prerelease version: $PRERELEASE_VERSION"
- name: Update Chart.yaml with prerelease version
run: |
sed -i "s/^version:.*/version: ${{ steps.version.outputs.version }}/" ./hosting/k8s/helm/Chart.yaml
- name: Package Helm Chart
run: |
helm package ./hosting/k8s/helm/ --destination /tmp/
- name: Push Helm Chart to GHCR
run: |
VERSION="${{ steps.version.outputs.version }}"
CHART_PACKAGE="/tmp/${{ env.CHART_NAME }}-${VERSION}.tgz"
# Push to GHCR OCI registry
helm push "$CHART_PACKAGE" "oci://${{ env.REGISTRY }}/${{ github.repository_owner }}/charts"
- name: Find existing comment
uses: peter-evans/find-comment@v3
id: find-comment
with:
issue-number: ${{ github.event.pull_request.number }}
comment-author: "github-actions[bot]"
body-includes: "Helm Chart Prerelease Published"
- name: Create or update PR comment
uses: peter-evans/create-or-update-comment@v4
with:
comment-id: ${{ steps.find-comment.outputs.comment-id }}
issue-number: ${{ github.event.pull_request.number }}
body: |
### 🧭 Helm Chart Prerelease Published
**Version:** `${{ steps.version.outputs.version }}`
**Install:**
```bash
helm upgrade --install trigger \
oci://ghcr.io/${{ github.repository_owner }}/charts/trigger \
--version "${{ steps.version.outputs.version }}"
```
> ⚠️ This is a prerelease for testing. Do not use in production.
edit-mode: replaceOption 2: Grant the GitHub App |
🧭 Helm Chart Prerelease PublishedVersion: Install: helm upgrade --install trigger \
oci://ghcr.io/triggerdotdev/charts/trigger \
--version "4.0.5-pr2860.012b943"
|
PR Review: fix(helm): support secrets.existingSecret for core secretsSummaryThis PR fixes issue #2859 where users setting ✅ Code Quality and Best PracticesThe Helm template fix is correct and minimal: The change from: {{- if .Values.secrets.enabled }}to: {{- if or .Values.secrets.enabled .Values.secrets.existingSecret }}is the right approach. The
The GitHub workflow follows existing patterns and is consistent with
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/helm-pr-prerelease.yml:
- Around line 34-37: Remove the whole CI step titled "Extract dependency charts"
that runs the for-loop extracting ./charts/*.tgz; delete that step from the helm
workflow (and the equivalent step in the release workflow) so the pipeline
relies on `helm dependency build` and keeps .tgz files intact for `helm lint`,
`helm template`, and `helm package`; ensure no other steps expect extracted
chart directories and leave the rest of the workflow unchanged.
🧹 Nitpick comments (2)
.github/workflows/helm-pr-prerelease.yml (2)
31-37: Consider extracting common setup steps (optional).The checkout, Helm setup, and dependency build steps are duplicated between both jobs. While this keeps jobs independent, you could optionally refactor common setup into a composite action or use job outputs to reduce duplication.
However, the current approach is straightforward and maintainable, so this is just a suggestion for future improvement.
Also applies to: 71-77
26-29: Consider upgrading to Helm 4.0.4 (the current stable release as of January 2026).Helm 3.18.3 is a valid release, but Helm 4 was released in November 2025, and version 4.0.4 is now the latest stable release. If Kubernetes compatibility with versions 1.30–1.33 is required, Helm 3.18.3 remains suitable; otherwise, evaluate upgrading to the current major version for latest features and support.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/helm-pr-prerelease.yml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2195
File: hosting/k8s/helm/values-production-example.yaml:95-102
Timestamp: 2025-06-25T14:14:11.965Z
Learning: In the Trigger.dev Helm chart production examples, the maintainer prefers to include initial/bootstrap credentials with clear warnings that they should be changed after first login, rather than requiring external secret references that could complicate initial setup. This follows their pattern of providing working examples with explicit security guidance.
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2195
File: hosting/k8s/helm/values.yaml:22-51
Timestamp: 2025-06-25T13:20:17.174Z
Learning: In the Trigger.dev Helm chart values.yaml, the maintainer prefers to use explicit comprehensive warnings for security-sensitive default values rather than implementing secure-by-default behavior that would fail installation. The project uses deterministic default secrets with clear "TESTING ONLY" warnings and instructions for production deployment.
Learnt from: julienvanbeveren
Repo: triggerdotdev/trigger.dev PR: 2417
File: apps/webapp/app/routes/api.v1.projects.$projectRef.envvars.$slug.import.ts:56-61
Timestamp: 2025-08-19T09:49:07.011Z
Learning: In the Trigger.dev codebase, environment variables should default to `isSecret: false` when not explicitly marked as secrets in the syncEnvVars functionality. This is the intended behavior for both regular variables and parent variables.
⏰ 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). (24)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 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 (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: claude-review
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (4)
.github/workflows/helm-pr-prerelease.yml (4)
1-16: LGTM: Workflow configuration is well-structured.The trigger conditions, concurrency control, and environment setup are appropriate for a PR-based Helm prerelease workflow. The path filter ensures the workflow only runs when Helm chart files change, and the concurrency group prevents duplicate runs for the same PR.
86-94: LGTM: Version generation logic is well-designed.The prerelease versioning scheme (
BASE_VERSION-prNUMBER.SHA) is appropriate:
- Follows semver prerelease conventions
- Ensures uniqueness per PR and commit
- Won't match standard semver selectors (as intended per requirements)
112-138: LGTM: PR comment approach correctly implements requirements.The workflow properly:
- Finds existing comments to avoid duplicates when the PR is updated
- Uses
edit-mode: replaceto update rather than append- Provides clear installation instructions with OCI registry syntax
- Includes appropriate warnings about prerelease usage
- Avoids creating GitHub releases (as requested)
This implementation aligns perfectly with the stated requirements.
49-53: No action required — kubeconform:v0.7.0 is the latest stable release as of January 2026.The version specified in the workflow is current.
Likely an incorrect or invalid review comment.
When users set
secrets.enabled=falseto use an external secret viasecrets.existingSecret, the environment variablesSESSION_SECRET,MAGIC_LINK_SECRET,ENCRYPTION_KEY, andMANAGED_WORKER_SECRETwere not being populated from the secret.Fixes #2859
Also adds automatic helm prereleases for PRs