From 76e1336916698e855474c8cd1b6e4f6e9663ea56 Mon Sep 17 00:00:00 2001 From: Aiden Carpenter Date: Mon, 13 Nov 2023 15:53:24 -0800 Subject: [PATCH 01/14] add flag and assume role method --- internal/pkg/cli/flag.go | 18 ++++--- internal/pkg/cli/run_local.go | 97 +++++++++++++++++++++++++++++++---- 2 files changed, 98 insertions(+), 17 deletions(-) diff --git a/internal/pkg/cli/flag.go b/internal/pkg/cli/flag.go index 126c91d9ab6..9e5cd036d0a 100644 --- a/internal/pkg/cli/flag.go +++ b/internal/pkg/cli/flag.go @@ -67,11 +67,12 @@ const ( containerFlag = "container" // Run local flags - portOverrideFlag = "port-override" - envVarOverrideFlag = "env-var-override" - proxyFlag = "proxy" - proxyNetworkFlag = "proxy-network" - watchFlag = "watch" + portOverrideFlag = "port-override" + envVarOverrideFlag = "env-var-override" + proxyFlag = "proxy" + proxyNetworkFlag = "proxy-network" + watchFlag = "watch" + retrieveTaskRoleFlag = "task-role" // Flags for CI/CD. githubURLFlag = "github-url" @@ -323,9 +324,10 @@ Defaults to all logs. Only one of end-time / follow may be used.` Format: [container]:KEY=VALUE. Omit container name to apply to all containers.` portOverridesFlagDescription = `Optional. Override ports exposed by service. Format: :. Example: --port-override 5000:80 binds localhost:5000 to the service's port 80.` - proxyFlagDescription = `Optional. Proxy outbound requests to your environment's VPC.` - proxyNetworkFlagDescription = `Optional. Set the IP Network used by --proxy.` - watchFlagDescription = `Optional. Watch changes to local files and restart containers when updated.` + proxyFlagDescription = `Optional. Proxy outbound requests to your environment's VPC.` + proxyNetworkFlagDescription = `Optional. Set the IP Network used by --proxy.` + watchFlagDescription = `Optional. Watch changes to local files and restart containers when updated.` + retrieveTaskRoleFlagDescription = "Optional. Run containers with TaskRole credentials instead of session credentials." svcManifestFlagDescription = `Optional. Name of the environment in which the service was deployed; output the manifest file used for that deployment.` diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index 712e86c9710..fff31792417 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -92,15 +92,16 @@ type recursiveWatcher interface { } type runLocalVars struct { - wkldName string - wkldType string - appName string - envName string - envOverrides map[string]string - watch bool - portOverrides portOverrides - proxy bool - proxyNetwork net.IPNet + wkldName string + wkldType string + appName string + envName string + envOverrides map[string]string + watch bool + retrieveTaskRole bool + portOverrides portOverrides + proxy bool + proxyNetwork net.IPNet } type runLocalOpts struct { @@ -436,6 +437,35 @@ func (o *runLocalOpts) getTask(ctx context.Context) (orchestrator.Task, error) { return orchestrator.Task{}, fmt.Errorf("get env vars: %w", err) } + if o.retrieveTaskRole { + taskRoleCredsVars, err := o.taskRoleCredentials(ctx) + if err != nil { + log.Warning("TaskRole retrieval failed. ", + "You can manually add permissions for your account to assume TaskRole permissions by adding the following YAML override to your service:\n", + `- op: add + path: /Resources/TaskRole/Properties/AssumeRolePolicyDocument/Statement/- + value: + Effect: Allow + Principal: + AWS: "arn:aws:iam::[account-ID]:root" + Action: 'sts:AssumeRole' +`, + "\n", + "For more information on YAML overrides see https://aws.github.io/copilot-cli/docs/developing/overrides/yamlpatch/") + return orchestrator.Task{}, fmt.Errorf("retrieve TaskRole credentials: %w", err) + } + + // overwrite environment variables + for ctr := range envVars { + for k, v := range taskRoleCredsVars { + envVars[ctr][k] = envVarValue{ + Value: v, + Secret: true, + } + } + } + } + task := orchestrator.Task{ Containers: make(map[string]orchestrator.ContainerDefinition, len(td.ContainerDefinitions)), } @@ -612,6 +642,54 @@ func sessionEnvVars(ctx context.Context, sess *session.Session) (map[string]stri return env, nil } +func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]string, error) { + // assumeRoleMethod tries to directly call sts:AssumeRole for TaskRole using default session + assumeRoleMethod := func() (map[string]string, error) { + taskDef, err := o.ecsClient.TaskDefinition(o.appName, o.envName, o.wkldName) + if err != nil { + return nil, err + } + + taskRoleSess, err := o.sessProvider.FromRole(aws.StringValue(taskDef.TaskRoleArn), o.targetEnv.Region) + if err != nil { + return nil, err + } + + creds, err := taskRoleSess.Config.Credentials.GetWithContext(ctx) + if err != nil { + return nil, err + } + + return map[string]string{ + "AWS_ACCESS_KEY_ID": creds.AccessKeyID, + "AWS_SECRET_ACCESS_KEY": creds.SecretAccessKey, + "AWS_SESSION_TOKEN": creds.SessionToken, + }, nil + } + + // ecsExecMethod tries to use ECS Exec to retrive credentials from running container + ecsExecMethod := func() (map[string]string, error) { + return nil, errors.New("ecs exec method not implemented") + } + + credentialsChain := []func() (map[string]string, error){ + assumeRoleMethod, + ecsExecMethod, + } + + // return TaskRole credentials from first succesful method + var errs []error + for _, method := range credentialsChain { + vars, err := method() + if err == nil { + return vars, nil + } + errs = append(errs, err) + } + + return nil, errors.Join(errs...) +} + type containerEnv map[string]envVarValue type envVarValue struct { @@ -961,6 +1039,7 @@ func BuildRunLocalCmd() *cobra.Command { cmd.Flags().StringVarP(&vars.envName, envFlag, envFlagShort, "", envFlagDescription) cmd.Flags().StringVarP(&vars.appName, appFlag, appFlagShort, tryReadingAppName(), appFlagDescription) cmd.Flags().BoolVar(&vars.watch, watchFlag, false, watchFlagDescription) + cmd.Flags().BoolVar(&vars.retrieveTaskRole, retrieveTaskRoleFlag, false, retrieveTaskRoleFlagDescription) cmd.Flags().Var(&vars.portOverrides, portOverrideFlag, portOverridesFlagDescription) cmd.Flags().StringToStringVar(&vars.envOverrides, envVarOverrideFlag, nil, envVarOverrideFlagDescription) cmd.Flags().BoolVar(&vars.proxy, proxyFlag, false, proxyFlagDescription) From f349d28b9b52241daa827151d1d22535ed717e37 Mon Sep 17 00:00:00 2001 From: Aiden Carpenter Date: Tue, 14 Nov 2023 11:13:01 -0800 Subject: [PATCH 02/14] create error, disable unfinished feature --- internal/pkg/cli/errors.go | 22 ++++++++++++++++++++++ internal/pkg/cli/run_local.go | 17 ++--------------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/internal/pkg/cli/errors.go b/internal/pkg/cli/errors.go index bcfa606e2e4..b30e47e7384 100644 --- a/internal/pkg/cli/errors.go +++ b/internal/pkg/cli/errors.go @@ -163,3 +163,25 @@ func (e *errPipelineDependsOnEnv) RecommendActions() string { or run %s to delete the pipeline before running %s to delete the environment`, e.pipeline, e.env, color.HighlightCode(fmt.Sprintf("copilot pipeline delete -n %s", e.pipeline)), color.HighlightCode(fmt.Sprintf("copilot env delete -n %s", e.env))) } + +type errTaskRoleRetrievalFailed struct { + chainErrs []error +} + +func (e *errTaskRoleRetrievalFailed) Error() string { + return errors.Join(e.chainErrs...).Error() +} + +func (e *errTaskRoleRetrievalFailed) RecommendActions() string { + return fmt.Sprintf(`TaskRole retrieval failed. You can manually add permissions for your account to assume TaskRole permissions by adding the following YAML override to your service: +%s +For more information on YAML overrides see %s`, + color.HighlightCodeBlock(`- op: add + path: /Resources/TaskRole/Properties/AssumeRolePolicyDocument/Statement/- + value: + Effect: Allow + Principal: + AWS: "arn:aws:iam::[account-ID]:root" + Action: 'sts:AssumeRole'`), + color.Emphasize("https://aws.github.io/copilot-cli/docs/developing/overrides/yamlpatch/")) +} diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index fff31792417..9b58f084d07 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -440,18 +440,6 @@ func (o *runLocalOpts) getTask(ctx context.Context) (orchestrator.Task, error) { if o.retrieveTaskRole { taskRoleCredsVars, err := o.taskRoleCredentials(ctx) if err != nil { - log.Warning("TaskRole retrieval failed. ", - "You can manually add permissions for your account to assume TaskRole permissions by adding the following YAML override to your service:\n", - `- op: add - path: /Resources/TaskRole/Properties/AssumeRolePolicyDocument/Statement/- - value: - Effect: Allow - Principal: - AWS: "arn:aws:iam::[account-ID]:root" - Action: 'sts:AssumeRole' -`, - "\n", - "For more information on YAML overrides see https://aws.github.io/copilot-cli/docs/developing/overrides/yamlpatch/") return orchestrator.Task{}, fmt.Errorf("retrieve TaskRole credentials: %w", err) } @@ -682,12 +670,12 @@ func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]stri for _, method := range credentialsChain { vars, err := method() if err == nil { - return vars, nil + return vars, &errTaskRoleRetrievalFailed{} } errs = append(errs, err) } - return nil, errors.Join(errs...) + return nil, &errTaskRoleRetrievalFailed{errs} } type containerEnv map[string]envVarValue @@ -1039,7 +1027,6 @@ func BuildRunLocalCmd() *cobra.Command { cmd.Flags().StringVarP(&vars.envName, envFlag, envFlagShort, "", envFlagDescription) cmd.Flags().StringVarP(&vars.appName, appFlag, appFlagShort, tryReadingAppName(), appFlagDescription) cmd.Flags().BoolVar(&vars.watch, watchFlag, false, watchFlagDescription) - cmd.Flags().BoolVar(&vars.retrieveTaskRole, retrieveTaskRoleFlag, false, retrieveTaskRoleFlagDescription) cmd.Flags().Var(&vars.portOverrides, portOverrideFlag, portOverridesFlagDescription) cmd.Flags().StringToStringVar(&vars.envOverrides, envVarOverrideFlag, nil, envVarOverrideFlagDescription) cmd.Flags().BoolVar(&vars.proxy, proxyFlag, false, proxyFlagDescription) From 74024b07b72f9f63844ae3081b3547006831b7c4 Mon Sep 17 00:00:00 2001 From: Aiden Carpenter Date: Tue, 14 Nov 2023 12:20:15 -0800 Subject: [PATCH 03/14] tests --- internal/pkg/cli/run_local.go | 10 +-- internal/pkg/cli/run_local_test.go | 104 +++++++++++++++++++++++++++-- 2 files changed, 102 insertions(+), 12 deletions(-) diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index 9b58f084d07..da320ea3c0a 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -643,16 +643,12 @@ func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]stri return nil, err } - creds, err := taskRoleSess.Config.Credentials.GetWithContext(ctx) + creds, err := sessionEnvVars(ctx, taskRoleSess) if err != nil { return nil, err } - return map[string]string{ - "AWS_ACCESS_KEY_ID": creds.AccessKeyID, - "AWS_SECRET_ACCESS_KEY": creds.SecretAccessKey, - "AWS_SESSION_TOKEN": creds.SessionToken, - }, nil + return creds, nil } // ecsExecMethod tries to use ECS Exec to retrive credentials from running container @@ -670,7 +666,7 @@ func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]stri for _, method := range credentialsChain { vars, err := method() if err == nil { - return vars, &errTaskRoleRetrievalFailed{} + return vars, nil } errs = append(errs, err) } diff --git a/internal/pkg/cli/run_local_test.go b/internal/pkg/cli/run_local_test.go index b300a50f22f..b7fe1d2d4d3 100644 --- a/internal/pkg/cli/run_local_test.go +++ b/internal/pkg/cli/run_local_test.go @@ -203,6 +203,7 @@ type runLocalExecuteMocks struct { ecsClient *mocks.MockecsClient store *mocks.Mockstore sessCreds credentials.Provider + sessProvider *mocks.MocksessionProvider interpolator *mocks.Mockinterpolator ws *mocks.MockwsWlDirReader mockMft *mockWorkloadMft @@ -268,6 +269,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { } taskDef := &awsecs.TaskDefinition{ + TaskRoleArn: aws.String("mock-arn"), ContainerDefinitions: []*sdkecs.ContainerDefinition{ { Name: aws.String("foo"), @@ -320,6 +322,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { }, } alteredTaskDef := &awsecs.TaskDefinition{ + TaskRoleArn: aws.String("mock-arn"), ContainerDefinitions: []*sdkecs.ContainerDefinition{ { Name: aws.String("foo"), @@ -415,6 +418,46 @@ func TestRunLocalOpts_Execute(t *testing.T) { "AWS_SESSION_TOKEN": "myEnvToken", }, } + expectedTaskRoleTask := orchestrator.Task{ + Containers: map[string]orchestrator.ContainerDefinition{ + "foo": { + ImageURI: "image1", + EnvVars: map[string]string{ + "FOO_VAR": "foo-value", + }, + Secrets: map[string]string{ + "SHARED_SECRET": "secretvalue", + "AWS_ACCESS_KEY_ID": "taskRoleID", + "AWS_SECRET_ACCESS_KEY": "taskRoleSecret", + "AWS_SESSION_TOKEN": "taskRoleToken", + "AWS_DEFAULT_REGION": testRegion, + "AWS_REGION": testRegion, + }, + Ports: map[string]string{ + "80": "8080", + "999": "9999", + }, + }, + "bar": { + ImageURI: "image2", + EnvVars: map[string]string{ + "BAR_VAR": "bar-value", + }, + Secrets: map[string]string{ + "SHARED_SECRET": "secretvalue", + "AWS_ACCESS_KEY_ID": "taskRoleID", + "AWS_SECRET_ACCESS_KEY": "taskRoleSecret", + "AWS_SESSION_TOKEN": "taskRoleToken", + "AWS_DEFAULT_REGION": testRegion, + "AWS_REGION": testRegion, + }, + Ports: map[string]string{ + "777": "7777", + "10000": "10000", + }, + }, + }, + } testCases := map[string]struct { inputAppName string @@ -423,6 +466,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { inputEnvOverrides map[string]string inputPortOverrides []string inputWatch bool + inputTaskRole bool inputProxy bool buildImagesError error @@ -453,6 +497,20 @@ func TestRunLocalOpts_Execute(t *testing.T) { }, wantedError: errors.New(`get task: get env vars: parse env overrides: "bad:OVERRIDE" targets invalid container`), }, + "error retrieving TaskRole credentials": { + inputAppName: testAppName, + inputWkldName: testWkldName, + inputEnvName: testEnvName, + inputTaskRole: true, + setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { + m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil) + m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.sessProvider.EXPECT().FromRole("mock-arn", testRegion).Return(nil, errors.New("some error")) + }, + wantedError: errors.New(`get task: retrieve TaskRole credentials: some error +ecs exec method not implemented`), + }, "error reading workload manifest": { inputAppName: testAppName, inputWkldName: testWkldName, @@ -729,6 +787,39 @@ func TestRunLocalOpts_Execute(t *testing.T) { } }, }, + "success, one run task call, taskrole assumerole method": { + inputAppName: testAppName, + inputWkldName: testWkldName, + inputEnvName: testEnvName, + inputTaskRole: true, + setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { + m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil) + m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + taskRoleSess := &session.Session{ + Config: &aws.Config{ + Credentials: credentials.NewStaticCredentials("taskRoleID", "taskRoleSecret", "taskRoleToken"), + Region: aws.String(testRegion), + }, + } + m.sessProvider.EXPECT().FromRole("mock-arn", testRegion).Return(taskRoleSess, nil) + m.ws.EXPECT().ReadWorkloadManifest(testWkldName).Return([]byte(""), nil) + m.interpolator.EXPECT().Interpolate("").Return("", nil) + + errCh := make(chan error, 1) + m.orchestrator.StartFn = func() <-chan error { + errCh <- errors.New("some error") + return errCh + } + m.orchestrator.RunTaskFn = func(task orchestrator.Task, opts ...orchestrator.RunTaskOption) { + require.Equal(t, expectedTaskRoleTask, task) + } + m.orchestrator.StopFn = func() { + require.Len(t, errCh, 0) + close(errCh) + } + }, + }, "handles ctrl-c, waits to get all errors": { inputAppName: testAppName, inputWkldName: testWkldName, @@ -945,6 +1036,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { ssm: mocks.NewMocksecretGetter(ctrl), secretsManager: mocks.NewMocksecretGetter(ctrl), store: mocks.NewMockstore(ctrl), + sessProvider: mocks.NewMocksessionProvider(ctrl), interpolator: mocks.NewMockinterpolator(ctrl), ws: mocks.NewMockwsWlDirReader(ctrl), mockRunner: mocks.NewMockexecRunner(ctrl), @@ -959,11 +1051,12 @@ func TestRunLocalOpts_Execute(t *testing.T) { tc.setupMocks(t, m) opts := runLocalOpts{ runLocalVars: runLocalVars{ - appName: tc.inputAppName, - wkldName: tc.inputWkldName, - envName: tc.inputEnvName, - envOverrides: tc.inputEnvOverrides, - watch: tc.inputWatch, + appName: tc.inputAppName, + wkldName: tc.inputWkldName, + envName: tc.inputEnvName, + envOverrides: tc.inputEnvOverrides, + watch: tc.inputWatch, + retrieveTaskRole: tc.inputTaskRole, portOverrides: portOverrides{ { host: "777", @@ -993,6 +1086,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { ssm: m.ssm, secretsManager: m.secretsManager, store: m.store, + sessProvider: m.sessProvider, sess: &session.Session{ Config: &aws.Config{ Credentials: credentials.NewStaticCredentials("myID", "mySecret", "myToken"), From 9dc03fc412cccda976bf56ad19a583b6e327c6d9 Mon Sep 17 00:00:00 2001 From: Aiden Carpenter Date: Thu, 16 Nov 2023 11:55:37 -0800 Subject: [PATCH 04/14] attempt aws ssm start-session --- internal/pkg/cli/run_local.go | 93 ++++++++++++++++++++++++++++++++++- 1 file changed, 92 insertions(+), 1 deletion(-) diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index da320ea3c0a..98e19f89c00 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -4,11 +4,14 @@ package cli import ( + "bytes" "context" "errors" "fmt" + "io" "net" "os" + osexec "os/exec" "os/signal" "path/filepath" "slices" @@ -653,7 +656,42 @@ func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]stri // ecsExecMethod tries to use ECS Exec to retrive credentials from running container ecsExecMethod := func() (map[string]string, error) { - return nil, errors.New("ecs exec method not implemented") + svcDesc, err := ecs.New(o.sess).DescribeService(o.appName, o.envName, o.wkldName) + if err != nil { + return nil, fmt.Errorf("describe ECS service for %s in environment %s: %w", o.wkldName, o.envName, err) + } + + // try exec on each task + for _, task := range svcDesc.Tasks { + fmt.Printf("Task ARN: %s\n", aws.StringValue(task.TaskArn)) + id, err := awsecs.TaskID(aws.StringValue(task.TaskArn)) + if err != nil { + continue + } + fmt.Printf("Task ID: %s\n", id) + + awsContainerCredentialsRelativeURI, err := agentEndpoint(ctx, id) + if err != nil { + // continue + return nil, err + } + fmt.Printf("Endpoint Relative URI: %s\n", awsContainerCredentialsRelativeURI) + + err = awsecs.New(o.sess).ExecuteCommand(awsecs.ExecuteCommandInput{ + Cluster: svcDesc.ClusterName, + Command: fmt.Sprintf("curl 169.254.170.2%s", awsContainerCredentialsRelativeURI), + Task: id, + Container: o.wkldName, + }) + if err != nil { + continue + } + + fmt.Println("found!") + return nil, nil + } + + return nil, errors.New("fail") } credentialsChain := []func() (map[string]string, error){ @@ -674,6 +712,58 @@ func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]stri return nil, &errTaskRoleRetrievalFailed{errs} } +func (o *runLocalOpts) agentEndpoint(ctx context.Context, taskID string) (string, error) { + cmd := osexec.CommandContext(ctx, "aws", "--region", o.targetEnv.Region, "ssm", "start-session", "--target", taskID) + out := &bytes.Buffer{} + cmd.Stdout, cmd.Stderr = out, out + + stdin, err := cmd.StdinPipe() + if err != nil { + return "", fmt.Errorf("stdin pipe: %w", err) + } + + var url string + done := make(chan struct{}) + + go func() { + // keep stdin open until we've found the URI + defer stdin.Close() + io.WriteString(stdin, "curl 169.254.170.2$AWS_CONTAINER_CREDENTIALS_RELATIVE_URI\n") + io.WriteString(stdin, "exit\n") + <-done + }() + + go func() { + // parse stdout/err until we find the uri + defer close(done) + + for url == "" { + time.Sleep(100 * time.Millisecond) + lines := out.String() + + for _, line := range strings.Split(lines, "\n") { + fmt.Println(line) + if !strings.Contains(line, "LOCALRUNURI: ") || strings.Contains(line, "$AWS_CONTAINER_CREDENTIALS_RELATIVE_URI") { + continue + } + + split := strings.Fields(line) + url = strings.TrimSpace(split[2]) + return + } + } + }() + + if err := cmd.Run(); err != nil { + return "", err + } + if url == "" { + return "", fmt.Errorf("url not found") + } + + return url, nil +} + type containerEnv map[string]envVarValue type envVarValue struct { @@ -1023,6 +1113,7 @@ func BuildRunLocalCmd() *cobra.Command { cmd.Flags().StringVarP(&vars.envName, envFlag, envFlagShort, "", envFlagDescription) cmd.Flags().StringVarP(&vars.appName, appFlag, appFlagShort, tryReadingAppName(), appFlagDescription) cmd.Flags().BoolVar(&vars.watch, watchFlag, false, watchFlagDescription) + cmd.Flags().BoolVar(&vars.retrieveTaskRole, retrieveTaskRoleFlag, false, retrieveTaskRoleFlagDescription) cmd.Flags().Var(&vars.portOverrides, portOverrideFlag, portOverridesFlagDescription) cmd.Flags().StringToStringVar(&vars.envOverrides, envVarOverrideFlag, nil, envVarOverrideFlagDescription) cmd.Flags().BoolVar(&vars.proxy, proxyFlag, false, proxyFlagDescription) From 3efda40eb48e722e65f5d3a4ec01804f7cee6212 Mon Sep 17 00:00:00 2001 From: Aiden Carpenter Date: Fri, 1 Dec 2023 14:26:33 -0800 Subject: [PATCH 05/14] ecs exec method --- internal/pkg/cli/run_local.go | 165 ++++++++++++++++++---------------- 1 file changed, 89 insertions(+), 76 deletions(-) diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index 98e19f89c00..7ea87045bfe 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -6,12 +6,12 @@ package cli import ( "bytes" "context" + "encoding/json" "errors" "fmt" "io" "net" "os" - osexec "os/exec" "os/signal" "path/filepath" "slices" @@ -68,6 +68,10 @@ const ( workloadAskPrompt = "Which workload would you like to run locally?" ) +const ( + curlContainerCredentialsCmd = "curl 169.254.170.2$AWS_CONTAINER_CREDENTIALS_RELATIVE_URI" +) + type containerOrchestrator interface { Start() <-chan error RunTask(orchestrator.Task, ...orchestrator.RunTaskOption) @@ -112,6 +116,7 @@ type runLocalOpts struct { sel deploySelector ecsClient ecsClient + ecsExecutor ecsCommandExecutor ssm secretGetter secretsManager secretGetter sessProvider sessionProvider @@ -130,6 +135,7 @@ type runLocalOpts struct { envChecker versionCompatibilityChecker debounceTime time.Duration newRecursiveWatcher func() (recursiveWatcher, error) + outputCapturer *os.File buildContainerImages func(mft manifest.DynamicWorkload) (map[string]string, error) configureClients func() error @@ -187,6 +193,7 @@ func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) { // so use the default sess and *hope* they have permissions. o.ecsClient = ecs.New(o.envManagerSess) o.ssm = ssm.New(o.envManagerSess) + o.ecsExecutor = awsecs.New(o.envManagerSess) o.secretsManager = secretsmanager.New(defaultSessEnvRegion) resources, err := cloudformation.New(o.sess, cloudformation.WithProgressTracker(os.Stderr)).GetAppResourcesByRegion(o.targetApp, o.targetEnv.Region) @@ -259,6 +266,7 @@ func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) { o.newRecursiveWatcher = func() (recursiveWatcher, error) { return file.NewRecursiveWatcher(0) } + o.outputCapturer = os.Stdout return o, nil } @@ -656,42 +664,99 @@ func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]stri // ecsExecMethod tries to use ECS Exec to retrive credentials from running container ecsExecMethod := func() (map[string]string, error) { - svcDesc, err := ecs.New(o.sess).DescribeService(o.appName, o.envName, o.wkldName) + svcDesc, err := o.ecsClient.DescribeService(o.appName, o.envName, o.wkldName) if err != nil { return nil, fmt.Errorf("describe ECS service for %s in environment %s: %w", o.wkldName, o.envName, err) } - // try exec on each task + // capture stdout + stdoutReader, stdoutWriter, err := os.Pipe() + if err != nil { + return nil, fmt.Errorf("create os pipe: %w", err) + } + stdoutPointer := o.outputCapturer + defer func() { + o.outputCapturer = stdoutPointer + }() + o.outputCapturer = stdoutWriter + + // try exec on each container within the service + var wg sync.WaitGroup for _, task := range svcDesc.Tasks { - fmt.Printf("Task ARN: %s\n", aws.StringValue(task.TaskArn)) - id, err := awsecs.TaskID(aws.StringValue(task.TaskArn)) + taskID, err := awsecs.TaskID(aws.StringValue(task.TaskArn)) if err != nil { - continue - } - fmt.Printf("Task ID: %s\n", id) - - awsContainerCredentialsRelativeURI, err := agentEndpoint(ctx, id) - if err != nil { - // continue return nil, err } - fmt.Printf("Endpoint Relative URI: %s\n", awsContainerCredentialsRelativeURI) - err = awsecs.New(o.sess).ExecuteCommand(awsecs.ExecuteCommandInput{ - Cluster: svcDesc.ClusterName, - Command: fmt.Sprintf("curl 169.254.170.2%s", awsContainerCredentialsRelativeURI), - Task: id, - Container: o.wkldName, - }) - if err != nil { - continue + for _, container := range task.Containers { + wg.Add(1) + containerName := aws.StringValue(container.Name) + go func() { + defer wg.Done() + o.ecsExecutor.ExecuteCommand(awsecs.ExecuteCommandInput{ + Cluster: svcDesc.ClusterName, + Command: fmt.Sprintf("/bin/sh -c %q\n", curlContainerCredentialsCmd), + Task: taskID, + Container: containerName, + }) + }() } + } - fmt.Println("found!") - return nil, nil + // wait for containers to finish and reset stdout + containersFinished := make(chan struct{}) + go func() { + wg.Wait() + stdoutWriter.Close() + o.outputCapturer = stdoutPointer + close(containersFinished) + }() + + type containerCredentialsOutput struct { + AccessKeyId string + SecretAccessKey string + Token string } - return nil, errors.New("fail") + // parse stdout to try and find credentials + credsResult := make(chan map[string]string) + parseErr := make(chan error) + go func() { + select { + case <-containersFinished: + buf, err := io.ReadAll(stdoutReader) + if err != nil { + parseErr <- err + return + } + lines := bytes.Split(buf, []byte("\n")) + var creds containerCredentialsOutput + for _, line := range lines { + err := json.Unmarshal(line, &creds) + if err != nil { + continue + } + credsResult <- map[string]string{ + "AWS_ACCESS_KEY_ID": creds.AccessKeyId, + "AWS_SECRET_ACCESS_KEY": creds.SecretAccessKey, + "AWS_SESSION_TOKEN": creds.Token, + } + return + } + case <-ctx.Done(): + return + } + parseErr <- errors.New("all containers failed to retrieve credentials") + }() + + select { + case creds := <-credsResult: + return creds, nil + case <-ctx.Done(): + return nil, ctx.Err() + case err := <-parseErr: + return nil, err + } } credentialsChain := []func() (map[string]string, error){ @@ -712,58 +777,6 @@ func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]stri return nil, &errTaskRoleRetrievalFailed{errs} } -func (o *runLocalOpts) agentEndpoint(ctx context.Context, taskID string) (string, error) { - cmd := osexec.CommandContext(ctx, "aws", "--region", o.targetEnv.Region, "ssm", "start-session", "--target", taskID) - out := &bytes.Buffer{} - cmd.Stdout, cmd.Stderr = out, out - - stdin, err := cmd.StdinPipe() - if err != nil { - return "", fmt.Errorf("stdin pipe: %w", err) - } - - var url string - done := make(chan struct{}) - - go func() { - // keep stdin open until we've found the URI - defer stdin.Close() - io.WriteString(stdin, "curl 169.254.170.2$AWS_CONTAINER_CREDENTIALS_RELATIVE_URI\n") - io.WriteString(stdin, "exit\n") - <-done - }() - - go func() { - // parse stdout/err until we find the uri - defer close(done) - - for url == "" { - time.Sleep(100 * time.Millisecond) - lines := out.String() - - for _, line := range strings.Split(lines, "\n") { - fmt.Println(line) - if !strings.Contains(line, "LOCALRUNURI: ") || strings.Contains(line, "$AWS_CONTAINER_CREDENTIALS_RELATIVE_URI") { - continue - } - - split := strings.Fields(line) - url = strings.TrimSpace(split[2]) - return - } - } - }() - - if err := cmd.Run(); err != nil { - return "", err - } - if url == "" { - return "", fmt.Errorf("url not found") - } - - return url, nil -} - type containerEnv map[string]envVarValue type envVarValue struct { From d18afec7e1ef09e8983b409cc41f764c9d576b89 Mon Sep 17 00:00:00 2001 From: Aiden Carpenter Date: Tue, 5 Dec 2023 13:26:11 -0800 Subject: [PATCH 06/14] refactor output capturing, write tests, wrap errs with chain method --- internal/pkg/cli/run_local.go | 56 +++++++++++----- internal/pkg/cli/run_local_test.go | 103 +++++++++++++++++++++++++---- 2 files changed, 132 insertions(+), 27 deletions(-) diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index bf7b58ac8ca..fa5a4651a27 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -135,13 +135,15 @@ type runLocalOpts struct { envChecker versionCompatibilityChecker debounceTime time.Duration newRecursiveWatcher func() (recursiveWatcher, error) - outputCapturer *os.File buildContainerImages func(mft manifest.DynamicWorkload) (map[string]string, error) configureClients func() error labeledTermPrinter func(fw syncbuffer.FileWriter, bufs []*syncbuffer.LabeledSyncBuffer, opts ...syncbuffer.LabeledTermPrinterOption) clideploy.LabeledTermPrinter unmarshal func([]byte) (manifest.DynamicWorkload, error) newInterpolator func(app, env string) interpolator + + captureStdout func() (io.Reader, error) + releaseStdout func() } func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) { @@ -266,7 +268,32 @@ func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) { o.newRecursiveWatcher = func() (recursiveWatcher, error) { return file.NewRecursiveWatcher(0) } - o.outputCapturer = os.Stdout + + // Capture stdout by replacing it with a piped writer and returning an attached io.Reader. + // Functions are concurrency safe and idempotent. + var mu sync.Mutex + var savedWriter, savedStdout *os.File + savedStdout = os.Stdout + o.captureStdout = func() (io.Reader, error) { + if savedWriter != nil { + savedWriter.Close() + } + pipeReader, pipeWriter, err := os.Pipe() + if err != nil { + return nil, err + } + mu.Lock() + savedWriter = pipeWriter + os.Stdout = savedWriter + mu.Unlock() + return (io.Reader)(pipeReader), nil + } + o.releaseStdout = func() { + mu.Lock() + os.Stdout = savedStdout + mu.Unlock() + savedWriter.Close() + } return o, nil } @@ -682,16 +709,11 @@ func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]stri return nil, fmt.Errorf("describe ECS service for %s in environment %s: %w", o.wkldName, o.envName, err) } - // capture stdout - stdoutReader, stdoutWriter, err := os.Pipe() + stdoutReader, err := o.captureStdout() if err != nil { - return nil, fmt.Errorf("create os pipe: %w", err) + return nil, err } - stdoutPointer := o.outputCapturer - defer func() { - o.outputCapturer = stdoutPointer - }() - o.outputCapturer = stdoutWriter + defer o.releaseStdout() // try exec on each container within the service var wg sync.WaitGroup @@ -720,8 +742,7 @@ func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]stri containersFinished := make(chan struct{}) go func() { wg.Wait() - stdoutWriter.Close() - o.outputCapturer = stdoutPointer + o.releaseStdout() close(containersFinished) }() @@ -777,14 +798,19 @@ func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]stri ecsExecMethod, } + credentialsChainWrappedErrs := []string{ + "assume role", + "ecs exec", + } + // return TaskRole credentials from first successful method var errs []error - for _, method := range credentialsChain { + for errIndex, method := range credentialsChain { vars, err := method() if err == nil { return vars, nil } - errs = append(errs, err) + errs = append(errs, fmt.Errorf("%s: %w", credentialsChainWrappedErrs[errIndex], err)) } return nil, &errTaskRoleRetrievalFailed{errs} @@ -1159,7 +1185,7 @@ func BuildRunLocalCmd() *cobra.Command { cmd.Flags().StringVarP(&vars.envName, envFlag, envFlagShort, "", envFlagDescription) cmd.Flags().StringVarP(&vars.appName, appFlag, appFlagShort, tryReadingAppName(), appFlagDescription) cmd.Flags().BoolVar(&vars.watch, watchFlag, false, watchFlagDescription) - cmd.Flags().BoolVar(&vars.retrieveTaskRole, retrieveTaskRoleFlag, false, retrieveTaskRoleFlagDescription) + cmd.Flags().BoolVar(&vars.useTaskRole, useTaskRoleFlag, false, useTaskRoleFlagDescription) cmd.Flags().Var(&vars.portOverrides, portOverrideFlag, portOverridesFlagDescription) cmd.Flags().StringToStringVar(&vars.envOverrides, envVarOverrideFlag, nil, envVarOverrideFlagDescription) cmd.Flags().BoolVar(&vars.proxy, proxyFlag, false, proxyFlagDescription) diff --git a/internal/pkg/cli/run_local_test.go b/internal/pkg/cli/run_local_test.go index 9e7313cf46b..fba711e60ce 100644 --- a/internal/pkg/cli/run_local_test.go +++ b/internal/pkg/cli/run_local_test.go @@ -7,6 +7,8 @@ import ( "context" "errors" "fmt" + "io" + "strings" "syscall" "testing" @@ -201,6 +203,7 @@ func TestRunLocalOpts_Ask(t *testing.T) { type runLocalExecuteMocks struct { ecsClient *mocks.MockecsClient + ecsExecutor *mocks.MockecsCommandExecutor store *mocks.Mockstore sessCreds credentials.Provider sessProvider *mocks.MocksessionProvider @@ -432,7 +435,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { "AWS_SESSION_TOKEN": "myEnvToken", }, } - expectedTaskRoleTask := orchestrator.Task{ + expectedTaskWithRegion := orchestrator.Task{ Containers: map[string]orchestrator.ContainerDefinition{ "foo": { ImageURI: "image1", @@ -441,9 +444,9 @@ func TestRunLocalOpts_Execute(t *testing.T) { }, Secrets: map[string]string{ "SHARED_SECRET": "secretvalue", - "AWS_ACCESS_KEY_ID": "taskRoleID", - "AWS_SECRET_ACCESS_KEY": "taskRoleSecret", - "AWS_SESSION_TOKEN": "taskRoleToken", + "AWS_ACCESS_KEY_ID": "myID", + "AWS_SECRET_ACCESS_KEY": "mySecret", + "AWS_SESSION_TOKEN": "myToken", "AWS_DEFAULT_REGION": testRegion, "AWS_REGION": testRegion, }, @@ -463,9 +466,9 @@ func TestRunLocalOpts_Execute(t *testing.T) { }, Secrets: map[string]string{ "SHARED_SECRET": "secretvalue", - "AWS_ACCESS_KEY_ID": "taskRoleID", - "AWS_SECRET_ACCESS_KEY": "taskRoleSecret", - "AWS_SESSION_TOKEN": "taskRoleToken", + "AWS_ACCESS_KEY_ID": "myID", + "AWS_SECRET_ACCESS_KEY": "mySecret", + "AWS_SESSION_TOKEN": "myToken", "AWS_DEFAULT_REGION": testRegion, "AWS_REGION": testRegion, }, @@ -488,6 +491,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { inputWatch bool inputTaskRole bool inputProxy bool + inputReader io.Reader buildImagesError error setupMocks func(t *testing.T, m *runLocalExecuteMocks) @@ -522,14 +526,35 @@ func TestRunLocalOpts_Execute(t *testing.T) { inputWkldName: testWkldName, inputEnvName: testEnvName, inputTaskRole: true, + inputReader: strings.NewReader("some error"), setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil) m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) m.sessProvider.EXPECT().FromRole("mock-arn", testRegion).Return(nil, errors.New("some error")) + m.ecsClient.EXPECT().DescribeService(testAppName, testEnvName, testWkldName).Return(&ecs.ServiceDesc{ + Tasks: []*awsecs.Task{ + { + TaskArn: aws.String("arn:aws:ecs:us-west-2:123456789:task/clusterName/taskName"), + Containers: []*sdkecs.Container{ + { + RuntimeId: aws.String("runtime-id"), + LastStatus: aws.String("RUNNING"), + ManagedAgents: []*sdkecs.ManagedAgent{ + { + Name: aws.String("ExecuteCommandAgent"), + LastStatus: aws.String("RUNNING"), + }, + }, + }, + }, + }, + }, + }, nil) + m.ecsExecutor.EXPECT().ExecuteCommand(gomock.Any()).Return(nil) }, - wantedError: errors.New(`get task: retrieve task role credentials: some error -ecs exec method not implemented`), + wantedError: errors.New(`get task: retrieve task role credentials: assume role: some error +ecs exec: all containers failed to retrieve credentials`), }, "error reading workload manifest": { inputAppName: testAppName, @@ -807,7 +832,7 @@ ecs exec method not implemented`), } }, }, - "success, one run task call, taskrole assumerole method": { + "success, one run task call, task role assume role method": { inputAppName: testAppName, inputWkldName: testWkldName, inputEnvName: testEnvName, @@ -818,7 +843,7 @@ ecs exec method not implemented`), m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) taskRoleSess := &session.Session{ Config: &aws.Config{ - Credentials: credentials.NewStaticCredentials("taskRoleID", "taskRoleSecret", "taskRoleToken"), + Credentials: credentials.NewStaticCredentials("myID", "mySecret", "myToken"), Region: aws.String(testRegion), }, } @@ -832,7 +857,55 @@ ecs exec method not implemented`), return errCh } m.orchestrator.RunTaskFn = func(task orchestrator.Task, opts ...orchestrator.RunTaskOption) { - require.Equal(t, expectedTaskRoleTask, task) + require.Equal(t, expectedTaskWithRegion, task) + } + m.orchestrator.StopFn = func() { + require.Len(t, errCh, 0) + close(errCh) + } + }, + }, + "success, one run task call, task role ecs exec method": { + inputAppName: testAppName, + inputWkldName: testWkldName, + inputEnvName: testEnvName, + inputTaskRole: true, + inputReader: strings.NewReader(`{"AccessKeyId":"myID","SecretAccessKey":"mySecret","Token":"myToken"}`), + setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { + m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil) + m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.sessProvider.EXPECT().FromRole("mock-arn", testRegion).Return(nil, errors.New("some error")) + m.ecsClient.EXPECT().DescribeService(testAppName, testEnvName, testWkldName).Return(&ecs.ServiceDesc{ + Tasks: []*awsecs.Task{ + { + TaskArn: aws.String("arn:aws:ecs:us-west-2:123456789:task/clusterName/taskName"), + Containers: []*sdkecs.Container{ + { + RuntimeId: aws.String("runtime-id"), + LastStatus: aws.String("RUNNING"), + ManagedAgents: []*sdkecs.ManagedAgent{ + { + Name: aws.String("ExecuteCommandAgent"), + LastStatus: aws.String("RUNNING"), + }, + }, + }, + }, + }, + }, + }, nil) + m.ecsExecutor.EXPECT().ExecuteCommand(gomock.Any()).Return(nil) + m.ws.EXPECT().ReadWorkloadManifest(testWkldName).Return([]byte(""), nil) + m.interpolator.EXPECT().Interpolate("").Return("", nil) + + errCh := make(chan error, 1) + m.orchestrator.StartFn = func() <-chan error { + errCh <- errors.New("some error") + return errCh + } + m.orchestrator.RunTaskFn = func(task orchestrator.Task, opts ...orchestrator.RunTaskOption) { + require.Equal(t, expectedTask, task) } m.orchestrator.StopFn = func() { require.Len(t, errCh, 0) @@ -1053,6 +1126,7 @@ ecs exec method not implemented`), defer ctrl.Finish() m := &runLocalExecuteMocks{ ecsClient: mocks.NewMockecsClient(ctrl), + ecsExecutor: mocks.NewMockecsCommandExecutor(ctrl), ssm: mocks.NewMocksecretGetter(ctrl), secretsManager: mocks.NewMocksecretGetter(ctrl), store: mocks.NewMockstore(ctrl), @@ -1103,6 +1177,7 @@ ecs exec method not implemented`), }, ws: m.ws, ecsClient: m.ecsClient, + ecsExecutor: m.ecsExecutor, ssm: m.ssm, secretsManager: m.secretsManager, store: m.store, @@ -1130,6 +1205,10 @@ ecs exec method not implemented`), newRecursiveWatcher: func() (recursiveWatcher, error) { return m.watcher, nil }, + captureStdout: func() (io.Reader, error) { + return tc.inputReader, nil + }, + releaseStdout: func() {}, } // WHEN err := opts.Execute() From 63006436c86cb324b6a07a6f27c6249c60f73c34 Mon Sep 17 00:00:00 2001 From: Aiden Carpenter Date: Tue, 5 Dec 2023 13:58:45 -0800 Subject: [PATCH 07/14] default flag to true, make recommend actions more helpful --- internal/pkg/cli/errors.go | 4 +++- internal/pkg/cli/run_local.go | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/pkg/cli/errors.go b/internal/pkg/cli/errors.go index 58561972c52..9faba29bb78 100644 --- a/internal/pkg/cli/errors.go +++ b/internal/pkg/cli/errors.go @@ -173,9 +173,11 @@ func (e *errTaskRoleRetrievalFailed) Error() string { } func (e *errTaskRoleRetrievalFailed) RecommendActions() string { - return fmt.Sprintf(`TaskRole retrieval failed. You can manually add permissions for your account to assume TaskRole by adding the following YAML override to your service: + return fmt.Sprintf(`TaskRole retrieval failed. If your containers don't require the TaskRole for local testing, you can run %s to disable this feature. +If you require the TaskRole, you can manually add permissions for your account to assume TaskRole by adding the following YAML override to your service: %s For more information on YAML overrides see %s`, + color.HighlightCode(`copilot run local --use-task-role=false`), color.HighlightCodeBlock(`- op: add path: /Resources/TaskRole/Properties/AssumeRolePolicyDocument/Statement/- value: diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index fa5a4651a27..b1608cd5d06 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -1185,7 +1185,7 @@ func BuildRunLocalCmd() *cobra.Command { cmd.Flags().StringVarP(&vars.envName, envFlag, envFlagShort, "", envFlagDescription) cmd.Flags().StringVarP(&vars.appName, appFlag, appFlagShort, tryReadingAppName(), appFlagDescription) cmd.Flags().BoolVar(&vars.watch, watchFlag, false, watchFlagDescription) - cmd.Flags().BoolVar(&vars.useTaskRole, useTaskRoleFlag, false, useTaskRoleFlagDescription) + cmd.Flags().BoolVar(&vars.useTaskRole, useTaskRoleFlag, true, useTaskRoleFlagDescription) cmd.Flags().Var(&vars.portOverrides, portOverrideFlag, portOverridesFlagDescription) cmd.Flags().StringToStringVar(&vars.envOverrides, envVarOverrideFlag, nil, envVarOverrideFlagDescription) cmd.Flags().BoolVar(&vars.proxy, proxyFlag, false, proxyFlagDescription) From e6eda39d450eaf3015b022005ca9f51539ff3bcc Mon Sep 17 00:00:00 2001 From: Aiden Carpenter Date: Tue, 5 Dec 2023 14:11:00 -0800 Subject: [PATCH 08/14] append container errors to parse error --- internal/pkg/cli/run_local.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index b1608cd5d06..3e1b700892f 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -717,6 +717,7 @@ func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]stri // try exec on each container within the service var wg sync.WaitGroup + containerErr := make(chan error) for _, task := range svcDesc.Tasks { taskID, err := awsecs.TaskID(aws.StringValue(task.TaskArn)) if err != nil { @@ -728,7 +729,7 @@ func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]stri containerName := aws.StringValue(container.Name) go func() { defer wg.Done() - o.ecsExecutor.ExecuteCommand(awsecs.ExecuteCommandInput{ + containerErr <- o.ecsExecutor.ExecuteCommand(awsecs.ExecuteCommandInput{ Cluster: svcDesc.ClusterName, Command: fmt.Sprintf("/bin/sh -c %q\n", curlContainerCredentialsCmd), Task: taskID, @@ -783,13 +784,18 @@ func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]stri parseErr <- errors.New("all containers failed to retrieve credentials") }() - select { - case creds := <-credsResult: - return creds, nil - case <-ctx.Done(): - return nil, ctx.Err() - case err := <-parseErr: - return nil, err + var containerErrs []error + for { + select { + case creds := <-credsResult: + return creds, nil + case <-ctx.Done(): + return nil, ctx.Err() + case err := <-parseErr: + return nil, errors.Join(append([]error{err}, containerErrs...)...) + case err := <-containerErr: + containerErrs = append(containerErrs, err) + } } } From 92e10a80cde2d889bc47756f29c9a1354732a502 Mon Sep 17 00:00:00 2001 From: Aiden Carpenter Date: Wed, 6 Dec 2023 10:28:43 -0800 Subject: [PATCH 09/14] change error wording --- internal/pkg/cli/errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/cli/errors.go b/internal/pkg/cli/errors.go index 9faba29bb78..eb89caa4017 100644 --- a/internal/pkg/cli/errors.go +++ b/internal/pkg/cli/errors.go @@ -173,7 +173,7 @@ func (e *errTaskRoleRetrievalFailed) Error() string { } func (e *errTaskRoleRetrievalFailed) RecommendActions() string { - return fmt.Sprintf(`TaskRole retrieval failed. If your containers don't require the TaskRole for local testing, you can run %s to disable this feature. + return fmt.Sprintf(`TaskRole retrieval failed. If your containers don't require the TaskRole for local testing, you can use %s to disable this feature. If you require the TaskRole, you can manually add permissions for your account to assume TaskRole by adding the following YAML override to your service: %s For more information on YAML overrides see %s`, From de82b5d496108cf5a6aea8bd198db4686f903940 Mon Sep 17 00:00:00 2001 From: Aiden Carpenter Date: Wed, 6 Dec 2023 13:35:24 -0800 Subject: [PATCH 10/14] address penghao feedback --- internal/pkg/cli/run_local.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index 3e1b700892f..591069d5afc 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -69,6 +69,8 @@ const ( ) const ( + // Command to retrieve container credentials with ecs exec. See more at https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task-iam-roles.html. + // Example output: {"AccessKeyId":"ACCESS_KEY_ID","Expiration":"EXPIRATION_DATE","RoleArn":"TASK_ROLE_ARN","SecretAccessKey":"SECRET_ACCESS_KEY","Token":"SECURITY_TOKEN_STRING"} curlContainerCredentialsCmd = "curl 169.254.170.2$AWS_CONTAINER_CREDENTIALS_RELATIVE_URI" ) @@ -778,10 +780,10 @@ func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]stri } return } + parseErr <- errors.New("all containers failed to retrieve credentials") case <-ctx.Done(): return } - parseErr <- errors.New("all containers failed to retrieve credentials") }() var containerErrs []error From fa810af98451ccde5e1f0fc82d360e9c3d0c37bd Mon Sep 17 00:00:00 2001 From: Aiden Carpenter Date: Thu, 7 Dec 2023 14:42:27 -0800 Subject: [PATCH 11/14] address feedback --- internal/pkg/cli/run_local.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index 591069d5afc..577e04fc046 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -285,15 +285,15 @@ func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) { return nil, err } mu.Lock() + defer mu.Unlock() savedWriter = pipeWriter os.Stdout = savedWriter - mu.Unlock() return (io.Reader)(pipeReader), nil } o.releaseStdout = func() { mu.Lock() + defer mu.Unlock() os.Stdout = savedStdout - mu.Unlock() savedWriter.Close() } return o, nil @@ -731,12 +731,13 @@ func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]stri containerName := aws.StringValue(container.Name) go func() { defer wg.Done() - containerErr <- o.ecsExecutor.ExecuteCommand(awsecs.ExecuteCommandInput{ + err := o.ecsExecutor.ExecuteCommand(awsecs.ExecuteCommandInput{ Cluster: svcDesc.ClusterName, Command: fmt.Sprintf("/bin/sh -c %q\n", curlContainerCredentialsCmd), Task: taskID, Container: containerName, }) + containerErr <- fmt.Errorf("container %s in task %s: %w", containerName, taskID, err) }() } } From 0bb15607370799f7efc903d534f63d99fd34b16c Mon Sep 17 00:00:00 2001 From: Aiden Carpenter Date: Thu, 7 Dec 2023 14:46:37 -0800 Subject: [PATCH 12/14] fix test --- internal/pkg/cli/run_local.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index 577e04fc046..3fd82a913e2 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -737,7 +737,9 @@ func (o *runLocalOpts) taskRoleCredentials(ctx context.Context) (map[string]stri Task: taskID, Container: containerName, }) - containerErr <- fmt.Errorf("container %s in task %s: %w", containerName, taskID, err) + if err != nil { + containerErr <- fmt.Errorf("container %s in task %s: %w", containerName, taskID, err) + } }() } } From 39968e6bbc536dab640f5e7e36c70e5aca683e28 Mon Sep 17 00:00:00 2001 From: Aiden Carpenter Date: Mon, 11 Dec 2023 12:49:36 -0800 Subject: [PATCH 13/14] disable flag --- internal/pkg/cli/run_local.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index 3fd82a913e2..369bf671c86 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -1196,7 +1196,6 @@ func BuildRunLocalCmd() *cobra.Command { cmd.Flags().StringVarP(&vars.envName, envFlag, envFlagShort, "", envFlagDescription) cmd.Flags().StringVarP(&vars.appName, appFlag, appFlagShort, tryReadingAppName(), appFlagDescription) cmd.Flags().BoolVar(&vars.watch, watchFlag, false, watchFlagDescription) - cmd.Flags().BoolVar(&vars.useTaskRole, useTaskRoleFlag, true, useTaskRoleFlagDescription) cmd.Flags().Var(&vars.portOverrides, portOverrideFlag, portOverridesFlagDescription) cmd.Flags().StringToStringVar(&vars.envOverrides, envVarOverrideFlag, nil, envVarOverrideFlagDescription) cmd.Flags().BoolVar(&vars.proxy, proxyFlag, false, proxyFlagDescription) From 1ff0b0591af7c4c24767b797ebd6a5802b711f6d Mon Sep 17 00:00:00 2001 From: Aiden Carpenter Date: Wed, 13 Dec 2023 10:21:47 -0800 Subject: [PATCH 14/14] reenable flag --- internal/pkg/cli/run_local.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index 369bf671c86..3fd82a913e2 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -1196,6 +1196,7 @@ func BuildRunLocalCmd() *cobra.Command { cmd.Flags().StringVarP(&vars.envName, envFlag, envFlagShort, "", envFlagDescription) cmd.Flags().StringVarP(&vars.appName, appFlag, appFlagShort, tryReadingAppName(), appFlagDescription) cmd.Flags().BoolVar(&vars.watch, watchFlag, false, watchFlagDescription) + cmd.Flags().BoolVar(&vars.useTaskRole, useTaskRoleFlag, true, useTaskRoleFlagDescription) cmd.Flags().Var(&vars.portOverrides, portOverrideFlag, portOverridesFlagDescription) cmd.Flags().StringToStringVar(&vars.envOverrides, envVarOverrideFlag, nil, envVarOverrideFlagDescription) cmd.Flags().BoolVar(&vars.proxy, proxyFlag, false, proxyFlagDescription)