diff --git a/cmd/aws_devops_agent/flags_test.go b/cmd/aws_devops_agent/flags_test.go index bb4e85ef..e3656583 100644 --- a/cmd/aws_devops_agent/flags_test.go +++ b/cmd/aws_devops_agent/flags_test.go @@ -62,13 +62,14 @@ func spaceIDRun(t *testing.T, flags map[string]string) []string { return w.calls } -// An auth flag asks for full setup. -func TestSpaceIDWithAuthFlagRunsFullSetup(t *testing.T) { +// --full-setup asks for full setup, with any sign-in method. +func TestSpaceIDWithFullSetupRunsFullSetup(t *testing.T) { cases := map[string]map[string]string{ - "auth-flow": {authFlowFlag: "iam"}, - "idc-instance-arn": {authFlowFlag: "idc", idcInstanceARNFlag: "arn:aws:sso:::instance/ssoins-1"}, + "full-setup": {fullSetupFlag: "true"}, + "auth-flow": {fullSetupFlag: "true", authFlowFlag: "iam"}, + "idc-instance-arn": {fullSetupFlag: "true", authFlowFlag: "idc", idcInstanceARNFlag: "arn:aws:sso:::instance/ssoins-1"}, "idp": { - authFlowFlag: "idp", issuerURLFlag: "https://idp.example.com", + fullSetupFlag: "true", authFlowFlag: "idp", issuerURLFlag: "https://idp.example.com", idpClientIDFlag: "client", idpClientSecretFlag: "secret", }, } @@ -146,6 +147,12 @@ func TestSetupOptionsRecordsAuthFlow(t *testing.T) { opts, err = setupOptions(cmd) require.NoError(t, err) assert.True(t, opts.AuthFlowSet) + assert.False(t, opts.FullSetup) + + require.NoError(t, cmd.Flags().Set(fullSetupFlag, "true")) + opts, err = setupOptions(cmd) + require.NoError(t, err) + assert.True(t, opts.FullSetup) } // Setup does not ask for the replacement token, so the flag needs one. diff --git a/cmd/aws_devops_agent/plan_print_test.go b/cmd/aws_devops_agent/plan_print_test.go index ac50a0ee..814f5fe0 100644 --- a/cmd/aws_devops_agent/plan_print_test.go +++ b/cmd/aws_devops_agent/plan_print_test.go @@ -363,10 +363,10 @@ func twoSimilarSpaces() (awsdevops.Clients, *writes) { ) } -// --auth-flow runs full setup on choice. -func TestSetupChosenSpaceWithAuthFlow(t *testing.T) { +// --full-setup runs full setup on choice. +func TestSetupChosenSpaceWithFullSetup(t *testing.T) { cmd, out := setupTestCmd(t, "2\ny\n") - require.NoError(t, cmd.Flags().Set(authFlowFlag, "iam")) + require.NoError(t, cmd.Flags().Set(fullSetupFlag, "true")) clients, w := twoSimilarSpaces() opts, err := setupOptions(cmd) require.NoError(t, err) @@ -375,13 +375,30 @@ func TestSetupChosenSpaceWithAuthFlow(t *testing.T) { require.NoError(t, setupWith(cmd, clients, opts, true, true)) assert.Contains(t, out.String(), "Reuse agent space LaunchDarkly (space-2)") + assert.Contains(t, out.String(), "Setup: Full setup") assert.Contains(t, w.calls, "EnableOperatorApp") assert.NotContains(t, w.calls, "CreateAgentSpace") assert.Contains(t, out.String(), "Reused agent space LaunchDarkly (space-2)") assert.NotContains(t, out.String(), "agent space launchdarkly (space-1)") } -func TestSetupChosenSpaceWithoutAuthFlowConnects(t *testing.T) { +// A connect run does not change the sign-in, so --auth-flow alone stops it. +func TestSetupChosenSpaceWithOnlyAuthFlowStops(t *testing.T) { + cmd, _ := setupTestCmd(t, "2\ny\n") + require.NoError(t, cmd.Flags().Set(authFlowFlag, "iam")) + clients, w := twoSimilarSpaces() + opts, err := setupOptions(cmd) + require.NoError(t, err) + opts.LDAccessToken = "api-token" + + err = setupWith(cmd, clients, opts, true, true) + + require.EqualError(t, err, "only a full setup changes the operator app sign-in. To use --auth-flow on agent space LaunchDarkly (space-2), also pass --full-setup. "+ + "To only connect the MCP server, remove --auth-flow") + assert.Empty(t, w.calls) +} + +func TestSetupChosenSpaceWithoutFullSetupConnects(t *testing.T) { cmd, _ := setupTestCmd(t, "2\ny\n") clients, w := twoSimilarSpaces() @@ -391,9 +408,11 @@ func TestSetupChosenSpaceWithoutAuthFlowConnects(t *testing.T) { assert.NotContains(t, w.calls, "CreateRole") assert.Contains(t, stderrOf(cmd), "Using agent space LaunchDarkly (space-2)") assert.Contains(t, stderrOf(cmd), "No change. The operator app is off") + assert.Contains(t, stderrOf(cmd), "Setup: Connect the MCP server only.") + assert.Contains(t, stderrOf(cmd), "run setup again with --full-setup") assert.Contains(t, stdoutOf(cmd), "The operator app of agent space space-2 is off") assert.Contains(t, stdoutOf(cmd), "use the AWS console") - assert.NotContains(t, stdoutOf(cmd), "--auth-flow") + assert.NotContains(t, stdoutOf(cmd), "--full-setup") assert.NotContains(t, stdoutOf(cmd), "Open the DevOps Agent at") } diff --git a/cmd/aws_devops_agent/select_test.go b/cmd/aws_devops_agent/select_test.go index 54458388..a726dc5d 100644 --- a/cmd/aws_devops_agent/select_test.go +++ b/cmd/aws_devops_agent/select_test.go @@ -616,19 +616,19 @@ func TestResolveAgentSpaceIDWithUnreadTagsConnectsOnly(t *testing.T) { require.NoError(t, err) assert.False(t, provision) assert.Contains(t, out.String(), "could not read the tags of agent space launchdarkly (space-2), so it only connects the MCP server") - assert.Contains(t, out.String(), "--auth-flow") + assert.Contains(t, out.String(), "--full-setup") } -// --auth-flow forces the full setup on a space that setup did not make, and +// --full-setup forces the full setup on a space that setup did not make, and // setup warns about it before the plan. -func TestResolveAgentSpaceWarnsAboutAuthFlowOnAnotherSpace(t *testing.T) { +func TestResolveAgentSpaceWarnsAboutFullSetupOnAnotherSpace(t *testing.T) { for name, space := range map[string]agenttypes.AgentSpace{ "made by someone else": {AgentSpaceId: aws.String("space-2"), Name: aws.String("platform")}, "made by setup": ldcliSpace("space-2", "platform"), } { t.Run(name, func(t *testing.T) { cmd, out := testCmd("") - opts := awsdevops.SetupOptions{AgentSpaceID: "space-2", AuthFlowSet: true} + opts := awsdevops.SetupOptions{AgentSpaceID: "space-2", FullSetup: true} provision, err := resolveAgentSpace(cmd, newStubClients(space), &opts, false) diff --git a/cmd/aws_devops_agent/setup.go b/cmd/aws_devops_agent/setup.go index b8fac104..5e149f65 100644 --- a/cmd/aws_devops_agent/setup.go +++ b/cmd/aws_devops_agent/setup.go @@ -20,6 +20,7 @@ const ( agentSpaceNameFlag = "agent-space-name" newAgentSpaceFlag = "new-agent-space" authFlowFlag = "auth-flow" + fullSetupFlag = "full-setup" idcInstanceARNFlag = "idc-instance-arn" issuerURLFlag = "issuer-url" idpClientIDFlag = "idp-client-id" @@ -39,7 +40,7 @@ LaunchDarkly MCP server connection the AWS DevOps Agent needs. When the account already has agent spaces, setup asks which one to connect LaunchDarkly to. On a space that setup did not make, setup only registers the -MCP server. On a space that setup made, or with --auth-flow, setup runs the +MCP server. On a space that setup made, or with --full-setup, setup runs the full setup. Pass --agent-space-id to pick a space without the list, or --new-agent-space to provision another space. A new space needs a name: pass --agent-space-name, or type one when setup asks. The run ends with the @@ -61,10 +62,11 @@ for --access-token.`, cmd.Flags().String(regionFlag, "", "AWS region to provision in. Defaults to the region of the current AWS session") cmd.Flags().String(profileFlag, "", "Named AWS profile to use. Overrides AWS_PROFILE for this command") - cmd.Flags().String(agentSpaceIDFlag, "", "Use this agent space. --auth-flow runs full setup on a space that setup did not make") + cmd.Flags().String(agentSpaceIDFlag, "", "Use this agent space. --full-setup runs full setup on a space that setup did not make") cmd.Flags().String(agentSpaceNameFlag, "", "Name of the agent space to use or create. In a terminal, setup asks for a name if it needs one") cmd.Flags().Bool(newAgentSpaceFlag, false, "Provision a new agent space instead of asking which existing one to use") cmd.MarkFlagsMutuallyExclusive(agentSpaceIDFlag, newAgentSpaceFlag) + cmd.Flags().Bool(fullSetupFlag, false, "Run the full setup on an agent space that setup did not make. Setup can change its IAM roles, account association, and operator app sign-in") cmd.Flags().String(authFlowFlag, "iam", "Operator app sign-in method: iam, idc or idp") cmd.Flags().String(idcInstanceARNFlag, "", "IAM Identity Center instance ARN, required when --auth-flow=idc") cmd.Flags().String(issuerURLFlag, "", "OIDC issuer URL, required when --auth-flow=idp") @@ -132,6 +134,14 @@ func setupWith( if err != nil { return err } + if !provision && opts.AuthFlowSet { + // A connect run ignores --auth-flow, so stop and name --full-setup. + return fmt.Errorf( + "only a full setup changes the operator app sign-in. To use --%s on agent space %s (%s), also pass --%s. "+ + "To only connect the MCP server, remove --%s", + authFlowFlag, plan.Space.Name, plan.Space.ID, fullSetupFlag, authFlowFlag, + ) + } if plan.MCP.Action == awsdevops.MCPAsk && !ask { return fmt.Errorf("create a service token at %s, then pass --access-token", awsdevops.AccessTokenURL(opts.LDBaseURI)) } @@ -189,7 +199,7 @@ func setupWith( // resolveAgentSpace picks the agent space for this run. If no flag picks one, // it asks in a terminal. A space that setup made gets the full setup. Another -// space gets only the MCP server, unless --auth-flow is passed. It returns true +// space gets only the MCP server, unless --full-setup is passed. It returns true // if setup must provision a new space, and then sets opts.AgentSpaceName. // Otherwise, it sets opts.AgentSpaceID. func resolveAgentSpace( @@ -211,12 +221,12 @@ func resolveAgentSpace( ) case err != nil: return false, err - case space.TagErr != nil && !opts.AuthFlowSet: + case space.TagErr != nil && !opts.FullSetup: // The owner is unknown, so do only what is safe on any space. _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Setup could not read the tags of agent space %s (%s), so it only connects the MCP server. "+ "To run the full setup on it, pass --%s: %v\n", - space.Name, space.AgentSpaceID, authFlowFlag, space.TagErr) + space.Name, space.AgentSpaceID, fullSetupFlag, space.TagErr) return false, nil } @@ -246,7 +256,7 @@ func resolveAgentSpace( if !interactive { provision, err := pickWithoutAsking(spaces, opts, clients.Region) if err == nil && opts.AgentSpaceID != "" { - warnAuthFlow(cmd, *opts, summaryOf(spaces, opts.AgentSpaceID)) + warnFullSetup(cmd, *opts, summaryOf(spaces, opts.AgentSpaceID)) } return provision, err @@ -268,24 +278,24 @@ func resolveAgentSpace( } // fullSetupOn reports whether the chosen space gets the full setup. A space -// that setup made gets it. --auth-flow forces it on another space, with a +// that setup made gets it. --full-setup forces it on another space, with a // warning. func fullSetupOn(cmd *cobra.Command, opts awsdevops.SetupOptions, space awsdevops.AgentSpaceSummary) bool { - warnAuthFlow(cmd, opts, space) + warnFullSetup(cmd, opts, space) - return opts.AuthFlowSet || space.ManagedByLDCLI + return opts.FullSetup || space.ManagedByLDCLI } -// warnAuthFlow warns that --auth-flow runs the full setup on a space that +// warnFullSetup warns that --full-setup runs the full setup on a space that // setup did not make, so that a wrong --agent-space-id shows before the plan. -func warnAuthFlow(cmd *cobra.Command, opts awsdevops.SetupOptions, space awsdevops.AgentSpaceSummary) { - if !opts.AuthFlowSet || space.ManagedByLDCLI { +func warnFullSetup(cmd *cobra.Command, opts awsdevops.SetupOptions, space awsdevops.AgentSpaceSummary) { + if !opts.FullSetup || space.ManagedByLDCLI { return } _, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: setup did not make agent space %s (%s). --%s runs the full setup on it, "+ "so setup can change its IAM roles, account association, and operator app sign-in.\n", - space.Name, space.AgentSpaceID, authFlowFlag) + space.Name, space.AgentSpaceID, fullSetupFlag) } // summaryOf returns the summary of the space with this ID. @@ -327,7 +337,7 @@ func pickWithoutAsking(all []awsdevops.AgentSpaceSummary, opts *awsdevops.SetupO if i := spaceNamed(spaces, name); i >= 0 { opts.AgentSpaceID = spaces[i].AgentSpaceID - return opts.AuthFlowSet || spaces[i].ManagedByLDCLI, nil + return opts.FullSetup || spaces[i].ManagedByLDCLI, nil } if !hasSpaceNamed(spaces, name) { opts.NewAgentSpace = true @@ -489,6 +499,7 @@ func confirmSetup(cmd *cobra.Command, plan awsdevops.SetupPlan, ask bool) error _, _ = fmt.Fprintf(out, " Region: %s\n", plan.Region) _, _ = fmt.Fprintf(out, " AWS profile: %s\n", profileLabel(cmd)) text := plan.Text() + _, _ = fmt.Fprintf(out, " Setup: %s\n", text.Scope) for _, role := range text.Roles { _, _ = fmt.Fprintf(out, " IAM role: %s\n", role) } @@ -672,6 +683,7 @@ func setupOptions(cmd *cobra.Command) (awsdevops.SetupOptions, error) { AgentSpaceName: mustString(cmd, agentSpaceNameFlag), AuthFlow: strings.ToLower(mustString(cmd, authFlowFlag)), AuthFlowSet: cmd.Flags().Changed(authFlowFlag), + FullSetup: mustBool(cmd, fullSetupFlag), IdcInstanceARN: mustString(cmd, idcInstanceARNFlag), IssuerURL: mustString(cmd, issuerURLFlag), IdpClientID: mustString(cmd, idpClientIDFlag), diff --git a/internal/awsdevops/plan_test.go b/internal/awsdevops/plan_test.go index 6fa2e5ae..10405515 100644 --- a/internal/awsdevops/plan_test.go +++ b/internal/awsdevops/plan_test.go @@ -268,6 +268,8 @@ func TestPlanTextOfAConnectPlan(t *testing.T) { text := plan.Text() + assert.Contains(t, text.Scope, "Connect the MCP server only") + assert.Contains(t, text.Scope, "--full-setup") assert.Equal(t, "Connect to agent space platform (space-7)", text.AgentSpace) assert.Equal(t, "No change", text.Association) assert.Equal(t, []string{"No change"}, text.Roles) diff --git a/internal/awsdevops/setup.go b/internal/awsdevops/setup.go index 42a6a7cd..b4a293b5 100644 --- a/internal/awsdevops/setup.go +++ b/internal/awsdevops/setup.go @@ -94,6 +94,8 @@ type SetupOptions struct { AgentSpaceID string AgentSpaceName string NewAgentSpace bool + // FullSetup: the user passed --full-setup. + FullSetup bool AuthFlow string // AuthFlowSet: the user passed --auth-flow. @@ -400,6 +402,8 @@ type SetupPlan struct { // PlanText is the plan as lines for a person to read. type PlanText struct { + // Scope tells if the run is a full setup. + Scope string AgentSpace string Association string Roles []string @@ -409,8 +413,10 @@ type PlanText struct { // Text describes each step of the plan. func (p SetupPlan) Text() PlanText { - text := PlanText{AgentSpace: p.Space.describe(), MCPServer: p.MCP.describe()} + text := PlanText{Scope: "Full setup", AgentSpace: p.Space.describe(), MCPServer: p.MCP.describe()} if p.Space.Action == SpaceConnect { + text.Scope = "Connect the MCP server only. To also update the IAM roles, " + + "account association, and operator app of the space, run setup again with --full-setup" text.Association = "No change" text.Roles = []string{"No change"} text.OperatorApp = "No change"