fix(ec2): descriptive error message when selecting 0 subnets

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;

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 '???';
+}

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 {

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]),
       }
     };

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,

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'); }

packages/@aws-cdk/aws-lambda/lib/function.ts

@@ -504,15 +504,21 @@ 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)) {
         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: props.vpc.subnetIds(props.vpcPlacement),
       securityGroupIds: [securityGroup.securityGroupId]
     };
   }

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;