From 82d5e5b3c8ad40463dd4a4aecbbc8f47ab69c37a Mon Sep 17 00:00:00 2001 From: Huang Date: Tue, 14 Nov 2023 14:49:43 -0800 Subject: [PATCH 1/4] chore: get to a working state --- internal/pkg/cli/deploy/backend.go | 49 +++- internal/pkg/cli/deploy/backend_test.go | 210 ++++++++++++++++++ internal/pkg/cli/deploy/workload_test.go | 1 + .../cloudformation/stack/backend_svc.go | 19 +- .../cloudformation/stack/transformers.go | 30 +++ internal/pkg/manifest/http.go | 2 +- .../partials/cf/imported-alb-resources.yml | 2 +- .../workloads/services/backend/cf.yml | 25 ++- internal/pkg/template/workload.go | 2 +- internal/pkg/template/workload_test.go | 10 + 10 files changed, 340 insertions(+), 10 deletions(-) diff --git a/internal/pkg/cli/deploy/backend.go b/internal/pkg/cli/deploy/backend.go index 8df9d749504..58e10ac551d 100644 --- a/internal/pkg/cli/deploy/backend.go +++ b/internal/pkg/cli/deploy/backend.go @@ -5,6 +5,8 @@ package deploy import ( "fmt" + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/copilot-cli/internal/pkg/aws/elbv2" "github.com/aws/aws-sdk-go/aws/session" "github.com/aws/copilot-cli/internal/pkg/aws/acm" @@ -21,6 +23,7 @@ import ( type backendSvcDeployer struct { *svcDeployer + elbGetter elbGetter backendMft *manifest.BackendService // Overriden in tests. @@ -41,6 +44,7 @@ func NewBackendDeployer(in *WorkloadDeployerInput) (*backendSvcDeployer, error) } return &backendSvcDeployer{ svcDeployer: svcDeployer, + elbGetter: elbv2.New(svcDeployer.envSess), backendMft: bsMft, aliasCertValidator: acm.New(svcDeployer.envSess), }, nil @@ -94,6 +98,14 @@ func (d *backendSvcDeployer) stackConfiguration(in *StackRuntimeConfiguration) ( if err := d.validateALBRuntime(); err != nil { return nil, err } + var opts []stack.BackendServiceOption + if d.backendMft.HTTP.ImportedALB != nil { + lb, err := d.elbGetter.LoadBalancer(aws.StringValue(d.backendMft.HTTP.ImportedALB)) + if err != nil { + return nil, err + } + opts = append(opts, stack.WithImportedInternalALB(lb)) + } var conf cloudformation.StackConfiguration switch { @@ -108,7 +120,7 @@ func (d *backendSvcDeployer) stackConfiguration(in *StackRuntimeConfiguration) ( ArtifactBucketName: d.resources.S3Bucket, RuntimeConfig: *rc, Addons: d.addons, - }) + }, opts...) if err != nil { return nil, fmt.Errorf("create stack configuration: %w", err) } @@ -124,8 +136,12 @@ func (d *backendSvcDeployer) stackConfiguration(in *StackRuntimeConfiguration) ( func (d *backendSvcDeployer) validateALBRuntime() error { if d.backendMft.HTTP.IsEmpty() { + fmt.Println("http is empty") return nil } + if err := d.validateImportedALBConfig(); err != nil { + return fmt.Errorf(`validate imported ALB configuration for "http": %w`, err) + } if err := d.validateRuntimeRoutingRule(d.backendMft.HTTP.Main); err != nil { return fmt.Errorf(`validate ALB runtime configuration for "http": %w`, err) } @@ -137,6 +153,37 @@ func (d *backendSvcDeployer) validateALBRuntime() error { return nil } +func (d *backendSvcDeployer) validateImportedALBConfig() error { + if d.backendMft.HTTP.ImportedALB == nil { + return nil + } + alb, err := d.elbGetter.LoadBalancer(aws.StringValue(d.backendMft.HTTP.ImportedALB)) + if err != nil { + return fmt.Errorf(`retrieve load balancer %q: %w`, aws.StringValue(d.backendMft.HTTP.ImportedALB), err) + } + if alb.Scheme != "internal" { + return fmt.Errorf(`imported ALB %q for Backend Service %q should have "internal" Scheme value`, alb.ARN, aws.StringValue(d.backendMft.Name)) + } + if len(alb.Listeners) == 0 || len(alb.Listeners) > 2 { + return fmt.Errorf(`imported ALB %q must have either one or two listeners`, alb.ARN) + } + if len(alb.Listeners) == 1 { + return nil + } + var isHTTP, isHTTPS bool + for _, listener := range alb.Listeners { + if listener.Protocol == "HTTP" { + isHTTP = true + } else if listener.Protocol == "HTTPS" { + isHTTPS = true + } + } + if !(isHTTP && isHTTPS) { + return fmt.Errorf("imported ALB %q must have listeners of protocols HTTP and HTTPS", alb.ARN) + } + return nil +} + func (d *backendSvcDeployer) validateRuntimeRoutingRule(rule manifest.RoutingRule) error { if rule.IsEmpty() { return nil diff --git a/internal/pkg/cli/deploy/backend_test.go b/internal/pkg/cli/deploy/backend_test.go index ccb1b706624..967caa77b04 100644 --- a/internal/pkg/cli/deploy/backend_test.go +++ b/internal/pkg/cli/deploy/backend_test.go @@ -5,6 +5,7 @@ package deploy import ( "errors" + "github.com/aws/copilot-cli/internal/pkg/aws/elbv2" "testing" "time" @@ -254,6 +255,213 @@ func TestBackendSvcDeployer_stackConfiguration(t *testing.T) { }, expectedErr: `validate ALB runtime configuration for "http.additional_rules[0]": cannot deploy service mock-svc without "alias" to environment mock-env with certificate imported`, }, + "failure if can't retrieve imported ALB": { + App: &config.Application{ + Name: mockAppName, + }, + Env: &config.Environment{ + Name: mockEnvName, + }, + Manifest: &manifest.BackendService{ + BackendServiceConfig: manifest.BackendServiceConfig{ + HTTP: manifest.HTTP{ + Main: manifest.RoutingRule{ + Path: aws.String("/"), + }, + ImportedALB: aws.String("mockALB"), + }, + }, + }, + setupMocks: func(m *deployMocks) { + m.mockEndpointGetter.EXPECT().ServiceDiscoveryEndpoint().Return(mockAppName+".local", nil) + m.mockEnvVersionGetter.EXPECT().Version().Return("v1.42.0", nil) + m.mockELBGetter.EXPECT().LoadBalancer("mockALB").Return(nil, errors.New("some error")) + }, + expectedErr: `validate imported ALB configuration for "http": retrieve load balancer "mockALB": some error`, + }, + "failure if imported ALB has 'internet-facing' not 'internal' scheme": { + App: &config.Application{ + Name: mockAppName, + }, + Env: &config.Environment{ + Name: mockEnvName, + }, + Manifest: &manifest.BackendService{ + Workload: manifest.Workload{ + Name: aws.String("be"), + }, + BackendServiceConfig: manifest.BackendServiceConfig{ + HTTP: manifest.HTTP{ + Main: manifest.RoutingRule{ + Path: aws.String("/"), + }, + ImportedALB: aws.String("mockALB"), + }, + }, + }, + setupMocks: func(m *deployMocks) { + m.mockEndpointGetter.EXPECT().ServiceDiscoveryEndpoint().Return(mockAppName+".local", nil) + m.mockEnvVersionGetter.EXPECT().Version().Return("v1.42.0", nil) + m.mockELBGetter.EXPECT().LoadBalancer("mockALB").Return(&elbv2.LoadBalancer{ + ARN: "mockALBARN", + Name: "mockALB", + Scheme: "internet-facing", + }, nil) + }, + expectedErr: `validate imported ALB configuration for "http": imported ALB "mockALBARN" for Backend Service "be" should have "internal" Scheme value`, + }, + "failure if imported ALB has no listeners": { + App: &config.Application{ + Name: mockAppName, + }, + Env: &config.Environment{ + Name: mockEnvName, + }, + Manifest: &manifest.BackendService{ + BackendServiceConfig: manifest.BackendServiceConfig{ + HTTP: manifest.HTTP{ + Main: manifest.RoutingRule{ + Path: aws.String("/"), + }, + ImportedALB: aws.String("mockALB"), + }, + }, + }, + setupMocks: func(m *deployMocks) { + m.mockEndpointGetter.EXPECT().ServiceDiscoveryEndpoint().Return(mockAppName+".local", nil) + m.mockEnvVersionGetter.EXPECT().Version().Return("v1.42.0", nil) + m.mockELBGetter.EXPECT().LoadBalancer("mockALB").Return(&elbv2.LoadBalancer{ + ARN: "mockALBARN", + Name: "mockALB", + Scheme: "internal", + Listeners: []elbv2.Listener{}, + }, nil) + }, + expectedErr: `validate imported ALB configuration for "http": imported ALB "mockALBARN" must have either one or two listeners`, + }, + "failure if imported ALB has more than 3 listeners": { + App: &config.Application{ + Name: mockAppName, + }, + Env: &config.Environment{ + Name: mockEnvName, + }, + Manifest: &manifest.BackendService{ + BackendServiceConfig: manifest.BackendServiceConfig{ + HTTP: manifest.HTTP{ + Main: manifest.RoutingRule{ + Path: aws.String("/"), + }, + ImportedALB: aws.String("mockALB"), + }, + }, + }, + setupMocks: func(m *deployMocks) { + m.mockEndpointGetter.EXPECT().ServiceDiscoveryEndpoint().Return(mockAppName+".local", nil) + m.mockEnvVersionGetter.EXPECT().Version().Return("v1.42.0", nil) + m.mockELBGetter.EXPECT().LoadBalancer("mockALB").Return(&elbv2.LoadBalancer{ + ARN: "mockALBARN", + Name: "mockALB", + Scheme: "internal", + Listeners: []elbv2.Listener{ + { + ARN: "default", + Port: 0, + Protocol: "something", + }, + { + ARN: "second", + Port: 80, + Protocol: "http", + }, + { + ARN: "third", + Port: 443, + Protocol: "https", + }}, + }, nil) + }, + expectedErr: `validate imported ALB configuration for "http": imported ALB "mockALBARN" must have either one or two listeners`, + }, + "failure if imported ALB has two listeners but they don't have HTTP and HTTPS protocols": { + App: &config.Application{ + Name: mockAppName, + }, + Env: &config.Environment{ + Name: mockEnvName, + }, + Manifest: &manifest.BackendService{ + Workload: manifest.Workload{ + Name: aws.String("be"), + }, + BackendServiceConfig: manifest.BackendServiceConfig{ + HTTP: manifest.HTTP{ + Main: manifest.RoutingRule{ + Path: aws.String("/"), + }, + ImportedALB: aws.String("mockALB"), + }, + }, + }, + setupMocks: func(m *deployMocks) { + m.mockEndpointGetter.EXPECT().ServiceDiscoveryEndpoint().Return(mockAppName+".local", nil) + m.mockEnvVersionGetter.EXPECT().Version().Return("v1.42.0", nil) + m.mockELBGetter.EXPECT().LoadBalancer("mockALB").Return(&elbv2.LoadBalancer{ + ARN: "mockALBARN", + Name: "mockALB", + Scheme: "internal", + Listeners: []elbv2.Listener{ + { + ARN: "default", + Port: 0, + Protocol: "something", + }, + { + ARN: "second", + Port: 80, + Protocol: "boop", + }}, + }, nil) + }, + expectedErr: `validate imported ALB configuration for "http": imported ALB "mockALBARN" must have listeners of protocols HTTP and HTTPS`, + }, + "success imported ALB": { + App: &config.Application{ + Name: mockAppName, + }, + Env: &config.Environment{ + Name: mockEnvName, + }, + Manifest: &manifest.BackendService{ + Workload: manifest.Workload{ + Name: aws.String("be"), + }, + BackendServiceConfig: manifest.BackendServiceConfig{ + HTTP: manifest.HTTP{ + Main: manifest.RoutingRule{ + Path: aws.String("/"), + }, + ImportedALB: aws.String("mockALB"), + }, + }, + }, + setupMocks: func(m *deployMocks) { + m.mockEndpointGetter.EXPECT().ServiceDiscoveryEndpoint().Return(mockAppName+".local", nil) + m.mockEnvVersionGetter.EXPECT().Version().Return("v1.42.0", nil) + m.mockELBGetter.EXPECT().LoadBalancer("mockALB").Return(&elbv2.LoadBalancer{ + ARN: "mockALBARN", + Name: "mockALB", + Scheme: "internal", + Listeners: []elbv2.Listener{ + { + ARN: "yarn", + Port: 80, + Protocol: "HTTP", + }, + }, + }, nil).Times(2) + }, + }, "success if env has imported certs but alb not configured": { App: &config.Application{ Name: mockAppName, @@ -281,6 +489,7 @@ func TestBackendSvcDeployer_stackConfiguration(t *testing.T) { mockEndpointGetter: mocks.NewMockendpointGetter(ctrl), mockValidator: mocks.NewMockaliasCertValidator(ctrl), mockEnvVersionGetter: mocks.NewMockversionGetter(ctrl), + mockELBGetter: mocks.NewMockelbGetter(ctrl), } if tc.setupMocks != nil { tc.setupMocks(m) @@ -305,6 +514,7 @@ func TestBackendSvcDeployer_stackConfiguration(t *testing.T) { return nil }, }, + elbGetter: m.mockELBGetter, backendMft: tc.Manifest, aliasCertValidator: m.mockValidator, newStack: func() cloudformation.StackConfiguration { diff --git a/internal/pkg/cli/deploy/workload_test.go b/internal/pkg/cli/deploy/workload_test.go index c667ce0e35c..25e6e81897b 100644 --- a/internal/pkg/cli/deploy/workload_test.go +++ b/internal/pkg/cli/deploy/workload_test.go @@ -63,6 +63,7 @@ type deployMocks struct { mockLabeledTermPrinter *mocks.MockLabeledTermPrinter mockdockerEngineRunChecker *mocks.MockdockerEngineRunChecker mockdomainHostedZonegetter *mocks.MockdomainHostedZoneGetter + mockELBGetter *mocks.MockelbGetter } type mockTemplateFS struct { diff --git a/internal/pkg/deploy/cloudformation/stack/backend_svc.go b/internal/pkg/deploy/cloudformation/stack/backend_svc.go index 61e37c55efc..824f5df42b3 100644 --- a/internal/pkg/deploy/cloudformation/stack/backend_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/backend_svc.go @@ -5,6 +5,7 @@ package stack import ( "fmt" + "github.com/aws/copilot-cli/internal/pkg/aws/elbv2" "strconv" "strings" @@ -24,10 +25,21 @@ type BackendService struct { manifest *manifest.BackendService httpsEnabled bool albEnabled bool + importedALB *elbv2.LoadBalancer parser backendSvcReadParser } +// BackendServiceOption is used to configuring an optional field for LoadBalancedWebService. +type BackendServiceOption func(s *BackendService) + +// WithImportedInternalALB specifies an imported load balancer. +func WithImportedInternalALB(alb *elbv2.LoadBalancer) func(s *BackendService) { + return func(s *BackendService) { + s.importedALB = alb + } +} + // BackendServiceConfig contains data required to initialize a backend service stack. type BackendServiceConfig struct { App *config.Application @@ -40,7 +52,7 @@ type BackendServiceConfig struct { } // NewBackendService creates a new BackendService stack from a manifest file. -func NewBackendService(conf BackendServiceConfig) (*BackendService, error) { +func NewBackendService(conf BackendServiceConfig, opts ...BackendServiceOption) (*BackendService, error) { crs, err := customresource.Backend(fs) if err != nil { return nil, fmt.Errorf("backend service custom resources: %w", err) @@ -70,6 +82,9 @@ func NewBackendService(conf BackendServiceConfig) (*BackendService, error) { parser: fs, albEnabled: !conf.Manifest.HTTP.IsEmpty(), } + for _, opt := range opts { + opt(b) + } if len(conf.EnvManifest.HTTPConfig.Private.Certificates) != 0 { b.httpsEnabled = b.albEnabled @@ -127,6 +142,7 @@ func (s *BackendService) Template() (string, error) { if err != nil { return "", err } + importedALBConfig, err := s.convertImportedALB() scTarget := s.manifest.ServiceConnectTarget(exposedPorts) scOpts := template.ServiceConnectOpts{ Server: convertServiceConnectServer(s.manifest.Network.Connect, scTarget), @@ -178,6 +194,7 @@ func (s *BackendService) Template() (string, error) { ALBEnabled: s.albEnabled, GracePeriod: s.convertGracePeriod(), ALBListener: albListenerConfig, + ImportedALB: importedALBConfig, // Custom Resource Config. CustomResources: crs, diff --git a/internal/pkg/deploy/cloudformation/stack/transformers.go b/internal/pkg/deploy/cloudformation/stack/transformers.go index 08f8f100fa7..223703ef5c5 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers.go @@ -561,6 +561,36 @@ func (s *BackendService) convertGracePeriod() *int64 { return aws.Int64(int64(manifest.DefaultHealthCheckGracePeriod)) } +func (s *BackendService) convertImportedALB() (*template.ImportedALB, error) { + if s.importedALB == nil { + return nil, nil + } + var listeners []template.LBListener + for _, listener := range s.importedALB.Listeners { + listeners = append(listeners, template.LBListener{ + ARN: listener.ARN, + Port: listener.Port, + Protocol: listener.Protocol, + }) + } + var securityGroups []template.LBSecurityGroup + for _, sg := range s.importedALB.SecurityGroups { + securityGroups = append(securityGroups, template.LBSecurityGroup{ + ID: sg, + }) + } + fmt.Println(s.importedALB.Name) + fmt.Println(s.importedALB.Scheme) + return &template.ImportedALB{ + Name: s.importedALB.Name, + ARN: s.importedALB.ARN, + DNSName: s.importedALB.DNSName, + HostedZoneID: s.importedALB.HostedZoneID, + Listeners: listeners, + SecurityGroups: securityGroups, + }, nil +} + type loadBalancerTargeter interface { MainContainerPort() string ExposedPorts() (manifest.ExposedPortsIndex, error) diff --git a/internal/pkg/manifest/http.go b/internal/pkg/manifest/http.go index 1dbb59f691f..71ff60159ae 100644 --- a/internal/pkg/manifest/http.go +++ b/internal/pkg/manifest/http.go @@ -75,7 +75,7 @@ func (cfg HTTP) RoutingRules() []RoutingRule { // IsEmpty returns true if HTTP has empty configuration. func (r *HTTP) IsEmpty() bool { - return r.Main.IsEmpty() && r.TargetContainerCamelCase == nil && len(r.AdditionalRoutingRules) == 0 + return r.Main.IsEmpty() && r.TargetContainerCamelCase == nil && len(r.AdditionalRoutingRules) == 0 && r.ImportedALB == nil } // RoutingRule holds listener rule configuration for ALB. diff --git a/internal/pkg/template/templates/workloads/partials/cf/imported-alb-resources.yml b/internal/pkg/template/templates/workloads/partials/cf/imported-alb-resources.yml index 02a5e9f8aeb..10a20b82a42 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/imported-alb-resources.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/imported-alb-resources.yml @@ -141,7 +141,7 @@ HTTPListenerRuleForImportedALB{{ if ne $i 0 }}{{ $i }}{{ end }}: Type: AWS::ElasticLoadBalancingV2::ListenerRule Properties: Actions: - - TargetGroupArn: !Ref TargetGroup{{ if ne $i 0 }}{{ $i }}{{ end }} + - TargetGroupArn: !Ref TargetGroupForImportedALB{{ if ne $i 0 }}{{ $i }}{{ end }} Type: forward Conditions: {{- if $rule.AllowedSourceIps}} diff --git a/internal/pkg/template/templates/workloads/services/backend/cf.yml b/internal/pkg/template/templates/workloads/services/backend/cf.yml index 5ec9c6ededf..516cb3989a7 100644 --- a/internal/pkg/template/templates/workloads/services/backend/cf.yml +++ b/internal/pkg/template/templates/workloads/services/backend/cf.yml @@ -112,14 +112,23 @@ Resources: DependsOn: - EnvControllerAction {{- if .ALBListener }} - {{- range $i, $rule := .ALBListener.Rules }} - {{- if $.ALBListener.IsHTTPS}} + {{- range $i, $rule := .ALBListener.Rules }} + {{- if $.ImportedALB}} + {{- if ne $.ImportedALB.HTTPSListenerARN ""}} + - HTTPSListenerRuleForImportedALB{{ if ne $i 0 }}{{ $i }}{{ end }} + {{- if ne $.ImportedALB.HTTPListenerARN ""}} + - HTTPListenerRedirectRuleForImportedALB{{ if ne $i 0 }}{{ $i }}{{ end }} + {{- end}} + {{- else if ne $.ImportedALB.HTTPListenerARN ""}} + - HTTPListenerRuleForImportedALB{{ if ne $i 0 }}{{ $i }}{{ end }} + {{- end}} + {{- else if $.ALBListener.IsHTTPS}} - HTTPListenerRuleWithDomain{{ if ne $i 0 }}{{ $i }}{{ end }} - HTTPSListenerRule{{ if ne $i 0 }}{{ $i }}{{ end }} - {{- else }} + {{- else }} - HTTPListenerRule{{ if ne $i 0 }}{{ $i }}{{ end }} - {{- end}} - {{- end }} + {{- end}} + {{- end }} {{- end }} Properties: {{- "\n"}}{{ include "service-base-properties" . | indent 6 }} @@ -130,9 +139,15 @@ Resources: {{- end }} LoadBalancers: {{- range $i, $rule := .ALBListener.Rules}} + {{- if $.ImportedALB}} + - ContainerName: {{$rule.TargetContainer}} + ContainerPort: {{$rule.TargetPort}} + TargetGroupArn: !Ref TargetGroupForImportedALB{{ if ne $i 0 }}{{ $i }}{{ end }} + {{- else}} - ContainerName: {{$rule.TargetContainer}} ContainerPort: {{$rule.TargetPort}} TargetGroupArn: !Ref TargetGroup{{ if ne $i 0 }}{{ $i }}{{ end }} + {{- end}} {{- end}} {{- end }} {{include "efs-access-point" . | indent 2}} diff --git a/internal/pkg/template/workload.go b/internal/pkg/template/workload.go index 85094e62967..84437852ee7 100644 --- a/internal/pkg/template/workload.go +++ b/internal/pkg/template/workload.go @@ -1053,7 +1053,7 @@ func envControllerParameters(o WorkloadOpts) []string { } } if o.WorkloadType == "Backend Service" { - if o.ALBEnabled { + if o.ALBEnabled && o.ImportedALB == nil { parameters = append(parameters, "InternalALBWorkloads,") } } diff --git a/internal/pkg/template/workload_test.go b/internal/pkg/template/workload_test.go index 50fed479dd0..3ce75194316 100644 --- a/internal/pkg/template/workload_test.go +++ b/internal/pkg/template/workload_test.go @@ -417,6 +417,16 @@ func TestEnvControllerParameters(t *testing.T) { }, expected: []string{"InternalALBWorkloads,"}, }, + "Backend with imported ALB": { + opts: WorkloadOpts{ + WorkloadType: "Backend Service", + ALBEnabled: true, + ImportedALB: &ImportedALB{ + Name: "MyExistingALB", + }, + }, + expected: []string{}, + }, "RDWS": { opts: WorkloadOpts{ WorkloadType: "Request-Driven Web Service", From 00d91c3c1736dbd3739ce6e2d2e9e61eb373fe5e Mon Sep 17 00:00:00 2001 From: Huang Date: Thu, 16 Nov 2023 14:37:42 -0800 Subject: [PATCH 2/4] chore: clean up, cap SC, add lb_dns env var to be --- internal/pkg/cli/deploy/backend.go | 1 - internal/pkg/cli/run_local.go | 2 +- internal/pkg/cli/svc_deploy.go | 2 +- .../deploy/cloudformation/stack/transformers.go | 2 -- .../workloads/partials/cf/envvars-common.yml | 16 +++++++++++----- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/internal/pkg/cli/deploy/backend.go b/internal/pkg/cli/deploy/backend.go index 58e10ac551d..c89ed525c6c 100644 --- a/internal/pkg/cli/deploy/backend.go +++ b/internal/pkg/cli/deploy/backend.go @@ -136,7 +136,6 @@ func (d *backendSvcDeployer) stackConfiguration(in *StackRuntimeConfiguration) ( func (d *backendSvcDeployer) validateALBRuntime() error { if d.backendMft.HTTP.IsEmpty() { - fmt.Println("http is empty") return nil } if err := d.validateImportedALBConfig(); err != nil { diff --git a/internal/pkg/cli/run_local.go b/internal/pkg/cli/run_local.go index 712e86c9710..82589be5a05 100644 --- a/internal/pkg/cli/run_local.go +++ b/internal/pkg/cli/run_local.go @@ -820,7 +820,7 @@ func (h *hostDiscoverer) Hosts(ctx context.Context) ([]orchestrator.Host, error) var hosts []orchestrator.Host for _, svc := range svcs { - // find the primary deployment with service connect enabled + // find the primary deployment with Service Connect enabled idx := slices.IndexFunc(svc.Deployments, func(dep *sdkecs.Deployment) bool { return aws.StringValue(dep.Status) == "PRIMARY" && aws.BoolValue(dep.ServiceConnectConfiguration.Enabled) }) diff --git a/internal/pkg/cli/svc_deploy.go b/internal/pkg/cli/svc_deploy.go index d5685348073..548e2664fec 100644 --- a/internal/pkg/cli/svc_deploy.go +++ b/internal/pkg/cli/svc_deploy.go @@ -596,7 +596,7 @@ func (o *deploySvcOpts) uriRecommendedActions() ([]string, error) { case describe.URIAccessTypeServiceDiscovery: network = "with service discovery." case describe.URIAccessTypeServiceConnect: - network = "with service connect." + network = "with Service Connect." case describe.URIAccessTypeNone: return []string{}, nil } diff --git a/internal/pkg/deploy/cloudformation/stack/transformers.go b/internal/pkg/deploy/cloudformation/stack/transformers.go index 223703ef5c5..a0652e1e36d 100644 --- a/internal/pkg/deploy/cloudformation/stack/transformers.go +++ b/internal/pkg/deploy/cloudformation/stack/transformers.go @@ -579,8 +579,6 @@ func (s *BackendService) convertImportedALB() (*template.ImportedALB, error) { ID: sg, }) } - fmt.Println(s.importedALB.Name) - fmt.Println(s.importedALB.Scheme) return &template.ImportedALB{ Name: s.importedALB.Name, ARN: s.importedALB.ARN, diff --git a/internal/pkg/template/templates/workloads/partials/cf/envvars-common.yml b/internal/pkg/template/templates/workloads/partials/cf/envvars-common.yml index a856d6f89ff..2e7faa9731b 100644 --- a/internal/pkg/template/templates/workloads/partials/cf/envvars-common.yml +++ b/internal/pkg/template/templates/workloads/partials/cf/envvars-common.yml @@ -28,13 +28,19 @@ {{- end}} {{- end}} {{- end}}{{- end}} -{{- if eq .WorkloadType "Load Balanced Web Service"}} {{- if .ALBListener}} - Name: COPILOT_LB_DNS -{{- if .ImportedALB}} + {{- if eq .WorkloadType "Load Balanced Web Service"}} + {{- if .ImportedALB}} Value: {{.ImportedALB.DNSName}} -{{- else}} + {{- else}} Value: !GetAtt EnvControllerAction.PublicLoadBalancerDNSName -{{- end}} -{{- end}} + {{- end}} + {{- else if eq .WorkloadType "Backend Service"}} + {{- if .ImportedALB}} + Value: {{.ImportedALB.DNSName}} + {{- else}} + Value: !GetAtt EnvControllerAction.InternalLoadBalancerDNSName + {{- end}} + {{- end}} {{- end}} \ No newline at end of file From 3622066f833a21094d04f769e2c39142fc9d1e54 Mon Sep 17 00:00:00 2001 From: Huang Date: Thu, 14 Mar 2024 12:15:01 -0700 Subject: [PATCH 3/4] chore: addr fb --- internal/pkg/cli/deploy/backend.go | 14 +++++++------- internal/pkg/cli/deploy/backend_test.go | 8 ++++---- internal/pkg/cli/deploy/workload_test.go | 1 + .../templates/workloads/services/backend/cf.yml | 8 +++----- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/internal/pkg/cli/deploy/backend.go b/internal/pkg/cli/deploy/backend.go index 97833e97c82..6af1d8c14be 100644 --- a/internal/pkg/cli/deploy/backend.go +++ b/internal/pkg/cli/deploy/backend.go @@ -164,22 +164,22 @@ func (d *backendSvcDeployer) validateImportedALBConfig() error { if alb.Scheme != "internal" { return fmt.Errorf(`imported ALB %q for Backend Service %q should have "internal" Scheme value`, alb.ARN, aws.StringValue(d.backendMft.Name)) } - if len(alb.Listeners) == 0 || len(alb.Listeners) > 2 { - return fmt.Errorf(`imported ALB %q must have either one or two listeners`, alb.ARN) + if len(alb.Listeners) == 0 { + return fmt.Errorf(`imported ALB %q must have at least one listener. For two listeners, one must be of protocol HTTP and the other of protocol HTTPS`, alb.ARN) } if len(alb.Listeners) == 1 { return nil } - var isHTTP, isHTTPS bool + var quantHTTP, quantHTTPS int for _, listener := range alb.Listeners { if listener.Protocol == "HTTP" { - isHTTP = true + quantHTTP += 1 } else if listener.Protocol == "HTTPS" { - isHTTPS = true + quantHTTPS += 1 } } - if !(isHTTP && isHTTPS) { - return fmt.Errorf("imported ALB %q must have listeners of protocols HTTP and HTTPS", alb.ARN) + if quantHTTP != 1 || quantHTTPS != 1 { + return fmt.Errorf("imported ALB %q must have exactly one listener of protocol HTTP and exactly one listener of protocol HTTPS", alb.ARN) } return nil } diff --git a/internal/pkg/cli/deploy/backend_test.go b/internal/pkg/cli/deploy/backend_test.go index 967caa77b04..7a80218bdd2 100644 --- a/internal/pkg/cli/deploy/backend_test.go +++ b/internal/pkg/cli/deploy/backend_test.go @@ -337,9 +337,9 @@ func TestBackendSvcDeployer_stackConfiguration(t *testing.T) { Listeners: []elbv2.Listener{}, }, nil) }, - expectedErr: `validate imported ALB configuration for "http": imported ALB "mockALBARN" must have either one or two listeners`, + expectedErr: `validate imported ALB configuration for "http": imported ALB "mockALBARN" must have at least one listener. For two listeners, one must be of protocol HTTP and the other of protocol HTTPS`, }, - "failure if imported ALB has more than 3 listeners": { + "failure if imported ALB has more than 2 listeners": { App: &config.Application{ Name: mockAppName, }, @@ -381,7 +381,7 @@ func TestBackendSvcDeployer_stackConfiguration(t *testing.T) { }}, }, nil) }, - expectedErr: `validate imported ALB configuration for "http": imported ALB "mockALBARN" must have either one or two listeners`, + expectedErr: `validate imported ALB configuration for "http": imported ALB "mockALBARN" must have exactly one listener of protocol HTTP and exactly one listener of protocol HTTPS`, }, "failure if imported ALB has two listeners but they don't have HTTP and HTTPS protocols": { App: &config.Application{ @@ -423,7 +423,7 @@ func TestBackendSvcDeployer_stackConfiguration(t *testing.T) { }}, }, nil) }, - expectedErr: `validate imported ALB configuration for "http": imported ALB "mockALBARN" must have listeners of protocols HTTP and HTTPS`, + expectedErr: `validate imported ALB configuration for "http": imported ALB "mockALBARN" must have exactly one listener of protocol HTTP and exactly one listener of protocol HTTPS`, }, "success imported ALB": { App: &config.Application{ diff --git a/internal/pkg/cli/deploy/workload_test.go b/internal/pkg/cli/deploy/workload_test.go index 8b18c76be80..f337a4b9473 100644 --- a/internal/pkg/cli/deploy/workload_test.go +++ b/internal/pkg/cli/deploy/workload_test.go @@ -61,6 +61,7 @@ type deployMocks struct { mockValidator *mocks.MockaliasCertValidator mockLabeledTermPrinter *mocks.MockLabeledTermPrinter mockdockerEngineRunChecker *mocks.MockdockerEngineRunChecker + mockELBGetter *mocks.MockelbGetter } type mockTemplateFS struct { diff --git a/internal/pkg/template/templates/workloads/services/backend/cf.yml b/internal/pkg/template/templates/workloads/services/backend/cf.yml index 5d099b30c3f..8b59a57b355 100644 --- a/internal/pkg/template/templates/workloads/services/backend/cf.yml +++ b/internal/pkg/template/templates/workloads/services/backend/cf.yml @@ -142,15 +142,13 @@ Resources: {{- end }} LoadBalancers: {{- range $i, $rule := .ALBListener.Rules}} - {{- if $.ImportedALB}} - ContainerName: {{$rule.TargetContainer}} ContainerPort: {{$rule.TargetPort}} + {{- if $.ImportedALB}} TargetGroupArn: !Ref TargetGroupForImportedALB{{ if ne $i 0 }}{{ $i }}{{ end }} - {{- else}} - - ContainerName: {{$rule.TargetContainer}} - ContainerPort: {{$rule.TargetPort}} + {{- else}} TargetGroupArn: !Ref TargetGroup{{ if ne $i 0 }}{{ $i }}{{ end }} - {{- end}} + {{- end}} {{- end}} {{- end }} {{include "efs-access-point" . | indent 2}} From f505a34395500d82777b65a473f09b4620f5e9c4 Mon Sep 17 00:00:00 2001 From: Huang Date: Thu, 14 Mar 2024 14:28:10 -0700 Subject: [PATCH 4/4] chore: catch err --- internal/pkg/deploy/cloudformation/stack/backend_svc.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/pkg/deploy/cloudformation/stack/backend_svc.go b/internal/pkg/deploy/cloudformation/stack/backend_svc.go index 4d44740a588..cbd69dd7b6d 100644 --- a/internal/pkg/deploy/cloudformation/stack/backend_svc.go +++ b/internal/pkg/deploy/cloudformation/stack/backend_svc.go @@ -145,6 +145,9 @@ func (s *BackendService) Template() (string, error) { return "", err } importedALBConfig, err := s.convertImportedALB() + if err != nil { + return "", err + } scTarget := s.manifest.ServiceConnectTarget(exposedPorts) scOpts := template.ServiceConnectOpts{ Server: convertServiceConnectServer(s.manifest.Network.Connect, scTarget),