Conversation
Add -e and -s flags to `miren deploy` for setting environment variables atomically with the deploy. This ensures env vars are available to the very first sandbox, solving ordering issues where apps need env vars present at initial startup (e.g., Grafana's GF_SECURITY_ADMIN_PASSWORD). Syntax: miren deploy -e KEY=VALUE # Regular env var miren deploy -s SECRET=value # Sensitive (masked in output) miren deploy -e KEY=@file # Read value from file miren deploy -e KEY # Prompt interactively Implementation: - Add EnvironmentVariable type and envVars param to buildFromTar RPC - Add mergeCliEnvVars() that applies CLI vars with source="manual" - CLI vars are merged last in buildVersionConfig() (highest precedence) - Single atomic AppVersion created with merged config The merge order ensures CLI vars always win: 1. Start with existing config from previous version 2. Layer in app.toml vars (source="config") 3. Layer in CLI vars (source="manual") Closes MIR-606
📝 WalkthroughWalkthroughThis change introduces environment variable support across the deployment pipeline. New types define environment variables in the RPC layer, CLI parsing logic converts flag inputs to structured types, and server-side code merges CLI-provided variables with highest precedence over existing configuration variables. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as CLI<br/>(deploy.go)
participant Parser as CLI<br/>(env.go)
participant RPC as RPC Client
participant Server as Build Server<br/>(build.go)
participant Config as Config Merge
User->>CLI: deploy with -e KEY=VALUE -s SENSITIVE=val
CLI->>Parser: ParseEnvVarSpecs(envSpecs, sensitiveSpecs)
Parser-->>CLI: []EnvVarSpec (parsed & validated)
note over Parser: Handles KEY=VALUE, KEY=@file<br/>file reads, validation
CLI->>RPC: BuildFromTar(..., envVars []*EnvironmentVariable)
note over CLI: Convert EnvVarSpec → EnvironmentVariable
RPC->>Server: Receive envVars parameter
Server->>Server: args.EnvVars() retrieves CLI vars
Server->>Config: mergeCliEnvVars(existingVars, cliVars)
note over Config: CLI vars get highest precedence<br/>marked source="manual"
Config-->>Server: merged variables
Server->>Server: buildVersionConfig applies merged vars
note over Server: Final config includes CLI vars
Server-->>RPC: BuildFromTarResults
RPC-->>CLI: Success with applied env vars
CLI-->>User: Deploy complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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
🤖 Fix all issues with AI agents
In @api/build/build_v1alpha/rpc.gen.go:
- Around line 758-764: BuilderBuildFromTarArgs.EnvVars() currently dereferences
v.data.EnvVars and can panic when nil; change the accessor to match other slice
accessors by returning the pointer (i.e., return v.data.EnvVars) or otherwise
guard the dereference (check v.data.EnvVars != nil and return nil or empty
slice). Update the code generator or the RPC schema (api/build/rpc.yml) so
generated code for EnvVars uses the same pointer-return pattern as
Services()/Hostnames(), and regenerate the file so HasEnvVars() remains the
nil-check while EnvVars() does not dereference the pointer.
🧹 Nitpick comments (1)
cli/commands/env.go (1)
18-20: Consider renamingFromFile_to a more descriptive name.The trailing underscore in
FromFile_is unconventional for Go. Consider renaming toSourceFileorSourceFilenamefor clarity and to better distinguish it from the booleanFromFilefield.♻️ Suggested rename
type EnvVarSpec struct { Key string Value string Sensitive bool - FromFile bool // true if value was read from a file - FromFile_ string // original filename if FromFile is true + FromFile bool // true if value was read from a file + SourceFilename string // original filename if FromFile is true }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
api/build/build_v1alpha/rpc.gen.goapi/build/rpc.ymlcli/commands/deploy.gocli/commands/env.gocli/commands/env_test.goservers/build/build.goservers/build/build_test.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: Follow standard Go formatting conventions
Only add comments when they provide valuable context or explain 'why' something is done - avoid redundant comments that restate what the code does
Good comments should explain complex logic, document assumptions, or clarify non-obvious behavior rather than restating the code
Function/method comments should explain the purpose and any important side effects, not just restate the function name
Files:
cli/commands/env_test.goservers/build/build_test.goservers/build/build.gocli/commands/env.goapi/build/build_v1alpha/rpc.gen.gocli/commands/deploy.go
**/*.gen.go
📄 CodeRabbit inference engine (CLAUDE.md)
Code generation outputs for entity schemas and RPC interfaces should have
.gen.gosuffix
Files:
api/build/build_v1alpha/rpc.gen.go
🧠 Learnings (1)
📚 Learning: 2025-12-15T17:54:35.854Z
Learnt from: phinze
Repo: mirendev/runtime PR: 478
File: cli/commands/debug_netdb.go:155-156
Timestamp: 2025-12-15T17:54:35.854Z
Learning: In reviews of Go CLI commands under cli/commands (mirendev/runtime), prefer validating inputs on the server side and skip duplicating client-side validation when the CLI is the only client. This avoids code duplication and keeps validation centralized. If local validation is absolutely necessary for UX, keep it slim and clearly justified, and ensure it does not duplicate server logic. Document the rationale and ensure server API validation remains the source of truth.
Applied to files:
cli/commands/env_test.gocli/commands/env.gocli/commands/deploy.go
🧬 Code graph analysis (4)
cli/commands/env_test.go (1)
cli/commands/env.go (1)
ParseEnvVarSpecs(25-47)
servers/build/build_test.go (3)
api/build/build_v1alpha/rpc.gen.go (1)
EnvironmentVariable(483-485)api/core/core_v1alpha/schema.gen.go (1)
Variable(705-710)api/compute/compute_v1alpha/schema.gen.go (1)
Key(1936-1939)
servers/build/build.go (2)
api/build/build_v1alpha/rpc.gen.go (1)
EnvironmentVariable(483-485)api/core/core_v1alpha/schema.gen.go (1)
Variable(705-710)
cli/commands/deploy.go (3)
api/core/core_v1alpha/schema.gen.go (1)
Env(571-576)api/build/build_v1alpha/rpc.gen.go (1)
EnvironmentVariable(483-485)cli/commands/env.go (1)
ParseEnvVarSpecs(25-47)
⏰ 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). (5)
- GitHub Check: build-binaries (darwin, amd64, macos-latest)
- GitHub Check: lint
- GitHub Check: build-binaries (darwin, arm64, macos-latest)
- GitHub Check: test-e2e
- GitHub Check: test
🔇 Additional comments (6)
cli/commands/env_test.go (1)
9-119: Test coverage looks good.The test function comprehensively covers various parsing scenarios including edge cases and error conditions. The use of
t.TempDir()for temporary file handling is clean and appropriate.cli/commands/env.go (3)
23-47: LGTM!The parsing logic correctly processes both regular and sensitive env var specs, with proper error propagation.
49-101: LGTM!The parsing handles all three formats (KEY, KEY=VALUE, KEY=@file) correctly. Error handling for file operations and empty keys is appropriate.
103-148: LGTM!The refactored
EnvSetcleanly delegates parsing toParseEnvVarSpecsand handles all cases appropriately, including sensitive variable masking in logs.api/build/build_v1alpha/rpc.gen.go (2)
477-546: LGTM!The
EnvironmentVariabletype follows the established patterns in this generated file with proper accessors and serialization methods.
1013-1037: LGTM!The
BuildFromTarclient method correctly includes the newenvVarsparameter and wires it to the RPC arguments.
phinze
deleted the
phinze/mir-606-support-setting-env-vars-at-deploy-time-miren-deploy-e-s
branch
2 participants
Summary
Add
-eand-sflags tomiren deployfor setting environment variables atomically with the deploy. This ensures env vars are available to the very first sandbox, solving ordering issues where apps need env vars present at initial startup.Example use case: Grafana's
GF_SECURITY_ADMIN_PASSWORDmust be set before the first DB creation - setting it after deploy is too late.Syntax
Implementation
buildFromTarRPC to acceptenvVarsparametermergeCliEnvVars()helper that applies CLI vars withsource="manual"buildVersionConfig()(highest precedence)AppVersionwith merged config (no intermediate versions)Merge order:
source="config")source="manual") - always winsTest plan
ParseEnvVarSpecs()(KEY=VALUE, KEY=@file, errors)mergeCliEnvVars()(override, merge, sensitive flag)miren deploy -e FOO=barsets env var on first sandboxmiren deploy -s SECRET=xmasks value in outputSummary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.