Skip to content

Commit

Permalink
feat: support importing internal ALBs for backend services (#5490)
Browse files Browse the repository at this point in the history 
Related: #5438. 
Integ test changes in #5483.

#5483 adds the Load Balancer DNS name to the service as an env var (and therefore appears in `svc show` output. However, it is not (yet) included in URI output as a recommended action in `svc deploy` output. 

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
  • Loading branch information
@huanjani
huanjani committed Jun 3, 2024
1 parent 83a0be3 commit 28e656c
Show file tree
Hide file tree
Showing 11 changed files with 339 additions and 11 deletions.
48 changes: 47 additions & 1 deletion internal/pkg/cli/deploy/backend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -21,6 +23,7 @@ import (

type backendSvcDeployer struct {
*svcDeployer
elbGetter elbGetter
backendMft *manifest.BackendService

// Overriden in tests.
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -109,7 +121,7 @@ func (d *backendSvcDeployer) stackConfiguration(in *StackRuntimeConfiguration) (
ArtifactKey: d.resources.KMSKeyARN,
RuntimeConfig: *rc,
Addons: d.addons,
})
}, opts...)
if err != nil {
return nil, fmt.Errorf("create stack configuration: %w", err)
}
Expand All @@ -127,6 +139,9 @@ func (d *backendSvcDeployer) validateALBRuntime() error {
if d.backendMft.HTTP.IsEmpty() {
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)
}
Expand All @@ -138,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 {
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 quantHTTP, quantHTTPS int
for _, listener := range alb.Listeners {
if listener.Protocol == "HTTP" {
quantHTTP += 1
} else if listener.Protocol == "HTTPS" {
quantHTTPS += 1
}
}
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
}

func (d *backendSvcDeployer) validateRuntimeRoutingRule(rule manifest.RoutingRule) error {
if rule.IsEmpty() {
return nil
Expand Down
210 changes: 210 additions & 0 deletions internal/pkg/cli/deploy/backend_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package deploy

import (
"errors"
"github.com/aws/copilot-cli/internal/pkg/aws/elbv2"
"testing"
"time"

Expand Down Expand Up @@ -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 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 2 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 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{
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 exactly one listener of protocol HTTP and exactly one listener of protocol 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,
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions internal/pkg/cli/deploy/workload_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ type deployMocks struct {
mockValidator *mocks.MockaliasCertValidator
mockLabeledTermPrinter *mocks.MockLabeledTermPrinter
mockdockerEngineRunChecker *mocks.MockdockerEngineRunChecker
mockELBGetter *mocks.MockelbGetter
}

type mockTemplateFS struct {
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/cli/run_local.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1102,7 +1102,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)
})
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/cli/svc_deploy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading

0 comments on commit 28e656c

Please sign in to comment.