From c83ac79f08bd2fbcda24637fd947f91de54da1ef Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Wed, 21 Apr 2021 12:51:08 -0500 Subject: [PATCH 01/18] add generate-cmd flag to task run --- internal/pkg/cli/flag.go | 5 ++ internal/pkg/cli/task_run.go | 90 ++++++++++++++++++++++++++++++- internal/pkg/cli/task_run_test.go | 85 ++++++++++++++++++++++++++++- 3 files changed, 178 insertions(+), 2 deletions(-) diff --git a/internal/pkg/cli/flag.go b/internal/pkg/cli/flag.go index acabc0e0e2b..df13779882f 100644 --- a/internal/pkg/cli/flag.go +++ b/internal/pkg/cli/flag.go @@ -75,6 +75,7 @@ const ( commandFlag = "command" entrypointFlag = "entrypoint" taskDefaultFlag = "default" + generateCMDFlag = "generate-cmd" vpcIDFlag = "import-vpc-id" publicSubnetsFlag = "import-public-subnets" @@ -225,6 +226,10 @@ Must be either "MySQL" or "PostgreSQL".` Tasks with the same group name share the same set of resources. (default directory name)` taskImageTagFlagDescription = `Optional. The container image tag in addition to "latest".` + generateCMDFlagDescription = `Optional. Given a non-Copilot ECS service or a Copilot service, generate a command with pre-filled values for each flags. +To provide a non-Copilot ECS service, specify --generate-cmd /. +To provide a Copilot service, specify --generate-cmd //. +Cannot be specified with any other flags.` vpcIDFlagDescription = "Optional. Use an existing VPC ID." publicSubnetsFlagDescription = "Optional. Use existing public subnet IDs." diff --git a/internal/pkg/cli/task_run.go b/internal/pkg/cli/task_run.go index 84b10863fa6..6de8d50fb5f 100644 --- a/internal/pkg/cli/task_run.go +++ b/internal/pkg/cli/task_run.go @@ -10,6 +10,10 @@ import ( "path/filepath" "strings" + "github.com/aws/copilot-cli/internal/pkg/generator" + + "github.com/aws/aws-sdk-go/aws/arn" + awscloudformation "github.com/aws/copilot-cli/internal/pkg/aws/cloudformation" "github.com/aws/copilot-cli/internal/pkg/describe" "github.com/aws/copilot-cli/internal/pkg/logging" @@ -49,6 +53,10 @@ const ( fmtImageURI = "%s:%s" ) +type cmdGenerator interface { + Generate() (*generator.GenerateCommandOpts, error) +} + var ( errNumNotPositive = errors.New("number of tasks must be positive") errCPUNotPositive = errors.New("CPU units must be positive") @@ -92,12 +100,14 @@ type runTaskVars struct { entrypoint string resourceTags map[string]string - follow bool + follow bool + generateCMDTarget string } type runTaskOpts struct { runTaskVars isDockerfileSet bool + nFlag int // Interfaces to interact with dependencies. fs afero.Fs @@ -247,6 +257,12 @@ func (o *runTaskOpts) configureSessAndEnv() error { // Validate returns an error if the flag values passed by the user are invalid. func (o *runTaskOpts) Validate() error { + if o.generateCMDTarget != "" { + if o.nFlag >= 2 { + return errors.New("cannot specify `--generate-cmd` with any other flag") + } + } + if o.count <= 0 { return errNumNotPositive } @@ -383,6 +399,9 @@ func (o *runTaskOpts) validateFlagsWithSecurityGroups() error { // Ask prompts the user for any required or important fields that are not provided. func (o *runTaskOpts) Ask() error { + if o.generateCMDTarget != "" { + return nil + } if o.shouldPromptForAppEnv() { if err := o.askAppName(); err != nil { return err @@ -407,6 +426,10 @@ func (o *runTaskOpts) shouldPromptForAppEnv() bool { // Execute deploys and runs the task. func (o *runTaskOpts) Execute() error { + if o.generateCMDTarget != "" { + return o.generateCommand() + } + if o.groupName == "" { dir, err := os.Getwd() if err != nil { @@ -481,6 +504,68 @@ func (o *runTaskOpts) Execute() error { return nil } +func (o *runTaskOpts) generateCommand() error { + g, err := o.configureGenerator() + if err != nil { + return err + } + + command, err := g.Generate() + if err != nil { + return fmt.Errorf("generate command: %w", err) + } + log.Infoln(command.String()) + return nil +} + +func (o *runTaskOpts) configureGenerator() (cmdGenerator, error) { + var g cmdGenerator + sess, err := sessions.NewProvider().Default() + if err != nil { + return nil, fmt.Errorf("get default session: %s", err) + } + + if arn.IsARN(o.generateCMDTarget) { + svcARN := awsecs.ServiceArn(o.generateCMDTarget) + clusterName, err := svcARN.ClusterName() + if err != nil { + return nil, fmt.Errorf("extract cluster name from arn %s", svcARN) + } + serviceName, err := svcARN.ServiceName() + if err != nil { + return nil, fmt.Errorf("extract service name from arn %s", svcARN) + } + g = generator.ECSServiceCommandGenerator{ + Cluster: clusterName, + Service: serviceName, + ECSClient: awsecs.New(sess), + } + return g, nil + } + + parts := strings.Split(o.generateCMDTarget, "/") + switch len(parts) { + case 2: + clusterName, serviceName := parts[0], parts[1] + g = generator.ECSServiceCommandGenerator{ + Cluster: clusterName, + Service: serviceName, + ECSClient: awsecs.New(sess), + } + case 3: + appName, envName, serviceName := parts[0], parts[1], parts[2] + g = generator.ServiceCommandGenerator{ + App: appName, + Env: envName, + Service: serviceName, + ECSInformationGetter: ecs.New(sess), + } + default: + return nil, fmt.Errorf("invalid input") //TODO + } + return g, nil +} + func (o *runTaskOpts) displayLogStream() error { if err := o.eventsWriter.WriteEventsUntilStopped(); err != nil { return fmt.Errorf("write events: %w", err) @@ -688,6 +773,7 @@ Run a task with a command. /code $ copilot task run --command "python migrate-script.py"`, RunE: runCmdE(func(cmd *cobra.Command, args []string) error { opts, err := newTaskRunOpts(vars) + opts.nFlag = cmd.Flags().NFlag() if err != nil { return err } @@ -738,5 +824,7 @@ Run a task with a command. cmd.Flags().StringToStringVar(&vars.resourceTags, resourceTagsFlag, nil, resourceTagsFlagDescription) cmd.Flags().BoolVar(&vars.follow, followFlag, false, followFlagDescription) + cmd.Flags().StringVar(&vars.generateCMDTarget, generateCMDFlag, "", generateCMDFlagDescription) + return cmd } diff --git a/internal/pkg/cli/task_run_test.go b/internal/pkg/cli/task_run_test.go index b0c155e45d4..42f7ac1c1dc 100644 --- a/internal/pkg/cli/task_run_test.go +++ b/internal/pkg/cli/task_run_test.go @@ -9,6 +9,10 @@ import ( "path/filepath" "testing" + "github.com/aws/copilot-cli/internal/pkg/aws/sessions" + + "github.com/aws/copilot-cli/internal/pkg/generator" + "github.com/aws/copilot-cli/internal/pkg/cli/mocks" "github.com/aws/copilot-cli/internal/pkg/exec" @@ -60,7 +64,8 @@ func TestTaskRunOpts_Validate(t *testing.T) { inCommand string inEntryPoint string - inDefault bool + inDefault bool + inGenerateCMDTarget string appName string isDockerfileSet bool @@ -302,6 +307,14 @@ func TestTaskRunOpts_Validate(t *testing.T) { wantedError: errors.New("cannot specify both `--env` and `--cluster`"), }, + "generate-cmd specified with another flag": { + basicOpts: defaultOpts, + + inGenerateCMDTarget: "cluster/service", + inEnv: "env", + + wantedError: errors.New("cannot specify `--generate-cmd` with any other flag"), + }, } for name, tc := range testCases { @@ -893,3 +906,73 @@ func TestTaskRunOpts_Execute(t *testing.T) { }) } } + +func TestTaskRunOpts_configureGenerator(t *testing.T) { + testCases := map[string]struct { + inGenerateCommandTarget string + + wantedGenerator cmdGenerator + wantedError error + }{ + "should configure an ECS service command generator given an service ARN": { + inGenerateCommandTarget: "arn:aws:ecs:us-east-1:123456789012:service/crowded-cluster/good-service", + wantedGenerator: generator.ECSServiceCommandGenerator{ + Cluster: "crowded-cluster", + Service: "good-service", + }, + }, + "should configure an ECS service command generator given a cluster/service target": { + inGenerateCommandTarget: "crowded-cluster/good-service", + wantedGenerator: generator.ECSServiceCommandGenerator{ + Cluster: "crowded-cluster", + Service: "good-service", + }, + }, + "should configure a service command generator given an app/env/svc target": { + inGenerateCommandTarget: "good-app/good-env/good-service", + wantedGenerator: generator.ServiceCommandGenerator{ + App: "good-app", + Env: "good-env", + Service: "good-service", + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + sess, _ := sessions.NewProvider().Default() + opts := &runTaskOpts{ + runTaskVars: runTaskVars{ + generateCMDTarget: tc.inGenerateCommandTarget, + }, + sess: sess, + } + + got, err := opts.configureGenerator() + if tc.wantedError != nil { + require.EqualError(t, tc.wantedError, err.Error()) + } else { + require.NoError(t, err) + require.True(t, equalGenerator(tc.wantedGenerator, got)) + } + }) + } +} + +func equalGenerator(wanted, got cmdGenerator) bool { + switch wantedG := wanted.(type) { + case generator.ECSServiceCommandGenerator: + gotG, ok := got.(generator.ECSServiceCommandGenerator) + if !ok { + return false + } + return gotG.Cluster == wantedG.Cluster && gotG.Service == wantedG.Service + case generator.ServiceCommandGenerator: + gotG, ok := got.(generator.ServiceCommandGenerator) + if !ok { + return false + } + return gotG.App == wantedG.App && gotG.Env == wantedG.Env && gotG.Service == wantedG.Service + } + return false +} From 3c2457513c7dc6e112a0ae22aa1252a3890a51f3 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Wed, 21 Apr 2021 16:38:30 -0500 Subject: [PATCH 02/18] update condition to prompt for app/env --- internal/pkg/cli/task_run.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/pkg/cli/task_run.go b/internal/pkg/cli/task_run.go index 6de8d50fb5f..a0584326422 100644 --- a/internal/pkg/cli/task_run.go +++ b/internal/pkg/cli/task_run.go @@ -417,7 +417,7 @@ func (o *runTaskOpts) shouldPromptForAppEnv() bool { // NOTE: if security groups are specified but subnets are not, then we use the default subnets with the // specified security groups. useDefault := o.useDefaultSubnetsAndCluster || (o.securityGroups != nil && o.subnets == nil && o.cluster == "") - useConfig := o.subnets != nil + useConfig := o.subnets != nil || o.cluster != "" // if user hasn't specified that they want to use the default subnets, and that they didn't provide specific subnets // that they want to use, then we prompt. @@ -509,7 +509,6 @@ func (o *runTaskOpts) generateCommand() error { if err != nil { return err } - command, err := g.Generate() if err != nil { return fmt.Errorf("generate command: %w", err) From c5f08d5b58d0e05e3119c71db21dc51a6e3a7a93 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Wed, 21 Apr 2021 16:49:35 -0500 Subject: [PATCH 03/18] rearange code and add missing test --- internal/pkg/cli/task_run.go | 35 +++++++++++++++++-------------- internal/pkg/cli/task_run_test.go | 4 ++++ 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/internal/pkg/cli/task_run.go b/internal/pkg/cli/task_run.go index a0584326422..3b8fa98b076 100644 --- a/internal/pkg/cli/task_run.go +++ b/internal/pkg/cli/task_run.go @@ -525,21 +525,7 @@ func (o *runTaskOpts) configureGenerator() (cmdGenerator, error) { } if arn.IsARN(o.generateCMDTarget) { - svcARN := awsecs.ServiceArn(o.generateCMDTarget) - clusterName, err := svcARN.ClusterName() - if err != nil { - return nil, fmt.Errorf("extract cluster name from arn %s", svcARN) - } - serviceName, err := svcARN.ServiceName() - if err != nil { - return nil, fmt.Errorf("extract service name from arn %s", svcARN) - } - g = generator.ECSServiceCommandGenerator{ - Cluster: clusterName, - Service: serviceName, - ECSClient: awsecs.New(sess), - } - return g, nil + return o.configureGeneratorFromARN(sess) } parts := strings.Split(o.generateCMDTarget, "/") @@ -560,11 +546,28 @@ func (o *runTaskOpts) configureGenerator() (cmdGenerator, error) { ECSInformationGetter: ecs.New(sess), } default: - return nil, fmt.Errorf("invalid input") //TODO + return nil, errors.New("invalid input to --generate-cmd") } return g, nil } +func (o *runTaskOpts) configureGeneratorFromARN(sess *session.Session) (cmdGenerator, error) { + svcARN := awsecs.ServiceArn(o.generateCMDTarget) + clusterName, err := svcARN.ClusterName() + if err != nil { + return nil, fmt.Errorf("extract cluster name from arn %s", svcARN) + } + serviceName, err := svcARN.ServiceName() + if err != nil { + return nil, fmt.Errorf("extract service name from arn %s", svcARN) + } + return generator.ECSServiceCommandGenerator{ + Cluster: clusterName, + Service: serviceName, + ECSClient: awsecs.New(sess), + }, nil +} + func (o *runTaskOpts) displayLogStream() error { if err := o.eventsWriter.WriteEventsUntilStopped(); err != nil { return fmt.Errorf("write events: %w", err) diff --git a/internal/pkg/cli/task_run_test.go b/internal/pkg/cli/task_run_test.go index 42f7ac1c1dc..d5c0d42f876 100644 --- a/internal/pkg/cli/task_run_test.go +++ b/internal/pkg/cli/task_run_test.go @@ -936,6 +936,10 @@ func TestTaskRunOpts_configureGenerator(t *testing.T) { Service: "good-service", }, }, + "invalid input": { + inGenerateCommandTarget: "good-service", + wantedError: errors.New("invalid input to --generate-cmd"), + }, } for name, tc := range testCases { From 8ca34f9dea95ef546452b6f5de43b89474476650 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Wed, 21 Apr 2021 17:09:36 -0500 Subject: [PATCH 04/18] fix unit test --- internal/pkg/cli/task_run_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/pkg/cli/task_run_test.go b/internal/pkg/cli/task_run_test.go index d5c0d42f876..0848522633c 100644 --- a/internal/pkg/cli/task_run_test.go +++ b/internal/pkg/cli/task_run_test.go @@ -310,8 +310,7 @@ func TestTaskRunOpts_Validate(t *testing.T) { "generate-cmd specified with another flag": { basicOpts: defaultOpts, - inGenerateCMDTarget: "cluster/service", - inEnv: "env", + inGenerateCMDTarget: "cluster/service", // nFlag is set to 2. wantedError: errors.New("cannot specify `--generate-cmd` with any other flag"), }, @@ -343,8 +342,10 @@ func TestTaskRunOpts_Validate(t *testing.T) { command: tc.inCommand, entrypoint: tc.inEntryPoint, useDefaultSubnetsAndCluster: tc.inDefault, + generateCMDTarget: tc.inGenerateCMDTarget, }, isDockerfileSet: tc.isDockerfileSet, + nFlag: 2, fs: &afero.Afero{Fs: afero.NewMemMapFs()}, store: mockStore, From b90ada60cea6af67f0564e5ae2c2ccc4c66714a9 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Thu, 22 Apr 2021 12:39:37 -0500 Subject: [PATCH 05/18] address feedback to change var name and flag description --- internal/pkg/cli/flag.go | 40 +++++++++++++++---------------- internal/pkg/cli/task_run.go | 18 +++++++------- internal/pkg/cli/task_run_test.go | 4 ++-- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/internal/pkg/cli/flag.go b/internal/pkg/cli/flag.go index df13779882f..caa9448e952 100644 --- a/internal/pkg/cli/flag.go +++ b/internal/pkg/cli/flag.go @@ -60,22 +60,22 @@ const ( storageRDSInitialDBFlag = "initial-db" storageRDSParameterGroupFlag = "parameter-group" - taskGroupNameFlag = "task-group-name" - countFlag = "count" - cpuFlag = "cpu" - memoryFlag = "memory" - imageFlag = "image" - taskRoleFlag = "task-role" - executionRoleFlag = "execution-role" - clusterFlag = "cluster" - subnetsFlag = "subnets" - securityGroupsFlag = "security-groups" - envVarsFlag = "env-vars" - secretsFlag = "secrets" - commandFlag = "command" - entrypointFlag = "entrypoint" - taskDefaultFlag = "default" - generateCMDFlag = "generate-cmd" + taskGroupNameFlag = "task-group-name" + countFlag = "count" + cpuFlag = "cpu" + memoryFlag = "memory" + imageFlag = "image" + taskRoleFlag = "task-role" + executionRoleFlag = "execution-role" + clusterFlag = "cluster" + subnetsFlag = "subnets" + securityGroupsFlag = "security-groups" + envVarsFlag = "env-vars" + secretsFlag = "secrets" + commandFlag = "command" + entrypointFlag = "entrypoint" + taskDefaultFlag = "default" + generateCommandFlag = "generate-cmd" vpcIDFlag = "import-vpc-id" publicSubnetsFlag = "import-public-subnets" @@ -225,10 +225,10 @@ Must be either "MySQL" or "PostgreSQL".` taskGroupFlagDescription = `Optional. The group name of the task. Tasks with the same group name share the same set of resources. (default directory name)` - taskImageTagFlagDescription = `Optional. The container image tag in addition to "latest".` - generateCMDFlagDescription = `Optional. Given a non-Copilot ECS service or a Copilot service, generate a command with pre-filled values for each flags. -To provide a non-Copilot ECS service, specify --generate-cmd /. -To provide a Copilot service, specify --generate-cmd //. + taskImageTagFlagDescription = `Optional. The container image tag in addition to "latest".` + generateCommandFlagDescription = `Optional. Generate a command with a pre-filled value for each flag. +To use a non-Copilot ECS service, specify --generate-cmd /. +To use a Copilot service, specify --generate-cmd //. Cannot be specified with any other flags.` vpcIDFlagDescription = "Optional. Use an existing VPC ID." diff --git a/internal/pkg/cli/task_run.go b/internal/pkg/cli/task_run.go index 3b8fa98b076..058d93a69af 100644 --- a/internal/pkg/cli/task_run.go +++ b/internal/pkg/cli/task_run.go @@ -100,8 +100,8 @@ type runTaskVars struct { entrypoint string resourceTags map[string]string - follow bool - generateCMDTarget string + follow bool + generateCommandTarget string } type runTaskOpts struct { @@ -257,7 +257,7 @@ func (o *runTaskOpts) configureSessAndEnv() error { // Validate returns an error if the flag values passed by the user are invalid. func (o *runTaskOpts) Validate() error { - if o.generateCMDTarget != "" { + if o.generateCommandTarget != "" { if o.nFlag >= 2 { return errors.New("cannot specify `--generate-cmd` with any other flag") } @@ -399,7 +399,7 @@ func (o *runTaskOpts) validateFlagsWithSecurityGroups() error { // Ask prompts the user for any required or important fields that are not provided. func (o *runTaskOpts) Ask() error { - if o.generateCMDTarget != "" { + if o.generateCommandTarget != "" { return nil } if o.shouldPromptForAppEnv() { @@ -426,7 +426,7 @@ func (o *runTaskOpts) shouldPromptForAppEnv() bool { // Execute deploys and runs the task. func (o *runTaskOpts) Execute() error { - if o.generateCMDTarget != "" { + if o.generateCommandTarget != "" { return o.generateCommand() } @@ -524,11 +524,11 @@ func (o *runTaskOpts) configureGenerator() (cmdGenerator, error) { return nil, fmt.Errorf("get default session: %s", err) } - if arn.IsARN(o.generateCMDTarget) { + if arn.IsARN(o.generateCommandTarget) { return o.configureGeneratorFromARN(sess) } - parts := strings.Split(o.generateCMDTarget, "/") + parts := strings.Split(o.generateCommandTarget, "/") switch len(parts) { case 2: clusterName, serviceName := parts[0], parts[1] @@ -552,7 +552,7 @@ func (o *runTaskOpts) configureGenerator() (cmdGenerator, error) { } func (o *runTaskOpts) configureGeneratorFromARN(sess *session.Session) (cmdGenerator, error) { - svcARN := awsecs.ServiceArn(o.generateCMDTarget) + svcARN := awsecs.ServiceArn(o.generateCommandTarget) clusterName, err := svcARN.ClusterName() if err != nil { return nil, fmt.Errorf("extract cluster name from arn %s", svcARN) @@ -826,7 +826,7 @@ Run a task with a command. cmd.Flags().StringToStringVar(&vars.resourceTags, resourceTagsFlag, nil, resourceTagsFlagDescription) cmd.Flags().BoolVar(&vars.follow, followFlag, false, followFlagDescription) - cmd.Flags().StringVar(&vars.generateCMDTarget, generateCMDFlag, "", generateCMDFlagDescription) + cmd.Flags().StringVar(&vars.generateCommandTarget, generateCommandFlag, "", generateCommandFlagDescription) return cmd } diff --git a/internal/pkg/cli/task_run_test.go b/internal/pkg/cli/task_run_test.go index 0848522633c..5ac79760250 100644 --- a/internal/pkg/cli/task_run_test.go +++ b/internal/pkg/cli/task_run_test.go @@ -342,7 +342,7 @@ func TestTaskRunOpts_Validate(t *testing.T) { command: tc.inCommand, entrypoint: tc.inEntryPoint, useDefaultSubnetsAndCluster: tc.inDefault, - generateCMDTarget: tc.inGenerateCMDTarget, + generateCommandTarget: tc.inGenerateCMDTarget, }, isDockerfileSet: tc.isDockerfileSet, nFlag: 2, @@ -948,7 +948,7 @@ func TestTaskRunOpts_configureGenerator(t *testing.T) { sess, _ := sessions.NewProvider().Default() opts := &runTaskOpts{ runTaskVars: runTaskVars{ - generateCMDTarget: tc.inGenerateCommandTarget, + generateCommandTarget: tc.inGenerateCommandTarget, }, sess: sess, } From 32d489c5a9b28d24762a77e18374cb27f6673d68 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Thu, 22 Apr 2021 13:33:14 -0500 Subject: [PATCH 06/18] address feedback to change remaining var name --- internal/pkg/cli/task_run_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/pkg/cli/task_run_test.go b/internal/pkg/cli/task_run_test.go index 5ac79760250..0e30c4a5dfc 100644 --- a/internal/pkg/cli/task_run_test.go +++ b/internal/pkg/cli/task_run_test.go @@ -64,8 +64,8 @@ func TestTaskRunOpts_Validate(t *testing.T) { inCommand string inEntryPoint string - inDefault bool - inGenerateCMDTarget string + inDefault bool + inGenerateCommandTarget string appName string isDockerfileSet bool @@ -310,7 +310,7 @@ func TestTaskRunOpts_Validate(t *testing.T) { "generate-cmd specified with another flag": { basicOpts: defaultOpts, - inGenerateCMDTarget: "cluster/service", // nFlag is set to 2. + inGenerateCommandTarget: "cluster/service", // nFlag is set to 2. wantedError: errors.New("cannot specify `--generate-cmd` with any other flag"), }, @@ -342,7 +342,7 @@ func TestTaskRunOpts_Validate(t *testing.T) { command: tc.inCommand, entrypoint: tc.inEntryPoint, useDefaultSubnetsAndCluster: tc.inDefault, - generateCommandTarget: tc.inGenerateCMDTarget, + generateCommandTarget: tc.inGenerateCommandTarget, }, isDockerfileSet: tc.isDockerfileSet, nFlag: 2, From 5027eb1efc2407cb670cb477b28bdbf1589e7ed7 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Thu, 29 Apr 2021 15:39:32 -0500 Subject: [PATCH 07/18] refactor generator into ecs pkg --- Makefile | 4 +- .../pkg/ecs/mocks/mock_run_task_request.go | 148 +++++++++ internal/pkg/ecs/run_task_request.go | 128 ++++++++ internal/pkg/ecs/run_task_request_test.go | 280 ++++++++++++++++++ internal/pkg/generator/ecs_service.go | 63 ---- internal/pkg/generator/ecs_service_test.go | 170 ----------- internal/pkg/generator/generate.go | 134 --------- internal/pkg/generator/generate_test.go | 67 ----- .../pkg/generator/mocks/mock_ecs_service.go | 80 ----- internal/pkg/generator/mocks/mock_service.go | 80 ----- internal/pkg/generator/service.go | 58 ---- internal/pkg/generator/service_test.go | 149 ---------- 12 files changed, 558 insertions(+), 803 deletions(-) create mode 100644 internal/pkg/ecs/mocks/mock_run_task_request.go create mode 100644 internal/pkg/ecs/run_task_request.go create mode 100644 internal/pkg/ecs/run_task_request_test.go delete mode 100644 internal/pkg/generator/ecs_service.go delete mode 100644 internal/pkg/generator/ecs_service_test.go delete mode 100644 internal/pkg/generator/generate.go delete mode 100644 internal/pkg/generator/generate_test.go delete mode 100644 internal/pkg/generator/mocks/mock_ecs_service.go delete mode 100644 internal/pkg/generator/mocks/mock_service.go delete mode 100644 internal/pkg/generator/service.go delete mode 100644 internal/pkg/generator/service_test.go diff --git a/Makefile b/Makefile index 5a067934ea6..c18b7e83e64 100644 --- a/Makefile +++ b/Makefile @@ -188,6 +188,6 @@ gen-mocks: tools ${GOBIN}/mockgen -package=mocks -destination=./internal/pkg/list/mocks/mock_list.go -source=./internal/pkg/list/list.go ${GOBIN}/mockgen -package=mocks -destination=./internal/pkg/initialize/mocks/mock_workload.go -source=./internal/pkg/initialize/workload.go ${GOBIN}/mockgen -package=mocks -destination=./internal/pkg/ecs/mocks/mock_ecs.go -source=./internal/pkg/ecs/ecs.go - ${GOBIN}/mockgen -package=mocks -destination=./internal/pkg/generator/mocks/mock_ecs_service.go -source=./internal/pkg/generator/ecs_service.go - ${GOBIN}/mockgen -package=mocks -destination=./internal/pkg/generator/mocks/mock_service.go -source=./internal/pkg/generator/service.go + ${GOBIN}/mockgen -package=mocks -destination=./internal/pkg/ecs/mocks/mock_run_task_request.go -source=./internal/pkg/ecs/run_task_request.go + diff --git a/internal/pkg/ecs/mocks/mock_run_task_request.go b/internal/pkg/ecs/mocks/mock_run_task_request.go new file mode 100644 index 00000000000..2c3efdbec1b --- /dev/null +++ b/internal/pkg/ecs/mocks/mock_run_task_request.go @@ -0,0 +1,148 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: ./internal/pkg/ecs/run_task_request.go + +// Package mocks is a generated GoMock package. +package mocks + +import ( + reflect "reflect" + + ecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" + gomock "github.com/golang/mock/gomock" +) + +// MockecsServiceDescriber is a mock of ecsServiceDescriber interface. +type MockecsServiceDescriber struct { + ctrl *gomock.Controller + recorder *MockecsServiceDescriberMockRecorder +} + +// MockecsServiceDescriberMockRecorder is the mock recorder for MockecsServiceDescriber. +type MockecsServiceDescriberMockRecorder struct { + mock *MockecsServiceDescriber +} + +// NewMockecsServiceDescriber creates a new mock instance. +func NewMockecsServiceDescriber(ctrl *gomock.Controller) *MockecsServiceDescriber { + mock := &MockecsServiceDescriber{ctrl: ctrl} + mock.recorder = &MockecsServiceDescriberMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockecsServiceDescriber) EXPECT() *MockecsServiceDescriberMockRecorder { + return m.recorder +} + +// NetworkConfiguration mocks base method. +func (m *MockecsServiceDescriber) NetworkConfiguration(cluster, serviceName string) (*ecs.NetworkConfiguration, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "NetworkConfiguration", cluster, serviceName) + ret0, _ := ret[0].(*ecs.NetworkConfiguration) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// NetworkConfiguration indicates an expected call of NetworkConfiguration. +func (mr *MockecsServiceDescriberMockRecorder) NetworkConfiguration(cluster, serviceName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NetworkConfiguration", reflect.TypeOf((*MockecsServiceDescriber)(nil).NetworkConfiguration), cluster, serviceName) +} + +// Service mocks base method. +func (m *MockecsServiceDescriber) Service(clusterName, serviceName string) (*ecs.Service, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Service", clusterName, serviceName) + ret0, _ := ret[0].(*ecs.Service) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Service indicates an expected call of Service. +func (mr *MockecsServiceDescriberMockRecorder) Service(clusterName, serviceName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Service", reflect.TypeOf((*MockecsServiceDescriber)(nil).Service), clusterName, serviceName) +} + +// TaskDefinition mocks base method. +func (m *MockecsServiceDescriber) TaskDefinition(taskDefName string) (*ecs.TaskDefinition, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "TaskDefinition", taskDefName) + ret0, _ := ret[0].(*ecs.TaskDefinition) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// TaskDefinition indicates an expected call of TaskDefinition. +func (mr *MockecsServiceDescriberMockRecorder) TaskDefinition(taskDefName interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TaskDefinition", reflect.TypeOf((*MockecsServiceDescriber)(nil).TaskDefinition), taskDefName) +} + +// MockserviceDescriber is a mock of serviceDescriber interface. +type MockserviceDescriber struct { + ctrl *gomock.Controller + recorder *MockserviceDescriberMockRecorder +} + +// MockserviceDescriberMockRecorder is the mock recorder for MockserviceDescriber. +type MockserviceDescriberMockRecorder struct { + mock *MockserviceDescriber +} + +// NewMockserviceDescriber creates a new mock instance. +func NewMockserviceDescriber(ctrl *gomock.Controller) *MockserviceDescriber { + mock := &MockserviceDescriber{ctrl: ctrl} + mock.recorder = &MockserviceDescriberMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockserviceDescriber) EXPECT() *MockserviceDescriberMockRecorder { + return m.recorder +} + +// ClusterARN mocks base method. +func (m *MockserviceDescriber) ClusterARN(app, env string) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ClusterARN", app, env) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ClusterARN indicates an expected call of ClusterARN. +func (mr *MockserviceDescriberMockRecorder) ClusterARN(app, env interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterARN", reflect.TypeOf((*MockserviceDescriber)(nil).ClusterARN), app, env) +} + +// NetworkConfiguration mocks base method. +func (m *MockserviceDescriber) NetworkConfiguration(app, env, svc string) (*ecs.NetworkConfiguration, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "NetworkConfiguration", app, env, svc) + ret0, _ := ret[0].(*ecs.NetworkConfiguration) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// NetworkConfiguration indicates an expected call of NetworkConfiguration. +func (mr *MockserviceDescriberMockRecorder) NetworkConfiguration(app, env, svc interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NetworkConfiguration", reflect.TypeOf((*MockserviceDescriber)(nil).NetworkConfiguration), app, env, svc) +} + +// TaskDefinition mocks base method. +func (m *MockserviceDescriber) TaskDefinition(app, env, svc string) (*ecs.TaskDefinition, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "TaskDefinition", app, env, svc) + ret0, _ := ret[0].(*ecs.TaskDefinition) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// TaskDefinition indicates an expected call of TaskDefinition. +func (mr *MockserviceDescriberMockRecorder) TaskDefinition(app, env, svc interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TaskDefinition", reflect.TypeOf((*MockserviceDescriber)(nil).TaskDefinition), app, env, svc) +} diff --git a/internal/pkg/ecs/run_task_request.go b/internal/pkg/ecs/run_task_request.go new file mode 100644 index 00000000000..59164eaea52 --- /dev/null +++ b/internal/pkg/ecs/run_task_request.go @@ -0,0 +1,128 @@ +package ecs + +import ( + "fmt" + + "github.com/aws/copilot-cli/internal/pkg/cli" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/copilot-cli/internal/pkg/aws/ecs" +) + +type ecsServiceDescriber interface { + Service(clusterName, serviceName string) (*ecs.Service, error) + TaskDefinition(taskDefName string) (*ecs.TaskDefinition, error) + NetworkConfiguration(cluster, serviceName string) (*ecs.NetworkConfiguration, error) +} + +type serviceDescriber interface { + TaskDefinition(app, env, svc string) (*ecs.TaskDefinition, error) + NetworkConfiguration(app, env, svc string) (*ecs.NetworkConfiguration, error) + ClusterARN(app, env string) (string, error) +} + +func RunTaskRequestFromECSService(client ecsServiceDescriber, cluster, service string) (*cli.RunTaskRequest, error) { + networkConfig, err := client.NetworkConfiguration(cluster, service) + if err != nil { + return nil, fmt.Errorf("retrieve network configuration for service %s in cluster %s: %w", service, cluster, err) + } + + svc, err := client.Service(cluster, service) + if err != nil { + return nil, fmt.Errorf("retrieve service %s in cluster %s: %w", service, cluster, err) + } + + taskDefNameOrARN := aws.StringValue(svc.TaskDefinition) + taskDef, err := client.TaskDefinition(taskDefNameOrARN) + if err != nil { + return nil, fmt.Errorf("retrieve task definition %s: %w", taskDefNameOrARN, err) + } + + if len(taskDef.ContainerDefinitions) > 1 { + return nil, fmt.Errorf("found more than one container in task definition: %s", taskDefNameOrARN) + } + + containerName := aws.StringValue(taskDef.ContainerDefinitions[0].Name) + containerInfo, err := containerInformation(taskDef, containerName) + if err != nil { + return nil, err + } + + return &cli.RunTaskRequest{ + NetworkConfiguration: *networkConfig, + ExecutionRole: aws.StringValue(taskDef.ExecutionRoleArn), + TaskRole: aws.StringValue(taskDef.TaskRoleArn), + ContainerInfo: *containerInfo, + Cluster: cluster, + }, nil +} + +func RunTaskRequestFromService(client serviceDescriber, app, env, svc string) (*cli.RunTaskRequest, error) { + networkConfig, err := client.NetworkConfiguration(app, env, svc) + if err != nil { + return nil, fmt.Errorf("retrieve network configuration for service %s: %w", svc, err) + } + + cluster, err := client.ClusterARN(app, env) + if err != nil { + return nil, fmt.Errorf("retrieve cluster ARN created for environment %s in application %s: %w", env, app, err) + } + + taskDef, err := client.TaskDefinition(app, env, svc) + if err != nil { + return nil, fmt.Errorf("retrieve task definition for service %s: %w", svc, err) + } + + containerName := svc // NOTE: refer to workload's CloudFormation template. The container name is set to be the workload's name. + containerInfo, err := containerInformation(taskDef, containerName) + if err != nil { + return nil, err + } + + return &cli.RunTaskRequest{ + NetworkConfiguration: *networkConfig, + ExecutionRole: aws.StringValue(taskDef.ExecutionRoleArn), + TaskRole: aws.StringValue(taskDef.TaskRoleArn), + ContainerInfo: *containerInfo, + Cluster: cluster, + }, nil +} + +func containerInformation(taskDef *ecs.TaskDefinition, containerName string) (*cli.ContainerInfo, error) { + image, err := taskDef.Image(containerName) + if err != nil { + return nil, err + } + + entrypoint, err := taskDef.EntryPoint(containerName) + if err != nil { + return nil, err + } + + command, err := taskDef.Command(containerName) + if err != nil { + return nil, err + } + + envVars := make(map[string]string) + for _, envVar := range taskDef.EnvironmentVariables() { + if envVar.Container == containerName { + envVars[envVar.Name] = envVar.Value + } + } + + secrets := make(map[string]string) + for _, secret := range taskDef.Secrets() { + if secret.Container == containerName { + secrets[secret.Name] = secret.ValueFrom + } + } + + return &cli.ContainerInfo{ + Image: image, + EntryPoint: entrypoint, + Command: command, + EnvVars: envVars, + Secrets: secrets, + }, nil +} diff --git a/internal/pkg/ecs/run_task_request_test.go b/internal/pkg/ecs/run_task_request_test.go new file mode 100644 index 00000000000..6b9198d1334 --- /dev/null +++ b/internal/pkg/ecs/run_task_request_test.go @@ -0,0 +1,280 @@ +package ecs + +import ( + "errors" + "testing" + + "github.com/aws/copilot-cli/internal/pkg/ecs/mocks" + + "github.com/aws/aws-sdk-go/aws" + awsecs "github.com/aws/aws-sdk-go/service/ecs" + "github.com/aws/copilot-cli/internal/pkg/aws/ecs" + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" +) + +func Test_RunTaskRequestFromECSService(t *testing.T) { + var ( + testCluster = "crowded-cluster" + testService = "good-service" + ) + testCases := map[string]struct { + setUpMock func(m *mocks.MockecsServiceDescriber) + + wantedGenerateCommandOpts *RunTaskRequest + wantedError error + }{ + "success": { + setUpMock: func(m *mocks.MockecsServiceDescriber) { + m.EXPECT().Service(testCluster, testService).Return(&ecs.Service{ + TaskDefinition: aws.String("task-def"), + }, nil) + m.EXPECT().TaskDefinition("task-def").Return(&ecs.TaskDefinition{ + ExecutionRoleArn: aws.String("execution-role"), + TaskRoleArn: aws.String("task-role"), + ContainerDefinitions: []*awsecs.ContainerDefinition{ + { + Name: aws.String("the-one-and-only-one-container"), + Image: aws.String("beautiful-image"), + EntryPoint: aws.StringSlice([]string{"enter", "here"}), + Command: aws.StringSlice([]string{"do", "not", "enter", "here"}), + Environment: []*awsecs.KeyValuePair{ + { + Name: aws.String("enter"), + Value: aws.String("no"), + }, + { + Name: aws.String("kidding"), + Value: aws.String("yes"), + }, + }, + Secrets: []*awsecs.Secret{ + { + Name: aws.String("truth"), + ValueFrom: aws.String("go-ask-the-wise"), + }, + }, + }, + }, + }, nil) + m.EXPECT().NetworkConfiguration(testCluster, testService).Return(&ecs.NetworkConfiguration{ + AssignPublicIp: "1.2.3.4", + Subnets: []string{"sbn-1", "sbn-2"}, + SecurityGroups: []string{"sg-1", "sg-2"}, + }, nil) + }, + wantedGenerateCommandOpts: &RunTaskRequest{ + networkConfiguration: ecs.NetworkConfiguration{ + AssignPublicIp: "1.2.3.4", + Subnets: []string{"sbn-1", "sbn-2"}, + SecurityGroups: []string{"sg-1", "sg-2"}, + }, + + executionRole: "execution-role", + taskRole: "task-role", + + containerInfo: containerInfo{ + image: "beautiful-image", + entryPoint: []string{"enter", "here"}, + command: []string{"do", "not", "enter", "here"}, + envVars: map[string]string{ + "enter": "no", + "kidding": "yes", + }, + secrets: map[string]string{ + "truth": "go-ask-the-wise", + }, + }, + + cluster: testCluster, + }, + }, + "unable to retrieve service": { + setUpMock: func(m *mocks.MockecsServiceDescriber) { + m.EXPECT().Service(testCluster, testService).Return(nil, errors.New("some error")) + m.EXPECT().NetworkConfiguration(gomock.Any(), gomock.Any()).AnyTimes() + }, + wantedError: errors.New("retrieve service good-service in cluster crowded-cluster: some error"), + }, + "unable to retrieve task definition": { + setUpMock: func(m *mocks.MockecsServiceDescriber) { + m.EXPECT().Service(testCluster, testService).Return(&ecs.Service{ + TaskDefinition: aws.String("task-def"), + }, nil) + m.EXPECT().TaskDefinition("task-def").Return(nil, errors.New("some error")) + m.EXPECT().NetworkConfiguration(gomock.Any(), gomock.Any()).AnyTimes() + }, + wantedError: errors.New("retrieve task definition task-def: some error"), + }, + "unable to retrieve network configuration": { + setUpMock: func(m *mocks.MockecsServiceDescriber) { + m.EXPECT().Service(gomock.Any(), gomock.Any()).AnyTimes() + m.EXPECT().TaskDefinition(gomock.Any()).AnyTimes() + m.EXPECT().NetworkConfiguration(testCluster, testService).Return(nil, errors.New("some error")) + }, + wantedError: errors.New("retrieve network configuration for service good-service in cluster crowded-cluster: some error"), + }, + "error if found more than one container": { + setUpMock: func(m *mocks.MockecsServiceDescriber) { + m.EXPECT().Service(testCluster, testService).Return(&ecs.Service{ + TaskDefinition: aws.String("task-def"), + }, nil) + m.EXPECT().TaskDefinition("task-def").Return(&ecs.TaskDefinition{ + ExecutionRoleArn: aws.String("execution-role"), + TaskRoleArn: aws.String("task-role"), + ContainerDefinitions: []*awsecs.ContainerDefinition{ + { + Name: aws.String("the-first-container"), + }, + { + Name: aws.String("sad-container"), + }, + }, + }, nil) + m.EXPECT().NetworkConfiguration(gomock.Any(), gomock.Any()).AnyTimes() + }, + wantedError: errors.New("found more than one container in task definition: task-def"), + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + m := mocks.NewMockecsServiceDescriber(ctrl) + tc.setUpMock(m) + + got, err := RunTaskRequestFromECSService(m, testCluster, testService) + if tc.wantedError != nil { + require.EqualError(t, tc.wantedError, err.Error()) + } else { + require.NoError(t, err) + require.Equal(t, tc.wantedGenerateCommandOpts, got) + } + }) + } +} + +func Test_RunTaskRequestFromService(t *testing.T) { + var ( + testApp = "app" + testEnv = "env" + testSvc = "svc" + ) + testCases := map[string]struct { + setUpMock func(m *mocks.MockserviceDescriber) + + wantedGenerateCommandOpts *RunTaskRequest + wantedError error + }{ + "returns generateCommandOpts with service's main container": { + setUpMock: func(m *mocks.MockserviceDescriber) { + m.EXPECT().TaskDefinition(testApp, testEnv, testSvc).Return(&ecs.TaskDefinition{ + ExecutionRoleArn: aws.String("execution-role"), + TaskRoleArn: aws.String("task-role"), + ContainerDefinitions: []*awsecs.ContainerDefinition{ + { + Name: aws.String(testSvc), + Image: aws.String("beautiful-image"), + EntryPoint: aws.StringSlice([]string{"enter", "here"}), + Command: aws.StringSlice([]string{"do", "not", "enter", "here"}), + Environment: []*awsecs.KeyValuePair{ + { + Name: aws.String("enter"), + Value: aws.String("no"), + }, + { + Name: aws.String("kidding"), + Value: aws.String("yes"), + }, + }, + Secrets: []*awsecs.Secret{ + { + Name: aws.String("truth"), + ValueFrom: aws.String("go-ask-the-wise"), + }, + }, + }, + { + Name: aws.String("random-container-that-we-do-not-care"), + }, + }, + }, nil) + m.EXPECT().NetworkConfiguration(testApp, testEnv, testSvc).Return(&ecs.NetworkConfiguration{ + AssignPublicIp: "1.2.3.4", + Subnets: []string{"sbn-1", "sbn-2"}, + SecurityGroups: []string{"sg-1", "sg-2"}, + }, nil) + m.EXPECT().ClusterARN(testApp, testEnv).Return("kamura-village", nil) + }, + wantedGenerateCommandOpts: &RunTaskRequest{ + networkConfiguration: ecs.NetworkConfiguration{ + AssignPublicIp: "1.2.3.4", + Subnets: []string{"sbn-1", "sbn-2"}, + SecurityGroups: []string{"sg-1", "sg-2"}, + }, + + executionRole: "execution-role", + taskRole: "task-role", + + containerInfo: containerInfo{ + image: "beautiful-image", + entryPoint: []string{"enter", "here"}, + command: []string{"do", "not", "enter", "here"}, + envVars: map[string]string{ + "enter": "no", + "kidding": "yes", + }, + secrets: map[string]string{ + "truth": "go-ask-the-wise", + }, + }, + + cluster: "kamura-village", + }, + }, + "unable to retrieve task definition": { + setUpMock: func(m *mocks.MockserviceDescriber) { + m.EXPECT().TaskDefinition(testApp, testEnv, testSvc).Return(nil, errors.New("some error")) + m.EXPECT().NetworkConfiguration(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + m.EXPECT().ClusterARN(gomock.Any(), gomock.Any()).AnyTimes() + }, + wantedError: errors.New("retrieve task definition for service svc: some error"), + }, + "unable to retrieve network configuration": { + setUpMock: func(m *mocks.MockserviceDescriber) { + m.EXPECT().TaskDefinition(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + m.EXPECT().NetworkConfiguration(testApp, testEnv, testSvc).Return(nil, errors.New("some error")) + m.EXPECT().ClusterARN(gomock.Any(), gomock.Any()).AnyTimes() + }, + wantedError: errors.New("retrieve network configuration for service svc: some error"), + }, + "unable to obtain cluster ARN": { + setUpMock: func(m *mocks.MockserviceDescriber) { + m.EXPECT().TaskDefinition(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + m.EXPECT().NetworkConfiguration(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() + m.EXPECT().ClusterARN(testApp, testEnv).Return("", errors.New("some error")) + }, + wantedError: errors.New("retrieve cluster ARN created for environment env in application app: some error"), + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + m := mocks.NewMockserviceDescriber(ctrl) + tc.setUpMock(m) + + got, err := RunTaskRequestFromService(m, testApp, testEnv, testSvc) + if tc.wantedError != nil { + require.EqualError(t, tc.wantedError, err.Error()) + } else { + require.NoError(t, err) + require.Equal(t, tc.wantedGenerateCommandOpts, got) + } + }) + } +} diff --git a/internal/pkg/generator/ecs_service.go b/internal/pkg/generator/ecs_service.go deleted file mode 100644 index 3cd6daf2f7e..00000000000 --- a/internal/pkg/generator/ecs_service.go +++ /dev/null @@ -1,63 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -// Package generator generates a command given an ECS service or a workload. -package generator - -import ( - "fmt" - - "github.com/aws/copilot-cli/internal/pkg/aws/ecs" - - "github.com/aws/aws-sdk-go/aws" -) - -type ecsClient interface { - Service(clusterName, serviceName string) (*ecs.Service, error) - TaskDefinition(taskDefName string) (*ecs.TaskDefinition, error) - NetworkConfiguration(cluster, serviceName string) (*ecs.NetworkConfiguration, error) -} - -// ECSServiceCommandGenerator generates task run command given an ECS service. -type ECSServiceCommandGenerator struct { - Cluster string - Service string - ECSClient ecsClient -} - -// Generate generates a task run command. -func (g ECSServiceCommandGenerator) Generate() (*GenerateCommandOpts, error) { - networkConfig, err := g.ECSClient.NetworkConfiguration(g.Cluster, g.Service) - if err != nil { - return nil, fmt.Errorf("retrieve network configuration for service %s in cluster %s: %w", g.Service, g.Cluster, err) - } - - svc, err := g.ECSClient.Service(g.Cluster, g.Service) - if err != nil { - return nil, fmt.Errorf("retrieve service %s in cluster %s: %w", g.Service, g.Cluster, err) - } - - taskDefNameOrARN := aws.StringValue(svc.TaskDefinition) - taskDef, err := g.ECSClient.TaskDefinition(taskDefNameOrARN) - if err != nil { - return nil, fmt.Errorf("retrieve task definition %s: %w", taskDefNameOrARN, err) - } - - if len(taskDef.ContainerDefinitions) > 1 { - return nil, fmt.Errorf("found more than one container in task definition: %s", taskDefNameOrARN) - } - - containerName := aws.StringValue(taskDef.ContainerDefinitions[0].Name) - containerInfo, err := containerInformation(taskDef, containerName) - if err != nil { - return nil, err - } - - return &GenerateCommandOpts{ - networkConfiguration: *networkConfig, - executionRole: aws.StringValue(taskDef.ExecutionRoleArn), - taskRole: aws.StringValue(taskDef.TaskRoleArn), - containerInfo: *containerInfo, - cluster: g.Cluster, - }, nil -} diff --git a/internal/pkg/generator/ecs_service_test.go b/internal/pkg/generator/ecs_service_test.go deleted file mode 100644 index 2d3cfa604bd..00000000000 --- a/internal/pkg/generator/ecs_service_test.go +++ /dev/null @@ -1,170 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -// Package generator generates a command given an ECS service or a workload. -package generator - -import ( - "errors" - "testing" - - "github.com/stretchr/testify/require" - - "github.com/golang/mock/gomock" - - awsecs "github.com/aws/aws-sdk-go/service/ecs" - - "github.com/aws/aws-sdk-go/aws" - - "github.com/aws/copilot-cli/internal/pkg/aws/ecs" - "github.com/aws/copilot-cli/internal/pkg/generator/mocks" -) - -func TestECSServiceCommandGenerator_Generate(t *testing.T) { - var ( - testCluster = "crowded-cluster" - testService = "good-service" - ) - testCases := map[string]struct { - setUpMock func(m *mocks.MockecsServiceGetter) - - wantedGenerateCommandOpts *GenerateCommandOpts - wantedError error - }{ - "success": { - setUpMock: func(m *mocks.MockecsServiceGetter) { - m.EXPECT().Service(testCluster, testService).Return(&ecs.Service{ - TaskDefinition: aws.String("task-def"), - }, nil) - m.EXPECT().TaskDefinition("task-def").Return(&ecs.TaskDefinition{ - ExecutionRoleArn: aws.String("execution-role"), - TaskRoleArn: aws.String("task-role"), - ContainerDefinitions: []*awsecs.ContainerDefinition{ - { - Name: aws.String("the-one-and-only-one-container"), - Image: aws.String("beautiful-image"), - EntryPoint: aws.StringSlice([]string{"enter", "here"}), - Command: aws.StringSlice([]string{"do", "not", "enter", "here"}), - Environment: []*awsecs.KeyValuePair{ - { - Name: aws.String("enter"), - Value: aws.String("no"), - }, - { - Name: aws.String("kidding"), - Value: aws.String("yes"), - }, - }, - Secrets: []*awsecs.Secret{ - { - Name: aws.String("truth"), - ValueFrom: aws.String("go-ask-the-wise"), - }, - }, - }, - }, - }, nil) - m.EXPECT().NetworkConfiguration(testCluster, testService).Return(&ecs.NetworkConfiguration{ - AssignPublicIp: "1.2.3.4", - Subnets: []string{"sbn-1", "sbn-2"}, - SecurityGroups: []string{"sg-1", "sg-2"}, - }, nil) - }, - wantedGenerateCommandOpts: &GenerateCommandOpts{ - networkConfiguration: ecs.NetworkConfiguration{ - AssignPublicIp: "1.2.3.4", - Subnets: []string{"sbn-1", "sbn-2"}, - SecurityGroups: []string{"sg-1", "sg-2"}, - }, - - executionRole: "execution-role", - taskRole: "task-role", - - containerInfo: containerInfo{ - image: "beautiful-image", - entryPoint: []string{"enter", "here"}, - command: []string{"do", "not", "enter", "here"}, - envVars: map[string]string{ - "enter": "no", - "kidding": "yes", - }, - secrets: map[string]string{ - "truth": "go-ask-the-wise", - }, - }, - - cluster: testCluster, - }, - }, - "unable to retrieve service": { - setUpMock: func(m *mocks.MockecsServiceGetter) { - m.EXPECT().Service(testCluster, testService).Return(nil, errors.New("some error")) - m.EXPECT().NetworkConfiguration(gomock.Any(), gomock.Any()).AnyTimes() - }, - wantedError: errors.New("retrieve service good-service in cluster crowded-cluster: some error"), - }, - "unable to retrieve task definition": { - setUpMock: func(m *mocks.MockecsServiceGetter) { - m.EXPECT().Service(testCluster, testService).Return(&ecs.Service{ - TaskDefinition: aws.String("task-def"), - }, nil) - m.EXPECT().TaskDefinition("task-def").Return(nil, errors.New("some error")) - m.EXPECT().NetworkConfiguration(gomock.Any(), gomock.Any()).AnyTimes() - }, - wantedError: errors.New("retrieve task definition task-def: some error"), - }, - "unable to retrieve network configuration": { - setUpMock: func(m *mocks.MockecsServiceGetter) { - m.EXPECT().Service(gomock.Any(), gomock.Any()).AnyTimes() - m.EXPECT().TaskDefinition(gomock.Any()).AnyTimes() - m.EXPECT().NetworkConfiguration(testCluster, testService).Return(nil, errors.New("some error")) - }, - wantedError: errors.New("retrieve network configuration for service good-service in cluster crowded-cluster: some error"), - }, - "error if found more than one container": { - setUpMock: func(m *mocks.MockecsServiceGetter) { - m.EXPECT().Service(testCluster, testService).Return(&ecs.Service{ - TaskDefinition: aws.String("task-def"), - }, nil) - m.EXPECT().TaskDefinition("task-def").Return(&ecs.TaskDefinition{ - ExecutionRoleArn: aws.String("execution-role"), - TaskRoleArn: aws.String("task-role"), - ContainerDefinitions: []*awsecs.ContainerDefinition{ - { - Name: aws.String("the-first-container"), - }, - { - Name: aws.String("sad-container"), - }, - }, - }, nil) - m.EXPECT().NetworkConfiguration(gomock.Any(), gomock.Any()).AnyTimes() - }, - wantedError: errors.New("found more than one container in task definition: task-def"), - }, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - m := mocks.NewMockecsServiceGetter(ctrl) - tc.setUpMock(m) - - g := ECSServiceCommandGenerator{ - Cluster: testCluster, - Service: testService, - ECSClient: m, - } - - got, err := g.Generate() - if tc.wantedError != nil { - require.EqualError(t, tc.wantedError, err.Error()) - } else { - require.NoError(t, err) - require.Equal(t, tc.wantedGenerateCommandOpts, got) - } - }) - } -} diff --git a/internal/pkg/generator/generate.go b/internal/pkg/generator/generate.go deleted file mode 100644 index 94ffe7d9afe..00000000000 --- a/internal/pkg/generator/generate.go +++ /dev/null @@ -1,134 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -// Package generator generates a command given an ECS service or a workload. -package generator - -import ( - "fmt" - "sort" - "strings" - - "github.com/aws/copilot-cli/internal/pkg/aws/ecs" -) - -// GenerateCommandOpts contains information to generate a task run command. -type GenerateCommandOpts struct { - networkConfiguration ecs.NetworkConfiguration - - executionRole string - taskRole string - cluster string - - containerInfo -} - -type containerInfo struct { - image string - entryPoint []string - command []string - envVars map[string]string - secrets map[string]string -} - -func containerInformation(taskDef *ecs.TaskDefinition, containerName string) (*containerInfo, error) { - image, err := taskDef.Image(containerName) - if err != nil { - return nil, err - } - - entrypoint, err := taskDef.EntryPoint(containerName) - if err != nil { - return nil, err - } - - command, err := taskDef.Command(containerName) - if err != nil { - return nil, err - } - - envVars := make(map[string]string) - for _, envVar := range taskDef.EnvironmentVariables() { - if envVar.Container == containerName { - envVars[envVar.Name] = envVar.Value - } - } - - secrets := make(map[string]string) - for _, secret := range taskDef.Secrets() { - if secret.Container == containerName { - secrets[secret.Name] = secret.ValueFrom - } - } - - return &containerInfo{ - image: image, - entryPoint: entrypoint, - command: command, - envVars: envVars, - secrets: secrets, - }, nil -} - -// String stringifies a GenerateCommandOpts. -func (o GenerateCommandOpts) String() string { - output := []string{"copilot task run"} - if o.executionRole != "" { - output = append(output, fmt.Sprintf("--execution-role %s", o.executionRole)) - } - - if o.taskRole != "" { - output = append(output, fmt.Sprintf("--task-role %s", o.taskRole)) - } - - if o.image != "" { - output = append(output, fmt.Sprintf("--image %s", o.image)) - } - - if o.entryPoint != nil { - output = append(output, fmt.Sprintf("--entrypoint %s", fmt.Sprintf("\"%s\"", strings.Join(o.entryPoint, " ")))) - } - - if o.command != nil { - output = append(output, fmt.Sprintf("--command %s", fmt.Sprintf("\"%s\"", strings.Join(o.command, " ")))) - } - - if o.envVars != nil && len(o.envVars) != 0 { - output = append(output, fmt.Sprintf("--env-vars %s", fmtStringMapToString(o.envVars))) - } - - if o.secrets != nil && len(o.secrets) != 0 { - output = append(output, fmt.Sprintf("--secrets %s", fmtStringMapToString(o.secrets))) - } - - if o.networkConfiguration.Subnets != nil && len(o.networkConfiguration.Subnets) != 0 { - output = append(output, fmt.Sprintf("--subnets %s", strings.Join(o.networkConfiguration.Subnets, ","))) - } - - if o.networkConfiguration.SecurityGroups != nil && len(o.networkConfiguration.SecurityGroups) != 0 { - output = append(output, fmt.Sprintf("--security-groups %s", strings.Join(o.networkConfiguration.SecurityGroups, ","))) - } - - if o.cluster != "" { - output = append(output, fmt.Sprintf("--cluster %s", o.cluster)) - } - - return strings.Join(output, " \\\n") -} - -// This function will format a map to a string as "key1=value1,key2=value2,key3=value3". -func fmtStringMapToString(m map[string]string) string { - var output []string - - // Sort the map so that `output` is consistent and the unit test won't be flaky. - keys := make([]string, 0, len(m)) - for k := range m { - keys = append(keys, k) - } - sort.Strings(keys) - - for _, k := range keys { - output = append(output, fmt.Sprintf("%s=%v", k, m[k])) - } - return strings.Join(output, ",") -} diff --git a/internal/pkg/generator/generate_test.go b/internal/pkg/generator/generate_test.go deleted file mode 100644 index f64ed5fb07c..00000000000 --- a/internal/pkg/generator/generate_test.go +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -// Package generator generates a command given an ECS service or a workload. -package generator - -import ( - "testing" - - "github.com/aws/copilot-cli/internal/pkg/aws/ecs" - - "github.com/stretchr/testify/require" -) - -func TestGenerateCommandOpts_String(t *testing.T) { - testCases := map[string]struct { - inGenerateCommandOpts GenerateCommandOpts - wantedCommand string - }{ - "return the correct command string": { - inGenerateCommandOpts: GenerateCommandOpts{ - networkConfiguration: ecs.NetworkConfiguration{ - AssignPublicIp: "1.2.3.4", - Subnets: []string{"sbn-1", "sbn-2"}, - SecurityGroups: []string{"sg-1", "sg-2"}, - }, - executionRole: "good-doggo", - taskRole: "good-kitty", - - containerInfo: containerInfo{ - image: "beautiful-image", - entryPoint: []string{"enter", "from", "here"}, - command: []string{"do", "not", "enter"}, - envVars: map[string]string{ - "weather": "snowy", - "hasHotChocolate": "yes", - }, - secrets: map[string]string{ - "truth": "ask-the-wise", - "lie": "ask-the-villagers", - }, - }, - - cluster: "kamura-village", - }, - wantedCommand: `copilot task run \ ---execution-role good-doggo \ ---task-role good-kitty \ ---image beautiful-image \ ---entrypoint "enter from here" \ ---command "do not enter" \ ---env-vars hasHotChocolate=yes,weather=snowy \ ---secrets lie=ask-the-villagers,truth=ask-the-wise \ ---subnets sbn-1,sbn-2 \ ---security-groups sg-1,sg-2 \ ---cluster kamura-village`, - }, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - opts := tc.inGenerateCommandOpts - got := opts.String() - require.Equal(t, tc.wantedCommand, got) - }) - } -} diff --git a/internal/pkg/generator/mocks/mock_ecs_service.go b/internal/pkg/generator/mocks/mock_ecs_service.go deleted file mode 100644 index 88b36b15ffd..00000000000 --- a/internal/pkg/generator/mocks/mock_ecs_service.go +++ /dev/null @@ -1,80 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: ./internal/pkg/generator/ecs_service.go - -// Package mocks is a generated GoMock package. -package mocks - -import ( - reflect "reflect" - - ecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" - gomock "github.com/golang/mock/gomock" -) - -// MockecsServiceGetter is a mock of ecsServiceGetter interface. -type MockecsServiceGetter struct { - ctrl *gomock.Controller - recorder *MockecsServiceGetterMockRecorder -} - -// MockecsServiceGetterMockRecorder is the mock recorder for MockecsServiceGetter. -type MockecsServiceGetterMockRecorder struct { - mock *MockecsServiceGetter -} - -// NewMockecsServiceGetter creates a new mock instance. -func NewMockecsServiceGetter(ctrl *gomock.Controller) *MockecsServiceGetter { - mock := &MockecsServiceGetter{ctrl: ctrl} - mock.recorder = &MockecsServiceGetterMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockecsServiceGetter) EXPECT() *MockecsServiceGetterMockRecorder { - return m.recorder -} - -// NetworkConfiguration mocks base method. -func (m *MockecsServiceGetter) NetworkConfiguration(cluster, serviceName string) (*ecs.NetworkConfiguration, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "NetworkConfiguration", cluster, serviceName) - ret0, _ := ret[0].(*ecs.NetworkConfiguration) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// NetworkConfiguration indicates an expected call of NetworkConfiguration. -func (mr *MockecsServiceGetterMockRecorder) NetworkConfiguration(cluster, serviceName interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NetworkConfiguration", reflect.TypeOf((*MockecsServiceGetter)(nil).NetworkConfiguration), cluster, serviceName) -} - -// Service mocks base method. -func (m *MockecsServiceGetter) Service(clusterName, serviceName string) (*ecs.Service, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "Service", clusterName, serviceName) - ret0, _ := ret[0].(*ecs.Service) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// Service indicates an expected call of Service. -func (mr *MockecsServiceGetterMockRecorder) Service(clusterName, serviceName interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Service", reflect.TypeOf((*MockecsServiceGetter)(nil).Service), clusterName, serviceName) -} - -// TaskDefinition mocks base method. -func (m *MockecsServiceGetter) TaskDefinition(taskDefName string) (*ecs.TaskDefinition, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "TaskDefinition", taskDefName) - ret0, _ := ret[0].(*ecs.TaskDefinition) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// TaskDefinition indicates an expected call of TaskDefinition. -func (mr *MockecsServiceGetterMockRecorder) TaskDefinition(taskDefName interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TaskDefinition", reflect.TypeOf((*MockecsServiceGetter)(nil).TaskDefinition), taskDefName) -} diff --git a/internal/pkg/generator/mocks/mock_service.go b/internal/pkg/generator/mocks/mock_service.go deleted file mode 100644 index 4f736d0dea9..00000000000 --- a/internal/pkg/generator/mocks/mock_service.go +++ /dev/null @@ -1,80 +0,0 @@ -// Code generated by MockGen. DO NOT EDIT. -// Source: ./internal/pkg/generator/service.go - -// Package mocks is a generated GoMock package. -package mocks - -import ( - reflect "reflect" - - ecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" - gomock "github.com/golang/mock/gomock" -) - -// MockecsInformationGetter is a mock of ecsInformationGetter interface. -type MockecsInformationGetter struct { - ctrl *gomock.Controller - recorder *MockecsInformationGetterMockRecorder -} - -// MockecsInformationGetterMockRecorder is the mock recorder for MockecsInformationGetter. -type MockecsInformationGetterMockRecorder struct { - mock *MockecsInformationGetter -} - -// NewMockecsInformationGetter creates a new mock instance. -func NewMockecsInformationGetter(ctrl *gomock.Controller) *MockecsInformationGetter { - mock := &MockecsInformationGetter{ctrl: ctrl} - mock.recorder = &MockecsInformationGetterMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockecsInformationGetter) EXPECT() *MockecsInformationGetterMockRecorder { - return m.recorder -} - -// ClusterARN mocks base method. -func (m *MockecsInformationGetter) ClusterARN(app, env string) (string, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ClusterARN", app, env) - ret0, _ := ret[0].(string) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// ClusterARN indicates an expected call of ClusterARN. -func (mr *MockecsInformationGetterMockRecorder) ClusterARN(app, env interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ClusterARN", reflect.TypeOf((*MockecsInformationGetter)(nil).ClusterARN), app, env) -} - -// NetworkConfiguration mocks base method. -func (m *MockecsInformationGetter) NetworkConfiguration(app, env, svc string) (*ecs.NetworkConfiguration, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "NetworkConfiguration", app, env, svc) - ret0, _ := ret[0].(*ecs.NetworkConfiguration) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// NetworkConfiguration indicates an expected call of NetworkConfiguration. -func (mr *MockecsInformationGetterMockRecorder) NetworkConfiguration(app, env, svc interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "NetworkConfiguration", reflect.TypeOf((*MockecsInformationGetter)(nil).NetworkConfiguration), app, env, svc) -} - -// TaskDefinition mocks base method. -func (m *MockecsInformationGetter) TaskDefinition(app, env, svc string) (*ecs.TaskDefinition, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "TaskDefinition", app, env, svc) - ret0, _ := ret[0].(*ecs.TaskDefinition) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// TaskDefinition indicates an expected call of TaskDefinition. -func (mr *MockecsInformationGetterMockRecorder) TaskDefinition(app, env, svc interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TaskDefinition", reflect.TypeOf((*MockecsInformationGetter)(nil).TaskDefinition), app, env, svc) -} diff --git a/internal/pkg/generator/service.go b/internal/pkg/generator/service.go deleted file mode 100644 index c9c8d2079c0..00000000000 --- a/internal/pkg/generator/service.go +++ /dev/null @@ -1,58 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -// Package generator generates a command given an ECS service or a workload. -package generator - -import ( - "fmt" - - "github.com/aws/aws-sdk-go/aws" - "github.com/aws/copilot-cli/internal/pkg/aws/ecs" -) - -type ecsInformationGetter interface { - TaskDefinition(app, env, svc string) (*ecs.TaskDefinition, error) - NetworkConfiguration(app, env, svc string) (*ecs.NetworkConfiguration, error) - ClusterARN(app, env string) (string, error) -} - -// ServiceCommandGenerator generates task run command given a Copilot service. -type ServiceCommandGenerator struct { - App string - Env string - Service string - ECSInformationGetter ecsInformationGetter -} - -// Generate generates a task run command. -func (g ServiceCommandGenerator) Generate() (*GenerateCommandOpts, error) { - networkConfig, err := g.ECSInformationGetter.NetworkConfiguration(g.App, g.Env, g.Service) - if err != nil { - return nil, fmt.Errorf("retrieve network configuration for service %s: %w", g.Service, err) - } - - cluster, err := g.ECSInformationGetter.ClusterARN(g.App, g.Env) - if err != nil { - return nil, fmt.Errorf("retrieve cluster ARN created for environment %s in application %s: %w", g.Env, g.App, err) - } - - taskDef, err := g.ECSInformationGetter.TaskDefinition(g.App, g.Env, g.Service) - if err != nil { - return nil, fmt.Errorf("retrieve task definition for service %s: %w", g.Service, err) - } - - containerName := g.Service // NOTE: refer to workload's CloudFormation template. The container name is set to be the workload's name. - containerInfo, err := containerInformation(taskDef, containerName) - if err != nil { - return nil, err - } - - return &GenerateCommandOpts{ - networkConfiguration: *networkConfig, - executionRole: aws.StringValue(taskDef.ExecutionRoleArn), - taskRole: aws.StringValue(taskDef.TaskRoleArn), - containerInfo: *containerInfo, - cluster: cluster, - }, nil -} diff --git a/internal/pkg/generator/service_test.go b/internal/pkg/generator/service_test.go deleted file mode 100644 index 5c2692c134d..00000000000 --- a/internal/pkg/generator/service_test.go +++ /dev/null @@ -1,149 +0,0 @@ -// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. -// SPDX-License-Identifier: Apache-2.0 - -// Package generator generates a command given an ECS service or a workload. -package generator - -import ( - "errors" - "testing" - - "github.com/aws/aws-sdk-go/aws" - awsecs "github.com/aws/aws-sdk-go/service/ecs" - "github.com/aws/copilot-cli/internal/pkg/aws/ecs" - - "github.com/aws/copilot-cli/internal/pkg/generator/mocks" - "github.com/golang/mock/gomock" - "github.com/stretchr/testify/require" -) - -func TestServiceCommandGenerator_Generate(t *testing.T) { - var ( - testApp = "app" - testEnv = "env" - testSvc = "svc" - ) - testCases := map[string]struct { - setUpMock func(m *mocks.MockecsInformationGetter) - - wantedGenerateCommandOpts *GenerateCommandOpts - wantedError error - }{ - "returns generateCommandOpts with service's main container": { - setUpMock: func(m *mocks.MockecsInformationGetter) { - m.EXPECT().TaskDefinition(testApp, testEnv, testSvc).Return(&ecs.TaskDefinition{ - ExecutionRoleArn: aws.String("execution-role"), - TaskRoleArn: aws.String("task-role"), - ContainerDefinitions: []*awsecs.ContainerDefinition{ - { - Name: aws.String(testSvc), - Image: aws.String("beautiful-image"), - EntryPoint: aws.StringSlice([]string{"enter", "here"}), - Command: aws.StringSlice([]string{"do", "not", "enter", "here"}), - Environment: []*awsecs.KeyValuePair{ - { - Name: aws.String("enter"), - Value: aws.String("no"), - }, - { - Name: aws.String("kidding"), - Value: aws.String("yes"), - }, - }, - Secrets: []*awsecs.Secret{ - { - Name: aws.String("truth"), - ValueFrom: aws.String("go-ask-the-wise"), - }, - }, - }, - { - Name: aws.String("random-container-that-we-do-not-care"), - }, - }, - }, nil) - m.EXPECT().NetworkConfiguration(testApp, testEnv, testSvc).Return(&ecs.NetworkConfiguration{ - AssignPublicIp: "1.2.3.4", - Subnets: []string{"sbn-1", "sbn-2"}, - SecurityGroups: []string{"sg-1", "sg-2"}, - }, nil) - m.EXPECT().ClusterARN(testApp, testEnv).Return("kamura-village", nil) - }, - wantedGenerateCommandOpts: &GenerateCommandOpts{ - networkConfiguration: ecs.NetworkConfiguration{ - AssignPublicIp: "1.2.3.4", - Subnets: []string{"sbn-1", "sbn-2"}, - SecurityGroups: []string{"sg-1", "sg-2"}, - }, - - executionRole: "execution-role", - taskRole: "task-role", - - containerInfo: containerInfo{ - image: "beautiful-image", - entryPoint: []string{"enter", "here"}, - command: []string{"do", "not", "enter", "here"}, - envVars: map[string]string{ - "enter": "no", - "kidding": "yes", - }, - secrets: map[string]string{ - "truth": "go-ask-the-wise", - }, - }, - - cluster: "kamura-village", - }, - }, - "unable to retrieve task definition": { - setUpMock: func(m *mocks.MockecsInformationGetter) { - m.EXPECT().TaskDefinition(testApp, testEnv, testSvc).Return(nil, errors.New("some error")) - m.EXPECT().NetworkConfiguration(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() - m.EXPECT().ClusterARN(gomock.Any(), gomock.Any()).AnyTimes() - }, - wantedError: errors.New("retrieve task definition for service svc: some error"), - }, - "unable to retrieve network configuration": { - setUpMock: func(m *mocks.MockecsInformationGetter) { - m.EXPECT().TaskDefinition(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() - m.EXPECT().NetworkConfiguration(testApp, testEnv, testSvc).Return(nil, errors.New("some error")) - m.EXPECT().ClusterARN(gomock.Any(), gomock.Any()).AnyTimes() - }, - wantedError: errors.New("retrieve network configuration for service svc: some error"), - }, - "unable to obtain cluster ARN": { - setUpMock: func(m *mocks.MockecsInformationGetter) { - m.EXPECT().TaskDefinition(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() - m.EXPECT().NetworkConfiguration(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes() - m.EXPECT().ClusterARN(testApp, testEnv).Return("", errors.New("some error")) - }, - wantedError: errors.New("retrieve cluster ARN created for environment env in application app: some error"), - }, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - m := mocks.NewMockecsInformationGetter(ctrl) - tc.setUpMock(m) - - g := ServiceCommandGenerator{ - App: testApp, - Env: testEnv, - Service: testSvc, - - ECSInformationGetter: m, - } - - got, err := g.Generate() - if tc.wantedError != nil { - require.EqualError(t, tc.wantedError, err.Error()) - } else { - require.NoError(t, err) - require.Equal(t, tc.wantedGenerateCommandOpts, got) - } - }) - } -} From a17af885a1ce258bcb7483f3b57bf0503939a7d1 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Thu, 29 Apr 2021 16:19:09 -0500 Subject: [PATCH 08/18] update task run --- internal/pkg/cli/task_run.go | 52 ++++------ internal/pkg/cli/task_run_test.go | 78 --------------- internal/pkg/ecs/run_task_request.go | 140 +++++++++++++++++++++------ 3 files changed, 131 insertions(+), 139 deletions(-) diff --git a/internal/pkg/cli/task_run.go b/internal/pkg/cli/task_run.go index 058d93a69af..010cffa4a13 100644 --- a/internal/pkg/cli/task_run.go +++ b/internal/pkg/cli/task_run.go @@ -10,8 +10,6 @@ import ( "path/filepath" "strings" - "github.com/aws/copilot-cli/internal/pkg/generator" - "github.com/aws/aws-sdk-go/aws/arn" awscloudformation "github.com/aws/copilot-cli/internal/pkg/aws/cloudformation" @@ -53,10 +51,6 @@ const ( fmtImageURI = "%s:%s" ) -type cmdGenerator interface { - Generate() (*generator.GenerateCommandOpts, error) -} - var ( errNumNotPositive = errors.New("number of tasks must be positive") errCPUNotPositive = errors.New("CPU units must be positive") @@ -86,8 +80,8 @@ type runTaskVars struct { taskRole string executionRole string + cluster string - cluster string subnets []string securityGroups []string env string @@ -505,53 +499,47 @@ func (o *runTaskOpts) Execute() error { } func (o *runTaskOpts) generateCommand() error { - g, err := o.configureGenerator() + command, err := o.runTaskCommand() if err != nil { return err } - command, err := g.Generate() - if err != nil { - return fmt.Errorf("generate command: %w", err) - } log.Infoln(command.String()) return nil } -func (o *runTaskOpts) configureGenerator() (cmdGenerator, error) { - var g cmdGenerator +func (o *runTaskOpts) runTaskCommand() (*ecs.RunTaskRequest, error) { + var cmd *ecs.RunTaskRequest sess, err := sessions.NewProvider().Default() if err != nil { return nil, fmt.Errorf("get default session: %s", err) } if arn.IsARN(o.generateCommandTarget) { - return o.configureGeneratorFromARN(sess) + return o.runTaskCommandFromARN(sess) } parts := strings.Split(o.generateCommandTarget, "/") switch len(parts) { case 2: clusterName, serviceName := parts[0], parts[1] - g = generator.ECSServiceCommandGenerator{ - Cluster: clusterName, - Service: serviceName, - ECSClient: awsecs.New(sess), + cmd, err = ecs.RunTaskRequestFromECSService(awsecs.New(sess), clusterName, serviceName) + if err != nil { + return nil, fmt.Errorf("generate task run command from ECS service: %w", err) } case 3: appName, envName, serviceName := parts[0], parts[1], parts[2] - g = generator.ServiceCommandGenerator{ - App: appName, - Env: envName, - Service: serviceName, - ECSInformationGetter: ecs.New(sess), + cmd, err = ecs.RunTaskRequestFromService(ecs.New(sess), appName, envName, serviceName) + if err != nil { + return nil, fmt.Errorf("generate task run command from service: %w", err) } default: - return nil, errors.New("invalid input to --generate-cmd") + return nil, errors.New("invalid input to --generate-cmd: must be of the form / or //") } - return g, nil + + return cmd, nil } -func (o *runTaskOpts) configureGeneratorFromARN(sess *session.Session) (cmdGenerator, error) { +func (o *runTaskOpts) runTaskCommandFromARN(sess *session.Session) (*ecs.RunTaskRequest, error) { svcARN := awsecs.ServiceArn(o.generateCommandTarget) clusterName, err := svcARN.ClusterName() if err != nil { @@ -561,11 +549,11 @@ func (o *runTaskOpts) configureGeneratorFromARN(sess *session.Session) (cmdGener if err != nil { return nil, fmt.Errorf("extract service name from arn %s", svcARN) } - return generator.ECSServiceCommandGenerator{ - Cluster: clusterName, - Service: serviceName, - ECSClient: awsecs.New(sess), - }, nil + cmd, err := ecs.RunTaskRequestFromECSService(awsecs.New(sess), clusterName, serviceName) + if err != nil { + return nil, fmt.Errorf("generate task run command from ECS service: %w", err) + } + return cmd, nil } func (o *runTaskOpts) displayLogStream() error { diff --git a/internal/pkg/cli/task_run_test.go b/internal/pkg/cli/task_run_test.go index 0e30c4a5dfc..08b4392da55 100644 --- a/internal/pkg/cli/task_run_test.go +++ b/internal/pkg/cli/task_run_test.go @@ -9,10 +9,6 @@ import ( "path/filepath" "testing" - "github.com/aws/copilot-cli/internal/pkg/aws/sessions" - - "github.com/aws/copilot-cli/internal/pkg/generator" - "github.com/aws/copilot-cli/internal/pkg/cli/mocks" "github.com/aws/copilot-cli/internal/pkg/exec" @@ -907,77 +903,3 @@ func TestTaskRunOpts_Execute(t *testing.T) { }) } } - -func TestTaskRunOpts_configureGenerator(t *testing.T) { - testCases := map[string]struct { - inGenerateCommandTarget string - - wantedGenerator cmdGenerator - wantedError error - }{ - "should configure an ECS service command generator given an service ARN": { - inGenerateCommandTarget: "arn:aws:ecs:us-east-1:123456789012:service/crowded-cluster/good-service", - wantedGenerator: generator.ECSServiceCommandGenerator{ - Cluster: "crowded-cluster", - Service: "good-service", - }, - }, - "should configure an ECS service command generator given a cluster/service target": { - inGenerateCommandTarget: "crowded-cluster/good-service", - wantedGenerator: generator.ECSServiceCommandGenerator{ - Cluster: "crowded-cluster", - Service: "good-service", - }, - }, - "should configure a service command generator given an app/env/svc target": { - inGenerateCommandTarget: "good-app/good-env/good-service", - wantedGenerator: generator.ServiceCommandGenerator{ - App: "good-app", - Env: "good-env", - Service: "good-service", - }, - }, - "invalid input": { - inGenerateCommandTarget: "good-service", - wantedError: errors.New("invalid input to --generate-cmd"), - }, - } - - for name, tc := range testCases { - t.Run(name, func(t *testing.T) { - sess, _ := sessions.NewProvider().Default() - opts := &runTaskOpts{ - runTaskVars: runTaskVars{ - generateCommandTarget: tc.inGenerateCommandTarget, - }, - sess: sess, - } - - got, err := opts.configureGenerator() - if tc.wantedError != nil { - require.EqualError(t, tc.wantedError, err.Error()) - } else { - require.NoError(t, err) - require.True(t, equalGenerator(tc.wantedGenerator, got)) - } - }) - } -} - -func equalGenerator(wanted, got cmdGenerator) bool { - switch wantedG := wanted.(type) { - case generator.ECSServiceCommandGenerator: - gotG, ok := got.(generator.ECSServiceCommandGenerator) - if !ok { - return false - } - return gotG.Cluster == wantedG.Cluster && gotG.Service == wantedG.Service - case generator.ServiceCommandGenerator: - gotG, ok := got.(generator.ServiceCommandGenerator) - if !ok { - return false - } - return gotG.App == wantedG.App && gotG.Env == wantedG.Env && gotG.Service == wantedG.Service - } - return false -} diff --git a/internal/pkg/ecs/run_task_request.go b/internal/pkg/ecs/run_task_request.go index 59164eaea52..33e70c1112e 100644 --- a/internal/pkg/ecs/run_task_request.go +++ b/internal/pkg/ecs/run_task_request.go @@ -2,26 +2,45 @@ package ecs import ( "fmt" - - "github.com/aws/copilot-cli/internal/pkg/cli" + "sort" + "strings" "github.com/aws/aws-sdk-go/aws" - "github.com/aws/copilot-cli/internal/pkg/aws/ecs" + awsecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" ) type ecsServiceDescriber interface { - Service(clusterName, serviceName string) (*ecs.Service, error) - TaskDefinition(taskDefName string) (*ecs.TaskDefinition, error) - NetworkConfiguration(cluster, serviceName string) (*ecs.NetworkConfiguration, error) + Service(clusterName, serviceName string) (*awsecs.Service, error) + TaskDefinition(taskDefName string) (*awsecs.TaskDefinition, error) + NetworkConfiguration(cluster, serviceName string) (*awsecs.NetworkConfiguration, error) } type serviceDescriber interface { - TaskDefinition(app, env, svc string) (*ecs.TaskDefinition, error) - NetworkConfiguration(app, env, svc string) (*ecs.NetworkConfiguration, error) + TaskDefinition(app, env, svc string) (*awsecs.TaskDefinition, error) + NetworkConfiguration(app, env, svc string) (*awsecs.NetworkConfiguration, error) ClusterARN(app, env string) (string, error) } -func RunTaskRequestFromECSService(client ecsServiceDescriber, cluster, service string) (*cli.RunTaskRequest, error) { +// RunTaskRequest contains information to generate a task run command. +type RunTaskRequest struct { + networkConfiguration awsecs.NetworkConfiguration + + executionRole string + taskRole string + cluster string + + containerInfo +} + +type containerInfo struct { + image string + entryPoint []string + command []string + envVars map[string]string + secrets map[string]string +} + +func RunTaskRequestFromECSService(client ecsServiceDescriber, cluster, service string) (*RunTaskRequest, error) { networkConfig, err := client.NetworkConfiguration(cluster, service) if err != nil { return nil, fmt.Errorf("retrieve network configuration for service %s in cluster %s: %w", service, cluster, err) @@ -48,16 +67,16 @@ func RunTaskRequestFromECSService(client ecsServiceDescriber, cluster, service s return nil, err } - return &cli.RunTaskRequest{ - NetworkConfiguration: *networkConfig, - ExecutionRole: aws.StringValue(taskDef.ExecutionRoleArn), - TaskRole: aws.StringValue(taskDef.TaskRoleArn), - ContainerInfo: *containerInfo, - Cluster: cluster, + return &RunTaskRequest{ + networkConfiguration: *networkConfig, + executionRole: aws.StringValue(taskDef.ExecutionRoleArn), + taskRole: aws.StringValue(taskDef.TaskRoleArn), + containerInfo: *containerInfo, + cluster: cluster, }, nil } -func RunTaskRequestFromService(client serviceDescriber, app, env, svc string) (*cli.RunTaskRequest, error) { +func RunTaskRequestFromService(client serviceDescriber, app, env, svc string) (*RunTaskRequest, error) { networkConfig, err := client.NetworkConfiguration(app, env, svc) if err != nil { return nil, fmt.Errorf("retrieve network configuration for service %s: %w", svc, err) @@ -79,16 +98,16 @@ func RunTaskRequestFromService(client serviceDescriber, app, env, svc string) (* return nil, err } - return &cli.RunTaskRequest{ - NetworkConfiguration: *networkConfig, - ExecutionRole: aws.StringValue(taskDef.ExecutionRoleArn), - TaskRole: aws.StringValue(taskDef.TaskRoleArn), - ContainerInfo: *containerInfo, - Cluster: cluster, + return &RunTaskRequest{ + networkConfiguration: *networkConfig, + executionRole: aws.StringValue(taskDef.ExecutionRoleArn), + taskRole: aws.StringValue(taskDef.TaskRoleArn), + containerInfo: *containerInfo, + cluster: cluster, }, nil } -func containerInformation(taskDef *ecs.TaskDefinition, containerName string) (*cli.ContainerInfo, error) { +func containerInformation(taskDef *awsecs.TaskDefinition, containerName string) (*containerInfo, error) { image, err := taskDef.Image(containerName) if err != nil { return nil, err @@ -118,11 +137,74 @@ func containerInformation(taskDef *ecs.TaskDefinition, containerName string) (*c } } - return &cli.ContainerInfo{ - Image: image, - EntryPoint: entrypoint, - Command: command, - EnvVars: envVars, - Secrets: secrets, + return &containerInfo{ + image: image, + entryPoint: entrypoint, + command: command, + envVars: envVars, + secrets: secrets, }, nil } + +// String stringifies a GenerateCommandOpts. +func (r RunTaskRequest) String() string { + output := []string{"copilot task run"} + if r.executionRole != "" { + output = append(output, fmt.Sprintf("--execution-role %s", r.executionRole)) + } + + if r.taskRole != "" { + output = append(output, fmt.Sprintf("--task-role %s", r.taskRole)) + } + + if r.image != "" { + output = append(output, fmt.Sprintf("--image %s", r.image)) + } + + if r.entryPoint != nil { + output = append(output, fmt.Sprintf("--entrypoint %s", fmt.Sprintf("\"%s\"", strings.Join(r.entryPoint, " ")))) + } + + if r.command != nil { + output = append(output, fmt.Sprintf("--command %s", fmt.Sprintf("\"%s\"", strings.Join(r.command, " ")))) + } + + if r.envVars != nil && len(r.envVars) != 0 { + output = append(output, fmt.Sprintf("--env-vars %s", fmtStringMapToString(r.envVars))) + } + + if r.secrets != nil && len(r.secrets) != 0 { + output = append(output, fmt.Sprintf("--secrets %s", fmtStringMapToString(r.secrets))) + } + + if r.networkConfiguration.Subnets != nil && len(r.networkConfiguration.Subnets) != 0 { + output = append(output, fmt.Sprintf("--subnets %s", strings.Join(r.networkConfiguration.Subnets, ","))) + } + + if r.networkConfiguration.SecurityGroups != nil && len(r.networkConfiguration.SecurityGroups) != 0 { + output = append(output, fmt.Sprintf("--security-groups %s", strings.Join(r.networkConfiguration.SecurityGroups, ","))) + } + + if r.cluster != "" { + output = append(output, fmt.Sprintf("--cluster %s", r.cluster)) + } + + return strings.Join(output, " \\\n") +} + +// This function will format a map to a string as "key1=value1,key2=value2,key3=value3". +func fmtStringMapToString(m map[string]string) string { + var output []string + + // Sort the map so that `output` is consistent and the unit test won't be flaky. + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + + for _, k := range keys { + output = append(output, fmt.Sprintf("%s=%v", k, m[k])) + } + return strings.Join(output, ",") +} From 522fac58cdbc1c65e73a855a35b5bc4a0822e7cb Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Thu, 29 Apr 2021 16:28:25 -0500 Subject: [PATCH 09/18] update public function comment --- internal/pkg/ecs/run_task_request.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/pkg/ecs/run_task_request.go b/internal/pkg/ecs/run_task_request.go index 33e70c1112e..f9ff6b47c7d 100644 --- a/internal/pkg/ecs/run_task_request.go +++ b/internal/pkg/ecs/run_task_request.go @@ -40,6 +40,7 @@ type containerInfo struct { secrets map[string]string } +// RunTaskRequestFromECSService populates a RunTaskRequest with information from an ECS service. func RunTaskRequestFromECSService(client ecsServiceDescriber, cluster, service string) (*RunTaskRequest, error) { networkConfig, err := client.NetworkConfiguration(cluster, service) if err != nil { @@ -76,6 +77,7 @@ func RunTaskRequestFromECSService(client ecsServiceDescriber, cluster, service s }, nil } +// RunTaskRequestFromECSService populates a RunTaskRequest with information from a Copilot service. func RunTaskRequestFromService(client serviceDescriber, app, env, svc string) (*RunTaskRequest, error) { networkConfig, err := client.NetworkConfiguration(app, env, svc) if err != nil { @@ -146,7 +148,7 @@ func containerInformation(taskDef *awsecs.TaskDefinition, containerName string) }, nil } -// String stringifies a GenerateCommandOpts. +// String stringifies a RunTaskRequest. func (r RunTaskRequest) String() string { output := []string{"copilot task run"} if r.executionRole != "" { From c1a56031f8583e8984dc7c517126b0d53fc1bce7 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Fri, 30 Apr 2021 11:04:35 -0500 Subject: [PATCH 10/18] move private function to bottom --- internal/pkg/ecs/run_task_request.go | 78 ++++++++++++++-------------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/internal/pkg/ecs/run_task_request.go b/internal/pkg/ecs/run_task_request.go index f9ff6b47c7d..bb0d0590169 100644 --- a/internal/pkg/ecs/run_task_request.go +++ b/internal/pkg/ecs/run_task_request.go @@ -109,45 +109,6 @@ func RunTaskRequestFromService(client serviceDescriber, app, env, svc string) (* }, nil } -func containerInformation(taskDef *awsecs.TaskDefinition, containerName string) (*containerInfo, error) { - image, err := taskDef.Image(containerName) - if err != nil { - return nil, err - } - - entrypoint, err := taskDef.EntryPoint(containerName) - if err != nil { - return nil, err - } - - command, err := taskDef.Command(containerName) - if err != nil { - return nil, err - } - - envVars := make(map[string]string) - for _, envVar := range taskDef.EnvironmentVariables() { - if envVar.Container == containerName { - envVars[envVar.Name] = envVar.Value - } - } - - secrets := make(map[string]string) - for _, secret := range taskDef.Secrets() { - if secret.Container == containerName { - secrets[secret.Name] = secret.ValueFrom - } - } - - return &containerInfo{ - image: image, - entryPoint: entrypoint, - command: command, - envVars: envVars, - secrets: secrets, - }, nil -} - // String stringifies a RunTaskRequest. func (r RunTaskRequest) String() string { output := []string{"copilot task run"} @@ -194,6 +155,45 @@ func (r RunTaskRequest) String() string { return strings.Join(output, " \\\n") } +func containerInformation(taskDef *awsecs.TaskDefinition, containerName string) (*containerInfo, error) { + image, err := taskDef.Image(containerName) + if err != nil { + return nil, err + } + + entrypoint, err := taskDef.EntryPoint(containerName) + if err != nil { + return nil, err + } + + command, err := taskDef.Command(containerName) + if err != nil { + return nil, err + } + + envVars := make(map[string]string) + for _, envVar := range taskDef.EnvironmentVariables() { + if envVar.Container == containerName { + envVars[envVar.Name] = envVar.Value + } + } + + secrets := make(map[string]string) + for _, secret := range taskDef.Secrets() { + if secret.Container == containerName { + secrets[secret.Name] = secret.ValueFrom + } + } + + return &containerInfo{ + image: image, + entryPoint: entrypoint, + command: command, + envVars: envVars, + secrets: secrets, + }, nil +} + // This function will format a map to a string as "key1=value1,key2=value2,key3=value3". func fmtStringMapToString(m map[string]string) string { var output []string From e1a31001c4930360285fda44715a96a81b8bd01b Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Fri, 30 Apr 2021 11:51:55 -0500 Subject: [PATCH 11/18] addd file header --- internal/pkg/ecs/run_task_request.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/pkg/ecs/run_task_request.go b/internal/pkg/ecs/run_task_request.go index bb0d0590169..2d48e32d20c 100644 --- a/internal/pkg/ecs/run_task_request.go +++ b/internal/pkg/ecs/run_task_request.go @@ -1,3 +1,7 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package ecs provides a client to retrieve Copilot ECS information. package ecs import ( From 3b387f3037464119e20efca94a73020c0c5e5e02 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Fri, 30 Apr 2021 15:00:43 -0500 Subject: [PATCH 12/18] rename variable --- internal/pkg/ecs/run_task_request_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/pkg/ecs/run_task_request_test.go b/internal/pkg/ecs/run_task_request_test.go index 6b9198d1334..4c979016fef 100644 --- a/internal/pkg/ecs/run_task_request_test.go +++ b/internal/pkg/ecs/run_task_request_test.go @@ -21,8 +21,8 @@ func Test_RunTaskRequestFromECSService(t *testing.T) { testCases := map[string]struct { setUpMock func(m *mocks.MockecsServiceDescriber) - wantedGenerateCommandOpts *RunTaskRequest - wantedError error + wantedRunTaskRequest *RunTaskRequest + wantedError error }{ "success": { setUpMock: func(m *mocks.MockecsServiceDescriber) { @@ -63,7 +63,7 @@ func Test_RunTaskRequestFromECSService(t *testing.T) { SecurityGroups: []string{"sg-1", "sg-2"}, }, nil) }, - wantedGenerateCommandOpts: &RunTaskRequest{ + wantedRunTaskRequest: &RunTaskRequest{ networkConfiguration: ecs.NetworkConfiguration{ AssignPublicIp: "1.2.3.4", Subnets: []string{"sbn-1", "sbn-2"}, @@ -150,7 +150,7 @@ func Test_RunTaskRequestFromECSService(t *testing.T) { require.EqualError(t, tc.wantedError, err.Error()) } else { require.NoError(t, err) - require.Equal(t, tc.wantedGenerateCommandOpts, got) + require.Equal(t, tc.wantedRunTaskRequest, got) } }) } @@ -165,10 +165,10 @@ func Test_RunTaskRequestFromService(t *testing.T) { testCases := map[string]struct { setUpMock func(m *mocks.MockserviceDescriber) - wantedGenerateCommandOpts *RunTaskRequest - wantedError error + wantedRunTaskRequest *RunTaskRequest + wantedError error }{ - "returns generateCommandOpts with service's main container": { + "returns RunTaskRequest with service's main container": { setUpMock: func(m *mocks.MockserviceDescriber) { m.EXPECT().TaskDefinition(testApp, testEnv, testSvc).Return(&ecs.TaskDefinition{ ExecutionRoleArn: aws.String("execution-role"), @@ -208,7 +208,7 @@ func Test_RunTaskRequestFromService(t *testing.T) { }, nil) m.EXPECT().ClusterARN(testApp, testEnv).Return("kamura-village", nil) }, - wantedGenerateCommandOpts: &RunTaskRequest{ + wantedRunTaskRequest: &RunTaskRequest{ networkConfiguration: ecs.NetworkConfiguration{ AssignPublicIp: "1.2.3.4", Subnets: []string{"sbn-1", "sbn-2"}, @@ -273,7 +273,7 @@ func Test_RunTaskRequestFromService(t *testing.T) { require.EqualError(t, tc.wantedError, err.Error()) } else { require.NoError(t, err) - require.Equal(t, tc.wantedGenerateCommandOpts, got) + require.Equal(t, tc.wantedRunTaskRequest, got) } }) } From 191ea9e0eb00d61534e9a80b21978e10ebf0f5a9 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Wed, 5 May 2021 10:03:35 -0500 Subject: [PATCH 13/18] bulk address feedback on error messaging --- internal/pkg/cli/task_run.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/pkg/cli/task_run.go b/internal/pkg/cli/task_run.go index 010cffa4a13..cf505daccad 100644 --- a/internal/pkg/cli/task_run.go +++ b/internal/pkg/cli/task_run.go @@ -524,16 +524,16 @@ func (o *runTaskOpts) runTaskCommand() (*ecs.RunTaskRequest, error) { clusterName, serviceName := parts[0], parts[1] cmd, err = ecs.RunTaskRequestFromECSService(awsecs.New(sess), clusterName, serviceName) if err != nil { - return nil, fmt.Errorf("generate task run command from ECS service: %w", err) + return nil, fmt.Errorf("generate task run command from ECS service %s: %w", clusterName+"/"+serviceName, err) } case 3: appName, envName, serviceName := parts[0], parts[1], parts[2] cmd, err = ecs.RunTaskRequestFromService(ecs.New(sess), appName, envName, serviceName) if err != nil { - return nil, fmt.Errorf("generate task run command from service: %w", err) + return nil, fmt.Errorf("generate task run command from service %s: %w", serviceName, err) } default: - return nil, errors.New("invalid input to --generate-cmd: must be of the form / or //") + return nil, errors.New("invalid input to --generate-cmd: must be of one the form / or //") } return cmd, nil @@ -543,15 +543,15 @@ func (o *runTaskOpts) runTaskCommandFromARN(sess *session.Session) (*ecs.RunTask svcARN := awsecs.ServiceArn(o.generateCommandTarget) clusterName, err := svcARN.ClusterName() if err != nil { - return nil, fmt.Errorf("extract cluster name from arn %s", svcARN) + return nil, fmt.Errorf("extract cluster name from arn %s: %w", svcARN, err) } serviceName, err := svcARN.ServiceName() if err != nil { - return nil, fmt.Errorf("extract service name from arn %s", svcARN) + return nil, fmt.Errorf("extract service name from arn %s: %w", svcARN, err) } cmd, err := ecs.RunTaskRequestFromECSService(awsecs.New(sess), clusterName, serviceName) if err != nil { - return nil, fmt.Errorf("generate task run command from ECS service: %w", err) + return nil, fmt.Errorf("generate task run command from ECS service %s: %w", clusterName+"/"+serviceName, err) } return cmd, nil } From b2ae755bda286fd10c007a7883380108bf6c9f20 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Wed, 5 May 2021 10:46:17 -0500 Subject: [PATCH 14/18] implement unit test for runTaskCommand() --- internal/pkg/cli/task_run.go | 15 +++-- internal/pkg/cli/task_run_test.go | 98 ++++++++++++++++++++++++++++ internal/pkg/ecs/run_task_request.go | 8 +-- 3 files changed, 113 insertions(+), 8 deletions(-) diff --git a/internal/pkg/cli/task_run.go b/internal/pkg/cli/task_run.go index cf505daccad..002a3b04aac 100644 --- a/internal/pkg/cli/task_run.go +++ b/internal/pkg/cli/task_run.go @@ -120,11 +120,15 @@ type runTaskOpts struct { sess *session.Session targetEnvironment *config.Environment - // Configurer methods. + // Configurer functions. configureRuntimeOpts func() error configureRepository func() error // NOTE: configureEventsWriter is only called when tailing logs (i.e. --follow is specified) configureEventsWriter func(tasks []*task.Task) + + // Functions to generate a task run command. + runTaskRequestFromECSService func(client ecs.ECSServiceDescriber, cluster, service string) (*ecs.RunTaskRequest, error) + runTaskRequestFromService func(client ecs.ServiceDescriber, app, env, svc string) (*ecs.RunTaskRequest, error) } func newTaskRunOpts(vars runTaskVars) (*runTaskOpts, error) { @@ -167,6 +171,9 @@ func newTaskRunOpts(vars runTaskVars) (*runTaskOpts, error) { opts.configureEventsWriter = func(tasks []*task.Task) { opts.eventsWriter = logging.NewTaskClient(opts.sess, opts.groupName, tasks) } + + opts.runTaskRequestFromECSService = ecs.RunTaskRequestFromECSService + opts.runTaskRequestFromService = ecs.RunTaskRequestFromService return &opts, nil } @@ -522,13 +529,13 @@ func (o *runTaskOpts) runTaskCommand() (*ecs.RunTaskRequest, error) { switch len(parts) { case 2: clusterName, serviceName := parts[0], parts[1] - cmd, err = ecs.RunTaskRequestFromECSService(awsecs.New(sess), clusterName, serviceName) + cmd, err = o.runTaskRequestFromECSService(awsecs.New(sess), clusterName, serviceName) if err != nil { return nil, fmt.Errorf("generate task run command from ECS service %s: %w", clusterName+"/"+serviceName, err) } case 3: appName, envName, serviceName := parts[0], parts[1], parts[2] - cmd, err = ecs.RunTaskRequestFromService(ecs.New(sess), appName, envName, serviceName) + cmd, err = o.runTaskRequestFromService(ecs.New(sess), appName, envName, serviceName) if err != nil { return nil, fmt.Errorf("generate task run command from service %s: %w", serviceName, err) } @@ -549,7 +556,7 @@ func (o *runTaskOpts) runTaskCommandFromARN(sess *session.Session) (*ecs.RunTask if err != nil { return nil, fmt.Errorf("extract service name from arn %s: %w", svcARN, err) } - cmd, err := ecs.RunTaskRequestFromECSService(awsecs.New(sess), clusterName, serviceName) + cmd, err := o.runTaskRequestFromECSService(awsecs.New(sess), clusterName, serviceName) if err != nil { return nil, fmt.Errorf("generate task run command from ECS service %s: %w", clusterName+"/"+serviceName, err) } diff --git a/internal/pkg/cli/task_run_test.go b/internal/pkg/cli/task_run_test.go index 08b4392da55..544b6cd2886 100644 --- a/internal/pkg/cli/task_run_test.go +++ b/internal/pkg/cli/task_run_test.go @@ -9,6 +9,8 @@ import ( "path/filepath" "testing" + "github.com/aws/copilot-cli/internal/pkg/ecs" + "github.com/aws/copilot-cli/internal/pkg/cli/mocks" "github.com/aws/copilot-cli/internal/pkg/exec" @@ -903,3 +905,99 @@ func TestTaskRunOpts_Execute(t *testing.T) { }) } } + +type mockRunTaskRequester struct { + mockRunTaskRequestFromECSService func(client ecs.ECSServiceDescriber, cluster string, service string) (*ecs.RunTaskRequest, error) + mockRunTaskRequestFromService func(client ecs.ServiceDescriber, app, env, svc string) (*ecs.RunTaskRequest, error) +} + +func TestTaskRunOpts_runTaskCommand(t *testing.T) { + wantedCommand := ecs.RunTaskRequest{} + + testCases := map[string]struct { + inGenerateCommandTarget string + + m mockRunTaskRequester + + wantedCommand *ecs.RunTaskRequest + wantedError error + }{ + "should generate a command given an service ARN": { + inGenerateCommandTarget: "arn:aws:ecs:us-east-1:123456789012:service/crowded-cluster/good-service", + m: mockRunTaskRequester{ + mockRunTaskRequestFromECSService: func(client ecs.ECSServiceDescriber, cluster string, service string) (*ecs.RunTaskRequest, error) { + return &wantedCommand, nil + }, + }, + wantedCommand: &wantedCommand, + }, + "fail to generate a command given an service ARN": { + inGenerateCommandTarget: "arn:aws:ecs:us-east-1:123456789012:service/crowded-cluster/good-service", + m: mockRunTaskRequester{ + mockRunTaskRequestFromECSService: func(client ecs.ECSServiceDescriber, cluster string, service string) (*ecs.RunTaskRequest, error) { + return nil, errors.New("some error") + }, + }, + wantedError: fmt.Errorf("generate task run command from ECS service crowded-cluster/good-service: some error"), + }, + "should generate a command given a cluster/service target": { + inGenerateCommandTarget: "crowded-cluster/good-service", + m: mockRunTaskRequester{ + mockRunTaskRequestFromECSService: func(client ecs.ECSServiceDescriber, cluster string, service string) (*ecs.RunTaskRequest, error) { + return &wantedCommand, nil + }, + }, + wantedCommand: &wantedCommand, + }, + "fail to generate a command given a cluster/service target": { + inGenerateCommandTarget: "crowded-cluster/good-service", + m: mockRunTaskRequester{ + mockRunTaskRequestFromECSService: func(client ecs.ECSServiceDescriber, cluster string, service string) (*ecs.RunTaskRequest, error) { + return nil, errors.New("some error") + }, + }, + wantedError: fmt.Errorf("generate task run command from ECS service crowded-cluster/good-service: some error"), + }, + "should generate a command given an app/env/svc target": { + inGenerateCommandTarget: "good-app/good-env/good-service", + m: mockRunTaskRequester{ + mockRunTaskRequestFromService: func(client ecs.ServiceDescriber, app, env, svc string) (*ecs.RunTaskRequest, error) { + return &wantedCommand, nil + }, + }, + wantedCommand: &wantedCommand, + }, + "fail to generate a command given an app/env/svc target": { + inGenerateCommandTarget: "good-app/good-env/good-service", + m: mockRunTaskRequester{ + mockRunTaskRequestFromService: func(client ecs.ServiceDescriber, app, env, svc string) (*ecs.RunTaskRequest, error) { + return nil, errors.New("some error") + }, + }, + wantedError: fmt.Errorf("generate task run command from service good-service: some error"), + }, + "invalid input": { + inGenerateCommandTarget: "invalid/illegal/not-good/input/is/bad", + wantedError: errors.New("invalid input to --generate-cmd: must be of one the form / or //"), + }, + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + opts := &runTaskOpts{ + runTaskVars: runTaskVars{ + generateCommandTarget: tc.inGenerateCommandTarget, + }, + runTaskRequestFromECSService: tc.m.mockRunTaskRequestFromECSService, + runTaskRequestFromService: tc.m.mockRunTaskRequestFromService, + } + + got, err := opts.runTaskCommand() + if tc.wantedError != nil { + require.EqualError(t, tc.wantedError, err.Error()) + } else { + require.NoError(t, err) + require.Equal(t, tc.wantedCommand, got) + } + }) + } +} diff --git a/internal/pkg/ecs/run_task_request.go b/internal/pkg/ecs/run_task_request.go index 2d48e32d20c..2feaecdf9bd 100644 --- a/internal/pkg/ecs/run_task_request.go +++ b/internal/pkg/ecs/run_task_request.go @@ -13,13 +13,13 @@ import ( awsecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" ) -type ecsServiceDescriber interface { +type ECSServiceDescriber interface { Service(clusterName, serviceName string) (*awsecs.Service, error) TaskDefinition(taskDefName string) (*awsecs.TaskDefinition, error) NetworkConfiguration(cluster, serviceName string) (*awsecs.NetworkConfiguration, error) } -type serviceDescriber interface { +type ServiceDescriber interface { TaskDefinition(app, env, svc string) (*awsecs.TaskDefinition, error) NetworkConfiguration(app, env, svc string) (*awsecs.NetworkConfiguration, error) ClusterARN(app, env string) (string, error) @@ -45,7 +45,7 @@ type containerInfo struct { } // RunTaskRequestFromECSService populates a RunTaskRequest with information from an ECS service. -func RunTaskRequestFromECSService(client ecsServiceDescriber, cluster, service string) (*RunTaskRequest, error) { +func RunTaskRequestFromECSService(client ECSServiceDescriber, cluster, service string) (*RunTaskRequest, error) { networkConfig, err := client.NetworkConfiguration(cluster, service) if err != nil { return nil, fmt.Errorf("retrieve network configuration for service %s in cluster %s: %w", service, cluster, err) @@ -82,7 +82,7 @@ func RunTaskRequestFromECSService(client ecsServiceDescriber, cluster, service s } // RunTaskRequestFromECSService populates a RunTaskRequest with information from a Copilot service. -func RunTaskRequestFromService(client serviceDescriber, app, env, svc string) (*RunTaskRequest, error) { +func RunTaskRequestFromService(client ServiceDescriber, app, env, svc string) (*RunTaskRequest, error) { networkConfig, err := client.NetworkConfiguration(app, env, svc) if err != nil { return nil, fmt.Errorf("retrieve network configuration for service %s: %w", svc, err) From 6d139c3d72838bd6236bdd7a846963247476e2f6 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Wed, 5 May 2021 11:57:07 -0500 Subject: [PATCH 15/18] address feedback to improve flag message, error message, and comment --- internal/pkg/cli/flag.go | 4 ++-- internal/pkg/cli/task_run.go | 2 +- internal/pkg/cli/task_run_test.go | 2 +- internal/pkg/ecs/run_task_request.go | 2 ++ 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/pkg/cli/flag.go b/internal/pkg/cli/flag.go index caa9448e952..1fd8106895a 100644 --- a/internal/pkg/cli/flag.go +++ b/internal/pkg/cli/flag.go @@ -227,8 +227,8 @@ Tasks with the same group name share the same set of resources. (default directory name)` taskImageTagFlagDescription = `Optional. The container image tag in addition to "latest".` generateCommandFlagDescription = `Optional. Generate a command with a pre-filled value for each flag. -To use a non-Copilot ECS service, specify --generate-cmd /. -To use a Copilot service, specify --generate-cmd //. +To use it for an ECS service, specify --generate-cmd /. +Alternatively, if the service is created with Copilot, specify --generate-cmd //. Cannot be specified with any other flags.` vpcIDFlagDescription = "Optional. Use an existing VPC ID." diff --git a/internal/pkg/cli/task_run.go b/internal/pkg/cli/task_run.go index 002a3b04aac..71b13a79c77 100644 --- a/internal/pkg/cli/task_run.go +++ b/internal/pkg/cli/task_run.go @@ -537,7 +537,7 @@ func (o *runTaskOpts) runTaskCommand() (*ecs.RunTaskRequest, error) { appName, envName, serviceName := parts[0], parts[1], parts[2] cmd, err = o.runTaskRequestFromService(ecs.New(sess), appName, envName, serviceName) if err != nil { - return nil, fmt.Errorf("generate task run command from service %s: %w", serviceName, err) + return nil, fmt.Errorf("generate task run command from service %s of application %s deployed in environment %s: %w", serviceName, appName, envName, err) } default: return nil, errors.New("invalid input to --generate-cmd: must be of one the form / or //") diff --git a/internal/pkg/cli/task_run_test.go b/internal/pkg/cli/task_run_test.go index 544b6cd2886..4c376d2afe3 100644 --- a/internal/pkg/cli/task_run_test.go +++ b/internal/pkg/cli/task_run_test.go @@ -974,7 +974,7 @@ func TestTaskRunOpts_runTaskCommand(t *testing.T) { return nil, errors.New("some error") }, }, - wantedError: fmt.Errorf("generate task run command from service good-service: some error"), + wantedError: fmt.Errorf("generate task run command from service good-service of application good-app deployed in environment good-env: some error"), }, "invalid input": { inGenerateCommandTarget: "invalid/illegal/not-good/input/is/bad", diff --git a/internal/pkg/ecs/run_task_request.go b/internal/pkg/ecs/run_task_request.go index 2feaecdf9bd..e044366c9e4 100644 --- a/internal/pkg/ecs/run_task_request.go +++ b/internal/pkg/ecs/run_task_request.go @@ -13,12 +13,14 @@ import ( awsecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" ) +// ECSServiceDescriber provides information on an ECS service. type ECSServiceDescriber interface { Service(clusterName, serviceName string) (*awsecs.Service, error) TaskDefinition(taskDefName string) (*awsecs.TaskDefinition, error) NetworkConfiguration(cluster, serviceName string) (*awsecs.NetworkConfiguration, error) } +// ServiceDescriber provides information on a Copilot service. type ServiceDescriber interface { TaskDefinition(app, env, svc string) (*awsecs.TaskDefinition, error) NetworkConfiguration(app, env, svc string) (*awsecs.NetworkConfiguration, error) From f6dde4ac7af8403d0a6b2d25b009fd25c2efd7e0 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Wed, 5 May 2021 12:13:46 -0500 Subject: [PATCH 16/18] address feedback to improve logging message --- internal/pkg/cli/task_run.go | 24 ++++++++++++++++++------ internal/pkg/ecs/errors.go | 14 ++++++++++++++ internal/pkg/ecs/run_task_request.go | 4 +++- 3 files changed, 35 insertions(+), 7 deletions(-) create mode 100644 internal/pkg/ecs/errors.go diff --git a/internal/pkg/cli/task_run.go b/internal/pkg/cli/task_run.go index 71b13a79c77..c5fe6c63771 100644 --- a/internal/pkg/cli/task_run.go +++ b/internal/pkg/cli/task_run.go @@ -522,16 +522,20 @@ func (o *runTaskOpts) runTaskCommand() (*ecs.RunTaskRequest, error) { } if arn.IsARN(o.generateCommandTarget) { - return o.runTaskCommandFromARN(sess) + clusterName, serviceName, err := o.parseARN() + if err != nil { + return nil, err + } + return o.runTaskCommandFromECSService(sess, clusterName, serviceName) } parts := strings.Split(o.generateCommandTarget, "/") switch len(parts) { case 2: clusterName, serviceName := parts[0], parts[1] - cmd, err = o.runTaskRequestFromECSService(awsecs.New(sess), clusterName, serviceName) + cmd, err = o.runTaskCommandFromECSService(sess, clusterName, serviceName) if err != nil { - return nil, fmt.Errorf("generate task run command from ECS service %s: %w", clusterName+"/"+serviceName, err) + return nil, err } case 3: appName, envName, serviceName := parts[0], parts[1], parts[2] @@ -546,20 +550,28 @@ func (o *runTaskOpts) runTaskCommand() (*ecs.RunTaskRequest, error) { return cmd, nil } -func (o *runTaskOpts) runTaskCommandFromARN(sess *session.Session) (*ecs.RunTaskRequest, error) { +func (o *runTaskOpts) parseARN() (string, string, error) { svcARN := awsecs.ServiceArn(o.generateCommandTarget) clusterName, err := svcARN.ClusterName() if err != nil { - return nil, fmt.Errorf("extract cluster name from arn %s: %w", svcARN, err) + return "", "", fmt.Errorf("extract cluster name from arn %s: %w", svcARN, err) } serviceName, err := svcARN.ServiceName() if err != nil { - return nil, fmt.Errorf("extract service name from arn %s: %w", svcARN, err) + return "", "", fmt.Errorf("extract service name from arn %s: %w", svcARN, err) } + return clusterName, serviceName, nil +} + +func (o *runTaskOpts) runTaskCommandFromECSService(sess *session.Session, clusterName, serviceName string) (*ecs.RunTaskRequest, error) { cmd, err := o.runTaskRequestFromECSService(awsecs.New(sess), clusterName, serviceName) if err != nil { return nil, fmt.Errorf("generate task run command from ECS service %s: %w", clusterName+"/"+serviceName, err) } + var errMultipleContainers *ecs.ErrMultipleContainersInTaskDef + if errors.As(err, &errMultipleContainers) { + log.Errorf("`copilot task run` does not support running more than one container") + } return cmd, nil } diff --git a/internal/pkg/ecs/errors.go b/internal/pkg/ecs/errors.go new file mode 100644 index 00000000000..b7a0d2baca9 --- /dev/null +++ b/internal/pkg/ecs/errors.go @@ -0,0 +1,14 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +package ecs + +import "fmt" + +type ErrMultipleContainersInTaskDef struct { + taskDefIdentifier string +} + +func (e *ErrMultipleContainersInTaskDef) Error() string { + return fmt.Sprintf("found more than one container in task definition: %s", e.taskDefIdentifier) +} diff --git a/internal/pkg/ecs/run_task_request.go b/internal/pkg/ecs/run_task_request.go index e044366c9e4..b00b0fe64bd 100644 --- a/internal/pkg/ecs/run_task_request.go +++ b/internal/pkg/ecs/run_task_request.go @@ -65,7 +65,9 @@ func RunTaskRequestFromECSService(client ECSServiceDescriber, cluster, service s } if len(taskDef.ContainerDefinitions) > 1 { - return nil, fmt.Errorf("found more than one container in task definition: %s", taskDefNameOrARN) + return nil, &ErrMultipleContainersInTaskDef{ + taskDefIdentifier: taskDefNameOrARN, + } } containerName := aws.StringValue(taskDef.ContainerDefinitions[0].Name) From d68b6f0e69350fc97dabfc7302ba684a66f3732e Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Wed, 5 May 2021 12:19:16 -0500 Subject: [PATCH 17/18] fix --- internal/pkg/cli/task_run.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/pkg/cli/task_run.go b/internal/pkg/cli/task_run.go index c5fe6c63771..65924870284 100644 --- a/internal/pkg/cli/task_run.go +++ b/internal/pkg/cli/task_run.go @@ -566,12 +566,12 @@ func (o *runTaskOpts) parseARN() (string, string, error) { func (o *runTaskOpts) runTaskCommandFromECSService(sess *session.Session, clusterName, serviceName string) (*ecs.RunTaskRequest, error) { cmd, err := o.runTaskRequestFromECSService(awsecs.New(sess), clusterName, serviceName) if err != nil { + var errMultipleContainers *ecs.ErrMultipleContainersInTaskDef + if errors.As(err, &errMultipleContainers) { + log.Errorln("`copilot task run` does not support running more than one container.") + } return nil, fmt.Errorf("generate task run command from ECS service %s: %w", clusterName+"/"+serviceName, err) } - var errMultipleContainers *ecs.ErrMultipleContainersInTaskDef - if errors.As(err, &errMultipleContainers) { - log.Errorf("`copilot task run` does not support running more than one container") - } return cmd, nil } From b7c54179bfda6161ce391d9f87cfb78ee47a99f7 Mon Sep 17 00:00:00 2001 From: Wanxian Yang Date: Wed, 5 May 2021 14:03:37 -0500 Subject: [PATCH 18/18] abstract away clistringer --- internal/pkg/cli/interfaces.go | 4 ++++ internal/pkg/cli/task_run.go | 8 ++++---- internal/pkg/ecs/run_task_request.go | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/internal/pkg/cli/interfaces.go b/internal/pkg/cli/interfaces.go index 4a06ae112c8..045e6da4f2c 100644 --- a/internal/pkg/cli/interfaces.go +++ b/internal/pkg/cli/interfaces.go @@ -565,3 +565,7 @@ type codestar interface { type publicIPGetter interface { PublicIP(ENI string) (string, error) } + +type cliStringer interface { + CLIString() string +} diff --git a/internal/pkg/cli/task_run.go b/internal/pkg/cli/task_run.go index 65924870284..10cd6606b02 100644 --- a/internal/pkg/cli/task_run.go +++ b/internal/pkg/cli/task_run.go @@ -510,12 +510,12 @@ func (o *runTaskOpts) generateCommand() error { if err != nil { return err } - log.Infoln(command.String()) + log.Infoln(command.CLIString()) return nil } -func (o *runTaskOpts) runTaskCommand() (*ecs.RunTaskRequest, error) { - var cmd *ecs.RunTaskRequest +func (o *runTaskOpts) runTaskCommand() (cliStringer, error) { + var cmd cliStringer sess, err := sessions.NewProvider().Default() if err != nil { return nil, fmt.Errorf("get default session: %s", err) @@ -563,7 +563,7 @@ func (o *runTaskOpts) parseARN() (string, string, error) { return clusterName, serviceName, nil } -func (o *runTaskOpts) runTaskCommandFromECSService(sess *session.Session, clusterName, serviceName string) (*ecs.RunTaskRequest, error) { +func (o *runTaskOpts) runTaskCommandFromECSService(sess *session.Session, clusterName, serviceName string) (cliStringer, error) { cmd, err := o.runTaskRequestFromECSService(awsecs.New(sess), clusterName, serviceName) if err != nil { var errMultipleContainers *ecs.ErrMultipleContainersInTaskDef diff --git a/internal/pkg/ecs/run_task_request.go b/internal/pkg/ecs/run_task_request.go index b00b0fe64bd..530315f3d31 100644 --- a/internal/pkg/ecs/run_task_request.go +++ b/internal/pkg/ecs/run_task_request.go @@ -118,7 +118,7 @@ func RunTaskRequestFromService(client ServiceDescriber, app, env, svc string) (* } // String stringifies a RunTaskRequest. -func (r RunTaskRequest) String() string { +func (r RunTaskRequest) CLIString() string { output := []string{"copilot task run"} if r.executionRole != "" { output = append(output, fmt.Sprintf("--execution-role %s", r.executionRole))