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 0134c6b2a0360..405d6352733dd 100644 --- a/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts +++ b/packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts @@ -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.vpcPlacement); this.autoScalingGroup = new CfnAutoScalingGroup(this, 'ASG', asgProps); this.osType = machineImage.os.type; diff --git a/packages/@aws-cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/package-lock.json b/packages/@aws-cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/package-lock.json index e3fb799be9e3a..659ed05dfc00a 100644 --- a/packages/@aws-cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/package-lock.json +++ b/packages/@aws-cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/package-lock.json @@ -1,6 +1,6 @@ { "name": "dns_validated_certificate_handler", - "version": "0.25.2", + "version": "0.25.3", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts b/packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts index 8ac6acad7fd63..26de03ced6ad9 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 { @@ -62,9 +62,23 @@ export interface IVpcNetwork extends IConstruct { /** * Return the subnets appropriate for the placement strategy + * + * Prefer using {@link subnetIds} if you need to pass subnet IDs to a + * CloudFormation Resource. */ subnets(placement?: VpcPlacementStrategy): IVpcSubnet[]; + /** + * Return IDs of the subnets appropriate for the given placement 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. + */ + subnetIds(placement?: VpcPlacementStrategy): string[]; + /** * Return whether the given subnet is one of this VPC's public subnets. * @@ -203,9 +217,7 @@ export abstract class VpcNetworkBase extends Construct implements IVpcNetwork { * Return the subnets appropriate for the placement strategy */ 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'); - } + placement = reifyDefaultsPlacement(placement); // Select by name if (placement.subnetName !== undefined) { @@ -227,6 +239,20 @@ export abstract class VpcNetworkBase extends Construct implements IVpcNetwork { }[placement.subnetsToUse]; } + /** + * Returns IDs of selected subnets + */ + public subnetIds(placement: VpcPlacementStrategy = {}): string[] { + placement = reifyDefaultsPlacement(placement); + + const nets = this.subnets(placement); + if (nets.length === 0) { + throw new Error(`There are no ${describePlacement(placement)} in this VPC. Use a different VPC placement strategy.`); + } + + return nets.map(n => n.subnetId); + } + /** * Adds a new VPN connection to this VPC */ @@ -335,3 +361,34 @@ 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 reifyDefaultsPlacement(placement: VpcPlacementStrategy): VpcPlacementStrategy { + if (placement.subnetsToUse !== undefined && placement.subnetName !== undefined) { + throw new Error('At most one of subnetsToUse and subnetName can be supplied'); + } + + if (placement.subnetsToUse === undefined && placement.subnetName === undefined) { + return { subnetsToUse: SubnetType.Private }; + } + + return placement; +} + +/** + * Describe the given placement strategy + */ +function describePlacement(placement: VpcPlacementStrategy) { + if (placement.subnetsToUse !== undefined) { + return `'${DEFAULT_SUBNET_NAME[placement.subnetsToUse]}' subnets`; + } + if (placement.subnetName !== undefined) { + return `subnets named '${placement.subnetName}'`; + } + return '???'; +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts index 23a3b79056506..d9a4030008df8 100644 --- a/packages/@aws-cdk/aws-ec2/test/test.vpc.ts +++ b/packages/@aws-cdk/aws-ec2/test/test.vpc.ts @@ -666,7 +666,24 @@ export = { test.done(); }, + + 'selecting default subnets in a VPC with only public subnets'(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(); + } }, + }; 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 fa8365cc1d266..e58757989a5d2 100644 --- a/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts +++ b/packages/@aws-cdk/aws-ecs/lib/base/base-service.ts @@ -184,13 +184,12 @@ export abstract class BaseService extends cdk.Construct 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(vpcPlacement), securityGroups: new cdk.Token(() => [securityGroup!.securityGroupId]), } }; diff --git a/packages/@aws-cdk/aws-eks/lib/cluster.ts b/packages/@aws-cdk/aws-eks/lib/cluster.ts index 46f743b513e55..fd41ce7f31667 100644 --- a/packages/@aws-cdk/aws-eks/lib/cluster.ts +++ b/packages/@aws-cdk/aws-eks/lib/cluster.ts @@ -161,7 +161,7 @@ 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 subnetIds = flatMap(placements, p => this.vpc.subnetIds(p)); const resource = new CfnCluster(this, 'Resource', { name: props.clusterName, 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..0dae9334a3ba0 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 @@ -98,20 +98,23 @@ 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 vpcPlacement = ifUndefined(baseProps.vpcPlacement, + { subnetsToUse: internetFacing ? ec2.SubnetType.Public : ec2.SubnetType.Private }); + + const subnets = baseProps.vpc.subnetIds(vpcPlacement); 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)); + const subnetObjs = baseProps.vpc.subnets(vpcPlacement); + resource.node.addDependency(...subnetObjs.map(s => s.internetConnectivityEstablished)); } 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 9d603e43dc860..eb3ff12d216c1 100644 --- a/packages/@aws-cdk/aws-lambda/lib/function.ts +++ b/packages/@aws-cdk/aws-lambda/lib/function.ts @@ -504,6 +504,8 @@ 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. + // Why are we not simply forcing vpcPlacement? Because you might still be choosing + // Isolated networks or selecting among 2 sets of Private subnets by name. const subnets = props.vpc.subnets(props.vpcPlacement); for (const subnet of subnets) { if (props.vpc.isPublicSubnet(subnet)) { @@ -511,8 +513,12 @@ export class Function extends FunctionBase { } } + // 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: props.vpc.subnetIds(props.vpcPlacement), securityGroupIds: [securityGroup.securityGroupId] }; } diff --git a/packages/@aws-cdk/aws-rds/lib/cluster.ts b/packages/@aws-cdk/aws-rds/lib/cluster.ts index 5510bfafebc1c..a78243575083a 100644 --- a/packages/@aws-cdk/aws-rds/lib/cluster.ts +++ b/packages/@aws-cdk/aws-rds/lib/cluster.ts @@ -139,16 +139,16 @@ export class DatabaseCluster extends cdk.Construct implements IDatabaseCluster { constructor(scope: cdk.Construct, id: string, props: DatabaseClusterProps) { super(scope, id); - const subnets = props.instanceProps.vpc.subnets(props.instanceProps.vpcPlacement); + const subnetIds = props.instanceProps.vpc.subnetIds(props.instanceProps.vpcPlacement); // 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 ? @@ -187,6 +187,8 @@ export class DatabaseCluster extends cdk.Construct implements IDatabaseCluster { throw new Error('At least one instance is required'); } + // Get the actual subnet objects so we can depend on internet connectivity. + const subnets = props.instanceProps.vpc.subnets(props.instanceProps.vpcPlacement); for (let i = 0; i < instanceCount; i++) { const instanceIndex = i + 1; diff --git a/packages/@aws-cdk/region-info/package-lock.json b/packages/@aws-cdk/region-info/package-lock.json index 3a21f56178c2b..b6dfc9aa49e07 100644 --- a/packages/@aws-cdk/region-info/package-lock.json +++ b/packages/@aws-cdk/region-info/package-lock.json @@ -1,6 +1,6 @@ { "name": "@aws-cdk/region-info", - "version": "0.25.2", + "version": "0.25.3", "lockfileVersion": 1, "requires": true, "dependencies": { diff --git a/packages/decdk/package-lock.json b/packages/decdk/package-lock.json index df2ad3fc4aa9a..683752c1ff76b 100644 --- a/packages/decdk/package-lock.json +++ b/packages/decdk/package-lock.json @@ -1,6 +1,6 @@ { "name": "decdk", - "version": "0.25.2", + "version": "0.25.3", "lockfileVersion": 1, "requires": true, "dependencies": {