Skip to content

Commit 3714eb5

Browse files
huanjanihuanjani
authored
feat(network): allow imported VPCs with zero public subnets (#2356)
Previously, if users imported their own VPC resources, Copilot assumed they were bringing exactly two public and two private subnets and would error out if this was not the case. These changes allow for more flexibility, with the warning that load-balanced web services cannot be deployed in an environment with fewer than two public subnets, as the Load Balancer requires two in different AZs. I believe we do not need to warn users that it's impossible to deploy workloads with 'private' network placement specified if their VPC does not have private subnets, and the same with 'public.' I have manually tested: - importing a console-generated VPC with 0 subnets - importing a console-generated VPC with 1 public subnet - importing a console-generated VPC with 1 private subnet - importing a Copilot-generated VPC with: 1 public, 2 public, 1 private, 2 private, 1 public + 1 private - deploying a load-balanced web service (public placement) to the five configs above - deploying a backend service (public placement) to the five configs above - deploying a load-balanced web service (private placement) to the five configs above - deploying a backend service (private placement) to the five configs above To-do: - [x] Make the same changes for jobs - [x] Make sure `env update` works without public subnets - [x] Make sure these changes are backwards-compatible with previously imported VPCs To-do in a future PR: - Create a docs page for custom envs (#2224) Addresses #2079, #2233. By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
1 parent 196e241 commit 3714eb5

File tree

11 files changed

+106
-115
lines changed

11 files changed

+106
-115
lines changed

‎internal/pkg/aws/cloudformation/testdata/parse/lb-web-svc.yaml

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -131,16 +131,9 @@ Resources:
131131
AwsvpcConfiguration:
132132
AssignPublicIp: ENABLED
133133
Subnets:
134-
- Fn::Select:
135-
- 0
136-
- Fn::Split:
137-
- ','
138-
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
139-
- Fn::Select:
140-
- 1
141-
- Fn::Split:
142-
- ','
143-
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
134+
Fn::Split:
135+
- ','
136+
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
144137
SecurityGroups:
145138
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-EnvironmentSecurityGroup'
146139
DeploymentConfiguration:

‎internal/pkg/cli/env_init.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ const (
4444
- New IAM Roles to manage services and jobs in your environment
4545
`
4646
envInitVPCSelectPrompt = "Which VPC would you like to use?"
47-
envInitPublicSubnetsSelectPrompt = "Which public subnets would you like to use?"
47+
envInitPublicSubnetsSelectPrompt = "Which public subnets would you like to use?\nYou may choose to press 'Enter' to skip this step if the services and/or jobs you'll deploy to this environment are not internet-facing."
4848
envInitPrivateSubnetsSelectPrompt = "Which private subnets would you like to use?"
4949

5050
envInitVPCCIDRPrompt = "What VPC CIDR would you like to use?"
@@ -460,11 +460,15 @@ https://aws.amazon.com/premiumsupport/knowledge-center/ecs-pull-container-api-er
460460
publicSubnets, err := o.selVPC.PublicSubnets(envInitPublicSubnetsSelectPrompt, "", o.importVPC.ID)
461461
if err != nil {
462462
if err == selector.ErrSubnetsNotFound {
463-
log.Errorf(`No existing public subnets were found in VPC %s. You can either:
464-
- Create new public subnets and then import them.
465-
- Use the default Copilot environment configuration.`, o.importVPC.ID)
463+
log.Warningf(`No existing public subnets were found in VPC %s.
464+
If you proceed without public subnets, you will not be able to deploy Load Balanced Web Services in this environment.
465+
`, o.importVPC.ID)
466+
} else {
467+
return fmt.Errorf("select public subnets: %w", err)
466468
}
467-
return fmt.Errorf("select public subnets: %w", err)
469+
}
470+
if len(publicSubnets) == 1 {
471+
return errors.New("select public subnets: at least two public subnets must be selected to enable Load Balancing")
468472
}
469473
o.importVPC.PublicSubnetIDs = publicSubnets
470474
}
@@ -478,6 +482,9 @@ https://aws.amazon.com/premiumsupport/knowledge-center/ecs-pull-container-api-er
478482
}
479483
return fmt.Errorf("select private subnets: %w", err)
480484
}
485+
if len(privateSubnets) == 1 {
486+
return errors.New("select private subnets: at least two private subnets must be selected")
487+
}
481488
o.importVPC.PrivateSubnetIDs = privateSubnets
482489
}
483490
return nil

‎internal/pkg/cli/env_init_test.go

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ func TestInitEnvOpts_Ask(t *testing.T) {
359359
},
360360
wantedError: fmt.Errorf("select public subnets: some error"),
361361
},
362-
"fail to select private subnets": {
362+
"fail to select TWO public subnets": {
363363
inAppName: mockApp,
364364
inEnv: mockEnv,
365365
inProfile: mockProfile,
@@ -371,12 +371,27 @@ func TestInitEnvOpts_Ask(t *testing.T) {
371371
m.ec2Client.EXPECT().HasDNSSupport("mockVPC").Return(true, nil)
372372
m.selVPC.EXPECT().PublicSubnets(envInitPublicSubnetsSelectPrompt, "", "mockVPC").
373373
Return([]string{"mockPublicSubnet"}, nil)
374+
},
375+
wantedError: fmt.Errorf("select public subnets: at least two public subnets must be selected to enable Load Balancing"),
376+
},
377+
"fail to select private subnets": {
378+
inAppName: mockApp,
379+
inEnv: mockEnv,
380+
inProfile: mockProfile,
381+
setupMocks: func(m initEnvMocks) {
382+
m.sessProvider.EXPECT().FromProfile(gomock.Any()).Return(mockSession, nil)
383+
m.prompt.EXPECT().SelectOne(envInitDefaultEnvConfirmPrompt, "", envInitCustomizedEnvTypes).
384+
Return(envInitImportEnvResourcesSelectOption, nil)
385+
m.selVPC.EXPECT().VPC(envInitVPCSelectPrompt, "").Return("mockVPC", nil)
386+
m.ec2Client.EXPECT().HasDNSSupport("mockVPC").Return(true, nil)
387+
m.selVPC.EXPECT().PublicSubnets(envInitPublicSubnetsSelectPrompt, "", "mockVPC").
388+
Return([]string{"mockPublicSubnet", "anotherMockPublicSubnet"}, nil)
374389
m.selVPC.EXPECT().PrivateSubnets(envInitPrivateSubnetsSelectPrompt, "", "mockVPC").
375390
Return(nil, mockErr)
376391
},
377392
wantedError: fmt.Errorf("select private subnets: some error"),
378393
},
379-
"success with importing env resources with no flags": {
394+
"fail to select TWO private subnets": {
380395
inAppName: mockApp,
381396
inEnv: mockEnv,
382397
inProfile: mockProfile,
@@ -387,19 +402,52 @@ func TestInitEnvOpts_Ask(t *testing.T) {
387402
m.selVPC.EXPECT().VPC(envInitVPCSelectPrompt, "").Return("mockVPC", nil)
388403
m.ec2Client.EXPECT().HasDNSSupport("mockVPC").Return(true, nil)
389404
m.selVPC.EXPECT().PublicSubnets(envInitPublicSubnetsSelectPrompt, "", "mockVPC").
390-
Return([]string{"mockPublicSubnet"}, nil)
405+
Return([]string{"mockPublicSubnet", "anotherMockPublicSubnet"}, nil)
391406
m.selVPC.EXPECT().PrivateSubnets(envInitPrivateSubnetsSelectPrompt, "", "mockVPC").
392407
Return([]string{"mockPrivateSubnet"}, nil)
393408
},
409+
wantedError: fmt.Errorf("select private subnets: at least two private subnets must be selected"),
410+
},
411+
"success with selecting zero public subnets and two private subnets": {
412+
inAppName: mockApp,
413+
inEnv: mockEnv,
414+
inProfile: mockProfile,
415+
setupMocks: func(m initEnvMocks) {
416+
m.sessProvider.EXPECT().FromProfile(gomock.Any()).Return(mockSession, nil)
417+
m.prompt.EXPECT().SelectOne(envInitDefaultEnvConfirmPrompt, "", envInitCustomizedEnvTypes).
418+
Return(envInitImportEnvResourcesSelectOption, nil)
419+
m.selVPC.EXPECT().VPC(envInitVPCSelectPrompt, "").Return("mockVPC", nil)
420+
m.ec2Client.EXPECT().HasDNSSupport("mockVPC").Return(true, nil)
421+
m.selVPC.EXPECT().PublicSubnets(envInitPublicSubnetsSelectPrompt, "", "mockVPC").
422+
Return([]string{}, nil)
423+
m.selVPC.EXPECT().PrivateSubnets(envInitPrivateSubnetsSelectPrompt, "", "mockVPC").
424+
Return([]string{"mockPrivateSubnet", "anotherMockPrivateSubnet"}, nil)
425+
},
426+
},
427+
"success with importing env resources with no flags": {
428+
inAppName: mockApp,
429+
inEnv: mockEnv,
430+
inProfile: mockProfile,
431+
setupMocks: func(m initEnvMocks) {
432+
m.sessProvider.EXPECT().FromProfile(gomock.Any()).Return(mockSession, nil)
433+
m.prompt.EXPECT().SelectOne(envInitDefaultEnvConfirmPrompt, "", envInitCustomizedEnvTypes).
434+
Return(envInitImportEnvResourcesSelectOption, nil)
435+
m.selVPC.EXPECT().VPC(envInitVPCSelectPrompt, "").Return("mockVPC", nil)
436+
m.ec2Client.EXPECT().HasDNSSupport("mockVPC").Return(true, nil)
437+
m.selVPC.EXPECT().PublicSubnets(envInitPublicSubnetsSelectPrompt, "", "mockVPC").
438+
Return([]string{"mockPublicSubnet", "anotherMockPublicSubnet"}, nil)
439+
m.selVPC.EXPECT().PrivateSubnets(envInitPrivateSubnetsSelectPrompt, "", "mockVPC").
440+
Return([]string{"mockPrivateSubnet", "anotherMockPrivateSubnet"}, nil)
441+
},
394442
},
395443
"success with importing env resources with flags": {
396444
inAppName: mockApp,
397445
inEnv: mockEnv,
398446
inProfile: mockProfile,
399447
inImportVPCVars: importVPCVars{
400448
ID: "mockVPCID",
401-
PrivateSubnetIDs: []string{"mockPrivateSubnetID"},
402-
PublicSubnetIDs: []string{"mockPublicSubnetID"},
449+
PrivateSubnetIDs: []string{"mockPrivateSubnetID", "anotherMockPrivateSubnetID"},
450+
PublicSubnetIDs: []string{"mockPublicSubnetID", "anotherMockPublicSubnetID"},
403451
},
404452
setupMocks: func(m initEnvMocks) {
405453
m.sessProvider.EXPECT().FromProfile(gomock.Any()).Return(mockSession, nil)

‎internal/pkg/deploy/cloudformation/stack/testdata/workloads/job-test.stack.yml

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -322,16 +322,9 @@ Resources:
322322
Subnets:
323323
Fn::Join:
324324
- '","'
325-
- - Fn::Select:
326-
- 0
327-
- Fn::Split:
328-
- ','
329-
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
330-
- Fn::Select:
331-
- 1
332-
- Fn::Split:
333-
- ','
334-
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
325+
- Fn::Split:
326+
- ','
327+
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
335328
AssignPublicIp: ENABLED # Should be DISABLED if we use private subnets
336329
SecurityGroups:
337330
Fn::Join:

‎internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-prod.stack.yml

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -353,16 +353,9 @@ Resources:
353353
AwsvpcConfiguration:
354354
AssignPublicIp: ENABLED
355355
Subnets:
356-
- Fn::Select:
357-
- 0
358-
- Fn::Split:
359-
- ','
360-
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
361-
- Fn::Select:
362-
- 1
363-
- Fn::Split:
364-
- ','
365-
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
356+
Fn::Split:
357+
- ','
358+
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
366359
SecurityGroups:
367360
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-EnvironmentSecurityGroup'
368361

‎internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-staging.stack.yml

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -291,16 +291,9 @@ Resources:
291291
AwsvpcConfiguration:
292292
AssignPublicIp: ENABLED
293293
Subnets:
294-
- Fn::Select:
295-
- 0
296-
- Fn::Split:
297-
- ','
298-
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
299-
- Fn::Select:
300-
- 1
301-
- Fn::Split:
302-
- ','
303-
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
294+
Fn::Split:
295+
- ','
296+
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
304297
SecurityGroups:
305298
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-EnvironmentSecurityGroup'
306299

‎internal/pkg/deploy/cloudformation/stack/testdata/workloads/svc-test.stack.yml

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -346,16 +346,9 @@ Resources:
346346
AwsvpcConfiguration:
347347
AssignPublicIp: ENABLED
348348
Subnets:
349-
- Fn::Select:
350-
- 0
351-
- Fn::Split:
352-
- ','
353-
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
354-
- Fn::Select:
355-
- 1
356-
- Fn::Split:
357-
- ','
358-
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
349+
Fn::Split:
350+
- ','
351+
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
359352
SecurityGroups:
360353
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-EnvironmentSecurityGroup'
361354
# This may need to be adjusted if the container takes a while to start up

‎internal/pkg/template/workload_test.go

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -163,14 +163,7 @@ func TestTemplate_ParseNetwork(t *testing.T) {
163163
AwsvpcConfiguration:
164164
AssignPublicIp: ENABLED
165165
Subnets:
166-
- Fn::Select:
167-
- 0
168-
- Fn::Split:
169-
- ','
170-
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
171-
- Fn::Select:
172-
- 1
173-
- Fn::Split:
166+
Fn::Split:
174167
- ','
175168
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PublicSubnets'
176169
SecurityGroups:
@@ -186,14 +179,7 @@ func TestTemplate_ParseNetwork(t *testing.T) {
186179
AwsvpcConfiguration:
187180
AssignPublicIp: DISABLED
188181
Subnets:
189-
- Fn::Select:
190-
- 0
191-
- Fn::Split:
192-
- ','
193-
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PrivateSubnets'
194-
- Fn::Select:
195-
- 1
196-
- Fn::Split:
182+
Fn::Split:
197183
- ','
198184
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PrivateSubnets'
199185
SecurityGroups:
@@ -213,14 +199,7 @@ func TestTemplate_ParseNetwork(t *testing.T) {
213199
AwsvpcConfiguration:
214200
AssignPublicIp: DISABLED
215201
Subnets:
216-
- Fn::Select:
217-
- 0
218-
- Fn::Split:
219-
- ','
220-
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PrivateSubnets'
221-
- Fn::Select:
222-
- 1
223-
- Fn::Split:
202+
Fn::Split:
224203
- ','
225204
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-PrivateSubnets'
226205
SecurityGroups:

‎templates/environment/versions/cf-v1.4.0.yml

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,11 @@ Resources:
5252
ServiceDiscoveryNamespace:
5353
Type: AWS::ServiceDiscovery::PrivateDnsNamespace
5454
Properties:
55-
Name: !Sub ${AppName}.local
55+
Name: !Sub ${AppName}.local
5656
{{- if .ImportVPC}}
57-
Vpc: {{.ImportVPC.ID}}
57+
Vpc: {{.ImportVPC.ID}}
5858
{{- else}}
59-
Vpc: !Ref VPC
59+
Vpc: !Ref VPC
6060
{{- end}}
6161
Cluster:
6262
Metadata:
@@ -285,22 +285,28 @@ Outputs:
285285
{{- end}}
286286
Export:
287287
Name: !Sub ${AWS::StackName}-VpcId
288+
{{- if not .ImportVPC}}
288289
PublicSubnets:
289-
{{- if .ImportVPC}}
290-
Value: !Join [ ',', [ {{range $id := .ImportVPC.PublicSubnetIDs}}{{$id}}, {{end}}] ]
291-
{{- else}}
292290
Value: !Join [ ',', [ {{range $ind, $cidr := .VPCConfig.PublicSubnetCIDRs}}!Ref PublicSubnet{{inc $ind}}, {{end}}] ]
293-
{{- end}}
294291
Export:
295292
Name: !Sub ${AWS::StackName}-PublicSubnets
293+
{{- else if ne (len .ImportVPC.PublicSubnetIDs) 0}}
294+
PublicSubnets:
295+
Value: !Join [ ',', [ {{range $id := .ImportVPC.PublicSubnetIDs}}{{$id}}, {{end}}] ]
296+
Export:
297+
Name: !Sub ${AWS::StackName}-PublicSubnets
298+
{{- end}}
299+
{{- if not .ImportVPC}}
296300
PrivateSubnets:
297-
{{- if .ImportVPC}}
298-
Value: !Join [ ',', [ {{range $id := .ImportVPC.PrivateSubnetIDs}}{{$id}}, {{end}}] ]
299-
{{- else}}
300301
Value: !Join [ ',', [ {{range $ind, $cidr := .VPCConfig.PrivateSubnetCIDRs}}!Ref PrivateSubnet{{inc $ind}}, {{end}}] ]
301-
{{- end}}
302302
Export:
303303
Name: !Sub ${AWS::StackName}-PrivateSubnets
304+
{{- else if ne (len .ImportVPC.PrivateSubnetIDs) 0}}
305+
PrivateSubnets:
306+
Value: !Join [ ',', [ {{range $id := .ImportVPC.PrivateSubnetIDs}}{{$id}}, {{end}}] ]
307+
Export:
308+
Name: !Sub ${AWS::StackName}-PrivateSubnets
309+
{{- end}}
304310
ServiceDiscoveryNamespaceID:
305311
Value: !GetAtt ServiceDiscoveryNamespace.Id
306312
Export:

‎templates/workloads/partials/cf/service-base-properties.yml

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,16 +36,9 @@ NetworkConfiguration:
3636
AwsvpcConfiguration:
3737
AssignPublicIp: {{.Network.AssignPublicIP}}
3838
Subnets:
39-
- Fn::Select:
40-
- 0
41-
- Fn::Split:
42-
- ','
43-
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-{{.Network.SubnetsType}}'
44-
- Fn::Select:
45-
- 1
46-
- Fn::Split:
47-
- ','
48-
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-{{.Network.SubnetsType}}'
39+
Fn::Split:
40+
- ','
41+
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-{{.Network.SubnetsType}}'
4942
SecurityGroups:
5043
- Fn::ImportValue: !Sub '${AppName}-${EnvName}-EnvironmentSecurityGroup'
5144
{{- range $sg := .Network.SecurityGroups}}

0 commit comments

Comments
 (0)