From 00527f746cf829c4c14429d166dd30618fde878d Mon Sep 17 00:00:00 2001 From: penghaoh Date: Thu, 29 Sep 2022 10:01:42 -0700 Subject: [PATCH 01/12] feat: add service connect mft --- .../cloudformation/stack/backend_svc.go | 20 ++++--- .../cloudformation/stack/backend_svc_test.go | 10 ++++ .../deploy/cloudformation/stack/lb_web_svc.go | 5 +- .../cloudformation/stack/lb_web_svc_test.go | 16 +++-- .../workloads/backend/simple-manifest.yml | 2 + .../workloads/backend/simple-params.json | 4 +- .../workloads/backend/simple-template.yml | 27 +++++++-- .../cloudformation/stack/transformers.go | 12 ++++ .../deploy/cloudformation/stack/worker_svc.go | 3 +- internal/pkg/manifest/svc_test.go | 5 ++ internal/pkg/manifest/transform.go | 27 +++++++++ internal/pkg/manifest/transform_test.go | 58 +++++++++++++++++++ internal/pkg/manifest/validate.go | 13 +++++ internal/pkg/manifest/validate_test.go | 8 +-- internal/pkg/manifest/workload.go | 54 ++++++++++++++--- internal/pkg/manifest/workload_test.go | 43 ++++++++++++++ .../partials/cf/service-base-properties.yml | 2 +- .../workloads/partials/cf/sidecars.yml | 2 +- .../partials/cf/workload-container.yml | 6 +- .../workloads/services/backend/cf.yml | 2 +- internal/pkg/template/workload.go | 7 +-- 21 files changed, 281 insertions(+), 45 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/backend_svc.go b/internal/pkg/deploy/cloudformation/stack/backend_svc.go index 42a8d0796a1..f21b24087d3 100644 --- a/internal/pkg/deploy/cloudformation/stack/backend_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/backend_svc.go @@ -142,7 +142,7 @@ func (s *BackendService) Template() (string, error) { allowedSourceIPs = append(allowedSourceIPs, string(ipNet)) } - _, targetContainerPort := s.httpLoadBalancerTarget() + targetContainer, targetContainerPort := s.httpLoadBalancerTarget() content, err := s.parser.ParseBackendService(template.WorkloadOpts{ AppName: s.app, EnvName: s.env, @@ -166,7 +166,9 @@ func (s *BackendService) Template() (string, error) { HealthCheck: convertContainerHealthCheck(s.manifest.BackendServiceConfig.ImageConfig.HealthCheck), HTTPTargetContainer: template.HTTPTargetContainer{ Port: aws.StringValue(targetContainerPort), + Name: aws.StringValue(targetContainer), }, + ServiceConnect: convertServiceConnect(s.manifest.Network.Connect), HTTPHealthCheck: convertHTTPHealthCheck(&s.manifest.RoutingRule.HealthCheck), DeregistrationDelay: deregistrationDelay, AllowedSourceIps: allowedSourceIPs, @@ -241,18 +243,18 @@ func (s *BackendService) Parameters() ([]*cloudformation.Parameter, error) { ParameterKey: aws.String(WorkloadEnvFileARNParamKey), ParameterValue: aws.String(s.rc.EnvFileARN), }, + { + ParameterKey: aws.String(WorkloadTargetContainerParamKey), + ParameterValue: targetContainer, + }, + { + ParameterKey: aws.String(WorkloadTargetPortParamKey), + ParameterValue: targetPort, + }, }...) if !s.manifest.RoutingRule.IsEmpty() { params = append(params, []*cloudformation.Parameter{ - { - ParameterKey: aws.String(WorkloadTargetContainerParamKey), - ParameterValue: targetContainer, - }, - { - ParameterKey: aws.String(WorkloadTargetPortParamKey), - ParameterValue: targetPort, - }, { ParameterKey: aws.String(WorkloadRulePathParamKey), ParameterValue: s.manifest.RoutingRule.Path, diff --git a/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go b/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go index b1b4dbdacdd..67889cd6765 100644 --- a/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go +++ b/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go @@ -271,6 +271,7 @@ Outputs: HostedZoneAliases: make(template.AliasesForHostedZone), HTTPTargetContainer: template.HTTPTargetContainer{ Port: "8080", + Name: "api", }, HTTPHealthCheck: template.HTTPHealthCheckOpts{ HealthCheckPath: manifest.DefaultHealthCheckPath, @@ -435,6 +436,7 @@ Outputs: }, }, HTTPTargetContainer: template.HTTPTargetContainer{ + Name: "envoy", Port: "443", }, HTTPHealthCheck: template.HTTPHealthCheckOpts{ @@ -561,6 +563,14 @@ func TestBackendService_Parameters(t *testing.T) { ParameterKey: aws.String(WorkloadEnvFileARNParamKey), ParameterValue: aws.String(""), }, + { + ParameterKey: aws.String(WorkloadTargetContainerParamKey), + ParameterValue: aws.String("frontend"), + }, + { + ParameterKey: aws.String(WorkloadTargetPortParamKey), + ParameterValue: aws.String("8080"), + }, }, params) } diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go index 25916287594..a4e40a418e9 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go @@ -214,9 +214,10 @@ func (s *LoadBalancedWebService) Template() (string, error) { ExecuteCommand: convertExecuteCommand(&s.manifest.ExecuteCommand), WorkloadType: manifest.LoadBalancedWebServiceType, HTTPTargetContainer: template.HTTPTargetContainer{ - Port: aws.StringValue(targetContainerPort), - Container: aws.StringValue(targetContainer), + Port: aws.StringValue(targetContainerPort), + Name: aws.StringValue(targetContainer), }, + ServiceConnect: convertServiceConnect(s.manifest.Network.Connect), HealthCheck: convertContainerHealthCheck(s.manifest.ImageConfig.HealthCheck), HTTPHealthCheck: convertHTTPHealthCheck(&s.manifest.RoutingRule.HealthCheck), DeregistrationDelay: deregistrationDelay, diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go b/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go index cabce3e5f45..22244758205 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go @@ -207,6 +207,11 @@ Outputs: String: nil, StringSlice: []string{"world"}, } + mft.Network.Connect = manifest.ServiceConnect{ + ServiceConnectArgs: manifest.ServiceConnectArgs{ + Alias: aws.String("frontend"), + }, + } var actual template.WorkloadOpts parser := mocks.NewMockloadBalancedWebSvcReadParser(ctrl) @@ -262,8 +267,11 @@ Outputs: HTTPRedirect: true, DeregistrationDelay: aws.Int64(60), HTTPTargetContainer: template.HTTPTargetContainer{ - Container: "frontend", - Port: "80", + Name: "frontend", + Port: "80", + }, + ServiceConnect: &template.ServiceConnect{ + Alias: aws.String("frontend"), }, HealthCheck: &template.ContainerHealthCheck{ Command: []string{"CMD-SHELL", "curl -f http://localhost/ || exit 1"}, @@ -415,8 +423,8 @@ Outputs: // THEN require.NoError(t, err) require.Equal(t, template.HTTPTargetContainer{ - Port: "443", - Container: "envoy", + Port: "443", + Name: "envoy", }, actual.HTTPTargetContainer) }) } diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-manifest.yml index 54419ac29a7..53fdabea06a 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-manifest.yml @@ -4,6 +4,8 @@ image: build: Dockerfile port: 8080 +network: + connect: true cpu: 256 memory: 512 count: 1 diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-params.json b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-params.json index f74920a8e45..a9cd4be2b8d 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-params.json +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-params.json @@ -10,7 +10,9 @@ "TaskCount": "1", "LogRetention": "30", "ContainerPort": "8080", - "EnvFileARN": "" + "EnvFileARN": "", + "TargetContainer": "simple-backend", + "TargetPort": "8080" }, "Tags": { "copilot-application": "my-app", diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-template.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-template.yml index 28dd81d3b68..64b4435034c 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-template.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-template.yml @@ -30,6 +30,10 @@ Parameters: LogRetention: Type: Number Default: 30 + TargetContainer: + Type: String + TargetPort: + Type: Number Conditions: HasAddons: !Not [!Equals [!Ref AddonsTemplateURL, ""]] HasEnvFile: !Not [!Equals [!Ref EnvFileARN, ""]] @@ -85,12 +89,7 @@ Resources: awslogs-region: !Ref AWS::Region awslogs-group: !Ref LogGroup awslogs-stream-prefix: copilot - PortMappings: - !If [ - ExposePort, - [{ ContainerPort: !Ref ContainerPort }], - !Ref "AWS::NoValue", - ] + PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort, Name: TargetPort}], !Ref "AWS::NoValue"] ExecutionRole: Metadata: "aws:copilot:description": "An IAM Role for the Fargate agent to make AWS API calls on your behalf" @@ -275,6 +274,22 @@ Resources: PropagateTags: SERVICE EnableExecuteCommand: true LaunchType: FARGATE + ServiceConnectConfiguration: + Enabled: True + Namespace: my-env.my-app.local + LogConfiguration: + LogDriver: awslogs + Options: + awslogs-region: !Ref AWS::Region + awslogs-group: !Ref LogGroup + awslogs-stream-prefix: copilot + Services: + - PortName: TargetPort + # Avoid using the same service with Service Discovery in a namespace. + DiscoveryName: !Join ["-", [!Ref WorkloadName, "sc"]] + ClientAliases: + - Port: !Ref TargetPort + DnsName: !Ref WorkloadName NetworkConfiguration: AwsvpcConfiguration: AssignPublicIp: ENABLED diff --git a/internal/pkg/deploy/cloudformation/stack/transformers.go b/internal/pkg/deploy/cloudformation/stack/transformers.go index b69966f058c..82917c74773 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers.go @@ -496,6 +496,18 @@ func convertExecuteCommand(e *manifest.ExecuteCommand) *template.ExecuteCommandO return &template.ExecuteCommandOpts{} } +func convertServiceConnect(s manifest.ServiceConnect) *template.ServiceConnect { + if s.EnableServiceConnect != nil && *s.EnableServiceConnect { + return &template.ServiceConnect{} + } + if !s.ServiceConnectArgs.IsEmpty() { + return &template.ServiceConnect{ + Alias: s.ServiceConnectArgs.Alias, + } + } + return nil +} + func convertLogging(lc manifest.Logging) *template.LogConfigOpts { if lc.IsEmpty() { return nil diff --git a/internal/pkg/deploy/cloudformation/stack/worker_svc.go b/internal/pkg/deploy/cloudformation/stack/worker_svc.go index 73abd19eef4..09b377980cd 100644 --- a/internal/pkg/deploy/cloudformation/stack/worker_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/worker_svc.go @@ -5,9 +5,10 @@ package stack import ( "fmt" - "github.com/aws/copilot-cli/internal/pkg/config" "strings" + "github.com/aws/copilot-cli/internal/pkg/config" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/cloudformation" "github.com/aws/copilot-cli/internal/pkg/manifest" diff --git a/internal/pkg/manifest/svc_test.go b/internal/pkg/manifest/svc_test.go index b0ddf5bc17a..1a19f0a3db9 100644 --- a/internal/pkg/manifest/svc_test.go +++ b/internal/pkg/manifest/svc_test.go @@ -41,6 +41,8 @@ cpu: 512 memory: 1024 count: 1 exec: true +network: + connect: true http: path: "svc" target_container: "frontend" @@ -161,6 +163,9 @@ environments: PlacementString: placementStringP(PublicSubnetPlacement), }, }, + Connect: ServiceConnect{ + EnableServiceConnect: aws.Bool(true), + }, }, TaskDefOverrides: []OverrideRule{ { diff --git a/internal/pkg/manifest/transform.go b/internal/pkg/manifest/transform.go index b28b294109b..de3b05b87f5 100644 --- a/internal/pkg/manifest/transform.go +++ b/internal/pkg/manifest/transform.go @@ -23,6 +23,7 @@ var defaultTransformers = []mergo.Transformers{ stringSliceOrStringTransformer{}, platformArgsOrStringTransformer{}, securityGroupsIDsOrConfigTransformer{}, + serviceConnectTransformer{}, placementArgOrStringTransformer{}, subnetListOrArgsTransformer{}, healthCheckArgsOrStringTransformer{}, @@ -238,6 +239,32 @@ func (t placementArgOrStringTransformer) Transformer(typ reflect.Type) func(dst, } } +type serviceConnectTransformer struct{} + +// Transformer returns custom merge logic for serviceConnectTransformer's fields. +func (t serviceConnectTransformer) Transformer(typ reflect.Type) func(dst, src reflect.Value) error { + if typ != reflect.TypeOf(ServiceConnect{}) { + return nil + } + + return func(dst, src reflect.Value) error { + dstStruct, srcStruct := dst.Interface().(ServiceConnect), src.Interface().(ServiceConnect) + + if srcStruct.EnableServiceConnect != nil { + dstStruct.ServiceConnectArgs = ServiceConnectArgs{} + } + + if !srcStruct.ServiceConnectArgs.IsEmpty() { + dstStruct.EnableServiceConnect = nil + } + + if dst.CanSet() { // For extra safety to prevent panicking. + dst.Set(reflect.ValueOf(dstStruct)) + } + return nil + } +} + type subnetListOrArgsTransformer struct{} // Transformer returns custom merge logic for subnetListOrArgsTransformer's fields. diff --git a/internal/pkg/manifest/transform_test.go b/internal/pkg/manifest/transform_test.go index 0286bfbeaa4..472158522cb 100644 --- a/internal/pkg/manifest/transform_test.go +++ b/internal/pkg/manifest/transform_test.go @@ -592,6 +592,64 @@ func TestSubnetListOrArgsTransformer_Transformer(t *testing.T) { } } +func TestServiceConnectTransformer_Transformer(t *testing.T) { + testCases := map[string]struct { + original func(p *ServiceConnect) + override func(p *ServiceConnect) + wanted func(p *ServiceConnect) + }{ + "bool set to empty if args is not nil": { + original: func(s *ServiceConnect) { + s.EnableServiceConnect = aws.Bool(false) + }, + override: func(s *ServiceConnect) { + s.ServiceConnectArgs = ServiceConnectArgs{ + Alias: aws.String("api"), + } + }, + wanted: func(s *ServiceConnect) { + s.ServiceConnectArgs = ServiceConnectArgs{ + Alias: aws.String("api"), + } + }, + }, + "args set to empty if bool is not nil": { + original: func(s *ServiceConnect) { + s.ServiceConnectArgs = ServiceConnectArgs{ + Alias: aws.String("api"), + } + }, + override: func(s *ServiceConnect) { + s.EnableServiceConnect = aws.Bool(true) + }, + wanted: func(s *ServiceConnect) { + s.EnableServiceConnect = aws.Bool(true) + }, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + var dst, override, wanted ServiceConnect + + tc.original(&dst) + tc.override(&override) + tc.wanted(&wanted) + + // Perform default merge. + err := mergo.Merge(&dst, override, mergo.WithOverride) + require.NoError(t, err) + + // Use custom transformer. + err = mergo.Merge(&dst, override, mergo.WithOverride, mergo.WithTransformers(serviceConnectTransformer{})) + require.NoError(t, err) + + require.NoError(t, err) + require.Equal(t, wanted, dst) + }) + } +} + func TestHealthCheckArgsOrStringTransformer_Transformer(t *testing.T) { testCases := map[string]struct { original func(h *HealthCheckArgsOrString) diff --git a/internal/pkg/manifest/validate.go b/internal/pkg/manifest/validate.go index 0c8d511b7f9..f6c41af7524 100644 --- a/internal/pkg/manifest/validate.go +++ b/internal/pkg/manifest/validate.go @@ -1245,6 +1245,19 @@ func (n NetworkConfig) validate() error { if err := n.VPC.validate(); err != nil { return fmt.Errorf(`validate "vpc": %w`, err) } + if err := n.Connect.validate(); err != nil { + return fmt.Errorf(`validate "connect": %w`, err) + } + return nil +} + +// validate returns nil if ServiceConnect is configured correctly. +func (s ServiceConnect) validate() error { + return s.ServiceConnectArgs.validate() +} + +// validate is a no-op for ServiceConnectArgs. +func (ServiceConnectArgs) validate() error { return nil } diff --git a/internal/pkg/manifest/validate_test.go b/internal/pkg/manifest/validate_test.go index 7a123d4f2b1..5faea6bb067 100644 --- a/internal/pkg/manifest/validate_test.go +++ b/internal/pkg/manifest/validate_test.go @@ -82,7 +82,7 @@ func TestLoadBalancedWebService_validate(t *testing.T) { LoadBalancedWebServiceConfig: LoadBalancedWebServiceConfig{ ImageConfig: testImageConfig, Network: NetworkConfig{ - vpcConfig{ + VPC: vpcConfig{ Placement: PlacementArgOrString{ PlacementString: (*PlacementString)(aws.String("")), }, @@ -409,7 +409,7 @@ func TestBackendService_validate(t *testing.T) { BackendServiceConfig: BackendServiceConfig{ ImageConfig: testImageConfig, Network: NetworkConfig{ - vpcConfig{ + VPC: vpcConfig{ Placement: PlacementArgOrString{ PlacementString: (*PlacementString)(aws.String("")), }, @@ -786,7 +786,7 @@ func TestWorkerService_validate(t *testing.T) { WorkerServiceConfig: WorkerServiceConfig{ ImageConfig: testImageConfig, Network: NetworkConfig{ - vpcConfig{ + VPC: vpcConfig{ Placement: PlacementArgOrString{ PlacementString: (*PlacementString)(aws.String("")), }, @@ -992,7 +992,7 @@ func TestScheduledJob_validate(t *testing.T) { ScheduledJobConfig: ScheduledJobConfig{ ImageConfig: testImageConfig, Network: NetworkConfig{ - vpcConfig{ + VPC: vpcConfig{ Placement: PlacementArgOrString{ PlacementString: (*PlacementString)(aws.String("")), }, diff --git a/internal/pkg/manifest/workload.go b/internal/pkg/manifest/workload.go index 2767a3a9066..683719df804 100644 --- a/internal/pkg/manifest/workload.go +++ b/internal/pkg/manifest/workload.go @@ -47,13 +47,14 @@ var ( var ( ErrAppRunnerInvalidPlatformWindows = errors.New("Windows is not supported for App Runner services") - errUnmarshalBuildOpts = errors.New("unable to unmarshal build field into string or compose-style map") - errUnmarshalPlatformOpts = errors.New("unable to unmarshal platform field into string or compose-style map") - errUnmarshalSecurityGroupOpts = errors.New(`unable to unmarshal "security_groups" field into slice of strings or compose-style map`) - errUnmarshalPlacementOpts = errors.New("unable to unmarshal placement field into string or compose-style map") - errUnmarshalSubnetsOpts = errors.New("unable to unmarshal subnets field into string slice or compose-style map") - errUnmarshalCountOpts = errors.New(`unable to unmarshal "count" field to an integer or autoscaling configuration`) - errUnmarshalRangeOpts = errors.New(`unable to unmarshal "range" field`) + errUnmarshalBuildOpts = errors.New("unable to unmarshal build field into string or compose-style map") + errUnmarshalPlatformOpts = errors.New("unable to unmarshal platform field into string or compose-style map") + errUnmarshalSecurityGroupOpts = errors.New(`unable to unmarshal "security_groups" field into slice of strings or compose-style map`) + errUnmarshalPlacementOpts = errors.New("unable to unmarshal placement field into string or compose-style map") + errUnmarshalServiceConnectOpts = errors.New(`unable to unmarshal "connect" field into boolean or compose-style map`) + errUnmarshalSubnetsOpts = errors.New("unable to unmarshal subnets field into string slice or compose-style map") + errUnmarshalCountOpts = errors.New(`unable to unmarshal "count" field to an integer or autoscaling configuration`) + errUnmarshalRangeOpts = errors.New(`unable to unmarshal "range" field`) errUnmarshalExec = errors.New(`unable to unmarshal "exec" field into boolean or exec configuration`) errUnmarshalEntryPoint = errors.New(`unable to unmarshal "entrypoint" into string or slice of strings`) @@ -458,7 +459,8 @@ func (t *FIFOTopicAdvanceConfigOrBool) UnmarshalYAML(value *yaml.Node) error { // NetworkConfig represents options for network connection to AWS resources within a VPC. type NetworkConfig struct { - VPC vpcConfig `yaml:"vpc"` + VPC vpcConfig `yaml:"vpc"` + Connect ServiceConnect `yaml:"connect"` } // IsEmpty returns empty if the struct has all zero members. @@ -473,6 +475,42 @@ func (c *NetworkConfig) requiredEnvFeatures() []string { return nil } +// ServiceConnect represents ECS Service Connect. +type ServiceConnect struct { + EnableServiceConnect *bool + ServiceConnectArgs +} + +// UnmarshalYAML overrides the default YAML unmarshaling logic for the ServiceConnect +// struct, allowing it to perform more complex unmarshaling behavior. +// This method implements the yaml.Unmarshaler (v3) interface. +func (s *ServiceConnect) UnmarshalYAML(value *yaml.Node) error { + if err := value.Decode(&s.ServiceConnectArgs); err != nil { + var yamlTypeErr *yaml.TypeError + if !errors.As(err, &yamlTypeErr) { + return err + } + } + if !s.ServiceConnectArgs.IsEmpty() { + s.EnableServiceConnect = nil + return nil + } + if err := value.Decode(&s.EnableServiceConnect); err != nil { + return errUnmarshalServiceConnectOpts + } + return nil +} + +// ServiceConnectArgs represents the configuration for ECS Service Connect. +type ServiceConnectArgs struct { + Alias *string +} + +// IsEmpty returns if ServiceConnectArgs is empty. +func (s *ServiceConnectArgs) IsEmpty() bool { + return s.Alias == nil +} + // PlacementArgOrString represents where to place tasks. type PlacementArgOrString struct { *PlacementString diff --git a/internal/pkg/manifest/workload_test.go b/internal/pkg/manifest/workload_test.go index bf18c7eb5cd..81376988dbb 100644 --- a/internal/pkg/manifest/workload_test.go +++ b/internal/pkg/manifest/workload_test.go @@ -354,6 +354,49 @@ func TestPlatformArgsOrString_UnmarshalYAML(t *testing.T) { } } +func TestServiceConnect_UnmarshalYAML(t *testing.T) { + testCases := map[string]struct { + inContent []byte + + wantedStruct ServiceConnect + wantedError error + }{ + "returns error if both bool and args specified": { + inContent: []byte(`connect: true + alias: api`), + + wantedError: errors.New("yaml: line 2: mapping values are not allowed in this context"), + }, + "error if unmarshalable": { + inContent: []byte(`connect: + ohess: linus + archie: leg64`), + wantedError: errUnmarshalServiceConnectOpts, + }, + "success": { + inContent: []byte(`connect: + alias: api`), + wantedStruct: ServiceConnect{ + ServiceConnectArgs: ServiceConnectArgs{ + Alias: aws.String("api"), + }, + }, + }, + } + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + v := NetworkConfig{} + err := yaml.Unmarshal(tc.inContent, &v) + if tc.wantedError != nil { + require.EqualError(t, err, tc.wantedError.Error()) + } else { + require.NoError(t, err) + require.Equal(t, tc.wantedStruct, v.Connect) + } + }) + } +} + func TestPlacementArgOrString_UnmarshalYAML(t *testing.T) { testCases := map[string]struct { inContent []byte diff --git a/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml b/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml index 03e9a27ec1d..7d6bbb9e5b9 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml @@ -36,7 +36,7 @@ CapacityProviderStrategy: {{- if .ServiceConnect }} ServiceConnectConfiguration: Enabled: True - Namespace: {{.ServiceConnect.Namespace}} + Namespace: {{.ServiceDiscoveryEndpoint}} LogConfiguration: LogDriver: awslogs Options: diff --git a/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml b/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml index 770df616c85..2d5d1dd7ea7 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml @@ -49,7 +49,7 @@ PortMappings: - ContainerPort: {{$sidecar.Port}} {{- if $.ServiceConnect }} # remove when release - {{- if eq $.HTTPTargetContainer.Container $sidecar.Name}} + {{- if eq $.HTTPTargetContainer.Name $sidecar.Name}} {{- if eq $.HTTPTargetContainer.Port $sidecar.Port}} Name: TargetPort {{- end}} diff --git a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml index 9845a28f0cb..e3c18498419 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml @@ -30,7 +30,7 @@ PortMappings: - ContainerPort: !Ref ContainerPort {{- if .ServiceConnect }} # remove when release - {{- if eq .HTTPTargetContainer.Container .WorkloadName}} + {{- if eq .HTTPTargetContainer.Name .WorkloadName}} Name: TargetPort {{- end}} {{- end}} @@ -43,8 +43,8 @@ {{- end}} {{- end}} {{/* end if eq .WorkloadType "Load Balanced Web Service"*/}} {{- if eq .WorkloadType "Backend Service"}} -{{- if .ServiceConnect }} # remove when release - PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort, Name: !Ref WorkloadName}], !Ref "AWS::NoValue"] +{{- if and .ServiceConnect (eq .HTTPTargetContainer.Name .WorkloadName)}} # remove when release + PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort, Name: TargetPort}], !Ref "AWS::NoValue"] {{- else}} PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort}], !Ref "AWS::NoValue"] {{- end}} diff --git a/internal/pkg/template/templates/workloads/services/backend/cf.yml b/internal/pkg/template/templates/workloads/services/backend/cf.yml index 84e43d939c6..d7bcee767c6 100644 --- a/internal/pkg/template/templates/workloads/services/backend/cf.yml +++ b/internal/pkg/template/templates/workloads/services/backend/cf.yml @@ -35,11 +35,11 @@ Parameters: LogRetention: Type: Number Default: 30 - {{- if .ALBEnabled}} TargetContainer: Type: String TargetPort: Type: Number + {{- if .ALBEnabled}} HTTPSEnabled: Type: String AllowedValues: [true, false] diff --git a/internal/pkg/template/workload.go b/internal/pkg/template/workload.go index 21a0041b02f..f21824a5a71 100644 --- a/internal/pkg/template/workload.go +++ b/internal/pkg/template/workload.go @@ -210,8 +210,8 @@ type LogConfigOpts struct { // HTTPTargetContainer represents the target group of a load balancer that points to a container. type HTTPTargetContainer struct { - Container string - Port string // Port of the container. + Name string + Port string } // IsHTTPS returns true if the target container's port is 443. @@ -325,8 +325,7 @@ type NetworkLoadBalancer struct { // ServiceConnect holds configuration for ECS Service Connect. type ServiceConnect struct { - Namespace string - Alias *string + Alias *string } // AdvancedCount holds configuration for autoscaling and capacity provider From 9d478715e6eb70ecd0d6c5754d5cfba4f1c5fc5d Mon Sep 17 00:00:00 2001 From: penghaoh Date: Thu, 29 Sep 2022 11:34:20 -0700 Subject: [PATCH 02/12] Add validation for backend with no ports --- internal/pkg/manifest/validate.go | 23 ++++++++++-- internal/pkg/manifest/validate_test.go | 35 +++++++++++++++++-- internal/pkg/manifest/workload.go | 7 ++++ .../workloads/partials/cf/sidecars.yml | 2 -- 4 files changed, 61 insertions(+), 6 deletions(-) diff --git a/internal/pkg/manifest/validate.go b/internal/pkg/manifest/validate.go index f6c41af7524..01f12f7e820 100644 --- a/internal/pkg/manifest/validate.go +++ b/internal/pkg/manifest/validate.go @@ -78,6 +78,7 @@ func (l LoadBalancedWebService) validate() error { } if err = validateTargetContainer(validateTargetContainerOpts{ mainContainerName: aws.StringValue(l.Name), + mainContainerPort: l.ImageConfig.Port, targetContainer: l.RoutingRule.GetTargetContainer(), sidecarConfig: l.Sidecars, }); err != nil { @@ -85,6 +86,7 @@ func (l LoadBalancedWebService) validate() error { } if err = validateTargetContainer(validateTargetContainerOpts{ mainContainerName: aws.StringValue(l.Name), + mainContainerPort: l.ImageConfig.Port, targetContainer: l.NLBConfig.TargetContainer, sidecarConfig: l.Sidecars, }); err != nil { @@ -193,6 +195,19 @@ func (b BackendService) validate() error { if err = b.Workload.validate(); err != nil { return err } + if err = validateTargetContainer(validateTargetContainerOpts{ + mainContainerName: aws.StringValue(b.Name), + mainContainerPort: b.ImageConfig.Port, + targetContainer: b.RoutingRule.GetTargetContainer(), + sidecarConfig: b.Sidecars, + }); err != nil { + return fmt.Errorf("validate HTTP load balancer target: %w", err) + } + if b.BackendServiceConfig.Network.Connect.enabled() { + if b.RoutingRule.GetTargetContainer() == nil && b.ImageConfig.Port == nil { + return fmt.Errorf(`cannot enable "network.connect" when no port exposed`) + } + } if err = validateContainerDeps(validateDependenciesOpts{ sidecarConfig: b.Sidecars, imageConfig: b.ImageConfig.Image, @@ -1603,6 +1618,7 @@ type containerDependency struct { type validateTargetContainerOpts struct { mainContainerName string + mainContainerPort *uint16 targetContainer *string sidecarConfig map[string]*SidecarConfig } @@ -1622,14 +1638,17 @@ func validateTargetContainer(opts validateTargetContainerOpts) error { } targetContainer := aws.StringValue(opts.targetContainer) if targetContainer == opts.mainContainerName { + if opts.mainContainerPort == nil { + return fmt.Errorf("target container %q doesn't expose any port", targetContainer) + } return nil } sidecar, ok := opts.sidecarConfig[targetContainer] if !ok { - return fmt.Errorf("target container %s doesn't exist", targetContainer) + return fmt.Errorf("target container %q doesn't exist", targetContainer) } if sidecar.Port == nil { - return fmt.Errorf("target container %s doesn't expose any port", targetContainer) + return fmt.Errorf("target container %q doesn't expose any port", targetContainer) } return nil } diff --git a/internal/pkg/manifest/validate_test.go b/internal/pkg/manifest/validate_test.go index 5faea6bb067..cde3928432d 100644 --- a/internal/pkg/manifest/validate_test.go +++ b/internal/pkg/manifest/validate_test.go @@ -580,6 +580,37 @@ func TestBackendService_validate(t *testing.T) { }, wantedErrorMsgPrefix: `validate "publish": `, }, + "error if target container not found": { + config: BackendService{ + BackendServiceConfig: BackendServiceConfig{ + ImageConfig: testImageConfig, + RoutingRule: RoutingRuleConfiguration{ + TargetContainer: aws.String("api"), + Path: aws.String("/"), + }, + }, + Workload: Workload{ + Name: aws.String("api"), + }, + }, + wantedError: fmt.Errorf(`validate HTTP load balancer target: target container "api" doesn't expose any port`), + }, + "error if service connect is enabled without any port exposed": { + config: BackendService{ + BackendServiceConfig: BackendServiceConfig{ + ImageConfig: testImageConfig, + Network: NetworkConfig{ + Connect: ServiceConnect{ + EnableServiceConnect: aws.Bool(true), + }, + }, + }, + Workload: Workload{ + Name: aws.String("api"), + }, + }, + wantedError: fmt.Errorf(`cannot enable "network.connect" when no port exposed`), + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { @@ -3033,7 +3064,7 @@ func TestValidateLoadBalancerTarget(t *testing.T) { mainContainerName: "mockMainContainer", targetContainer: aws.String("foo"), }, - wanted: fmt.Errorf("target container foo doesn't exist"), + wanted: fmt.Errorf(`target container "foo" doesn't exist`), }, "should return an error if target container doesn't expose any port": { in: validateTargetContainerOpts{ @@ -3043,7 +3074,7 @@ func TestValidateLoadBalancerTarget(t *testing.T) { "foo": {}, }, }, - wanted: fmt.Errorf("target container foo doesn't expose any port"), + wanted: fmt.Errorf(`target container "foo" doesn't expose any port`), }, "success with no target container set": { in: validateTargetContainerOpts{ diff --git a/internal/pkg/manifest/workload.go b/internal/pkg/manifest/workload.go index 683719df804..c3b7163db91 100644 --- a/internal/pkg/manifest/workload.go +++ b/internal/pkg/manifest/workload.go @@ -481,6 +481,13 @@ type ServiceConnect struct { ServiceConnectArgs } +func (s *ServiceConnect) enabled() bool { + if s.EnableServiceConnect != nil && *s.EnableServiceConnect { + return true + } + return !s.ServiceConnectArgs.IsEmpty() +} + // UnmarshalYAML overrides the default YAML unmarshaling logic for the ServiceConnect // struct, allowing it to perform more complex unmarshaling behavior. // This method implements the yaml.Unmarshaler (v3) interface. diff --git a/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml b/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml index 2d5d1dd7ea7..b3aba5439b6 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml @@ -50,11 +50,9 @@ - ContainerPort: {{$sidecar.Port}} {{- if $.ServiceConnect }} # remove when release {{- if eq $.HTTPTargetContainer.Name $sidecar.Name}} - {{- if eq $.HTTPTargetContainer.Port $sidecar.Port}} Name: TargetPort {{- end}} {{- end}} - {{- end}} {{- if $sidecar.Protocol}} Protocol: {{$sidecar.Protocol}} {{- end}} From b3e242af01124cb706cc70bde583a04d7db941fb Mon Sep 17 00:00:00 2001 From: penghaoh Date: Thu, 29 Sep 2022 11:52:13 -0700 Subject: [PATCH 03/12] Fix target port name (no cap) --- .../stack/testdata/workloads/backend/simple-template.yml | 4 ++-- .../workloads/partials/cf/service-base-properties.yml | 2 +- .../pkg/template/templates/workloads/partials/cf/sidecars.yml | 2 +- .../templates/workloads/partials/cf/workload-container.yml | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-template.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-template.yml index 64b4435034c..02f27f0ffa6 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-template.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-template.yml @@ -89,7 +89,7 @@ Resources: awslogs-region: !Ref AWS::Region awslogs-group: !Ref LogGroup awslogs-stream-prefix: copilot - PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort, Name: TargetPort}], !Ref "AWS::NoValue"] + PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort, Name: target}], !Ref "AWS::NoValue"] ExecutionRole: Metadata: "aws:copilot:description": "An IAM Role for the Fargate agent to make AWS API calls on your behalf" @@ -284,7 +284,7 @@ Resources: awslogs-group: !Ref LogGroup awslogs-stream-prefix: copilot Services: - - PortName: TargetPort + - PortName: target # Avoid using the same service with Service Discovery in a namespace. DiscoveryName: !Join ["-", [!Ref WorkloadName, "sc"]] ClientAliases: diff --git a/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml b/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml index 7d6bbb9e5b9..f4ef5748d0b 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml @@ -44,7 +44,7 @@ ServiceConnectConfiguration: awslogs-group: !Ref LogGroup awslogs-stream-prefix: copilot Services: - - PortName: TargetPort + - PortName: target # Avoid using the same service with Service Discovery in a namespace. DiscoveryName: !Join ["-", [!Ref WorkloadName, "sc"]] ClientAliases: diff --git a/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml b/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml index b3aba5439b6..3da105c03cb 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml @@ -50,7 +50,7 @@ - ContainerPort: {{$sidecar.Port}} {{- if $.ServiceConnect }} # remove when release {{- if eq $.HTTPTargetContainer.Name $sidecar.Name}} - Name: TargetPort + Name: target {{- end}} {{- end}} {{- if $sidecar.Protocol}} diff --git a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml index e3c18498419..321e848c8ed 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml @@ -31,7 +31,7 @@ - ContainerPort: !Ref ContainerPort {{- if .ServiceConnect }} # remove when release {{- if eq .HTTPTargetContainer.Name .WorkloadName}} - Name: TargetPort + Name: target {{- end}} {{- end}} {{- if .NLB}} {{ $nlbListener := .NLB.Listener }} @@ -44,7 +44,7 @@ {{- end}} {{/* end if eq .WorkloadType "Load Balanced Web Service"*/}} {{- if eq .WorkloadType "Backend Service"}} {{- if and .ServiceConnect (eq .HTTPTargetContainer.Name .WorkloadName)}} # remove when release - PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort, Name: TargetPort}], !Ref "AWS::NoValue"] + PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort, Name: target}], !Ref "AWS::NoValue"] {{- else}} PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort}], !Ref "AWS::NoValue"] {{- end}} From 75860fb78e0eaa50afa68db45353bb98fc3374f3 Mon Sep 17 00:00:00 2001 From: penghaoh Date: Thu, 29 Sep 2022 14:19:52 -0700 Subject: [PATCH 04/12] Add fb --- .../stack/testdata/workloads/backend/simple-params.json | 4 ++-- internal/pkg/deploy/cloudformation/stack/transformers.go | 2 +- internal/pkg/manifest/workload.go | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-params.json b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-params.json index a9cd4be2b8d..b323326f94d 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-params.json +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-params.json @@ -11,8 +11,8 @@ "LogRetention": "30", "ContainerPort": "8080", "EnvFileARN": "", - "TargetContainer": "simple-backend", - "TargetPort": "8080" + "TargetContainer": "simple-backend", + "TargetPort": "8080" }, "Tags": { "copilot-application": "my-app", diff --git a/internal/pkg/deploy/cloudformation/stack/transformers.go b/internal/pkg/deploy/cloudformation/stack/transformers.go index 82917c74773..1dc44e52b1e 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers.go @@ -497,7 +497,7 @@ func convertExecuteCommand(e *manifest.ExecuteCommand) *template.ExecuteCommandO } func convertServiceConnect(s manifest.ServiceConnect) *template.ServiceConnect { - if s.EnableServiceConnect != nil && *s.EnableServiceConnect { + if aws.BoolValue(s.EnableServiceConnect) { return &template.ServiceConnect{} } if !s.ServiceConnectArgs.IsEmpty() { diff --git a/internal/pkg/manifest/workload.go b/internal/pkg/manifest/workload.go index c3b7163db91..f94d4d8a1e5 100644 --- a/internal/pkg/manifest/workload.go +++ b/internal/pkg/manifest/workload.go @@ -475,14 +475,14 @@ func (c *NetworkConfig) requiredEnvFeatures() []string { return nil } -// ServiceConnect represents ECS Service Connect. +// ServiceConnect represents ECS Service Connect configuration. type ServiceConnect struct { EnableServiceConnect *bool ServiceConnectArgs } func (s *ServiceConnect) enabled() bool { - if s.EnableServiceConnect != nil && *s.EnableServiceConnect { + if aws.BoolValue(s.EnableServiceConnect) { return true } return !s.ServiceConnectArgs.IsEmpty() @@ -508,12 +508,12 @@ func (s *ServiceConnect) UnmarshalYAML(value *yaml.Node) error { return nil } -// ServiceConnectArgs represents the configuration for ECS Service Connect. +// ServiceConnectArgs includes the advanced configuration for ECS Service Connect. type ServiceConnectArgs struct { Alias *string } -// IsEmpty returns if ServiceConnectArgs is empty. +// IsEmpty returns empty if the struct has all zero members. func (s *ServiceConnectArgs) IsEmpty() bool { return s.Alias == nil } From 250acbb2f1db97e45800caaab1c937c15a0813d6 Mon Sep 17 00:00:00 2001 From: penghaoh Date: Thu, 29 Sep 2022 15:58:12 -0700 Subject: [PATCH 05/12] Make better default value --- .../cloudformation/stack/backend_svc.go | 7 +- .../cloudformation/stack/backend_svc_test.go | 4 +- .../deploy/cloudformation/stack/lb_web_svc.go | 6 +- .../stack/testdata/stacklocal/manifest.yml | 2 + .../backend/http-autoscaling-manifest.yml | 3 + .../backend/http-full-config-manifest.yml | 3 + .../backend/http-only-path-manifest.yml | 3 + .../backend/https-path-alias-manifest.yml | 3 + .../workloads/backend/simple-manifest.yml | 2 - .../testdata/workloads/svc-grpc-manifest.yml | 3 + .../stack/testdata/workloads/svc-manifest.yml | 11 ++- .../testdata/workloads/svc-nlb-manifest.yml | 2 + .../testdata/workloads/svc-prod.params.json | 4 +- .../testdata/workloads/svc-prod.stack.yml | 29 +++++-- .../testdata/workloads/svc-test.stack.yml | 17 ++++ .../workloads/windows-svc-manifest.yml | 3 + .../cloudformation/stack/transformers.go | 12 +-- .../cloudformation/stack/transformers_test.go | 20 ++--- internal/pkg/manifest/backend_svc.go | 16 ++++ internal/pkg/manifest/backend_svc_test.go | 77 +++++++++++++++++++ internal/pkg/manifest/lb_web_svc.go | 8 ++ internal/pkg/manifest/lb_web_svc_test.go | 35 +++++++++ internal/pkg/manifest/transform.go | 2 +- internal/pkg/manifest/validate.go | 2 +- internal/pkg/manifest/workload.go | 12 +-- internal/pkg/template/workload.go | 2 +- 26 files changed, 241 insertions(+), 47 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/backend_svc.go b/internal/pkg/deploy/cloudformation/stack/backend_svc.go index f21b24087d3..25ab03b2d80 100644 --- a/internal/pkg/deploy/cloudformation/stack/backend_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/backend_svc.go @@ -141,7 +141,10 @@ func (s *BackendService) Template() (string, error) { for _, ipNet := range s.manifest.RoutingRule.AllowedSourceIps { allowedSourceIPs = append(allowedSourceIPs, string(ipNet)) } - + var scConfig *template.ServiceConnect + if s.manifest.ServiceConnectEnabled() { + scConfig = convertServiceConnect(s.manifest.Network.Connect) + } targetContainer, targetContainerPort := s.httpLoadBalancerTarget() content, err := s.parser.ParseBackendService(template.WorkloadOpts{ AppName: s.app, @@ -168,7 +171,7 @@ func (s *BackendService) Template() (string, error) { Port: aws.StringValue(targetContainerPort), Name: aws.StringValue(targetContainer), }, - ServiceConnect: convertServiceConnect(s.manifest.Network.Connect), + ServiceConnect: scConfig, HTTPHealthCheck: convertHTTPHealthCheck(&s.manifest.RoutingRule.HealthCheck), DeregistrationDelay: deregistrationDelay, AllowedSourceIps: allowedSourceIPs, diff --git a/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go b/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go index 67889cd6765..d0b06e82b42 100644 --- a/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go +++ b/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go @@ -288,6 +288,7 @@ Outputs: Key: "sha2/count.zip", }, }, + ServiceConnect: &template.ServiceConnect{}, ExecuteCommand: &template.ExecuteCommandOpts{}, NestedStack: &template.WorkloadNestedStackOpts{ StackName: addon.StackName, @@ -431,7 +432,7 @@ Outputs: }, Sidecars: []*template.SidecarOpts{ { - Name: aws.String("envoy"), + Name: "envoy", Port: aws.String("443"), }, }, @@ -439,6 +440,7 @@ Outputs: Name: "envoy", Port: "443", }, + ServiceConnect: &template.ServiceConnect{}, HTTPHealthCheck: template.HTTPHealthCheckOpts{ HealthCheckPath: "/healthz", Port: "4200", diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go index a4e40a418e9..be3468ff3d2 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go @@ -190,6 +190,10 @@ func (s *LoadBalancedWebService) Template() (string, error) { if s.manifest.RoutingRule.RedirectToHTTPS != nil { httpRedirect = aws.BoolValue(s.manifest.RoutingRule.RedirectToHTTPS) } + var scConfig *template.ServiceConnect + if s.manifest.ServiceConnectEnabled() { + scConfig = convertServiceConnect(s.manifest.Network.Connect) + } targetContainer, targetContainerPort := s.httpLoadBalancerTarget() content, err := s.parser.ParseLoadBalancedWebService(template.WorkloadOpts{ AppName: s.app, @@ -217,7 +221,7 @@ func (s *LoadBalancedWebService) Template() (string, error) { Port: aws.StringValue(targetContainerPort), Name: aws.StringValue(targetContainer), }, - ServiceConnect: convertServiceConnect(s.manifest.Network.Connect), + ServiceConnect: scConfig, HealthCheck: convertContainerHealthCheck(s.manifest.ImageConfig.HealthCheck), HTTPHealthCheck: convertHTTPHealthCheck(&s.manifest.RoutingRule.HealthCheck), DeregistrationDelay: deregistrationDelay, diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/manifest.yml index bb95b6f1bb2..be2565b4a81 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/stacklocal/manifest.yml @@ -31,6 +31,8 @@ count: memory_percentage: 80 # Enable running commands in your container. exec: true +network: + connect: false taskdef_overrides: - path: "ContainerDefinitions[0].Ulimits[-].Name" diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-autoscaling-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-autoscaling-manifest.yml index 0547c19f8f7..f77c95217ce 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-autoscaling-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-autoscaling-manifest.yml @@ -10,6 +10,9 @@ image: # Port exposed through your container to route traffic to it. port: 8080 +network: + connect: false + cpu: 512 # Number of CPU units for the task. memory: 1024 # Amount of memory in MiB used by the task. exec: true # Enable running commands in your container. diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-full-config-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-full-config-manifest.yml index e856a9f9207..409382394aa 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-full-config-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-full-config-manifest.yml @@ -16,6 +16,9 @@ http: timeout: 10s grace_period: 45s +network: + connect: false + image: # Docker build arguments. For additional overrides: https://aws.github.io/copilot-cli/docs/manifest/backend-service/#image-build build: Dockerfile diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-only-path-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-only-path-manifest.yml index 677fb876bd7..6a0fd564f78 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-only-path-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/http-only-path-manifest.yml @@ -10,6 +10,9 @@ image: # Port exposed through your container to route traffic to it. port: 8080 +network: + connect: false + cpu: 512 # Number of CPU units for the task. memory: 1024 # Amount of memory in MiB used by the task. count: 1 # Number of tasks that should be running in your service. diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/https-path-alias-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/https-path-alias-manifest.yml index 91e06831d51..cb80c24ceef 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/https-path-alias-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/https-path-alias-manifest.yml @@ -10,6 +10,9 @@ http: - name: "*.foobar.com" hosted_zone: mockHostedZone1 +network: + connect: false + image: # Docker build arguments. For additional overrides: https://aws.github.io/copilot-cli/docs/manifest/backend-service/#image-build build: Dockerfile diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-manifest.yml index 53fdabea06a..54419ac29a7 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/backend/simple-manifest.yml @@ -4,8 +4,6 @@ image: build: Dockerfile port: 8080 -network: - connect: true cpu: 256 memory: 512 count: 1 diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-grpc-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-grpc-manifest.yml index 32ecf794a21..5478c9d5504 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-grpc-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-grpc-manifest.yml @@ -37,6 +37,9 @@ publish: topics: - name: givesdogs +network: + connect: false + # Optional fields for more advanced use-cases. # variables: # Pass environment variables as key value pairs. diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-manifest.yml index 87c8d6dc759..07ddad01086 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-manifest.yml @@ -60,6 +60,8 @@ environments: path: / grace_period: 30s deregistration_delay: 30s + network: + connect: false prod: count: range: @@ -80,9 +82,16 @@ environments: TEST: TEST secrets: GITHUB_TOKEN: GITHUB_TOKEN + http: + path: '/' + alias: example.com + target_container: nginx + network: + connect: + alias: api sidecars: nginx: - port: 80 + port: 8080 image: 1234567890.dkr.ecr.us-west-2.amazonaws.com/reverse-proxy:revision_1 variables: NGINX_PORT: 80 diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-manifest.yml index 2c2e8e5a404..3f461a5ea32 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-manifest.yml @@ -10,6 +10,8 @@ image: build: ./Dockerfile port: 80 http: false +network: + connect: false nlb: port: 443/tls count: 5 diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.params.json b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.params.json index 024d53b78ff..43bbeac60c7 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.params.json +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.params.json @@ -11,8 +11,8 @@ "LogRetention": "1", "ContainerPort": "4000", "DNSDelegated": "false", - "TargetContainer": "fe", - "TargetPort": "4000", + "TargetContainer": "nginx", + "TargetPort": "8080", "EnvFileARN": "", "RulePath": "/", "HTTPSEnabled": "true", diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.stack.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.stack.yml index 2b66f628900..bee900fd244 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.stack.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.stack.yml @@ -143,7 +143,8 @@ Resources: # If a bucket URL is specified, that means the template exists. - Name: nginx Image: 1234567890.dkr.ecr.us-west-2.amazonaws.com/reverse-proxy:revision_1 PortMappings: - - ContainerPort: 80 + - ContainerPort: 8080 + Name: target HealthCheck: Command: ["CMD-SHELL", "curl -f http://localhost:8080 || exit 1"] Interval: 10 @@ -190,11 +191,11 @@ Resources: # If a bucket URL is specified, that means the template exists. - Name: COPILOT_LB_DNS Value: !GetAtt EnvControllerAction.PublicLoadBalancerDNSName LogConfiguration: - LogDriver: awslogs - Options: - awslogs-region: !Ref AWS::Region - awslogs-group: !Ref LogGroup - awslogs-stream-prefix: copilot + LogDriver: awslogs + Options: + awslogs-region: !Ref AWS::Region + awslogs-group: !Ref LogGroup + awslogs-stream-prefix: copilot Volumes: - Name: persistence ExecutionRole: @@ -518,6 +519,22 @@ Resources: # If a bucket URL is specified, that means the template exists. - CapacityProvider: FARGATE Weight: 0 Base: 5 + ServiceConnectConfiguration: + Enabled: True + Namespace: prod.my-app.local + LogConfiguration: + LogDriver: awslogs + Options: + awslogs-region: !Ref AWS::Region + awslogs-group: !Ref LogGroup + awslogs-stream-prefix: copilot + Services: + - PortName: target + # Avoid using the same service with Service Discovery in a namespace. + DiscoveryName: !Join ["-", [!Ref WorkloadName, "sc"]] + ClientAliases: + - Port: !Ref TargetPort + DnsName: api NetworkConfiguration: AwsvpcConfiguration: AssignPublicIp: ENABLED diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-test.stack.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-test.stack.yml index 34ee38df6e6..448025a9698 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-test.stack.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-test.stack.yml @@ -109,6 +109,7 @@ Resources: # If a bucket URL is specified, that means the template exists. SourceVolume: persistence PortMappings: - ContainerPort: !Ref ContainerPort + Name: target Volumes: - Name: persistence ExecutionRole: @@ -430,6 +431,22 @@ Resources: # If a bucket URL is specified, that means the template exists. MaximumPercent: 200 PropagateTags: SERVICE LaunchType: FARGATE + ServiceConnectConfiguration: + Enabled: True + Namespace: test.my-app.local + LogConfiguration: + LogDriver: awslogs + Options: + awslogs-region: !Ref AWS::Region + awslogs-group: !Ref LogGroup + awslogs-stream-prefix: copilot + Services: + - PortName: target + # Avoid using the same service with Service Discovery in a namespace. + DiscoveryName: !Join ["-", [!Ref WorkloadName, "sc"]] + ClientAliases: + - Port: !Ref TargetPort + DnsName: !Ref WorkloadName NetworkConfiguration: AwsvpcConfiguration: AssignPublicIp: ENABLED diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/windows-svc-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/windows-svc-manifest.yml index 8b8638ab2e9..2138052f58a 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/windows-svc-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/windows-svc-manifest.yml @@ -21,6 +21,9 @@ image: # Port exposed through your container to route traffic to it. port: 80 +network: + connect: false + cpu: 1024 # Number of CPU units for the task. memory: 2048 # Amount of memory in MiB used by the task. count: 1 # Number of tasks that should be running in your service. diff --git a/internal/pkg/deploy/cloudformation/stack/transformers.go b/internal/pkg/deploy/cloudformation/stack/transformers.go index 1dc44e52b1e..ea302c02c22 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers.go @@ -96,7 +96,7 @@ func convertSidecar(s map[string]*manifest.SidecarConfig) ([]*template.SidecarOp } mp := convertSidecarMountPoints(config.MountPoints) sidecars = append(sidecars, &template.SidecarOpts{ - Name: aws.String(name), + Name: name, Image: config.Image, Essential: config.Essential, Port: port, @@ -497,15 +497,9 @@ func convertExecuteCommand(e *manifest.ExecuteCommand) *template.ExecuteCommandO } func convertServiceConnect(s manifest.ServiceConnect) *template.ServiceConnect { - if aws.BoolValue(s.EnableServiceConnect) { - return &template.ServiceConnect{} + return &template.ServiceConnect{ + Alias: s.ServiceConnectArgs.Alias, } - if !s.ServiceConnectArgs.IsEmpty() { - return &template.ServiceConnect{ - Alias: s.ServiceConnectArgs.Alias, - } - } - return nil } func convertLogging(lc manifest.Logging) *template.LogConfigOpts { diff --git a/internal/pkg/deploy/cloudformation/stack/transformers_test.go b/internal/pkg/deploy/cloudformation/stack/transformers_test.go index 37ac078239d..9baaca2bce0 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers_test.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers_test.go @@ -44,7 +44,7 @@ func Test_convertSidecar(t *testing.T) { inEssential: true, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", Port: aws.String("2000"), CredsParam: mockCredsParam, Image: mockImage, @@ -58,7 +58,7 @@ func Test_convertSidecar(t *testing.T) { inEssential: true, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", Port: aws.String("2000"), Protocol: aws.String("udp"), CredsParam: mockCredsParam, @@ -76,7 +76,7 @@ func Test_convertSidecar(t *testing.T) { }, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", Port: aws.String("2000"), CredsParam: mockCredsParam, Image: mockImage, @@ -96,7 +96,7 @@ func Test_convertSidecar(t *testing.T) { }, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", Port: aws.String("2000"), CredsParam: mockCredsParam, Image: mockImage, @@ -110,7 +110,7 @@ func Test_convertSidecar(t *testing.T) { }, "do not specify image override": { wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", CredsParam: mockCredsParam, Image: mockImage, Secrets: mockSecrets, @@ -126,7 +126,7 @@ func Test_convertSidecar(t *testing.T) { }, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", CredsParam: mockCredsParam, Image: mockImage, Secrets: mockSecrets, @@ -142,7 +142,7 @@ func Test_convertSidecar(t *testing.T) { }, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", CredsParam: mockCredsParam, Image: mockImage, Secrets: mockSecrets, @@ -158,7 +158,7 @@ func Test_convertSidecar(t *testing.T) { }, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", CredsParam: mockCredsParam, Image: mockImage, Secrets: mockSecrets, @@ -174,7 +174,7 @@ func Test_convertSidecar(t *testing.T) { }, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", CredsParam: mockCredsParam, Image: mockImage, Secrets: mockSecrets, @@ -190,7 +190,7 @@ func Test_convertSidecar(t *testing.T) { }, wanted: &template.SidecarOpts{ - Name: aws.String("foo"), + Name: "foo", CredsParam: mockCredsParam, Image: mockImage, Secrets: mockSecrets, diff --git a/internal/pkg/manifest/backend_svc.go b/internal/pkg/manifest/backend_svc.go index f79f9243db3..dc01b383c4f 100644 --- a/internal/pkg/manifest/backend_svc.go +++ b/internal/pkg/manifest/backend_svc.go @@ -98,6 +98,22 @@ func (s *BackendService) Port() (port uint16, ok bool) { return aws.Uint16Value(value), true } +// ServiceConnectEnabled returns if ServiceConnect is enabled or not. +func (s *BackendService) ServiceConnectEnabled() bool { + if s.Network.Connect.EnableServiceConnect != nil { + return *s.Network.Connect.EnableServiceConnect + } + if !s.Network.Connect.ServiceConnectArgs.isEmpty() { + return true + } + // Try our best to enable Service Connect by default. + if s.BackendServiceConfig.ImageConfig.Port != nil || + s.BackendServiceConfig.RoutingRule.TargetContainer != nil { + return true + } + return false +} + // Publish returns the list of topics where notifications can be published. func (s *BackendService) Publish() []Topic { return s.BackendServiceConfig.PublishConfig.publishedTopics() diff --git a/internal/pkg/manifest/backend_svc_test.go b/internal/pkg/manifest/backend_svc_test.go index aaa883bd831..fff85ecf941 100644 --- a/internal/pkg/manifest/backend_svc_test.go +++ b/internal/pkg/manifest/backend_svc_test.go @@ -297,6 +297,83 @@ func TestBackendService_Port(t *testing.T) { } } +func TestBackendService_ServiceConnectEnabled(t *testing.T) { + testCases := map[string]struct { + mft *BackendService + + wanted bool + }{ + "enabled by default if main container exposes port": { + mft: &BackendService{ + BackendServiceConfig: BackendServiceConfig{ + ImageConfig: ImageWithHealthcheckAndOptionalPort{ + ImageWithOptionalPort: ImageWithOptionalPort{ + Port: uint16P(80), + }, + }, + }, + }, + wanted: true, + }, + "enabled by default if target container is set": { + mft: &BackendService{ + BackendServiceConfig: BackendServiceConfig{ + RoutingRule: RoutingRuleConfiguration{ + TargetContainer: aws.String("nginx"), + }, + }, + }, + wanted: true, + }, + "disabled by default if no exposed port or no target container": { + mft: &BackendService{ + BackendServiceConfig: BackendServiceConfig{ + Network: NetworkConfig{ + Connect: ServiceConnect{}, + }, + }, + }, + wanted: false, + }, + "set by bool": { + mft: &BackendService{ + BackendServiceConfig: BackendServiceConfig{ + Network: NetworkConfig{ + Connect: ServiceConnect{ + EnableServiceConnect: aws.Bool(true), + }, + }, + }, + }, + wanted: true, + }, + "set by args": { + mft: &BackendService{ + BackendServiceConfig: BackendServiceConfig{ + Network: NetworkConfig{ + Connect: ServiceConnect{ + ServiceConnectArgs: ServiceConnectArgs{ + Alias: aws.String("api"), + }, + }, + }, + }, + }, + wanted: true, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + // WHEN + enabled := tc.mft.ServiceConnectEnabled() + + // THEN + require.Equal(t, tc.wanted, enabled) + }) + } +} + func TestBackendService_Publish(t *testing.T) { testCases := map[string]struct { mft *BackendService diff --git a/internal/pkg/manifest/lb_web_svc.go b/internal/pkg/manifest/lb_web_svc.go index 7d9b23ecf26..41afeca19cd 100644 --- a/internal/pkg/manifest/lb_web_svc.go +++ b/internal/pkg/manifest/lb_web_svc.go @@ -169,6 +169,14 @@ func (s *LoadBalancedWebService) Port() (port uint16, ok bool) { return aws.Uint16Value(s.ImageConfig.Port), true } +// ServiceConnectEnabled returns if ServiceConnect is enabled or not. +func (s *LoadBalancedWebService) ServiceConnectEnabled() bool { + if s.Network.Connect.EnableServiceConnect != nil && !*s.Network.Connect.EnableServiceConnect { + return false + } + return true +} + // Publish returns the list of topics where notifications can be published. func (s *LoadBalancedWebService) Publish() []Topic { return s.LoadBalancedWebServiceConfig.PublishConfig.publishedTopics() diff --git a/internal/pkg/manifest/lb_web_svc_test.go b/internal/pkg/manifest/lb_web_svc_test.go index 27ad87879bf..89eed93ae13 100644 --- a/internal/pkg/manifest/lb_web_svc_test.go +++ b/internal/pkg/manifest/lb_web_svc_test.go @@ -1338,6 +1338,41 @@ func TestLoadBalancedWebService_Port(t *testing.T) { require.Equal(t, uint16(80), actual) } +func TestLoadBalancedWebService_ServiceConnectEnabled(t *testing.T) { + testCases := map[string]struct { + mft *LoadBalancedWebService + + wanted bool + }{ + "enabled by default": { + mft: &LoadBalancedWebService{}, + wanted: true, + }, + "disable if explicitly set": { + mft: &LoadBalancedWebService{ + LoadBalancedWebServiceConfig: LoadBalancedWebServiceConfig{ + Network: NetworkConfig{ + Connect: ServiceConnect{ + EnableServiceConnect: aws.Bool(false), + }, + }, + }, + }, + wanted: false, + }, + } + + for name, tc := range testCases { + t.Run(name, func(t *testing.T) { + // WHEN + enabled := tc.mft.ServiceConnectEnabled() + + // THEN + require.Equal(t, tc.wanted, enabled) + }) + } +} + func TestLoadBalancedWebService_Publish(t *testing.T) { testCases := map[string]struct { mft *LoadBalancedWebService diff --git a/internal/pkg/manifest/transform.go b/internal/pkg/manifest/transform.go index de3b05b87f5..c3bc8e72be6 100644 --- a/internal/pkg/manifest/transform.go +++ b/internal/pkg/manifest/transform.go @@ -254,7 +254,7 @@ func (t serviceConnectTransformer) Transformer(typ reflect.Type) func(dst, src r dstStruct.ServiceConnectArgs = ServiceConnectArgs{} } - if !srcStruct.ServiceConnectArgs.IsEmpty() { + if !srcStruct.ServiceConnectArgs.isEmpty() { dstStruct.EnableServiceConnect = nil } diff --git a/internal/pkg/manifest/validate.go b/internal/pkg/manifest/validate.go index 01f12f7e820..516a8b14463 100644 --- a/internal/pkg/manifest/validate.go +++ b/internal/pkg/manifest/validate.go @@ -203,7 +203,7 @@ func (b BackendService) validate() error { }); err != nil { return fmt.Errorf("validate HTTP load balancer target: %w", err) } - if b.BackendServiceConfig.Network.Connect.enabled() { + if b.ServiceConnectEnabled() { if b.RoutingRule.GetTargetContainer() == nil && b.ImageConfig.Port == nil { return fmt.Errorf(`cannot enable "network.connect" when no port exposed`) } diff --git a/internal/pkg/manifest/workload.go b/internal/pkg/manifest/workload.go index f94d4d8a1e5..fb2c5be5f28 100644 --- a/internal/pkg/manifest/workload.go +++ b/internal/pkg/manifest/workload.go @@ -481,13 +481,6 @@ type ServiceConnect struct { ServiceConnectArgs } -func (s *ServiceConnect) enabled() bool { - if aws.BoolValue(s.EnableServiceConnect) { - return true - } - return !s.ServiceConnectArgs.IsEmpty() -} - // UnmarshalYAML overrides the default YAML unmarshaling logic for the ServiceConnect // struct, allowing it to perform more complex unmarshaling behavior. // This method implements the yaml.Unmarshaler (v3) interface. @@ -498,7 +491,7 @@ func (s *ServiceConnect) UnmarshalYAML(value *yaml.Node) error { return err } } - if !s.ServiceConnectArgs.IsEmpty() { + if !s.ServiceConnectArgs.isEmpty() { s.EnableServiceConnect = nil return nil } @@ -513,8 +506,7 @@ type ServiceConnectArgs struct { Alias *string } -// IsEmpty returns empty if the struct has all zero members. -func (s *ServiceConnectArgs) IsEmpty() bool { +func (s *ServiceConnectArgs) isEmpty() bool { return s.Alias == nil } diff --git a/internal/pkg/template/workload.go b/internal/pkg/template/workload.go index f21824a5a71..f8f6f933d38 100644 --- a/internal/pkg/template/workload.go +++ b/internal/pkg/template/workload.go @@ -121,7 +121,7 @@ type WorkloadNestedStackOpts struct { // SidecarOpts holds configuration that's needed if the service has sidecar containers. type SidecarOpts struct { - Name *string + Name string Image *string Essential *bool Port *string From 4b809814f2ca1196f994f518c4c303c68dfb8abb Mon Sep 17 00:00:00 2001 From: penghaoh Date: Tue, 4 Oct 2022 10:26:37 -0700 Subject: [PATCH 06/12] Nit --- internal/pkg/manifest/lb_web_svc.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/internal/pkg/manifest/lb_web_svc.go b/internal/pkg/manifest/lb_web_svc.go index 41afeca19cd..0f6cd323dd0 100644 --- a/internal/pkg/manifest/lb_web_svc.go +++ b/internal/pkg/manifest/lb_web_svc.go @@ -171,10 +171,7 @@ func (s *LoadBalancedWebService) Port() (port uint16, ok bool) { // ServiceConnectEnabled returns if ServiceConnect is enabled or not. func (s *LoadBalancedWebService) ServiceConnectEnabled() bool { - if s.Network.Connect.EnableServiceConnect != nil && !*s.Network.Connect.EnableServiceConnect { - return false - } - return true + return s.Network.Connect.EnableServiceConnect == nil || *s.Network.Connect.EnableServiceConnect } // Publish returns the list of topics where notifications can be published. From eac707f47b65df6c6011b4ebacfda43b8b831647 Mon Sep 17 00:00:00 2001 From: penghaoh Date: Tue, 4 Oct 2022 11:03:06 -0700 Subject: [PATCH 07/12] Addr more nits --- .../testdata/workloads/svc-nlb-manifest.yml | 2 ++ .../testdata/workloads/svc-nlb-test.stack.yml | 17 +++++++++++++++++ .../deploy/cloudformation/stack/worker_svc.go | 3 +-- internal/pkg/manifest/backend_svc.go | 1 + internal/pkg/manifest/validate.go | 4 ++-- internal/pkg/manifest/validate_test.go | 6 +++--- 6 files changed, 26 insertions(+), 7 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-manifest.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-manifest.yml index 3f461a5ea32..f6a11dd4f3a 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-manifest.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-manifest.yml @@ -28,6 +28,8 @@ environments: nlb: healthcheck: port: 80 + network: + connect: true dev: http: path: '/' diff --git a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-test.stack.yml b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-test.stack.yml index 5e16baba2ba..a789f3673a5 100644 --- a/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-test.stack.yml +++ b/internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-nlb-test.stack.yml @@ -95,6 +95,7 @@ Resources: # If a bucket URL is specified, that means the template exists. awslogs-stream-prefix: copilot PortMappings: - ContainerPort: !Ref ContainerPort + Name: target - ContainerPort: 443 Protocol: tcp ExecutionRole: @@ -294,6 +295,22 @@ Resources: # If a bucket URL is specified, that means the template exists. MaximumPercent: 200 PropagateTags: SERVICE LaunchType: FARGATE + ServiceConnectConfiguration: + Enabled: True + Namespace: test.my-app.local + LogConfiguration: + LogDriver: awslogs + Options: + awslogs-region: !Ref AWS::Region + awslogs-group: !Ref LogGroup + awslogs-stream-prefix: copilot + Services: + - PortName: target + # Avoid using the same service with Service Discovery in a namespace. + DiscoveryName: !Join ["-", [!Ref WorkloadName, "sc"]] + ClientAliases: + - Port: !Ref TargetPort + DnsName: !Ref WorkloadName NetworkConfiguration: AwsvpcConfiguration: AssignPublicIp: ENABLED diff --git a/internal/pkg/deploy/cloudformation/stack/worker_svc.go b/internal/pkg/deploy/cloudformation/stack/worker_svc.go index 09b377980cd..f40a5f97311 100644 --- a/internal/pkg/deploy/cloudformation/stack/worker_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/worker_svc.go @@ -7,10 +7,9 @@ import ( "fmt" "strings" - "github.com/aws/copilot-cli/internal/pkg/config" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/cloudformation" + "github.com/aws/copilot-cli/internal/pkg/config" "github.com/aws/copilot-cli/internal/pkg/manifest" "github.com/aws/copilot-cli/internal/pkg/template" "github.com/aws/copilot-cli/internal/pkg/template/override" diff --git a/internal/pkg/manifest/backend_svc.go b/internal/pkg/manifest/backend_svc.go index dc01b383c4f..e064eabcfc8 100644 --- a/internal/pkg/manifest/backend_svc.go +++ b/internal/pkg/manifest/backend_svc.go @@ -99,6 +99,7 @@ func (s *BackendService) Port() (port uint16, ok bool) { } // ServiceConnectEnabled returns if ServiceConnect is enabled or not. +// Unless explicitly disabled in the manifest or if no server is configured we default to true. func (s *BackendService) ServiceConnectEnabled() bool { if s.Network.Connect.EnableServiceConnect != nil { return *s.Network.Connect.EnableServiceConnect diff --git a/internal/pkg/manifest/validate.go b/internal/pkg/manifest/validate.go index 516a8b14463..ca0fee99c1a 100644 --- a/internal/pkg/manifest/validate.go +++ b/internal/pkg/manifest/validate.go @@ -1639,7 +1639,7 @@ func validateTargetContainer(opts validateTargetContainerOpts) error { targetContainer := aws.StringValue(opts.targetContainer) if targetContainer == opts.mainContainerName { if opts.mainContainerPort == nil { - return fmt.Errorf("target container %q doesn't expose any port", targetContainer) + return fmt.Errorf("target container %q doesn't expose a port", targetContainer) } return nil } @@ -1648,7 +1648,7 @@ func validateTargetContainer(opts validateTargetContainerOpts) error { return fmt.Errorf("target container %q doesn't exist", targetContainer) } if sidecar.Port == nil { - return fmt.Errorf("target container %q doesn't expose any port", targetContainer) + return fmt.Errorf("target container %q doesn't expose a port", targetContainer) } return nil } diff --git a/internal/pkg/manifest/validate_test.go b/internal/pkg/manifest/validate_test.go index cde3928432d..e37f9db9975 100644 --- a/internal/pkg/manifest/validate_test.go +++ b/internal/pkg/manifest/validate_test.go @@ -593,7 +593,7 @@ func TestBackendService_validate(t *testing.T) { Name: aws.String("api"), }, }, - wantedError: fmt.Errorf(`validate HTTP load balancer target: target container "api" doesn't expose any port`), + wantedError: fmt.Errorf(`validate HTTP load balancer target: target container "api" doesn't expose a port`), }, "error if service connect is enabled without any port exposed": { config: BackendService{ @@ -3066,7 +3066,7 @@ func TestValidateLoadBalancerTarget(t *testing.T) { }, wanted: fmt.Errorf(`target container "foo" doesn't exist`), }, - "should return an error if target container doesn't expose any port": { + "should return an error if target container doesn't expose a port": { in: validateTargetContainerOpts{ mainContainerName: "mockMainContainer", targetContainer: aws.String("foo"), @@ -3074,7 +3074,7 @@ func TestValidateLoadBalancerTarget(t *testing.T) { "foo": {}, }, }, - wanted: fmt.Errorf(`target container "foo" doesn't expose any port`), + wanted: fmt.Errorf(`target container "foo" doesn't expose a port`), }, "success with no target container set": { in: validateTargetContainerOpts{ From 928acd6259f2e8df4de4f9638c4f7b69f422f307 Mon Sep 17 00:00:00 2001 From: penghaoh Date: Tue, 4 Oct 2022 13:48:46 -0700 Subject: [PATCH 08/12] Add feature flag --- internal/pkg/deploy/cloudformation/stack/backend_svc.go | 4 +++- .../pkg/deploy/cloudformation/stack/backend_svc_test.go | 1 + .../stack/lb_network_web_service_integration_test.go | 1 + .../cloudformation/stack/lb_web_service_integration_test.go | 1 + internal/pkg/deploy/cloudformation/stack/lb_web_svc.go | 4 +++- .../workloads/partials/cf/service-base-properties.yml | 2 +- .../templates/workloads/partials/cf/workload-container.yml | 6 ++++-- internal/pkg/template/workload.go | 2 ++ 8 files changed, 16 insertions(+), 5 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/backend_svc.go b/internal/pkg/deploy/cloudformation/stack/backend_svc.go index 25ab03b2d80..5e7d45d7dcd 100644 --- a/internal/pkg/deploy/cloudformation/stack/backend_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/backend_svc.go @@ -33,7 +33,8 @@ type BackendService struct { httpsEnabled bool albEnabled bool - parser backendSvcReadParser + parser backendSvcReadParser + FeatureFlag bool } // BackendServiceConfig contains data required to initialize a backend service stack. @@ -195,6 +196,7 @@ func (s *BackendService) Template() (string, error) { }, HostedZoneAliases: hostedZoneAliases, PermissionsBoundary: s.permBound, + FeatureFlag: s.FeatureFlag, }) if err != nil { return "", fmt.Errorf("parse backend service template: %w", err) diff --git a/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go b/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go index d0b06e82b42..b82d12838d2 100644 --- a/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go +++ b/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go @@ -654,6 +654,7 @@ func TestBackendService_TemplateAndParamsGeneration(t *testing.T) { EnvVersion: "v1.42.0", }, }) + serializer.FeatureFlag = true require.NoError(t, err) // mock parser for lambda functions diff --git a/internal/pkg/deploy/cloudformation/stack/lb_network_web_service_integration_test.go b/internal/pkg/deploy/cloudformation/stack/lb_network_web_service_integration_test.go index 9f997e833ec..cdfbf9a1333 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_network_web_service_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_network_web_service_integration_test.go @@ -105,6 +105,7 @@ func TestNetworkLoadBalancedWebService_Template(t *testing.T) { }, RootUserARN: "arn:aws:iam::123456789123:root", }, stack.WithNLB([]string{"10.0.0.0/24", "10.1.0.0/24"})) + serializer.FeatureFlag = true tpl, err := serializer.Template() require.NoError(t, err, "template should render") regExpGUID := regexp.MustCompile(`([a-f\d]{8}-)([a-f\d]{4}-){3}([a-f\d]{12})`) // Matches random guids diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_service_integration_test.go b/internal/pkg/deploy/cloudformation/stack/lb_web_service_integration_test.go index 34042cac16c..8db34f3c1a6 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_service_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_service_integration_test.go @@ -105,6 +105,7 @@ func TestLoadBalancedWebService_Template(t *testing.T) { EnvVersion: "v1.42.0", }, }) + serializer.FeatureFlag = true tpl, err := serializer.Template() require.NoError(t, err, "template should render") regExpGUID := regexp.MustCompile(`([a-f\d]{8}-)([a-f\d]{4}-){3}([a-f\d]{12})`) // Matches random guids diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go index be3468ff3d2..54643b387c1 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go @@ -38,7 +38,8 @@ type LoadBalancedWebService struct { publicSubnetCIDRBlocks []string appInfo deploy.AppInformation - parser loadBalancedWebSvcReadParser + parser loadBalancedWebSvcReadParser + FeatureFlag bool } // LoadBalancedWebServiceOption is used to configuring an optional field for LoadBalancedWebService. @@ -247,6 +248,7 @@ func (s *LoadBalancedWebService) Template() (string, error) { }, HostedZoneAliases: aliasesFor, PermissionsBoundary: s.permBound, + FeatureFlag: s.FeatureFlag, }) if err != nil { return "", err diff --git a/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml b/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml index f4ef5748d0b..cc982d4b2b7 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml @@ -33,7 +33,7 @@ CapacityProviderStrategy: {{- end}} {{- end}} {{- end }} -{{- if .ServiceConnect }} +{{- if and .ServiceConnect .FeatureFlag }} ServiceConnectConfiguration: Enabled: True Namespace: {{.ServiceDiscoveryEndpoint}} diff --git a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml index 321e848c8ed..9e1f60dd763 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml @@ -29,7 +29,7 @@ {{- if eq .WorkloadType "Load Balanced Web Service"}} PortMappings: - ContainerPort: !Ref ContainerPort - {{- if .ServiceConnect }} # remove when release + {{- if and .ServiceConnect .FeatureFlag}} # remove when release {{- if eq .HTTPTargetContainer.Name .WorkloadName}} Name: target {{- end}} @@ -43,12 +43,14 @@ {{- end}} {{- end}} {{/* end if eq .WorkloadType "Load Balanced Web Service"*/}} {{- if eq .WorkloadType "Backend Service"}} -{{- if and .ServiceConnect (eq .HTTPTargetContainer.Name .WorkloadName)}} # remove when release +{{- if eq .HTTPTargetContainer.Name .WorkloadName}} +{{- if and .ServiceConnect .FeatureFlag}} # remove when release PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort, Name: target}], !Ref "AWS::NoValue"] {{- else}} PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort}], !Ref "AWS::NoValue"] {{- end}} {{- end}} +{{- end}} {{- if .HealthCheck}} HealthCheck: Command: {{quoteSlice .HealthCheck.Command | fmtSlice}} diff --git a/internal/pkg/template/workload.go b/internal/pkg/template/workload.go index f8f6f933d38..3bdcc52da1f 100644 --- a/internal/pkg/template/workload.go +++ b/internal/pkg/template/workload.go @@ -590,6 +590,8 @@ type WorkloadOpts struct { // Additional options for worker service templates. Subscribe *SubscribeOpts + + FeatureFlag bool } // ParseLoadBalancedWebService parses a load balanced web service's CloudFormation template From 92c25ad69675df68332e670f194584c48e4f5bf4 Mon Sep 17 00:00:00 2001 From: penghaoh Date: Tue, 4 Oct 2022 15:17:38 -0700 Subject: [PATCH 09/12] Add more feature flag --- .../pkg/template/templates/workloads/partials/cf/sidecars.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml b/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml index 3da105c03cb..cf863787cc7 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml @@ -48,7 +48,7 @@ {{- if $sidecar.Port}} PortMappings: - ContainerPort: {{$sidecar.Port}} - {{- if $.ServiceConnect }} # remove when release + {{- if and $.ServiceConnect $.FeatureFlag}} # remove when release {{- if eq $.HTTPTargetContainer.Name $sidecar.Name}} Name: target {{- end}} From 04b875348c3fe01a00ff512dfec24bf08649daf4 Mon Sep 17 00:00:00 2001 From: penghaoh Date: Tue, 4 Oct 2022 15:24:52 -0700 Subject: [PATCH 10/12] Rename feature flag --- internal/pkg/deploy/cloudformation/stack/backend_svc.go | 6 +++--- .../pkg/deploy/cloudformation/stack/backend_svc_test.go | 2 +- .../stack/lb_network_web_service_integration_test.go | 2 +- .../cloudformation/stack/lb_web_service_integration_test.go | 2 +- internal/pkg/deploy/cloudformation/stack/lb_web_svc.go | 6 +++--- .../workloads/partials/cf/service-base-properties.yml | 2 +- .../template/templates/workloads/partials/cf/sidecars.yml | 2 +- .../templates/workloads/partials/cf/workload-container.yml | 4 ++-- internal/pkg/template/workload.go | 2 +- 9 files changed, 14 insertions(+), 14 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/backend_svc.go b/internal/pkg/deploy/cloudformation/stack/backend_svc.go index 5e7d45d7dcd..a2da5bc51d4 100644 --- a/internal/pkg/deploy/cloudformation/stack/backend_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/backend_svc.go @@ -33,8 +33,8 @@ type BackendService struct { httpsEnabled bool albEnabled bool - parser backendSvcReadParser - FeatureFlag bool + parser backendSvcReadParser + SCFeatureFlag bool } // BackendServiceConfig contains data required to initialize a backend service stack. @@ -196,7 +196,7 @@ func (s *BackendService) Template() (string, error) { }, HostedZoneAliases: hostedZoneAliases, PermissionsBoundary: s.permBound, - FeatureFlag: s.FeatureFlag, + SCFeatureFlag: s.SCFeatureFlag, }) if err != nil { return "", fmt.Errorf("parse backend service template: %w", err) diff --git a/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go b/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go index b82d12838d2..0bb472bf5c6 100644 --- a/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go +++ b/internal/pkg/deploy/cloudformation/stack/backend_svc_test.go @@ -654,7 +654,7 @@ func TestBackendService_TemplateAndParamsGeneration(t *testing.T) { EnvVersion: "v1.42.0", }, }) - serializer.FeatureFlag = true + serializer.SCFeatureFlag = true require.NoError(t, err) // mock parser for lambda functions diff --git a/internal/pkg/deploy/cloudformation/stack/lb_network_web_service_integration_test.go b/internal/pkg/deploy/cloudformation/stack/lb_network_web_service_integration_test.go index cdfbf9a1333..503a3263e3a 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_network_web_service_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_network_web_service_integration_test.go @@ -105,7 +105,7 @@ func TestNetworkLoadBalancedWebService_Template(t *testing.T) { }, RootUserARN: "arn:aws:iam::123456789123:root", }, stack.WithNLB([]string{"10.0.0.0/24", "10.1.0.0/24"})) - serializer.FeatureFlag = true + serializer.SCFeatureFlag = true tpl, err := serializer.Template() require.NoError(t, err, "template should render") regExpGUID := regexp.MustCompile(`([a-f\d]{8}-)([a-f\d]{4}-){3}([a-f\d]{12})`) // Matches random guids diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_service_integration_test.go b/internal/pkg/deploy/cloudformation/stack/lb_web_service_integration_test.go index 8db34f3c1a6..59fb5dd04ac 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_service_integration_test.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_service_integration_test.go @@ -105,7 +105,7 @@ func TestLoadBalancedWebService_Template(t *testing.T) { EnvVersion: "v1.42.0", }, }) - serializer.FeatureFlag = true + serializer.SCFeatureFlag = true tpl, err := serializer.Template() require.NoError(t, err, "template should render") regExpGUID := regexp.MustCompile(`([a-f\d]{8}-)([a-f\d]{4}-){3}([a-f\d]{12})`) // Matches random guids diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go index 54643b387c1..73bf9ed985a 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_svc.go @@ -38,8 +38,8 @@ type LoadBalancedWebService struct { publicSubnetCIDRBlocks []string appInfo deploy.AppInformation - parser loadBalancedWebSvcReadParser - FeatureFlag bool + parser loadBalancedWebSvcReadParser + SCFeatureFlag bool } // LoadBalancedWebServiceOption is used to configuring an optional field for LoadBalancedWebService. @@ -248,7 +248,7 @@ func (s *LoadBalancedWebService) Template() (string, error) { }, HostedZoneAliases: aliasesFor, PermissionsBoundary: s.permBound, - FeatureFlag: s.FeatureFlag, + SCFeatureFlag: s.SCFeatureFlag, }) if err != nil { return "", err diff --git a/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml b/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml index cc982d4b2b7..e066eaf68f6 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/service-base-properties.yml @@ -33,7 +33,7 @@ CapacityProviderStrategy: {{- end}} {{- end}} {{- end }} -{{- if and .ServiceConnect .FeatureFlag }} +{{- if and .ServiceConnect .SCFeatureFlag }} ServiceConnectConfiguration: Enabled: True Namespace: {{.ServiceDiscoveryEndpoint}} diff --git a/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml b/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml index cf863787cc7..7db620c92cc 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml @@ -48,7 +48,7 @@ {{- if $sidecar.Port}} PortMappings: - ContainerPort: {{$sidecar.Port}} - {{- if and $.ServiceConnect $.FeatureFlag}} # remove when release + {{- if and $.ServiceConnect $.SCFeatureFlag}} # remove when release {{- if eq $.HTTPTargetContainer.Name $sidecar.Name}} Name: target {{- end}} diff --git a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml index 9e1f60dd763..feabdd2dc90 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml @@ -29,7 +29,7 @@ {{- if eq .WorkloadType "Load Balanced Web Service"}} PortMappings: - ContainerPort: !Ref ContainerPort - {{- if and .ServiceConnect .FeatureFlag}} # remove when release + {{- if and .ServiceConnect .SCFeatureFlag}} # remove when release {{- if eq .HTTPTargetContainer.Name .WorkloadName}} Name: target {{- end}} @@ -44,7 +44,7 @@ {{- end}} {{/* end if eq .WorkloadType "Load Balanced Web Service"*/}} {{- if eq .WorkloadType "Backend Service"}} {{- if eq .HTTPTargetContainer.Name .WorkloadName}} -{{- if and .ServiceConnect .FeatureFlag}} # remove when release +{{- if and .ServiceConnect .SCFeatureFlag}} # remove when release PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort, Name: target}], !Ref "AWS::NoValue"] {{- else}} PortMappings: !If [ExposePort, [{ContainerPort: !Ref ContainerPort}], !Ref "AWS::NoValue"] diff --git a/internal/pkg/template/workload.go b/internal/pkg/template/workload.go index 3bdcc52da1f..371d76f9af3 100644 --- a/internal/pkg/template/workload.go +++ b/internal/pkg/template/workload.go @@ -591,7 +591,7 @@ type WorkloadOpts struct { // Additional options for worker service templates. Subscribe *SubscribeOpts - FeatureFlag bool + SCFeatureFlag bool } // ParseLoadBalancedWebService parses a load balanced web service's CloudFormation template From 1c8f94b07556ded99228593df24cbffdcda75615 Mon Sep 17 00:00:00 2001 From: penghaoh Date: Tue, 4 Oct 2022 16:17:18 -0700 Subject: [PATCH 11/12] Add more comments --- .../pkg/template/templates/workloads/partials/cf/sidecars.yml | 1 + .../templates/workloads/partials/cf/workload-container.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml b/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml index 7db620c92cc..64691acb590 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/sidecars.yml @@ -49,6 +49,7 @@ PortMappings: - ContainerPort: {{$sidecar.Port}} {{- if and $.ServiceConnect $.SCFeatureFlag}} # remove when release + {{/* Multiple exposed port is not supported yet. Therefore, if the target container is the sidecar, the target port must be the one and only one port that the container exposes. */}} {{- if eq $.HTTPTargetContainer.Name $sidecar.Name}} Name: target {{- end}} diff --git a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml index feabdd2dc90..e451cb152ad 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/workload-container.yml @@ -30,6 +30,7 @@ PortMappings: - ContainerPort: !Ref ContainerPort {{- if and .ServiceConnect .SCFeatureFlag}} # remove when release + {{/* Multiple exposed port is not supported yet. Therefore, if the target container is the sidecar, the target port must be the one and only one port that the container exposes. */}} {{- if eq .HTTPTargetContainer.Name .WorkloadName}} Name: target {{- end}} From a8ae10adf71c870b6711c159cf02f5a621464411 Mon Sep 17 00:00:00 2001 From: penghaoh Date: Tue, 4 Oct 2022 16:41:12 -0700 Subject: [PATCH 12/12] Change name for manifest service connect --- .../cloudformation/stack/lb_web_svc_test.go | 2 +- .../cloudformation/stack/transformers.go | 2 +- internal/pkg/manifest/backend_svc_test.go | 6 +++--- internal/pkg/manifest/lb_web_svc_test.go | 2 +- internal/pkg/manifest/svc_test.go | 2 +- internal/pkg/manifest/transform.go | 4 ++-- internal/pkg/manifest/transform_test.go | 20 +++++++++---------- internal/pkg/manifest/validate.go | 4 ++-- internal/pkg/manifest/validate_test.go | 2 +- internal/pkg/manifest/workload.go | 10 +++++----- internal/pkg/manifest/workload_test.go | 4 ++-- 11 files changed, 29 insertions(+), 29 deletions(-) diff --git a/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go b/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go index 22244758205..d939fa5b9f3 100644 --- a/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go +++ b/internal/pkg/deploy/cloudformation/stack/lb_web_svc_test.go @@ -207,7 +207,7 @@ Outputs: String: nil, StringSlice: []string{"world"}, } - mft.Network.Connect = manifest.ServiceConnect{ + mft.Network.Connect = manifest.ServiceConnectBoolOrArgs{ ServiceConnectArgs: manifest.ServiceConnectArgs{ Alias: aws.String("frontend"), }, diff --git a/internal/pkg/deploy/cloudformation/stack/transformers.go b/internal/pkg/deploy/cloudformation/stack/transformers.go index ea302c02c22..88fb9267082 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers.go @@ -496,7 +496,7 @@ func convertExecuteCommand(e *manifest.ExecuteCommand) *template.ExecuteCommandO return &template.ExecuteCommandOpts{} } -func convertServiceConnect(s manifest.ServiceConnect) *template.ServiceConnect { +func convertServiceConnect(s manifest.ServiceConnectBoolOrArgs) *template.ServiceConnect { return &template.ServiceConnect{ Alias: s.ServiceConnectArgs.Alias, } diff --git a/internal/pkg/manifest/backend_svc_test.go b/internal/pkg/manifest/backend_svc_test.go index fff85ecf941..84ac90473cd 100644 --- a/internal/pkg/manifest/backend_svc_test.go +++ b/internal/pkg/manifest/backend_svc_test.go @@ -329,7 +329,7 @@ func TestBackendService_ServiceConnectEnabled(t *testing.T) { mft: &BackendService{ BackendServiceConfig: BackendServiceConfig{ Network: NetworkConfig{ - Connect: ServiceConnect{}, + Connect: ServiceConnectBoolOrArgs{}, }, }, }, @@ -339,7 +339,7 @@ func TestBackendService_ServiceConnectEnabled(t *testing.T) { mft: &BackendService{ BackendServiceConfig: BackendServiceConfig{ Network: NetworkConfig{ - Connect: ServiceConnect{ + Connect: ServiceConnectBoolOrArgs{ EnableServiceConnect: aws.Bool(true), }, }, @@ -351,7 +351,7 @@ func TestBackendService_ServiceConnectEnabled(t *testing.T) { mft: &BackendService{ BackendServiceConfig: BackendServiceConfig{ Network: NetworkConfig{ - Connect: ServiceConnect{ + Connect: ServiceConnectBoolOrArgs{ ServiceConnectArgs: ServiceConnectArgs{ Alias: aws.String("api"), }, diff --git a/internal/pkg/manifest/lb_web_svc_test.go b/internal/pkg/manifest/lb_web_svc_test.go index 89eed93ae13..2cf7fcbce31 100644 --- a/internal/pkg/manifest/lb_web_svc_test.go +++ b/internal/pkg/manifest/lb_web_svc_test.go @@ -1352,7 +1352,7 @@ func TestLoadBalancedWebService_ServiceConnectEnabled(t *testing.T) { mft: &LoadBalancedWebService{ LoadBalancedWebServiceConfig: LoadBalancedWebServiceConfig{ Network: NetworkConfig{ - Connect: ServiceConnect{ + Connect: ServiceConnectBoolOrArgs{ EnableServiceConnect: aws.Bool(false), }, }, diff --git a/internal/pkg/manifest/svc_test.go b/internal/pkg/manifest/svc_test.go index 1a19f0a3db9..192e6b4bad0 100644 --- a/internal/pkg/manifest/svc_test.go +++ b/internal/pkg/manifest/svc_test.go @@ -163,7 +163,7 @@ environments: PlacementString: placementStringP(PublicSubnetPlacement), }, }, - Connect: ServiceConnect{ + Connect: ServiceConnectBoolOrArgs{ EnableServiceConnect: aws.Bool(true), }, }, diff --git a/internal/pkg/manifest/transform.go b/internal/pkg/manifest/transform.go index c3bc8e72be6..79cc426968c 100644 --- a/internal/pkg/manifest/transform.go +++ b/internal/pkg/manifest/transform.go @@ -243,12 +243,12 @@ type serviceConnectTransformer struct{} // Transformer returns custom merge logic for serviceConnectTransformer's fields. func (t serviceConnectTransformer) Transformer(typ reflect.Type) func(dst, src reflect.Value) error { - if typ != reflect.TypeOf(ServiceConnect{}) { + if typ != reflect.TypeOf(ServiceConnectBoolOrArgs{}) { return nil } return func(dst, src reflect.Value) error { - dstStruct, srcStruct := dst.Interface().(ServiceConnect), src.Interface().(ServiceConnect) + dstStruct, srcStruct := dst.Interface().(ServiceConnectBoolOrArgs), src.Interface().(ServiceConnectBoolOrArgs) if srcStruct.EnableServiceConnect != nil { dstStruct.ServiceConnectArgs = ServiceConnectArgs{} diff --git a/internal/pkg/manifest/transform_test.go b/internal/pkg/manifest/transform_test.go index 472158522cb..9f7ed4cdf7c 100644 --- a/internal/pkg/manifest/transform_test.go +++ b/internal/pkg/manifest/transform_test.go @@ -594,35 +594,35 @@ func TestSubnetListOrArgsTransformer_Transformer(t *testing.T) { func TestServiceConnectTransformer_Transformer(t *testing.T) { testCases := map[string]struct { - original func(p *ServiceConnect) - override func(p *ServiceConnect) - wanted func(p *ServiceConnect) + original func(p *ServiceConnectBoolOrArgs) + override func(p *ServiceConnectBoolOrArgs) + wanted func(p *ServiceConnectBoolOrArgs) }{ "bool set to empty if args is not nil": { - original: func(s *ServiceConnect) { + original: func(s *ServiceConnectBoolOrArgs) { s.EnableServiceConnect = aws.Bool(false) }, - override: func(s *ServiceConnect) { + override: func(s *ServiceConnectBoolOrArgs) { s.ServiceConnectArgs = ServiceConnectArgs{ Alias: aws.String("api"), } }, - wanted: func(s *ServiceConnect) { + wanted: func(s *ServiceConnectBoolOrArgs) { s.ServiceConnectArgs = ServiceConnectArgs{ Alias: aws.String("api"), } }, }, "args set to empty if bool is not nil": { - original: func(s *ServiceConnect) { + original: func(s *ServiceConnectBoolOrArgs) { s.ServiceConnectArgs = ServiceConnectArgs{ Alias: aws.String("api"), } }, - override: func(s *ServiceConnect) { + override: func(s *ServiceConnectBoolOrArgs) { s.EnableServiceConnect = aws.Bool(true) }, - wanted: func(s *ServiceConnect) { + wanted: func(s *ServiceConnectBoolOrArgs) { s.EnableServiceConnect = aws.Bool(true) }, }, @@ -630,7 +630,7 @@ func TestServiceConnectTransformer_Transformer(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { - var dst, override, wanted ServiceConnect + var dst, override, wanted ServiceConnectBoolOrArgs tc.original(&dst) tc.override(&override) diff --git a/internal/pkg/manifest/validate.go b/internal/pkg/manifest/validate.go index ca0fee99c1a..bd4a442ee8e 100644 --- a/internal/pkg/manifest/validate.go +++ b/internal/pkg/manifest/validate.go @@ -1266,8 +1266,8 @@ func (n NetworkConfig) validate() error { return nil } -// validate returns nil if ServiceConnect is configured correctly. -func (s ServiceConnect) validate() error { +// validate returns nil if ServiceConnectBoolOrArgs is configured correctly. +func (s ServiceConnectBoolOrArgs) validate() error { return s.ServiceConnectArgs.validate() } diff --git a/internal/pkg/manifest/validate_test.go b/internal/pkg/manifest/validate_test.go index e37f9db9975..d1d0515f7d3 100644 --- a/internal/pkg/manifest/validate_test.go +++ b/internal/pkg/manifest/validate_test.go @@ -600,7 +600,7 @@ func TestBackendService_validate(t *testing.T) { BackendServiceConfig: BackendServiceConfig{ ImageConfig: testImageConfig, Network: NetworkConfig{ - Connect: ServiceConnect{ + Connect: ServiceConnectBoolOrArgs{ EnableServiceConnect: aws.Bool(true), }, }, diff --git a/internal/pkg/manifest/workload.go b/internal/pkg/manifest/workload.go index fb2c5be5f28..8a02e0184b8 100644 --- a/internal/pkg/manifest/workload.go +++ b/internal/pkg/manifest/workload.go @@ -459,8 +459,8 @@ func (t *FIFOTopicAdvanceConfigOrBool) UnmarshalYAML(value *yaml.Node) error { // NetworkConfig represents options for network connection to AWS resources within a VPC. type NetworkConfig struct { - VPC vpcConfig `yaml:"vpc"` - Connect ServiceConnect `yaml:"connect"` + VPC vpcConfig `yaml:"vpc"` + Connect ServiceConnectBoolOrArgs `yaml:"connect"` } // IsEmpty returns empty if the struct has all zero members. @@ -475,8 +475,8 @@ func (c *NetworkConfig) requiredEnvFeatures() []string { return nil } -// ServiceConnect represents ECS Service Connect configuration. -type ServiceConnect struct { +// ServiceConnectBoolOrArgs represents ECS Service Connect configuration. +type ServiceConnectBoolOrArgs struct { EnableServiceConnect *bool ServiceConnectArgs } @@ -484,7 +484,7 @@ type ServiceConnect struct { // UnmarshalYAML overrides the default YAML unmarshaling logic for the ServiceConnect // struct, allowing it to perform more complex unmarshaling behavior. // This method implements the yaml.Unmarshaler (v3) interface. -func (s *ServiceConnect) UnmarshalYAML(value *yaml.Node) error { +func (s *ServiceConnectBoolOrArgs) UnmarshalYAML(value *yaml.Node) error { if err := value.Decode(&s.ServiceConnectArgs); err != nil { var yamlTypeErr *yaml.TypeError if !errors.As(err, &yamlTypeErr) { diff --git a/internal/pkg/manifest/workload_test.go b/internal/pkg/manifest/workload_test.go index 81376988dbb..9fa794ee71b 100644 --- a/internal/pkg/manifest/workload_test.go +++ b/internal/pkg/manifest/workload_test.go @@ -358,7 +358,7 @@ func TestServiceConnect_UnmarshalYAML(t *testing.T) { testCases := map[string]struct { inContent []byte - wantedStruct ServiceConnect + wantedStruct ServiceConnectBoolOrArgs wantedError error }{ "returns error if both bool and args specified": { @@ -376,7 +376,7 @@ func TestServiceConnect_UnmarshalYAML(t *testing.T) { "success": { inContent: []byte(`connect: alias: api`), - wantedStruct: ServiceConnect{ + wantedStruct: ServiceConnectBoolOrArgs{ ServiceConnectArgs: ServiceConnectArgs{ Alias: aws.String("api"), },