-
-
Notifications
You must be signed in to change notification settings - Fork 808
fix(helm): external config and secret support #2245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
WalkthroughThis change updates the Helm chart for the "trigger" application from version 4.0.0-beta.16 to 4.0.0-beta.17. It introduces comprehensive enhancements to secret management and external service configuration across PostgreSQL, Redis, ClickHouse, and S3 services. The Helm templates and values are refactored to support referencing existing Kubernetes secrets for credentials, conditional secret creation, and flexible configuration fallbacks. Environment variables in deployment manifests are updated to source sensitive data from secrets when available. Validation logic is adjusted to require either URLs or secrets for external service connections, and new configuration options are added for connectivity checks and SSL certificate management. The documentation is expanded with detailed guidance on configuring external services and secure SSL connections using mounted CA certificates. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (8)
hosting/k8s/helm/templates/secrets.yaml (1)
14-21
: Careful with unintended secret bloat for in-cluster MinIOBecause
s3.auth.effectiveAccessKeyId/SecretAccessKey
falls back torootUser/rootPassword
, every default installation will embed those bootstrap creds in the generated secret – even if the chart user never intends the webapp to access MinIO directly.Not a blocker, just keep the behaviour in mind; if you want to avoid writing root credentials twice you could gate the block behind
and .Values.s3.deploy (or .Values.s3.auth.accessKeyId .Values.s3.auth.existingSecret)
hosting/k8s/helm/templates/webapp.yaml (1)
287-334
: S3 credential selection covers every path but duplicates secret-key names
OBJECT_STORE_ACCESS_KEY_ID
/OBJECT_STORE_SECRET_ACCESS_KEY
pull from
•s3.auth.*
keys,
• or the main chart secret,
• ors3.external.*
existing secret.That’s great, but note the two generated secret keys:
s3-auth-access-key-id s3-access-key-id
Down-stream scripts/config that expect a fixed key name will have to handle both. If practical, converging on a single identifier would reduce surprises.
hosting/k8s/helm/values.yaml (3)
102-105
: Connectivity-check switch is appreciated – document the precedence
webapp.connectivityCheck.postgres
overridespostgres.external.connectivityCheck.host
, but that’s easy to miss. Add a one-liner comment noting the override order so operators aren’t puzzled when the global flag masks the per-service host.
387-400
: Consider surfacingsslMode
directly next to the URL block for discoverabilityMany users look at the
external:
section first. A brief note or pointer to the top-levelconnection.sslMode
field here would save a docs hop, especially for RDS/GCP users hunting for TLS guidance.
570-576
: Existing-secret keys default to generic names – highlight that they’re configurableThe defaults (
access-key-id
,secret-access-key
) are fine, but a small inline reminder that any key name can be chosen by editingaccessKeyIdSecretKey
/secretAccessKeySecretKey
helps clusters that follow stricter naming conventions.hosting/k8s/helm/templates/_helpers.tpl (3)
99-107
: Consider returning a more meaningful value for external PostgreSQLThe hardcoded value "external-postgres" is not an actual hostname and could be misleading. Since this helper is deprecated, consider either:
- Returning an empty string or a clear placeholder like "EXTERNAL"
- Removing this helper entirely if the legacy DATABASE_HOST env var is no longer needed
164-173
: Consider improving TLS logic readabilityThe double negation in line 169 makes the logic harder to follow.
-{{ not (.Values.redis.external.tls.enabled | default false) }} +{{- if (.Values.redis.external.tls.enabled | default false) -}} +{{- false -}} +{{- else -}} +{{- true -}} +{{- end -}}Alternatively, consider renaming the helper to
trigger-v4.redis.tlsEnabled
and inverting the logic to avoid the confusion of a "disabled" flag.
460-471
: Simplify connectivity check logicThe nested conditionals can be simplified using Helm's default function.
-{{- define "trigger-v4.webapp.connectivityCheckEnabled" -}} -{{- $connectivityCheckEnabled := true -}} -{{- if hasKey .Values.webapp "connectivityCheck" -}} -{{- if hasKey .Values.webapp.connectivityCheck "postgres" -}} -{{- $connectivityCheckEnabled = .Values.webapp.connectivityCheck.postgres -}} -{{- end -}} -{{- end -}} -{{- $connectivityCheckEnabled -}} -{{- end }} +{{- define "trigger-v4.webapp.connectivityCheckEnabled" -}} +{{- .Values.webapp.connectivityCheck.postgres | default true -}} +{{- end }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
hosting/k8s/helm/Chart.yaml
(1 hunks)hosting/k8s/helm/templates/_helpers.tpl
(5 hunks)hosting/k8s/helm/templates/secrets.yaml
(1 hunks)hosting/k8s/helm/templates/validate-external-config.yaml
(2 hunks)hosting/k8s/helm/templates/webapp.yaml
(3 hunks)hosting/k8s/helm/values.yaml
(13 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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#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: .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/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: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task from backend code, use `tasks.trigger`, `tasks.batchTrigger`, or `tasks.triggerAndPoll` from `@trigger.dev/sdk/v3` and set the `TRIGGER_SECRET_KEY` environment variable.
Learnt from: nicktrn
PR: triggerdotdev/trigger.dev#2155
File: hosting/docker/.env.example:4-7
Timestamp: 2025-06-06T23:55:01.933Z
Learning: In the trigger.dev project, .env.example files should contain actual example secret values rather than placeholders, as these help users understand the expected format. The files include clear warnings about not using these defaults in production and instructions for generating proper secrets.
hosting/k8s/helm/Chart.yaml (10)
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.528Z
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.528Z
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.528Z
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.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : Use the `runs` and `tasks` APIs from `@trigger.dev/sdk/v3` to subscribe to run updates and implement realtime features.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
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).
hosting/k8s/helm/templates/secrets.yaml (4)
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#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#2155
File: hosting/docker/.env.example:4-7
Timestamp: 2025-06-06T23:55:01.933Z
Learning: In the trigger.dev project, .env.example files should contain actual example secret values rather than placeholders, as these help users understand the expected format. The files include clear warnings about not using these defaults in production and instructions for generating proper secrets.
hosting/k8s/helm/templates/webapp.yaml (5)
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/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/webapp.mdc:0-0
Timestamp: 2025-06-30T13:21:59.438Z
Learning: Applies to apps/webapp/**/*.ts : In the webapp, all environment variables must be accessed through the `env` export of `env.server.ts`, instead of directly accessing `process.env`.
Learnt from: matt-aitken
PR: triggerdotdev/trigger.dev#1923
File: packages/core/src/v3/schemas/api.ts:813-827
Timestamp: 2025-04-17T10:27:25.337Z
Learning: Creating secret environment variables is restricted to the dashboard UI only, and not allowed via the API/SDK for now. The `EnvironmentVariableWithSecret` type in the API schema is for reading/displaying purposes only.
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/validate-external-config.yaml (3)
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.
hosting/k8s/helm/values.yaml (4)
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/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#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/templates/minio.yaml:49-55
Timestamp: 2025-06-25T13:29:54.392Z
Learning: In MinIO deployments, the MINIO_ROOT_USER and MINIO_ROOT_PASSWORD environment variables are only used during the first start/initialization of MinIO. These are bootstrap credentials that should be changed after the initial setup is complete.
hosting/k8s/helm/templates/_helpers.tpl (5)
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/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/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: hosting/docker/.env.example:4-7
Timestamp: 2025-06-06T23:55:01.933Z
Learning: In the trigger.dev project, .env.example files should contain actual example secret values rather than placeholders, as these help users understand the expected format. The files include clear warnings about not using these defaults in production and instructions for generating proper secrets.
🪛 Checkov (3.2.334)
hosting/k8s/helm/values.yaml
[MEDIUM] 388-389: Basic Auth Credentials
(CKV_SECRET_4)
⏰ 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). (13)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
🔇 Additional comments (10)
hosting/k8s/helm/Chart.yaml (1)
5-5
: Double-check that the tagged dependency set actually exists in the Bitnami OCI registryThe chart version bump is fine, but the new sub-chart versions (
postgresql 16.7.14
,redis 21.2.6
,clickhouse 9.3.7
,minio 17.0.9
) are fairly recent and occasionally lag behind the public OCI registry sync. Before merging, make sure ahelm dependency build
succeeds in CI or locally; otherwise the release will fail at install/upgrade time.hosting/k8s/helm/templates/validate-external-config.yaml (1)
6-8
: 👍 Simplified PostgreSQL validation looks goodRequiring either
databaseUrl
orexistingSecret
lines up with the new values schema and removes a lot of redundant checks.hosting/k8s/helm/templates/secrets.yaml (1)
22-29
: 👍 Postgres URL/direct-URL secret generation is tidyFallback to
databaseUrl
whendirectUrl
is omitted prevents an extra value from being mandatory.hosting/k8s/helm/templates/_helpers.tpl (7)
112-118
: LGTM!The helper correctly prioritizes external database URL and falls back to internal connection string construction.
130-136
: LGTM!The helper correctly prioritizes external Redis host configuration.
175-217
: Well-structured secret management helpersThe PostgreSQL secret helpers provide excellent flexibility with proper fallback mechanisms for both secret names and keys. The direct URL fallback to database URL is a thoughtful addition.
219-239
: LGTM!Redis secret helpers follow the same consistent pattern as PostgreSQL, maintaining good code consistency.
263-349
: Comprehensive S3 secret managementThe S3 helpers provide excellent coverage for both external and auth scenarios with thoughtful backward compatibility through the effective auth helpers.
476-481
: LGTM!The helper correctly supports external connectivity check configuration while maintaining internal deployment support.
384-389
: Confirm runtime substitution of CLICKHOUSE_PASSWORDWe’ve verified that:
- In _helpers.tpl (lines 384–389 and 401–406) the URL helpers emit
${CLICKHOUSE_PASSWORD}
when using an existing secret.- In webapp.yaml the
CLICKHOUSE_PASSWORD
env var is pulled from the Kubernetes Secret viavalueFrom.secretKeyRef
.Please ensure that this placeholder is resolved at runtime by one of the following:
- Wrapping your container’s entrypoint in a shell (e.g.
command: ["sh","-c","./app --clickhouse-url=$CLICKHOUSE_URL"]
) so$CLICKHOUSE_PASSWORD
is expanded before the app starts, or- Having the application code itself perform environment‐variable expansion on the DSN.
Also, update the Helm chart README (or relevant documentation) to call out this requirement when configuring external ClickHouse credentials.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/self-hosting/kubernetes.mdx (5)
201-204
: Trim double-spaces before headingsThere are two consecutive spaces before each
####
service heading (e.g.,"management. #### PostgreSQL"
). Markdown renders fine, but the extra spaces trip linters and look untidy.
208-213
: Clarify whendirectUrl
is required
directUrl
is only needed for Prisma migrations; at runtime Prisma falls back todatabaseUrl
if it’s absent. Consider noting this explicitly to avoid confusion for users who don’t run migrations in-cluster.- directUrl: "" # Optional, defaults to databaseUrl + # Optional: only required if you run Prisma migrations. + # If omitted, Trigger.dev will reuse `databaseUrl`. + directUrl: ""
226-251
: Addrediss://
example & note on ACL usernameWith TLS enabled the resulting connection string is typically
rediss://
. Showing that makes the example copy-pasteable.
Redis ACLs also allow a username; mentioning theusername
key prevents questions later.- host: "my-redis.example.com" + # rediss://<username:password>@host:port if TLS is enabled + host: "my-redis.example.com" port: 6379 + # username: "default" # Optional (Redis ACL)
268-276
: Consider allowingusername
in the secret
username
can be sensitive in some ClickHouse setups (e.g., short-lived IAM users). Documenting that it may also come fromexistingSecret
would future-proof the chart and keep credentials in one place.
310-339
: Avoid duplicate SSL configurationBoth
sslmode=require
in the URL andconnection.sslMode: "require"
achieve the same result. Stating both could confuse users about precedence. Pick one approach and document it as authoritative.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/self-hosting/kubernetes.mdx
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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#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: .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/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#2155
File: hosting/docker/.env.example:4-7
Timestamp: 2025-06-06T23:55:01.933Z
Learning: In the trigger.dev project, .env.example files should contain actual example secret values rather than placeholders, as these help users understand the expected format. The files include clear warnings about not using these defaults in production and instructions for generating proper secrets.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task from backend code, use `tasks.trigger`, `tasks.batchTrigger`, or `tasks.triggerAndPoll` from `@trigger.dev/sdk/v3` and set the `TRIGGER_SECRET_KEY` environment variable.
docs/self-hosting/kubernetes.mdx (4)
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#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#2155
File: hosting/docker/registry/auth.htpasswd:1-1
Timestamp: 2025-06-06T18:49:18.144Z
Learning: Example credentials in self-hosting documentation files are acceptable when they serve as templates or examples for users to understand the setup process, particularly in hosting/docker configuration files.
🪛 LanguageTool
docs/self-hosting/kubernetes.mdx
[grammar] ~201-~201: Use proper spacing conventions.
Context: ...ecrets for secure credential management. #### PostgreSQL Direct configuration: ``...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~203-~203: Use proper spacing conventions.
Context: ... credential management. #### PostgreSQL Direct configuration: ```yaml postgres...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~205-~205: Use proper spacing conventions.
Context: ...#### PostgreSQL Direct configuration: yaml postgres: deploy: false external: databaseUrl: "postgresql://user:password@host:5432/database?schema=public" directUrl: "" # Optional, defaults to databaseUrl
Using existing secrets (recommended): ...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~214-~214: Use proper spacing conventions.
Context: ... Using existing secrets (recommended): yaml postgres: deploy: false external: existingSecret: "postgres-credentials" # Optional: Use secretKeys to specify the key names in the secret # secretKeys: # databaseUrlKey: "postgres-database-url" # default # directUrlKey: "postgres-direct-url" # default
#### Redis Direct configuration: ```yaml...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~226-~226: Use proper spacing conventions.
Context: ...irect-url" # default #### Redis **Direct configuration:**
yaml redis: ...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~228-~228: Use proper spacing conventions.
Context: ... #### Redis **Direct configuration:**
yaml redis: deploy: false external: host: "my-redis.example.com" port: 6379 password: "my-password" tls: enabled: true ``` Using existing secrets (recommended): ...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~240-~240: Use proper spacing conventions.
Context: ... Using existing secrets (recommended): yaml redis: deploy: false external: host: "my-redis.example.com" port: 6379 existingSecret: "redis-credentials" # existingSecretPasswordKey: "redis-password" # default (optional) tls: enabled: true
#### ClickHouse Direct configuration: ``...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~253-~253: Use proper spacing conventions.
Context: ... enabled: true #### ClickHouse **Direct configuration:**
yaml clickhou...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~255-~255: Use proper spacing conventions.
Context: ...#### ClickHouse Direct configuration: yaml clickhouse: deploy: false external: host: "my-clickhouse.example.com" port: 8123 username: "my-username" password: "my-password"
Using existing secrets (recommended): ...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~266-~266: Use proper spacing conventions.
Context: ... Using existing secrets (recommended): yaml clickhouse: deploy: false external: host: "my-clickhouse.example.com" port: 8123 username: "my-username" existingSecret: "clickhouse-credentials" # existingSecretKey: "clickhouse-password" # default (optional)
#### S3 Object Storage **Direct configuratio...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~278-~278: Use proper spacing conventions.
Context: ...t (optional) #### S3 Object Storage **Direct configuration:**
yaml minio: ...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~280-~280: Use proper spacing conventions.
Context: ... Object Storage Direct configuration: yaml minio: deploy: false s3: external: endpoint: "https://s3.amazonaws.com" accessKeyId: "my-access-key" secretAccessKey: "my-secret-key"
Using existing secrets (recommended): ...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~291-~291: Use proper spacing conventions.
Context: ... Using existing secrets (recommended): yaml minio: deploy: false s3: external: endpoint: "https://s3.amazonaws.com" existingSecret: "s3-credentials" # Optional: Use secretKeys to specify the key names in the secret # secretKeys: # accessKeyIdKey: "access-key-id" # default # secretAccessKeyKey: "secret-access-key" # default
### PostgreSQL SSL with custom CA certificat...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~305-~305: Use proper spacing conventions.
Context: ...stgreSQL SSL with custom CA certificates When connecting to PostgreSQL instances ...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~307-~307: Combining words like “every day” changes the meaning.
Context: ...rtificate as a volume and configure the webapp to use it: ```yaml postgres: deploy:...
(QB_NEW_EN_OTHER_ERROR_IDS_000001)
[grammar] ~307-~307: Use proper spacing conventions.
Context: ...lume and configure the webapp to use it: yaml postgres: deploy: false external: databaseUrl: "postgresql://user:[email protected]:5432/triggerdb?schema=public&sslmode=require" # Alternatively, use an existing secret existingSecret: "postgres-credentials" # secretKeys: # databaseUrlKey: "postgres-database-url" # default connection: sslMode: "require" # Webapp configuration with SSL CA certificate webapp: extraEnvVars: - name: NODE_EXTRA_CA_CERTS value: "/etc/ssl/certs/postgres-ca.crt" extraVolumes: - name: postgres-ca-cert secret: secretName: postgres-ca-secret items: - key: ca.crt path: postgres-ca.crt extraVolumeMounts: - name: postgres-ca-cert mountPath: /etc/ssl/certs readOnly: true
Benefits: - No plaintext credentials i...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~344-~344: Use proper spacing conventions.
Context: ... tools (External Secrets Operator, etc.) - Follows Kubernetes security best practic...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
[grammar] ~345-~345: Use proper spacing conventions.
Context: ...llows Kubernetes security best practices ## Worker token When using the default boo...
(QB_NEW_EN_OTHER_ERROR_IDS_000007)
⏰ 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). (27)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (python)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
hosting/k8s/helm/templates/_helpers.tpl (1)
99-107
: Consider using a more meaningful placeholder for external PostgreSQL hostname.The fixed hostname "external-postgres" returned when PostgreSQL is not deployed internally may not be useful since it doesn't correspond to a real endpoint. However, since this helper is marked as deprecated and only used for legacy DATABASE_HOST, this may be acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
hosting/k8s/helm/templates/_helpers.tpl
(5 hunks)hosting/k8s/helm/templates/validate-external-config.yaml
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- hosting/k8s/helm/templates/validate-external-config.yaml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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#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: .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/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#2155
File: hosting/docker/.env.example:4-7
Timestamp: 2025-06-06T23:55:01.933Z
Learning: In the trigger.dev project, .env.example files should contain actual example secret values rather than placeholders, as these help users understand the expected format. The files include clear warnings about not using these defaults in production and instructions for generating proper secrets.
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/writing-tasks.mdc:0-0
Timestamp: 2025-06-30T13:22:21.528Z
Learning: Applies to **/trigger/**/*.{ts,tsx,js,jsx} : When triggering a task from backend code, use `tasks.trigger`, `tasks.batchTrigger`, or `tasks.triggerAndPoll` from `@trigger.dev/sdk/v3` and set the `TRIGGER_SECRET_KEY` environment variable.
hosting/k8s/helm/templates/_helpers.tpl (5)
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/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/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: hosting/docker/.env.example:4-7
Timestamp: 2025-06-06T23:55:01.933Z
Learning: In the trigger.dev project, .env.example files should contain actual example secret values rather than placeholders, as these help users understand the expected format. The files include clear warnings about not using these defaults in production and instructions for generating proper secrets.
⏰ 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). (26)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (python)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (10, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 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 (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (9, 10)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
🔇 Additional comments (7)
hosting/k8s/helm/templates/_helpers.tpl (7)
110-118
: Well-implemented connection string fallback logic.The connection string helper properly prioritizes external database URL over internal deployment configuration, providing appropriate fallback behavior.
120-125
: Simplified logic addresses previous review feedback.The helper now correctly checks only for the existence of
postgres.external.existingSecret
, which is more concise than the previous redundant conditional logic.
131-173
: Redis external configuration helpers are well-structured.The Redis helpers properly handle external host, port, password, and TLS settings with appropriate fallbacks. The TLS disabled logic correctly negates the external TLS enabled flag (defaulting to false for external, true for internal).
175-349
: Excellent consistent pattern for secret management helpers.All secret name and key helpers follow the same pattern:
- Check if an existing secret is specified
- Use that secret name and corresponding key if available
- Fall back to generated secret name and default key names
This consistency will make the templates maintainable and predictable. The S3 auth effective helpers also provide good fallback logic from specific credentials to root credentials.
384-388
: ClickHouse URL helpers properly handle password placeholders.The helpers correctly use
${CLICKHOUSE_PASSWORD}
placeholders when existing secrets are configured, allowing the password to be injected as an environment variable at runtime.Also applies to: 401-406
460-471
: Connectivity check helper provides good configurability.The helper allows disabling PostgreSQL connectivity checks via
webapp.connectivityCheck.postgres
while defaulting to enabled, providing flexibility for different deployment scenarios.
477-482
: PostgreSQL host helper supports custom connectivity check endpoints.The helper properly prioritizes external connectivity check host over the internal PostgreSQL hostname, allowing for custom health check endpoints when needed.
Key Features Implemented
1. External Service Configuration Fixed
All helper templates now correctly reference external service configurations:
.Values.postgres.external.host
(instead of.Values.postgres.host
).Values.redis.external.host
with proper password and TLS support.Values.clickhouse.external.host
.Values.s3.external.accessKeyId
and.Values.s3.external.secretAccessKey
2. Kubernetes Secret Support
The chart now supports referencing existing Kubernetes secrets for all sensitive values:
postgres.external.existingSecret
withsecretKeys.databaseUrlKey
redis.external.existingSecret
withexistingSecretPasswordKey
clickhouse.external.existingSecret
withsecretKeys.passwordKey
s3.external.existingSecret
with configurable secret keys3. Redis TLS Support
REDIS_TLS_DISABLED="true"
redis.external.tls.enabled
configuration option4. S3 External Credentials
OBJECT_STORE_ACCESS_KEY_ID
andOBJECT_STORE_SECRET_ACCESS_KEY
environment variables5. SSL/TLS Certificate Support
While we didn't implement the specific
postgres.external.caCertSecretRef
pattern suggested in the issue, the existing Helm chart already supports custom CA certificates through the standardextraVolumes
,extraVolumeMounts
, andextraEnvVars
patterns.PostgreSQL SSL with Custom CA Certificates
Benefits:
values.yaml
or Helm releasesDATABASE_URL
stored securely in Kubernetes secretsTesting Completed
We verified all fixes by deploying to a local kind cluster with different configurations:
✅ Test Results
External service configuration paths - ✅ PASSED
.Values.*.external.*
pathsS3 credentials - ✅ PASSED
OBJECT_STORE_ACCESS_KEY_ID
andOBJECT_STORE_SECRET_ACCESS_KEY
properly setRedis TLS and passwords - ✅ PASSED
REDIS_TLS_DISABLED="false"
whenredis.external.tls.enabled=true
redis-credentials
secret viasecretKeyRef
Secret references - ✅ PASSED
secretKeyRef
for sensitive valuesdatabase-url
frompostgres-credentials
secretpassword
fromredis-credentials
secretpassword
fromclickhouse-credentials
secretPostgreSQL SSL with custom CA - ✅ PASSED
NODE_EXTRA_CA_CERTS=/etc/ssl/certs/postgres-ca.crt
properly set/etc/ssl/certs
postgres-ca.crt
in containerDeployment Behavior Verified
Supervisor waiting for worker token: ✅ EXPECTED BEHAVIOR
CreateContainerConfigError
waiting fortrigger-*-worker-token
secrettoken-syncer
sidecar after successful startupConnection failures to external services: ✅ EXPECTED BEHAVIOR
Resolution Status
✅ Fixed - Correct value paths for external services
✅ Fixed - S3 external credentials properly used
✅ Fixed - Kubernetes secret support for all sensitive values
✅ Fixed - Redis TLS support added
✅ Fixed - Redis password authentication implemented
✅ Supported - Custom CA certificates work with existing patterns
The chart now properly supports external managed services with secure credential management and TLS/SSL connections.
Fixes #2237