Skip to content

Commit 3154d01

Browse files
authored
fix(ec2-alpha): addInternetGW handles shared route table for subnets (#33824)
### Issue # (if applicable) Closes #33672 ### Reason for this change Earlier same routes were being added to shared route table between subnets causing deployment failures. ### Description of changes - Added helper method to unique we don't process single route table twice. - Also added return value for IGW and EIGW so that customers can refer them to as target later, similar to other gateway and endpoints. ### Describe any new or updated permissions being added NA ### Description of how you validated changes Added unit test and integration test Limitation for adding integration test for IPv6, we don't know the `AmazonprovidedIPv6` before for which local route gets added and needs to be added in expected assertion. ``` Error: Resolution error: Resolution error: Resolution error: Found an encoded list token string in a scalar string context. Use 'Fn.select(0, list)' (not 'list[0]') to extract elements from token lists. { DestinationCidrBlock: vpc.ipv6CidrBlocks[0], GatewayId: 'local', }, ``` ### 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) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 1c5cbfa commit 3154d01

12 files changed

+31904
-24
lines changed

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

+41-24
Original file line numberDiff line numberDiff line change
@@ -153,15 +153,15 @@ export interface IVpcV2 extends IVpc {
153153
* Can only be used for ipv6 enabled VPCs.
154154
* For more information, see the {@link https://docs.aws.amazon.com/vpc/latest/userguide/egress-only-internet-gateway-basics.html}.
155155
*/
156-
addEgressOnlyInternetGateway(options?: EgressOnlyInternetGatewayOptions): void;
156+
addEgressOnlyInternetGateway(options?: EgressOnlyInternetGatewayOptions): EgressOnlyInternetGateway;
157157

158158
/**
159159
* Adds an Internet Gateway to current VPC.
160160
* For more information, see the {@link https://docs.aws.amazon.com/vpc/latest/userguide/vpc-igw-internet-access.html}.
161161
*
162162
* @default - defines route for all ipv4('0.0.0.0') and ipv6 addresses('::/0')
163163
*/
164-
addInternetGateway(options?: InternetGatewayOptions): void;
164+
addInternetGateway(options?: InternetGatewayOptions): InternetGateway;
165165

166166
/**
167167
* Adds VPN Gateway to VPC and set route propogation.
@@ -431,7 +431,7 @@ export abstract class VpcV2Base extends Resource implements IVpcV2 {
431431
*
432432
* @default - in case of no input subnets, no route is created
433433
*/
434-
public addEgressOnlyInternetGateway(options?: EgressOnlyInternetGatewayOptions): void {
434+
public addEgressOnlyInternetGateway(options?: EgressOnlyInternetGatewayOptions): EgressOnlyInternetGateway {
435435
const egw = new EgressOnlyInternetGateway(this, 'EgressOnlyGW', {
436436
vpc: this,
437437
egressOnlyInternetGatewayName: options?.egressOnlyInternetGatewayName,
@@ -449,16 +449,13 @@ export abstract class VpcV2Base extends Resource implements IVpcV2 {
449449
}
450450

451451
if (options?.subnets) {
452-
// Use Set to ensure unique subnets
453-
const processedSubnets = new Set<string>();
452+
// Use helper function to ensure unique subnets
454453
const subnets = flatten(options.subnets.map(s => this.selectSubnets(s).subnets));
455-
subnets.forEach((subnet) => {
456-
if (!processedSubnets.has(subnet.node.id)) {
457-
this.createEgressRoute(subnet, egw, options.destination);
458-
processedSubnets.add(subnet.node.id);
459-
}
454+
this.processSubnets(subnets, (subnet) => {
455+
this.createEgressRoute(subnet, egw, options.destination);
460456
});
461457
}
458+
return egw;
462459
}
463460

464461
/**
@@ -480,7 +477,7 @@ export abstract class VpcV2Base extends Resource implements IVpcV2 {
480477
*
481478
* @default - creates a new route for public subnets(with all outbound access) to the Internet Gateway.
482479
*/
483-
public addInternetGateway(options?: InternetGatewayOptions): void {
480+
public addInternetGateway(options?: InternetGatewayOptions): InternetGateway {
484481
if (this._internetGatewayId) {
485482
throw new Error('The Internet Gateway has already been enabled.');
486483
}
@@ -495,22 +492,19 @@ export abstract class VpcV2Base extends Resource implements IVpcV2 {
495492

496493
// Add routes for subnets defined as an input
497494
if (options?.subnets) {
498-
// Use Set to ensure unique subnets
499-
const processedSubnets = new Set<string>();
500495
const subnets = flatten(options.subnets.map(s => this.selectSubnets(s).subnets));
501-
subnets.forEach((subnet) => {
502-
if (!processedSubnets.has(subnet.node.id)) {
503-
if (!this.publicSubnets.includes(subnet)) {
504-
Annotations.of(this).addWarningV2('InternetGatewayWarning',
505-
`Subnet ${subnet.node.id} is not a public subnet. Internet Gateway should be added only to public subnets.`);
506-
}
507-
this.addDefaultInternetRoute(subnet, igw, options);
508-
processedSubnets.add(subnet.node.id);
509-
}
510-
}); // If there are no input subnets defined, default route will be added to all public subnets
496+
// Use helper function to ensure unique subnets
497+
this.processSubnets(subnets, (subnet) => {
498+
this.addDefaultInternetRoute(subnet, igw, options);
499+
});
500+
// If there are no input subnets defined, default route will be added to all public subnets
511501
} else if (!options?.subnets && this.publicSubnets) {
512-
this.publicSubnets.forEach((publicSubnets) => this.addDefaultInternetRoute(publicSubnets, igw, options));
502+
// Track route tables that have already had routes added to prevent duplicates
503+
this.processSubnets(this.publicSubnets, (subnet) => {
504+
this.addDefaultInternetRoute(subnet, igw, options);
505+
});
513506
}
507+
return igw;
514508
}
515509

516510
/**
@@ -536,6 +530,29 @@ export abstract class VpcV2Base extends Resource implements IVpcV2 {
536530
});
537531
}
538532

533+
/**
534+
* Helper function to process unique subnets and route table for adding routes
535+
*/
536+
private processSubnets(
537+
subnets: ISubnetV2[],
538+
routeHandler: (subnet: ISubnetV2) => void,
539+
): void {
540+
const processedSubnets = new Set<string>();
541+
const processedRouteTables = new Set<string>();
542+
subnets.forEach((subnet) => {
543+
if (!this.publicSubnets.includes(subnet)) {
544+
Annotations.of(this).addWarningV2('InternetGatewayWarning', `Given ${subnet.node.id} is not a public subnet. Internet Gateway should be added only to public subnets.`);
545+
}
546+
if (!processedSubnets.has(subnet.node.id)) {
547+
if (subnet.routeTable && !processedRouteTables.has(subnet.routeTable.routeTableId)) {
548+
routeHandler(subnet);
549+
processedRouteTables.add(subnet.routeTable.routeTableId);
550+
}
551+
processedSubnets.add(subnet.node.id);
552+
}
553+
});
554+
}
555+
539556
/**
540557
* Adds a new NAT Gateway to the given subnet of this VPC
541558
* of given subnets.

Diff for: packages/@aws-cdk/aws-ec2-alpha/test/integ.vpc-shared-route-table.js.snapshot/VpcSharedRouteTableIntegDefaultTestDeployAssert4BB1B1C8.assets.json

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

0 commit comments

Comments
 (0)