From 0de2206f8ba324fbfe0a3685eb5a406d17f8a422 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 21 Mar 2019 12:04:32 +0100 Subject: [PATCH] fix(ec2): descriptive error message when selecting 0 subnets (#2025) Selecting 0 subnets is invalid for most CFN resources. We move the check for this upstream to the VPC construct, which can throw a nicely descriptive error message if it happens to not contain any subnets that match the type we're looking for. The alternative is that the error happens during CloudFormation deployment, but then it's not very clear anymore what the cause for the error was. Fixes #2011. BREAKING CHANGE: `vpcPlacement` has been renamed to `vpcSubnets` on all objects, `subnetsToUse` has been renamed to `subnetType`. `natGatewayPlacement` has been renamed to `natGatewaySubnets`. --- .../aws-autoscaling/lib/auto-scaling-group.ts | 5 +- packages/@aws-cdk/aws-ec2/README.md | 2 +- packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts | 144 ++++++++++++++---- packages/@aws-cdk/aws-ec2/lib/vpc.ts | 8 +- .../test/integ.import-default-vpc.lit.ts | 4 +- packages/@aws-cdk/aws-ec2/test/test.vpc.ts | 126 +++++++++------ .../@aws-cdk/aws-ecs/lib/base/base-service.ts | 9 +- .../@aws-cdk/aws-ecs/lib/ec2/ec2-service.ts | 8 +- .../aws-ecs/lib/fargate/fargate-service.ts | 4 +- .../aws-ecs/test/ec2/test.ec2-service.ts | 12 +- packages/@aws-cdk/aws-eks/lib/cluster.ts | 18 +-- .../test/example.ssh-into-nodes.lit.ts | 2 +- .../lib/shared/base-load-balancer.ts | 12 +- packages/@aws-cdk/aws-lambda/lib/function.ts | 17 ++- .../aws-lambda/test/test.vpc-lambda.ts | 2 +- packages/@aws-cdk/aws-rds/README.md | 6 +- packages/@aws-cdk/aws-rds/lib/cluster.ts | 22 +-- packages/@aws-cdk/aws-rds/lib/props.ts | 2 +- .../aws-rds/lib/rotation-single-user.ts | 6 +- .../@aws-cdk/aws-rds/test/integ.cluster.ts | 2 +- 20 files changed, 270 insertions(+), 141 deletions(-) diff --git a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts index 3b5962c1dcb9a..697862ddf6bcd 100644 --- a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts +++ b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts @@ -53,7 +53,7 @@ export interface CommonAutoScalingGroupProps { /** * Where to place instances within the VPC */ - vpcPlacement?: ec2.VpcPlacementStrategy; + vpcSubnets?: ec2.SubnetSelection; /** * SNS topic to send notifications about fleet changes @@ -291,8 +291,7 @@ export class AutoScalingGroup extends cdk.Construct implements IAutoScalingGroup }); } - const subnets = props.vpc.subnets(props.vpcPlacement); - asgProps.vpcZoneIdentifier = subnets.map(n => n.subnetId); + asgProps.vpcZoneIdentifier = props.vpc.subnetIds(props.vpcSubnets); this.autoScalingGroup = new CfnAutoScalingGroup(this, 'ASG', asgProps); this.osType = machineImage.os.type; diff --git a/packages/@aws-cdk/aws-ec2/README.md b/packages/@aws-cdk/aws-ec2/README.md index 26a04b5a6d5df..f5b310ddad0e7 100644 --- a/packages/@aws-cdk/aws-ec2/README.md +++ b/packages/@aws-cdk/aws-ec2/README.md @@ -21,7 +21,7 @@ instances for your project. Our default `VpcNetwork` class creates a private and public subnet for every availability zone. Classes that use the VPC will generally launch instances -into all private subnets, and provide a parameter called `vpcPlacement` to +into all private subnets, and provide a parameter called `vpcSubnets` to allow you to override the placement. [Read more about subnets](https://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/VPC_Subnets.html). diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts b/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts index 8ac6acad7fd63..69972d73fcc8d 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts @@ -1,5 +1,5 @@ import { Construct, IConstruct, IDependable } from "@aws-cdk/cdk"; -import { subnetName } from './util'; +import { DEFAULT_SUBNET_NAME, subnetName } from './util'; import { VpnConnection, VpnConnectionOptions } from './vpn'; export interface IVpcSubnet extends IConstruct { @@ -61,9 +61,20 @@ export interface IVpcNetwork extends IConstruct { readonly vpnGatewayId?: string; /** - * Return the subnets appropriate for the placement strategy + * Return IDs of the subnets appropriate for the given selection strategy + * + * Requires that at least once subnet is matched, throws a descriptive + * error message otherwise. + * + * Prefer to use this method over {@link subnets} if you need to pass subnet + * IDs to a CloudFormation Resource. */ - subnets(placement?: VpcPlacementStrategy): IVpcSubnet[]; + subnetIds(selection?: SubnetSelection): string[]; + + /** + * Return a dependable object representing internet connectivity for the given subnets + */ + subnetInternetDependencies(selection?: SubnetSelection): IDependable; /** * Return whether the given subnet is one of this VPC's public subnets. @@ -125,29 +136,29 @@ export enum SubnetType { } /** - * Customize how instances are placed inside a VPC + * Customize subnets that are selected for placement of ENIs * * Constructs that allow customization of VPC placement use parameters of this * type to provide placement settings. * * By default, the instances are placed in the private subnets. */ -export interface VpcPlacementStrategy { +export interface SubnetSelection { /** * Place the instances in the subnets of the given type * - * At most one of `subnetsToUse` and `subnetName` can be supplied. + * At most one of `subnetType` and `subnetName` can be supplied. * * @default SubnetType.Private */ - subnetsToUse?: SubnetType; + subnetType?: SubnetType; /** * Place the instances in the subnets with the given name * * (This is the name supplied in subnetConfiguration). * - * At most one of `subnetsToUse` and `subnetName` can be supplied. + * At most one of `subnetType` and `subnetName` can be supplied. * * @default name */ @@ -200,31 +211,30 @@ export abstract class VpcNetworkBase extends Construct implements IVpcNetwork { public readonly natDependencies = new Array(); /** - * Return the subnets appropriate for the placement strategy + * Returns IDs of selected subnets */ - public subnets(placement: VpcPlacementStrategy = {}): IVpcSubnet[] { - if (placement.subnetsToUse !== undefined && placement.subnetName !== undefined) { - throw new Error('At most one of subnetsToUse and subnetName can be supplied'); - } + public subnetIds(selection: SubnetSelection = {}): string[] { + selection = reifySelectionDefaults(selection); - // Select by name - if (placement.subnetName !== undefined) { - const allSubnets = this.privateSubnets.concat(this.publicSubnets).concat(this.isolatedSubnets); - const selectedSubnets = allSubnets.filter(s => subnetName(s) === placement.subnetName); - if (selectedSubnets.length === 0) { - throw new Error(`No subnets with name: ${placement.subnetName}`); - } - return selectedSubnets; + const nets = this.subnets(selection); + if (nets.length === 0) { + throw new Error(`There are no ${describeSelection(selection)} in this VPC. Use a different VPC subnet selection.`); } - // Select by type - if (placement.subnetsToUse === undefined) { return this.privateSubnets; } + return nets.map(n => n.subnetId); + } - return { - [SubnetType.Isolated]: this.isolatedSubnets, - [SubnetType.Private]: this.privateSubnets, - [SubnetType.Public]: this.publicSubnets, - }[placement.subnetsToUse]; + /** + * Return a dependable object representing internet connectivity for the given subnets + */ + public subnetInternetDependencies(selection: SubnetSelection = {}): IDependable { + selection = reifySelectionDefaults(selection); + + const ret = new CompositeDependable(); + for (const subnet of this.subnets(selection)) { + ret.add(subnet.internetConnectivityEstablished); + } + return ret; } /** @@ -260,6 +270,31 @@ export abstract class VpcNetworkBase extends Construct implements IVpcNetwork { return this.node.stack.region; } + /** + * Return the subnets appropriate for the placement strategy + */ + protected subnets(selection: SubnetSelection = {}): IVpcSubnet[] { + selection = reifySelectionDefaults(selection); + + // Select by name + if (selection.subnetName !== undefined) { + const allSubnets = this.privateSubnets.concat(this.publicSubnets).concat(this.isolatedSubnets); + const selectedSubnets = allSubnets.filter(s => subnetName(s) === selection.subnetName); + if (selectedSubnets.length === 0) { + throw new Error(`No subnets with name: ${selection.subnetName}`); + } + return selectedSubnets; + } + + // Select by type + if (selection.subnetType === undefined) { return this.privateSubnets; } + + return { + [SubnetType.Isolated]: this.isolatedSubnets, + [SubnetType.Private]: this.privateSubnets, + [SubnetType.Public]: this.publicSubnets, + }[selection.subnetType]; + } } /** @@ -335,3 +370,56 @@ export interface VpcSubnetImportProps { */ subnetId: string; } + +/** + * If the placement strategy is completely "default", reify the defaults so + * consuming code doesn't have to reimplement the same analysis every time. + * + * Returns "private subnets" by default. + */ +function reifySelectionDefaults(placement: SubnetSelection): SubnetSelection { + if (placement.subnetType !== undefined && placement.subnetName !== undefined) { + throw new Error('Only one of subnetType and subnetName can be supplied'); + } + + if (placement.subnetType === undefined && placement.subnetName === undefined) { + return { subnetType: SubnetType.Private }; + } + + return placement; +} + +/** + * Describe the given placement strategy + */ +function describeSelection(placement: SubnetSelection): string { + if (placement.subnetType !== undefined) { + return `'${DEFAULT_SUBNET_NAME[placement.subnetType]}' subnets`; + } + if (placement.subnetName !== undefined) { + return `subnets named '${placement.subnetName}'`; + } + return JSON.stringify(placement); +} + +class CompositeDependable implements IDependable { + private readonly dependables = new Array(); + + /** + * Add a construct to the dependency roots + */ + public add(dep: IDependable) { + this.dependables.push(dep); + } + + /** + * Retrieve the current set of dependency roots + */ + public get dependencyRoots(): IConstruct[] { + const ret = []; + for (const dep of this.dependables) { + ret.push(...dep.dependencyRoots); + } + return ret; + } +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc.ts b/packages/@aws-cdk/aws-ec2/lib/vpc.ts index 3d74cb89479c1..9465fdcf6dfc7 100644 --- a/packages/@aws-cdk/aws-ec2/lib/vpc.ts +++ b/packages/@aws-cdk/aws-ec2/lib/vpc.ts @@ -5,7 +5,7 @@ import { CfnRouteTable, CfnSubnet, CfnSubnetRouteTableAssociation, CfnVPC, CfnVP import { NetworkBuilder } from './network-util'; import { DEFAULT_SUBNET_NAME, ExportSubnetGroup, ImportSubnetGroup, subnetId } from './util'; import { VpcNetworkProvider, VpcNetworkProviderProps } from './vpc-network-provider'; -import { IVpcNetwork, IVpcSubnet, SubnetType, VpcNetworkBase, VpcNetworkImportProps, VpcPlacementStrategy, VpcSubnetImportProps } from './vpc-ref'; +import { IVpcNetwork, IVpcSubnet, SubnetSelection, SubnetType, VpcNetworkBase, VpcNetworkImportProps, VpcSubnetImportProps } from './vpc-ref'; import { VpnConnectionOptions, VpnConnectionType } from './vpn'; /** @@ -80,7 +80,7 @@ export interface VpcNetworkProps { * * @default All public subnets */ - natGatewayPlacement?: VpcPlacementStrategy; + natGatewaySubnets?: SubnetSelection; /** * Configure the subnets to build for each AZ @@ -365,7 +365,7 @@ export class VpcNetwork extends VpcNetworkBase { }); // if gateways are needed create them - this.createNatGateways(props.natGateways, props.natGatewayPlacement); + this.createNatGateways(props.natGateways, props.natGatewaySubnets); (this.privateSubnets as VpcPrivateSubnet[]).forEach((privateSubnet, i) => { let ngwId = this.natGatewayByAZ[privateSubnet.availabilityZone]; @@ -445,7 +445,7 @@ export class VpcNetwork extends VpcNetworkBase { }; } - private createNatGateways(gateways?: number, placement?: VpcPlacementStrategy): void { + private createNatGateways(gateways?: number, placement?: SubnetSelection): void { const useNatGateway = this.subnetConfiguration.filter( subnet => (subnet.subnetType === SubnetType.Private)).length > 0; diff --git a/packages/@aws-cdk/aws-ec2/test/integ.import-default-vpc.lit.ts b/packages/@aws-cdk/aws-ec2/test/integ.import-default-vpc.lit.ts index 981929957583c..9208c3eaf09bd 100644 --- a/packages/@aws-cdk/aws-ec2/test/integ.import-default-vpc.lit.ts +++ b/packages/@aws-cdk/aws-ec2/test/integ.import-default-vpc.lit.ts @@ -18,7 +18,7 @@ new ec2.SecurityGroup(stack, 'SecurityGroup', { }); // Try subnet selection -new cdk.CfnOutput(stack, 'PublicSubnets', { value: 'ids:' + vpc.subnets({ subnetsToUse: ec2.SubnetType.Public }).map(s => s.subnetId).join(',') }); -new cdk.CfnOutput(stack, 'PrivateSubnets', { value: 'ids:' + vpc.subnets({ subnetsToUse: ec2.SubnetType.Private }).map(s => s.subnetId).join(',') }); +new cdk.CfnOutput(stack, 'PublicSubnets', { value: 'ids:' + vpc.publicSubnets.map(s => s.subnetId).join(',') }); +new cdk.CfnOutput(stack, 'PrivateSubnets', { value: 'ids:' + vpc.privateSubnets.map(s => s.subnetId).join(',') }); app.run(); diff --git a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts index 23a3b79056506..0a7dc1e4f1607 100644 --- a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts +++ b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts @@ -289,7 +289,7 @@ export = { subnetType: SubnetType.Private, }, ], - natGatewayPlacement: { + natGatewaySubnets: { subnetName: 'egress' }, }); @@ -317,7 +317,7 @@ export = { subnetType: SubnetType.Private, }, ], - natGatewayPlacement: { + natGatewaySubnets: { subnetName: 'notthere', }, })); @@ -526,55 +526,86 @@ export = { }, }, - 'can select public subnets'(test: Test) { - // GIVEN - const stack = getTestStack(); - const vpc = new VpcNetwork(stack, 'VPC'); + 'subnet selection': { + 'selecting default subnets returns the private ones'(test: Test) { + // GIVEN + const stack = getTestStack(); + const vpc = new VpcNetwork(stack, 'VPC'); + + // WHEN + const nets = vpc.subnetIds(); + + // THEN + test.deepEqual(nets, vpc.privateSubnets.map(s => s.subnetId)); + test.done(); + }, - // WHEN - const nets = vpc.subnets({ subnetsToUse: SubnetType.Public }); + 'can select public subnets'(test: Test) { + // GIVEN + const stack = getTestStack(); + const vpc = new VpcNetwork(stack, 'VPC'); - // THEN - test.deepEqual(nets, vpc.publicSubnets); + // WHEN + const nets = vpc.subnetIds({ subnetType: SubnetType.Public }); - test.done(); - }, + // THEN + test.deepEqual(nets, vpc.publicSubnets.map(s => s.subnetId)); - 'can select isolated subnets'(test: Test) { - // GIVEN - const stack = getTestStack(); - const vpc = new VpcNetwork(stack, 'VPC', { - subnetConfiguration: [ - { subnetType: SubnetType.Private, name: 'Private' }, - { subnetType: SubnetType.Isolated, name: 'Isolated' }, - ] - }); + test.done(); + }, - // WHEN - const nets = vpc.subnets({ subnetsToUse: SubnetType.Isolated }); + 'can select isolated subnets'(test: Test) { + // GIVEN + const stack = getTestStack(); + const vpc = new VpcNetwork(stack, 'VPC', { + subnetConfiguration: [ + { subnetType: SubnetType.Private, name: 'Private' }, + { subnetType: SubnetType.Isolated, name: 'Isolated' }, + ] + }); - // THEN - test.deepEqual(nets, vpc.isolatedSubnets); + // WHEN + const nets = vpc.subnetIds({ subnetType: SubnetType.Isolated }); - test.done(); - }, + // THEN + test.deepEqual(nets, vpc.isolatedSubnets.map(s => s.subnetId)); + + test.done(); + }, + + 'can select subnets by name'(test: Test) { + // GIVEN + const stack = getTestStack(); + const vpc = new VpcNetwork(stack, 'VPC', { + subnetConfiguration: [ + { subnetType: SubnetType.Private, name: 'DontTalkToMe' }, + { subnetType: SubnetType.Isolated, name: 'DontTalkAtAll' }, + ] + }); + + // WHEN + const nets = vpc.subnetIds({ subnetName: 'DontTalkToMe' }); - 'can select subnets by name'(test: Test) { - // GIVEN - const stack = getTestStack(); - const vpc = new VpcNetwork(stack, 'VPC', { - subnetConfiguration: [ - { subnetType: SubnetType.Private, name: 'DontTalkToMe' }, - { subnetType: SubnetType.Isolated, name: 'DontTalkAtAll' }, - ] - }); - - // WHEN - const nets = vpc.subnets({ subnetName: 'DontTalkToMe' }); - - // THEN - test.deepEqual(nets, vpc.privateSubnets); - test.done(); + // THEN + test.deepEqual(nets, vpc.privateSubnets.map(s => s.subnetId)); + test.done(); + }, + + 'selecting default subnets in a VPC with only public subnets throws an error'(test: Test) { + // GIVEN + const stack = new Stack(); + const vpc = VpcNetwork.import(stack, 'VPC', { + vpcId: 'vpc-1234', + availabilityZones: ['dummy1a', 'dummy1b', 'dummy1c'], + publicSubnetIds: ['pub-1', 'pub-2', 'pub-3'], + }); + + test.throws(() => { + vpc.subnetIds(); + }, /There are no 'Private' subnets in this VPC/); + + test.done(); + } }, 'export/import': { @@ -634,11 +665,11 @@ export = { }); // WHEN - const nets = importedVpc.subnets({ subnetsToUse: SubnetType.Isolated }); + const nets = importedVpc.subnetIds({ subnetType: SubnetType.Isolated }); // THEN test.equal(3, importedVpc.isolatedSubnets.length); - test.deepEqual(nets, importedVpc.isolatedSubnets); + test.deepEqual(nets, importedVpc.isolatedSubnets.map(s => s.subnetId)); test.done(); }, @@ -657,16 +688,17 @@ export = { }); // WHEN - const nets = importedVpc.subnets({ subnetName: isolatedName }); + const nets = importedVpc.subnetIds({ subnetName: isolatedName }); // THEN test.equal(3, importedVpc.isolatedSubnets.length); - test.deepEqual(nets, importedVpc.isolatedSubnets); + test.deepEqual(nets, importedVpc.isolatedSubnets.map(s => s.subnetId)); } test.done(); }, }, + }; function getTestStack(): Stack { diff --git a/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts b/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts index f6811d72eec3f..0ce286a03d0e6 100644 --- a/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts +++ b/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts @@ -177,20 +177,19 @@ export abstract class BaseService extends cdk.Construct * Set up AWSVPC networking for this construct */ // tslint:disable-next-line:max-line-length - protected configureAwsVpcNetworking(vpc: ec2.IVpcNetwork, assignPublicIp?: boolean, vpcPlacement?: ec2.VpcPlacementStrategy, securityGroup?: ec2.ISecurityGroup) { - if (vpcPlacement === undefined) { - vpcPlacement = { subnetsToUse: assignPublicIp ? ec2.SubnetType.Public : ec2.SubnetType.Private }; + protected configureAwsVpcNetworking(vpc: ec2.IVpcNetwork, assignPublicIp?: boolean, vpcSubnets?: ec2.SubnetSelection, securityGroup?: ec2.ISecurityGroup) { + if (vpcSubnets === undefined) { + vpcSubnets = { subnetType: assignPublicIp ? ec2.SubnetType.Public : ec2.SubnetType.Private }; } if (securityGroup === undefined) { securityGroup = new ec2.SecurityGroup(this, 'SecurityGroup', { vpc }); } - const subnets = vpc.subnets(vpcPlacement); this.connections.addSecurityGroup(securityGroup); this.networkConfiguration = { awsvpcConfiguration: { assignPublicIp: assignPublicIp ? 'ENABLED' : 'DISABLED', - subnets: subnets.map(x => x.subnetId), + subnets: vpc.subnetIds(vpcSubnets), securityGroups: new cdk.Token(() => [securityGroup!.securityGroupId]).toList(), } }; diff --git a/packages/@aws-cdk/aws-ecs/lib/ec2/ec2-service.ts b/packages/@aws-cdk/aws-ecs/lib/ec2/ec2-service.ts index 80628ad98ccb6..1a20739cc4362 100644 --- a/packages/@aws-cdk/aws-ecs/lib/ec2/ec2-service.ts +++ b/packages/@aws-cdk/aws-ecs/lib/ec2/ec2-service.ts @@ -29,7 +29,7 @@ export interface Ec2ServiceProps extends BaseServiceProps { * * @default Private subnets */ - vpcPlacement?: ec2.VpcPlacementStrategy; + vpcSubnets?: ec2.SubnetSelection; /** * Existing security group to use for the task's ENIs @@ -102,7 +102,7 @@ export class Ec2Service extends BaseService implements elb.ILoadBalancerTarget { this.daemon = props.daemon || false; if (props.taskDefinition.networkMode === NetworkMode.AwsVpc) { - this.configureAwsVpcNetworking(props.cluster.vpc, false, props.vpcPlacement, props.securityGroup); + this.configureAwsVpcNetworking(props.cluster.vpc, false, props.vpcSubnets, props.securityGroup); } else { // Either None, Bridge or Host networking. Copy SecurityGroup from ASG. validateNoNetworkingProps(props); @@ -244,8 +244,8 @@ export class Ec2Service extends BaseService implements elb.ILoadBalancerTarget { * Validate combinations of networking arguments */ function validateNoNetworkingProps(props: Ec2ServiceProps) { - if (props.vpcPlacement !== undefined || props.securityGroup !== undefined) { - throw new Error('vpcPlacement and securityGroup can only be used in AwsVpc networking mode'); + if (props.vpcSubnets !== undefined || props.securityGroup !== undefined) { + throw new Error('vpcSubnets and securityGroup can only be used in AwsVpc networking mode'); } } diff --git a/packages/@aws-cdk/aws-ecs/lib/fargate/fargate-service.ts b/packages/@aws-cdk/aws-ecs/lib/fargate/fargate-service.ts index 718102faf0909..5a146d1fca2b1 100644 --- a/packages/@aws-cdk/aws-ecs/lib/fargate/fargate-service.ts +++ b/packages/@aws-cdk/aws-ecs/lib/fargate/fargate-service.ts @@ -31,7 +31,7 @@ export interface FargateServiceProps extends BaseServiceProps { * * @default Private subnet if assignPublicIp, public subnets otherwise */ - vpcPlacement?: ec2.VpcPlacementStrategy; + vpcSubnets?: ec2.SubnetSelection; /** * Existing security group to use for the tasks @@ -70,7 +70,7 @@ export class FargateService extends BaseService { platformVersion: props.platformVersion, }, props.cluster.clusterName, props.taskDefinition); - this.configureAwsVpcNetworking(props.cluster.vpc, props.assignPublicIp, props.vpcPlacement, props.securityGroup); + this.configureAwsVpcNetworking(props.cluster.vpc, props.assignPublicIp, props.vpcSubnets, props.securityGroup); if (!props.taskDefinition.defaultContainer) { throw new Error('A TaskDefinition must have at least one essential container'); diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/test.ec2-service.ts b/packages/@aws-cdk/aws-ecs/test/ec2/test.ec2-service.ts index 3712676bbcb36..45eb699e9785e 100644 --- a/packages/@aws-cdk/aws-ecs/test/ec2/test.ec2-service.ts +++ b/packages/@aws-cdk/aws-ecs/test/ec2/test.ec2-service.ts @@ -147,7 +147,7 @@ export = { }, "with a TaskDefinition with Bridge network mode": { - "it errors if vpcPlacement is specified"(test: Test) { + "it errors if vpcSubnets is specified"(test: Test) { // GIVEN const stack = new cdk.Stack(); const vpc = new ec2.VpcNetwork(stack, 'MyVpc', {}); @@ -167,8 +167,8 @@ export = { new ecs.Ec2Service(stack, "Ec2Service", { cluster, taskDefinition, - vpcPlacement: { - subnetsToUse: ec2.SubnetType.Public + vpcSubnets: { + subnetType: ec2.SubnetType.Public } }); }); @@ -230,7 +230,7 @@ export = { test.done(); }, - "it allows vpcPlacement"(test: Test) { + "it allows vpcSubnets"(test: Test) { // GIVEN const stack = new cdk.Stack(); const vpc = new ec2.VpcNetwork(stack, 'MyVpc', {}); @@ -248,8 +248,8 @@ export = { new ecs.Ec2Service(stack, "Ec2Service", { cluster, taskDefinition, - vpcPlacement: { - subnetsToUse: ec2.SubnetType.Public + vpcSubnets: { + subnetType: ec2.SubnetType.Public } }); diff --git a/packages/@aws-cdk/aws-eks/lib/cluster.ts b/packages/@aws-cdk/aws-eks/lib/cluster.ts index 46f743b513e55..0992cca1b9e20 100644 --- a/packages/@aws-cdk/aws-eks/lib/cluster.ts +++ b/packages/@aws-cdk/aws-eks/lib/cluster.ts @@ -21,17 +21,19 @@ export interface ClusterProps { * * If you want to create public load balancers, this must include public subnets. * + * @example + * * For example, to only select private subnets, supply the following: * - * ``` - * vpcPlacements: [ - * { subnetsToUse: ec2.SubnetType.Private } + * ```ts + * vpcSubnets: [ + * { subnetType: ec2.SubnetType.Private } * ] * ``` * * @default All public and private subnets */ - vpcPlacements?: ec2.VpcPlacementStrategy[]; + vpcSubnets?: ec2.SubnetSelection[]; /** * Role that provides permissions for the Kubernetes control plane to make calls to AWS API operations on your behalf. @@ -160,8 +162,8 @@ export class Cluster extends ClusterBase { }); // Get subnetIds for all selected subnets - const placements = props.vpcPlacements || [{ subnetsToUse: ec2.SubnetType.Public }, { subnetsToUse: ec2.SubnetType.Private }]; - const subnetIds = flatMap(placements, p => this.vpc.subnets(p)).map(s => s.subnetId); + const placements = props.vpcSubnets || [{ subnetType: ec2.SubnetType.Public }, { subnetType: ec2.SubnetType.Private }]; + const subnetIds = flatMap(placements, p => this.vpc.subnetIds(p)); const resource = new CfnCluster(this, 'Resource', { name: props.clusterName, @@ -264,9 +266,7 @@ export class Cluster extends ClusterBase { * @see https://docs.aws.amazon.com/eks/latest/userguide/network_reqs.html */ private tagSubnets() { - const privates = this.vpc.subnets({ subnetsToUse: ec2.SubnetType.Private }); - - for (const subnet of privates) { + for (const subnet of this.vpc.privateSubnets) { if (!isRealSubnetConstruct(subnet)) { // Just give up, all of them will be the same. this.node.addWarning('Could not auto-tag private subnets with "kubernetes.io/role/internal-elb=1", please remember to do this manually'); diff --git a/packages/@aws-cdk/aws-eks/test/example.ssh-into-nodes.lit.ts b/packages/@aws-cdk/aws-eks/test/example.ssh-into-nodes.lit.ts index 0784d1aca03f5..5d188d352cadf 100644 --- a/packages/@aws-cdk/aws-eks/test/example.ssh-into-nodes.lit.ts +++ b/packages/@aws-cdk/aws-eks/test/example.ssh-into-nodes.lit.ts @@ -15,7 +15,7 @@ class EksClusterStack extends cdk.Stack { /// !show const asg = cluster.addCapacity('Nodes', { instanceType: new ec2.InstanceType('t2.medium'), - vpcPlacement: { subnetsToUse: ec2.SubnetType.Public }, + vpcSubnets: { subnetType: ec2.SubnetType.Public }, keyName: 'my-key-name', }); diff --git a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts index 4ed43405deacf..81902d69ad91a 100644 --- a/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts +++ b/packages/@aws-cdk/aws-elasticloadbalancingv2/lib/shared/base-load-balancer.ts @@ -32,7 +32,7 @@ export interface BaseLoadBalancerProps { * * @default Public subnets if internetFacing, otherwise private subnets */ - vpcPlacement?: ec2.VpcPlacementStrategy; + vpcSubnets?: ec2.SubnetSelection; /** * Indicates whether deletion protection is enabled. @@ -98,20 +98,22 @@ export abstract class BaseLoadBalancer extends cdk.Construct implements route53. const internetFacing = ifUndefined(baseProps.internetFacing, false); - const subnets = baseProps.vpc.subnets(ifUndefined(baseProps.vpcPlacement, - { subnetsToUse: internetFacing ? ec2.SubnetType.Public : ec2.SubnetType.Private })); + const vpcSubnets = ifUndefined(baseProps.vpcSubnets, + { subnetType: internetFacing ? ec2.SubnetType.Public : ec2.SubnetType.Private }); + + const subnets = baseProps.vpc.subnetIds(vpcSubnets); this.vpc = baseProps.vpc; const resource = new CfnLoadBalancer(this, 'Resource', { name: baseProps.loadBalancerName, - subnets: subnets.map(s => s.subnetId), + subnets, scheme: internetFacing ? 'internet-facing' : 'internal', loadBalancerAttributes: new cdk.Token(() => renderAttributes(this.attributes)), ...additionalProps }); if (internetFacing) { - resource.node.addDependency(...subnets.map(s => s.internetConnectivityEstablished)); + resource.node.addDependency(baseProps.vpc.subnetInternetDependencies(vpcSubnets)); } if (baseProps.deletionProtection) { this.setAttribute('deletion_protection.enabled', 'true'); } diff --git a/packages/@aws-cdk/aws-lambda/lib/function.ts b/packages/@aws-cdk/aws-lambda/lib/function.ts index 657219c575cfc..1ef51384bc6ca 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function.ts @@ -130,7 +130,7 @@ export interface FunctionProps { * * @default All private subnets */ - vpcPlacement?: ec2.VpcPlacementStrategy; + vpcSubnets?: ec2.SubnetSelection; /** * What security group to associate with the Lambda's network interfaces. @@ -504,15 +504,22 @@ export class Function extends FunctionBase { // Pick subnets, make sure they're not Public. Routing through an IGW // won't work because the ENIs don't get a Public IP. - const subnets = props.vpc.subnets(props.vpcPlacement); - for (const subnet of subnets) { - if (props.vpc.isPublicSubnet(subnet)) { + // Why are we not simply forcing vpcSubnets? Because you might still be choosing + // Isolated networks or selecting among 2 sets of Private subnets by name. + const subnetIds = props.vpc.subnetIds(props.vpcSubnets); + const publicSubnetIds = new Set(props.vpc.publicSubnets.map(s => s.subnetId)); + for (const subnetId of subnetIds) { + if (publicSubnetIds.has(subnetId)) { throw new Error('Not possible to place Lambda Functions in a Public subnet'); } } + // List can't be empty here, if we got this far you intended to put your Lambda + // in subnets. We're going to guarantee that we get the nice error message by + // making VpcNetwork do the selection again. + return { - subnetIds: subnets.map(s => s.subnetId), + subnetIds, securityGroupIds: [securityGroup.securityGroupId] }; } diff --git a/packages/@aws-cdk/aws-lambda/test/test.vpc-lambda.ts b/packages/@aws-cdk/aws-lambda/test/test.vpc-lambda.ts index 878dc3c3efc51..b069c11a8d2cc 100644 --- a/packages/@aws-cdk/aws-lambda/test/test.vpc-lambda.ts +++ b/packages/@aws-cdk/aws-lambda/test/test.vpc-lambda.ts @@ -137,7 +137,7 @@ export = { handler: 'index.handler', runtime: lambda.Runtime.NodeJS610, vpc, - vpcPlacement: { subnetsToUse: ec2.SubnetType.Public } + vpcSubnets: { subnetType: ec2.SubnetType.Public } }); }); diff --git a/packages/@aws-cdk/aws-rds/README.md b/packages/@aws-cdk/aws-rds/README.md index 74c6a8f402e93..606b0c21a9144 100644 --- a/packages/@aws-cdk/aws-rds/README.md +++ b/packages/@aws-cdk/aws-rds/README.md @@ -15,7 +15,7 @@ Not supported: ### Starting a Clustered Database To set up a clustered database (like Aurora), create an instance of `DatabaseCluster`. You must -always launch a database in a VPC. Use the `vpcPlacement` attribute to control whether +always launch a database in a VPC. Use the `vpcSubnets` attribute to control whether your instances will be launched privately or publicly: ```ts @@ -26,8 +26,8 @@ const cluster = new DatabaseCluster(this, 'Database', { }, instanceProps: { instanceType: new InstanceTypePair(InstanceClass.Burstable2, InstanceSize.Small), - vpcPlacement: { - subnetsToUse: ec2.SubnetType.Public, + vpcSubnets: { + subnetType: ec2.SubnetType.Public, }, vpc } diff --git a/packages/@aws-cdk/aws-rds/lib/cluster.ts b/packages/@aws-cdk/aws-rds/lib/cluster.ts index 66fda81a3fac2..39465a1231a5c 100644 --- a/packages/@aws-cdk/aws-rds/lib/cluster.ts +++ b/packages/@aws-cdk/aws-rds/lib/cluster.ts @@ -203,29 +203,29 @@ export class DatabaseCluster extends DatabaseClusterBase implements IDatabaseClu /** * The VPC where the DB subnet group is created. */ - public readonly vpc: ec2.IVpcNetwork; + private readonly vpc: ec2.IVpcNetwork; /** * The subnets used by the DB subnet group. */ - public readonly vpcPlacement?: ec2.VpcPlacementStrategy; + private readonly vpcSubnets?: ec2.SubnetSelection; constructor(scope: cdk.Construct, id: string, props: DatabaseClusterProps) { super(scope, id); this.vpc = props.instanceProps.vpc; - this.vpcPlacement = props.instanceProps.vpcPlacement; + this.vpcSubnets = props.instanceProps.vpcSubnets; - const subnets = this.vpc.subnets(this.vpcPlacement); + const subnetIds = props.instanceProps.vpc.subnetIds(props.instanceProps.vpcSubnets); // Cannot test whether the subnets are in different AZs, but at least we can test the amount. - if (subnets.length < 2) { - throw new Error(`Cluster requires at least 2 subnets, got ${subnets.length}`); + if (subnetIds.length < 2) { + throw new Error(`Cluster requires at least 2 subnets, got ${subnetIds.length}`); } const subnetGroup = new CfnDBSubnetGroup(this, 'Subnets', { dbSubnetGroupDescription: `Subnets for ${id} database`, - subnetIds: subnets.map(s => s.subnetId) + subnetIds, }); const securityGroup = props.instanceProps.securityGroup !== undefined ? @@ -280,6 +280,8 @@ export class DatabaseCluster extends DatabaseClusterBase implements IDatabaseClu throw new Error('At least one instance is required'); } + // Get the actual subnet objects so we can depend on internet connectivity. + const internetConnected = props.instanceProps.vpc.subnetInternetDependencies(props.instanceProps.vpcSubnets); for (let i = 0; i < instanceCount; i++) { const instanceIndex = i + 1; @@ -287,7 +289,7 @@ export class DatabaseCluster extends DatabaseClusterBase implements IDatabaseClu props.clusterIdentifier != null ? `${props.clusterIdentifier}instance${instanceIndex}` : undefined; - const publiclyAccessible = props.instanceProps.vpcPlacement && props.instanceProps.vpcPlacement.subnetsToUse === ec2.SubnetType.Public; + const publiclyAccessible = props.instanceProps.vpcSubnets && props.instanceProps.vpcSubnets.subnetType === ec2.SubnetType.Public; const instance = new CfnDBInstance(this, `Instance${instanceIndex}`, { // Link to cluster @@ -303,7 +305,7 @@ export class DatabaseCluster extends DatabaseClusterBase implements IDatabaseClu // We must have a dependency on the NAT gateway provider here to create // things in the right order. - instance.node.addDependency(...subnets.map(s => s.internetConnectivityEstablished)); + instance.node.addDependency(internetConnected); this.instanceIdentifiers.push(instance.ref); this.instanceEndpoints.push(new Endpoint(instance.dbInstanceEndpointAddress, instance.dbInstanceEndpointPort)); @@ -324,7 +326,7 @@ export class DatabaseCluster extends DatabaseClusterBase implements IDatabaseClu secret: this.secret, engine: toDatabaseEngine(this.engine), vpc: this.vpc, - vpcPlacement: this.vpcPlacement, + vpcSubnets: this.vpcSubnets, target: this, ...options }); diff --git a/packages/@aws-cdk/aws-rds/lib/props.ts b/packages/@aws-cdk/aws-rds/lib/props.ts index 68925721034c1..4816f249cc083 100644 --- a/packages/@aws-cdk/aws-rds/lib/props.ts +++ b/packages/@aws-cdk/aws-rds/lib/props.ts @@ -30,7 +30,7 @@ export interface InstanceProps { /** * Where to place the instances within the VPC */ - vpcPlacement?: ec2.VpcPlacementStrategy; + vpcSubnets?: ec2.SubnetSelection; /** * Security group. If not specified a new one will be created. diff --git a/packages/@aws-cdk/aws-rds/lib/rotation-single-user.ts b/packages/@aws-cdk/aws-rds/lib/rotation-single-user.ts index 5f269534334e3..9a67f5b49904b 100644 --- a/packages/@aws-cdk/aws-rds/lib/rotation-single-user.ts +++ b/packages/@aws-cdk/aws-rds/lib/rotation-single-user.ts @@ -110,7 +110,7 @@ export interface RotationSingleUserProps extends RotationSingleUserOptions { * * @default private subnets */ - vpcPlacement?: ec2.VpcPlacementStrategy; + vpcSubnets?: ec2.SubnetSelection; /** * The target database cluster or instance @@ -139,7 +139,7 @@ export class RotationSingleUser extends cdk.Construct { vpc: props.vpc }); - const subnets = props.vpc.subnets(props.vpcPlacement); + const vpcSubnetIds = props.vpc.subnetIds(props.vpcSubnets); props.target.connections.allowDefaultPortFrom(securityGroup); @@ -149,7 +149,7 @@ export class RotationSingleUser extends cdk.Construct { endpoint: `https://secretsmanager.${this.node.stack.region}.${this.node.stack.urlSuffix}`, functionName: rotationFunctionName, vpcSecurityGroupIds: securityGroup.securityGroupId, - vpcSubnetIds: subnets.map(s => s.subnetId).join(',') + vpcSubnetIds: vpcSubnetIds.join(',') } }); diff --git a/packages/@aws-cdk/aws-rds/test/integ.cluster.ts b/packages/@aws-cdk/aws-rds/test/integ.cluster.ts index 468b1d442ce77..8a787668a2533 100644 --- a/packages/@aws-cdk/aws-rds/test/integ.cluster.ts +++ b/packages/@aws-cdk/aws-rds/test/integ.cluster.ts @@ -24,7 +24,7 @@ const cluster = new DatabaseCluster(stack, 'Database', { }, instanceProps: { instanceType: new ec2.InstanceTypePair(ec2.InstanceClass.Burstable2, ec2.InstanceSize.Small), - vpcPlacement: { subnetsToUse: ec2.SubnetType.Public }, + vpcSubnets: { subnetType: ec2.SubnetType.Public }, vpc }, parameterGroup: params,