helm: set default webapp container and add extraVolumes #2222
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
WalkthroughThe changes update the Helm chart version for the "trigger" application from 4.0.0-beta.12 to 4.0.0-beta.14. In the deployment template for the webapp component, a fixed pod annotation ( 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ericallam
approved these changes
Jul 2, 2025
matt-aitken
approved these changes
Jul 2, 2025
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
hosting/k8s/helm/Chart.yaml(1 hunks)hosting/k8s/helm/templates/webapp.yaml(3 hunks)hosting/k8s/helm/values.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#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: nicktrn
PR: triggerdotdev/trigger.dev#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.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: .github/workflows/release-helm.yml:42-46
Timestamp: 2025-06-25T13:24:23.836Z
Learning: In .github/workflows/release-helm.yml, the user nicktrn confirmed that using 'entrypoint' with 'docker://' steps works fine, contrary to previous analysis suggesting it's unsupported.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/templates/extra-manifests.yaml:1-4
Timestamp: 2025-06-25T13:18:04.827Z
Learning: In the Trigger.dev v4 Helm chart, the user prefers to rely on documentation and examples in values files rather than implementing defensive coding in templates, particularly for features like extraManifests where proper usage is documented.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#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
PR: triggerdotdev/trigger.dev#2155
File: docs/docs.json:179-183
Timestamp: 2025-06-06T16:54:23.316Z
Learning: In the docs.json configuration for the Trigger.dev documentation (Mintlify system), both "tags": ["v4"] and "tag": "v4" properties can be used together and work correctly, even though this behavior is undocumented and may not work in local development environments.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.495Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use the `metadata` API from `@trigger.dev/sdk/v3` to attach, update, and access structured metadata within task runs.
hosting/k8s/helm/Chart.yaml (9)
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#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.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#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: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/templates/extra-manifests.yaml:1-4
Timestamp: 2025-06-25T13:18:04.827Z
Learning: In the Trigger.dev v4 Helm chart, the user prefers to rely on documentation and examples in values files rather than implementing defensive coding in templates, particularly for features like extraManifests where proper usage is documented.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#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: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.494Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : You MUST use `@trigger.dev/sdk/v3` when writing Trigger.dev tasks.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2155
File: docs/docs.json:179-183
Timestamp: 2025-06-06T16:54:23.316Z
Learning: In the docs.json configuration for the Trigger.dev documentation (Mintlify system), both "tags": ["v4"] and "tag": "v4" properties can be used together and work correctly, even though this behavior is undocumented and may not work in local development environments.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.495Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Each Trigger.dev task must have a unique `id` within your project.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.495Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Schema tasks must use `schemaTask` from `@trigger.dev/sdk/v3` and validate payloads using a schema (e.g., Zod).
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.495Z
Learning: Applies to trigger.config.ts : Build extensions for Trigger.dev must be configured in the `build` property of `trigger.config.ts` using provided or custom extensions.
hosting/k8s/helm/values.yaml (1)
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/templates/extra-manifests.yaml:1-4
Timestamp: 2025-06-25T13:18:04.827Z
Learning: In the Trigger.dev v4 Helm chart, the user prefers to rely on documentation and examples in values files rather than implementing defensive coding in templates, particularly for features like extraManifests where proper usage is documented.
hosting/k8s/helm/templates/webapp.yaml (3)
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2195
File: hosting/k8s/helm/templates/extra-manifests.yaml:1-4
Timestamp: 2025-06-25T13:18:04.827Z
Learning: In the Trigger.dev v4 Helm chart, the user prefers to rely on documentation and examples in values files rather than implementing defensive coding in templates, particularly for features like extraManifests where proper usage is documented.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#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: nicktrn
PR: triggerdotdev/trigger.dev#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.
⏰ Context from checks skipped due to timeout of 90000ms (25)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
hosting/k8s/helm/templates/webapp.yaml (1)
327-340: Good, flexible injection of extra mounts/volumesThe conditional
withblocks correctly append user-defined mounts and volumes and are nicely indented. Once thevalues.yamlempty-list issue is fixed, this will work out of the box.hosting/k8s/helm/Chart.yaml (1)
5-5: Chart version bump LGTMVersion update to
4.0.0-beta.14cleanly reflects the new functionality.
Comment on lines
+118
to
+126
| # Extra volumes for the webapp pod | ||
| extraVolumes: | ||
| [] | ||
| # - name: config-volume | ||
| # configMap: | ||
| # name: my-config | ||
| # - name: secret-volume | ||
| # secret: | ||
| # secretName: my-secret |
There was a problem hiding this comment.
extraVolumes is parsed as a string, breaking Helm rendering
Placing the literal [] on the next line indents it as a scalar under the key, not as the empty list you intended.
toYaml will therefore output '[]', and the with block in templates/webapp.yaml will inject a quoted string inside spec.volumes, yielding invalid Kubernetes YAML.
- extraVolumes:
- []
+ # Extra volumes for the webapp pod
+ extraVolumes: []Apply the same pattern used by numerous stable charts (key: []) to ensure the value is an actual empty sequence.
🤖 Prompt for AI Agents
In hosting/k8s/helm/values.yaml around lines 118 to 126, the extraVolumes key is
assigned a literal '[]' on the next line, causing it to be parsed as a string
instead of an empty list, which breaks Helm rendering. Fix this by assigning
extraVolumes directly to an empty list using the inline syntax (extraVolumes:
[]) without indentation or quotes, matching the pattern used in stable charts to
ensure it is parsed as an actual empty sequence.
Comment on lines
+128
to
+136
| # Extra volume mounts for the webapp container | ||
| extraVolumeMounts: | ||
| [] | ||
| # - name: config-volume | ||
| # mountPath: /etc/config | ||
| # readOnly: true | ||
| # - name: secret-volume | ||
| # mountPath: /etc/secrets | ||
| # readOnly: true |
There was a problem hiding this comment.
Same issue for extraVolumeMounts
The misplaced [] will be treated as a string, leading to a quoted entry under volumeMounts and a failed deployment.
- extraVolumeMounts:
- []
+ # Extra volume mounts for the webapp container
+ extraVolumeMounts: []Fixing both blocks keeps the chart installable even when users do not override these values.
🤖 Prompt for AI Agents
In hosting/k8s/helm/values.yaml around lines 128 to 136, the extraVolumeMounts
field is incorrectly set to an empty list using square brackets on a separate
line, which YAML interprets as a string, causing deployment failures. Replace
the line containing [] with a properly formatted empty list by removing the
brackets and ensuring the field is an empty array without quotes or misplaced
characters, so the Helm chart remains installable when users do not override
this value.
Comment on lines
+52
to
+55
| kubectl.kubernetes.io/default-container: webapp | ||
| {{- with .Values.webapp.podAnnotations }} | ||
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Potential duplicate-key collision in metadata.annotations
A hard-coded kubectl.kubernetes.io/default-container annotation is added before user-supplied podAnnotations. If a user also defines this key, Helm will render two identical keys and fail with:
Error: YAML parse error on ...: found duplicate key "kubectl.kubernetes.io/default-container"
Consider merging the maps so the user can still override when necessary:
- kubectl.kubernetes.io/default-container: webapp
- {{- with .Values.webapp.podAnnotations }}
- {{- toYaml . | nindent 8 }}
- {{- end }}
+ {{- $base := dict "kubectl.kubernetes.io/default-container" "webapp" }}
+ {{- $anns := mergeOverwrite $base (default dict .Values.webapp.podAnnotations) }}
+ {{- toYaml $anns | nindent 8 }}This keeps the default while remaining user-configurable.
🤖 Prompt for AI Agents
In hosting/k8s/helm/templates/webapp.yaml around lines 52 to 55, the annotation
kubectl.kubernetes.io/default-container is hard-coded before user
podAnnotations,
which can cause duplicate key errors if the user also sets this annotation. To
fix,
merge the default annotation with the user-supplied podAnnotations map so that
the
user can override the default value. Use Helm's merge function or a similar
approach
to combine the maps before rendering them, ensuring no duplicate keys appear.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since adding the token sync sidecar any kubectl commands would default to this instead of the webapp container. This broke most kubectl commands listed in the docs 🫠
Also adds webapp.extraVolumes and webapp.extraVolumeMounts