From a6fb99097a337b9cb7010a18f137930dfba62696 Mon Sep 17 00:00:00 2001 From: Mike Cowgill Date: Tue, 9 Oct 2018 00:10:14 -0700 Subject: [PATCH 1/3] BREAKING(aws-ec2) Configure NAT to Subnet Placement * BREAKING change to VPC configuration only if natGateways was previously set * closes #741 * Enable NAT placement by subnet name * Change VpcNeworkProps.natGateways from number to { gatewayCount: number; subnetName: string } --- packages/@aws-cdk/aws-ec2/lib/vpc.ts | 87 +++++++++++++++------- packages/@aws-cdk/aws-ec2/test/test.vpc.ts | 67 ++++++++++++++++- 2 files changed, 122 insertions(+), 32 deletions(-) diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc.ts b/packages/@aws-cdk/aws-ec2/lib/vpc.ts index 11c4f9c918cb4..de5ff5844f13d 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc.ts @@ -1,7 +1,7 @@ import cdk = require('@aws-cdk/cdk'); import { cloudformation } from './ec2.generated'; import { NetworkBuilder } from './network-util'; -import { DEFAULT_SUBNET_NAME, subnetId } from './util'; +import { DEFAULT_SUBNET_NAME, subnetId, subnetName } from './util'; import { SubnetType, VpcNetworkRef, VpcSubnetRef } from './vpc-ref'; /** @@ -61,16 +61,9 @@ export interface VpcNetworkProps { maxAZs?: number; /** - * Define the maximum number of NAT Gateways for this VPC - * - * Setting this number enables a VPC to trade availability for the cost of - * running a NAT Gateway. For example, if set this to 1 and your subnet - * configuration is for 3 Public subnets with natGateway = `true` then only - * one of the Public subnets will have a gateway and all Private subnets - * will route to this NAT Gateway. - * @default maxAZs + * Define the NAT Gateway Configuration for this VPC */ - natGateways?: number; + natGateways?: NatGatewayConfiguration; /** * Configure the subnets to build for each AZ @@ -123,6 +116,27 @@ export enum DefaultInstanceTenancy { Dedicated = 'dedicated' } +export interface NatGatewayConfiguration { + /** + * The number of NAT Gateways to create. + * + * For example, if set this to 1 and your subnet configuration is for 3 Public subnets then only + * one of the Public subnets will have a gateway and all Private subnets will route to this NAT Gateway. + * @default maxAZs + */ + gatewayCount?: number; + + /** + * The names of the subnets that will have NAT Gateways + * + * The names of the corresponding subnets in `SubnetConfiguration` that will + * have a NAT Gateway. If a corresponding subnet name is not found this will + * throw an error. By default the first public subnets will receive NAT + * Gateways until the `gatewayCount` is reached. + */ + subnetName?: string; +} + /** * Specify configuration parameters for a VPC to be built */ @@ -231,13 +245,6 @@ export class VpcNetwork extends VpcNetworkRef implements cdk.ITaggable { */ public readonly tags: cdk.TagManager; - /** - * Maximum Number of NAT Gateways used to control cost - * - * @default {VpcNetworkProps.maxAZs} - */ - private readonly natGateways: number; - /** * The VPC resource */ @@ -303,8 +310,6 @@ export class VpcNetwork extends VpcNetworkRef implements cdk.ITaggable { this.subnetConfiguration = ifUndefined(props.subnetConfiguration, VpcNetwork.DEFAULT_SUBNETS); const useNatGateway = this.subnetConfiguration.filter( subnet => (subnet.subnetType === SubnetType.Private)).length > 0; - this.natGateways = ifUndefined(props.natGateways, - useNatGateway ? this.availabilityZones.length : 0); // subnetConfiguration and natGateways must be set before calling createSubnets this.createSubnets(); @@ -321,12 +326,33 @@ export class VpcNetwork extends VpcNetworkRef implements cdk.ITaggable { internetGatewayId: igw.ref, vpcId: this.resource.ref }); + this.dependencyElements.push(igw, att); + + const natConfig = ifUndefined(props.natGateways, { + gatewayCount: undefined, + subnetName: undefined, + }); + const natCount = ifUndefined(natConfig.gatewayCount, + useNatGateway ? this.availabilityZones.length : 0); + const natSubnet = natConfig.subnetName; + + if (natSubnet !== undefined) { + const subnetNames = (this.publicSubnets as VpcPublicSubnet[]).map( subnet => { + return subnetName(subnet); + }); + if (!subnetNames.includes(natSubnet)) { + throw new Error(`NatGatewayConfiguration contains subnet name ${natSubnet} which is not a Public Subnet in SubnetConfiguration`); + } + } + (this.publicSubnets as VpcPublicSubnet[]).forEach(publicSubnet => { publicSubnet.addDefaultIGWRouteEntry(igw.ref); + const currentNatCount = Object.values(this.natGatewayByAZ).length; + if (addNatGatewy(publicSubnet, natSubnet, natCount, currentNatCount)) { + this.natGatewayByAZ[publicSubnet.availabilityZone] = publicSubnet.addNatGateway(); + } }); - this.dependencyElements.push(igw, att); - (this.privateSubnets as VpcPrivateSubnet[]).forEach((privateSubnet, i) => { let ngwId = this.natGatewayByAZ[privateSubnet.availabilityZone]; if (ngwId === undefined) { @@ -386,12 +412,6 @@ export class VpcNetwork extends VpcNetworkRef implements cdk.ITaggable { switch (subnetConfig.subnetType) { case SubnetType.Public: const publicSubnet = new VpcPublicSubnet(this, name, subnetProps); - if (this.natGateways > 0) { - const ngwArray = Array.from(Object.values(this.natGatewayByAZ)); - if (ngwArray.length < this.natGateways) { - this.natGatewayByAZ[zone] = publicSubnet.addNatGateway(); - } - } this.publicSubnets.push(publicSubnet); break; case SubnetType.Private: @@ -561,5 +581,16 @@ export class VpcPrivateSubnet extends VpcSubnet { } function ifUndefined(value: T | undefined, defaultValue: T): T { - return value !== undefined ? value : defaultValue; + return value !== undefined ? value : defaultValue; +} + +function addNatGatewy(subnet: VpcPublicSubnet, natSubnet: string | undefined, maxNats: number, natsCreated: number): boolean { + const name = subnetName(subnet); + if (natSubnet !== undefined && natSubnet !== name) { + return false; + } + if (natsCreated >= maxNats) { + return false; + } + return true; } diff --git a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts index f571bfb6d7e48..006de959a33ea 100644 --- a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts +++ b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts @@ -171,7 +171,7 @@ export = { const stack = getTestStack(); new VpcNetwork(stack, 'TheVPC', { cidr: '10.0.0.0/21', - natGateways: 2, + natGateways: { gatewayCount: 2}, subnetConfiguration: [ { cidrMask: 24, @@ -251,17 +251,76 @@ export = { }, "with natGateway set to 1"(test: Test) { const stack = getTestStack(); - new VpcNetwork(stack, 'VPC', { natGateways: 1 }); + new VpcNetwork(stack, 'VPC', { + natGateways: { gatewayCount: 1 } + }); expect(stack).to(countResources("AWS::EC2::Subnet", 6)); expect(stack).to(countResources("AWS::EC2::Route", 6)); - expect(stack).to(countResources("AWS::EC2::Subnet", 6)); expect(stack).to(countResources("AWS::EC2::NatGateway", 1)); expect(stack).to(haveResource("AWS::EC2::Route", { DestinationCidrBlock: '0.0.0.0/0', NatGatewayId: { }, })); test.done(); - } + }, + 'with natGateway subnets defined'(test: Test) { + const stack = getTestStack(); + new VpcNetwork(stack, 'VPC', { + subnetConfiguration: [ + { + cidrMask: 24, + name: 'ingress', + subnetType: SubnetType.Public, + }, + { + cidrMask: 24, + name: 'egress', + subnetType: SubnetType.Public, + }, + { + cidrMask: 24, + name: 'private', + subnetType: SubnetType.Private, + }, + ], + natGateways: { + subnetName: 'egress', + }, + }); + expect(stack).to(countResources("AWS::EC2::NatGateway", 3)); + for (let i = 1; i < 4; i++) { + expect(stack).to(haveResource("AWS::EC2::NatGateway", { + Tags: [ + { + Key: 'Name', + Value: `VPC/egressSubnet${i}`, + } + ] + })); + } + test.done(); + }, + 'with mis-matched nat and subnet configs it throws'(test: Test) { + const stack = getTestStack(); + test.throws(() => new VpcNetwork(stack, 'VPC', { + subnetConfiguration: [ + { + cidrMask: 24, + name: 'ingress', + subnetType: SubnetType.Public, + }, + { + cidrMask: 24, + name: 'private', + subnetType: SubnetType.Private, + }, + ], + natGateways: { + subnetName: 'notthere', + }, + })); + test.done(); + }, }, From ef3e4a04955f7b2663478b0d58967879bca82a05 Mon Sep 17 00:00:00 2001 From: Mike Cowgill Date: Tue, 9 Oct 2018 11:22:48 -0700 Subject: [PATCH 2/3] feat(aws-ec2) Configure NAT to Subnet Placement (#874) * closes #741 * Enable NAT placement by `VpcPlacementStrategy` * Add `natGatewayPlacement` to `VpcNeworkProps` * Refactor NAT creation logic into one place --- packages/@aws-cdk/aws-ec2/lib/vpc.ts | 107 +++++++++------------ packages/@aws-cdk/aws-ec2/test/test.vpc.ts | 10 +- 2 files changed, 52 insertions(+), 65 deletions(-) diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc.ts b/packages/@aws-cdk/aws-ec2/lib/vpc.ts index de5ff5844f13d..a53837f30abee 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc.ts @@ -1,8 +1,8 @@ import cdk = require('@aws-cdk/cdk'); import { cloudformation } from './ec2.generated'; import { NetworkBuilder } from './network-util'; -import { DEFAULT_SUBNET_NAME, subnetId, subnetName } from './util'; -import { SubnetType, VpcNetworkRef, VpcSubnetRef } from './vpc-ref'; +import { DEFAULT_SUBNET_NAME, subnetId } from './util'; +import { SubnetType, VpcNetworkRef, VpcPlacementStrategy, VpcSubnetRef } from './vpc-ref'; /** * Name tag constant @@ -61,9 +61,23 @@ export interface VpcNetworkProps { maxAZs?: number; /** - * Define the NAT Gateway Configuration for this VPC + * The number of NAT Gateways to create. + * + * For example, if set this to 1 and your subnet configuration is for 3 Public subnets then only + * one of the Public subnets will have a gateway and all Private subnets will route to this NAT Gateway. + * @default maxAZs */ - natGateways?: NatGatewayConfiguration; + natGateways?: number; + + /** + * Configures the subnets which will have NAT Gateways + * + * The names of the corresponding subnets in `SubnetConfiguration` that will + * have a NAT Gateway. If a corresponding subnet name is not found this will + * throw an error. By default the first public subnets will receive NAT + * Gateways until the `natGateways` is reached. + */ + natGatewayPlacement?: VpcPlacementStrategy; /** * Configure the subnets to build for each AZ @@ -116,27 +130,6 @@ export enum DefaultInstanceTenancy { Dedicated = 'dedicated' } -export interface NatGatewayConfiguration { - /** - * The number of NAT Gateways to create. - * - * For example, if set this to 1 and your subnet configuration is for 3 Public subnets then only - * one of the Public subnets will have a gateway and all Private subnets will route to this NAT Gateway. - * @default maxAZs - */ - gatewayCount?: number; - - /** - * The names of the subnets that will have NAT Gateways - * - * The names of the corresponding subnets in `SubnetConfiguration` that will - * have a NAT Gateway. If a corresponding subnet name is not found this will - * throw an error. By default the first public subnets will receive NAT - * Gateways until the `gatewayCount` is reached. - */ - subnetName?: string; -} - /** * Specify configuration parameters for a VPC to be built */ @@ -308,9 +301,6 @@ export class VpcNetwork extends VpcNetworkRef implements cdk.ITaggable { this.dependencyElements.push(this.resource); this.subnetConfiguration = ifUndefined(props.subnetConfiguration, VpcNetwork.DEFAULT_SUBNETS); - const useNatGateway = this.subnetConfiguration.filter( - subnet => (subnet.subnetType === SubnetType.Private)).length > 0; - // subnetConfiguration and natGateways must be set before calling createSubnets this.createSubnets(); @@ -328,31 +318,13 @@ export class VpcNetwork extends VpcNetworkRef implements cdk.ITaggable { }); this.dependencyElements.push(igw, att); - const natConfig = ifUndefined(props.natGateways, { - gatewayCount: undefined, - subnetName: undefined, - }); - const natCount = ifUndefined(natConfig.gatewayCount, - useNatGateway ? this.availabilityZones.length : 0); - const natSubnet = natConfig.subnetName; - - if (natSubnet !== undefined) { - const subnetNames = (this.publicSubnets as VpcPublicSubnet[]).map( subnet => { - return subnetName(subnet); - }); - if (!subnetNames.includes(natSubnet)) { - throw new Error(`NatGatewayConfiguration contains subnet name ${natSubnet} which is not a Public Subnet in SubnetConfiguration`); - } - } - (this.publicSubnets as VpcPublicSubnet[]).forEach(publicSubnet => { publicSubnet.addDefaultIGWRouteEntry(igw.ref); - const currentNatCount = Object.values(this.natGatewayByAZ).length; - if (addNatGatewy(publicSubnet, natSubnet, natCount, currentNatCount)) { - this.natGatewayByAZ[publicSubnet.availabilityZone] = publicSubnet.addNatGateway(); - } }); + // if gateways are needed create them + this.createNatGateways(props.natGateways, props.natGatewayPlacement); + (this.privateSubnets as VpcPrivateSubnet[]).forEach((privateSubnet, i) => { let ngwId = this.natGatewayByAZ[privateSubnet.availabilityZone]; if (ngwId === undefined) { @@ -372,6 +344,32 @@ export class VpcNetwork extends VpcNetworkRef implements cdk.ITaggable { return this.resource.getAtt("CidrBlock").toString(); } + private createNatGateways(gateways?: number, placement?: VpcPlacementStrategy): void { + const useNatGateway = this.subnetConfiguration.filter( + subnet => (subnet.subnetType === SubnetType.Private)).length > 0; + + const natCount = ifUndefined(gateways, + useNatGateway ? this.availabilityZones.length : 0); + + let natSubnets: VpcPublicSubnet[]; + if (placement) { + const subnets = this.subnets(placement); + for (const sub of subnets) { + if (!this.isPublicSubnet(sub)) { + throw new Error(`natGatewayPlacement ${placement} contains non public subnet ${sub}`); + } + } + natSubnets = subnets as VpcPublicSubnet[]; + } else { + natSubnets = this.publicSubnets as VpcPublicSubnet[]; + } + + natSubnets = natSubnets.slice(0, natCount); + for (const sub of natSubnets) { + this.natGatewayByAZ[sub.availabilityZone] = sub.addNatGateway(); + } + } + /** * createSubnets creates the subnets specified by the subnet configuration * array or creates the `DEFAULT_SUBNETS` configuration @@ -583,14 +581,3 @@ export class VpcPrivateSubnet extends VpcSubnet { function ifUndefined(value: T | undefined, defaultValue: T): T { return value !== undefined ? value : defaultValue; } - -function addNatGatewy(subnet: VpcPublicSubnet, natSubnet: string | undefined, maxNats: number, natsCreated: number): boolean { - const name = subnetName(subnet); - if (natSubnet !== undefined && natSubnet !== name) { - return false; - } - if (natsCreated >= maxNats) { - return false; - } - return true; -} diff --git a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts index 006de959a33ea..403563c2875f5 100644 --- a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts +++ b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts @@ -171,7 +171,7 @@ export = { const stack = getTestStack(); new VpcNetwork(stack, 'TheVPC', { cidr: '10.0.0.0/21', - natGateways: { gatewayCount: 2}, + natGateways: 2, subnetConfiguration: [ { cidrMask: 24, @@ -252,7 +252,7 @@ export = { "with natGateway set to 1"(test: Test) { const stack = getTestStack(); new VpcNetwork(stack, 'VPC', { - natGateways: { gatewayCount: 1 } + natGateways: 1, }); expect(stack).to(countResources("AWS::EC2::Subnet", 6)); expect(stack).to(countResources("AWS::EC2::Route", 6)); @@ -283,8 +283,8 @@ export = { subnetType: SubnetType.Private, }, ], - natGateways: { - subnetName: 'egress', + natGatewayPlacement: { + subnetName: 'egress' }, }); expect(stack).to(countResources("AWS::EC2::NatGateway", 3)); @@ -315,7 +315,7 @@ export = { subnetType: SubnetType.Private, }, ], - natGateways: { + natGatewayPlacement: { subnetName: 'notthere', }, })); From 83f61e19eef5ccb99114b765fd3aafda91fd1db1 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Wed, 10 Oct 2018 10:03:08 +0200 Subject: [PATCH 3/3] Update nat gateway placement docs --- packages/@aws-cdk/aws-ec2/lib/vpc.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc.ts b/packages/@aws-cdk/aws-ec2/lib/vpc.ts index a53837f30abee..41bc9b97bcfdc 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc.ts @@ -72,10 +72,10 @@ export interface VpcNetworkProps { /** * Configures the subnets which will have NAT Gateways * - * The names of the corresponding subnets in `SubnetConfiguration` that will - * have a NAT Gateway. If a corresponding subnet name is not found this will - * throw an error. By default the first public subnets will receive NAT - * Gateways until the `natGateways` is reached. + * You can pick a specific group of subnets by specifying the group name; + * the picked subnets must be public subnets. + * + * @default All public subnets */ natGatewayPlacement?: VpcPlacementStrategy;