fix: inject environment variables at dequeue time for self-hosted deployments #2793
Conversation
…loyments This commit fixes the issue where environment variables configured in the Trigger.dev portal were not being injected into runner pods/containers in self-hosted deployments (GitHub issue triggerdotdev#2792). Root Cause: Environment variables were only being fetched during the 'startRunAttempt' phase, which occurs AFTER the runner pod/container has already been created. This meant that pods were created without access to portal-configured environment variables, causing tasks to fail immediately. Solution: Move environment variable fetching from the 'startRunAttempt' phase to the 'dequeue' phase, so that env vars are available when the supervisor creates the runner workload. Changes: 1. Added optional 'envVars' field to DequeuedMessage schema 2. Modified AuthenticatedWorkerInstance.dequeue() to fetch and include environment variables for each dequeued message 3. Added 'envVars' field to WorkloadManagerCreateOptions interface 4. Updated supervisor to pass envVars when creating workloads 5. Modified KubernetesWorkloadManager to inject envVars into pod spec 6. Modified DockerWorkloadManager to inject envVars into container Testing: - Tested in self-hosted Kubernetes deployment (EKS) - Verified environment variables appear in runner pods - Confirmed tasks execute successfully with portal-configured env vars Fixes triggerdotdev#2792
|
WalkthroughThis change implements environment variable injection throughout the workload creation pipeline. A new optional Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ 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 |
![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
🧹 Nitpick comments (1)
apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts (1)
381-408: Consider performance implications of N+1 database queries.For each dequeued message, the code fetches the runtime environment from the database (lines 385-392). When multiple messages are dequeued in a batch, this results in N separate database queries. While this may be acceptable for self-hosted deployments with lower dequeue rates, consider batching environment fetches if performance becomes a concern.
If needed, environments could be fetched in a single query:
// Fetch all unique environments in one query const envIds = [...new Set(messages.map(m => m.environment.id))]; const environments = await this._prisma.runtimeEnvironment.findMany({ where: { id: { in: envIds } }, include: { parentEnvironment: true }, }); const envMap = new Map(environments.map(e => [e.id, e])); // Then look up from map instead of querying per message const environment = envMap.get(message.environment.id);
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/supervisor/src/index.ts(1 hunks)apps/supervisor/src/workloadManager/docker.ts(1 hunks)apps/supervisor/src/workloadManager/kubernetes.ts(1 hunks)apps/supervisor/src/workloadManager/types.ts(1 hunks)apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts(1 hunks)packages/core/src/v3/schemas/runEngine.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
Files:
packages/core/src/v3/schemas/runEngine.tsapps/supervisor/src/workloadManager/docker.tsapps/supervisor/src/index.tsapps/supervisor/src/workloadManager/types.tsapps/supervisor/src/workloadManager/kubernetes.tsapps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
packages/core/src/v3/schemas/runEngine.tsapps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
packages/core/src/v3/schemas/runEngine.tsapps/supervisor/src/workloadManager/docker.tsapps/supervisor/src/index.tsapps/supervisor/src/workloadManager/types.tsapps/supervisor/src/workloadManager/kubernetes.tsapps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
**/*.{js,ts,jsx,tsx,json,md,css,scss}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier
Files:
packages/core/src/v3/schemas/runEngine.tsapps/supervisor/src/workloadManager/docker.tsapps/supervisor/src/index.tsapps/supervisor/src/workloadManager/types.tsapps/supervisor/src/workloadManager/kubernetes.tsapps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Access all environment variables through the
envexport ofenv.server.tsinstead of directly accessingprocess.envin the Trigger.dev webapp
Files:
apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: When importing from@trigger.dev/corein the webapp, use subpath exports from the package.json instead of importing from the root path
Follow the Remix 2.1.0 and Express server conventions when updating the main trigger.dev webapp
Files:
apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
apps/webapp/app/v3/services/**/*.server.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Organize services in the webapp following the pattern
app/v3/services/*/*.server.ts
Files:
apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts
🧠 Learnings (2)
📓 Common learnings
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.
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webapp
📚 Learning: 2025-11-27T16:26:58.661Z
Learnt from: CR
Repo: triggerdotdev/trigger.dev PR: 0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-11-27T16:26:58.661Z
Learning: Applies to apps/webapp/app/**/*.{ts,tsx} : Access all environment variables through the `env` export of `env.server.ts` instead of directly accessing `process.env` in the Trigger.dev webapp
Applied to files:
apps/supervisor/src/workloadManager/docker.tsapps/supervisor/src/index.tsapps/supervisor/src/workloadManager/kubernetes.ts
🧬 Code graph analysis (1)
apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts (2)
internal-packages/run-engine/src/run-queue/index.ts (5)
message(1458-1506)message(1763-1818)message(1859-1907)message(1909-1937)message(1958-1960)apps/webapp/app/v3/machinePresets.server.ts (1)
machinePresetFromName(27-32)
🔇 Additional comments (5)
apps/supervisor/src/workloadManager/types.ts (1)
27-27: LGTM! Clean type addition.The optional
envVarsfield is well-placed and appropriately typed for carrying per-message environment variables through the workload creation pipeline.apps/supervisor/src/index.ts (1)
270-270: LGTM! Proper forwarding of environment variables.The envVars are correctly passed from the dequeued message to the workload manager, completing the pipeline from webapp to container/pod creation.
apps/supervisor/src/workloadManager/docker.ts (1)
113-117: LGTM! Consistent environment variable injection.The implementation mirrors the existing
additionalEnvVarspattern. Note thatopts.envVarswill overrideadditionalEnvVarsif there are duplicate keys, which appears intentional (portal-configured variables should take precedence over global configuration).packages/core/src/v3/schemas/runEngine.ts (1)
273-273: LGTM! Backward-compatible schema extension.The optional
envVarsfield properly extends theDequeuedMessageschema with the correct Zod type, enabling environment variable propagation without breaking existing consumers.apps/supervisor/src/workloadManager/kubernetes.ts (1)
228-233: LGTM! Consistent Kubernetes environment variable injection.The implementation mirrors the
additionalEnvVarspattern and maintains consistency with the Docker workload manager. Portal-configured variables will override global configuration if keys conflict, which aligns with expected behavior.
| const messagesWithEnvVars = await Promise.all( | ||
| messages.map(async (message) => { | ||
| const defaultMachinePreset = machinePresetFromName(defaultMachine); | ||
|
|
||
| const environment = await this._prisma.runtimeEnvironment.findFirst({ | ||
| where: { | ||
| id: message.environment.id, | ||
| }, | ||
| include: { | ||
| parentEnvironment: true, | ||
| }, | ||
| }); | ||
|
|
||
| const envVars = environment | ||
| ? await this.getEnvVars( | ||
| environment, | ||
| message.run.id, | ||
| message.run.machine ?? defaultMachinePreset, | ||
| environment.parentEnvironment ?? undefined | ||
| ) | ||
| : {}; | ||
|
|
||
| return { | ||
| ...message, | ||
| envVars, | ||
| }; | ||
| }) | ||
| ); |
There was a problem hiding this comment.
Missing taskEventStore parameter in getEnvVars call.
The getEnvVars method expects an optional taskEventStore parameter (line 582), which is used to set OTEL_RESOURCE_ATTRIBUTES for observability (lines 599-610). The startRunAttempt method correctly passes this parameter (line 481), but the dequeue enrichment does not.
Apply this diff to include the taskEventStore parameter:
const envVars = environment
? await this.getEnvVars(
environment,
message.run.id,
message.run.machine ?? defaultMachinePreset,
- environment.parentEnvironment ?? undefined
+ environment.parentEnvironment ?? undefined,
+ message.run.taskEventStore ?? undefined
)
: {};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const messagesWithEnvVars = await Promise.all( | |
| messages.map(async (message) => { | |
| const defaultMachinePreset = machinePresetFromName(defaultMachine); | |
| const environment = await this._prisma.runtimeEnvironment.findFirst({ | |
| where: { | |
| id: message.environment.id, | |
| }, | |
| include: { | |
| parentEnvironment: true, | |
| }, | |
| }); | |
| const envVars = environment | |
| ? await this.getEnvVars( | |
| environment, | |
| message.run.id, | |
| message.run.machine ?? defaultMachinePreset, | |
| environment.parentEnvironment ?? undefined | |
| ) | |
| : {}; | |
| return { | |
| ...message, | |
| envVars, | |
| }; | |
| }) | |
| ); | |
| const messagesWithEnvVars = await Promise.all( | |
| messages.map(async (message) => { | |
| const defaultMachinePreset = machinePresetFromName(defaultMachine); | |
| const environment = await this._prisma.runtimeEnvironment.findFirst({ | |
| where: { | |
| id: message.environment.id, | |
| }, | |
| include: { | |
| parentEnvironment: true, | |
| }, | |
| }); | |
| const envVars = environment | |
| ? await this.getEnvVars( | |
| environment, | |
| message.run.id, | |
| message.run.machine ?? defaultMachinePreset, | |
| environment.parentEnvironment ?? undefined, | |
| message.run.taskEventStore ?? undefined | |
| ) | |
| : {}; | |
| return { | |
| ...message, | |
| envVars, | |
| }; | |
| }) | |
| ); |
🤖 Prompt for AI Agents
In apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts around
lines 381-408, the call to getEnvVars is missing the optional taskEventStore
parameter; update the call to pass the same taskEventStore used elsewhere in
this class (the same variable passed in startRunAttempt at ~line 481) — e.g. add
, taskEventStore (or this._taskEventStore if it's a class member) as the last
argument so getEnvVars(environment, message.run.id, message.run.machine ??
defaultMachinePreset, environment.parentEnvironment ?? undefined,
taskEventStore).
Testing & VerificationProcessWe've successfully tested this fix in a production-like environment:
Root Cause ConfirmedThe issue was that environment variables were being fetched in ImpactThis fix resolves the critical issue where environment variables configured in the Trigger.dev portal were not being injected into self-hosted runner containers, causing tasks to fail when accessing required secrets and configuration. 🤖 Tested and verified in production environment |
1 participant
Summary
This PR fixes the issue where environment variables configured in the Trigger.dev portal were not being injected into runner pods/containers in self-hosted deployments.
Fixes #2792
Root Cause
Environment variables were only being fetched during the
startRunAttemptphase, which occurs AFTER the runner pod/container has already been created. This meant that pods were created without access to portal-configured environment variables, causing tasks to fail immediately with "Missing required environment variables" errors.Solution
Move environment variable fetching from the
startRunAttemptphase to thedequeuephase, ensuring env vars are available when the supervisor creates the runner workload.Changes
Schema (
packages/core/src/v3/schemas/runEngine.ts)envVarsfield toDequeuedMessageschemaWebapp Service (
apps/webapp/app/v3/services/worker/workerGroupTokenService.server.ts)AuthenticatedWorkerInstance.dequeue()to fetch and include environment variables for each dequeued messagegetEnvVars()helper methodSupervisor Types (
apps/supervisor/src/workloadManager/types.ts)envVarsfield toWorkloadManagerCreateOptionsinterfaceSupervisor (
apps/supervisor/src/index.ts)envVarsfrom dequeued messageWorkload Managers
apps/supervisor/src/workloadManager/kubernetes.ts): InjectenvVarsinto pod environmentapps/supervisor/src/workloadManager/docker.ts): InjectenvVarsinto container environmentTesting
Backward Compatibility
This change is backward compatible:
envVarsfield is optional in the schemaenvVarsis undefined/empty, workload creation proceeds as beforeRelated Documentation