Skip to content

Commit d9ed362

Browse files
Lou1415926Lou1415926
authored
chore: enable NLB, TLS termination, and NLB aliases (#3142)
This PR: 1. Enable rendering NLB template if `nlb` is specified in manifest 2. Enable TLS termination, using a certificate managed by Copilot custom resource that is dedicated to the service 3. Enable alias for NLB By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
1 parent 2b0eb7e commit d9ed362

File tree

12 files changed

+309
-52
lines changed

12 files changed

+309
-52
lines changed

cf-custom-resources/lib/nlb-cert-validator-updater.js renamed to cf-custom-resources/lib/nlb-cert-manager.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -652,9 +652,9 @@ async function inUseByOtherServices(loadBalancerDNS, domainName, route53Client)
652652
({hostedZoneID} = await domainResources(domainName));
653653
} catch (err) {
654654
if (err instanceof UnrecognizedDomainTypeError) {
655-
console.log(`Found ${domainName} in subject alternative names.
656-
It does not match any of these patterns: ".<env>.<app>.<domain>"， ".<app>.<domain>" or ".<domain>".
657-
This is unexpected. We don't error out as it may not cause any issue.`);
655+
console.log(`Found ${domainName} in subject alternative names. ` +
656+
"It does not match any of these patterns: '.<env>.<app>.<domain>'， '.<app>.<domain>' or '.<domain>'. " +
657+
"This is unexpected. We don't error out as it may not cause any issue.");
658658
return true; // This option has unrecognized pattern, we can't check if it is in use, so we assume it is in use.
659659
}
660660
throw err;

cf-custom-resources/test/nlb-cert-validator-updater-test.js renamed to cf-custom-resources/test/nlb-cert-manager-test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ const sinon = require("sinon");
88
const nock = require("nock");
99
let origLog = console.log;
1010

11-
const { attemptsValidationOptionsReady } = require("../lib/nlb-cert-validator-updater");
11+
const { attemptsValidationOptionsReady } = require("../lib/nlb-cert-manager");
1212

1313
describe("DNS Certificate Validation And Custom Domains for NLB", () => {
1414
// Mock requests.
@@ -44,7 +44,7 @@ describe("DNS Certificate Validation And Custom Domains for NLB", () => {
4444
// This workaround follows the comment here: https://github.com/dwyl/aws-sdk-mock/issues/206#issuecomment-640418772.
4545
jest.resetModules();
4646
AWS.setSDKInstance(require('aws-sdk'));
47-
const imported = require("../lib/nlb-cert-validator-updater");
47+
const imported = require("../lib/nlb-cert-manager");
4848
handler = imported.handler;
4949
reset = imported.reset;
5050
withDeadlineExpired = imported.withDeadlineExpired;

‎internal/pkg/cli/svc_deploy.go‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -606,17 +606,15 @@ func (o *deploySvcOpts) stackConfiguration() (cloudformation.StackConfiguration,
606606
var conf cloudformation.StackConfiguration
607607
switch t := mft.(type) {
608608
case *manifest.LoadBalancedWebService:
609-
if o.targetApp.Domain == "" && !t.Alias.IsEmpty() {
609+
if o.targetApp.Domain == "" && t.HasAliases() {
610610
log.Errorf(aliasUsedWithoutDomainFriendlyText)
611611
return nil, errors.New("alias specified when application is not associated with a domain")
612612
}
613613

614614
var opts []stack.LoadBalancedWebServiceOption
615615

616616
// TODO: https://github.com/aws/copilot-cli/issues/2918
617-
// 1. Should error out if `nlb.alias` is specified with targetApp.Domain == ""
618-
// 2. Should validate `nlb.alias` is specified
619-
// 3. ALB block should not be executed if http is disabled
617+
// 1. ALB block should not be executed if http is disabled
620618
if o.targetApp.RequiresDNSDelegation() {
621619
var appVersionGetter versionGetter
622620
if appVersionGetter, err = o.newAppVersionGetter(o.appName); err != nil {
@@ -625,8 +623,21 @@ func (o *deploySvcOpts) stackConfiguration() (cloudformation.StackConfiguration,
625623
if err = validateLBSvcAliasAndAppVersion(aws.StringValue(t.Name), t.Alias, o.targetApp, o.envName, appVersionGetter); err != nil {
626624
return nil, err
627625
}
626+
if err = validateLBSvcAliasAndAppVersion(aws.StringValue(t.Name), t.NLBConfig.Aliases, o.targetApp, o.envName, appVersionGetter); err != nil {
627+
return nil, err
628+
}
628629
opts = append(opts, stack.WithHTTPS())
629-
opts = append(opts, stack.WithDNSDelegation())
630+
631+
var caller identity.Caller
632+
caller, err = o.identity.Get()
633+
if err != nil {
634+
return nil, fmt.Errorf("get identity: %w", err)
635+
}
636+
opts = append(opts, stack.WithDNSDelegation(deploy.AppInformation{
637+
Name: o.targetEnvironment.App,
638+
DNSName: o.targetApp.Domain,
639+
AccountPrincipalARN: caller.RootUserARN,
640+
}))
630641
}
631642
if !t.NLBConfig.IsEmpty() {
632643
cidrBlocks, err := o.publicCIDRBlocks()

‎internal/pkg/cli/svc_deploy_test.go‎

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ type deploySvcMocks struct {
4747
mockSubnetLister *mocks.MockvpcSubnetLister
4848
mockS3Svc *mocks.Mockuploader
4949
mockAddons *mocks.Mocktemplater
50+
mockIdentity *mocks.MockidentityService
5051
}
5152

5253
type mockWorkloadMft struct {
@@ -766,6 +767,27 @@ func TestSvcDeployOpts_deploySvc(t *testing.T) {
766767
},
767768
wantErr: fmt.Errorf("alias is not compatible with application versions below %s", deploy.AliasLeastAppTemplateVersion),
768769
},
770+
"fail to enable nlb alias because of incompatible app version": {
771+
inNLB: manifest.NetworkLoadBalancerConfiguration{
772+
Port: aws.String("80"),
773+
Aliases: manifest.Alias{String: aws.String("mockAlias")},
774+
},
775+
inEnvironment: &config.Environment{
776+
Name: mockEnvName,
777+
Region: "us-west-2",
778+
},
779+
inApp: &config.Application{
780+
Name: mockAppName,
781+
Domain: "mockDomain",
782+
},
783+
mock: func(m *deploySvcMocks) {
784+
m.mockWs.EXPECT().ReadWorkloadManifest(mockSvcName).Return([]byte{}, nil)
785+
m.mockInterpolator.EXPECT().Interpolate("").Return("", nil)
786+
m.mockAppVersionGetter.EXPECT().Version().Return("v0.0.0", nil)
787+
m.mockEndpointGetter.EXPECT().ServiceDiscoveryEndpoint().Return("mockApp.local", nil)
788+
},
789+
wantErr: fmt.Errorf("alias is not compatible with application versions below %s", deploy.AliasLeastAppTemplateVersion),
790+
},
769791
"fail to enable https alias because of invalid alias": {
770792
inAliases: manifest.Alias{String: aws.String("v1.v2.mockDomain")},
771793
inEnvironment: &config.Environment{
@@ -784,6 +806,27 @@ func TestSvcDeployOpts_deploySvc(t *testing.T) {
784806
},
785807
wantErr: fmt.Errorf(`alias "v1.v2.mockDomain" is not supported in hosted zones managed by Copilot`),
786808
},
809+
"fail to enable nlb alias because of invalid alias": {
810+
inNLB: manifest.NetworkLoadBalancerConfiguration{
811+
Port: aws.String("80"),
812+
Aliases: manifest.Alias{String: aws.String("v1.v2.mockDomain")},
813+
},
814+
inEnvironment: &config.Environment{
815+
Name: mockEnvName,
816+
Region: "us-west-2",
817+
},
818+
inApp: &config.Application{
819+
Name: mockAppName,
820+
Domain: "mockDomain",
821+
},
822+
mock: func(m *deploySvcMocks) {
823+
m.mockWs.EXPECT().ReadWorkloadManifest(mockSvcName).Return([]byte{}, nil)
824+
m.mockInterpolator.EXPECT().Interpolate("").Return("", nil)
825+
m.mockAppVersionGetter.EXPECT().Version().Return("v1.0.0", nil)
826+
m.mockEndpointGetter.EXPECT().ServiceDiscoveryEndpoint().Return("mockApp.local", nil)
827+
},
828+
wantErr: fmt.Errorf(`alias "v1.v2.mockDomain" is not supported in hosted zones managed by Copilot`),
829+
},
787830
"error if fail to deploy service": {
788831
inEnvironment: &config.Environment{
789832
Name: mockEnvName,
@@ -797,6 +840,9 @@ func TestSvcDeployOpts_deploySvc(t *testing.T) {
797840
m.mockWs.EXPECT().ReadWorkloadManifest(mockSvcName).Return([]byte{}, nil)
798841
m.mockInterpolator.EXPECT().Interpolate("").Return("", nil)
799842
m.mockEndpointGetter.EXPECT().ServiceDiscoveryEndpoint().Return("mockApp.local", nil)
843+
m.mockIdentity.EXPECT().Get().Return(identity.Caller{
844+
RootUserARN: "1234",
845+
}, nil)
800846
m.mockServiceDeployer.EXPECT().DeployService(gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("some error"))
801847
},
802848
wantErr: fmt.Errorf("deploy service: some error"),
@@ -814,6 +860,9 @@ func TestSvcDeployOpts_deploySvc(t *testing.T) {
814860
m.mockWs.EXPECT().ReadWorkloadManifest(mockSvcName).Return([]byte{}, nil)
815861
m.mockInterpolator.EXPECT().Interpolate("").Return("", nil)
816862
m.mockEndpointGetter.EXPECT().ServiceDiscoveryEndpoint().Return("mockApp.local", nil)
863+
m.mockIdentity.EXPECT().Get().Return(identity.Caller{
864+
RootUserARN: "1234",
865+
}, nil)
817866
m.mockServiceDeployer.EXPECT().DeployService(gomock.Any(), gomock.Any(), gomock.Any()).Return(cloudformation.NewMockErrChangeSetEmpty())
818867
},
819868
wantErr: fmt.Errorf("deploy service: change set with name mockChangeSet for stack mockStack has no changes"),
@@ -832,6 +881,9 @@ func TestSvcDeployOpts_deploySvc(t *testing.T) {
832881
m.mockWs.EXPECT().ReadWorkloadManifest(mockSvcName).Return([]byte{}, nil)
833882
m.mockInterpolator.EXPECT().Interpolate("").Return("", nil)
834883
m.mockEndpointGetter.EXPECT().ServiceDiscoveryEndpoint().Return("mockApp.local", nil)
884+
m.mockIdentity.EXPECT().Get().Return(identity.Caller{
885+
RootUserARN: "1234",
886+
}, nil)
835887
m.mockServiceDeployer.EXPECT().DeployService(gomock.Any(), gomock.Any(), gomock.Any()).
836888
Return(nil)
837889
m.mockServiceUpdater.EXPECT().LastUpdatedAt(mockAppName, mockEnvName, mockSvcName).
@@ -853,6 +905,9 @@ func TestSvcDeployOpts_deploySvc(t *testing.T) {
853905
m.mockWs.EXPECT().ReadWorkloadManifest(mockSvcName).Return([]byte{}, nil)
854906
m.mockInterpolator.EXPECT().Interpolate("").Return("", nil)
855907
m.mockEndpointGetter.EXPECT().ServiceDiscoveryEndpoint().Return("mockApp.local", nil)
908+
m.mockIdentity.EXPECT().Get().Return(identity.Caller{
909+
RootUserARN: "1234",
910+
}, nil)
856911
m.mockServiceDeployer.EXPECT().DeployService(gomock.Any(), gomock.Any(), gomock.Any()).
857912
Return(nil)
858913
m.mockServiceUpdater.EXPECT().LastUpdatedAt(mockAppName, mockEnvName, mockSvcName).
@@ -877,6 +932,9 @@ func TestSvcDeployOpts_deploySvc(t *testing.T) {
877932
Return(cloudformation.NewMockErrChangeSetEmpty())
878933
m.mockServiceUpdater.EXPECT().LastUpdatedAt(mockAppName, mockEnvName, mockSvcName).
879934
Return(mockBeforeTime, nil)
935+
m.mockIdentity.EXPECT().Get().Return(identity.Caller{
936+
RootUserARN: "1234",
937+
}, nil)
880938
m.mockSpinner.EXPECT().Start(fmt.Sprintf(fmtForceUpdateSvcStart, mockSvcName, mockEnvName))
881939
m.mockServiceUpdater.EXPECT().ForceUpdateService(mockAppName, mockEnvName, mockSvcName).Return(mockError)
882940
m.mockSpinner.EXPECT().Stop(log.Serrorf(fmtForceUpdateSvcFailed, mockSvcName, mockEnvName, mockError))
@@ -897,6 +955,9 @@ func TestSvcDeployOpts_deploySvc(t *testing.T) {
897955
m.mockWs.EXPECT().ReadWorkloadManifest(mockSvcName).Return([]byte{}, nil)
898956
m.mockInterpolator.EXPECT().Interpolate("").Return("", nil)
899957
m.mockEndpointGetter.EXPECT().ServiceDiscoveryEndpoint().Return("mockApp.local", nil)
958+
m.mockIdentity.EXPECT().Get().Return(identity.Caller{
959+
RootUserARN: "1234",
960+
}, nil)
900961
m.mockServiceDeployer.EXPECT().DeployService(gomock.Any(), gomock.Any(), gomock.Any()).
901962
Return(cloudformation.NewMockErrChangeSetEmpty())
902963
m.mockServiceUpdater.EXPECT().LastUpdatedAt(mockAppName, mockEnvName, mockSvcName).
@@ -931,8 +992,10 @@ func TestSvcDeployOpts_deploySvc(t *testing.T) {
931992
m.mockInterpolator.EXPECT().Interpolate("").Return("", nil)
932993
m.mockAppVersionGetter.EXPECT().Version().Return("v1.0.0", nil)
933994
m.mockEndpointGetter.EXPECT().ServiceDiscoveryEndpoint().Return("mockApp.local", nil)
995+
m.mockIdentity.EXPECT().Get().Return(identity.Caller{
996+
RootUserARN: "1234",
997+
}, nil)
934998
m.mockServiceDeployer.EXPECT().DeployService(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
935-
936999
},
9371000
},
9381001
"success with force update": {
@@ -949,6 +1012,9 @@ func TestSvcDeployOpts_deploySvc(t *testing.T) {
9491012
m.mockWs.EXPECT().ReadWorkloadManifest(mockSvcName).Return([]byte{}, nil)
9501013
m.mockInterpolator.EXPECT().Interpolate("").Return("", nil)
9511014
m.mockEndpointGetter.EXPECT().ServiceDiscoveryEndpoint().Return("mockApp.local", nil)
1015+
m.mockIdentity.EXPECT().Get().Return(identity.Caller{
1016+
RootUserARN: "1234",
1017+
}, nil)
9521018
m.mockServiceDeployer.EXPECT().DeployService(gomock.Any(), gomock.Any(), gomock.Any()).
9531019
Return(cloudformation.NewMockErrChangeSetEmpty())
9541020
m.mockServiceUpdater.EXPECT().LastUpdatedAt(mockAppName, mockEnvName, mockSvcName).
@@ -976,6 +1042,7 @@ func TestSvcDeployOpts_deploySvc(t *testing.T) {
9761042
mockInterpolator: mocks.NewMockinterpolator(ctrl),
9771043
mockEnvDescriber: mocks.NewMockenvDescriber(ctrl),
9781044
mockSubnetLister: mocks.NewMockvpcSubnetLister(ctrl),
1045+
mockIdentity: mocks.NewMockidentityService(ctrl),
9791046
}
9801047
tc.mock(m)
9811048

@@ -993,6 +1060,7 @@ func TestSvcDeployOpts_deploySvc(t *testing.T) {
9931060
return m.mockAppVersionGetter, nil
9941061
},
9951062
endpointGetter: m.mockEndpointGetter,
1063+
identity: m.mockIdentity,
9961064
targetApp: tc.inApp,
9971065
targetEnvironment: tc.inEnvironment,
9981066
newInterpolator: func(app, env string) interpolator {

‎internal/pkg/deploy/cloudformation/stack/lb_web_svc.go‎

Lines changed: 48 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/aws/aws-sdk-go/aws"
1111
"github.com/aws/aws-sdk-go/service/cloudformation"
1212
"github.com/aws/copilot-cli/internal/pkg/addon"
13+
"github.com/aws/copilot-cli/internal/pkg/deploy"
1314
"github.com/aws/copilot-cli/internal/pkg/manifest"
1415
"github.com/aws/copilot-cli/internal/pkg/template"
1516
"github.com/aws/copilot-cli/internal/pkg/template/override"
@@ -20,6 +21,7 @@ const (
2021
lbWebSvcRulePriorityGeneratorPath = "custom-resources/alb-rule-priority-generator.js"
2122
desiredCountGeneratorPath = "custom-resources/desired-count-delegation.js"
2223
envControllerPath = "custom-resources/env-controller.js"
24+
nlbCertManagerPath = "custom-resources/nlb-cert-validator-updater.js"
2325
)
2426

2527
// Parameter logical IDs for a load balanced web service.
@@ -52,6 +54,7 @@ type LoadBalancedWebService struct {
5254
// should always be false; hence they could have different values at this time.
5355
dnsDelegationEnabled bool
5456
publicSubnetCIDRBlocks []string
57+
appInfo deploy.AppInformation
5558

5659
parser loadBalancedWebSvcReadParser
5760
}
@@ -76,9 +79,10 @@ func WithNLB(cidrBlocks []string) func(s *LoadBalancedWebService) {
7679
}
7780

7881
// WithDNSDelegation enables DNS delegation for a LoadBalancedWebService.
79-
func WithDNSDelegation() func(s *LoadBalancedWebService) {
82+
func WithDNSDelegation(app deploy.AppInformation) func(s *LoadBalancedWebService) {
8083
return func(s *LoadBalancedWebService) {
8184
s.dnsDelegationEnabled = true
85+
s.appInfo = app
8286
}
8387
}
8488

@@ -187,37 +191,45 @@ func (s *LoadBalancedWebService) Template() (string, error) {
187191
allowedSourceIPs = append(allowedSourceIPs, string(ipNet))
188192
}
189193

194+
nlbConfig, err := s.convertNetworkLoadBalancer()
195+
if err != nil {
196+
return "", err
197+
}
190198
content, err := s.parser.ParseLoadBalancedWebService(template.WorkloadOpts{
191-
Variables: s.manifest.TaskConfig.Variables,
192-
Secrets: s.manifest.TaskConfig.Secrets,
193-
Aliases: aliases,
194-
NestedStack: addonsOutputs,
195-
AddonsExtraParams: addonsParams,
196-
Sidecars: sidecars,
197-
LogConfig: convertLogging(s.manifest.Logging),
198-
DockerLabels: s.manifest.ImageConfig.Image.DockerLabels,
199-
Autoscaling: autoscaling,
200-
CapacityProviders: capacityProviders,
201-
DesiredCountOnSpot: desiredCountOnSpot,
202-
ExecuteCommand: convertExecuteCommand(&s.manifest.ExecuteCommand),
203-
WorkloadType: manifest.LoadBalancedWebServiceType,
204-
HealthCheck: convertContainerHealthCheck(s.manifest.ImageConfig.HealthCheck),
205-
HTTPHealthCheck: convertHTTPHealthCheck(&s.manifest.HealthCheck),
206-
DeregistrationDelay: deregistrationDelay,
207-
AllowedSourceIps: allowedSourceIPs,
208-
RulePriorityLambda: rulePriorityLambda.String(),
209-
DesiredCountLambda: desiredCountLambda.String(),
210-
EnvControllerLambda: envControllerLambda.String(),
211-
Storage: convertStorageOpts(s.manifest.Name, s.manifest.Storage),
212-
Network: convertNetworkConfig(s.manifest.Network),
213-
EntryPoint: entrypoint,
214-
Command: command,
215-
DependsOn: convertDependsOn(s.manifest.ImageConfig.Image.DependsOn),
216-
CredentialsParameter: aws.StringValue(s.manifest.ImageConfig.Image.Credentials),
217-
ServiceDiscoveryEndpoint: s.rc.ServiceDiscoveryEndpoint,
218-
Publish: publishers,
219-
Platform: convertPlatform(s.manifest.Platform),
220-
HTTPVersion: convertHTTPVersion(s.manifest.ProtocolVersion),
199+
Variables: s.manifest.TaskConfig.Variables,
200+
Secrets: s.manifest.TaskConfig.Secrets,
201+
Aliases: aliases,
202+
NestedStack: addonsOutputs,
203+
AddonsExtraParams: addonsParams,
204+
Sidecars: sidecars,
205+
LogConfig: convertLogging(s.manifest.Logging),
206+
DockerLabels: s.manifest.ImageConfig.Image.DockerLabels,
207+
Autoscaling: autoscaling,
208+
CapacityProviders: capacityProviders,
209+
DesiredCountOnSpot: desiredCountOnSpot,
210+
ExecuteCommand: convertExecuteCommand(&s.manifest.ExecuteCommand),
211+
WorkloadType: manifest.LoadBalancedWebServiceType,
212+
HealthCheck: convertContainerHealthCheck(s.manifest.ImageConfig.HealthCheck),
213+
HTTPHealthCheck: convertHTTPHealthCheck(&s.manifest.HealthCheck),
214+
DeregistrationDelay: deregistrationDelay,
215+
AllowedSourceIps: allowedSourceIPs,
216+
RulePriorityLambda: rulePriorityLambda.String(),
217+
DesiredCountLambda: desiredCountLambda.String(),
218+
EnvControllerLambda: envControllerLambda.String(),
219+
NLBCertManagerFunctionLambda: nlbConfig.certManagerLambda,
220+
Storage: convertStorageOpts(s.manifest.Name, s.manifest.Storage),
221+
Network: convertNetworkConfig(s.manifest.Network),
222+
EntryPoint: entrypoint,
223+
Command: command,
224+
DependsOn: convertDependsOn(s.manifest.ImageConfig.Image.DependsOn),
225+
CredentialsParameter: aws.StringValue(s.manifest.ImageConfig.Image.Credentials),
226+
ServiceDiscoveryEndpoint: s.rc.ServiceDiscoveryEndpoint,
227+
Publish: publishers,
228+
Platform: convertPlatform(s.manifest.Platform),
229+
HTTPVersion: convertHTTPVersion(s.manifest.ProtocolVersion),
230+
NLB: nlbConfig.settings,
231+
AppDNSName: nlbConfig.appDNSName,
232+
AppDNSDelegationRole: nlbConfig.appDNSDelegationRole,
221233
})
222234
if err != nil {
223235
return "", err
@@ -247,6 +259,10 @@ func (s *LoadBalancedWebService) httpLoadBalancerTarget() (targetContainer *stri
247259
return
248260
}
249261

262+
func (s *LoadBalancedWebService) containerPort() string {
263+
return strconv.FormatUint(uint64(aws.Uint16Value(s.manifest.ImageConfig.Port)), 10)
264+
}
265+
250266
// Parameters returns the list of CloudFormation parameters used by the template.
251267
func (s *LoadBalancedWebService) Parameters() ([]*cloudformation.Parameter, error) {
252268
wkldParams, err := s.ecsWkld.Parameters()
@@ -257,7 +273,7 @@ func (s *LoadBalancedWebService) Parameters() ([]*cloudformation.Parameter, erro
257273
return append(wkldParams, []*cloudformation.Parameter{
258274
{
259275
ParameterKey: aws.String(LBWebServiceContainerPortParamKey),
260-
ParameterValue: aws.String(strconv.FormatUint(uint64(aws.Uint16Value(s.manifest.ImageConfig.Port)), 10)),
276+
ParameterValue: aws.String(s.containerPort()),
261277
},
262278
{
263279
ParameterKey: aws.String(LBWebServiceRulePathParamKey),

0 commit comments

Comments
 (0)