diff --git a/cmd/auth_exec.go b/cmd/auth_exec.go index 1184b8f3ca..a176659130 100644 --- a/cmd/auth_exec.go +++ b/cmd/auth_exec.go @@ -110,24 +110,17 @@ func executeAuthExecCommandCore(cmd *cobra.Command, args []string) error { // Prepare shell environment with file-based credentials. // Start with current OS environment + global env from atmos.yaml and let PrepareShellEnvironment configure auth. + // PrepareShellEnvironment sanitizes the env (removes IRSA/credential vars) and adds auth vars. baseEnv := envpkg.MergeGlobalEnv(os.Environ(), atmosConfig.Env) envList, err := authManager.PrepareShellEnvironment(ctx, identityName, baseEnv) if err != nil { return fmt.Errorf("failed to prepare command environment: %w", err) } - // Convert environment list to map for executeCommandWithEnv. - envMap := make(map[string]string) - for _, envVar := range envList { - if idx := strings.IndexByte(envVar, '='); idx >= 0 { - key := envVar[:idx] - value := envVar[idx+1:] - envMap[key] = value - } - } - - // Execute the command with authentication environment. - err = executeCommandWithEnv(commandArgs, envMap) + // Execute the command with the sanitized environment directly. + // The envList already includes os.Environ() (sanitized) + auth vars, + // so we pass it as the complete subprocess environment. + err = executeCommandWithEnv(commandArgs, envList) if err != nil { // For any subprocess error, provide a tip about refreshing credentials. // This helps users when AWS tokens are expired or invalid. @@ -137,29 +130,25 @@ func executeAuthExecCommandCore(cmd *cobra.Command, args []string) error { return nil } -// executeCommandWithEnv executes a command with additional environment variables. -func executeCommandWithEnv(args []string, envVars map[string]string) error { +// executeCommandWithEnv executes a command with a complete environment. +// The env parameter should be a fully prepared environment (e.g., from PrepareShellEnvironment). +// It is used directly as the subprocess environment without re-reading os.Environ(). +func executeCommandWithEnv(args []string, env []string) error { if len(args) == 0 { return fmt.Errorf(errUtils.ErrWrapFormat, errUtils.ErrNoCommandSpecified, errUtils.ErrInvalidSubcommand) } - // Prepare the command + // Prepare the command. cmdName := args[0] cmdArgs := args[1:] - // Look for the command in PATH + // Look for the command in PATH. cmdPath, err := exec.LookPath(cmdName) if err != nil { return fmt.Errorf(errUtils.ErrWrapFormat, errUtils.ErrCommandNotFound, err) } - // Prepare environment variables - env := os.Environ() - for key, value := range envVars { - env = append(env, fmt.Sprintf("%s=%s", key, value)) - } - - // Execute the command + // Execute the command with the provided environment directly. execCmd := exec.Command(cmdPath, cmdArgs...) execCmd.Env = env execCmd.Stdin = os.Stdin diff --git a/cmd/auth_exec_test.go b/cmd/auth_exec_test.go index 75c386267a..95d0be01a9 100644 --- a/cmd/auth_exec_test.go +++ b/cmd/auth_exec_test.go @@ -3,6 +3,7 @@ package cmd import ( "bytes" "errors" + "os" "runtime" "testing" @@ -218,52 +219,57 @@ func TestExtractIdentityFlag(t *testing.T) { } func TestExecuteCommandWithEnv(t *testing.T) { + // Use the test binary itself as a cross-platform subprocess helper. + // TestMain in testing_main_test.go handles _ATMOS_TEST_EXIT_ONE. + exePath, err := os.Executable() + require.NoError(t, err, "os.Executable() must succeed") + + // Prerequisite: verify that env vars reach the child process. + t.Run("env propagation to subprocess", func(t *testing.T) { + // Running the test binary with -test.run=^$ matches no tests and exits 0, + // confirming the subprocess receives the provided environment. + err := executeCommandWithEnv( + []string{exePath, "-test.run=^$"}, + []string{"TEST_VAR=test-value"}, + ) + assert.NoError(t, err) + }) + // Test the command execution helper directly. tests := []struct { name string args []string - envVars map[string]string - skipOnWindows bool + envVars []string expectedError string - expectedCode int // Expected exit code if error is ExitCodeError + expectedCode int // Expected exit code if error is ExitCodeError. }{ { name: "empty args", args: []string{}, - envVars: map[string]string{}, + envVars: []string{}, expectedError: "no command specified", }, { - name: "simple echo command", - args: []string{"echo", "hello"}, - envVars: map[string]string{ - "TEST_VAR": "test-value", - }, - skipOnWindows: true, + name: "successful command", + args: []string{exePath, "-test.run=^$"}, + envVars: []string{"TEST_VAR=test-value"}, }, { name: "nonexistent command", args: []string{"nonexistent-command-xyz"}, - envVars: map[string]string{}, + envVars: []string{}, expectedError: "command not found", }, { - name: "command with non-zero exit code", - args: []string{"sh", "-c", "exit 2"}, - envVars: map[string]string{ - "TEST_VAR": "test-value", - }, - skipOnWindows: true, - expectedCode: 2, + name: "command with non-zero exit code", + args: []string{exePath, "-test.run=^$"}, + envVars: []string{"_ATMOS_TEST_EXIT_ONE=1"}, + expectedCode: 1, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if tt.skipOnWindows && runtime.GOOS == "windows" { - t.Skipf("Skipping test on Windows: command behaves differently") - } - err := executeCommandWithEnv(tt.args, tt.envVars) switch { @@ -274,7 +280,7 @@ func TestExecuteCommandWithEnv(t *testing.T) { } case tt.expectedCode != 0: assert.Error(t, err) - // Check that it's an ExitCodeError with the correct code + // Check that it's an ExitCodeError with the correct code. var exitCodeErr errUtils.ExitCodeError if assert.True(t, errors.As(err, &exitCodeErr), "error should be ExitCodeError") { assert.Equal(t, tt.expectedCode, exitCodeErr.Code) diff --git a/cmd/auth_shell.go b/cmd/auth_shell.go index 302e773007..b479d487c8 100644 --- a/cmd/auth_shell.go +++ b/cmd/auth_shell.go @@ -115,6 +115,7 @@ func executeAuthShellCommandCore(cmd *cobra.Command, args []string) error { // Prepare shell environment with file-based credentials. // Start with current OS environment and let PrepareShellEnvironment configure auth. + // PrepareShellEnvironment sanitizes the env (removes IRSA/credential vars) and adds auth vars. envList, err := authManager.PrepareShellEnvironment(ctx, identityName, os.Environ()) if err != nil { return fmt.Errorf("failed to prepare shell environment: %w", err) @@ -129,18 +130,9 @@ func executeAuthShellCommandCore(cmd *cobra.Command, args []string) error { // Get provider name from the identity to display in shell messages. providerName := authManager.GetProviderForIdentity(identityName) - // Execute the shell with authentication environment. - // ExecAuthShellCommand expects env vars as a map, so convert the list. - envMap := make(map[string]string) - for _, envVar := range envList { - if idx := strings.IndexByte(envVar, '='); idx >= 0 { - key := envVar[:idx] - value := envVar[idx+1:] - envMap[key] = value - } - } - - return exec.ExecAuthShellCommand(atmosConfigPtr, identityName, providerName, envMap, shell, shellArgs) + // Execute the shell with the sanitized environment directly. + // envList already includes os.Environ() (sanitized) + auth vars. + return exec.ExecAuthShellCommand(atmosConfigPtr, identityName, providerName, envList, shell, shellArgs) } // extractAuthShellFlags extracts --identity and --shell flags from args and returns the remaining shell args. diff --git a/cmd/testing_main_test.go b/cmd/testing_main_test.go index fc1515da37..ef00b1a1cf 100644 --- a/cmd/testing_main_test.go +++ b/cmd/testing_main_test.go @@ -8,6 +8,12 @@ import ( // TestMain provides package-level test setup and teardown. // It ensures RootCmd state is properly managed across all tests in the package. func TestMain(m *testing.M) { + // Cross-platform subprocess helper: exit with code 1 when env flag is set. + // This lets tests use the test binary itself as a cross-platform "exit 1" command. + if os.Getenv("_ATMOS_TEST_EXIT_ONE") == "1" { + os.Exit(1) + } + // Capture initial RootCmd state. initialSnapshot := snapshotRootCmdState() diff --git a/docs/fixes/2026-03-23-auth-credential-chain-skipping-assume-role.md b/docs/fixes/2026-03-23-auth-credential-chain-skipping-assume-role.md new file mode 100644 index 0000000000..decdd461a7 --- /dev/null +++ b/docs/fixes/2026-03-23-auth-credential-chain-skipping-assume-role.md @@ -0,0 +1,75 @@ +# Fix: Auth credential chain skipping AssumeRole when target identity credentials are cached + +**Date:** 2026-03-23 + +**Reported by:** Alexander Matveev (9spokes) + +## Problem + +On EKS ARC (Actions Runner Controller) runners with IRSA, `atmos auth` returns stale cached credentials instead of performing the actual `AssumeRole` API call. Terraform runs with the runner's pod credentials instead of the Atmos-authenticated planner role. + +This is the **second bug** affecting IRSA on EKS. The first handles env var scrubbing — removing `AWS_WEB_IDENTITY_TOKEN_FILE`, `AWS_ROLE_ARN`, and `AWS_ROLE_SESSION_NAME` from the subprocess environment so the AWS SDK doesn't prefer pod identity over file-based credentials. This second bug is in the credential chain cache lookup and causes the `AssumeRole` step to be skipped entirely. + +### Root Cause + +In `findFirstValidCachedCredentials()`, the bottom-up scan finds valid cached credentials at the **last** identity in the chain (the target). The caller `fetchCachedCredentials()` then advances `startIndex` past the end of the chain, causing `authenticateIdentityChain`'s loop to never execute. + +**Walkthrough with a 2-element chain `[github-oidc, core-artifacts/terraform]`:** + +1. `findFirstValidCachedCredentials()` scans bottom-up, finds valid cached credentials at index 1 (`core-artifacts/terraform`) → returns `1` +2. `fetchCachedCredentials(1)` loads those cached creds and returns `startIndex = 1 + 1 = 2` +3. `authenticateIdentityChain(ctx, 2, cachedCreds)` runs the loop `for i := 2; i < 2` → **loop never executes** +4. The cached credentials are returned as-is, without ever calling `identity.Authenticate()` (the actual `AssumeRole` API call) + +The cached credentials at the last index represent the **output** of that step. `fetchCachedCredentials` is designed to return `startIndex + 1` so the **next** step can use them as input. But when there is no next step (last in chain), the index goes past the end and the identity loop is skipped entirely. + +This means if the credential file ever contains stale or incorrect credentials (e.g., written by a previous auth step in the same run, or from a provider-level auth that wrote base credentials under the identity's profile), they get returned without validation through the actual AWS STS call. + +### Why This Manifests on EKS ARC Runners + +On EKS runners with IRSA, the credential file can contain provider-level (OIDC) credentials that were cached during the provider authentication step. When `findFirstValidCachedCredentials` returns the last index, `fetchCachedCredentials` advances past the chain end, and these provider-level credentials are returned as if they were the assume-role credentials — without the actual `AssumeRole` call ever happening. + +## Fix + +In `findFirstValidCachedCredentials()`, skip cached credentials at the target (last) identity and continue scanning earlier in the chain. This ensures the identity's `Authenticate()` method is always called for the final step. + +**Changed file:** `pkg/auth/manager_chain.go` + +```go +// After validating credentials are not expired: +if i == len(m.chain)-1 { + log.Debug("Skipping cached target identity credentials to force re-authentication", + logKeyChainIndex, i, identityNameKey, identityName) + continue +} +return i +``` + +**Behavior with the fix for different chain configurations:** + +| Chain | Cache state | `findFirstValidCachedCredentials` returns | Result | +|-------|-------------|------------------------------------------|--------| +| `[provider, assume-role]` | Cache at index 1 (last) | Skip index 1 → check index 0 → if valid, return 0 | AssumeRole executes with provider creds as input | +| `[provider, assume-role]` | Cache only at index 1, nothing at 0 | Skip index 1 → index 0 invalid → return -1 | Full re-auth from provider | +| `[provider, assume-role]` | Cache at index 0 (provider) | Return 0 | Identity chain starts at 1, AssumeRole executes | +| `[provider, permset, assume-role]` | Cache at index 1 (middle) | Return 1 | `fetchCachedCredentials(1)` returns startIndex=2, assume-role step executes | + +This aligns with the existing comment on lines 30-33 of `manager_chain.go`: + +> "CRITICAL: Always re-authenticate through the full chain, even if the target identity has cached credentials." + +## Two Independent Bugs Affecting EKS ARC Runners + +There are two independent bugs that both affect EKS ARC runners with IRSA. Both fixes are needed for correct behavior: + +1. **IRSA env var scrubbing:** Pod-injected IRSA env vars (`AWS_WEB_IDENTITY_TOKEN_FILE`, `AWS_ROLE_ARN`, `AWS_ROLE_SESSION_NAME`) leak into the subprocess, causing AWS SDK to prefer web identity token auth over the Atmos-managed credential files. Fix: scrub these vars from the subprocess environment via `PrepareShellEnvironment`. + +2. **Credential chain cache lookup (this fix):** `findFirstValidCachedCredentials` returns the last chain index, `fetchCachedCredentials` advances past the chain end, and the `AssumeRole` call is skipped entirely. Fix: skip the last index in cache lookup to force re-authentication. + +Alexander confirmed that both fixes together resolved the issue for 9spokes. + +## Test Coverage + +Updated `TestManager_findFirstValidCachedCredentials` in `pkg/auth/manager_test.go`: + +- When both `id1` and `id2` have valid credentials, the function now returns `id1` (second-to-last, index 1) instead of `id2` (last, index 2), forcing re-authentication of the target identity. diff --git a/internal/exec/helmfile.go b/internal/exec/helmfile.go index a1af048e4c..d2fdc4498a 100644 --- a/internal/exec/helmfile.go +++ b/internal/exec/helmfile.go @@ -403,6 +403,7 @@ func ExecuteHelmfile(info schema.ConfigAndStacksInfo) error { envVars, info.DryRun, info.RedirectStdErr, + WithEnvironment(info.SanitizedEnv), ) if err != nil { return err diff --git a/internal/exec/packer.go b/internal/exec/packer.go index 856da3c932..96d99da490 100644 --- a/internal/exec/packer.go +++ b/internal/exec/packer.go @@ -289,5 +289,6 @@ func ExecutePacker( envVars, info.DryRun, info.RedirectStdErr, + WithEnvironment(info.SanitizedEnv), ) } diff --git a/internal/exec/shell_utils.go b/internal/exec/shell_utils.go index 9717deb69f..c7dce3593f 100644 --- a/internal/exec/shell_utils.go +++ b/internal/exec/shell_utils.go @@ -41,6 +41,10 @@ type shellCommandConfig struct { stdoutCapture io.Writer stderrCapture io.Writer stdoutOverride io.Writer + // processEnv replaces os.Environ() as the process environment. + // When set, ExecuteShellCommand uses this instead of re-reading os.Environ(). + // This is used when auth has already sanitized the environment (e.g., removed IRSA vars). + processEnv []string } // WithStdoutCapture returns a ShellCommandOption that tees stdout to the provided writer. @@ -68,6 +72,17 @@ func WithStdoutOverride(w io.Writer) ShellCommandOption { } } +// WithEnvironment provides a pre-sanitized process environment for subprocess execution. +// When provided, ExecuteShellCommand uses this instead of re-reading os.Environ(). +// Pass nil to fall back to the default os.Environ() behavior. +func WithEnvironment(env []string) ShellCommandOption { + defer perf.Track(nil, "exec.WithEnvironment")() + + return func(c *shellCommandConfig) { + c.processEnv = env + } +} + // ExecuteShellCommand prints and executes the provided command with args and flags. func ExecuteShellCommand( atmosConfig schema.AtmosConfiguration, @@ -93,9 +108,14 @@ func ExecuteShellCommand( } cmd := exec.Command(command, args...) - // Build environment: os.Environ() + global env (atmos.yaml) + command-specific env. - // Global env has lowest priority after system env, command-specific env overrides both. - cmdEnv := envpkg.MergeGlobalEnv(os.Environ(), atmosConfig.Env) + // Build environment: process env + global env (atmos.yaml) + command-specific env. + // When auth has sanitized the environment, cfg.processEnv is used instead of + // os.Environ() to avoid reintroducing problematic vars (e.g., IRSA credentials). + baseEnv := os.Environ() + if cfg.processEnv != nil { + baseEnv = cfg.processEnv + } + cmdEnv := envpkg.MergeGlobalEnv(baseEnv, atmosConfig.Env) cmdEnv = append(cmdEnv, env...) cmdEnv = append(cmdEnv, fmt.Sprintf("ATMOS_SHLVL=%d", newShellLevel)) @@ -395,12 +415,16 @@ func execTerraformShellCommand( } // ExecAuthShellCommand starts a new interactive shell with the provided authentication environment variables. -// It increments ATMOS_SHLVL for the session, sets ATMOS_IDENTITY plus the supplied auth env vars into the shell environment (merged with the host environment), prints enter/exit messages, and launches the resolved shell command; returns an error if no suitable shell is found or if the shell process fails. +// It increments ATMOS_SHLVL for the session, sets ATMOS_IDENTITY plus the supplied auth env vars into the shell environment, prints enter/exit messages, and launches the resolved shell command; returns an error if no suitable shell is found or if the shell process fails. +// +// The sanitizedEnv parameter should be a complete, pre-sanitized environment from PrepareShellEnvironment. +// It is used directly without re-reading os.Environ(), ensuring problematic vars (e.g., IRSA credentials) +// that were removed during auth preparation are not reintroduced. func ExecAuthShellCommand( atmosConfig *schema.AtmosConfiguration, identityName string, providerName string, - authEnvVars map[string]string, + sanitizedEnv []string, shellOverride string, shellArgs []string, ) error { @@ -414,12 +438,18 @@ func ExecAuthShellCommand( // Decrement the value after exiting the shell. defer decrementAtmosShellLevel() - // Convert auth env vars map to slice format. - authEnvList := envpkg.ConvertMapToSlice(authEnvVars) + // Append shell-specific env vars to the sanitized environment. + // The sanitizedEnv already includes os.Environ() (sanitized) + auth vars. + // Use UpdateEnvVar to replace-or-append each key, because os.StartProcess + // does not deduplicate — the first occurrence wins if duplicates exist. + shellEnv := append([]string{}, sanitizedEnv...) + shellEnv = envpkg.UpdateEnvVar(shellEnv, "ATMOS_IDENTITY", identityName) + shellEnv = envpkg.UpdateEnvVar(shellEnv, atmosShellLevelEnvVar, strconv.Itoa(atmosShellVal)) - // Set environment variables to indicate the details of the Atmos auth shell configuration. - authEnvList = append(authEnvList, fmt.Sprintf("ATMOS_IDENTITY=%s", identityName)) - authEnvList = append(authEnvList, fmt.Sprintf("%s=%d", atmosShellLevelEnvVar, atmosShellVal)) + // Append global env from atmos.yaml. + for k, v := range atmosConfig.Env { + shellEnv = envpkg.UpdateEnvVar(shellEnv, k, v) + } log.Debug("Starting a new interactive shell with authentication environment variables (type 'exit' to go back)", "identity", identityName) @@ -435,9 +465,8 @@ func ExecAuthShellCommand( // Print user-facing message about entering the shell. printShellEnterMessage(identityName, providerName) - // Merge env vars, ensuring authEnvList takes precedence. - // Include global env from atmos.yaml (lowest priority after system env). - mergedEnv := envpkg.MergeSystemEnvSimpleWithGlobal(authEnvList, atmosConfig.Env) + // Use the sanitized environment directly (no re-reading os.Environ()). + mergedEnv := shellEnv // Determine shell command and args. shellCommand, shellCommandArgs := determineShell(shellOverride, shellArgs) diff --git a/internal/exec/shell_utils_test.go b/internal/exec/shell_utils_test.go index 3e7aa7d83e..47033e8d5c 100644 --- a/internal/exec/shell_utils_test.go +++ b/internal/exec/shell_utils_test.go @@ -463,9 +463,7 @@ func TestExecAuthShellCommand_ExitCodePropagation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - envVars := map[string]string{ - "TEST_VAR": "test_value", - } + envVars := []string{"TEST_VAR=test_value"} atmosConfig := &schema.AtmosConfiguration{} err := ExecAuthShellCommand(atmosConfig, "test-identity", "test-provider", envVars, "/bin/sh", tt.shellArgs) diff --git a/internal/exec/terraform.go b/internal/exec/terraform.go index 788671efe6..6cd9ae3b99 100644 --- a/internal/exec/terraform.go +++ b/internal/exec/terraform.go @@ -112,6 +112,8 @@ func ExecuteTerraform(info schema.ConfigAndStacksInfo, opts ...ShellCommandOptio } // Run the full command pipeline: init, arg build, workspace, execute, cleanup. + // Forward caller-provided options (e.g. CI stdout/stderr capture) alongside the environment option. + opts = append(opts, WithEnvironment(info.SanitizedEnv)) return executeCommandPipeline(&atmosConfig, &info, execCtx, opts...) } diff --git a/internal/exec/terraform_execute_helpers.go b/internal/exec/terraform_execute_helpers.go index 3b9501facc..74444072d7 100644 --- a/internal/exec/terraform_execute_helpers.go +++ b/internal/exec/terraform_execute_helpers.go @@ -483,7 +483,7 @@ func prepareInitExecution(atmosConfig *schema.AtmosConfiguration, info *schema.C // buildInitSubcommandArgs in terraform_execute_helpers_args.go handles the provisioner // invocation via prepareInitExecution. These two code paths must never both execute // in the same command invocation or provisioners will run twice. -func executeTerraformInitPhase(atmosConfig *schema.AtmosConfiguration, info *schema.ConfigAndStacksInfo, componentPath, varFile string) (string, error) { +func executeTerraformInitPhase(atmosConfig *schema.AtmosConfiguration, info *schema.ConfigAndStacksInfo, componentPath, varFile string, opts ...ShellCommandOption) (string, error) { newPath, err := prepareInitExecution(atmosConfig, info, componentPath) if err != nil { return componentPath, err @@ -498,6 +498,7 @@ func executeTerraformInitPhase(atmosConfig *schema.AtmosConfiguration, info *sch info.ComponentEnvList, info.DryRun, info.RedirectStdErr, + opts..., ); err != nil { return newPath, err } diff --git a/internal/exec/terraform_execute_helpers_exec.go b/internal/exec/terraform_execute_helpers_exec.go index c1a8b5dab1..1abc4b1c4f 100644 --- a/internal/exec/terraform_execute_helpers_exec.go +++ b/internal/exec/terraform_execute_helpers_exec.go @@ -132,7 +132,7 @@ func executeCommandPipeline( if shouldRunTerraformInit(atmosConfig, info) { var err error - componentPath, err = executeTerraformInitPhase(atmosConfig, info, componentPath, execCtx.varFile) + componentPath, err = executeTerraformInitPhase(atmosConfig, info, componentPath, execCtx.varFile, opts...) if err != nil { return err } @@ -146,7 +146,7 @@ func executeCommandPipeline( return err } - if err = runWorkspaceSetup(atmosConfig, info, componentPath); err != nil { + if err = runWorkspaceSetup(atmosConfig, info, componentPath, opts...); err != nil { return err } @@ -178,7 +178,7 @@ func shouldSkipWorkspaceSetup(info *schema.ConfigAndStacksInfo) bool { // runWorkspaceSetup selects (or creates) the Terraform workspace before the main command // runs. It is a no-op when shouldSkipWorkspaceSetup returns true. -func runWorkspaceSetup(atmosConfig *schema.AtmosConfiguration, info *schema.ConfigAndStacksInfo, componentPath string) error { +func runWorkspaceSetup(atmosConfig *schema.AtmosConfiguration, info *schema.ConfigAndStacksInfo, componentPath string, opts ...ShellCommandOption) error { if shouldSkipWorkspaceSetup(info) { return nil } @@ -191,7 +191,8 @@ func runWorkspaceSetup(atmosConfig *schema.AtmosConfiguration, info *schema.Conf // For data-producing subcommands redirect "Switched to workspace…" to stderr // so it doesn't pollute captured stdout in $() substitutions. - var wsOpts []ShellCommandOption + // Merge caller-provided opts (e.g., WithEnvironment) with workspace-specific opts. + wsOpts := append([]ShellCommandOption{}, opts...) if info.SubCommand == "output" || info.SubCommand == "show" { wsOpts = append(wsOpts, WithStdoutOverride(os.Stderr)) } @@ -216,6 +217,13 @@ func runWorkspaceSetup(atmosConfig *schema.AtmosConfiguration, info *schema.Conf return err } + return createWorkspaceFallback(atmosConfig, info, componentPath, opts...) +} + +// createWorkspaceFallback attempts to create a new workspace when select fails. +// If workspace creation also fails with exit code 1 and the workspace is already +// active (stale .terraform/environment file), it proceeds with a warning. +func createWorkspaceFallback(atmosConfig *schema.AtmosConfiguration, info *schema.ConfigAndStacksInfo, componentPath string, opts ...ShellCommandOption) error { newErr := ExecuteShellCommand( *atmosConfig, info.Command, @@ -224,6 +232,7 @@ func runWorkspaceSetup(atmosConfig *schema.AtmosConfiguration, info *schema.Conf info.ComponentEnvList, info.DryRun, info.RedirectStdErr, + opts..., ) if newErr == nil { return nil diff --git a/pkg/auth/cloud/aws/env.go b/pkg/auth/cloud/aws/env.go index 49483d97c2..70e1cab1d9 100644 --- a/pkg/auth/cloud/aws/env.go +++ b/pkg/auth/cloud/aws/env.go @@ -35,6 +35,14 @@ var problematicAWSEnvVars = []string{ "AWS_CONFIG_FILE", "AWS_SHARED_CREDENTIALS_FILE", + // Web identity / IRSA (EKS pod-injected variables). + // On EKS pods with IRSA, these are injected by the pod identity webhook. + // They must be cleared during Atmos auth to prevent the AWS SDK from using + // pod credentials instead of Atmos-managed credentials. + "AWS_WEB_IDENTITY_TOKEN_FILE", + "AWS_ROLE_ARN", + "AWS_ROLE_SESSION_NAME", + // Note: AWS_REGION is intentionally NOT in this list as it's safe to inherit. } @@ -246,6 +254,11 @@ func PrepareEnvironment(environ map[string]string, profile, credentialsFile, con // Clear problematic credential environment variables. // When using profile-based authentication, these variables would override // the credentials from AWS_SHARED_CREDENTIALS_FILE, causing auth to fail. + // + // This deletes keys from the map. Callers that pass os.Environ() as input will + // get a sanitized result with IRSA/credential vars removed. The sanitized env + // should be passed to subprocess execution via WithEnvironment to avoid re-reading + // os.Environ() (which would reintroduce the problematic vars). for _, key := range environmentVarsToClear { if _, exists := result[key]; exists { log.Debug("Clearing AWS credential environment variable", "key", key) diff --git a/pkg/auth/cloud/aws/env_test.go b/pkg/auth/cloud/aws/env_test.go index d23356f682..e7a8d8b2a3 100644 --- a/pkg/auth/cloud/aws/env_test.go +++ b/pkg/auth/cloud/aws/env_test.go @@ -41,6 +41,8 @@ func TestPrepareEnvironment(t *testing.T) { "AWS_REGION": "us-west-2", "AWS_DEFAULT_REGION": "us-west-2", "AWS_EC2_METADATA_DISABLED": "true", + // Credential/IRSA vars are deleted (not present in result). + // Scrubbing happens at subprocess level via WithEnvironment. }, }, { @@ -66,6 +68,7 @@ func TestPrepareEnvironment(t *testing.T) { "AWS_PROFILE": "test-profile", "AWS_SDK_LOAD_CONFIG": "1", "AWS_EC2_METADATA_DISABLED": "true", + // Credential/IRSA vars are deleted from the result map. }, }, { @@ -165,11 +168,12 @@ func TestPrepareEnvironment_DoesNotMutateInput(t *testing.T) { assert.Equal(t, originalAccessKey, inputEnv["AWS_ACCESS_KEY_ID"], "AWS_ACCESS_KEY_ID should not be modified in input") assert.Equal(t, originalSecretKey, inputEnv["AWS_SECRET_ACCESS_KEY"], "AWS_SECRET_ACCESS_KEY should not be modified in input") - // Verify result does not contain credentials. + // Verify result has credentials deleted (not present in the result map). + // Scrubbing from subprocess env happens at the WithEnvironment level. _, hasAccessKey := result["AWS_ACCESS_KEY_ID"] + assert.False(t, hasAccessKey, "AWS_ACCESS_KEY_ID should be absent from result") _, hasSecretKey := result["AWS_SECRET_ACCESS_KEY"] - assert.False(t, hasAccessKey, "result should not contain AWS_ACCESS_KEY_ID") - assert.False(t, hasSecretKey, "result should not contain AWS_SECRET_ACCESS_KEY") + assert.False(t, hasSecretKey, "AWS_SECRET_ACCESS_KEY should be absent from result") // Verify result contains expected values. assert.Equal(t, "test-profile", result["AWS_PROFILE"]) @@ -344,6 +348,9 @@ func TestProblematicAWSEnvVars_ContainsExpectedVars(t *testing.T) { "AWS_PROFILE", "AWS_CONFIG_FILE", "AWS_SHARED_CREDENTIALS_FILE", + "AWS_WEB_IDENTITY_TOKEN_FILE", + "AWS_ROLE_ARN", + "AWS_ROLE_SESSION_NAME", } for _, expected := range expectedVars { @@ -510,3 +517,80 @@ func TestEnvironmentVarsToClear_ContainsExpectedVars(t *testing.T) { assert.True(t, found, "environmentVarsToClear should contain %s", expected) } } + +// TestPrepareEnvironment_IRSALeakPrevention verifies that IRSA (IAM Roles for +// Service Accounts) environment variables injected by the EKS pod identity +// webhook are deleted by PrepareEnvironment and do not leak through to +// Terraform subprocesses. This reproduces the real-world scenario from +// DEV-4216 where ARC runner pods have IRSA env vars injected by webhook. +// +// With the sanitized base env approach: +// 1. os.Environ() (including IRSA vars) is passed to PrepareEnvironment +// 2. PrepareEnvironment deletes IRSA vars and adds Atmos-managed auth vars +// 3. The sanitized result is used as the subprocess base env via WithEnvironment +func TestPrepareEnvironment_IRSALeakPrevention(t *testing.T) { + // Simulate IRSA vars injected by EKS pod identity webhook. + irsaTokenFile := "/var/run/secrets/eks.amazonaws.com/serviceaccount/token" + irsaRoleArn := "arn:aws:iam::241533146093:role/gold-core-use1-auto-actions-runner@actions-runner-system" + + // Simulate os.Environ() from the pod (what the subprocess would inherit). + podEnviron := []string{ + "HOME=/home/runner", + "PATH=/usr/bin", + "AWS_WEB_IDENTITY_TOKEN_FILE=" + irsaTokenFile, + "AWS_ROLE_ARN=" + irsaRoleArn, + "AWS_ROLE_SESSION_NAME=irsa-session", + "AWS_DEFAULT_REGION=us-east-1", + "AWS_REGION=us-east-1", + "AWS_STS_REGIONAL_ENDPOINTS=regional", + } + + // Convert podEnviron []string to map[string]string for PrepareEnvironment. + // This simulates the new flow: os.Environ() (with IRSA vars) is converted to a map + // and passed to PrepareEnvironment, which deletes IRSA vars and adds auth vars. + podEnvMap := make(map[string]string) + for _, entry := range podEnviron { + if idx := indexOf(entry, '='); idx >= 0 { + podEnvMap[entry[:idx]] = entry[idx+1:] + } + } + + result := PrepareEnvironment( + podEnvMap, + "core-artifacts/terraform", + "/home/runner/.config/atmos/aws/github-oidc/credentials", + "/home/runner/.config/atmos/aws/github-oidc/config", + "us-east-1", + ) + + // The fix: IRSA vars should be deleted from the result, not leak through to subprocess. + _, hasWebIdentity := result["AWS_WEB_IDENTITY_TOKEN_FILE"] + assert.False(t, hasWebIdentity, + "AWS_WEB_IDENTITY_TOKEN_FILE should be deleted, not leak IRSA token path") + _, hasRoleARN := result["AWS_ROLE_ARN"] + assert.False(t, hasRoleARN, + "AWS_ROLE_ARN should be deleted, not leak IRSA role ARN") + _, hasRoleSession := result["AWS_ROLE_SESSION_NAME"] + assert.False(t, hasRoleSession, + "AWS_ROLE_SESSION_NAME should be deleted, not leak IRSA session name") + + // Atmos-managed credentials should be set correctly. + assert.Equal(t, "/home/runner/.config/atmos/aws/github-oidc/credentials", + result["AWS_SHARED_CREDENTIALS_FILE"]) + assert.Equal(t, "core-artifacts/terraform", result["AWS_PROFILE"]) + assert.Equal(t, "true", result["AWS_EC2_METADATA_DISABLED"]) + + // Non-AWS vars from the pod environment should be preserved. + assert.Equal(t, "/home/runner", result["HOME"]) + assert.Equal(t, "/usr/bin", result["PATH"]) +} + +// indexOf returns the index of the first occurrence of b in s, or -1. +func indexOf(s string, b byte) int { + for i := range len(s) { + if s[i] == b { + return i + } + } + return -1 +} diff --git a/pkg/auth/cloud/aws/setup_test.go b/pkg/auth/cloud/aws/setup_test.go index d46a3414d5..ccf5a2a4fd 100644 --- a/pkg/auth/cloud/aws/setup_test.go +++ b/pkg/auth/cloud/aws/setup_test.go @@ -206,22 +206,22 @@ func TestSetEnvironmentVariables_ClearsConflictingCredentials(t *testing.T) { err := SetEnvironmentVariables(authContext, stack) require.NoError(t, err) - // Verify conflicting credential environment variables were cleared. + // Verify conflicting credential environment variables were deleted (not present). + // Scrubbing from subprocess env happens at the WithEnvironment level. _, hasAccessKey := stack.ComponentEnvSection["AWS_ACCESS_KEY_ID"] + assert.False(t, hasAccessKey, "AWS_ACCESS_KEY_ID should be absent") _, hasSecretKey := stack.ComponentEnvSection["AWS_SECRET_ACCESS_KEY"] + assert.False(t, hasSecretKey, "AWS_SECRET_ACCESS_KEY should be absent") _, hasSessionToken := stack.ComponentEnvSection["AWS_SESSION_TOKEN"] + assert.False(t, hasSessionToken, "AWS_SESSION_TOKEN should be absent") _, hasSecurityToken := stack.ComponentEnvSection["AWS_SECURITY_TOKEN"] - _, hasWebIdentityToken := stack.ComponentEnvSection["AWS_WEB_IDENTITY_TOKEN_FILE"] - _, hasRoleArn := stack.ComponentEnvSection["AWS_ROLE_ARN"] - _, hasRoleSessionName := stack.ComponentEnvSection["AWS_ROLE_SESSION_NAME"] - - assert.False(t, hasAccessKey, "AWS_ACCESS_KEY_ID should be cleared") - assert.False(t, hasSecretKey, "AWS_SECRET_ACCESS_KEY should be cleared") - assert.False(t, hasSessionToken, "AWS_SESSION_TOKEN should be cleared") - assert.False(t, hasSecurityToken, "AWS_SECURITY_TOKEN should be cleared") - assert.False(t, hasWebIdentityToken, "AWS_WEB_IDENTITY_TOKEN_FILE should be cleared") - assert.False(t, hasRoleArn, "AWS_ROLE_ARN should be cleared") - assert.False(t, hasRoleSessionName, "AWS_ROLE_SESSION_NAME should be cleared") + assert.False(t, hasSecurityToken, "AWS_SECURITY_TOKEN should be absent") + _, hasWebIdentity := stack.ComponentEnvSection["AWS_WEB_IDENTITY_TOKEN_FILE"] + assert.False(t, hasWebIdentity, "AWS_WEB_IDENTITY_TOKEN_FILE should be absent") + _, hasRoleARN := stack.ComponentEnvSection["AWS_ROLE_ARN"] + assert.False(t, hasRoleARN, "AWS_ROLE_ARN should be absent") + _, hasRoleSession := stack.ComponentEnvSection["AWS_ROLE_SESSION_NAME"] + assert.False(t, hasRoleSession, "AWS_ROLE_SESSION_NAME should be absent") // Verify non-AWS environment variables were preserved. assert.Equal(t, "/home/user", stack.ComponentEnvSection["HOME"]) diff --git a/pkg/auth/hooks.go b/pkg/auth/hooks.go index 33074a5c61..b52c60125d 100644 --- a/pkg/auth/hooks.go +++ b/pkg/auth/hooks.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "os" "strings" "github.com/go-viper/mapstructure/v2" @@ -127,18 +128,27 @@ func authenticateAndWriteEnv(ctx context.Context, authManager types.AuthManager, // This is needed for Terraform components that use multiple AWS provider aliases. authenticateAdditionalIdentities(ctx, authManager, identityName) - // Convert ComponentEnvSection to env list for PrepareShellEnvironment. - // This includes any component-specific env vars already set in the stack config. - baseEnvList := componentEnvSectionToList(stackInfo.ComponentEnvSection) + // Build base env: os.Environ() + existing stack env vars from ComponentEnvSection. + // Including os.Environ() ensures PrepareShellEnvironment can delete problematic vars + // (e.g., IRSA credentials injected by EKS pod identity webhook) from the full + // process environment, producing a sanitized base for subprocess execution. + baseEnvList := mergeOsEnvironWithSection(os.Environ(), stackInfo.ComponentEnvSection) // Prepare shell environment with auth credentials. - // This configures file-based credentials (AWS_SHARED_CREDENTIALS_FILE, AWS_PROFILE, etc.). + // This configures file-based credentials (AWS_SHARED_CREDENTIALS_FILE, AWS_PROFILE, etc.) + // and removes problematic credential vars (IRSA, direct credentials). envList, err := authManager.PrepareShellEnvironment(ctx, identityName, baseEnvList) if err != nil { return fmt.Errorf("failed to prepare environment variables: %w", err) } - // Convert back to ComponentEnvSection map for downstream processing. + // Store sanitized environment as the base for subprocess execution. + // ExecuteShellCommand will use this instead of re-reading os.Environ(), + // which would reintroduce the problematic vars that were just removed. + stackInfo.SanitizedEnv = envList + + // Extract auth-specific vars back to ComponentEnvSection for logging/display + // and downstream processing (e.g., terraform.go adds ATMOS-specific vars). if stackInfo.ComponentEnvSection == nil { stackInfo.ComponentEnvSection = make(map[string]any) } @@ -146,7 +156,10 @@ func authenticateAndWriteEnv(ctx context.Context, authManager types.AuthManager, if idx := strings.IndexByte(envVar, '='); idx >= 0 { key := envVar[:idx] value := envVar[idx+1:] - stackInfo.ComponentEnvSection[key] = value + // Only write auth-managed vars to ComponentEnvSection, not the full os env. + if isAuthManagedVar(key) { + stackInfo.ComponentEnvSection[key] = value + } } } @@ -156,6 +169,55 @@ func authenticateAndWriteEnv(ctx context.Context, authManager types.AuthManager, return nil } +// mergeOsEnvironWithSection merges os.Environ() with ComponentEnvSection into a single env list. +// ComponentEnvSection values override os.Environ() values for the same key. +func mergeOsEnvironWithSection(osEnviron []string, envSection map[string]any) []string { + // Start with os.Environ() as base. + result := make([]string, 0, len(osEnviron)+len(envSection)) + result = append(result, osEnviron...) + + // Append ComponentEnvSection entries (override os.Environ() via last-occurrence-wins). + for k, v := range envSection { + if v != nil { + result = append(result, fmt.Sprintf("%s=%v", k, v)) + } + } + + return result +} + +// isAuthManagedVar returns true if the env var key is managed by the auth system. +// These are the vars that PrepareEnvironment sets for Atmos-managed credentials. +func isAuthManagedVar(key string) bool { + switch key { + case "AWS_SHARED_CREDENTIALS_FILE", + "AWS_CONFIG_FILE", + "AWS_PROFILE", + "AWS_SDK_LOAD_CONFIG", + "AWS_REGION", + "AWS_DEFAULT_REGION", + "AWS_EC2_METADATA_DISABLED", + // Azure auth-managed vars. + "AZURE_CONFIG_DIR", + "AZURE_SUBSCRIPTION_ID", + "ARM_SUBSCRIPTION_ID", + "ARM_TENANT_ID", + "ARM_CLIENT_ID", + "ARM_CLIENT_SECRET", + "ARM_USE_OIDC", + "ARM_OIDC_TOKEN", + // GCP auth-managed vars. + "GOOGLE_APPLICATION_CREDENTIALS", + "CLOUDSDK_CONFIG", + "GOOGLE_CLOUD_PROJECT", + "GCLOUD_PROJECT", + "CLOUDSDK_CORE_PROJECT": + return true + default: + return false + } +} + // authenticateAdditionalIdentities authenticates non-primary identities marked as required. // Failures are non-fatal: errors are logged as warnings but don't fail the hook. // This ensures all required profiles exist in the shared credentials file for Terraform diff --git a/pkg/auth/manager_chain.go b/pkg/auth/manager_chain.go index 2422a9df74..16a7ca136e 100644 --- a/pkg/auth/manager_chain.go +++ b/pkg/auth/manager_chain.go @@ -58,6 +58,21 @@ func (m *manager) findFirstValidCachedCredentials() int { // Credentials without expiration (API keys, long-lived tokens, etc.). log.Debug("Found valid cached credentials", logKeyChainIndex, i, identityNameKey, identityName, logKeyExpirationChain, "none") } + + // Skip cached credentials at the target (last) identity in the chain. + // The cached output of the last step cannot be used as input to any further step + // because there are no further steps. fetchCachedCredentials would advance the + // startIndex past the end of the chain, causing authenticateIdentityChain's loop + // to never execute — returning stale/incorrect cached credentials without + // performing the actual AssumeRole (or equivalent) API call. + // Instead, continue scanning earlier in the chain for a valid cache point + // whose output CAN feed into the identity chain for re-authentication. + if i == len(m.chain)-1 { + log.Debug("Skipping cached target identity credentials to force re-authentication", + logKeyChainIndex, i, identityNameKey, identityName) + continue + } + return i } diff --git a/pkg/auth/manager_test.go b/pkg/auth/manager_test.go index 55d97b5408..6db745a549 100644 --- a/pkg/auth/manager_test.go +++ b/pkg/auth/manager_test.go @@ -702,11 +702,14 @@ func TestManager_findFirstValidCachedCredentials(t *testing.T) { expiredExp := now.Add(-1 * time.Hour) // Expired: expired 1 hour ago. m := &manager{credentialStore: s, chain: []string{"prov", "id1", "id2"}} - // Both id1 and id2 have valid credentials -> should return id2 (last in chain). + // Both id1 and id2 have valid credentials -> should return id1 (second-to-last). + // The target identity (last in chain) is always skipped to force re-authentication + // via its Authenticate() method, preventing stale cached credentials from being + // returned without performing the actual API call (e.g., AssumeRole). s.data["id2"] = &testCreds{exp: &validExp} s.data["id1"] = &testCreds{exp: &validExp} idx := m.findFirstValidCachedCredentials() - require.Equal(t, 2, idx) + require.Equal(t, 1, idx) // id2 expired, id1 still valid -> should pick id1. s.data["id2"] = &testCreds{exp: &expiredExp} diff --git a/pkg/list/utils/check_component_test.go b/pkg/list/utils/check_component_test.go index 616821cf5b..e6603e281b 100644 --- a/pkg/list/utils/check_component_test.go +++ b/pkg/list/utils/check_component_test.go @@ -1,186 +1,140 @@ package utils import ( - "errors" "testing" - "github.com/agiledragon/gomonkey/v2" "github.com/stretchr/testify/assert" - - e "github.com/cloudposse/atmos/internal/exec" - "github.com/cloudposse/atmos/pkg/auth" - "github.com/cloudposse/atmos/pkg/schema" ) -// TestCheckComponentExists_EmptyComponentName tests early return at utils.go:18-20. +// TestCheckComponentExists_EmptyComponentName tests early return for empty name. func TestCheckComponentExists_EmptyComponentName(t *testing.T) { - atmosConfig := &schema.AtmosConfiguration{} - - result := CheckComponentExists(atmosConfig, "") + result := CheckComponentExists(nil, "") assert.False(t, result) } -// TestCheckComponentExists_ExecuteDescribeStacksError tests error path at utils.go:23-26. -func TestCheckComponentExists_ExecuteDescribeStacksError(t *testing.T) { - atmosConfig := &schema.AtmosConfiguration{} - - // Mock ExecuteDescribeStacks to return an error. - patches := gomonkey.ApplyFunc(e.ExecuteDescribeStacks, - func(*schema.AtmosConfiguration, string, []string, []string, []string, bool, bool, bool, bool, []string, auth.AuthManager) (map[string]any, error) { - return nil, errors.New("simulated ExecuteDescribeStacks error") - }) - defer patches.Reset() - - result := CheckComponentExists(atmosConfig, "test-component") - assert.False(t, result) // Should return false on error -} - -// TestCheckComponentExists_EmptyStacksMap tests path at utils.go:29. -func TestCheckComponentExists_EmptyStacksMap(t *testing.T) { - atmosConfig := &schema.AtmosConfiguration{} - - // Mock ExecuteDescribeStacks to return empty map. - patches := gomonkey.ApplyFunc(e.ExecuteDescribeStacks, - func(*schema.AtmosConfiguration, string, []string, []string, []string, bool, bool, bool, bool, []string, auth.AuthManager) (map[string]any, error) { - return map[string]any{}, nil - }) - defer patches.Reset() - - result := CheckComponentExists(atmosConfig, "test-component") - assert.False(t, result) // Should return false when no stacks +// TestComponentExistsInStacks_EmptyMap tests empty stacks map. +func TestComponentExistsInStacks_EmptyMap(t *testing.T) { + result := componentExistsInStacks(map[string]any{}, "test-component") + assert.False(t, result) } -// TestCheckComponentExists_InvalidStackData tests type assertion at utils.go:30-33. -func TestCheckComponentExists_InvalidStackData(t *testing.T) { - atmosConfig := &schema.AtmosConfiguration{} - - // Mock ExecuteDescribeStacks to return invalid stack data. - patches := gomonkey.ApplyFunc(e.ExecuteDescribeStacks, - func(*schema.AtmosConfiguration, string, []string, []string, []string, bool, bool, bool, bool, []string, auth.AuthManager) (map[string]any, error) { - return map[string]any{ - "stack1": "invalid-not-a-map", // Invalid type - }, nil - }) - defer patches.Reset() - - result := CheckComponentExists(atmosConfig, "test-component") - assert.False(t, result) // Should skip invalid stack data +// TestComponentExistsInStacks_InvalidStackData tests invalid stack data type. +func TestComponentExistsInStacks_InvalidStackData(t *testing.T) { + stacks := map[string]any{ + "stack1": "invalid-not-a-map", + } + result := componentExistsInStacks(stacks, "test-component") + assert.False(t, result) } -// TestCheckComponentExists_NoComponentsKey tests path at utils.go:35-38. -func TestCheckComponentExists_NoComponentsKey(t *testing.T) { - atmosConfig := &schema.AtmosConfiguration{} - - // Mock ExecuteDescribeStacks to return stack without components key. - patches := gomonkey.ApplyFunc(e.ExecuteDescribeStacks, - func(*schema.AtmosConfiguration, string, []string, []string, []string, bool, bool, bool, bool, []string, auth.AuthManager) (map[string]any, error) { - return map[string]any{ - "stack1": map[string]interface{}{ - "other_key": "value", - // No "components" key - }, - }, nil - }) - defer patches.Reset() - - result := CheckComponentExists(atmosConfig, "test-component") +// TestComponentExistsInStacks_NoComponentsKey tests stack without components key. +func TestComponentExistsInStacks_NoComponentsKey(t *testing.T) { + stacks := map[string]any{ + "stack1": map[string]interface{}{ + "other_key": "value", + }, + } + result := componentExistsInStacks(stacks, "test-component") assert.False(t, result) } -// TestCheckComponentExists_InvalidComponentsType tests type assertion at utils.go:35-38. -func TestCheckComponentExists_InvalidComponentsType(t *testing.T) { - atmosConfig := &schema.AtmosConfiguration{} - - // Mock ExecuteDescribeStacks to return invalid components type. - patches := gomonkey.ApplyFunc(e.ExecuteDescribeStacks, - func(*schema.AtmosConfiguration, string, []string, []string, []string, bool, bool, bool, bool, []string, auth.AuthManager) (map[string]any, error) { - return map[string]any{ - "stack1": map[string]interface{}{ - "components": "invalid-not-a-map", - }, - }, nil - }) - defer patches.Reset() - - result := CheckComponentExists(atmosConfig, "test-component") +// TestComponentExistsInStacks_InvalidComponentsType tests invalid components type. +func TestComponentExistsInStacks_InvalidComponentsType(t *testing.T) { + stacks := map[string]any{ + "stack1": map[string]interface{}{ + "components": "invalid-not-a-map", + }, + } + result := componentExistsInStacks(stacks, "test-component") assert.False(t, result) } -// TestCheckComponentExists_InvalidComponentTypeMap tests type assertion at utils.go:41-44. -func TestCheckComponentExists_InvalidComponentTypeMap(t *testing.T) { - atmosConfig := &schema.AtmosConfiguration{} - - // Mock ExecuteDescribeStacks to return invalid component type map. - patches := gomonkey.ApplyFunc(e.ExecuteDescribeStacks, - func(*schema.AtmosConfiguration, string, []string, []string, []string, bool, bool, bool, bool, []string, auth.AuthManager) (map[string]any, error) { - return map[string]any{ - "stack1": map[string]interface{}{ - "components": map[string]interface{}{ - "terraform": "invalid-not-a-map", // Invalid type - }, - }, - }, nil - }) - defer patches.Reset() - - result := CheckComponentExists(atmosConfig, "test-component") +// TestComponentExistsInStacks_InvalidComponentTypeMap tests invalid component type map. +func TestComponentExistsInStacks_InvalidComponentTypeMap(t *testing.T) { + stacks := map[string]any{ + "stack1": map[string]interface{}{ + "components": map[string]interface{}{ + "terraform": "invalid-not-a-map", + }, + }, + } + result := componentExistsInStacks(stacks, "test-component") assert.False(t, result) } -// TestCheckComponentExists_ComponentNotFound tests path at utils.go:46-49 (not found). -func TestCheckComponentExists_ComponentNotFound(t *testing.T) { - atmosConfig := &schema.AtmosConfiguration{} - - // Mock ExecuteDescribeStacks to return stacks without the target component. - patches := gomonkey.ApplyFunc(e.ExecuteDescribeStacks, - func(*schema.AtmosConfiguration, string, []string, []string, []string, bool, bool, bool, bool, []string, auth.AuthManager) (map[string]any, error) { - return map[string]any{ - "stack1": map[string]interface{}{ - "components": map[string]interface{}{ - "terraform": map[string]interface{}{ - "vpc": map[string]interface{}{"var1": "value1"}, - "eks": map[string]interface{}{"var2": "value2"}, - "other": map[string]interface{}{"var3": "value3"}, - // "test-component" is NOT here - }, - }, +// TestComponentExistsInStacks_ComponentNotFound tests component not found in stacks. +func TestComponentExistsInStacks_ComponentNotFound(t *testing.T) { + stacks := map[string]any{ + "stack1": map[string]interface{}{ + "components": map[string]interface{}{ + "terraform": map[string]interface{}{ + "vpc": map[string]interface{}{"var1": "value1"}, + "eks": map[string]interface{}{"var2": "value2"}, }, - "stack2": map[string]interface{}{ - "components": map[string]interface{}{ - "helmfile": map[string]interface{}{ - "nginx": map[string]interface{}{"var4": "value4"}, - }, - }, + }, + }, + "stack2": map[string]interface{}{ + "components": map[string]interface{}{ + "helmfile": map[string]interface{}{ + "nginx": map[string]interface{}{"var4": "value4"}, }, - }, nil - }) - - result := CheckComponentExists(atmosConfig, "test-component") - patches.Reset() + }, + }, + } + result := componentExistsInStacks(stacks, "test-component") assert.False(t, result) } -// Note: Success path tests (ComponentFound, ComponentFoundInSecondStack, MultipleComponentTypes, MixedValidInvalidStacks) -// are covered by TestCheckComponentExistsLogic integration test which uses actual stack configurations. -// Gomonkey mocking causes interference when running tests sequentially, so we rely on the integration test instead. - -// TestCheckComponentExists_EmptyComponentName_NilConfig tests edge case. -func TestCheckComponentExists_EmptyComponentName_NilConfig(t *testing.T) { - // Even with nil config, empty component name should return false immediately - result := CheckComponentExists(nil, "") - assert.False(t, result) +// TestComponentExistsInStacks_ComponentFound tests component found in stacks. +func TestComponentExistsInStacks_ComponentFound(t *testing.T) { + stacks := map[string]any{ + "stack1": map[string]interface{}{ + "components": map[string]interface{}{ + "terraform": map[string]interface{}{ + "vpc": map[string]interface{}{"var1": "value1"}, + "test-component": map[string]interface{}{"var2": "value2"}, + }, + }, + }, + } + result := componentExistsInStacks(stacks, "test-component") + assert.True(t, result) } -// TestCheckComponentExists_RealComponentName_NilConfig tests nil config handling. -func TestCheckComponentExists_RealComponentName_NilConfig(t *testing.T) { - // Mock ExecuteDescribeStacks to handle nil config gracefully. - patches := gomonkey.ApplyFunc(e.ExecuteDescribeStacks, - func(*schema.AtmosConfiguration, string, []string, []string, []string, bool, bool, bool, bool, []string, auth.AuthManager) (map[string]any, error) { - // ExecuteDescribeStacks might handle nil config. - return map[string]any{}, nil - }) - defer patches.Reset() +// TestComponentExistsInStacks_ComponentFoundInSecondStack tests component found in non-first stack. +func TestComponentExistsInStacks_ComponentFoundInSecondStack(t *testing.T) { + stacks := map[string]any{ + "stack1": map[string]interface{}{ + "components": map[string]interface{}{ + "terraform": map[string]interface{}{ + "vpc": map[string]interface{}{}, + }, + }, + }, + "stack2": map[string]interface{}{ + "components": map[string]interface{}{ + "helmfile": map[string]interface{}{ + "test-component": map[string]interface{}{}, + }, + }, + }, + } + result := componentExistsInStacks(stacks, "test-component") + assert.True(t, result) +} - result := CheckComponentExists(nil, "nil-config-component") - assert.False(t, result) +// TestComponentExistsInStacks_MixedValidInvalidStacks tests mixed valid and invalid stack data. +func TestComponentExistsInStacks_MixedValidInvalidStacks(t *testing.T) { + stacks := map[string]any{ + "invalid-stack": "not-a-map", + "valid-stack": map[string]interface{}{ + "components": map[string]interface{}{ + "terraform": map[string]interface{}{ + "test-component": map[string]interface{}{}, + }, + }, + }, + } + result := componentExistsInStacks(stacks, "test-component") + assert.True(t, result) } diff --git a/pkg/list/utils/utils.go b/pkg/list/utils/utils.go index eae9478347..32116d277c 100644 --- a/pkg/list/utils/utils.go +++ b/pkg/list/utils/utils.go @@ -19,13 +19,17 @@ func CheckComponentExists(atmosConfig *schema.AtmosConfiguration, componentName return false } - // Get all stacks to check for the component + // Get all stacks to check for the component. stacksMap, err := e.ExecuteDescribeStacks(atmosConfig, "", nil, nil, nil, false, false, false, false, nil, nil) if err != nil { return false } - // Process all stacks to find the component + return componentExistsInStacks(stacksMap, componentName) +} + +// componentExistsInStacks checks if a component exists in the given stacks map. +func componentExistsInStacks(stacksMap map[string]any, componentName string) bool { for _, stackData := range stacksMap { stackMap, ok := stackData.(map[string]interface{}) if !ok { diff --git a/pkg/schema/schema.go b/pkg/schema/schema.go index 7e185c25f8..6388508841 100644 --- a/pkg/schema/schema.go +++ b/pkg/schema/schema.go @@ -966,7 +966,11 @@ type ConfigAndStacksInfo struct { ComponentEnvSection AtmosSectionMapType ComponentAuthSection AtmosSectionMapType ComponentEnvList []string - ComponentBackendSection AtmosSectionMapType + // SanitizedEnv holds the sanitized process environment from auth. + // When set, subprocess execution uses this instead of re-reading os.Environ(), + // which would reintroduce problematic vars (e.g., IRSA credentials on EKS pods). + SanitizedEnv []string + ComponentBackendSection AtmosSectionMapType // AuthContext holds active authentication credentials for cloud providers. // This is the SINGLE SOURCE OF TRUTH for auth credentials. // ComponentEnvSection/ComponentEnvList are derived from this context.