From 4a57fcc0acb062831bf69b8b681f6b74dd2bcdeb Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Fri, 20 Oct 2023 15:48:51 -0700 Subject: [PATCH 01/17] get ecs services --- internal/pkg/aws/ecs/ecs.go | 33 ++++++++++++++++++++ internal/pkg/cli/interfaces.go | 1 + internal/pkg/cli/run_local.go | 55 ++++++++++++++++++++++++++++++++++ internal/pkg/ecs/ecs.go | 31 +++++++++++++++++++ 4 files changed, 120 insertions(+) diff --git a/internal/pkg/aws/ecs/ecs.go b/internal/pkg/aws/ecs/ecs.go index e7f32e53dc5..4b7ce378832 100644 --- a/internal/pkg/aws/ecs/ecs.go +++ b/internal/pkg/aws/ecs/ecs.go @@ -39,6 +39,7 @@ type api interface { StopTask(input *ecs.StopTaskInput) (*ecs.StopTaskOutput, error) UpdateService(input *ecs.UpdateServiceInput) (*ecs.UpdateServiceOutput, error) WaitUntilTasksRunning(input *ecs.DescribeTasksInput) error + ListServicesByNamespace(*ecs.ListServicesByNamespaceInput) (*ecs.ListServicesByNamespaceOutput, error) } type ssmSessionStarter interface { @@ -100,6 +101,7 @@ func (e *ECS) TaskDefinition(taskDefName string) (*TaskDefinition, error) { // Service calls ECS API and returns the specified service running in the cluster. func (e *ECS) Service(clusterName, serviceName string) (*Service, error) { + // TODO use Services() below resp, err := e.client.DescribeServices(&ecs.DescribeServicesInput{ Cluster: aws.String(clusterName), Services: aws.StringSlice([]string{serviceName}), @@ -116,6 +118,37 @@ func (e *ECS) Service(clusterName, serviceName string) (*Service, error) { return nil, fmt.Errorf("cannot find service %s", serviceName) } +func (e *ECS) Services(cluster string, services ...string) ([]*Service, error) { + // TODO split into groups of 10 + resp, err := e.client.DescribeServices(&ecs.DescribeServicesInput{ + Cluster: aws.String(cluster), + Services: aws.StringSlice(services), + }) + if err != nil { + return nil, fmt.Errorf("describe services: %w", err) + } + + var out []*Service + + for i := range resp.Services { + svc := Service(*resp.Services[i]) + out = append(out, &svc) + } + + // TODO make sure len(services) == len(out) + return out, nil +} + +func (e *ECS) ListServicesByNamespace(namespace string) ([]string, error) { + arns, err := e.client.ListServicesByNamespace(&ecs.ListServicesByNamespaceInput{ + Namespace: aws.String(namespace), + }) + if err != nil { + return nil, err + } + return aws.StringValueSlice(arns.ServiceArns), nil +} + // UpdateServiceOpts sets the optional parameter for UpdateService. type UpdateServiceOpts func(*ecs.UpdateServiceInput) diff --git a/internal/pkg/cli/interfaces.go b/internal/pkg/cli/interfaces.go index 758ca93a9ea..0d698dca9ad 100644 --- a/internal/pkg/cli/interfaces.go +++ b/internal/pkg/cli/interfaces.go @@ -185,6 +185,7 @@ type repositoryService interface { type ecsLocalClient interface { TaskDefinition(app, env, svc string) (*awsecs.TaskDefinition, error) + ServiceConnectServices(app, env, svc string) ([]*awsecs.Service, error) } type logEventsWriter interface { diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index 71e3c4ebecf..7a2b01cf30c 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -262,6 +262,18 @@ func (o *runLocalOpts) Execute() error { return fmt.Errorf("get task: %w", err) } + // TODO add proxy flag + hd := &hostDiscoverer{ + app: o.appName, + env: o.envName, + wkld: o.wkldName, + ecs: ecs.New(o.sess), // TODO update env manager role to support actions + } + _, err = hd.Hosts(ctx) + if err != nil { + return fmt.Errorf("find hosts to connect to: %w", err) + } + mft, _, err := workloadManifest(&workloadManifestInput{ name: o.wkldName, appName: o.appName, @@ -559,6 +571,49 @@ func (o *runLocalOpts) getSecret(ctx context.Context, valueFrom string) (string, return getter.GetSecretValue(ctx, valueFrom) } +type host struct { + host string + port string +} + +type hostDiscoverer struct { + rg resourceGetter + ecs ecsLocalClient + app string + env string + wkld string + // rg := resourcegroups.New(o.sess) // TODO move to newRunLocalOpts() +} + +func (h *hostDiscoverer) Hosts(ctx context.Context) ([]host, error) { + // rds instances + // elasticcache instances + // ecs services via + // ecs services via SC + svcs, err := h.ecs.ServiceConnectServices(h.app, h.env, h.wkld) + if err != nil { + return nil, fmt.Errorf("get service: %w", err) + } + + var hosts []host + for _, svc := range svcs { + // TODO is there only one deployment? + for _, dep := range svc.Deployments { + for _, sc := range dep.ServiceConnectConfiguration.Services { + for _, alias := range sc.ClientAliases { + hosts = append(hosts, host{ + host: aws.StringValue(alias.DnsName), + port: strconv.Itoa(int(aws.Int64Value(alias.Port))), + }) + } + } + } + } + + fmt.Printf("hosts: %+v\n", hosts) + return nil, nil +} + // BuildRunLocalCmd builds the command for running a workload locally func BuildRunLocalCmd() *cobra.Command { vars := runLocalVars{} diff --git a/internal/pkg/ecs/ecs.go b/internal/pkg/ecs/ecs.go index 9035eb8a7af..0497fef7b93 100644 --- a/internal/pkg/ecs/ecs.go +++ b/internal/pkg/ecs/ecs.go @@ -7,6 +7,7 @@ package ecs import ( "encoding/json" "fmt" + "slices" "sort" "strings" "time" @@ -47,6 +48,8 @@ type ecsClient interface { DescribeTasks(cluster string, taskARNs []string) ([]*ecs.Task, error) ActiveClusters(arns ...string) ([]string, error) ActiveServices(clusterName string, serviceARNs ...string) ([]string, error) + ListServicesByNamespace(namespace string) ([]string, error) + Services(cluster string, services ...string) ([]*ecs.Service, error) } type stepFunctionsClient interface { @@ -133,6 +136,34 @@ func (c Client) Service(app, env, svc string) (*ecs.Service, error) { return service, nil } +// ServiceConnectServices returns a list of services that are in the same +// service connect namespace as the given service. +func (c Client) ServiceConnectServices(app, env, svc string) ([]*ecs.Service, error) { + s, err := c.Service(app, env, svc) + if err != nil { + return nil, fmt.Errorf("get service: %w", err) + } + if len(s.Deployments) == 0 { + return nil, nil + } + + arns, err := c.ecsClient.ListServicesByNamespace(aws.StringValue(s.Deployments[0].ServiceConnectConfiguration.Namespace)) + if err != nil { + return nil, fmt.Errorf("get services in same namespace: %w", err) + } + + // remove this service's arn + arns = slices.DeleteFunc(arns, func(arn string) bool { + return arn == aws.StringValue(s.ServiceArn) + }) + + svcs, err := c.ecsClient.Services(aws.StringValue(s.ClusterArn), arns...) + if err != nil { + return nil, fmt.Errorf("get services: %w", err) + } + return svcs, nil +} + // LastUpdatedAt returns the last updated time of the ECS service. func (c Client) LastUpdatedAt(app, env, svc string) (time.Time, error) { detail, err := c.Service(app, env, svc) From 61cc150cf3109a77980a9860157211430a136bd4 Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Fri, 20 Oct 2023 16:30:25 -0700 Subject: [PATCH 02/17] =?UTF-8?q?=F0=9F=A4=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index e5fd4cd246f..e13acaca6f5 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/aws/copilot-cli -go 1.20 +go 1.21 require ( github.com/AlecAivazis/survey/v2 v2.3.2 From ccd5ebfa777ed6148d6f77f2e61785d48ca77a7f Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Fri, 20 Oct 2023 16:30:51 -0700 Subject: [PATCH 03/17] get all services if there's a large list --- internal/pkg/aws/ecs/ecs.go | 51 +++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/internal/pkg/aws/ecs/ecs.go b/internal/pkg/aws/ecs/ecs.go index 4b7ce378832..f0e435ef267 100644 --- a/internal/pkg/aws/ecs/ecs.go +++ b/internal/pkg/aws/ecs/ecs.go @@ -101,42 +101,38 @@ func (e *ECS) TaskDefinition(taskDefName string) (*TaskDefinition, error) { // Service calls ECS API and returns the specified service running in the cluster. func (e *ECS) Service(clusterName, serviceName string) (*Service, error) { - // TODO use Services() below - resp, err := e.client.DescribeServices(&ecs.DescribeServicesInput{ - Cluster: aws.String(clusterName), - Services: aws.StringSlice([]string{serviceName}), - }) + svcs, err := e.Services(clusterName, serviceName) if err != nil { - return nil, fmt.Errorf("describe service %s: %w", serviceName, err) - } - for _, service := range resp.Services { - if aws.StringValue(service.ServiceName) == serviceName { - svc := Service(*service) - return &svc, nil - } + return nil, err } - return nil, fmt.Errorf("cannot find service %s", serviceName) + + return svcs[0], nil } func (e *ECS) Services(cluster string, services ...string) ([]*Service, error) { - // TODO split into groups of 10 - resp, err := e.client.DescribeServices(&ecs.DescribeServicesInput{ - Cluster: aws.String(cluster), - Services: aws.StringSlice(services), - }) - if err != nil { - return nil, fmt.Errorf("describe services: %w", err) - } + var svcs []*Service - var out []*Service + for i := 0; i < len(services); i += 10 { + split := services[i:min(10+i, len(services))] - for i := range resp.Services { - svc := Service(*resp.Services[i]) - out = append(out, &svc) + resp, err := e.client.DescribeServices(&ecs.DescribeServicesInput{ + Cluster: aws.String(cluster), + Services: aws.StringSlice(split), + }) + switch { + case err != nil: + return nil, fmt.Errorf("describe services: %w", err) + case len(resp.Failures) > 0: + return nil, fmt.Errorf("describe services: %s", resp.Failures[0].GoString()) + } + + for j := range resp.Services { + svc := Service(*resp.Services[j]) + svcs = append(svcs, &svc) + } } - // TODO make sure len(services) == len(out) - return out, nil + return svcs, nil } func (e *ECS) ListServicesByNamespace(namespace string) ([]string, error) { @@ -146,6 +142,7 @@ func (e *ECS) ListServicesByNamespace(namespace string) ([]string, error) { if err != nil { return nil, err } + // TODO ensure fn doesn't return duplicates? return aws.StringValueSlice(arns.ServiceArns), nil } From d49a8babb5b8ed74b42c418bf747a3533f42f00a Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Fri, 20 Oct 2023 16:50:36 -0700 Subject: [PATCH 04/17] use paginator instead --- internal/pkg/aws/ecs/ecs.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/pkg/aws/ecs/ecs.go b/internal/pkg/aws/ecs/ecs.go index f0e435ef267..f631c21b29a 100644 --- a/internal/pkg/aws/ecs/ecs.go +++ b/internal/pkg/aws/ecs/ecs.go @@ -39,7 +39,7 @@ type api interface { StopTask(input *ecs.StopTaskInput) (*ecs.StopTaskOutput, error) UpdateService(input *ecs.UpdateServiceInput) (*ecs.UpdateServiceOutput, error) WaitUntilTasksRunning(input *ecs.DescribeTasksInput) error - ListServicesByNamespace(*ecs.ListServicesByNamespaceInput) (*ecs.ListServicesByNamespaceOutput, error) + ListServicesByNamespacePages(input *ecs.ListServicesByNamespaceInput, fn func(*ecs.ListServicesByNamespaceOutput, bool) bool) error } type ssmSessionStarter interface { @@ -136,14 +136,18 @@ func (e *ECS) Services(cluster string, services ...string) ([]*Service, error) { } func (e *ECS) ListServicesByNamespace(namespace string) ([]string, error) { - arns, err := e.client.ListServicesByNamespace(&ecs.ListServicesByNamespaceInput{ + // TODO ensure ListServicesByNamesspace doesn't return duplicates? + var arns []string + err := e.client.ListServicesByNamespacePages(&ecs.ListServicesByNamespaceInput{ Namespace: aws.String(namespace), + }, func(resp *ecs.ListServicesByNamespaceOutput, b bool) bool { + arns = append(arns, aws.StringValueSlice(resp.ServiceArns)...) + return true }) if err != nil { return nil, err } - // TODO ensure fn doesn't return duplicates? - return aws.StringValueSlice(arns.ServiceArns), nil + return arns, nil } // UpdateServiceOpts sets the optional parameter for UpdateService. From 0a911d286db9a337ab903212ffb7aea5dc0a0687 Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Mon, 23 Oct 2023 11:49:31 -0700 Subject: [PATCH 05/17] update some logic --- internal/pkg/cli/run_local.go | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index 7a2b01cf30c..d3e765bf669 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -16,6 +16,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/arn" "github.com/aws/aws-sdk-go/aws/session" + sdkecs "github.com/aws/aws-sdk-go/service/ecs" sdksecretsmanager "github.com/aws/aws-sdk-go/service/secretsmanager" sdkssm "github.com/aws/aws-sdk-go/service/ssm" "github.com/aws/copilot-cli/cmd/copilot/template" @@ -45,6 +46,7 @@ import ( "github.com/aws/copilot-cli/internal/pkg/workspace" "github.com/spf13/afero" "github.com/spf13/cobra" + "golang.org/x/exp/slices" "golang.org/x/sync/errgroup" ) @@ -577,19 +579,13 @@ type host struct { } type hostDiscoverer struct { - rg resourceGetter ecs ecsLocalClient app string env string wkld string - // rg := resourcegroups.New(o.sess) // TODO move to newRunLocalOpts() } func (h *hostDiscoverer) Hosts(ctx context.Context) ([]host, error) { - // rds instances - // elasticcache instances - // ecs services via - // ecs services via SC svcs, err := h.ecs.ServiceConnectServices(h.app, h.env, h.wkld) if err != nil { return nil, fmt.Errorf("get service: %w", err) @@ -597,15 +593,20 @@ func (h *hostDiscoverer) Hosts(ctx context.Context) ([]host, error) { var hosts []host for _, svc := range svcs { - // TODO is there only one deployment? - for _, dep := range svc.Deployments { - for _, sc := range dep.ServiceConnectConfiguration.Services { - for _, alias := range sc.ClientAliases { - hosts = append(hosts, host{ - host: aws.StringValue(alias.DnsName), - port: strconv.Itoa(int(aws.Int64Value(alias.Port))), - }) - } + // find the primary deployment with service connect enabled + idx := slices.IndexFunc(svc.Deployments, func(dep *sdkecs.Deployment) bool { + return aws.StringValue(dep.Status) == "PRIMARY" && aws.BoolValue(dep.ServiceConnectConfiguration.Enabled) + }) + if idx == -1 { + continue + } + + for _, sc := range svc.Deployments[idx].ServiceConnectConfiguration.Services { + for _, alias := range sc.ClientAliases { + hosts = append(hosts, host{ + host: aws.StringValue(alias.DnsName), + port: strconv.Itoa(int(aws.Int64Value(alias.Port))), + }) } } } From 373fae9ebf10edb18a81b353457ae86c5a23d8e8 Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Mon, 23 Oct 2023 12:59:03 -0700 Subject: [PATCH 06/17] add proxy flag. set things up for testing. --- internal/pkg/cli/flag.go | 2 + internal/pkg/cli/run_local.go | 93 ++++++++++++++++++----------------- 2 files changed, 51 insertions(+), 44 deletions(-) diff --git a/internal/pkg/cli/flag.go b/internal/pkg/cli/flag.go index 7e65fbc93de..060c04dd0c8 100644 --- a/internal/pkg/cli/flag.go +++ b/internal/pkg/cli/flag.go @@ -69,6 +69,7 @@ const ( // Run local flags portOverrideFlag = "port-override" envVarOverrideFlag = "env-var-override" + proxyFlag = "proxy" // Flags for CI/CD. githubURLFlag = "github-url" @@ -320,6 +321,7 @@ Defaults to all logs. Only one of end-time / follow may be used.` Format: [container]:KEY=VALUE. Omit container name to apply to all containers.` portOverridesFlagDescription = `Optional. Override ports exposed by service. Format: :. Example: --port-override 5000:80 binds localhost:5000 to the service's port 80.` + proxyFlagDescription = `Optional. Proxy outbound requests to your environment's VPC.` svcManifestFlagDescription = `Optional. Name of the environment in which the service was deployed; output the manifest file used for that deployment.` diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index d3e765bf669..4809eb8a38d 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -60,6 +60,10 @@ type containerOrchestrator interface { Stop() } +type hostFinder interface { + Hosts(context.Context) ([]host, error) +} + type runLocalVars struct { wkldName string wkldType string @@ -67,27 +71,29 @@ type runLocalVars struct { envName string envOverrides map[string]string portOverrides portOverrides + proxy bool } type runLocalOpts struct { runLocalVars - sel deploySelector - ecsLocalClient ecsLocalClient - ssm secretGetter - secretsManager secretGetter - sessProvider sessionProvider - sess *session.Session - envSess *session.Session - targetEnv *config.Environment - targetApp *config.Application - store store - ws wsWlDirReader - cmd execRunner - dockerEngine dockerEngineRunner - repository repositoryService - prog progress - newOrchestrator func() containerOrchestrator + sel deploySelector + ecsLocalClient ecsLocalClient + ssm secretGetter + secretsManager secretGetter + sessProvider sessionProvider + sess *session.Session + envSess *session.Session + targetEnv *config.Environment + targetApp *config.Application + store store + ws wsWlDirReader + cmd execRunner + dockerEngine dockerEngineRunner + repository repositoryService + prog progress + orchestrator containerOrchestrator + hostFinder hostFinder buildContainerImages func(mft manifest.DynamicWorkload) (map[string]string, error) configureClients func(o *runLocalOpts) error @@ -153,6 +159,23 @@ func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) { } repoName := clideploy.RepoName(o.appName, o.wkldName) o.repository = repository.NewWithURI(ecr.New(defaultSessEnvRegion), repoName, resources.RepositoryURLs[o.wkldName]) + + idPrefix := fmt.Sprintf("%s-%s-%s-", opts.appName, opts.envName, opts.wkldName) + colorGen := termcolor.ColorGenerator() + o.orchestrator = orchestrator.New(opts.dockerEngine, idPrefix, func(name string, ctr orchestrator.ContainerDefinition) dockerengine.RunLogOptions { + return dockerengine.RunLogOptions{ + Color: colorGen(), + Output: os.Stderr, + LinePrefix: fmt.Sprintf("[%s] ", name), + } + }) + + o.hostFinder = &hostDiscoverer{ + app: o.appName, + env: o.envName, + wkld: o.wkldName, + ecs: ecs.New(o.sess), // TODO update env manager role to support actions + } return nil } opts.buildContainerImages = func(mft manifest.DynamicWorkload) (map[string]string, error) { @@ -185,18 +208,6 @@ func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) { } return containerURIs, nil } - opts.newOrchestrator = func() containerOrchestrator { - idPrefix := fmt.Sprintf("%s-%s-%s-", opts.appName, opts.envName, opts.wkldName) - colorGen := termcolor.ColorGenerator() - - return orchestrator.New(opts.dockerEngine, idPrefix, func(name string, ctr orchestrator.ContainerDefinition) dockerengine.RunLogOptions { - return dockerengine.RunLogOptions{ - Color: colorGen(), - Output: os.Stderr, - LinePrefix: fmt.Sprintf("[%s] ", name), - } - }) - } return opts, nil } @@ -264,16 +275,11 @@ func (o *runLocalOpts) Execute() error { return fmt.Errorf("get task: %w", err) } - // TODO add proxy flag - hd := &hostDiscoverer{ - app: o.appName, - env: o.envName, - wkld: o.wkldName, - ecs: ecs.New(o.sess), // TODO update env manager role to support actions - } - _, err = hd.Hosts(ctx) - if err != nil { - return fmt.Errorf("find hosts to connect to: %w", err) + if o.proxy { + _, err = o.hostFinder.Hosts(ctx) + if err != nil { + return fmt.Errorf("find hosts to connect to: %w", err) + } } mft, _, err := workloadManifest(&workloadManifestInput{ @@ -305,13 +311,11 @@ func (o *runLocalOpts) Execute() error { task.Containers[name] = ctr } - orch := o.newOrchestrator() - sigCh := make(chan os.Signal, 1) signal.Notify(sigCh, syscall.SIGINT, syscall.SIGTERM) - errCh := orch.Start() - orch.RunTask(task) + errCh := o.orchestrator.Start() + o.orchestrator.RunTask(task) for { select { @@ -323,10 +327,10 @@ func (o *runLocalOpts) Execute() error { } fmt.Printf("error: %s\n", err) - orch.Stop() + o.orchestrator.Stop() case <-sigCh: signal.Stop(sigCh) - orch.Stop() + o.orchestrator.Stop() } } } @@ -640,5 +644,6 @@ func BuildRunLocalCmd() *cobra.Command { cmd.Flags().StringVarP(&vars.appName, appFlag, appFlagShort, tryReadingAppName(), appFlagDescription) cmd.Flags().Var(&vars.portOverrides, portOverrideFlag, portOverridesFlagDescription) cmd.Flags().StringToStringVar(&vars.envOverrides, envVarOverrideFlag, nil, envVarOverrideFlagDescription) + cmd.Flags().BoolVar(&vars.proxy, proxyFlag, false, proxyFlagDescription) return cmd } From 6cf7f8bb782911d5a2f517f83102b3554dbed185 Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Mon, 23 Oct 2023 15:00:02 -0700 Subject: [PATCH 07/17] gen mocks, add run local tests --- internal/pkg/aws/ecs/mocks/mock_ecs.go | 14 +++ internal/pkg/cli/mocks/mock_interfaces.go | 15 +++ internal/pkg/cli/run_local.go | 12 +- internal/pkg/cli/run_local_test.go | 145 +++++++++++++++++++++- internal/pkg/ecs/mocks/mock_ecs.go | 35 ++++++ 5 files changed, 213 insertions(+), 8 deletions(-) diff --git a/internal/pkg/aws/ecs/mocks/mock_ecs.go b/internal/pkg/aws/ecs/mocks/mock_ecs.go index bc89156b860..cd63d4c4f8b 100644 --- a/internal/pkg/aws/ecs/mocks/mock_ecs.go +++ b/internal/pkg/aws/ecs/mocks/mock_ecs.go @@ -109,6 +109,20 @@ func (mr *MockapiMockRecorder) ExecuteCommand(input interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ExecuteCommand", reflect.TypeOf((*Mockapi)(nil).ExecuteCommand), input) } +// ListServicesByNamespacePages mocks base method. +func (m *Mockapi) ListServicesByNamespacePages(input *ecs.ListServicesByNamespaceInput, fn func(*ecs.ListServicesByNamespaceOutput, bool) bool) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListServicesByNamespacePages", input, fn) + ret0, _ := ret[0].(error) + return ret0 +} + +// ListServicesByNamespacePages indicates an expected call of ListServicesByNamespacePages. +func (mr *MockapiMockRecorder) ListServicesByNamespacePages(input, fn interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListServicesByNamespacePages", reflect.TypeOf((*Mockapi)(nil).ListServicesByNamespacePages), input, fn) +} + // ListTasks mocks base method. func (m *Mockapi) ListTasks(input *ecs.ListTasksInput) (*ecs.ListTasksOutput, error) { m.ctrl.T.Helper() diff --git a/internal/pkg/cli/mocks/mock_interfaces.go b/internal/pkg/cli/mocks/mock_interfaces.go index f780a80cfab..f1ecfabcfb9 100644 --- a/internal/pkg/cli/mocks/mock_interfaces.go +++ b/internal/pkg/cli/mocks/mock_interfaces.go @@ -1738,6 +1738,21 @@ func (m *MockecsLocalClient) EXPECT() *MockecsLocalClientMockRecorder { return m.recorder } +// ServiceConnectServices mocks base method. +func (m *MockecsLocalClient) ServiceConnectServices(app, env, svc string) ([]*ecs.Service, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ServiceConnectServices", app, env, svc) + ret0, _ := ret[0].([]*ecs.Service) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ServiceConnectServices indicates an expected call of ServiceConnectServices. +func (mr *MockecsLocalClientMockRecorder) ServiceConnectServices(app, env, svc interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ServiceConnectServices", reflect.TypeOf((*MockecsLocalClient)(nil).ServiceConnectServices), app, env, svc) +} + // TaskDefinition mocks base method. func (m *MockecsLocalClient) TaskDefinition(app, env, svc string) (*ecs.TaskDefinition, error) { m.ctrl.T.Helper() diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index 4809eb8a38d..01088d73630 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "os/signal" + "slices" "strconv" "strings" "sync" @@ -46,7 +47,6 @@ import ( "github.com/aws/copilot-cli/internal/pkg/workspace" "github.com/spf13/afero" "github.com/spf13/cobra" - "golang.org/x/exp/slices" "golang.org/x/sync/errgroup" ) @@ -276,10 +276,13 @@ func (o *runLocalOpts) Execute() error { } if o.proxy { - _, err = o.hostFinder.Hosts(ctx) + hosts, err := o.hostFinder.Hosts(ctx) if err != nil { return fmt.Errorf("find hosts to connect to: %w", err) } + + // TODO(dannyrandall): inject into orchestrator and use in pause container + fmt.Printf("hosts: %+v\n", hosts) } mft, _, err := workloadManifest(&workloadManifestInput{ @@ -592,7 +595,7 @@ type hostDiscoverer struct { func (h *hostDiscoverer) Hosts(ctx context.Context) ([]host, error) { svcs, err := h.ecs.ServiceConnectServices(h.app, h.env, h.wkld) if err != nil { - return nil, fmt.Errorf("get service: %w", err) + return nil, fmt.Errorf("get service connect services: %w", err) } var hosts []host @@ -615,8 +618,7 @@ func (h *hostDiscoverer) Hosts(ctx context.Context) ([]host, error) { } } - fmt.Printf("hosts: %+v\n", hosts) - return nil, nil + return hosts, nil } // BuildRunLocalCmd builds the command for running a workload locally diff --git a/internal/pkg/cli/run_local_test.go b/internal/pkg/cli/run_local_test.go index e2bdb20b0a8..1cbd6be909e 100644 --- a/internal/pkg/cli/run_local_test.go +++ b/internal/pkg/cli/run_local_test.go @@ -15,6 +15,7 @@ import ( "github.com/aws/aws-sdk-go/aws/session" sdkecs "github.com/aws/aws-sdk-go/service/ecs" "github.com/aws/copilot-cli/internal/pkg/aws/ecs" + awsecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" "github.com/aws/copilot-cli/internal/pkg/cli/mocks" "github.com/aws/copilot-cli/internal/pkg/config" "github.com/aws/copilot-cli/internal/pkg/docker/orchestrator" @@ -206,6 +207,7 @@ type runLocalExecuteMocks struct { secretsManager *mocks.MocksecretGetter prog *mocks.Mockprogress orchestrator *orchestratortest.Double + hostFinder *hostFinderDouble } type mockProvider struct { @@ -220,6 +222,17 @@ func (m *mockProvider) IsExpired() bool { return false } +type hostFinderDouble struct { + HostsFn func(context.Context) ([]host, error) +} + +func (d *hostFinderDouble) Hosts(ctx context.Context) ([]host, error) { + if d.HostsFn == nil { + return nil, nil + } + return d.HostsFn(ctx) +} + func TestRunLocalOpts_Execute(t *testing.T) { const ( testAppName = "testApp" @@ -342,6 +355,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { inputEnvOverrides map[string]string inputPortOverrides []string buildImagesError error + inProxy bool setupMocks func(t *testing.T, m *runLocalExecuteMocks) wantedWkldName string @@ -370,6 +384,20 @@ func TestRunLocalOpts_Execute(t *testing.T) { }, wantedError: errors.New(`get task: get env vars: parse env overrides: "bad:OVERRIDE" targets invalid container`), }, + "error getting hosts to proxy to": { + inputAppName: testAppName, + inputWkldName: testWkldName, + inputEnvName: testEnvName, + inProxy: true, + setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { + m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil) + m.hostFinder.HostsFn = func(ctx context.Context) ([]host, error) { + return nil, fmt.Errorf("some error") + } + }, + wantedError: errors.New(`find hosts to connect to: some error`), + }, "error reading workload manifest": { inputAppName: testAppName, inputWkldName: testWkldName, @@ -483,6 +511,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { repository: mocks.NewMockrepositoryService(ctrl), prog: mocks.NewMockprogress(ctrl), orchestrator: &orchestratortest.Double{}, + hostFinder: &hostFinderDouble{}, } tc.setupMocks(t, m) opts := runLocalOpts{ @@ -501,6 +530,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { container: "9999", }, }, + proxy: tc.inProxy, }, newInterpolator: func(app, env string) interpolator { return m.interpolator @@ -530,9 +560,8 @@ func TestRunLocalOpts_Execute(t *testing.T) { targetEnv: &mockEnv, targetApp: &mockApp, prog: m.prog, - newOrchestrator: func() containerOrchestrator { - return m.orchestrator - }, + orchestrator: m.orchestrator, + hostFinder: m.hostFinder, } // WHEN err := opts.Execute() @@ -944,3 +973,113 @@ func TestRunLocalOpts_getEnvVars(t *testing.T) { }) } } + +func TestRunLocal_HostDiscovery(t *testing.T) { + type testMocks struct { + ecs *mocks.MockecsLocalClient + } + + tests := map[string]struct { + setupMocks func(t *testing.T, m *testMocks) + + wantHosts []host + wantError string + }{ + "error getting services": { + setupMocks: func(t *testing.T, m *testMocks) { + m.ecs.EXPECT().ServiceConnectServices(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, errors.New("some error")) + }, + wantError: "get service connect services: some error", + }, + "ignores non-primary deployments": { + setupMocks: func(t *testing.T, m *testMocks) { + m.ecs.EXPECT().ServiceConnectServices(gomock.Any(), gomock.Any(), gomock.Any()).Return([]*awsecs.Service{ + { + Deployments: []*sdkecs.Deployment{ + { + Status: aws.String("ACTIVE"), + ServiceConnectConfiguration: &sdkecs.ServiceConnectConfiguration{ + Enabled: aws.Bool(true), + Services: []*sdkecs.ServiceConnectService{ + { + ClientAliases: []*sdkecs.ServiceConnectClientAlias{ + { + DnsName: aws.String("old"), + Port: aws.Int64(80), + }, + }, + }, + }, + }, + }, + { + Status: aws.String("PRIMARY"), + ServiceConnectConfiguration: &sdkecs.ServiceConnectConfiguration{ + Enabled: aws.Bool(true), + Services: []*sdkecs.ServiceConnectService{ + { + ClientAliases: []*sdkecs.ServiceConnectClientAlias{ + { + DnsName: aws.String("primary"), + Port: aws.Int64(80), + }, + }, + }, + }, + }, + }, + }, + }, + { + Deployments: []*sdkecs.Deployment{ + { + Status: aws.String("INACTIVE"), + ServiceConnectConfiguration: &sdkecs.ServiceConnectConfiguration{ + Enabled: aws.Bool(true), + Services: []*sdkecs.ServiceConnectService{ + { + ClientAliases: []*sdkecs.ServiceConnectClientAlias{ + { + DnsName: aws.String("inactive"), + Port: aws.Int64(80), + }, + }, + }, + }, + }, + }, + }, + }, + }, nil) + }, + wantHosts: []host{ + { + host: "primary", + port: "80", + }, + }, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + m := &testMocks{ + ecs: mocks.NewMockecsLocalClient(ctrl), + } + tc.setupMocks(t, m) + + h := &hostDiscoverer{ + ecs: m.ecs, + } + + hosts, err := h.Hosts(context.Background()) + if tc.wantError != "" { + require.EqualError(t, err, tc.wantError) + } else { + require.NoError(t, err) + require.Equal(t, tc.wantHosts, hosts) + } + }) + } +} diff --git a/internal/pkg/ecs/mocks/mock_ecs.go b/internal/pkg/ecs/mocks/mock_ecs.go index 698d6d45ed2..29fa4ccaf55 100644 --- a/internal/pkg/ecs/mocks/mock_ecs.go +++ b/internal/pkg/ecs/mocks/mock_ecs.go @@ -142,6 +142,21 @@ func (mr *MockecsClientMockRecorder) DescribeTasks(cluster, taskARNs interface{} return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DescribeTasks", reflect.TypeOf((*MockecsClient)(nil).DescribeTasks), cluster, taskARNs) } +// ListServicesByNamespace mocks base method. +func (m *MockecsClient) ListServicesByNamespace(namespace string) ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "ListServicesByNamespace", namespace) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// ListServicesByNamespace indicates an expected call of ListServicesByNamespace. +func (mr *MockecsClientMockRecorder) ListServicesByNamespace(namespace interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ListServicesByNamespace", reflect.TypeOf((*MockecsClient)(nil).ListServicesByNamespace), namespace) +} + // NetworkConfiguration mocks base method. func (m *MockecsClient) NetworkConfiguration(cluster, serviceName string) (*ecs.NetworkConfiguration, error) { m.ctrl.T.Helper() @@ -217,6 +232,26 @@ func (mr *MockecsClientMockRecorder) ServiceRunningTasks(clusterName, serviceNam return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ServiceRunningTasks", reflect.TypeOf((*MockecsClient)(nil).ServiceRunningTasks), clusterName, serviceName) } +// Services mocks base method. +func (m *MockecsClient) Services(cluster string, services ...string) ([]*ecs.Service, error) { + m.ctrl.T.Helper() + varargs := []interface{}{cluster} + for _, a := range services { + varargs = append(varargs, a) + } + ret := m.ctrl.Call(m, "Services", varargs...) + ret0, _ := ret[0].([]*ecs.Service) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Services indicates an expected call of Services. +func (mr *MockecsClientMockRecorder) Services(cluster interface{}, services ...interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + varargs := append([]interface{}{cluster}, services...) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Services", reflect.TypeOf((*MockecsClient)(nil).Services), varargs...) +} + // StopTasks mocks base method. func (m *MockecsClient) StopTasks(tasks []string, opts ...ecs.StopTasksOpts) error { m.ctrl.T.Helper() From efcf1d84cce0133f3164bc2a1f2f97e8619e2580 Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Tue, 24 Oct 2023 14:11:44 -0700 Subject: [PATCH 08/17] write ecs tests --- internal/pkg/aws/ecs/ecs.go | 6 +- internal/pkg/aws/ecs/ecs_test.go | 221 ++++++++++++++++++++++++++++++- 2 files changed, 223 insertions(+), 4 deletions(-) diff --git a/internal/pkg/aws/ecs/ecs.go b/internal/pkg/aws/ecs/ecs.go index f631c21b29a..403deb3f7c2 100644 --- a/internal/pkg/aws/ecs/ecs.go +++ b/internal/pkg/aws/ecs/ecs.go @@ -105,6 +105,9 @@ func (e *ECS) Service(clusterName, serviceName string) (*Service, error) { if err != nil { return nil, err } + if aws.StringValue(svcs[0].ServiceName) != serviceName { + return nil, fmt.Errorf("cannot find service %s", serviceName) + } return svcs[0], nil } @@ -123,7 +126,7 @@ func (e *ECS) Services(cluster string, services ...string) ([]*Service, error) { case err != nil: return nil, fmt.Errorf("describe services: %w", err) case len(resp.Failures) > 0: - return nil, fmt.Errorf("describe services: %s", resp.Failures[0].GoString()) + return nil, fmt.Errorf("describe services: %s", resp.Failures[0].String()) } for j := range resp.Services { @@ -136,7 +139,6 @@ func (e *ECS) Services(cluster string, services ...string) ([]*Service, error) { } func (e *ECS) ListServicesByNamespace(namespace string) ([]string, error) { - // TODO ensure ListServicesByNamesspace doesn't return duplicates? var arns []string err := e.client.ListServicesByNamespacePages(&ecs.ListServicesByNamespaceInput{ Namespace: aws.String(namespace), diff --git a/internal/pkg/aws/ecs/ecs_test.go b/internal/pkg/aws/ecs/ecs_test.go index 39136d3868a..44d5221e79a 100644 --- a/internal/pkg/aws/ecs/ecs_test.go +++ b/internal/pkg/aws/ecs/ecs_test.go @@ -141,7 +141,7 @@ func TestECS_Service(t *testing.T) { Services: aws.StringSlice([]string{"mockService"}), }).Return(nil, errors.New("some error")) }, - wantErr: fmt.Errorf("describe service mockService: some error"), + wantErr: fmt.Errorf("describe services: some error"), }, "errors if failed to find the service": { clusterName: "mockCluster", @@ -187,6 +187,223 @@ func TestECS_Service(t *testing.T) { } } +func TestECS_Services(t *testing.T) { + testCases := map[string]struct { + clusterName string + services []string + mockECSClient func(m *mocks.Mockapi) + + wantErr string + wantSvcs []*Service + }{ + "error if api call error": { + clusterName: "mockCluster", + services: []string{"1"}, + mockECSClient: func(m *mocks.Mockapi) { + m.EXPECT().DescribeServices(&ecs.DescribeServicesInput{ + Cluster: aws.String("mockCluster"), + Services: aws.StringSlice([]string{"1"}), + }).Return(nil, errors.New("some error")) + }, + wantErr: "describe services: some error", + }, + "error if api returns failure": { + clusterName: "mockCluster", + services: []string{"1"}, + mockECSClient: func(m *mocks.Mockapi) { + m.EXPECT().DescribeServices(&ecs.DescribeServicesInput{ + Cluster: aws.String("mockCluster"), + Services: aws.StringSlice([]string{"1"}), + }).Return(&ecs.DescribeServicesOutput{ + Failures: []*ecs.Failure{ + { + Arn: aws.String("arn:1"), + Reason: aws.String("some error"), + }, + }, + }, nil) + }, + wantErr: `describe services: { + Arn: "arn:1", + Reason: "some error" +}`, + }, + "success with > 10": { + clusterName: "mockCluster", + services: []string{ + "1", "2", "3", "4", "5", "6", "7", "8", "9", "10", + "11", + }, + mockECSClient: func(m *mocks.Mockapi) { + m.EXPECT().DescribeServices(&ecs.DescribeServicesInput{ + Cluster: aws.String("mockCluster"), + Services: aws.StringSlice([]string{"1", "2", "3", "4", "5", "6", "7", "8", "9", "10"}), + }).Return(&ecs.DescribeServicesOutput{ + Services: []*ecs.Service{ + { + ServiceName: aws.String("1"), + }, + { + ServiceName: aws.String("2"), + }, + { + ServiceName: aws.String("3"), + }, + { + ServiceName: aws.String("4"), + }, + { + ServiceName: aws.String("5"), + }, + { + ServiceName: aws.String("6"), + }, + { + ServiceName: aws.String("7"), + }, + { + ServiceName: aws.String("8"), + }, + { + ServiceName: aws.String("9"), + }, + { + ServiceName: aws.String("10"), + }, + }, + }, nil) + m.EXPECT().DescribeServices(&ecs.DescribeServicesInput{ + Cluster: aws.String("mockCluster"), + Services: aws.StringSlice([]string{"11"}), + }).Return(&ecs.DescribeServicesOutput{ + Services: []*ecs.Service{ + { + ServiceName: aws.String("11"), + }, + }, + }, nil) + }, + wantSvcs: []*Service{ + { + ServiceName: aws.String("1"), + }, + { + ServiceName: aws.String("2"), + }, + { + ServiceName: aws.String("3"), + }, + { + ServiceName: aws.String("4"), + }, + { + ServiceName: aws.String("5"), + }, + { + ServiceName: aws.String("6"), + }, + { + ServiceName: aws.String("7"), + }, + { + ServiceName: aws.String("8"), + }, + { + ServiceName: aws.String("9"), + }, + { + ServiceName: aws.String("10"), + }, + { + ServiceName: aws.String("11"), + }, + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + // GIVEN + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockECSClient := mocks.NewMockapi(ctrl) + tc.mockECSClient(mockECSClient) + + service := ECS{ + client: mockECSClient, + } + + gotSvcs, gotErr := service.Services(tc.clusterName, tc.services...) + + if tc.wantErr != "" { + require.EqualError(t, gotErr, tc.wantErr) + } else { + require.Equal(t, tc.wantSvcs, gotSvcs) + require.NoError(t, gotErr) + } + }) + } +} + +func TestECS_ListServicesByNamespace(t *testing.T) { + testCases := map[string]struct { + namespace string + mockECSClient func(m *mocks.Mockapi) + + wantErr string + wantARNs []string + }{ + "error if api call error": { + namespace: "mockNamespace", + mockECSClient: func(m *mocks.Mockapi) { + m.EXPECT().ListServicesByNamespacePages(&ecs.ListServicesByNamespaceInput{ + Namespace: aws.String("mockNamespace"), + }, gomock.Any()).Return(errors.New("some error")) + }, + wantErr: "some error", + }, + "success": { + namespace: "mockNamespace", + mockECSClient: func(m *mocks.Mockapi) { + m.EXPECT().ListServicesByNamespacePages(&ecs.ListServicesByNamespaceInput{ + Namespace: aws.String("mockNamespace"), + }, gomock.Any()).DoAndReturn(func(in *ecs.ListServicesByNamespaceInput, fn func(*ecs.ListServicesByNamespaceOutput, bool) bool) error { + fn(&ecs.ListServicesByNamespaceOutput{ + ServiceArns: []*string{aws.String("svc1"), aws.String("svc2"), aws.String("svc3")}, + }, true) + return nil + }) + }, + wantARNs: []string{"svc1", "svc2", "svc3"}, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + // GIVEN + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockECSClient := mocks.NewMockapi(ctrl) + tc.mockECSClient(mockECSClient) + + service := ECS{ + client: mockECSClient, + } + + gotARNs, gotErr := service.ListServicesByNamespace(tc.namespace) + + if tc.wantErr != "" { + require.EqualError(t, gotErr, tc.wantErr) + } else { + require.Equal(t, tc.wantARNs, gotARNs) + require.NoError(t, gotErr) + } + }) + } +} + func TestECS_UpdateService(t *testing.T) { const ( clusterName = "mockCluster" @@ -260,7 +477,7 @@ func TestECS_UpdateService(t *testing.T) { Services: aws.StringSlice([]string{serviceName}), }).Return(nil, errors.New("some error")) }, - wantErr: fmt.Errorf("wait until service mockService becomes stable: describe service mockService: some error"), + wantErr: fmt.Errorf("wait until service mockService becomes stable: describe services: some error"), }, "success": { forceUpdate: true, From 4bde8ffaac7a46147f01a8dd8d8f9e103dde228a Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Wed, 25 Oct 2023 10:20:48 -0700 Subject: [PATCH 09/17] tests for ecs --- internal/pkg/ecs/ecs_test.go | 180 +++++++++++++++++++++++++++++++++++ 1 file changed, 180 insertions(+) diff --git a/internal/pkg/ecs/ecs_test.go b/internal/pkg/ecs/ecs_test.go index f4b6224b5ff..cb69ce558dd 100644 --- a/internal/pkg/ecs/ecs_test.go +++ b/internal/pkg/ecs/ecs_test.go @@ -533,6 +533,186 @@ func TestClient_Service(t *testing.T) { } } +func TestClient_ServiceConnectServices(t *testing.T) { + const ( + mockApp = "mockApp" + mockEnv = "mockEnv" + mockSvc = "mockSvc" + mockSvcARN = "arn:aws:ecs:us-west-2:1234567890:service/mockCluster/mockService" + mockCluster = "mockCluster" + mockService = "mockService" + ) + mockError := errors.New("some error") + getRgEnvClusterInput := map[string]string{ + deploy.AppTagKey: mockApp, + deploy.EnvTagKey: mockEnv, + } + getRgInput := map[string]string{ + deploy.AppTagKey: mockApp, + deploy.EnvTagKey: mockEnv, + deploy.ServiceTagKey: mockSvc, + } + + tests := map[string]struct { + setupMocks func(mocks clientMocks) + + wantedError error + wanted []*ecs.Service + }{ + "error getting the service": { + setupMocks: func(m clientMocks) { + m.resourceGetter.EXPECT().GetResourcesByTags(serviceResourceType, getRgInput). + Return([]*resourcegroups.Resource{ + {ARN: mockSvcARN}, + }, nil) + m.resourceGetter.EXPECT().GetResourcesByTags(clusterResourceType, getRgEnvClusterInput). + Return([]*resourcegroups.Resource{ + {ARN: "mockARN1"}, {ARN: "mockARN2"}, + }, nil) + m.ecsClient.EXPECT().ActiveClusters("mockARN1", "mockARN2").Return([]string{"mockARN1"}, nil) + m.ecsClient.EXPECT().ActiveServices("mockARN1", []string{mockSvcARN}).Return([]string{mockSvcARN}, nil) + m.ecsClient.EXPECT().Service(mockCluster, mockService).Return(nil, mockError) + }, + wantedError: fmt.Errorf(`get service: get ECS service mockService: some error`), + }, + "error listing namespace": { + setupMocks: func(m clientMocks) { + m.resourceGetter.EXPECT().GetResourcesByTags(serviceResourceType, getRgInput). + Return([]*resourcegroups.Resource{ + {ARN: mockSvcARN}, + }, nil) + m.resourceGetter.EXPECT().GetResourcesByTags(clusterResourceType, getRgEnvClusterInput). + Return([]*resourcegroups.Resource{ + {ARN: "mockARN1"}, {ARN: "mockARN2"}, + }, nil) + m.ecsClient.EXPECT().ActiveClusters("mockARN1", "mockARN2").Return([]string{"mockARN1"}, nil) + m.ecsClient.EXPECT().ActiveServices("mockARN1", []string{mockSvcARN}).Return([]string{mockSvcARN}, nil) + m.ecsClient.EXPECT().Service(mockCluster, mockService).Return(&ecs.Service{ + Deployments: []*awsecs.Deployment{ + { + ServiceConnectConfiguration: &awsecs.ServiceConnectConfiguration{ + Namespace: aws.String("namespace"), + }, + }, + }, + }, nil) + m.ecsClient.EXPECT().ListServicesByNamespace("namespace").Return(nil, errors.New("some error")) + }, + wantedError: fmt.Errorf(`get services in same namespace: some error`), + }, + "error getting namespaced services, svc arn removed": { + setupMocks: func(m clientMocks) { + m.resourceGetter.EXPECT().GetResourcesByTags(serviceResourceType, getRgInput). + Return([]*resourcegroups.Resource{ + {ARN: mockSvcARN}, + }, nil) + m.resourceGetter.EXPECT().GetResourcesByTags(clusterResourceType, getRgEnvClusterInput). + Return([]*resourcegroups.Resource{ + {ARN: "cluster1"}, {ARN: "cluster2"}, + }, nil) + m.ecsClient.EXPECT().ActiveClusters("cluster1", "cluster2").Return([]string{"cluster1"}, nil) + m.ecsClient.EXPECT().ActiveServices("cluster1", []string{mockSvcARN}).Return([]string{mockSvcARN}, nil) + m.ecsClient.EXPECT().Service(mockCluster, mockService).Return(&ecs.Service{ + ServiceArn: aws.String(mockSvcARN), + ClusterArn: aws.String("cluster1"), + Deployments: []*awsecs.Deployment{ + { + ServiceConnectConfiguration: &awsecs.ServiceConnectConfiguration{ + Namespace: aws.String("namespace"), + }, + }, + }, + }, nil) + m.ecsClient.EXPECT().ListServicesByNamespace("namespace").Return([]string{ + mockSvcARN, + "svc1", + "svc2", + }, nil) + m.ecsClient.EXPECT().Services("cluster1", "svc1", "svc2").Return(nil, errors.New("some error")) + }, + wantedError: fmt.Errorf(`get services: some error`), + }, + "success, svc arn not removed": { + setupMocks: func(m clientMocks) { + m.resourceGetter.EXPECT().GetResourcesByTags(serviceResourceType, getRgInput). + Return([]*resourcegroups.Resource{ + {ARN: mockSvcARN}, + }, nil) + m.resourceGetter.EXPECT().GetResourcesByTags(clusterResourceType, getRgEnvClusterInput). + Return([]*resourcegroups.Resource{ + {ARN: "cluster1"}, {ARN: "cluster2"}, + }, nil) + m.ecsClient.EXPECT().ActiveClusters("cluster1", "cluster2").Return([]string{"cluster1"}, nil) + m.ecsClient.EXPECT().ActiveServices("cluster1", []string{mockSvcARN}).Return([]string{mockSvcARN}, nil) + m.ecsClient.EXPECT().Service(mockCluster, mockService).Return(&ecs.Service{ + ServiceArn: aws.String(mockSvcARN), + ClusterArn: aws.String("cluster1"), + Deployments: []*awsecs.Deployment{ + { + ServiceConnectConfiguration: &awsecs.ServiceConnectConfiguration{ + Namespace: aws.String("namespace"), + }, + }, + }, + }, nil) + m.ecsClient.EXPECT().ListServicesByNamespace("namespace").Return([]string{ + "svc1", + "svc2", + }, nil) + m.ecsClient.EXPECT().Services("cluster1", "svc1", "svc2").Return([]*ecs.Service{ + { + ServiceArn: aws.String("svc1"), + }, + { + ServiceArn: aws.String("svc2"), + }, + }, nil) + }, + wanted: []*ecs.Service{ + { + ServiceArn: aws.String("svc1"), + }, + { + ServiceArn: aws.String("svc2"), + }, + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + // GIVEN + mockRgGetter := mocks.NewMockresourceGetter(ctrl) + mockECSClient := mocks.NewMockecsClient(ctrl) + mocks := clientMocks{ + resourceGetter: mockRgGetter, + ecsClient: mockECSClient, + } + + test.setupMocks(mocks) + + client := Client{ + rgGetter: mockRgGetter, + ecsClient: mockECSClient, + } + + // WHEN + got, err := client.ServiceConnectServices(mockApp, mockEnv, mockSvc) + + // THEN + if test.wantedError != nil { + require.EqualError(t, err, test.wantedError.Error()) + } else { + require.NoError(t, err) + require.Equal(t, got, test.wanted) + } + }) + } +} + func TestClient_LastUpdatedAt(t *testing.T) { const ( mockApp = "mockApp" From 0b29eb93611276b43970e84e4be35c0cd61beaa4 Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Wed, 25 Oct 2023 10:25:32 -0700 Subject: [PATCH 10/17] ensure correct number of services returned --- internal/pkg/aws/ecs/ecs.go | 2 ++ internal/pkg/aws/ecs/ecs_test.go | 17 +++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/internal/pkg/aws/ecs/ecs.go b/internal/pkg/aws/ecs/ecs.go index 403deb3f7c2..d9318b8a0cc 100644 --- a/internal/pkg/aws/ecs/ecs.go +++ b/internal/pkg/aws/ecs/ecs.go @@ -127,6 +127,8 @@ func (e *ECS) Services(cluster string, services ...string) ([]*Service, error) { return nil, fmt.Errorf("describe services: %w", err) case len(resp.Failures) > 0: return nil, fmt.Errorf("describe services: %s", resp.Failures[0].String()) + case len(resp.Services) != len(split): + return nil, fmt.Errorf("describe services: got %v services, but expected %v", len(resp.Services), len(split)) } for j := range resp.Services { diff --git a/internal/pkg/aws/ecs/ecs_test.go b/internal/pkg/aws/ecs/ecs_test.go index 44d5221e79a..f42a0bb53a5 100644 --- a/internal/pkg/aws/ecs/ecs_test.go +++ b/internal/pkg/aws/ecs/ecs_test.go @@ -228,6 +228,23 @@ func TestECS_Services(t *testing.T) { Reason: "some error" }`, }, + "error if api returns incorrect count": { + clusterName: "mockCluster", + services: []string{"1", "2"}, + mockECSClient: func(m *mocks.Mockapi) { + m.EXPECT().DescribeServices(&ecs.DescribeServicesInput{ + Cluster: aws.String("mockCluster"), + Services: aws.StringSlice([]string{"1", "2"}), + }).Return(&ecs.DescribeServicesOutput{ + Services: []*ecs.Service{ + { + ServiceName: aws.String("1"), + }, + }, + }, nil) + }, + wantErr: "describe services: got 1 services, but expected 2", + }, "success with > 10": { clusterName: "mockCluster", services: []string{ From 4f160485b946d5464ea0c467dbcaed972b23013d Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Wed, 25 Oct 2023 11:09:56 -0700 Subject: [PATCH 11/17] add `ListServicesByNamespace` to env manager --- internal/pkg/cli/run_local.go | 29 ++++++++++++++----- internal/pkg/cli/run_local_test.go | 28 ++++++++++++++++++ .../partials/environment-manager-role.yml | 3 +- 3 files changed, 52 insertions(+), 8 deletions(-) diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index 01088d73630..b4b004ae96f 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -32,6 +32,7 @@ import ( "github.com/aws/copilot-cli/internal/pkg/config" "github.com/aws/copilot-cli/internal/pkg/deploy" "github.com/aws/copilot-cli/internal/pkg/deploy/cloudformation" + "github.com/aws/copilot-cli/internal/pkg/describe" "github.com/aws/copilot-cli/internal/pkg/docker/dockerengine" "github.com/aws/copilot-cli/internal/pkg/docker/orchestrator" "github.com/aws/copilot-cli/internal/pkg/ecs" @@ -83,7 +84,7 @@ type runLocalOpts struct { secretsManager secretGetter sessProvider sessionProvider sess *session.Session - envSess *session.Session + envManagerSess *session.Session targetEnv *config.Environment targetApp *config.Application store store @@ -94,6 +95,7 @@ type runLocalOpts struct { prog progress orchestrator containerOrchestrator hostFinder hostFinder + envChecker versionCompatibilityChecker buildContainerImages func(mft manifest.DynamicWorkload) (map[string]string, error) configureClients func(o *runLocalOpts) error @@ -141,16 +143,16 @@ func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) { if err != nil { return fmt.Errorf("create default session with region %s: %w", o.targetEnv.Region, err) } - o.envSess, err = o.sessProvider.FromRole(o.targetEnv.ManagerRoleARN, o.targetEnv.Region) + o.envManagerSess, err = o.sessProvider.FromRole(o.targetEnv.ManagerRoleARN, o.targetEnv.Region) if err != nil { - return fmt.Errorf("create env session %s: %w", o.targetEnv.Region, err) + return fmt.Errorf("create env manager session %s: %w", o.targetEnv.Region, err) } // EnvManagerRole has permissions to get task def and get SSM values. // However, it doesn't have permissions to get secrets from secrets manager, // so use the default sess and *hope* they have permissions. - o.ecsLocalClient = ecs.New(o.envSess) - o.ssm = ssm.New(o.envSess) + o.ecsLocalClient = ecs.New(o.envManagerSess) + o.ssm = ssm.New(o.envManagerSess) o.secretsManager = secretsmanager.New(defaultSessEnvRegion) resources, err := cloudformation.New(o.sess, cloudformation.WithProgressTracker(os.Stderr)).GetAppResourcesByRegion(o.targetApp, o.targetEnv.Region) @@ -174,8 +176,17 @@ func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) { app: o.appName, env: o.envName, wkld: o.wkldName, - ecs: ecs.New(o.sess), // TODO update env manager role to support actions + ecs: ecs.New(o.envManagerSess), } + envDesc, err := describe.NewEnvDescriber(describe.NewEnvDescriberConfig{ + App: opts.appName, + Env: opts.envName, + ConfigStore: store, + }) + if err != nil { + return fmt.Errorf("create env describer: %w", err) + } + o.envChecker = envDesc return nil } opts.buildContainerImages = func(mft manifest.DynamicWorkload) (map[string]string, error) { @@ -276,6 +287,10 @@ func (o *runLocalOpts) Execute() error { } if o.proxy { + if err := validateMinEnvVersion(o.ws, o.envChecker, o.appName, o.envName, "v1.32.0", "run local --proxy"); err != nil { + return err + } + hosts, err := o.hostFinder.Hosts(ctx) if err != nil { return fmt.Errorf("find hosts to connect to: %w", err) @@ -292,7 +307,7 @@ func (o *runLocalOpts) Execute() error { ws: o.ws, interpolator: o.newInterpolator(o.appName, o.envName), unmarshal: o.unmarshal, - sess: o.envSess, + sess: o.envManagerSess, }) if err != nil { return err diff --git a/internal/pkg/cli/run_local_test.go b/internal/pkg/cli/run_local_test.go index 1cbd6be909e..be19080f1fa 100644 --- a/internal/pkg/cli/run_local_test.go +++ b/internal/pkg/cli/run_local_test.go @@ -208,6 +208,7 @@ type runLocalExecuteMocks struct { prog *mocks.Mockprogress orchestrator *orchestratortest.Double hostFinder *hostFinderDouble + envChecker *mocks.MockversionCompatibilityChecker } type mockProvider struct { @@ -384,6 +385,30 @@ func TestRunLocalOpts_Execute(t *testing.T) { }, wantedError: errors.New(`get task: get env vars: parse env overrides: "bad:OVERRIDE" targets invalid container`), }, + "error getting env version": { + inputAppName: testAppName, + inputWkldName: testWkldName, + inputEnvName: testEnvName, + inProxy: true, + setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { + m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil) + m.envChecker.EXPECT().Version().Return("", fmt.Errorf("some error")) + }, + wantedError: errors.New(`retrieve version of environment stack "testEnv" in application "testApp": some error`), + }, + "error due to old env version": { + inputAppName: testAppName, + inputWkldName: testWkldName, + inputEnvName: testEnvName, + inProxy: true, + setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { + m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil) + m.envChecker.EXPECT().Version().Return("v1.31.0", nil) + }, + wantedError: errors.New(`environment "testEnv" is on version "v1.31.0" which does not support the "run local --proxy" feature`), + }, "error getting hosts to proxy to": { inputAppName: testAppName, inputWkldName: testWkldName, @@ -392,6 +417,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil) + m.envChecker.EXPECT().Version().Return("v1.32.0", nil) m.hostFinder.HostsFn = func(ctx context.Context) ([]host, error) { return nil, fmt.Errorf("some error") } @@ -512,6 +538,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { prog: mocks.NewMockprogress(ctrl), orchestrator: &orchestratortest.Double{}, hostFinder: &hostFinderDouble{}, + envChecker: mocks.NewMockversionCompatibilityChecker(ctrl), } tc.setupMocks(t, m) opts := runLocalOpts{ @@ -562,6 +589,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { prog: m.prog, orchestrator: m.orchestrator, hostFinder: m.hostFinder, + envChecker: m.envChecker, } // WHEN err := opts.Execute() diff --git a/internal/pkg/template/templates/environment/partials/environment-manager-role.yml b/internal/pkg/template/templates/environment/partials/environment-manager-role.yml index ff39256ead2..006f4c66e53 100644 --- a/internal/pkg/template/templates/environment/partials/environment-manager-role.yml +++ b/internal/pkg/template/templates/environment/partials/environment-manager-role.yml @@ -81,7 +81,8 @@ EnvironmentManagerRole: "ecs:DescribeTaskDefinition", "ecs:ListTaskDefinitions", "ecs:ListClusters", - "ecs:RunTask" + "ecs:RunTask", + "ecs:ListServicesByNamespace" ] Resource: "*" - Sid: ExecuteCommand From 3876ead2e46d622259b0e19941d480007bcf135c Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Wed, 25 Oct 2023 11:13:07 -0700 Subject: [PATCH 12/17] fix all env integ tests --- .../testdata/environments/template-with-basic-manifest.yml | 3 ++- .../environments/template-with-cloudfront-observability.yml | 3 ++- .../environments/template-with-custom-security-group.yml | 3 ++- .../environments/template-with-default-access-log-config.yml | 3 ++- .../environments/template-with-defaultvpc-flowlogs.yml | 3 ++- ...th-imported-certs-sslpolicy-custom-empty-security-group.yml | 3 ++- .../environments/template-with-importedvpc-flowlogs.yml | 3 ++- 7 files changed, 14 insertions(+), 7 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-basic-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-basic-manifest.yml index 8eacf13f998..8ece1b5c6dc 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-basic-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-basic-manifest.yml @@ -161,7 +161,8 @@ Resources: "ecs:DescribeTaskDefinition", "ecs:ListTaskDefinitions", "ecs:ListClusters", - "ecs:RunTask" + "ecs:RunTask", + "ecs:ListServicesByNamespace" ] Resource: "*" - Sid: ExecuteCommand diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-cloudfront-observability.yml b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-cloudfront-observability.yml index 4b5d2e48da7..b8bc6894137 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-cloudfront-observability.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-cloudfront-observability.yml @@ -819,7 +819,8 @@ Resources: "ecs:DescribeTaskDefinition", "ecs:ListTaskDefinitions", "ecs:ListClusters", - "ecs:RunTask" + "ecs:RunTask", + "ecs:ListServicesByNamespace" ] Resource: "*" - Sid: ExecuteCommand diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-custom-security-group.yml b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-custom-security-group.yml index e68829e78de..fb6d9d44755 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-custom-security-group.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-custom-security-group.yml @@ -692,7 +692,8 @@ Resources: "ecs:DescribeTaskDefinition", "ecs:ListTaskDefinitions", "ecs:ListClusters", - "ecs:RunTask" + "ecs:RunTask", + "ecs:ListServicesByNamespace" ] Resource: "*" - Sid: ExecuteCommand diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-default-access-log-config.yml b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-default-access-log-config.yml index f2e1234c0f1..834e67877b8 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-default-access-log-config.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-default-access-log-config.yml @@ -218,7 +218,8 @@ Resources: "ecs:DescribeTaskDefinition", "ecs:ListTaskDefinitions", "ecs:ListClusters", - "ecs:RunTask" + "ecs:RunTask", + "ecs:ListServicesByNamespace" ] Resource: "*" - Sid: ExecuteCommand diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-defaultvpc-flowlogs.yml b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-defaultvpc-flowlogs.yml index 23a562dbba3..1553e6a22af 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-defaultvpc-flowlogs.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-defaultvpc-flowlogs.yml @@ -166,7 +166,8 @@ Resources: "ecs:DescribeTaskDefinition", "ecs:ListTaskDefinitions", "ecs:ListClusters", - "ecs:RunTask" + "ecs:RunTask", + "ecs:ListServicesByNamespace" ] Resource: "*" - Sid: ExecuteCommand diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-imported-certs-sslpolicy-custom-empty-security-group.yml b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-imported-certs-sslpolicy-custom-empty-security-group.yml index 18d22f46915..f1e81e2e297 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-imported-certs-sslpolicy-custom-empty-security-group.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-imported-certs-sslpolicy-custom-empty-security-group.yml @@ -669,7 +669,8 @@ Resources: "ecs:DescribeTaskDefinition", "ecs:ListTaskDefinitions", "ecs:ListClusters", - "ecs:RunTask" + "ecs:RunTask", + "ecs:ListServicesByNamespace" ] Resource: "*" - Sid: ExecuteCommand diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-importedvpc-flowlogs.yml b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-importedvpc-flowlogs.yml index 01cbe915544..a6c03bca27b 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-importedvpc-flowlogs.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/environments/template-with-importedvpc-flowlogs.yml @@ -173,7 +173,8 @@ Resources: "ecs:DescribeTaskDefinition", "ecs:ListTaskDefinitions", "ecs:ListClusters", - "ecs:RunTask" + "ecs:RunTask", + "ecs:ListServicesByNamespace" ] Resource: "*" - Sid: ExecuteCommand From 3ec00274260579ab28c3b0c766db67ae71f38ed9 Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Wed, 25 Oct 2023 11:23:08 -0700 Subject: [PATCH 13/17] fix staticcheck --- internal/pkg/aws/ecs/ecs.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/pkg/aws/ecs/ecs.go b/internal/pkg/aws/ecs/ecs.go index d9318b8a0cc..f08c806e4eb 100644 --- a/internal/pkg/aws/ecs/ecs.go +++ b/internal/pkg/aws/ecs/ecs.go @@ -112,6 +112,7 @@ func (e *ECS) Service(clusterName, serviceName string) (*Service, error) { return svcs[0], nil } +// Services calls the ECS API and returns all of the services running in cluster. func (e *ECS) Services(cluster string, services ...string) ([]*Service, error) { var svcs []*Service @@ -140,6 +141,8 @@ func (e *ECS) Services(cluster string, services ...string) ([]*Service, error) { return svcs, nil } +// ListServicesByNamespace returns a list of service ARNs of services that +// are in the given namespace. func (e *ECS) ListServicesByNamespace(namespace string) ([]string, error) { var arns []string err := e.client.ListServicesByNamespacePages(&ecs.ListServicesByNamespaceInput{ From fa47c3ac29ff3726bb50cba31102d972d1d1122b Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Wed, 25 Oct 2023 14:27:51 -0700 Subject: [PATCH 14/17] a bunch of nits from @Lou1415926 --- internal/pkg/aws/ecs/ecs.go | 2 +- internal/pkg/cli/interfaces.go | 2 +- internal/pkg/cli/run_local.go | 42 +++++++++++++++--------------- internal/pkg/cli/run_local_test.go | 4 +-- internal/pkg/ecs/ecs.go | 4 +-- internal/pkg/ecs/ecs_test.go | 2 +- 6 files changed, 28 insertions(+), 28 deletions(-) diff --git a/internal/pkg/aws/ecs/ecs.go b/internal/pkg/aws/ecs/ecs.go index f08c806e4eb..3cbe35678c1 100644 --- a/internal/pkg/aws/ecs/ecs.go +++ b/internal/pkg/aws/ecs/ecs.go @@ -112,7 +112,7 @@ func (e *ECS) Service(clusterName, serviceName string) (*Service, error) { return svcs[0], nil } -// Services calls the ECS API and returns all of the services running in cluster. +// Services calls the ECS API and returns all of the specified services running in cluster. func (e *ECS) Services(cluster string, services ...string) ([]*Service, error) { var svcs []*Service diff --git a/internal/pkg/cli/interfaces.go b/internal/pkg/cli/interfaces.go index 0d698dca9ad..7b6f23d25ba 100644 --- a/internal/pkg/cli/interfaces.go +++ b/internal/pkg/cli/interfaces.go @@ -183,7 +183,7 @@ type repositoryService interface { imageBuilderPusher } -type ecsLocalClient interface { +type ecsClient interface { TaskDefinition(app, env, svc string) (*awsecs.TaskDefinition, error) ServiceConnectServices(app, env, svc string) ([]*awsecs.Service, error) } diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index b4b004ae96f..839b5a33e8b 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -79,7 +79,7 @@ type runLocalOpts struct { runLocalVars sel deploySelector - ecsLocalClient ecsLocalClient + ecsClient ecsClient ssm secretGetter secretsManager secretGetter sessProvider sessionProvider @@ -98,7 +98,7 @@ type runLocalOpts struct { envChecker versionCompatibilityChecker buildContainerImages func(mft manifest.DynamicWorkload) (map[string]string, error) - configureClients func(o *runLocalOpts) error + configureClients func() error labeledTermPrinter func(fw syncbuffer.FileWriter, bufs []*syncbuffer.LabeledSyncBuffer, opts ...syncbuffer.LabeledTermPrinterOption) clideploy.LabeledTermPrinter unmarshal func([]byte) (manifest.DynamicWorkload, error) newInterpolator func(app, env string) interpolator @@ -124,7 +124,7 @@ func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) { labeledTermPrinter := func(fw syncbuffer.FileWriter, bufs []*syncbuffer.LabeledSyncBuffer, opts ...syncbuffer.LabeledTermPrinterOption) clideploy.LabeledTermPrinter { return syncbuffer.NewLabeledTermPrinter(fw, bufs, opts...) } - opts := &runLocalOpts{ + o := &runLocalOpts{ runLocalVars: vars, sel: selector.NewDeploySelect(prompt.New(), store, deployStore), store: store, @@ -138,7 +138,7 @@ func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) { labeledTermPrinter: labeledTermPrinter, prog: termprogress.NewSpinner(log.DiagnosticWriter), } - opts.configureClients = func(o *runLocalOpts) error { + o.configureClients = func() error { defaultSessEnvRegion, err := o.sessProvider.DefaultWithRegion(o.targetEnv.Region) if err != nil { return fmt.Errorf("create default session with region %s: %w", o.targetEnv.Region, err) @@ -151,7 +151,7 @@ func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) { // EnvManagerRole has permissions to get task def and get SSM values. // However, it doesn't have permissions to get secrets from secrets manager, // so use the default sess and *hope* they have permissions. - o.ecsLocalClient = ecs.New(o.envManagerSess) + o.ecsClient = ecs.New(o.envManagerSess) o.ssm = ssm.New(o.envManagerSess) o.secretsManager = secretsmanager.New(defaultSessEnvRegion) @@ -162,9 +162,9 @@ func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) { repoName := clideploy.RepoName(o.appName, o.wkldName) o.repository = repository.NewWithURI(ecr.New(defaultSessEnvRegion), repoName, resources.RepositoryURLs[o.wkldName]) - idPrefix := fmt.Sprintf("%s-%s-%s-", opts.appName, opts.envName, opts.wkldName) + idPrefix := fmt.Sprintf("%s-%s-%s-", o.appName, o.envName, o.wkldName) colorGen := termcolor.ColorGenerator() - o.orchestrator = orchestrator.New(opts.dockerEngine, idPrefix, func(name string, ctr orchestrator.ContainerDefinition) dockerengine.RunLogOptions { + o.orchestrator = orchestrator.New(o.dockerEngine, idPrefix, func(name string, ctr orchestrator.ContainerDefinition) dockerengine.RunLogOptions { return dockerengine.RunLogOptions{ Color: colorGen(), Output: os.Stderr, @@ -179,8 +179,8 @@ func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) { ecs: ecs.New(o.envManagerSess), } envDesc, err := describe.NewEnvDescriber(describe.NewEnvDescriberConfig{ - App: opts.appName, - Env: opts.envName, + App: o.appName, + Env: o.envName, ConfigStore: store, }) if err != nil { @@ -189,22 +189,22 @@ func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) { o.envChecker = envDesc return nil } - opts.buildContainerImages = func(mft manifest.DynamicWorkload) (map[string]string, error) { - gitShortCommit := imageTagFromGit(opts.cmd) + o.buildContainerImages = func(mft manifest.DynamicWorkload) (map[string]string, error) { + gitShortCommit := imageTagFromGit(o.cmd) image := clideploy.ContainerImageIdentifier{ GitShortCommitTag: gitShortCommit, } out := &clideploy.UploadArtifactsOutput{} if err := clideploy.BuildContainerImages(&clideploy.ImageActionInput{ - Name: opts.wkldName, - WorkspacePath: opts.ws.Path(), + Name: o.wkldName, + WorkspacePath: o.ws.Path(), Image: image, Mft: mft.Manifest(), GitShortCommitTag: gitShortCommit, - Builder: opts.repository, - Login: opts.repository.Login, - CheckDockerEngine: opts.dockerEngine.CheckDockerEngineRunning, - LabeledTermPrinter: opts.labeledTermPrinter, + Builder: o.repository, + Login: o.repository.Login, + CheckDockerEngine: o.dockerEngine.CheckDockerEngineRunning, + LabeledTermPrinter: o.labeledTermPrinter, }, out); err != nil { return nil, err } @@ -219,7 +219,7 @@ func newRunLocalOpts(vars runLocalVars) (*runLocalOpts, error) { } return containerURIs, nil } - return opts, nil + return o, nil } // Validate returns an error for any invalid optional flags. @@ -275,7 +275,7 @@ func (o *runLocalOpts) validateAndAskWkldEnvName() error { // Execute builds and runs the workload images locally. func (o *runLocalOpts) Execute() error { - if err := o.configureClients(o); err != nil { + if err := o.configureClients(); err != nil { return err } @@ -354,7 +354,7 @@ func (o *runLocalOpts) Execute() error { } func (o *runLocalOpts) getTask(ctx context.Context) (orchestrator.Task, error) { - td, err := o.ecsLocalClient.TaskDefinition(o.appName, o.envName, o.wkldName) + td, err := o.ecsClient.TaskDefinition(o.appName, o.envName, o.wkldName) if err != nil { return orchestrator.Task{}, fmt.Errorf("get task definition: %w", err) } @@ -601,7 +601,7 @@ type host struct { } type hostDiscoverer struct { - ecs ecsLocalClient + ecs ecsClient app string env string wkld string diff --git a/internal/pkg/cli/run_local_test.go b/internal/pkg/cli/run_local_test.go index be19080f1fa..fbbfa218f95 100644 --- a/internal/pkg/cli/run_local_test.go +++ b/internal/pkg/cli/run_local_test.go @@ -565,14 +565,14 @@ func TestRunLocalOpts_Execute(t *testing.T) { unmarshal: func(b []byte) (manifest.DynamicWorkload, error) { return m.mockMft, nil }, - configureClients: func(o *runLocalOpts) error { + configureClients: func() error { return nil }, buildContainerImages: func(mft manifest.DynamicWorkload) (map[string]string, error) { return mockContainerURIs, tc.buildImagesError }, ws: m.ws, - ecsLocalClient: m.ecsLocalClient, + ecsClient: m.ecsLocalClient, ssm: m.ssm, secretsManager: m.secretsManager, store: m.store, diff --git a/internal/pkg/ecs/ecs.go b/internal/pkg/ecs/ecs.go index 0497fef7b93..02f3f5a473d 100644 --- a/internal/pkg/ecs/ecs.go +++ b/internal/pkg/ecs/ecs.go @@ -137,7 +137,7 @@ func (c Client) Service(app, env, svc string) (*ecs.Service, error) { } // ServiceConnectServices returns a list of services that are in the same -// service connect namespace as the given service. +// service connect namespace as the given service, except for itself. func (c Client) ServiceConnectServices(app, env, svc string) ([]*ecs.Service, error) { s, err := c.Service(app, env, svc) if err != nil { @@ -149,7 +149,7 @@ func (c Client) ServiceConnectServices(app, env, svc string) ([]*ecs.Service, er arns, err := c.ecsClient.ListServicesByNamespace(aws.StringValue(s.Deployments[0].ServiceConnectConfiguration.Namespace)) if err != nil { - return nil, fmt.Errorf("get services in same namespace: %w", err) + return nil, fmt.Errorf("get services in the same namespace: %w", err) } // remove this service's arn diff --git a/internal/pkg/ecs/ecs_test.go b/internal/pkg/ecs/ecs_test.go index cb69ce558dd..d6e00dc1aba 100644 --- a/internal/pkg/ecs/ecs_test.go +++ b/internal/pkg/ecs/ecs_test.go @@ -598,7 +598,7 @@ func TestClient_ServiceConnectServices(t *testing.T) { }, nil) m.ecsClient.EXPECT().ListServicesByNamespace("namespace").Return(nil, errors.New("some error")) }, - wantedError: fmt.Errorf(`get services in same namespace: some error`), + wantedError: fmt.Errorf(`get services in the same namespace: some error`), }, "error getting namespaced services, svc arn removed": { setupMocks: func(m clientMocks) { From 519363a177e7a6432f73ba5ff51cd26bb814a5d1 Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Wed, 25 Oct 2023 15:05:26 -0700 Subject: [PATCH 15/17] move min env versions to template package --- internal/pkg/cli/job_run.go | 8 ++++---- internal/pkg/cli/run_local.go | 7 ++++--- internal/pkg/cli/secret_init.go | 30 +++++++++++------------------- internal/pkg/template/env.go | 6 ++++++ 4 files changed, 25 insertions(+), 26 deletions(-) diff --git a/internal/pkg/cli/job_run.go b/internal/pkg/cli/job_run.go index 33454fbf7f9..157943e1ac4 100644 --- a/internal/pkg/cli/job_run.go +++ b/internal/pkg/cli/job_run.go @@ -6,9 +6,6 @@ package cli import ( "fmt" - "github.com/aws/copilot-cli/internal/pkg/workspace" - "github.com/spf13/afero" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/aws-sdk-go/service/ssm" @@ -19,9 +16,12 @@ import ( "github.com/aws/copilot-cli/internal/pkg/config" "github.com/aws/copilot-cli/internal/pkg/describe" "github.com/aws/copilot-cli/internal/pkg/runner/jobrunner" + "github.com/aws/copilot-cli/internal/pkg/template" "github.com/aws/copilot-cli/internal/pkg/term/log" "github.com/aws/copilot-cli/internal/pkg/term/prompt" "github.com/aws/copilot-cli/internal/pkg/term/selector" + "github.com/aws/copilot-cli/internal/pkg/workspace" + "github.com/spf13/afero" "github.com/spf13/cobra" ) @@ -204,7 +204,7 @@ func (o *jobRunOpts) validateEnvCompatible() error { if err != nil { return err } - return validateMinEnvVersion(o.ws, envStack, o.appName, o.envName, "v1.12.0", "job run") + return validateMinEnvVersion(o.ws, envStack, o.appName, o.envName, template.LeastVersionForFeature(template.JobRunFeatureName), "job run") } func buildJobRunCmd() *cobra.Command { diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index 839b5a33e8b..f53febac4de 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -20,7 +20,7 @@ import ( sdkecs "github.com/aws/aws-sdk-go/service/ecs" sdksecretsmanager "github.com/aws/aws-sdk-go/service/secretsmanager" sdkssm "github.com/aws/aws-sdk-go/service/ssm" - "github.com/aws/copilot-cli/cmd/copilot/template" + cmdtemplate "github.com/aws/copilot-cli/cmd/copilot/template" "github.com/aws/copilot-cli/internal/pkg/aws/ecr" awsecs "github.com/aws/copilot-cli/internal/pkg/aws/ecs" "github.com/aws/copilot-cli/internal/pkg/aws/identity" @@ -39,6 +39,7 @@ import ( "github.com/aws/copilot-cli/internal/pkg/exec" "github.com/aws/copilot-cli/internal/pkg/manifest" "github.com/aws/copilot-cli/internal/pkg/repository" + "github.com/aws/copilot-cli/internal/pkg/template" termcolor "github.com/aws/copilot-cli/internal/pkg/term/color" "github.com/aws/copilot-cli/internal/pkg/term/log" termprogress "github.com/aws/copilot-cli/internal/pkg/term/progress" @@ -287,7 +288,7 @@ func (o *runLocalOpts) Execute() error { } if o.proxy { - if err := validateMinEnvVersion(o.ws, o.envChecker, o.appName, o.envName, "v1.32.0", "run local --proxy"); err != nil { + if err := validateMinEnvVersion(o.ws, o.envChecker, o.appName, o.envName, template.LeastVersionForFeature(template.RunLocalProxyFeatureName), "run local --proxy"); err != nil { return err } @@ -654,7 +655,7 @@ func BuildRunLocalCmd() *cobra.Command { "group": group.Develop, }, } - cmd.SetUsageTemplate(template.Usage) + cmd.SetUsageTemplate(cmdtemplate.Usage) cmd.Flags().StringVarP(&vars.wkldName, nameFlag, nameFlagShort, "", workloadFlagDescription) cmd.Flags().StringVarP(&vars.envName, envFlag, envFlagShort, "", envFlagDescription) diff --git a/internal/pkg/cli/secret_init.go b/internal/pkg/cli/secret_init.go index 3308fbce009..805f38226b2 100644 --- a/internal/pkg/cli/secret_init.go +++ b/internal/pkg/cli/secret_init.go @@ -9,33 +9,26 @@ import ( "sort" "strings" - "github.com/aws/copilot-cli/internal/pkg/workspace" - - "github.com/aws/copilot-cli/internal/pkg/describe" - - "golang.org/x/text/cases" - "golang.org/x/text/language" - - "github.com/aws/copilot-cli/internal/pkg/aws/identity" - - "github.com/dustin/go-humanize/english" - - "gopkg.in/yaml.v3" - "github.com/aws/aws-sdk-go/aws" awsssm "github.com/aws/aws-sdk-go/service/ssm" - - "github.com/spf13/afero" - "github.com/spf13/cobra" - + "github.com/aws/copilot-cli/internal/pkg/aws/identity" "github.com/aws/copilot-cli/internal/pkg/aws/sessions" "github.com/aws/copilot-cli/internal/pkg/aws/ssm" "github.com/aws/copilot-cli/internal/pkg/config" "github.com/aws/copilot-cli/internal/pkg/deploy" + "github.com/aws/copilot-cli/internal/pkg/describe" + "github.com/aws/copilot-cli/internal/pkg/template" "github.com/aws/copilot-cli/internal/pkg/term/color" "github.com/aws/copilot-cli/internal/pkg/term/log" "github.com/aws/copilot-cli/internal/pkg/term/prompt" "github.com/aws/copilot-cli/internal/pkg/term/selector" + "github.com/aws/copilot-cli/internal/pkg/workspace" + "github.com/dustin/go-humanize/english" + "github.com/spf13/afero" + "github.com/spf13/cobra" + "golang.org/x/text/cases" + "golang.org/x/text/language" + "gopkg.in/yaml.v3" ) const ( @@ -254,12 +247,11 @@ func (o *secretInitOpts) configureClientsAndUpgradeForEnvironments(secrets map[s } } - const minEnvVersionForSecretInit = "v1.4.0" for envName := range envNames { if err := o.configureClientsForEnv(envName); err != nil { return err } - if err := validateMinEnvVersion(o.ws, o.envCompatibilityChecker[envName], o.appName, envName, minEnvVersionForSecretInit, "secret init"); err != nil { + if err := validateMinEnvVersion(o.ws, o.envCompatibilityChecker[envName], o.appName, envName, template.LeastVersionForFeature(template.SecretInitFeatureName), "secret init"); err != nil { return err } } diff --git a/internal/pkg/template/env.go b/internal/pkg/template/env.go index 4504129f5e3..44225b673f8 100644 --- a/internal/pkg/template/env.go +++ b/internal/pkg/template/env.go @@ -26,6 +26,9 @@ const ( InternalALBFeatureName = "InternalALBWorkloads" AliasesFeatureName = "Aliases" AppRunnerPrivateServiceFeatureName = "AppRunnerPrivateWorkloads" + SecretInitFeatureName = "SecretInit" + JobRunFeatureName = "JobRun" + RunLocalProxyFeatureName = "RunLocalProxy" ) // LastForceDeployIDOutputName is the logical ID of the deployment controller output. @@ -47,6 +50,9 @@ var leastVersionForFeature = map[string]string{ InternalALBFeatureName: "v1.10.0", AliasesFeatureName: "v1.4.0", AppRunnerPrivateServiceFeatureName: "v1.23.0", + SecretInitFeatureName: "v1.4.0", + JobRunFeatureName: "v1.12.0", + RunLocalProxyFeatureName: "v1.32.0", } // AvailableEnvFeatures returns a list of the latest available feature, named after their corresponding parameter names. From dd1bd61cd54d9e2b37bbe8e342bc9df1312b80ff Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Wed, 25 Oct 2023 15:30:10 -0700 Subject: [PATCH 16/17] switch to consts, rerun gen mocks --- internal/pkg/cli/job_run.go | 2 +- internal/pkg/cli/mocks/mock_interfaces.go | 34 +++++++++++------------ internal/pkg/cli/run_local.go | 2 +- internal/pkg/cli/run_local_test.go | 30 ++++++++++---------- internal/pkg/cli/secret_init.go | 2 +- internal/pkg/template/env.go | 12 ++++---- 6 files changed, 41 insertions(+), 41 deletions(-) diff --git a/internal/pkg/cli/job_run.go b/internal/pkg/cli/job_run.go index 157943e1ac4..96a749cc27f 100644 --- a/internal/pkg/cli/job_run.go +++ b/internal/pkg/cli/job_run.go @@ -204,7 +204,7 @@ func (o *jobRunOpts) validateEnvCompatible() error { if err != nil { return err } - return validateMinEnvVersion(o.ws, envStack, o.appName, o.envName, template.LeastVersionForFeature(template.JobRunFeatureName), "job run") + return validateMinEnvVersion(o.ws, envStack, o.appName, o.envName, template.JobRunMinEnvVersion, "job run") } func buildJobRunCmd() *cobra.Command { diff --git a/internal/pkg/cli/mocks/mock_interfaces.go b/internal/pkg/cli/mocks/mock_interfaces.go index f1ecfabcfb9..db3c33780ce 100644 --- a/internal/pkg/cli/mocks/mock_interfaces.go +++ b/internal/pkg/cli/mocks/mock_interfaces.go @@ -1715,31 +1715,31 @@ func (mr *MockrepositoryServiceMockRecorder) Login() *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Login", reflect.TypeOf((*MockrepositoryService)(nil).Login)) } -// MockecsLocalClient is a mock of ecsLocalClient interface. -type MockecsLocalClient struct { +// MockecsClient is a mock of ecsClient interface. +type MockecsClient struct { ctrl *gomock.Controller - recorder *MockecsLocalClientMockRecorder + recorder *MockecsClientMockRecorder } -// MockecsLocalClientMockRecorder is the mock recorder for MockecsLocalClient. -type MockecsLocalClientMockRecorder struct { - mock *MockecsLocalClient +// MockecsClientMockRecorder is the mock recorder for MockecsClient. +type MockecsClientMockRecorder struct { + mock *MockecsClient } -// NewMockecsLocalClient creates a new mock instance. -func NewMockecsLocalClient(ctrl *gomock.Controller) *MockecsLocalClient { - mock := &MockecsLocalClient{ctrl: ctrl} - mock.recorder = &MockecsLocalClientMockRecorder{mock} +// NewMockecsClient creates a new mock instance. +func NewMockecsClient(ctrl *gomock.Controller) *MockecsClient { + mock := &MockecsClient{ctrl: ctrl} + mock.recorder = &MockecsClientMockRecorder{mock} return mock } // EXPECT returns an object that allows the caller to indicate expected use. -func (m *MockecsLocalClient) EXPECT() *MockecsLocalClientMockRecorder { +func (m *MockecsClient) EXPECT() *MockecsClientMockRecorder { return m.recorder } // ServiceConnectServices mocks base method. -func (m *MockecsLocalClient) ServiceConnectServices(app, env, svc string) ([]*ecs.Service, error) { +func (m *MockecsClient) ServiceConnectServices(app, env, svc string) ([]*ecs.Service, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "ServiceConnectServices", app, env, svc) ret0, _ := ret[0].([]*ecs.Service) @@ -1748,13 +1748,13 @@ func (m *MockecsLocalClient) ServiceConnectServices(app, env, svc string) ([]*ec } // ServiceConnectServices indicates an expected call of ServiceConnectServices. -func (mr *MockecsLocalClientMockRecorder) ServiceConnectServices(app, env, svc interface{}) *gomock.Call { +func (mr *MockecsClientMockRecorder) ServiceConnectServices(app, env, svc interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ServiceConnectServices", reflect.TypeOf((*MockecsLocalClient)(nil).ServiceConnectServices), app, env, svc) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ServiceConnectServices", reflect.TypeOf((*MockecsClient)(nil).ServiceConnectServices), app, env, svc) } // TaskDefinition mocks base method. -func (m *MockecsLocalClient) TaskDefinition(app, env, svc string) (*ecs.TaskDefinition, error) { +func (m *MockecsClient) 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) @@ -1763,9 +1763,9 @@ func (m *MockecsLocalClient) TaskDefinition(app, env, svc string) (*ecs.TaskDefi } // TaskDefinition indicates an expected call of TaskDefinition. -func (mr *MockecsLocalClientMockRecorder) TaskDefinition(app, env, svc interface{}) *gomock.Call { +func (mr *MockecsClientMockRecorder) TaskDefinition(app, env, svc interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TaskDefinition", reflect.TypeOf((*MockecsLocalClient)(nil).TaskDefinition), app, env, svc) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "TaskDefinition", reflect.TypeOf((*MockecsClient)(nil).TaskDefinition), app, env, svc) } // MocklogEventsWriter is a mock of logEventsWriter interface. diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index f53febac4de..f1881626edd 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -288,7 +288,7 @@ func (o *runLocalOpts) Execute() error { } if o.proxy { - if err := validateMinEnvVersion(o.ws, o.envChecker, o.appName, o.envName, template.LeastVersionForFeature(template.RunLocalProxyFeatureName), "run local --proxy"); err != nil { + if err := validateMinEnvVersion(o.ws, o.envChecker, o.appName, o.envName, template.RunLocalProxyMinEnvVersion, "run local --proxy"); err != nil { return err } diff --git a/internal/pkg/cli/run_local_test.go b/internal/pkg/cli/run_local_test.go index fbbfa218f95..31d30669ece 100644 --- a/internal/pkg/cli/run_local_test.go +++ b/internal/pkg/cli/run_local_test.go @@ -194,7 +194,7 @@ func TestRunLocalOpts_Ask(t *testing.T) { } type runLocalExecuteMocks struct { - ecsLocalClient *mocks.MockecsLocalClient + ecsClient *mocks.MockecsClient store *mocks.Mockstore sessCreds credentials.Provider interpolator *mocks.Mockinterpolator @@ -369,7 +369,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { inputWkldName: testWkldName, inputEnvName: testEnvName, setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { - m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(nil, testError) + m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(nil, testError) }, wantedError: fmt.Errorf("get task: get task definition: %w", testError), }, @@ -381,7 +381,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { "bad:OVERRIDE": "i fail", }, setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { - m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) }, wantedError: errors.New(`get task: get env vars: parse env overrides: "bad:OVERRIDE" targets invalid container`), }, @@ -391,7 +391,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { inputEnvName: testEnvName, inProxy: true, setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { - m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil) m.envChecker.EXPECT().Version().Return("", fmt.Errorf("some error")) }, @@ -403,7 +403,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { inputEnvName: testEnvName, inProxy: true, setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { - m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil) m.envChecker.EXPECT().Version().Return("v1.31.0", nil) }, @@ -415,7 +415,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { inputEnvName: testEnvName, inProxy: true, setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { - m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil) m.envChecker.EXPECT().Version().Return("v1.32.0", nil) m.hostFinder.HostsFn = func(ctx context.Context) ([]host, error) { @@ -429,7 +429,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { inputWkldName: testWkldName, inputEnvName: testEnvName, setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { - m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil) m.ws.EXPECT().ReadWorkloadManifest(testWkldName).Return(nil, errors.New("some error")) }, @@ -440,7 +440,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { inputWkldName: testWkldName, inputEnvName: testEnvName, setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { - m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil) m.ws.EXPECT().ReadWorkloadManifest(testWkldName).Return([]byte(""), nil) m.interpolator.EXPECT().Interpolate("").Return("", errors.New("some error")) @@ -453,7 +453,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { inputEnvName: testEnvName, buildImagesError: errors.New("some error"), setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { - m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil) m.ws.EXPECT().ReadWorkloadManifest(testWkldName).Return([]byte(""), nil) m.interpolator.EXPECT().Interpolate("").Return("", nil) @@ -465,7 +465,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { inputWkldName: testWkldName, inputEnvName: testEnvName, setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { - m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil) m.ws.EXPECT().ReadWorkloadManifest(testWkldName).Return([]byte(""), nil) m.interpolator.EXPECT().Interpolate("").Return("", nil) @@ -489,7 +489,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { inputWkldName: testWkldName, inputEnvName: testEnvName, setupMocks: func(t *testing.T, m *runLocalExecuteMocks) { - m.ecsLocalClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) + m.ecsClient.EXPECT().TaskDefinition(testAppName, testEnvName, testWkldName).Return(taskDef, nil) m.ssm.EXPECT().GetSecretValue(gomock.Any(), "mysecret").Return("secretvalue", nil) m.ws.EXPECT().ReadWorkloadManifest(testWkldName).Return([]byte(""), nil) m.interpolator.EXPECT().Interpolate("").Return("", nil) @@ -526,7 +526,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() m := &runLocalExecuteMocks{ - ecsLocalClient: mocks.NewMockecsLocalClient(ctrl), + ecsClient: mocks.NewMockecsClient(ctrl), ssm: mocks.NewMocksecretGetter(ctrl), secretsManager: mocks.NewMocksecretGetter(ctrl), store: mocks.NewMockstore(ctrl), @@ -572,7 +572,7 @@ func TestRunLocalOpts_Execute(t *testing.T) { return mockContainerURIs, tc.buildImagesError }, ws: m.ws, - ecsClient: m.ecsLocalClient, + ecsClient: m.ecsClient, ssm: m.ssm, secretsManager: m.secretsManager, store: m.store, @@ -1004,7 +1004,7 @@ func TestRunLocalOpts_getEnvVars(t *testing.T) { func TestRunLocal_HostDiscovery(t *testing.T) { type testMocks struct { - ecs *mocks.MockecsLocalClient + ecs *mocks.MockecsClient } tests := map[string]struct { @@ -1093,7 +1093,7 @@ func TestRunLocal_HostDiscovery(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() m := &testMocks{ - ecs: mocks.NewMockecsLocalClient(ctrl), + ecs: mocks.NewMockecsClient(ctrl), } tc.setupMocks(t, m) diff --git a/internal/pkg/cli/secret_init.go b/internal/pkg/cli/secret_init.go index 805f38226b2..5c32e36a3ad 100644 --- a/internal/pkg/cli/secret_init.go +++ b/internal/pkg/cli/secret_init.go @@ -251,7 +251,7 @@ func (o *secretInitOpts) configureClientsAndUpgradeForEnvironments(secrets map[s if err := o.configureClientsForEnv(envName); err != nil { return err } - if err := validateMinEnvVersion(o.ws, o.envCompatibilityChecker[envName], o.appName, envName, template.LeastVersionForFeature(template.SecretInitFeatureName), "secret init"); err != nil { + if err := validateMinEnvVersion(o.ws, o.envCompatibilityChecker[envName], o.appName, envName, template.SecretInitMinEnvVersion, "secret init"); err != nil { return err } } diff --git a/internal/pkg/template/env.go b/internal/pkg/template/env.go index 44225b673f8..789e6e7f6a6 100644 --- a/internal/pkg/template/env.go +++ b/internal/pkg/template/env.go @@ -18,6 +18,12 @@ const ( envBootstrapCFTemplatePath = "environment/bootstrap-cf.yml" ) +const ( + SecretInitMinEnvVersion = "v1.4.0" + JobRunMinEnvVersion = "v1.12.0" + RunLocalProxyMinEnvVersion = "v1.32.0" +) + // Available env-controller managed feature names. const ( ALBFeatureName = "ALBWorkloads" @@ -26,9 +32,6 @@ const ( InternalALBFeatureName = "InternalALBWorkloads" AliasesFeatureName = "Aliases" AppRunnerPrivateServiceFeatureName = "AppRunnerPrivateWorkloads" - SecretInitFeatureName = "SecretInit" - JobRunFeatureName = "JobRun" - RunLocalProxyFeatureName = "RunLocalProxy" ) // LastForceDeployIDOutputName is the logical ID of the deployment controller output. @@ -50,9 +53,6 @@ var leastVersionForFeature = map[string]string{ InternalALBFeatureName: "v1.10.0", AliasesFeatureName: "v1.4.0", AppRunnerPrivateServiceFeatureName: "v1.23.0", - SecretInitFeatureName: "v1.4.0", - JobRunFeatureName: "v1.12.0", - RunLocalProxyFeatureName: "v1.32.0", } // AvailableEnvFeatures returns a list of the latest available feature, named after their corresponding parameter names. From 1ce1cafd101986c7d148e28016ebe934cde63c78 Mon Sep 17 00:00:00 2001 From: Danny Randall <10566468+dannyrandall@users.noreply.github.com> Date: Wed, 25 Oct 2023 15:36:16 -0700 Subject: [PATCH 17/17] add comment --- internal/pkg/template/env.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/pkg/template/env.go b/internal/pkg/template/env.go index 789e6e7f6a6..059f95b9ec2 100644 --- a/internal/pkg/template/env.go +++ b/internal/pkg/template/env.go @@ -18,6 +18,7 @@ const ( envBootstrapCFTemplatePath = "environment/bootstrap-cf.yml" ) +// The minimum required environment template version for various features. const ( SecretInitMinEnvVersion = "v1.4.0" JobRunMinEnvVersion = "v1.12.0"