diff --git a/cmd/terraform/backend/backend_commands_test.go b/cmd/terraform/backend/backend_commands_test.go index 502f2fdbe6d..04035628e97 100644 --- a/cmd/terraform/backend/backend_commands_test.go +++ b/cmd/terraform/backend/backend_commands_test.go @@ -82,7 +82,7 @@ func TestExecuteDeleteCommandWithValues(t *testing.T) { force: true, setupMocks: func(mci *MockConfigInitializer, mp *MockProvisioner) { mci.EXPECT(). - InitConfigAndAuth("vpc", "dev", ""). + InitConfigAndAuth("vpc", "dev", "", false, false). Return(&schema.AtmosConfiguration{}, nil, nil) mp.EXPECT(). DeleteBackend(gomock.Any()). @@ -98,7 +98,7 @@ func TestExecuteDeleteCommandWithValues(t *testing.T) { force: false, setupMocks: func(mci *MockConfigInitializer, mp *MockProvisioner) { mci.EXPECT(). - InitConfigAndAuth("vpc", "dev", ""). + InitConfigAndAuth("vpc", "dev", "", false, false). Return(&schema.AtmosConfiguration{}, nil, nil) mp.EXPECT(). DeleteBackend(gomock.Any()). @@ -124,7 +124,7 @@ func TestExecuteDeleteCommandWithValues(t *testing.T) { force: true, setupMocks: func(mci *MockConfigInitializer, mp *MockProvisioner) { mci.EXPECT(). - InitConfigAndAuth("vpc", "dev", ""). + InitConfigAndAuth("vpc", "dev", "", false, false). Return(nil, nil, errors.New("config init failed")) }, expectError: true, @@ -137,7 +137,7 @@ func TestExecuteDeleteCommandWithValues(t *testing.T) { force: true, setupMocks: func(mci *MockConfigInitializer, mp *MockProvisioner) { mci.EXPECT(). - InitConfigAndAuth("vpc", "dev", ""). + InitConfigAndAuth("vpc", "dev", "", false, false). Return(&schema.AtmosConfiguration{}, nil, nil) mp.EXPECT(). DeleteBackend(gomock.Any()). @@ -152,7 +152,7 @@ func TestExecuteDeleteCommandWithValues(t *testing.T) { mockConfigInit, mockProv := setupTestWithMocks(t) tt.setupMocks(mockConfigInit, mockProv) - err := executeDeleteCommandWithValues(tt.component, tt.stack, tt.identity, tt.force) + err := executeDeleteCommandWithValues(tt.component, tt.stack, tt.identity, tt.force, promptedFlags{}) if tt.expectError { assert.Error(t, err) @@ -187,7 +187,7 @@ func TestExecuteDescribeCommandWithValues(t *testing.T) { setupMocks: func(mci *MockConfigInitializer, mp *MockProvisioner) { atmosConfig := &schema.AtmosConfiguration{} mci.EXPECT(). - InitConfigAndAuth("vpc", "dev", ""). + InitConfigAndAuth("vpc", "dev", "", false, false). Return(atmosConfig, nil, nil) mp.EXPECT(). DescribeBackend(atmosConfig, "vpc", map[string]string{"format": "yaml"}). @@ -204,7 +204,7 @@ func TestExecuteDescribeCommandWithValues(t *testing.T) { setupMocks: func(mci *MockConfigInitializer, mp *MockProvisioner) { atmosConfig := &schema.AtmosConfiguration{} mci.EXPECT(). - InitConfigAndAuth("vpc", "dev", ""). + InitConfigAndAuth("vpc", "dev", "", false, false). Return(atmosConfig, nil, nil) mp.EXPECT(). DescribeBackend(atmosConfig, "vpc", map[string]string{"format": "json"}). @@ -230,7 +230,7 @@ func TestExecuteDescribeCommandWithValues(t *testing.T) { format: "yaml", setupMocks: func(mci *MockConfigInitializer, mp *MockProvisioner) { mci.EXPECT(). - InitConfigAndAuth("vpc", "dev", ""). + InitConfigAndAuth("vpc", "dev", "", false, false). Return(nil, nil, errors.New("config init failed")) }, expectError: true, @@ -244,7 +244,7 @@ func TestExecuteDescribeCommandWithValues(t *testing.T) { setupMocks: func(mci *MockConfigInitializer, mp *MockProvisioner) { atmosConfig := &schema.AtmosConfiguration{} mci.EXPECT(). - InitConfigAndAuth("vpc", "dev", ""). + InitConfigAndAuth("vpc", "dev", "", false, false). Return(atmosConfig, nil, nil) mp.EXPECT(). DescribeBackend(atmosConfig, "vpc", map[string]string{"format": "yaml"}). @@ -259,7 +259,7 @@ func TestExecuteDescribeCommandWithValues(t *testing.T) { mockConfigInit, mockProv := setupTestWithMocks(t) tt.setupMocks(mockConfigInit, mockProv) - err := executeDescribeCommandWithValues(tt.component, tt.stack, tt.identity, tt.format) + err := executeDescribeCommandWithValues(tt.component, tt.stack, tt.identity, tt.format, promptedFlags{}) if tt.expectError { assert.Error(t, err) @@ -292,7 +292,7 @@ func TestExecuteListCommandWithValues(t *testing.T) { setupMocks: func(mci *MockConfigInitializer, mp *MockProvisioner) { atmosConfig := &schema.AtmosConfiguration{} mci.EXPECT(). - InitConfigAndAuth("", "dev", ""). + InitConfigAndAuth("", "dev", "", false, false). Return(atmosConfig, nil, nil) mp.EXPECT(). ListBackends(atmosConfig, map[string]string{"format": "table"}). @@ -308,7 +308,7 @@ func TestExecuteListCommandWithValues(t *testing.T) { setupMocks: func(mci *MockConfigInitializer, mp *MockProvisioner) { atmosConfig := &schema.AtmosConfiguration{} mci.EXPECT(). - InitConfigAndAuth("", "dev", ""). + InitConfigAndAuth("", "dev", "", false, false). Return(atmosConfig, nil, nil) mp.EXPECT(). ListBackends(atmosConfig, map[string]string{"format": "json"}). @@ -332,7 +332,7 @@ func TestExecuteListCommandWithValues(t *testing.T) { format: "table", setupMocks: func(mci *MockConfigInitializer, mp *MockProvisioner) { mci.EXPECT(). - InitConfigAndAuth("", "dev", ""). + InitConfigAndAuth("", "dev", "", false, false). Return(nil, nil, errors.New("config init failed")) }, expectError: true, @@ -345,7 +345,7 @@ func TestExecuteListCommandWithValues(t *testing.T) { setupMocks: func(mci *MockConfigInitializer, mp *MockProvisioner) { atmosConfig := &schema.AtmosConfiguration{} mci.EXPECT(). - InitConfigAndAuth("", "dev", ""). + InitConfigAndAuth("", "dev", "", false, false). Return(atmosConfig, nil, nil) mp.EXPECT(). ListBackends(atmosConfig, map[string]string{"format": "table"}). @@ -360,7 +360,7 @@ func TestExecuteListCommandWithValues(t *testing.T) { mockConfigInit, mockProv := setupTestWithMocks(t) tt.setupMocks(mockConfigInit, mockProv) - err := executeListCommandWithValues(tt.stack, tt.identity, tt.format) + err := executeListCommandWithValues(tt.stack, tt.identity, tt.format, false) if tt.expectError { assert.Error(t, err) @@ -456,7 +456,7 @@ func TestExecuteProvisionCommandWithValues(t *testing.T) { identity: "", setupMocks: func(mci *MockConfigInitializer, mp *MockProvisioner) { mci.EXPECT(). - InitConfigAndAuth("vpc", "dev", ""). + InitConfigAndAuth("vpc", "dev", "", false, false). Return(&schema.AtmosConfiguration{}, nil, nil) mp.EXPECT(). CreateBackend(gomock.Any()). @@ -485,7 +485,7 @@ func TestExecuteProvisionCommandWithValues(t *testing.T) { identity: "", setupMocks: func(mci *MockConfigInitializer, mp *MockProvisioner) { mci.EXPECT(). - InitConfigAndAuth("vpc", "dev", ""). + InitConfigAndAuth("vpc", "dev", "", false, false). Return(nil, nil, errors.New("config init failed")) }, expectError: true, @@ -497,7 +497,7 @@ func TestExecuteProvisionCommandWithValues(t *testing.T) { identity: "", setupMocks: func(mci *MockConfigInitializer, mp *MockProvisioner) { mci.EXPECT(). - InitConfigAndAuth("vpc", "dev", ""). + InitConfigAndAuth("vpc", "dev", "", false, false). Return(&schema.AtmosConfiguration{}, nil, nil) mp.EXPECT(). CreateBackend(gomock.Any()). @@ -512,7 +512,7 @@ func TestExecuteProvisionCommandWithValues(t *testing.T) { identity: "aws-prod", setupMocks: func(mci *MockConfigInitializer, mp *MockProvisioner) { mci.EXPECT(). - InitConfigAndAuth("vpc", "prod", "aws-prod"). + InitConfigAndAuth("vpc", "prod", "aws-prod", false, false). Return(&schema.AtmosConfiguration{}, &schema.AuthContext{AWS: &schema.AWSAuthContext{}}, nil) mp.EXPECT(). CreateBackend(gomock.Any()). @@ -532,7 +532,7 @@ func TestExecuteProvisionCommandWithValues(t *testing.T) { mockConfigInit, mockProv := setupTestWithMocks(t) tt.setupMocks(mockConfigInit, mockProv) - err := executeProvisionCommandWithValues(tt.component, tt.stack, tt.identity) + err := executeProvisionCommandWithValues(tt.component, tt.stack, tt.identity, promptedFlags{}) if tt.expectError { assert.Error(t, err) @@ -546,6 +546,95 @@ func TestExecuteProvisionCommandWithValues(t *testing.T) { } } +// TestExecuteCommandWithValues_ThreadsPromptedFlagsToConfigInitializer is the regression test +// CodeRabbit's review requested: it proves that when a backend subcommand's component/stack +// were resolved via an interactive prompt (StandardOptions.ComponentPrompted / +// StackPrompted), the executeXCommandWithValues helpers pass those booleans through to +// ConfigInitializer.InitConfigAndAuth unchanged, rather than silently defaulting to false (which +// would make profile-fallback re-exec drop the prompted value and force a second prompt, or +// fail outright, in the re-exec'd child -- see auth.ReExecContext). +func TestExecuteCommandWithValues_ThreadsPromptedFlagsToConfigInitializer(t *testing.T) { + tests := []struct { + name string + componentPrompted bool + stackPrompted bool + invoke func(mci *MockConfigInitializer, mp *MockProvisioner) error + }{ + { + name: "provision: both prompted", + componentPrompted: true, + stackPrompted: true, + invoke: func(mci *MockConfigInitializer, mp *MockProvisioner) error { + mci.EXPECT(). + InitConfigAndAuth("vpc", "dev", "", true, true). + Return(&schema.AtmosConfiguration{}, nil, nil) + mp.EXPECT().CreateBackend(gomock.Any()).Return(nil) + return executeProvisionCommandWithValues("vpc", "dev", "", promptedFlags{Component: true, Stack: true}) + }, + }, + { + name: "provision: only stack prompted", + componentPrompted: false, + stackPrompted: true, + invoke: func(mci *MockConfigInitializer, mp *MockProvisioner) error { + mci.EXPECT(). + InitConfigAndAuth("vpc", "dev", "", false, true). + Return(&schema.AtmosConfiguration{}, nil, nil) + mp.EXPECT().CreateBackend(gomock.Any()).Return(nil) + return executeProvisionCommandWithValues("vpc", "dev", "", promptedFlags{Component: false, Stack: true}) + }, + }, + { + name: "delete: both prompted", + componentPrompted: true, + stackPrompted: true, + invoke: func(mci *MockConfigInitializer, mp *MockProvisioner) error { + mci.EXPECT(). + InitConfigAndAuth("vpc", "dev", "", true, true). + Return(&schema.AtmosConfiguration{}, nil, nil) + mp.EXPECT().DeleteBackend(gomock.Any()).Return(nil) + return executeDeleteCommandWithValues("vpc", "dev", "", true, promptedFlags{Component: true, Stack: true}) + }, + }, + { + name: "describe: both prompted", + componentPrompted: true, + stackPrompted: true, + invoke: func(mci *MockConfigInitializer, mp *MockProvisioner) error { + atmosConfig := &schema.AtmosConfiguration{} + mci.EXPECT(). + InitConfigAndAuth("vpc", "dev", "", true, true). + Return(atmosConfig, nil, nil) + mp.EXPECT().DescribeBackend(atmosConfig, "vpc", map[string]string{"format": "yaml"}).Return(nil) + return executeDescribeCommandWithValues("vpc", "dev", "", "yaml", promptedFlags{Component: true, Stack: true}) + }, + }, + { + name: "list: stack prompted, no component parameter", + componentPrompted: false, + stackPrompted: true, + invoke: func(mci *MockConfigInitializer, mp *MockProvisioner) error { + atmosConfig := &schema.AtmosConfiguration{} + mci.EXPECT(). + InitConfigAndAuth("", "dev", "", false, true). + Return(atmosConfig, nil, nil) + mp.EXPECT().ListBackends(atmosConfig, map[string]string{"format": "table"}).Return(nil) + return executeListCommandWithValues("dev", "", "table", true) + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockConfigInit, mockProv := setupTestWithMocks(t) + + err := tt.invoke(mockConfigInit, mockProv) + + assert.NoError(t, err) + }) + } +} + func TestBackendSubcommands_BindStackFlagFromCommand(t *testing.T) { tests := []struct { name string @@ -596,7 +685,7 @@ func TestBackendSubcommands_BindStackFlagFromCommand(t *testing.T) { expectedErr := errors.New("stop after stack parse") mockConfigInit.EXPECT(). - InitConfigAndAuth(tt.component, "dev", ""). + InitConfigAndAuth(tt.component, "dev", "", false, false). Return(nil, nil, expectedErr) require.NoError(t, tt.cmd.Flags().Set("stack", "dev")) @@ -675,7 +764,7 @@ func TestBackendSubcommands_StackFromViperWhenNotSetOnCLI(t *testing.T) { expectedErr := errors.New("stop after stack parse") mockConfigInit.EXPECT(). - InitConfigAndAuth(tt.component, "dev", ""). + InitConfigAndAuth(tt.component, "dev", "", false, false). Return(nil, nil, expectedErr) err := tt.cmd.RunE(tt.cmd, tt.args) diff --git a/cmd/terraform/backend/backend_create.go b/cmd/terraform/backend/backend_create.go index ecb8792031a..335d01303e2 100644 --- a/cmd/terraform/backend/backend_create.go +++ b/cmd/terraform/backend/backend_create.go @@ -37,7 +37,8 @@ var createCmd = &cobra.Command{ stack = v.GetString("stack") } identity := flags.ParseGlobalFlags(cmd, v).Identity.Value() - return executeProvisionCommandWithValues(result.Component, stack, identity) + prompted := promptedFlags{Component: result.ComponentPrompted, Stack: result.StackPrompted} + return executeProvisionCommandWithValues(result.Component, stack, identity, prompted) }, } diff --git a/cmd/terraform/backend/backend_delete.go b/cmd/terraform/backend/backend_delete.go index b4d7634f57d..91ee3007dc0 100644 --- a/cmd/terraform/backend/backend_delete.go +++ b/cmd/terraform/backend/backend_delete.go @@ -47,7 +47,8 @@ Requires the --force flag for safety. The backend must be empty stack = v.GetString("stack") } identity := flags.ParseGlobalFlags(cmd, v).Identity.Value() - return executeDeleteCommandWithValues(result.Component, stack, identity, force) + prompted := promptedFlags{Component: result.ComponentPrompted, Stack: result.StackPrompted} + return executeDeleteCommandWithValues(result.Component, stack, identity, force, prompted) }, } diff --git a/cmd/terraform/backend/backend_describe.go b/cmd/terraform/backend/backend_describe.go index f39f227d676..7223ed5b58d 100644 --- a/cmd/terraform/backend/backend_describe.go +++ b/cmd/terraform/backend/backend_describe.go @@ -41,7 +41,8 @@ This includes backend settings, variables, and metadata from the stack manifest. stack = v.GetString("stack") } identity := flags.ParseGlobalFlags(cmd, v).Identity.Value() - return executeDescribeCommandWithValues(result.Component, stack, identity, result.Format) + prompted := promptedFlags{Component: result.ComponentPrompted, Stack: result.StackPrompted} + return executeDescribeCommandWithValues(result.Component, stack, identity, result.Format, prompted) }, } diff --git a/cmd/terraform/backend/backend_helpers.go b/cmd/terraform/backend/backend_helpers.go index fea764630a6..16aa94a5eeb 100644 --- a/cmd/terraform/backend/backend_helpers.go +++ b/cmd/terraform/backend/backend_helpers.go @@ -16,7 +16,12 @@ import ( // ConfigInitializer abstracts configuration and auth initialization for testability. type ConfigInitializer interface { - InitConfigAndAuth(component, stack, identity string) (*schema.AtmosConfiguration, *schema.AuthContext, error) + // InitConfigAndAuth initializes Atmos configuration and optional authentication. + // The componentPrompted and stackPrompted parameters record whether component/stack + // were resolved via an interactive prompt (rather than supplied on the command line, + // via config, or via an environment variable) so a profile-fallback re-exec can + // re-inject prompted values into the child's argv without duplicating CLI-supplied ones. + InitConfigAndAuth(component, stack, identity string, componentPrompted, stackPrompted bool) (*schema.AtmosConfiguration, *schema.AuthContext, error) } // CreateBackendParams contains parameters for CreateBackend operation. @@ -38,6 +43,16 @@ type DeleteBackendParams struct { AuthContext *schema.AuthContext } +// promptedFlags records which of a backend command's component and stack values were +// resolved via an interactive prompt (see flags.StandardOptions.ComponentPrompted / +// StackPrompted), rather than supplied via CLI flag, positional argument, environment +// variable, or config file. Bundled into a struct (instead of two bool parameters) so the +// execute*CommandWithValues helpers below stay within the linter's function argument limit. +type promptedFlags struct { + Component bool + Stack bool +} + // Provisioner abstracts provisioning operations for testability. type Provisioner interface { CreateBackend(params *CreateBackendParams) error @@ -49,8 +64,8 @@ type Provisioner interface { // defaultConfigInitializer implements ConfigInitializer using production code. type defaultConfigInitializer struct{} -func (d *defaultConfigInitializer) InitConfigAndAuth(component, stack, identity string) (*schema.AtmosConfiguration, *schema.AuthContext, error) { - return InitConfigAndAuth(component, stack, identity) +func (d *defaultConfigInitializer) InitConfigAndAuth(component, stack, identity string, componentPrompted, stackPrompted bool) (*schema.AtmosConfiguration, *schema.AuthContext, error) { + return InitConfigAndAuth(component, stack, identity, componentPrompted, stackPrompted) } // defaultProvisioner implements Provisioner using production code. @@ -122,7 +137,11 @@ func ResetDependencies() { // Returns atmosConfig, authContext, and error. // It loads component configuration, merges component-level auth with global auth, // and creates an AuthContext that respects component's default identity settings. -func InitConfigAndAuth(component, stack, identity string) (*schema.AtmosConfiguration, *schema.AuthContext, error) { +// The componentPrompted and stackPrompted parameters record whether component/stack +// were resolved via an interactive prompt; they flow into auth.ReExecContext so a +// profile-fallback re-exec re-injects prompted values into the child's argv instead +// of losing them (or re-prompting for them) after the re-exec. +func InitConfigAndAuth(component, stack, identity string, componentPrompted, stackPrompted bool) (*schema.AtmosConfiguration, *schema.AuthContext, error) { // Load atmos configuration. atmosConfig, err := cfg.InitCliConfig(schema.ConfigAndStacksInfo{ ComponentFromArg: component, @@ -157,7 +176,12 @@ func InitConfigAndAuth(component, stack, identity string) (*schema.AtmosConfigur // live endpoint (e.g. the emulator container started for this specific stack) into // AuthContext.AWS. Without the stack, that resolution silently no-ops and callers fall back // to the standard AWS SDK credential chain instead of the emulator/local sandbox. - authManager, err := auth.CreateAndAuthenticateManagerWithAtmosConfigForStack(identity, mergedAuthConfig, cfg.IdentityFlagSelectValue, &atmosConfig, stack) + authManager, err := auth.CreateAndAuthenticateManagerWithReExecContext(identity, mergedAuthConfig, cfg.IdentityFlagSelectValue, &atmosConfig, auth.ReExecContext{ + Component: component, + ComponentPrompted: componentPrompted, + Stack: stack, + StackPrompted: stackPrompted, + }) if err != nil { return nil, nil, err } @@ -189,8 +213,9 @@ func CreateDescribeComponentFunc(authManager auth.AuthManager) func(string, stri } // executeProvisionCommandWithValues is the internal implementation that accepts already-parsed values. -// Used by commands that use StandardParser's prompting infrastructure. -func executeProvisionCommandWithValues(component, stack, identity string) error { +// Used by commands that use StandardParser's prompting infrastructure. See promptedFlags for +// what prompted records and why it's threaded through to InitConfigAndAuth. +func executeProvisionCommandWithValues(component, stack, identity string, prompted promptedFlags) error { // Validate required values. if stack == "" { return errUtils.Build(errUtils.ErrRequiredFlagNotProvided). @@ -200,7 +225,7 @@ func executeProvisionCommandWithValues(component, stack, identity string) error } // Initialize config and auth using injected dependency. - atmosConfig, authContext, err := configInit.InitConfigAndAuth(component, stack, identity) + atmosConfig, authContext, err := configInit.InitConfigAndAuth(component, stack, identity, prompted.Component, prompted.Stack) if err != nil { return err } @@ -230,8 +255,9 @@ func executeProvisionCommandWithValues(component, stack, identity string) error } // executeDeleteCommandWithValues is the internal implementation for the delete command. -// Used by commands that use StandardParser's prompting infrastructure. -func executeDeleteCommandWithValues(component, stack, identity string, force bool) error { +// Used by commands that use StandardParser's prompting infrastructure. See promptedFlags for +// what prompted records and why it's threaded through to InitConfigAndAuth. +func executeDeleteCommandWithValues(component, stack, identity string, force bool, prompted promptedFlags) error { // Validate required values. if stack == "" { return errUtils.Build(errUtils.ErrRequiredFlagNotProvided). @@ -241,7 +267,7 @@ func executeDeleteCommandWithValues(component, stack, identity string, force boo } // Initialize config and auth using injected dependency. - atmosConfig, authContext, err := configInit.InitConfigAndAuth(component, stack, identity) + atmosConfig, authContext, err := configInit.InitConfigAndAuth(component, stack, identity, prompted.Component, prompted.Stack) if err != nil { return err } @@ -261,8 +287,9 @@ func executeDeleteCommandWithValues(component, stack, identity string, force boo } // executeDescribeCommandWithValues is the internal implementation for the describe command. -// Used by commands that use StandardParser's prompting infrastructure. -func executeDescribeCommandWithValues(component, stack, identity, format string) error { +// Used by commands that use StandardParser's prompting infrastructure. See promptedFlags for +// what prompted records and why it's threaded through to InitConfigAndAuth. +func executeDescribeCommandWithValues(component, stack, identity, format string, prompted promptedFlags) error { // Validate required values. if stack == "" { return errUtils.Build(errUtils.ErrRequiredFlagNotProvided). @@ -272,7 +299,7 @@ func executeDescribeCommandWithValues(component, stack, identity, format string) } // Initialize config using injected dependency. - atmosConfig, _, err := configInit.InitConfigAndAuth(component, stack, identity) + atmosConfig, _, err := configInit.InitConfigAndAuth(component, stack, identity, prompted.Component, prompted.Stack) if err != nil { return err } @@ -282,8 +309,12 @@ func executeDescribeCommandWithValues(component, stack, identity, format string) } // executeListCommandWithValues is the internal implementation for the list command. -// Used by commands that use StandardParser's prompting infrastructure. -func executeListCommandWithValues(stack, identity, format string) error { +// Used by commands that use StandardParser's prompting infrastructure. The stackPrompted +// parameter records whether stack was filled in via an interactive prompt (see +// flags.StandardOptions.StackPrompted), so it can be threaded into auth.ReExecContext for +// profile-fallback re-exec. There's no component parameter here (list operates across all +// components in the stack), so componentPrompted is always false when calling InitConfigAndAuth. +func executeListCommandWithValues(stack, identity, format string, stackPrompted bool) error { // Validate required values. if stack == "" { return errUtils.Build(errUtils.ErrRequiredFlagNotProvided). @@ -293,7 +324,7 @@ func executeListCommandWithValues(stack, identity, format string) error { } // Initialize config using injected dependency (no component needed for list). - atmosConfig, _, err := configInit.InitConfigAndAuth("", stack, identity) + atmosConfig, _, err := configInit.InitConfigAndAuth("", stack, identity, false, stackPrompted) if err != nil { return err } diff --git a/cmd/terraform/backend/backend_helpers_test.go b/cmd/terraform/backend/backend_helpers_test.go index 9bb4b921fec..e3bb5fb08d8 100644 --- a/cmd/terraform/backend/backend_helpers_test.go +++ b/cmd/terraform/backend/backend_helpers_test.go @@ -39,7 +39,7 @@ func TestInitConfigAndAuth_FailsFastWithoutRealConfig(t *testing.T) { // InitCliConfig -> ExecuteDescribeComponent wiring (rather than mocking it away). t.Chdir(t.TempDir()) - atmosConfig, authContext, err := InitConfigAndAuth("nonexistent-component", "nonexistent-stack", "") + atmosConfig, authContext, err := InitConfigAndAuth("nonexistent-component", "nonexistent-stack", "", false, false) assert.Error(t, err) assert.Nil(t, atmosConfig) @@ -50,7 +50,7 @@ func TestDefaultConfigInitializer_InitConfigAndAuth(t *testing.T) { t.Chdir(t.TempDir()) ci := &defaultConfigInitializer{} - atmosConfig, authContext, err := ci.InitConfigAndAuth("nonexistent-component", "nonexistent-stack", "") + atmosConfig, authContext, err := ci.InitConfigAndAuth("nonexistent-component", "nonexistent-stack", "", false, false) assert.Error(t, err) assert.Nil(t, atmosConfig) @@ -66,7 +66,29 @@ func TestInitConfigAndAuth_SucceedsWithNoAuthConfigured(t *testing.T) { t.Chdir(filepath.Join("..", "..", "..", "tests", "fixtures", "scenarios", "atmos-overrides-section")) t.Setenv("ATMOS_CLI_CONFIG_PATH", ".") - atmosConfig, authContext, err := InitConfigAndAuth("c1", "dev", "") + atmosConfig, authContext, err := InitConfigAndAuth("c1", "dev", "", false, false) + + require.NoError(t, err) + require.NotNil(t, atmosConfig) + assert.Nil(t, authContext) +} + +// TestInitConfigAndAuth_SucceedsWithNoAuthConfigured_PromptedValues is the regression test +// CodeRabbit's review requested: it proves componentPrompted/stackPrompted=true flow all the +// way through the real (unmocked) InitConfigAndAuth body -- InitCliConfig -> +// ExecuteDescribeComponent -> MergeComponentAuthFromConfig -> the ReExecContext-aware auth +// call -- without changing behavior (same fixture, same no-identities-configured outcome as +// TestInitConfigAndAuth_SucceedsWithNoAuthConfigured above). +// The auth.CreateAndAuthenticateManagerWithReExecContext call is a black box from here +// (pkg/auth is out of scope for this change), so this test can't directly inspect the +// ReExecContext it builds. See TestExecuteCommandWithValues_ThreadsPromptedFlagsToConfigInitializer +// in backend_commands_test.go for a mock-based assertion that the boolean values themselves +// are threaded through correctly. +func TestInitConfigAndAuth_SucceedsWithNoAuthConfigured_PromptedValues(t *testing.T) { + t.Chdir(filepath.Join("..", "..", "..", "tests", "fixtures", "scenarios", "atmos-overrides-section")) + t.Setenv("ATMOS_CLI_CONFIG_PATH", ".") + + atmosConfig, authContext, err := InitConfigAndAuth("c1", "dev", "", true, true) require.NoError(t, err) require.NotNil(t, atmosConfig) @@ -92,7 +114,7 @@ auth: require.NoError(t, os.WriteFile("atmos.yaml", config, 0o600)) t.Setenv("ATMOS_CLI_CONFIG_PATH", ".") - atmosConfig, authContext, err := InitConfigAndAuth("c1", "dev", "nonexistent-identity") + atmosConfig, authContext, err := InitConfigAndAuth("c1", "dev", "nonexistent-identity", false, false) require.Error(t, err) assert.ErrorIs(t, err, errUtils.ErrIdentityNotFound) diff --git a/cmd/terraform/backend/backend_list.go b/cmd/terraform/backend/backend_list.go index 5a31ac5fcff..73331039a41 100644 --- a/cmd/terraform/backend/backend_list.go +++ b/cmd/terraform/backend/backend_list.go @@ -37,7 +37,7 @@ var listCmd = &cobra.Command{ stack = v.GetString("stack") } identity := flags.ParseGlobalFlags(cmd, v).Identity.Value() - return executeListCommandWithValues(stack, identity, result.Format) + return executeListCommandWithValues(stack, identity, result.Format, result.StackPrompted) }, } diff --git a/cmd/terraform/backend/backend_update.go b/cmd/terraform/backend/backend_update.go index 3774f275283..b8287c89c5d 100644 --- a/cmd/terraform/backend/backend_update.go +++ b/cmd/terraform/backend/backend_update.go @@ -40,7 +40,8 @@ versioning, encryption, and public access blocking to match secure defaults.`, stack = v.GetString("stack") } identity := flags.ParseGlobalFlags(cmd, v).Identity.Value() - return executeProvisionCommandWithValues(result.Component, stack, identity) + prompted := promptedFlags{Component: result.ComponentPrompted, Stack: result.StackPrompted} + return executeProvisionCommandWithValues(result.Component, stack, identity, prompted) }, } diff --git a/cmd/terraform/backend/mock_backend_helpers_test.go b/cmd/terraform/backend/mock_backend_helpers_test.go index 0be071d76cf..6457074d6bd 100644 --- a/cmd/terraform/backend/mock_backend_helpers_test.go +++ b/cmd/terraform/backend/mock_backend_helpers_test.go @@ -41,9 +41,9 @@ func (m *MockConfigInitializer) EXPECT() *MockConfigInitializerMockRecorder { } // InitConfigAndAuth mocks base method. -func (m *MockConfigInitializer) InitConfigAndAuth(component, stack, identity string) (*schema.AtmosConfiguration, *schema.AuthContext, error) { +func (m *MockConfigInitializer) InitConfigAndAuth(component, stack, identity string, componentPrompted, stackPrompted bool) (*schema.AtmosConfiguration, *schema.AuthContext, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "InitConfigAndAuth", component, stack, identity) + ret := m.ctrl.Call(m, "InitConfigAndAuth", component, stack, identity, componentPrompted, stackPrompted) ret0, _ := ret[0].(*schema.AtmosConfiguration) ret1, _ := ret[1].(*schema.AuthContext) ret2, _ := ret[2].(error) @@ -51,9 +51,9 @@ func (m *MockConfigInitializer) InitConfigAndAuth(component, stack, identity str } // InitConfigAndAuth indicates an expected call of InitConfigAndAuth. -func (mr *MockConfigInitializerMockRecorder) InitConfigAndAuth(component, stack, identity any) *gomock.Call { +func (mr *MockConfigInitializerMockRecorder) InitConfigAndAuth(component, stack, identity, componentPrompted, stackPrompted any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InitConfigAndAuth", reflect.TypeOf((*MockConfigInitializer)(nil).InitConfigAndAuth), component, stack, identity) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InitConfigAndAuth", reflect.TypeOf((*MockConfigInitializer)(nil).InitConfigAndAuth), component, stack, identity, componentPrompted, stackPrompted) } // MockProvisioner is a mock of Provisioner interface. diff --git a/cmd/terraform/shared/execution.go b/cmd/terraform/shared/execution.go index 2c5268780c7..56abe223e40 100644 --- a/cmd/terraform/shared/execution.go +++ b/cmd/terraform/shared/execution.go @@ -167,6 +167,7 @@ func promptMissingComponent(info *schema.ConfigAndStacksInfo, cmd *cobra.Command return err } info.ComponentFromArg = component + info.ComponentPrompted = component != "" return nil } @@ -179,6 +180,7 @@ func promptMissingStack(info *schema.ConfigAndStacksInfo, cmd *cobra.Command) er return err } info.Stack = stack + info.StackPrompted = stack != "" return nil } diff --git a/cmd/terraform/shared/execution_coverage_test.go b/cmd/terraform/shared/execution_coverage_test.go index bb639b7dcd2..dc88bb5a3fa 100644 --- a/cmd/terraform/shared/execution_coverage_test.go +++ b/cmd/terraform/shared/execution_coverage_test.go @@ -449,9 +449,11 @@ func TestPromptHelpersReturnEmptyWhenNonInteractive(t *testing.T) { info := &schema.ConfigAndStacksInfo{} require.NoError(t, promptMissingComponent(info, &cobra.Command{})) assert.Empty(t, info.ComponentFromArg) + assert.False(t, info.ComponentPrompted) require.NoError(t, promptMissingStack(info, &cobra.Command{})) assert.Empty(t, info.Stack) + assert.False(t, info.StackPrompted) } func TestHandleInteractiveIdentitySelectionInitCliConfigError(t *testing.T) { @@ -553,6 +555,62 @@ func TestPromptMissingComponent_AlreadySetShortCircuits(t *testing.T) { assert.Equal(t, "vpc", info.ComponentFromArg, "must not be overwritten") } +// TestPromptMissingComponentAndStack_SetsPromptedFlags verifies that +// promptMissingComponent/promptMissingStack record ComponentPrompted/ +// StackPrompted only when the interactive picker actually ran and returned a +// value — this is what lets a later profile-fallback re-exec (see +// pkg/auth.ReExecContext) tell a prompted value apart from one the user +// already supplied on the command line. +func TestPromptMissingComponentAndStack_SetsPromptedFlags(t *testing.T) { + origInteractive := isInteractiveFn + origSelect := selectFromOptions + origDescribe := executeDescribeStacks + origInit := initCliConfig + t.Cleanup(func() { + isInteractiveFn = origInteractive + selectFromOptions = origSelect + executeDescribeStacks = origDescribe + initCliConfig = origInit + }) + + initCliConfig = func(_ schema.ConfigAndStacksInfo, _ bool) (schema.AtmosConfiguration, error) { + return schema.AtmosConfiguration{}, nil + } + isInteractiveFn = func() bool { return true } + executeDescribeStacks = func(_ *schema.AtmosConfiguration, _ string, _, _, _ []string, _, _, _, _ bool, _ []string, _ auth.AuthManager) (map[string]any, error) { + return map[string]any{ + "core-ue2-auto": map[string]any{ + "components": map[string]any{ + "terraform": map[string]any{"vpc": map[string]any{}}, + }, + }, + }, nil + } + selectFromOptions = func(name, _ string, options []string) (string, error) { + return options[0], nil + } + + info := &schema.ConfigAndStacksInfo{} + require.NoError(t, promptMissingComponent(info, &cobra.Command{Use: "plan"})) + assert.Equal(t, "vpc", info.ComponentFromArg) + assert.True(t, info.ComponentPrompted, "component resolved via prompt must be flagged as prompted") + + cmd := &cobra.Command{Use: "plan"} + cmd.Flags().String("stack", "", "Stack flag") + require.NoError(t, promptMissingStack(info, cmd)) + assert.Equal(t, "core-ue2-auto", info.Stack) + assert.True(t, info.StackPrompted, "stack resolved via prompt must be flagged as prompted") + + // A component/stack already supplied on the command line must never be + // marked as prompted, since promptMissing* short-circuits before calling + // the picker. + preSupplied := &schema.ConfigAndStacksInfo{ComponentFromArg: "eks", Stack: "core-ue2-corp"} + require.NoError(t, promptMissingComponent(preSupplied, &cobra.Command{Use: "plan"})) + require.NoError(t, promptMissingStack(preSupplied, &cobra.Command{Use: "plan"})) + assert.False(t, preSupplied.ComponentPrompted, "command-line-supplied component must not be flagged as prompted") + assert.False(t, preSupplied.StackPrompted, "command-line-supplied stack must not be flagged as prompted") +} + func TestHandleInteractiveComponentStackSelectionPropagatesComponentPromptError(t *testing.T) { // Force interactive mode and make the component picker's underlying stack // enumeration fail, so PromptForComponent surfaces ErrLoadSelectionOptions diff --git a/internal/exec/packer_auth_test.go b/internal/exec/packer_auth_test.go index 37790e12b94..a79325cf57a 100644 --- a/internal/exec/packer_auth_test.go +++ b/internal/exec/packer_auth_test.go @@ -30,7 +30,7 @@ func stubPackerAuthSeams(t *testing.T, manager auth.AuthManager) { defaultMergedAuthConfigGetter = func(*schema.AtmosConfiguration, *schema.ConfigAndStacksInfo) (*schema.AuthConfig, error) { return &schema.AuthConfig{}, nil } - defaultAuthManagerCreator = func(identity string, authConfig *schema.AuthConfig, selectValue string, atmosConfig *schema.AtmosConfiguration, stack string) (auth.AuthManager, error) { + defaultAuthManagerCreator = func(identity string, authConfig *schema.AuthConfig, selectValue string, atmosConfig *schema.AtmosConfiguration, reExecCtx auth.ReExecContext) (auth.AuthManager, error) { return manager, nil } } diff --git a/internal/exec/terraform_execute_helpers.go b/internal/exec/terraform_execute_helpers.go index 604207e8d54..c69b2283f0e 100644 --- a/internal/exec/terraform_execute_helpers.go +++ b/internal/exec/terraform_execute_helpers.go @@ -111,13 +111,20 @@ func setupTerraformAuth(atmosConfig *schema.AtmosConfiguration, info *schema.Con // Wrap unexpected errors (e.g. MergeComponentAuthFromConfig failures) with the sentinel // to match the behaviour of createAndAuthenticateAuthManagerWithDeps. When the error names // a missing identity, offer the same profile-selection prompt `atmos auth login` has. - return nil, resolveIdentityConfigError(atmosConfig, err, errUtils.ErrInvalidAuthConfig) + return nil, resolveIdentityConfigError(atmosConfig, info, err, errUtils.ErrInvalidAuthConfig) } // Create and authenticate the AuthManager using the same injectable creator as - // createAndAuthenticateAuthManagerWithDeps to keep injection points unified. + // createAndAuthenticateAuthManagerWithDeps to keep injection points unified. Carry + // forward prompted component/stack so a later identity-not-found fallback inside + // Authenticate can re-inject them into a profile-fallback re-exec. authManager, err := defaultAuthManagerCreator( - info.Identity, mergedAuthConfig, cfg.IdentityFlagSelectValue, atmosConfig, info.Stack, + info.Identity, mergedAuthConfig, cfg.IdentityFlagSelectValue, atmosConfig, auth.ReExecContext{ + Component: info.ComponentFromArg, + ComponentPrompted: info.ComponentPrompted, + Stack: info.Stack, + StackPrompted: info.StackPrompted, + }, ) if err != nil { if errors.Is(err, errUtils.ErrUserAborted) { @@ -126,7 +133,7 @@ func setupTerraformAuth(atmosConfig *schema.AtmosConfiguration, info *schema.Con // Wrap auth creation failures with the sentinel to match createAndAuthenticateAuthManagerWithDeps. // When the error names a missing identity, offer the same profile-selection prompt // `atmos auth login` has. - return nil, resolveIdentityConfigError(atmosConfig, err, errUtils.ErrFailedToInitializeAuthManager) + return nil, resolveIdentityConfigError(atmosConfig, info, err, errUtils.ErrFailedToInitializeAuthManager) } // Store manager for nested YAML functions (e.g. !terraform.state). diff --git a/internal/exec/terraform_execute_helpers_auth_test.go b/internal/exec/terraform_execute_helpers_auth_test.go index 424711b7eb4..83d893728a9 100644 --- a/internal/exec/terraform_execute_helpers_auth_test.go +++ b/internal/exec/terraform_execute_helpers_auth_test.go @@ -72,7 +72,7 @@ func TestSetupTerraformAuth_ErrInvalidComponent(t *testing.T) { func TestSetupTerraformAuth_AuthCreatorError_WrapsWithSentinel(t *testing.T) { orig := defaultAuthManagerCreator t.Cleanup(func() { defaultAuthManagerCreator = orig }) - defaultAuthManagerCreator = func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ string) (auth.AuthManager, error) { + defaultAuthManagerCreator = func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ auth.ReExecContext) (auth.AuthManager, error) { return nil, errors.New("auth backend unavailable") } @@ -99,7 +99,7 @@ func TestSetupTerraformAuth_IdentityStoredAndManagerSet(t *testing.T) { orig := defaultAuthManagerCreator t.Cleanup(func() { defaultAuthManagerCreator = orig }) - defaultAuthManagerCreator = func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ string) (auth.AuthManager, error) { + defaultAuthManagerCreator = func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ auth.ReExecContext) (auth.AuthManager, error) { return mockMgr, nil } @@ -120,7 +120,7 @@ func TestSetupTerraformAuth_IdentityStoredAndManagerSet(t *testing.T) { func TestSetupTerraformAuth_NilManager_NoAuthBridge(t *testing.T) { orig := defaultAuthManagerCreator t.Cleanup(func() { defaultAuthManagerCreator = orig }) - defaultAuthManagerCreator = func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ string) (auth.AuthManager, error) { + defaultAuthManagerCreator = func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ auth.ReExecContext) (auth.AuthManager, error) { return nil, nil } @@ -205,7 +205,7 @@ func TestSetupTerraformAuth_IdentityFlagPropagatesToAuthCreator(t *testing.T) { var capturedIdentity string origCreator := defaultAuthManagerCreator t.Cleanup(func() { defaultAuthManagerCreator = origCreator }) - defaultAuthManagerCreator = func(identity string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ string) (auth.AuthManager, error) { + defaultAuthManagerCreator = func(identity string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ auth.ReExecContext) (auth.AuthManager, error) { capturedIdentity = identity // Return nil manager so we don't trip into authenticateWithIdentity logic. return nil, nil @@ -248,7 +248,7 @@ func TestSetupTerraformAuth_EmptyIdentity_AllowsAutoDetection(t *testing.T) { var capturedIdentity string origCreator := defaultAuthManagerCreator t.Cleanup(func() { defaultAuthManagerCreator = origCreator }) - defaultAuthManagerCreator = func(identity string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ string) (auth.AuthManager, error) { + defaultAuthManagerCreator = func(identity string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ auth.ReExecContext) (auth.AuthManager, error) { capturedIdentity = identity return nil, nil } diff --git a/internal/exec/terraform_nested_auth_helper.go b/internal/exec/terraform_nested_auth_helper.go index 067c4cb4e38..e2b238be31b 100644 --- a/internal/exec/terraform_nested_auth_helper.go +++ b/internal/exec/terraform_nested_auth_helper.go @@ -270,12 +270,15 @@ func createComponentAuthManager( // Use the stack-aware variant so the target component's stack is threaded into manager // construction: stack-scoped identities (e.g. kind: /emulator) need it to resolve // their endpoint and populate the in-process auth context read by `!terraform.state`. + // Target component's stack, for stack-scoped (emulator) identities. This is the nested + // component's own target, not the top-level prompted component/stack, so prompted flags + // are intentionally not threaded here (uses the stable stack-string signature). componentAuthManager, err := auth.CreateAndAuthenticateManagerWithAtmosConfigForStack( identityName, // Inherited from parent, or empty to trigger auto-detection mergedAuthConfig, // Merged component + global auth cfg.IdentityFlagSelectValue, atmosConfig, // Enable stack-level auth default loading - stack, // Target component's stack, for stack-scoped (emulator) identities + stack, ) if err != nil { log.Debug( diff --git a/internal/exec/utils_auth.go b/internal/exec/utils_auth.go index bd731878156..9e6db988e6e 100644 --- a/internal/exec/utils_auth.go +++ b/internal/exec/utils_auth.go @@ -29,17 +29,19 @@ import ( type componentConfigFetcher func(params *ExecuteDescribeComponentParams) (map[string]any, error) // authManagerCreator is a function type for creating and authenticating an AuthManager. -// The trailing stack is threaded into manager construction so stack-scoped identities -// (e.g. kind: /emulator) receive it before authentication and can populate the -// in-process auth context. This allows dependency injection for testing. -type authManagerCreator func(identity string, authConfig *schema.AuthConfig, selectValue string, atmosConfig *schema.AtmosConfiguration, stack string) (auth.AuthManager, error) +// The trailing ReExecContext is threaded into manager construction so stack-scoped +// identities (e.g. kind: /emulator) receive the target stack before authentication +// and can populate the in-process auth context, and so a later identity-not-found fallback +// can re-inject prompted component/stack values into a profile-fallback re-exec. This allows +// dependency injection for testing. +type authManagerCreator func(identity string, authConfig *schema.AuthConfig, selectValue string, atmosConfig *schema.AtmosConfiguration, reExecCtx auth.ReExecContext) (auth.AuthManager, error) // defaultComponentConfigFetcher is the default implementation that calls ExecuteDescribeComponent. var defaultComponentConfigFetcher componentConfigFetcher = ExecuteDescribeComponent -// defaultAuthManagerCreator is the default implementation that calls the stack-aware -// auth.CreateAndAuthenticateManagerWithAtmosConfigForStack. -var defaultAuthManagerCreator authManagerCreator = auth.CreateAndAuthenticateManagerWithAtmosConfigForStack +// defaultAuthManagerCreator is the default implementation that calls the ReExecContext-aware +// auth.CreateAndAuthenticateManagerWithReExecContext. +var defaultAuthManagerCreator authManagerCreator = auth.CreateAndAuthenticateManagerWithReExecContext // resolveIdentityConfigError checks whether err signals a missing/invalid identity that a // profile might resolve, offering the interactive profile-selection prompt `atmos auth @@ -47,8 +49,8 @@ var defaultAuthManagerCreator authManagerCreator = auth.CreateAndAuthenticateMan // non-nil error: either the fallback's own outcome (a successful re-exec never returns; // otherwise a hint-enriched error or ErrUserAborted) or err wrapped with wrapSentinel when // no fallback applies. -func resolveIdentityConfigError(atmosConfig *schema.AtmosConfiguration, err error, wrapSentinel error) error { - if fbErr := offerIdentityProfileFallback(atmosConfig, err); fbErr != nil { +func resolveIdentityConfigError(atmosConfig *schema.AtmosConfiguration, info *schema.ConfigAndStacksInfo, err error, wrapSentinel error) error { + if fbErr := offerIdentityProfileFallback(atmosConfig, info, err); fbErr != nil { return fbErr } return fmt.Errorf("%w: %w", wrapSentinel, err) @@ -59,7 +61,7 @@ func resolveIdentityConfigError(atmosConfig *schema.AtmosConfiguration, err erro // caller should proceed with its own wrap of err). A user-aborted fallback exits the // process with ExitCodeSIGINT rather than returning, matching the existing abort-handling // convention for identity-selection prompts. -func offerIdentityProfileFallback(atmosConfig *schema.AtmosConfiguration, err error) error { +func offerIdentityProfileFallback(atmosConfig *schema.AtmosConfiguration, info *schema.ConfigAndStacksInfo, err error) error { if !errors.Is(err, errUtils.ErrInvalidIdentityConfig) { return nil } @@ -67,7 +69,13 @@ func offerIdentityProfileFallback(atmosConfig *schema.AtmosConfiguration, err er if !ok || identityName == "" { return nil } - fbErr := auth.MaybeOfferProfileFallbackForIdentity(context.Background(), atmosConfig.CliConfigPath, identityName) + reExecCtx := auth.ReExecContext{ + Component: info.ComponentFromArg, + ComponentPrompted: info.ComponentPrompted, + Stack: info.Stack, + StackPrompted: info.StackPrompted, + } + fbErr := auth.MaybeOfferProfileFallbackForIdentity(context.Background(), atmosConfig.CliConfigPath, identityName, reExecCtx) if fbErr == nil { return nil } @@ -102,15 +110,22 @@ func createAndAuthenticateAuthManagerWithDeps( if errors.Is(err, errUtils.ErrInvalidComponent) { return nil, err } - return nil, resolveIdentityConfigError(atmosConfig, err, errUtils.ErrInvalidAuthConfig) + return nil, resolveIdentityConfigError(atmosConfig, info, err, errUtils.ErrInvalidAuthConfig) } // Create and authenticate AuthManager from --identity flag if specified. // Uses merged auth config that includes both global and component-specific identities/defaults. // This enables YAML template functions like !terraform.state to use authenticated credentials. - authManager, err := authCreator(info.Identity, mergedAuthConfig, cfg.IdentityFlagSelectValue, atmosConfig, info.Stack) + // Carry forward prompted component/stack so a later identity-not-found fallback inside + // Authenticate can re-inject them into a profile-fallback re-exec instead of dropping them. + authManager, err := authCreator(info.Identity, mergedAuthConfig, cfg.IdentityFlagSelectValue, atmosConfig, auth.ReExecContext{ + Component: info.ComponentFromArg, + ComponentPrompted: info.ComponentPrompted, + Stack: info.Stack, + StackPrompted: info.StackPrompted, + }) if err != nil { - return nil, resolveIdentityConfigError(atmosConfig, err, errUtils.ErrFailedToInitializeAuthManager) + return nil, resolveIdentityConfigError(atmosConfig, info, err, errUtils.ErrFailedToInitializeAuthManager) } // If AuthManager was created and identity was auto-detected (info.Identity was empty), diff --git a/internal/exec/utils_auth_test.go b/internal/exec/utils_auth_test.go index de617d6f9cd..e8a0d50bbb9 100644 --- a/internal/exec/utils_auth_test.go +++ b/internal/exec/utils_auth_test.go @@ -918,7 +918,7 @@ func TestCreateAndAuthenticateAuthManagerWithDeps_Success(t *testing.T) { } // Mock auth creator returns a mock manager. - mockCreator := func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ string) (auth.AuthManager, error) { + mockCreator := func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ auth.ReExecContext) (auth.AuthManager, error) { return mockManager, nil } @@ -929,6 +929,46 @@ func TestCreateAndAuthenticateAuthManagerWithDeps_Success(t *testing.T) { assert.Equal(t, "detected-identity", info.Identity) } +// TestCreateAndAuthenticateAuthManagerWithDeps_PassesPromptedContextToCreator verifies the +// authCreator is invoked with a ReExecContext carrying info's prompted component/stack, not an +// empty one. This is what lets manager.Authenticate (which reads the resulting manager's own +// stackInfo) re-inject prompted values into a later identity-not-found profile-fallback re-exec. +func TestCreateAndAuthenticateAuthManagerWithDeps_PassesPromptedContextToCreator(t *testing.T) { + ctrl := gomock.NewController(t) + + atmosConfig := &schema.AtmosConfiguration{} + info := &schema.ConfigAndStacksInfo{ + Stack: "core-ue2-auto", + StackPrompted: true, + ComponentFromArg: "vpc", + ComponentPrompted: true, + Identity: "", + } + + mockManager := mockTypes.NewMockAuthManager(ctrl) + mockManager.EXPECT().GetChain().Return([]string{"detected-identity"}) + + mockFetcher := func(_ *ExecuteDescribeComponentParams) (map[string]any, error) { + return nil, nil + } + + var gotReExecCtx auth.ReExecContext + mockCreator := func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, reExecCtx auth.ReExecContext) (auth.AuthManager, error) { + gotReExecCtx = reExecCtx + return mockManager, nil + } + + _, err := createAndAuthenticateAuthManagerWithDeps(atmosConfig, info, mockFetcher, mockCreator) + require.NoError(t, err) + + assert.Equal(t, auth.ReExecContext{ + Component: "vpc", + ComponentPrompted: true, + Stack: "core-ue2-auto", + StackPrompted: true, + }, gotReExecCtx, "authCreator must receive info's prompted component/stack, not an empty context") +} + func TestCreateAndAuthenticateAuthManagerWithDeps_InvalidComponentError(t *testing.T) { atmosConfig := &schema.AtmosConfiguration{} info := &schema.ConfigAndStacksInfo{ @@ -942,7 +982,7 @@ func TestCreateAndAuthenticateAuthManagerWithDeps_InvalidComponentError(t *testi } // Mock auth creator - should not be called. - mockCreator := func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ string) (auth.AuthManager, error) { + mockCreator := func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ auth.ReExecContext) (auth.AuthManager, error) { t.Fatal("auth creator should not be called when component is invalid") return nil, nil } @@ -966,7 +1006,7 @@ func TestCreateAndAuthenticateAuthManagerWithDeps_AuthCreatorError(t *testing.T) } // Mock auth creator returns an error. - mockCreator := func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ string) (auth.AuthManager, error) { + mockCreator := func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ auth.ReExecContext) (auth.AuthManager, error) { return nil, errors.New("auth failed") } @@ -995,7 +1035,7 @@ func TestCreateAndAuthenticateAuthManagerWithDeps_OtherMergeError(t *testing.T) } // Mock auth creator should still be called because getMergedAuthConfigWithFetcher handles errors gracefully. - mockCreator := func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ string) (auth.AuthManager, error) { + mockCreator := func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ auth.ReExecContext) (auth.AuthManager, error) { return nil, nil } @@ -1017,7 +1057,7 @@ func TestCreateAndAuthenticateAuthManagerWithDeps_NilAuthManager(t *testing.T) { } // Mock auth creator returns nil (no auth configured). - mockCreator := func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ string) (auth.AuthManager, error) { + mockCreator := func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ auth.ReExecContext) (auth.AuthManager, error) { return nil, nil } @@ -1213,7 +1253,7 @@ func TestCreateAndAuthenticateAuthManagerWithDeps_PreservesExistingIdentity(t *t return nil, nil } - mockCreator := func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ string) (auth.AuthManager, error) { + mockCreator := func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ auth.ReExecContext) (auth.AuthManager, error) { return mockManager, nil } @@ -1233,7 +1273,7 @@ func TestResolveIdentityConfigError_NonIdentityConfigErrorWrapsUnchanged(t *test tmpDir := setupExecProfileFallbackFixture(t) atmosConfig := &schema.AtmosConfiguration{CliConfigPath: tmpDir} - err := resolveIdentityConfigError(atmosConfig, errors.New("boom"), errUtils.ErrInvalidAuthConfig) + err := resolveIdentityConfigError(atmosConfig, &schema.ConfigAndStacksInfo{}, errors.New("boom"), errUtils.ErrInvalidAuthConfig) require.Error(t, err) assert.True(t, errors.Is(err, errUtils.ErrInvalidAuthConfig)) } @@ -1242,7 +1282,7 @@ func TestResolveIdentityConfigError_NoIdentityContextWrapsUnchanged(t *testing.T tmpDir := setupExecProfileFallbackFixture(t) atmosConfig := &schema.AtmosConfiguration{CliConfigPath: tmpDir} - err := resolveIdentityConfigError(atmosConfig, errUtils.ErrInvalidIdentityConfig, errUtils.ErrInvalidAuthConfig) + err := resolveIdentityConfigError(atmosConfig, &schema.ConfigAndStacksInfo{}, errUtils.ErrInvalidIdentityConfig, errUtils.ErrInvalidAuthConfig) require.Error(t, err) assert.True(t, errors.Is(err, errUtils.ErrInvalidAuthConfig)) } @@ -1255,7 +1295,7 @@ func TestResolveIdentityConfigError_NoCandidateProfileWrapsUnchanged(t *testing. WithContext("identity", "totally-unknown-identity"). Err() - err := resolveIdentityConfigError(atmosConfig, tagged, errUtils.ErrInvalidAuthConfig) + err := resolveIdentityConfigError(atmosConfig, &schema.ConfigAndStacksInfo{}, tagged, errUtils.ErrInvalidAuthConfig) require.Error(t, err) assert.True(t, errors.Is(err, errUtils.ErrInvalidAuthConfig), "no profile defines the identity, so the original wrap must be preserved") @@ -1269,7 +1309,7 @@ func TestResolveIdentityConfigError_CandidateProfileOffersFallback(t *testing.T) WithContext("identity", "root-admin"). Err() - err := resolveIdentityConfigError(atmosConfig, tagged, errUtils.ErrInvalidAuthConfig) + err := resolveIdentityConfigError(atmosConfig, &schema.ConfigAndStacksInfo{}, tagged, errUtils.ErrInvalidAuthConfig) require.Error(t, err) // Non-interactive test environment: the flat wrap is replaced by the fallback's // hint-enriched ErrIdentityNotFound naming the "alpha" profile. @@ -1296,7 +1336,7 @@ func TestCreateAndAuthenticateAuthManagerWithDeps_AuthCreatorError_IdentityConfi mockFetcher := func(_ *ExecuteDescribeComponentParams) (map[string]any, error) { return nil, nil } - mockCreator := func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ string) (auth.AuthManager, error) { + mockCreator := func(_ string, _ *schema.AuthConfig, _ string, _ *schema.AtmosConfiguration, _ auth.ReExecContext) (auth.AuthManager, error) { return nil, errUtils.Build(errUtils.ErrInvalidIdentityConfig). WithContext("identity", "root-admin"). Err() diff --git a/pkg/auth/manager.go b/pkg/auth/manager.go index 075d24891cf..39d829747e7 100644 --- a/pkg/auth/manager.go +++ b/pkg/auth/manager.go @@ -269,7 +269,20 @@ func (m *manager) Authenticate(ctx context.Context, identityName string) (*types // surface a hint naming the profile (non-interactive). Explicit // --profile / ATMOS_PROFILE selections are never overridden. // See PRD: interactive-profile-suggestion. - if fbErr := m.maybeOfferProfileFallback(ctx, identityName); fbErr != nil { + // + // Carry forward any component/stack values already resolved via an + // interactive prompt on this process's stackInfo, so the re-exec'd + // child doesn't re-prompt for values the user just supplied. + reExecCtx := ReExecContext{} + if m.stackInfo != nil { + reExecCtx = ReExecContext{ + Component: m.stackInfo.ComponentFromArg, + ComponentPrompted: m.stackInfo.ComponentPrompted, + Stack: m.stackInfo.Stack, + StackPrompted: m.stackInfo.StackPrompted, + } + } + if fbErr := m.maybeOfferProfileFallback(ctx, identityName, reExecCtx); fbErr != nil { return nil, fbErr } // Return a single rich error carrying the explanation and hint. diff --git a/pkg/auth/manager_helpers.go b/pkg/auth/manager_helpers.go index 988d2aed9e5..076c6532a2c 100644 --- a/pkg/auth/manager_helpers.go +++ b/pkg/auth/manager_helpers.go @@ -105,14 +105,20 @@ func resolveIdentityName(identityName string, authConfig *schema.AuthConfig, cli // createAuthManagerInstance creates a new AuthManager instance with the given configuration. // Note: This function is used internally for temporary managers during identity resolution. // For persistent auth managers, use NewAuthManager directly with the proper cliConfigPath. -func createAuthManagerInstance(authConfig *schema.AuthConfig, cliConfigPath string, stack string) (AuthManager, error) { +func createAuthManagerInstance(authConfig *schema.AuthConfig, cliConfigPath string, reExecCtx ReExecContext) (AuthManager, error) { authStackInfo := &schema.ConfigAndStacksInfo{ // Seed the target stack so stack-scoped identities (e.g. kind: /emulator) // receive it via SetStack at construction, before Authenticate/PostAuthenticate run. // Without this the emulator identity cannot resolve its endpoint and leaves // AuthContext.AWS nil for in-process consumers (`!terraform.state`, stores). - Stack: stack, - AuthContext: &schema.AuthContext{}, + Stack: reExecCtx.Stack, + // Carry forward component/stack values resolved via an interactive prompt so a + // later identity-not-found fallback (manager.Authenticate) can re-inject them into + // a profile-fallback re-exec instead of dropping them. + ComponentFromArg: reExecCtx.Component, + ComponentPrompted: reExecCtx.ComponentPrompted, + StackPrompted: reExecCtx.StackPrompted, + AuthContext: &schema.AuthContext{}, } credStore := credentials.NewCredentialStoreWithConfig(authConfig) @@ -258,6 +264,11 @@ func CreateAndAuthenticateManagerWithAtmosConfig( // auth context (AuthContext.AWS, including the emulator endpoint) consumed by `!terraform.state`, // `!store`, `!secret`, and store hooks. Callers with a concrete (component, stack) pair should // use this variant; callers without a target stack can use the no-stack wrapper above. +// +// This signature is preserved for source compatibility with external callers of this exported +// package API. Callers that also have component/stack values resolved via an interactive prompt +// (so a later identity-not-found fallback can re-inject them into a profile-fallback re-exec) +// should use CreateAndAuthenticateManagerWithReExecContext instead. func CreateAndAuthenticateManagerWithAtmosConfigForStack( identityName string, authConfig *schema.AuthConfig, @@ -265,9 +276,24 @@ func CreateAndAuthenticateManagerWithAtmosConfigForStack( atmosConfig *schema.AtmosConfiguration, stack string, ) (AuthManager, error) { - defer perf.Track(atmosConfig, "auth.CreateAndAuthenticateManagerWithAtmosConfigForStack")() + return CreateAndAuthenticateManagerWithReExecContext(identityName, authConfig, selectValue, atmosConfig, ReExecContext{Stack: stack}) +} + +// CreateAndAuthenticateManagerWithReExecContext is the ReExecContext-aware variant of +// CreateAndAuthenticateManagerWithAtmosConfigForStack. In addition to seeding the target stack, +// it carries component/stack values resolved via an interactive prompt into the manager's +// stackInfo, so a later identity-not-found fallback (manager.Authenticate) can re-inject them +// into a profile-fallback re-exec instead of dropping them. +func CreateAndAuthenticateManagerWithReExecContext( + identityName string, + authConfig *schema.AuthConfig, + selectValue string, + atmosConfig *schema.AtmosConfiguration, + reExecCtx ReExecContext, +) (AuthManager, error) { + defer perf.Track(atmosConfig, "auth.CreateAndAuthenticateManagerWithReExecContext")() - log.Debug("CreateAndAuthenticateManager called", "identityName", identityName, "hasAuthConfig", authConfig != nil, "stack", stack) + log.Debug("CreateAndAuthenticateManager called", "identityName", identityName, "hasAuthConfig", authConfig != nil, "stack", reExecCtx.Stack) // Check if authentication is explicitly disabled. if shouldDisableAuth(identityName) { @@ -300,7 +326,7 @@ func CreateAndAuthenticateManagerWithAtmosConfigForStack( // Create AuthManager instance, seeding the target stack so stack-scoped identities // receive it via SetStack before authentication. - authManager, err := createAuthManagerInstance(authConfig, cliConfigPath, stack) + authManager, err := createAuthManagerInstance(authConfig, cliConfigPath, reExecCtx) if err != nil { return nil, err } @@ -336,7 +362,7 @@ func CreateManagerWithAtmosConfigForStack( cliConfigPath = atmosConfig.CliConfigPath } - return createAuthManagerInstance(authConfig, cliConfigPath, stack) + return createAuthManagerInstance(authConfig, cliConfigPath, ReExecContext{Stack: stack}) } // CreateAndAuthenticateManagerWithStackScan creates and authenticates an AuthManager, first running diff --git a/pkg/auth/manager_helpers_test.go b/pkg/auth/manager_helpers_test.go index bfc4ee190bd..f16c60a4a13 100644 --- a/pkg/auth/manager_helpers_test.go +++ b/pkg/auth/manager_helpers_test.go @@ -846,7 +846,7 @@ func TestCreateAuthManagerInstance(t *testing.T) { }, } - manager, err := createAuthManagerInstance(authConfig, "", "") + manager, err := createAuthManagerInstance(authConfig, "", ReExecContext{}) require.NoError(t, err, "should successfully create manager") require.NotNil(t, manager, "manager should not be nil") @@ -865,7 +865,7 @@ func TestCreateAuthManagerInstance_ThreadsStack(t *testing.T) { }, } - manager, err := createAuthManagerInstance(authConfig, "", "plat-ue2-dev") + manager, err := createAuthManagerInstance(authConfig, "", ReExecContext{Stack: "plat-ue2-dev"}) require.NoError(t, err) require.NotNil(t, manager) @@ -874,9 +874,38 @@ func TestCreateAuthManagerInstance_ThreadsStack(t *testing.T) { assert.Equal(t, "plat-ue2-dev", si.Stack, "target stack must be threaded into the manager at construction") } +// TestCreateAuthManagerInstance_ThreadsPromptedComponentAndStack verifies that component and +// stack values resolved via an interactive prompt are threaded into the manager's stackInfo. +// Authenticate reads stackInfo to build the ReExecContext it passes to the profile-fallback +// re-exec, so without this the prompted values never survive an identity-not-found fallback +// for managers built through this constructor. +func TestCreateAuthManagerInstance_ThreadsPromptedComponentAndStack(t *testing.T) { + authConfig := &schema.AuthConfig{ + Identities: map[string]schema.Identity{ + "local-aws": {Kind: "aws/emulator", Emulator: "aws"}, + }, + } + + manager, err := createAuthManagerInstance(authConfig, "", ReExecContext{ + Component: "vpc", + ComponentPrompted: true, + Stack: "plat-ue2-dev", + StackPrompted: true, + }) + + require.NoError(t, err) + require.NotNil(t, manager) + si := manager.GetStackInfo() + require.NotNil(t, si, "manager should carry stack info") + assert.Equal(t, "vpc", si.ComponentFromArg, "prompted component must be threaded into the manager at construction") + assert.True(t, si.ComponentPrompted, "ComponentPrompted must be threaded into the manager at construction") + assert.Equal(t, "plat-ue2-dev", si.Stack) + assert.True(t, si.StackPrompted, "StackPrompted must be threaded into the manager at construction") +} + func TestCreateAuthManagerInstance_NilConfig(t *testing.T) { // Creating manager with nil config should fail validation. - manager, err := createAuthManagerInstance(nil, "", "") + manager, err := createAuthManagerInstance(nil, "", ReExecContext{}) // The NewAuthManager constructor should handle nil gracefully or error. // In this case, we expect an error since nil config is invalid. @@ -888,6 +917,20 @@ func TestCreateAuthManagerInstance_NilConfig(t *testing.T) { assert.Nil(t, manager, "manager should be nil on error") } +// TestCreateAndAuthenticateManagerWithAtmosConfigForStack_StringSignaturePreserved pins +// CreateAndAuthenticateManagerWithAtmosConfigForStack's exported signature to a plain `stack +// string` fifth parameter. This is a public pkg/auth API; changing it to ReExecContext would +// break any external caller compiled against the old signature. The ReExecContext-aware +// behavior lives in the separate CreateAndAuthenticateManagerWithReExecContext function instead. +// Uses an unconfigured authConfig (no identities) so the assertion doesn't depend on real auth. +func TestCreateAndAuthenticateManagerWithAtmosConfigForStack_StringSignaturePreserved(t *testing.T) { + manager, err := CreateAndAuthenticateManagerWithAtmosConfigForStack("some-identity", &schema.AuthConfig{}, "__SELECT__", nil, "plat-ue2-dev") + + require.Error(t, err, "an identity name with no auth configured must error") + assert.ErrorIs(t, err, errUtils.ErrAuthNotConfigured) + assert.Nil(t, manager) +} + // TestCreateManagerWithAtmosConfigForStack covers the no-auth (deferred-identity) manager // constructor: it must reject an unconfigured auth section, and otherwise create a manager // without authenticating any identity, threading atmosConfig.CliConfigPath and the target @@ -1196,7 +1239,7 @@ func TestAuthenticateWithIdentity_SelectValue(t *testing.T) { } // Create manager - manager, err := createAuthManagerInstance(authConfig, "", "") + manager, err := createAuthManagerInstance(authConfig, "", ReExecContext{}) require.NoError(t, err) // Call with identity matching select value - triggers forceSelect branch diff --git a/pkg/auth/profile_fallback.go b/pkg/auth/profile_fallback.go index 6d5f1f46f27..949663c312b 100644 --- a/pkg/auth/profile_fallback.go +++ b/pkg/auth/profile_fallback.go @@ -58,6 +58,20 @@ func newProfileFallbackKeyMap() *huh.KeyMap { // on Windows). Tests swap reexec.Exec to avoid actually replacing the test // process. +// ReExecContext carries CLI state that was resolved interactively (not via +// argv) before a profile-fallback re-exec, so the re-exec'd child doesn't +// have to re-prompt for values the user, or an earlier prompt, already +// supplied. The *Prompted fields distinguish "resolved via prompt" from +// "supplied on the command line" — only prompted values are injected into +// the child's argv, since command-line-supplied values are already present +// in os.Args and re-adding them would duplicate a positional argument. +type ReExecContext struct { + Component string + ComponentPrompted bool + Stack string + StackPrompted bool +} + // buildFallbackAtmosConfig returns a minimal AtmosConfiguration scoped to the // manager's loaded atmos.yaml so the config-layer profile helpers can discover // profiles consistently. The profiles.base_path value is read from the global @@ -93,7 +107,7 @@ func (m *manager) buildFallbackAtmosConfig() *schema.AtmosConfiguration { // we're already inside a re-exec'd child, skip the fallback and let the // original error surface so users don't get trapped in an endless prompt // cycle. -func (m *manager) maybeOfferProfileFallback(ctx context.Context, identityName string) error { +func (m *manager) maybeOfferProfileFallback(ctx context.Context, identityName string, reExecCtx ReExecContext) error { defer perf.Track(nil, "auth.Manager.maybeOfferProfileFallback")() // Loop guard — if we've already re-exec'd once for this identity and still @@ -140,7 +154,7 @@ func (m *manager) maybeOfferProfileFallback(ctx context.Context, identityName st } // Re-exec never returns on success; if it does, something went wrong. - if err := reExecWithProfile(picked); err != nil { + if err := reExecWithProfile(picked, reExecCtx); err != nil { return fmt.Errorf("failed to re-exec with profile %q: %w", picked, err) } // Unreachable on successful exec. @@ -315,7 +329,7 @@ func (m *manager) maybeOfferAnyProfileFallback(ctx context.Context) error { return promptErr } - if err := reExecWithProfile(picked); err != nil { + if err := reExecWithProfile(picked, ReExecContext{}); err != nil { return fmt.Errorf("failed to re-exec with profile %q: %w", picked, err) } // Unreachable on successful exec. @@ -341,9 +355,9 @@ func (m *manager) MaybeOfferAnyProfileFallback(ctx context.Context) error { // The maybeOfferProfileFallback method only touches the manager's cliConfigPath field // (via buildFallbackAtmosConfig), so a throwaway manager scoped to just that field is // sufficient to reuse the existing, already-tested flow unchanged. -func MaybeOfferProfileFallbackForIdentity(ctx context.Context, cliConfigPath string, identityName string) error { +func MaybeOfferProfileFallbackForIdentity(ctx context.Context, cliConfigPath string, identityName string, reExecCtx ReExecContext) error { fallbackManager := &manager{cliConfigPath: cliConfigPath} - return fallbackManager.maybeOfferProfileFallback(ctx, identityName) + return fallbackManager.maybeOfferProfileFallback(ctx, identityName, reExecCtx) } // buildAnyProfileSuggestionError wraps ErrNoIdentitiesAvailable with actionable @@ -448,7 +462,7 @@ func (m *manager) confirmSingleAnyProfileSelection(profile string) (string, erro // front of the argument list (after argv[0]) and the loop-guard env var set. // On success, this function does NOT return — the current process is replaced // (Unix) or exits with the child's status (Windows, via Go's syscall shim). -func reExecWithProfile(profileName string) error { +func reExecWithProfile(profileName string, reExecCtx ReExecContext) error { defer perf.Track(nil, "auth.reExecWithProfile")() exe, err := os.Executable() @@ -462,13 +476,26 @@ func reExecWithProfile(profileName string) error { // stripping it from the child's argv prevents a relative chdir from being // re-applied against the already-changed cwd. origArgs := os.Args - newArgs := make([]string, 0, len(origArgs)+2) + newArgs := make([]string, 0, len(origArgs)+6) newArgs = append(newArgs, origArgs[0]) newArgs = append(newArgs, profileFlagName, profileName) if len(origArgs) > 1 { newArgs = append(newArgs, reexec.StripChdirArgs(origArgs[1:])...) } + // Carry forward component/stack values that were resolved via an + // interactive prompt in this process — the child starts a brand-new Cobra + // invocation with no memory of those prompts, so without this it would + // re-prompt for both right after the user just picked a profile. Values + // the user typed on the command line are already present in origArgs + // above and must not be duplicated, hence the *Prompted guards. + if reExecCtx.StackPrompted && reExecCtx.Stack != "" { + newArgs = append(newArgs, "--stack", reExecCtx.Stack) + } + if reExecCtx.ComponentPrompted && reExecCtx.Component != "" { + newArgs = append(newArgs, reExecCtx.Component) + } + // Propagate environment + loop guard. ATMOS_CHDIR is filtered for the // same reason --chdir is stripped from argv. NextEnv increments // ATMOS_REEXEC_DEPTH so the child can detect it's inside a re-exec. diff --git a/pkg/auth/profile_fallback_test.go b/pkg/auth/profile_fallback_test.go index 82e75e3ce69..45430713103 100644 --- a/pkg/auth/profile_fallback_test.go +++ b/pkg/auth/profile_fallback_test.go @@ -16,6 +16,7 @@ import ( errUtils "github.com/cloudposse/atmos/errors" "github.com/cloudposse/atmos/pkg/reexec" + "github.com/cloudposse/atmos/pkg/schema" ) // stubRunForm replaces runForm for a single test with a function that returns @@ -114,7 +115,7 @@ func TestMaybeOfferProfileFallback_LoopGuardSkips(t *testing.T) { t.Setenv(reexec.DepthEnvVar, "1") m := newFallbackManager(tmpDir) - err := m.maybeOfferProfileFallback(context.Background(), "root-admin") + err := m.maybeOfferProfileFallback(context.Background(), "root-admin", ReExecContext{}) assert.NoError(t, err, "loop guard must short-circuit without error") } @@ -127,7 +128,7 @@ func TestMaybeOfferProfileFallback_ExplicitFlagSkips(t *testing.T) { os.Args = []string{"atmos", "--profile", "alpha", "terraform", "plan"} m := newFallbackManager(tmpDir) - err := m.maybeOfferProfileFallback(context.Background(), "dev-user") + err := m.maybeOfferProfileFallback(context.Background(), "dev-user", ReExecContext{}) assert.NoError(t, err, "explicit --profile must suppress the fallback") } @@ -139,7 +140,7 @@ func TestMaybeOfferProfileFallback_ExplicitEnvSkips(t *testing.T) { t.Setenv("ATMOS_PROFILE", "alpha") m := newFallbackManager(tmpDir) - err := m.maybeOfferProfileFallback(context.Background(), "dev-user") + err := m.maybeOfferProfileFallback(context.Background(), "dev-user", ReExecContext{}) assert.NoError(t, err, "ATMOS_PROFILE must suppress the fallback") } @@ -150,7 +151,7 @@ func TestMaybeOfferProfileFallback_NoCandidatesReturnsNil(t *testing.T) { tmpDir := profileFallbackFixture(t) m := newFallbackManager(tmpDir) - err := m.maybeOfferProfileFallback(context.Background(), "nonexistent") + err := m.maybeOfferProfileFallback(context.Background(), "nonexistent", ReExecContext{}) assert.NoError(t, err, "no candidate profile → no fallback error") } @@ -164,7 +165,7 @@ func TestMaybeOfferProfileFallback_NonInteractiveEnrichesError(t *testing.T) { // isInteractive() returns false without --interactive — this is the // non-interactive path. m := newFallbackManager(tmpDir) - err := m.maybeOfferProfileFallback(context.Background(), "root-admin") + err := m.maybeOfferProfileFallback(context.Background(), "root-admin", ReExecContext{}) require.Error(t, err, "non-interactive must return an enriched error") assert.ErrorIs(t, err, errUtils.ErrIdentityNotFound) @@ -242,7 +243,7 @@ func TestReExecWithProfile_BuildsArgvAndEnv(t *testing.T) { os.Args = []string{"atmos", "terraform", "plan", "--stack", "dev"} t.Cleanup(func() { os.Args = origArgs }) - err := reExecWithProfile("developer") + err := reExecWithProfile("developer", ReExecContext{}) require.ErrorIs(t, err, errExecMockCalled) assert.NotEmpty(t, gotArgv0, "argv0 (binary path) must be populated") @@ -281,13 +282,206 @@ func TestReExecWithProfile_NoExtraArgs(t *testing.T) { os.Args = []string{"atmos"} t.Cleanup(func() { os.Args = origArgs }) - err := reExecWithProfile("prod") + err := reExecWithProfile("prod", ReExecContext{}) require.ErrorIs(t, err, errExecMockCalled) require.Len(t, gotArgs, 3) assert.Equal(t, []string{"atmos", "--profile", "prod"}, gotArgs) } +// reExecWithProfile injects component/stack values that were resolved via an +// interactive prompt so the re-exec'd child doesn't have to re-prompt for +// them. This is the regression guard for the "profile prompt causes +// component/stack to be prompted twice" bug. +func TestReExecWithProfile_InjectsPromptedComponentAndStack(t *testing.T) { + t.Cleanup(func() { reexec.Exec = originalExecFunc }) + + var gotArgs []string + reexec.Exec = func(_ string, argv []string, _ []string) error { + gotArgs = argv + return errExecMockCalled + } + + origArgs := os.Args + os.Args = []string{"atmos", "terraform", "plan", "--ui"} + t.Cleanup(func() { os.Args = origArgs }) + + reExecCtx := ReExecContext{ + Component: "vpc", + ComponentPrompted: true, + Stack: "core-ue2-auto", + StackPrompted: true, + } + err := reExecWithProfile("managers", reExecCtx) + require.ErrorIs(t, err, errExecMockCalled) + + assert.Contains(t, gotArgs, "vpc", "prompted component must be injected into the child argv") + assert.Contains(t, gotArgs, "--stack", "prompted stack flag must be injected into the child argv") + assert.Contains(t, gotArgs, "core-ue2-auto", "prompted stack value must be injected into the child argv") +} + +// reExecWithProfile must NOT inject component/stack values the user already +// typed on the command line — only values resolved via a prompt. Otherwise a +// user-supplied component would be duplicated as a positional argument. +func TestReExecWithProfile_DoesNotInjectUnpromptedValues(t *testing.T) { + t.Cleanup(func() { reexec.Exec = originalExecFunc }) + + var gotArgs []string + reexec.Exec = func(_ string, argv []string, _ []string) error { + gotArgs = argv + return errExecMockCalled + } + + origArgs := os.Args + os.Args = []string{"atmos", "terraform", "plan", "vpc", "--stack", "core-ue2-auto"} + t.Cleanup(func() { os.Args = origArgs }) + + // Component/stack were resolved (e.g. from the command line), but not via + // a prompt, so ComponentPrompted/StackPrompted are false. + reExecCtx := ReExecContext{ + Component: "vpc", + Stack: "core-ue2-auto", + } + err := reExecWithProfile("managers", reExecCtx) + require.ErrorIs(t, err, errExecMockCalled) + + // New argv: [atmos, --profile, managers, terraform, plan, vpc, --stack, core-ue2-auto]. + require.Len(t, gotArgs, 8, "component/stack must not be duplicated: %v", gotArgs) +} + +// reExecWithProfile treats ComponentPrompted and StackPrompted independently: +// a mixed state (only one of the two prompted) must inject exactly the +// prompted value, never the other, and never duplicate or drop either one. +// This is the regression guard for the case the prior two tests didn't +// cover — "both prompted" and "both unprompted" could both pass even if a +// bug always injected/omitted the two fields together instead of tracking +// them independently. +func TestReExecWithProfile_MixedPromptedStates(t *testing.T) { + tests := []struct { + name string + componentPrompted bool + stackPrompted bool + wantArgs []string + }{ + { + name: "neither prompted", + componentPrompted: false, + stackPrompted: false, + wantArgs: []string{"atmos", "--profile", "managers", "terraform", "plan"}, + }, + { + name: "component only prompted", + componentPrompted: true, + stackPrompted: false, + wantArgs: []string{"atmos", "--profile", "managers", "terraform", "plan", "vpc"}, + }, + { + name: "stack only prompted", + componentPrompted: false, + stackPrompted: true, + wantArgs: []string{"atmos", "--profile", "managers", "terraform", "plan", "--stack", "core-ue2-auto"}, + }, + { + name: "both prompted", + componentPrompted: true, + stackPrompted: true, + wantArgs: []string{"atmos", "--profile", "managers", "terraform", "plan", "--stack", "core-ue2-auto", "vpc"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Cleanup(func() { reexec.Exec = originalExecFunc }) + + var gotArgs []string + reexec.Exec = func(_ string, argv []string, _ []string) error { + gotArgs = argv + return errExecMockCalled + } + + origArgs := os.Args + os.Args = []string{"atmos", "terraform", "plan"} + t.Cleanup(func() { os.Args = origArgs }) + + reExecCtx := ReExecContext{ + Component: "vpc", + ComponentPrompted: tt.componentPrompted, + Stack: "core-ue2-auto", + StackPrompted: tt.stackPrompted, + } + err := reExecWithProfile("managers", reExecCtx) + require.ErrorIs(t, err, errExecMockCalled) + + assert.Equal(t, tt.wantArgs, gotArgs, "complete child argv must match exactly") + }) + } +} + +// countOccurrences returns how many elements of argv equal val. +func countOccurrences(argv []string, val string) int { + count := 0 + for _, a := range argv { + if a == val { + count++ + } + } + return count +} + +// Authenticate builds its ReExecContext from the manager's own stackInfo, so a +// component/stack resolved via an interactive prompt survives an +// identity-not-found profile-fallback re-exec. This is the regression guard +// for the manager.go bug where an empty ReExecContext{} was passed +// unconditionally, silently dropping any prompted values before they ever +// reached reExecWithProfile. +func TestAuthenticate_PassesPromptedComponentAndStackToProfileFallback(t *testing.T) { + resetGlobalProfileState(t) + stubInteractiveTrue(t) + stubRunForm(t, nil) // huh.Select defaults the bound value to the first sorted option. + + // Two profiles defining the same identity so promptForProfileSelection + // takes the multi-candidate huh.Select branch — the single-candidate + // confirm branch can't have its bound bool flipped from a stub (see + // TestConfirmSingleProfileSelection_DefaultNoIsAbort). + tmpDir := t.TempDir() + for _, name := range []string{"alpha", "beta"} { + dir := filepath.Join(tmpDir, "profiles", name) + require.NoError(t, os.MkdirAll(dir, 0o755)) + yaml := `auth: + identities: + shared-admin: + kind: aws/user +` + require.NoError(t, os.WriteFile(filepath.Join(dir, "atmos.yaml"), []byte(yaml), 0o644)) + } + + t.Cleanup(func() { reexec.Exec = originalExecFunc }) + var gotArgs []string + reexec.Exec = func(_ string, argv []string, _ []string) error { + gotArgs = argv + return errExecMockCalled + } + + m := &manager{ + cliConfigPath: tmpDir, + config: &schema.AuthConfig{}, + stackInfo: &schema.ConfigAndStacksInfo{ + ComponentFromArg: "vpc", + ComponentPrompted: true, + Stack: "core-ue2-auto", + StackPrompted: true, + }, + } + + _, err := m.Authenticate(context.Background(), "shared-admin") + require.ErrorIs(t, err, errExecMockCalled, + "the identity-not-found path must reach reExecWithProfile via the manager's own stackInfo") + + assert.Equal(t, 1, countOccurrences(gotArgs, "vpc"), "prompted component must appear exactly once: %v", gotArgs) + assert.Equal(t, 1, countOccurrences(gotArgs, "--stack"), "prompted --stack flag must appear exactly once: %v", gotArgs) + assert.Equal(t, 1, countOccurrences(gotArgs, "core-ue2-auto"), "prompted stack value must appear exactly once: %v", gotArgs) +} + // anyProfileFallbackFixture creates profiles that exercise the identity- // agnostic fallback path: "auth-alpha" and "auth-beta" both define auth config // (via auth.identities or auth.providers); "plain" has none. Returns the @@ -477,7 +671,7 @@ func TestMaybeOfferProfileFallbackForIdentity_NoCandidatesReturnsNil(t *testing. resetGlobalProfileState(t) tmpDir := profileFallbackFixture(t) - err := MaybeOfferProfileFallbackForIdentity(context.Background(), tmpDir, "nonexistent") + err := MaybeOfferProfileFallbackForIdentity(context.Background(), tmpDir, "nonexistent", ReExecContext{}) assert.NoError(t, err, "no candidate profile → no fallback error") } @@ -486,7 +680,7 @@ func TestMaybeOfferProfileFallbackForIdentity_LoopGuardSkips(t *testing.T) { tmpDir := profileFallbackFixture(t) t.Setenv(reexec.DepthEnvVar, "1") - err := MaybeOfferProfileFallbackForIdentity(context.Background(), tmpDir, "root-admin") + err := MaybeOfferProfileFallbackForIdentity(context.Background(), tmpDir, "root-admin", ReExecContext{}) assert.NoError(t, err, "loop guard must short-circuit without error") } @@ -494,7 +688,7 @@ func TestMaybeOfferProfileFallbackForIdentity_NonInteractiveEnrichesError(t *tes resetGlobalProfileState(t) tmpDir := profileFallbackFixture(t) - err := MaybeOfferProfileFallbackForIdentity(context.Background(), tmpDir, "root-admin") + err := MaybeOfferProfileFallbackForIdentity(context.Background(), tmpDir, "root-admin", ReExecContext{}) require.Error(t, err, "non-interactive must return an enriched error") assert.ErrorIs(t, err, errUtils.ErrIdentityNotFound) @@ -522,7 +716,7 @@ func TestReExecWithProfile_StripsChdirFromArgv(t *testing.T) { os.Args = []string{"atmos", "--chdir", "/tmp", "auth", "login"} t.Cleanup(func() { os.Args = origArgs }) - err := reExecWithProfile("dev") + err := reExecWithProfile("dev", ReExecContext{}) require.ErrorIs(t, err, errExecMockCalled) for i, a := range gotArgs { @@ -559,7 +753,7 @@ func TestReExecWithProfile_FiltersChdirFromEnv(t *testing.T) { os.Args = []string{"atmos", "auth", "login"} t.Cleanup(func() { os.Args = origArgs }) - err := reExecWithProfile("dev") + err := reExecWithProfile("dev", ReExecContext{}) require.ErrorIs(t, err, errExecMockCalled) // FilterChdirEnv emits "ATMOS_CHDIR=" (empty) as an explicit override so the @@ -804,7 +998,7 @@ func TestMaybeOfferProfileFallback_InteractiveUserAborted(t *testing.T) { tmpDir := profileFallbackFixture(t) m := newFallbackManager(tmpDir) - err := m.maybeOfferProfileFallback(context.Background(), "root-admin") + err := m.maybeOfferProfileFallback(context.Background(), "root-admin", ReExecContext{}) assert.ErrorIs(t, err, errUtils.ErrUserAborted, "interactive + user aborts → whole fallback must return ErrUserAborted") } @@ -819,7 +1013,7 @@ func TestMaybeOfferProfileFallback_InteractivePromptError(t *testing.T) { tmpDir := profileFallbackFixture(t) m := newFallbackManager(tmpDir) - err := m.maybeOfferProfileFallback(context.Background(), "root-admin") + err := m.maybeOfferProfileFallback(context.Background(), "root-admin", ReExecContext{}) require.Error(t, err) assert.ErrorIs(t, err, errUtils.ErrUnsupportedInputType) } @@ -863,7 +1057,7 @@ func TestMaybeOfferProfileFallback_InteractiveReExecFails(t *testing.T) { } m := newFallbackManager(tmpDir) - err := m.maybeOfferProfileFallback(context.Background(), "shared-id") + err := m.maybeOfferProfileFallback(context.Background(), "shared-id", ReExecContext{}) require.Error(t, err) assert.Contains(t, err.Error(), "failed to re-exec", "re-exec failure must propagate through the interactive branch") diff --git a/pkg/flags/parser.go b/pkg/flags/parser.go index 3b19d7f53f5..48f068e044b 100644 --- a/pkg/flags/parser.go +++ b/pkg/flags/parser.go @@ -87,6 +87,27 @@ type ParsedConfig struct { // PositionalArgs: ["vpc"] // SeparatedArgs: ["-var", "foo=bar"] SeparatedArgs []string + + // PromptedFields records the names of flags and positional argument specs + // (e.g. "stack", "component") whose value was filled in by an interactive + // prompt during Parse, rather than supplied via CLI flag, positional + // argument, environment variable, or config file. StandardParser consults + // this to populate StandardOptions.ComponentPrompted / StackPrompted, which + // callers thread into auth.ReExecContext so a profile-fallback re-exec + // doesn't lose (or re-prompt for) a value the user just picked + // interactively. Nil/absent means nothing was prompted. + PromptedFields map[string]bool +} + +// markFieldPrompted records that name's value was filled in by an interactive +// prompt, lazily initializing PromptedFields on first use. +func markFieldPrompted(result *ParsedConfig, name string) { + defer perf.Track(nil, "flags.markFieldPrompted")() + + if result.PromptedFields == nil { + result.PromptedFields = make(map[string]bool) + } + result.PromptedFields[name] = true } // GetIdentity returns the identity value from parsed flags with proper type safety. diff --git a/pkg/flags/standard.go b/pkg/flags/standard.go index 59935e31516..8acb9b2bf98 100644 --- a/pkg/flags/standard.go +++ b/pkg/flags/standard.go @@ -1089,7 +1089,11 @@ func (p *StandardFlagParser) promptForOptionalValueFlags(result *ParsedConfig, c } // Update flag value with selection. + // The sentinel check above guarantees ctx.FlagValue was the sentinel, so a + // non-empty selectedValue here always came from the interactive prompt below, + // not from a pre-existing user-supplied value. result.Flags[flagName] = selectedValue + markFieldPrompted(result, flagName) } return nil @@ -1152,6 +1156,7 @@ func (p *StandardFlagParser) promptForSingleMissingFlag(flagName string, result if selectedValue != "" { result.Flags[flagName] = selectedValue + markFieldPrompted(result, flagName) } return nil @@ -1207,6 +1212,7 @@ func (p *StandardFlagParser) promptForMissingPositionalArgs(result *ParsedConfig // Append the selected value to positional args. result.PositionalArgs = append(result.PositionalArgs, selectedValue) + markFieldPrompted(result, spec.Name) } return nil diff --git a/pkg/flags/standard_options.go b/pkg/flags/standard_options.go index abfd7075bbc..e8f4991edcc 100644 --- a/pkg/flags/standard_options.go +++ b/pkg/flags/standard_options.go @@ -20,6 +20,14 @@ type StandardOptions struct { Component string // Component to operate on (--component, -c) Key string // Configuration key to filter (positional arg for list components) + // ComponentPrompted records whether Component was filled in via an + // interactive prompt (see WithPositionalArgPrompt / WithCompletionPrompt) + // rather than supplied on the command line, via config, or via an + // environment variable. + ComponentPrompted bool + // StackPrompted is the Stack equivalent of ComponentPrompted. + StackPrompted bool + // Output formatting flags. Format string // Output format (--format, -f): yaml, json, etc. File string // Write output to file (--file) diff --git a/pkg/flags/standard_parser.go b/pkg/flags/standard_parser.go index 717dd60fac5..067329d21b2 100644 --- a/pkg/flags/standard_parser.go +++ b/pkg/flags/standard_parser.go @@ -227,6 +227,8 @@ func (p *StandardParser) buildStandardOptions(parsedConfig *ParsedConfig, compon }, Stack: GetString(parsedConfig.Flags, "stack"), Component: component, + ComponentPrompted: parsedConfig.PromptedFields["component"], + StackPrompted: parsedConfig.PromptedFields["stack"], Format: GetString(parsedConfig.Flags, "format"), File: GetString(parsedConfig.Flags, "file"), ProcessTemplates: GetBool(parsedConfig.Flags, "process-templates"), diff --git a/pkg/flags/standard_prompted_test.go b/pkg/flags/standard_prompted_test.go new file mode 100644 index 00000000000..248c2d04f47 --- /dev/null +++ b/pkg/flags/standard_prompted_test.go @@ -0,0 +1,231 @@ +package flags + +import ( + "context" + "testing" + + "github.com/spf13/cobra" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestMarkFieldPrompted verifies the lazy-init helper backing PromptedFields tracking. +func TestMarkFieldPrompted(t *testing.T) { + t.Run("initializes map on first use", func(t *testing.T) { + result := &ParsedConfig{} + markFieldPrompted(result, "stack") + require.NotNil(t, result.PromptedFields) + assert.True(t, result.PromptedFields["stack"]) + }) + + t.Run("adds additional keys without clobbering existing ones", func(t *testing.T) { + result := &ParsedConfig{PromptedFields: map[string]bool{"stack": true}} + markFieldPrompted(result, "component") + assert.True(t, result.PromptedFields["stack"], "existing entry must survive") + assert.True(t, result.PromptedFields["component"]) + }) +} + +// TestStandardParser_BuildStandardOptions_PromptedFields verifies that +// StandardOptions.ComponentPrompted / StackPrompted are correctly derived from +// ParsedConfig.PromptedFields. This is the seam CodeRabbit's review flagged: +// StandardParser must surface which values were resolved via interactive +// prompt so callers (e.g. cmd/terraform/backend) can build a correct +// auth.ReExecContext without depending on a real TTY prompt actually running. +func TestStandardParser_BuildStandardOptions_PromptedFields(t *testing.T) { + tests := []struct { + name string + promptedFields map[string]bool + expectedComponentPrompted bool + expectedStackPrompted bool + }{ + { + name: "nothing prompted", + promptedFields: nil, + expectedComponentPrompted: false, + expectedStackPrompted: false, + }, + { + name: "only component prompted", + promptedFields: map[string]bool{"component": true}, + expectedComponentPrompted: true, + expectedStackPrompted: false, + }, + { + name: "only stack prompted", + promptedFields: map[string]bool{"stack": true}, + expectedComponentPrompted: false, + expectedStackPrompted: true, + }, + { + name: "both component and stack prompted", + promptedFields: map[string]bool{"component": true, "stack": true}, + expectedComponentPrompted: true, + expectedStackPrompted: true, + }, + { + name: "unrelated prompted field does not leak", + promptedFields: map[string]bool{"identity": true}, + expectedComponentPrompted: false, + expectedStackPrompted: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := &StandardParser{} + parsedConfig := &ParsedConfig{ + Flags: map[string]interface{}{"stack": "dev"}, + PromptedFields: tt.promptedFields, + } + + opts := p.buildStandardOptions(parsedConfig, "vpc", "", "") + + assert.Equal(t, tt.expectedComponentPrompted, opts.ComponentPrompted) + assert.Equal(t, tt.expectedStackPrompted, opts.StackPrompted) + // Sanity: unrelated fields still resolve normally (purely additive change). + assert.Equal(t, "vpc", opts.Component) + assert.Equal(t, "dev", opts.Stack) + }) + } +} + +// TestStandardParser_Parse_NotPromptedWhenSuppliedOnCLI verifies the negative +// path end-to-end through the real Parse() pipeline: when component and stack +// are supplied directly on the CLI (positional arg + flag), ComponentPrompted +// and StackPrompted must be false, even though both flags have prompts +// configured. This guards against a regression where any prompt-configured +// field is marked prompted regardless of how its value was actually resolved. +func TestStandardParser_Parse_NotPromptedWhenSuppliedOnCLI(t *testing.T) { + completionFunc := func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + return []string{"vpc", "eks"}, cobra.ShellCompDirectiveNoFileComp + } + + argsBuilder := NewPositionalArgsBuilder() + argsBuilder.AddArg(&PositionalArgSpec{ + Name: "component", + Description: "Component name", + Required: true, + TargetField: "Component", + CompletionFunc: completionFunc, + PromptTitle: "Choose a component", + }) + specs, _, usage := argsBuilder.Build() + + parser := NewStandardParser( + WithStringFlag("stack", "s", "", "Stack name"), + WithCompletionPrompt("stack", "Choose a stack", completionFunc), + WithPositionalArgPrompt("component", "Choose a component", completionFunc), + ) + parser.SetPositionalArgs(specs, nil, usage) + + cmd := &cobra.Command{Use: "test"} + parser.RegisterFlags(cmd) + + v := viper.New() + require.NoError(t, parser.BindToViper(v)) + require.NoError(t, cmd.Flags().Parse([]string{"--stack", "dev"})) + require.NoError(t, parser.BindFlagsToViper(cmd, v)) + + opts, err := parser.Parse(context.Background(), []string{"vpc", "--stack", "dev"}) + require.NoError(t, err) + + assert.Equal(t, "vpc", opts.Component) + assert.Equal(t, "dev", opts.Stack) + assert.False(t, opts.ComponentPrompted, "component supplied positionally must not be marked prompted") + assert.False(t, opts.StackPrompted, "stack supplied via flag must not be marked prompted") +} + +// TestStandardFlagParser_PromptForSingleMissingFlag_DoesNotMarkPromptedWhenSkipped +// extends the existing skip-path coverage in standard_test.go to assert that +// PromptedFields stays empty when promptForSingleMissingFlag returns early +// (value already present, or explicitly set to empty, or non-interactive). +func TestStandardFlagParser_PromptForSingleMissingFlag_DoesNotMarkPromptedWhenSkipped(t *testing.T) { + originalInteractive := viper.GetBool("interactive") + defer viper.Set("interactive", originalInteractive) + + completionFunc := func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + return []string{"stack1", "stack2"}, cobra.ShellCompDirectiveNoFileComp + } + + t.Run("flag already has value", func(t *testing.T) { + viper.Set("interactive", false) + + parser := NewStandardFlagParser( + WithStringFlag("stack", "s", "", "Stack name"), + WithCompletionPrompt("stack", "Choose stack", completionFunc), + ) + cmd := &cobra.Command{Use: "test"} + parser.RegisterFlags(cmd) + + result := &ParsedConfig{Flags: map[string]interface{}{"stack": "prod"}} + + require.NoError(t, parser.promptForSingleMissingFlag("stack", result, cmd.Flags())) + assert.Empty(t, result.PromptedFields) + }) + + t.Run("not interactive", func(t *testing.T) { + viper.Set("interactive", false) + + parser := NewStandardFlagParser( + WithStringFlag("stack", "s", "", "Stack name"), + WithCompletionPrompt("stack", "Choose stack", completionFunc), + ) + cmd := &cobra.Command{Use: "test"} + parser.RegisterFlags(cmd) + + result := &ParsedConfig{Flags: map[string]interface{}{"stack": ""}} + + require.NoError(t, parser.promptForSingleMissingFlag("stack", result, cmd.Flags())) + assert.Empty(t, result.PromptedFields) + }) +} + +// TestStandardFlagParser_PromptForMissingPositionalArgs_DoesNotMarkPromptedWhenSkipped +// mirrors the flag-prompt skip coverage above for the positional-arg prompt path +// (Use Case 3, used by e.g. cmd/terraform/backend's "component" positional arg). +func TestStandardFlagParser_PromptForMissingPositionalArgs_DoesNotMarkPromptedWhenSkipped(t *testing.T) { + originalInteractive := viper.GetBool("interactive") + defer viper.Set("interactive", originalInteractive) + + completionFunc := func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + return []string{"vpc", "eks"}, cobra.ShellCompDirectiveNoFileComp + } + + buildParser := func() *StandardFlagParser { + builder := NewPositionalArgsBuilder() + builder.AddArg(&PositionalArgSpec{ + Name: "component", + Description: "Component name", + Required: true, + CompletionFunc: completionFunc, + PromptTitle: "Choose a component", + }) + specs, validator, usage := builder.Build() + + parser := NewStandardFlagParser( + WithPositionalArgPrompt("component", "Choose a component", completionFunc), + ) + parser.SetPositionalArgs(specs, validator, usage) + return parser + } + + t.Run("argument already provided", func(t *testing.T) { + parser := buildParser() + result := &ParsedConfig{PositionalArgs: []string{"vpc"}} + + require.NoError(t, parser.promptForMissingPositionalArgs(result)) + assert.Empty(t, result.PromptedFields) + }) + + t.Run("not interactive", func(t *testing.T) { + viper.Set("interactive", false) + + parser := buildParser() + result := &ParsedConfig{PositionalArgs: []string{}} + + require.NoError(t, parser.promptForMissingPositionalArgs(result)) + assert.Empty(t, result.PromptedFields) + }) +} diff --git a/pkg/schema/schema.go b/pkg/schema/schema.go index d5d3f751bb0..24ef1f24fb6 100644 --- a/pkg/schema/schema.go +++ b/pkg/schema/schema.go @@ -1676,12 +1676,17 @@ type GCPAuthContext struct { } type ConfigAndStacksInfo struct { - StackFromArg string - Stack string - StackFile string - StackManifestName string // Stack-level 'name' override from manifest (highest precedence). - ComponentType string - ComponentFromArg string + StackFromArg string + Stack string + StackFile string + StackManifestName string // Stack-level 'name' override from manifest (highest precedence). + ComponentType string + ComponentFromArg string + // ComponentPrompted records whether ComponentFromArg was filled in via an + // interactive prompt rather than supplied on the command line. + ComponentPrompted bool + // StackPrompted is the Stack equivalent of ComponentPrompted. + StackPrompted bool Component string ComponentFolderPrefix string ComponentFolderPrefixReplaced string