feat: add keyway docker command for Docker secrets injection#4
feat: add keyway docker command for Docker secrets injection#4Alan-221b wants to merge 2 commits intokeywaysh:mainfrom
Conversation
Closes keywaysh#2 Add a new `keyway docker` command that injects vault secrets into Docker and Docker Compose commands. Features: - `keyway docker run` - injects secrets as -e KEY=VALUE flags - `keyway docker compose run` - injects secrets as -e flags - `keyway docker compose up` - injects secrets via --env-file - User-provided -e flags take precedence over vault secrets - Interactive environment selection when --env flag not provided Usage: keyway docker run --rm alpine env keyway docker --env production run -p 8080:8080 myapp:latest keyway docker compose up -d
📝 WalkthroughWalkthroughThe changes introduce a new Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as keyway docker
participant Repo as Git/Repo
participant Auth as Auth/Login
participant UI as UI
participant API as API Client
participant Docker as Docker
User->>CLI: keyway docker [--env NAME] DOCKER_ARGS
CLI->>Repo: Detect git repo context
Repo-->>CLI: Repo info
CLI->>Auth: Ensure login
Auth-->>CLI: Authenticated
alt Env not provided and interactive
CLI->>UI: Select environment
UI-->>CLI: Selected env name
else Env provided
CLI->>CLI: Use provided env
end
CLI->>API: Fetch secrets for environment
API-->>CLI: Vault secrets (key-value map)
alt docker run detected
CLI->>CLI: Parse user-provided -e flags
CLI->>CLI: Find image position in args
CLI->>Docker: Inject secrets before image, pass through docker args
else docker compose detected
alt compose run
CLI->>Docker: Inject secrets as -e flags
else compose up/down/etc
CLI->>CLI: Create temp env file
CLI->>Docker: Pass --env-file with secrets
end
end
Docker-->>User: Command output
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@internal/cmd/docker_test.go`:
- Around line 61-101: The test TestRunDockerWithDeps_DockerCompose_Success
should be updated to match runDockerWithDeps behavior: instead of asserting
LastArgs equals ["compose","up","-d"] and checking cmdRunner.LastSecrets, assert
that cmdRunner.LastArgs contains the "--env-file" flag followed by a non-empty
path, and verify that the referenced temp file exists and contains the expected
secret "API_KEY=secret123"; remove or replace the LastSecrets assertion. Locate
assertions in TestRunDockerWithDeps_DockerCompose_Success and change them to
search for "--env-file" within cmdRunner.LastArgs, validate the following arg is
a valid file path, read that file and assert it includes the secret, and no
longer expect secrets in cmdRunner.LastSecrets.
In `@internal/cmd/docker.go`:
- Around line 195-206: The compose-run branch injects all vault secrets into
newArgs without checking for user-supplied -e/--env flags, violating the
precedence rule; update the code in the block that handles opts.DockerArgs
starting with "run" to first scan opts.DockerArgs for user-provided environment
keys (handle "-e KEY=val", "-eKEY=val", "--env KEY=val", and "--env=KEY=val"
forms) and build a set of provided keys, then when appending vault secrets to
newArgs only add -e entries for keys not present in that set (mirror the
precedence logic used by runDockerRun), preserving the rest of opts.DockerArgs
and then call deps.CmdRunner.RunCommand("docker", newArgs, nil).
🧹 Nitpick comments (3)
internal/cmd/docker_test.go (1)
56-58: Simplify the nil check.Per static analysis (gosimple S1009),
len()for nil maps returns zero, so the nil check is redundant.Suggested fix
- if cmdRunner.LastSecrets != nil && len(cmdRunner.LastSecrets) > 0 { + if len(cmdRunner.LastSecrets) > 0 { t.Errorf("expected no secrets in environment for docker run, got %v", cmdRunner.LastSecrets) }internal/cmd/docker.go (2)
75-77: Consider adding a timeout to the context.The context created at line 77 has no timeout. If the API is slow or unresponsive,
GetVaultEnvironmentsandPullSecretscalls could block indefinitely.Suggested fix
// 3. Setup Client client := deps.APIFactory.NewClient(token) - ctx := context.Background() + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel()Don't forget to add
"time"to the imports.
216-219: Consider handling write errors to the temp file.
fmt.Fprintfreturns an error that is currently ignored. If the write fails (e.g., disk full), the command would run with an incomplete or empty env file.Suggested fix
for k, v := range secrets { - fmt.Fprintf(envFile, "%s=%s\n", k, v) + if _, err := fmt.Fprintf(envFile, "%s=%s\n", k, v); err != nil { + return fmt.Errorf("failed to write to temp env file: %w", err) + } }
| func TestRunDockerWithDeps_DockerCompose_Success(t *testing.T) { | ||
| deps, _, _, _, cmdRunner, apiClient := NewTestDepsWithRunner() | ||
|
|
||
| apiClient.PullResponse = &api.PullSecretsResponse{ | ||
| Content: "API_KEY=secret123", | ||
| } | ||
|
|
||
| opts := DockerOptions{ | ||
| EnvName: "production", | ||
| EnvFlagSet: true, | ||
| DockerCommand: "compose", | ||
| DockerArgs: []string{"up", "-d"}, | ||
| } | ||
|
|
||
| err := runDockerWithDeps(opts, deps) | ||
|
|
||
| if err != nil { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
|
|
||
| // Verify docker was called | ||
| if cmdRunner.LastCommand != "docker" { | ||
| t.Errorf("expected command 'docker', got %q", cmdRunner.LastCommand) | ||
| } | ||
|
|
||
| // Verify args are "compose up -d" | ||
| expectedArgs := []string{"compose", "up", "-d"} | ||
| if len(cmdRunner.LastArgs) != len(expectedArgs) { | ||
| t.Errorf("expected args %v, got %v", expectedArgs, cmdRunner.LastArgs) | ||
| } | ||
| for i, expected := range expectedArgs { | ||
| if i < len(cmdRunner.LastArgs) && cmdRunner.LastArgs[i] != expected { | ||
| t.Errorf("expected arg[%d] = %q, got %q", i, expected, cmdRunner.LastArgs[i]) | ||
| } | ||
| } | ||
|
|
||
| // Verify secrets were passed via environment (compose uses env injection) | ||
| if cmdRunner.LastSecrets["API_KEY"] != "secret123" { | ||
| t.Errorf("expected API_KEY in secrets, got %v", cmdRunner.LastSecrets) | ||
| } | ||
| } |
There was a problem hiding this comment.
Test expectations don't match implementation — causing CI failure.
The implementation in docker.go (lines 208-224) uses --env-file for docker compose up, writing secrets to a temp file rather than passing them via LastSecrets. This test expects:
- Args to be exactly
[compose up -d]— but actual args include--env-file <path> - Secrets in
LastSecrets— but secrets are written to a file, not passed via environment
Update the test to verify --env-file is present in args and that the file path is valid:
Proposed fix
// Verify args are "compose up -d"
- expectedArgs := []string{"compose", "up", "-d"}
- if len(cmdRunner.LastArgs) != len(expectedArgs) {
- t.Errorf("expected args %v, got %v", expectedArgs, cmdRunner.LastArgs)
- }
- for i, expected := range expectedArgs {
- if i < len(cmdRunner.LastArgs) && cmdRunner.LastArgs[i] != expected {
- t.Errorf("expected arg[%d] = %q, got %q", i, expected, cmdRunner.LastArgs[i])
- }
+ // Verify compose and --env-file are present
+ if len(cmdRunner.LastArgs) < 4 {
+ t.Fatalf("expected at least 4 args, got %v", cmdRunner.LastArgs)
+ }
+ if cmdRunner.LastArgs[0] != "compose" {
+ t.Errorf("expected first arg 'compose', got %q", cmdRunner.LastArgs[0])
+ }
+ if cmdRunner.LastArgs[1] != "--env-file" {
+ t.Errorf("expected '--env-file' flag, got %q", cmdRunner.LastArgs[1])
}
- // Verify secrets were passed via environment (compose uses env injection)
- if cmdRunner.LastSecrets["API_KEY"] != "secret123" {
- t.Errorf("expected API_KEY in secrets, got %v", cmdRunner.LastSecrets)
+ // Verify up -d are at the end
+ argsStr := strings.Join(cmdRunner.LastArgs, " ")
+ if !strings.Contains(argsStr, "up -d") {
+ t.Errorf("expected 'up -d' in args, got %v", cmdRunner.LastArgs)
}📝 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.
| func TestRunDockerWithDeps_DockerCompose_Success(t *testing.T) { | |
| deps, _, _, _, cmdRunner, apiClient := NewTestDepsWithRunner() | |
| apiClient.PullResponse = &api.PullSecretsResponse{ | |
| Content: "API_KEY=secret123", | |
| } | |
| opts := DockerOptions{ | |
| EnvName: "production", | |
| EnvFlagSet: true, | |
| DockerCommand: "compose", | |
| DockerArgs: []string{"up", "-d"}, | |
| } | |
| err := runDockerWithDeps(opts, deps) | |
| if err != nil { | |
| t.Fatalf("unexpected error: %v", err) | |
| } | |
| // Verify docker was called | |
| if cmdRunner.LastCommand != "docker" { | |
| t.Errorf("expected command 'docker', got %q", cmdRunner.LastCommand) | |
| } | |
| // Verify args are "compose up -d" | |
| expectedArgs := []string{"compose", "up", "-d"} | |
| if len(cmdRunner.LastArgs) != len(expectedArgs) { | |
| t.Errorf("expected args %v, got %v", expectedArgs, cmdRunner.LastArgs) | |
| } | |
| for i, expected := range expectedArgs { | |
| if i < len(cmdRunner.LastArgs) && cmdRunner.LastArgs[i] != expected { | |
| t.Errorf("expected arg[%d] = %q, got %q", i, expected, cmdRunner.LastArgs[i]) | |
| } | |
| } | |
| // Verify secrets were passed via environment (compose uses env injection) | |
| if cmdRunner.LastSecrets["API_KEY"] != "secret123" { | |
| t.Errorf("expected API_KEY in secrets, got %v", cmdRunner.LastSecrets) | |
| } | |
| } | |
| func TestRunDockerWithDeps_DockerCompose_Success(t *testing.T) { | |
| deps, _, _, _, cmdRunner, apiClient := NewTestDepsWithRunner() | |
| apiClient.PullResponse = &api.PullSecretsResponse{ | |
| Content: "API_KEY=secret123", | |
| } | |
| opts := DockerOptions{ | |
| EnvName: "production", | |
| EnvFlagSet: true, | |
| DockerCommand: "compose", | |
| DockerArgs: []string{"up", "-d"}, | |
| } | |
| err := runDockerWithDeps(opts, deps) | |
| if err != nil { | |
| t.Fatalf("unexpected error: %v", err) | |
| } | |
| // Verify docker was called | |
| if cmdRunner.LastCommand != "docker" { | |
| t.Errorf("expected command 'docker', got %q", cmdRunner.LastCommand) | |
| } | |
| // Verify compose and --env-file are present | |
| if len(cmdRunner.LastArgs) < 4 { | |
| t.Fatalf("expected at least 4 args, got %v", cmdRunner.LastArgs) | |
| } | |
| if cmdRunner.LastArgs[0] != "compose" { | |
| t.Errorf("expected first arg 'compose', got %q", cmdRunner.LastArgs[0]) | |
| } | |
| if cmdRunner.LastArgs[1] != "--env-file" { | |
| t.Errorf("expected '--env-file' flag, got %q", cmdRunner.LastArgs[1]) | |
| } | |
| // Verify up -d are at the end | |
| argsStr := strings.Join(cmdRunner.LastArgs, " ") | |
| if !strings.Contains(argsStr, "up -d") { | |
| t.Errorf("expected 'up -d' in args, got %v", cmdRunner.LastArgs) | |
| } | |
| } |
🧰 Tools
🪛 GitHub Actions: CI
[error] 89-99: TestRunDockerWithDeps_DockerCompose_Success failed: expected args [compose up -d], got [compose --env-file /tmp/keyway-env-2596303086.env up -d] (and subsequent argument misalignment). This indicates the docker compose command formatting logic or test expectations are out of sync.
[error] 99-99: API_KEY not present in secrets in Docker compose test; test expectations not met due to environment/file generation differences.
🤖 Prompt for AI Agents
In `@internal/cmd/docker_test.go` around lines 61 - 101, The test
TestRunDockerWithDeps_DockerCompose_Success should be updated to match
runDockerWithDeps behavior: instead of asserting LastArgs equals
["compose","up","-d"] and checking cmdRunner.LastSecrets, assert that
cmdRunner.LastArgs contains the "--env-file" flag followed by a non-empty path,
and verify that the referenced temp file exists and contains the expected secret
"API_KEY=secret123"; remove or replace the LastSecrets assertion. Locate
assertions in TestRunDockerWithDeps_DockerCompose_Success and change them to
search for "--env-file" within cmdRunner.LastArgs, validate the following arg is
a valid file path, read that file and assert it includes the secret, and no
longer expect secrets in cmdRunner.LastSecrets.
| if len(opts.DockerArgs) > 0 && opts.DockerArgs[0] == "run" { | ||
| // Find position after "run" to inject -e flags | ||
| newArgs := []string{"compose", "run"} | ||
| for k, v := range secrets { | ||
| newArgs = append(newArgs, "-e", fmt.Sprintf("%s=%s", k, v)) | ||
| } | ||
| // Append remaining args after "run" | ||
| if len(opts.DockerArgs) > 1 { | ||
| newArgs = append(newArgs, opts.DockerArgs[1:]...) | ||
| } | ||
| return deps.CmdRunner.RunCommand("docker", newArgs, nil) | ||
| } |
There was a problem hiding this comment.
Missing user env precedence check for compose run.
Unlike runDockerRun, this path injects all vault secrets without checking if the user already provided -e flags for those keys. This breaks the PR requirement that "user-provided -e flags take precedence over vault secrets."
Proposed fix
if len(opts.DockerArgs) > 0 && opts.DockerArgs[0] == "run" {
+ // Extract user's -e flags to ensure they take precedence
+ userEnvVars := extractUserEnvVars(opts.DockerArgs)
+
// Find position after "run" to inject -e flags
newArgs := []string{"compose", "run"}
for k, v := range secrets {
- newArgs = append(newArgs, "-e", fmt.Sprintf("%s=%s", k, v))
+ if _, userSet := userEnvVars[k]; !userSet {
+ newArgs = append(newArgs, "-e", fmt.Sprintf("%s=%s", k, v))
+ }
}
// Append remaining args after "run"
if len(opts.DockerArgs) > 1 {
newArgs = append(newArgs, opts.DockerArgs[1:]...)
}
return deps.CmdRunner.RunCommand("docker", newArgs, nil)
}🤖 Prompt for AI Agents
In `@internal/cmd/docker.go` around lines 195 - 206, The compose-run branch
injects all vault secrets into newArgs without checking for user-supplied
-e/--env flags, violating the precedence rule; update the code in the block that
handles opts.DockerArgs starting with "run" to first scan opts.DockerArgs for
user-provided environment keys (handle "-e KEY=val", "-eKEY=val", "--env
KEY=val", and "--env=KEY=val" forms) and build a set of provided keys, then when
appending vault secrets to newArgs only add -e entries for keys not present in
that set (mirror the precedence logic used by runDockerRun), preserving the rest
of opts.DockerArgs and then call deps.CmdRunner.RunCommand("docker", newArgs,
nil).
Closes #2
Summary
Add a new
keyway dockercommand that injects vault secrets into Docker and Docker Compose commands.Features
keyway docker run-e KEY=VALUEflags before image namekeyway docker compose run-e KEY=VALUEflags afterrunkeyway docker compose up/down/etc--env-file-eflags take precedence over vault secrets--envflag not providedUsage
Files Changed
internal/cmd/docker.go- New command implementationinternal/cmd/docker_test.go- Comprehensive testsinternal/cmd/root.go- Register command + help textTest Plan
keyway docker run --rm alpine envshows injected secretskeyway docker compose runwith secretskeyway docker compose upwith secrets-eflags override vault secretsmake test- all tests passSummary by CodeRabbit
New Features
dockercommand that automatically injects vault secrets into Docker and Docker Compose commandsTests
✏️ Tip: You can customize this high-level summary in your review settings.