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`.

Author: rix0rrr

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;

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).
 

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<IConstruct>();
 
   /**
-   * 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<IDependable>();
+
+  /**
+   * 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;
+  }
+}

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;
 

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();