Skip to content

Commit ccd8de7

Browse files
authored
fix(ec2-alpha): add multiple NATGW to the VPC using addNatGateway method (#34094)
Closes #<issue number here>. ### Reason for this change Cannot add multiple NATGW to current VPC construct as it is using the same construct id which cause conflict. ### Description of changes - Fix the NATGW construct id to be unique using provided subnet. - Fix the current domain being set in EIP to be `vpc` instead of `vpcId`. - Fix the validation for subnet IP ranges as those can be unresolved token values referred in VPC construct using vpc.attr.<ipaddressblock> . ### Describe any new or updated permissions being added NA ### Description of how you validated changes - Added unit test and integration test for NATGW. - Added unit test for EIP ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) BREAKING CHANGE: The logical ID for the NAT Gateway, defined using the `addNatGateways` method, will be changed, resulting in the NAT Gateway being recreated. Additionally, the domain for the Elastic IP (EIP) will be set to `vpc`, which will also trigger its recreation in the account. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 1ab924c commit ccd8de7

38 files changed

+32182
-4096
lines changed

Diff for: packages/@aws-cdk/aws-ec2-alpha/lib/route.ts

+8-8
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { CfnEIP, CfnEgressOnlyInternetGateway, CfnInternetGateway, CfnNatGateway, CfnVPCPeeringConnection, CfnRoute, CfnRouteTable, CfnVPCGatewayAttachment, CfnVPNGateway, CfnVPNGatewayRoutePropagation, GatewayVpcEndpoint, IRouteTable, IVpcEndpoint, RouterType } from 'aws-cdk-lib/aws-ec2';
22
import { Construct, IDependable } from 'constructs';
3-
import { Annotations, Duration, IResource, Resource, Tags } from 'aws-cdk-lib/core';
3+
import { Annotations, Duration, IResource, Resource, Tags, ValidationError } from 'aws-cdk-lib/core';
44
import { IVpcV2, VPNGatewayV2Options } from './vpc-v2-base';
55
import { NetworkUtils, allRouteTableIds, CidrBlock } from './util';
66
import { ISubnetV2 } from './subnet-v2';
@@ -438,7 +438,7 @@ export class NatGateway extends Resource implements IRouteTarget {
438438

439439
if (this.connectivityType === NatConnectivityType.PUBLIC) {
440440
if (!props.vpc && !props.allocationId) {
441-
throw new Error('Either provide vpc or allocationId');
441+
throw new ValidationError('Either provide vpc or allocationId', this);
442442
}
443443
}
444444

@@ -451,7 +451,7 @@ export class NatGateway extends Resource implements IRouteTarget {
451451
if (this.connectivityType === NatConnectivityType.PUBLIC) {
452452
if (!props.allocationId) {
453453
let eip = new CfnEIP(this, 'EIP', {
454-
domain: props.vpc?.vpcId,
454+
domain: 'vpc',
455455
});
456456
aId = eip.attrAllocationId;
457457
} else {
@@ -504,16 +504,16 @@ export class VPCPeeringConnection extends Resource implements IRouteTarget {
504504
const isCrossAccount = props.requestorVpc.ownerAccountId !== props.acceptorVpc.ownerAccountId;
505505

506506
if (!isCrossAccount && props.peerRoleArn) {
507-
throw new Error('peerRoleArn is not needed for same account peering');
507+
throw new ValidationError('peerRoleArn is not needed for same account peering', this);
508508
}
509509

510510
if (isCrossAccount && !props.peerRoleArn) {
511-
throw new Error('Cross account VPC peering requires peerRoleArn');
511+
throw new ValidationError('Cross account VPC peering requires peerRoleArn', this);
512512
}
513513

514514
const overlap = this.validateVpcCidrOverlap(props.requestorVpc, props.acceptorVpc);
515515
if (overlap) {
516-
throw new Error('CIDR block should not overlap with each other for establishing a peering connection');
516+
throw new ValidationError('CIDR block should not overlap with each other for establishing a peering connection', this);
517517
}
518518
if (props.vpcPeeringConnectionName) {
519519
Tags.of(this).add(NAME_TAG, props.vpcPeeringConnectionName);
@@ -735,11 +735,11 @@ export class Route extends Resource implements IRouteV2 {
735735
}
736736

737737
if (this.target.gateway?.routerType === RouterType.EGRESS_ONLY_INTERNET_GATEWAY && isDestinationIpv4) {
738-
throw new Error('Egress only internet gateway does not support IPv4 routing');
738+
throw new ValidationError('Egress only internet gateway does not support IPv4 routing', this);
739739
}
740740

741741
if ((props.target.gateway && props.target.endpoint) || (!props.target.gateway && !props.target.endpoint)) {
742-
throw new Error('Exactly one of `gateway` or `endpoint` must be specified.');
742+
throw new ValidationError('Exactly one of `gateway` or `endpoint` must be specified.', this);
743743
}
744744
this.targetRouterType = this.target.gateway ? this.target.gateway.routerType : RouterType.VPC_ENDPOINT;
745745
// Gateway generates route automatically via its RouteTable, thus we don't need to generate the resource for it

Diff for: packages/@aws-cdk/aws-ec2-alpha/lib/subnet-v2.ts

+14-12
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Resource, Names, Lazy, Tags } from 'aws-cdk-lib';
1+
import { Resource, Names, Lazy, Tags, Token, ValidationError, UnscopedValidationError } from 'aws-cdk-lib';
22
import { CfnSubnet, CfnSubnetRouteTableAssociation, INetworkAcl, IRouteTable, ISubnet, NetworkAcl, SubnetNetworkAclAssociation, SubnetType } from 'aws-cdk-lib/aws-ec2';
33
import { Construct, DependencyGroup, IDependable } from 'constructs';
44
import { IVpcV2 } from './vpc-v2-base';
@@ -262,30 +262,32 @@ export class SubnetV2 extends Resource implements ISubnetV2 {
262262
const ipv6CidrBlock = props.ipv6CidrBlock?.cidr;
263263

264264
if (!checkCidrRanges(props.vpc, props.ipv4CidrBlock.cidr)) {
265-
throw new Error('CIDR block should be within the range of VPC');
265+
throw new ValidationError('CIDR block should be within the range of VPC', this);
266266
}
267267

268268
let overlap: boolean = false;
269269
let overlapIpv6: boolean = false;
270270

271-
overlap = validateOverlappingCidrRanges(props.vpc, props.ipv4CidrBlock.cidr);
271+
if (!Token.isUnresolved(props.ipv4CidrBlock)) {
272+
overlap = validateOverlappingCidrRanges(props.vpc, props.ipv4CidrBlock.cidr);
273+
}
272274

273275
// check whether VPC supports ipv6
274-
if (props.ipv6CidrBlock?.cidr) {
276+
if (props.ipv6CidrBlock?.cidr && !Token.isUnresolved(props.ipv6CidrBlock?.cidr)) {
275277
validateSupportIpv6(props.vpc);
276278
overlapIpv6 = validateOverlappingCidrRangesipv6(props.vpc, props.ipv6CidrBlock?.cidr);
277279
}
278280

279281
if (overlap || overlapIpv6) {
280-
throw new Error('CIDR block should not overlap with existing subnet blocks');
282+
throw new ValidationError('CIDR block should not overlap with existing subnet blocks', this);
281283
}
282284

283285
if (props.assignIpv6AddressOnCreation && !props.ipv6CidrBlock) {
284-
throw new Error('IPv6 CIDR block is required when assigning IPv6 address on creation');
286+
throw new ValidationError('IPv6 CIDR block is required when assigning IPv6 address on creation', this);
285287
}
286288

287289
if (props.mapPublicIpOnLaunch === true && props.subnetType !== SubnetType.PUBLIC) {
288-
throw new Error('mapPublicIpOnLaunch can only be set to true for public subnets');
290+
throw new ValidationError('mapPublicIpOnLaunch can only be set to true for public subnets', this);
289291
}
290292

291293
const subnet = new CfnSubnet(this, 'Subnet', {
@@ -445,7 +447,7 @@ function storeSubnetToVpcByType(vpc: IVpcV2, subnet: SubnetV2, type: SubnetType)
445447
if (findFunctionType) {
446448
findFunctionType(vpc, subnet);
447449
} else {
448-
throw new Error(`Unsupported subnet type: ${type}`);
450+
throw new UnscopedValidationError(`Unsupported subnet type: ${type}`);
449451
}
450452

451453
/**
@@ -463,7 +465,7 @@ function storeSubnetToVpcByType(vpc: IVpcV2, subnet: SubnetV2, type: SubnetType)
463465
* Validates whether the provided VPC supports IPv6 addresses.
464466
*
465467
* @param vpc The VPC instance to be validated.
466-
* @throws Error if the VPC does not support IPv6 addresses.
468+
* @throws ValidationError if the VPC does not support IPv6 addresses.
467469
* @returns True if the VPC supports IPv6 addresses, false otherwise.
468470
* @internal
469471
*/
@@ -473,7 +475,7 @@ function validateSupportIpv6(vpc: IVpcV2) {
473475
secondaryAddress.ipv6IpamPoolId !== undefined || secondaryAddress.ipv6Pool !== undefined)) {
474476
return true;
475477
} else {
476-
throw new Error('To use IPv6, the VPC must enable IPv6 support.');
478+
throw new UnscopedValidationError('To use IPv6, the VPC must enable IPv6 support.');
477479
}
478480
} else {return false;}
479481
}
@@ -510,7 +512,7 @@ function checkCidrRanges(vpc: IVpcV2, cidrRange: string) {
510512

511513
// If no IPv4 is assigned as secondary address
512514
if (allCidrs.length === 0) {
513-
throw new Error('No secondary IP address attached to VPC');
515+
throw new UnscopedValidationError('No secondary IP address attached to VPC');
514516
}
515517

516518
return allCidrs.some(c => c.containsCidr(subnetCidrBlock));
@@ -560,7 +562,7 @@ function validateOverlappingCidrRanges(vpc: IVpcV2, ipv4CidrBlock: string): bool
560562
* @param vpc The VPC instance to check against.
561563
* @param ipv6CidrBlock The IPv6 CIDR block to be validated.
562564
* @returns True if the IPv6 CIDR block overlaps with existing subnet CIDR blocks, false otherwise.
563-
* @throws Error if no subnets are found in the VPC.
565+
* @throws ValidationError if no subnets are found in the VPC.
564566
* @internal
565567
*/
566568
function validateOverlappingCidrRangesipv6(vpc: IVpcV2, ipv6CidrBlock: string): boolean {

Diff for: packages/@aws-cdk/aws-ec2-alpha/lib/vpc-v2-base.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Aws, Resource, Annotations } from 'aws-cdk-lib';
1+
import { Aws, Resource, Annotations, ValidationError } from 'aws-cdk-lib';
22
import { IVpc, ISubnet, SubnetSelection, SelectedSubnets, EnableVpnGatewayOptions, VpnGateway, VpnConnectionType, CfnVPCGatewayAttachment, CfnVPNGatewayRoutePropagation, VpnConnectionOptions, VpnConnection, ClientVpnEndpointOptions, ClientVpnEndpoint, InterfaceVpcEndpointOptions, InterfaceVpcEndpoint, GatewayVpcEndpointOptions, GatewayVpcEndpoint, FlowLogOptions, FlowLog, FlowLogResourceType, SubnetType, SubnetFilter } from 'aws-cdk-lib/aws-ec2';
33
import { allRouteTableIds, flatten, subnetGroupNameFromConstructId } from './util';
44
import { IDependable, Dependable, IConstruct, DependencyGroup } from 'constructs';
@@ -559,9 +559,9 @@ export abstract class VpcV2Base extends Resource implements IVpcV2 {
559559
*/
560560
public addNatGateway(options: NatGatewayOptions): NatGateway {
561561
if (options.connectivityType === NatConnectivityType.PUBLIC && !this._internetGatewayId) {
562-
throw new Error('Cannot add a Public NAT Gateway without an Internet Gateway enabled on VPC');
562+
throw new ValidationError('Cannot add a Public NAT Gateway without an Internet Gateway enabled on VPC', this);
563563
}
564-
return new NatGateway(this, 'NATGateway', {
564+
return new NatGateway(this, `NATGateway-${options.subnet.node.id}`, {
565565
vpc: this,
566566
...options,
567567
});

Diff for: packages/@aws-cdk/aws-ec2-alpha/test/integ.test-import.js.snapshot/cdk.out

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: packages/@aws-cdk/aws-ec2-alpha/test/integ.test-import.js.snapshot/integ.json

+1-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: packages/@aws-cdk/aws-ec2-alpha/test/integ.test-import.js.snapshot/integtestmodelDefaultTestDeployAssertCF40BD53.assets.json

+2-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: packages/@aws-cdk/aws-ec2-alpha/test/integ.test-import.js.snapshot/manifest.json

+92-7
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)