fix(autoscaling): require public subnets for associatePublicIpAddress

packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts

@@ -292,6 +292,9 @@ export class AutoScalingGroup extends cdk.Construct implements IAutoScalingGroup
     }
 
     asgProps.vpcZoneIdentifier = props.vpc.subnetIds(props.vpcSubnets);
+    if (!props.vpc.isPublicSubnets(asgProps.vpcZoneIdentifier) && props.associatePublicIpAddress) {
+      throw new Error("To set 'associatePublicIpAddress: true' you must select Public subnets (vpcSubnets: { subnetType: SubnetType.Public })");
+    }
 
     this.autoScalingGroup = new CfnAutoScalingGroup(this, 'ASG', asgProps);
     this.osType = machineImage.os.type;

packages/@aws-cdk/aws-autoscaling/test/test.auto-scaling-group.ts

@@ -418,6 +418,8 @@ export = {
       minCapacity: 0,
       maxCapacity: 0,
       desiredCapacity: 0,
+
+      vpcSubnets: { subnetType: ec2.SubnetType.Public },
       associatePublicIpAddress: true,
     });
 
@@ -428,6 +430,25 @@ export = {
     ));
     test.done();
   },
+  'association of public IP address requires public subnet'(test: Test) {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const vpc = mockVpc(stack);
+
+    // WHEN
+    test.throws(() => {
+      new autoscaling.AutoScalingGroup(stack, 'MyStack', {
+        instanceType: new ec2.InstanceTypePair(ec2.InstanceClass.M4, ec2.InstanceSize.Micro),
+        machineImage: new ec2.AmazonLinuxImage(),
+        vpc,
+        minCapacity: 0,
+        maxCapacity: 0,
+        desiredCapacity: 0,
+        associatePublicIpAddress: true,
+      });
+    });
+    test.done();
+  },
   'allows disassociation of public IP address'(test: Test) {
     // GIVEN
     const stack = new cdk.Stack();

packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts

@@ -77,13 +77,9 @@ export interface IVpcNetwork extends IConstruct {
   subnetInternetDependencies(selection?: SubnetSelection): IDependable;
 
   /**
-   * Return whether the given subnet is one of this VPC's public subnets.
-   *
-   * The subnet must literally be one of the subnet object obtained from
-   * this VPC. A subnet that merely represents the same subnet will
-   * never return true.
+   * Return whether all of the given subnets are from the VPC's public subnets.
    */
-  isPublicSubnet(subnet: IVpcSubnet): boolean;
+  isPublicSubnets(subnetIds: string[]): boolean;
 
   /**
    * Adds a new VPN connection to this VPC
@@ -253,14 +249,11 @@ export abstract class VpcNetworkBase extends Construct implements IVpcNetwork {
   public abstract export(): VpcNetworkImportProps;
 
   /**
-   * Return whether the given subnet is one of this VPC's public subnets.
-   *
-   * The subnet must literally be one of the subnet object obtained from
-   * this VPC. A subnet that merely represents the same subnet will
-   * never return true.
+   * Return whether all of the given subnets are from the VPC's public subnets.
    */
-  public isPublicSubnet(subnet: IVpcSubnet) {
-    return this.publicSubnets.indexOf(subnet) > -1;
+  public isPublicSubnets(subnetIds: string[]): boolean {
+    const pubIds = new Set(this.publicSubnets.map(n => n.subnetId));
+    return subnetIds.every(pubIds.has.bind(pubIds));
   }
 
   /**

packages/@aws-cdk/aws-ec2/lib/vpc.ts

@@ -456,7 +456,7 @@ export class VpcNetwork extends VpcNetworkBase {
     if (placement) {
       const subnets = this.subnets(placement);
       for (const sub of subnets) {
-        if (!this.isPublicSubnet(sub)) {
+        if (this.publicSubnets.indexOf(sub) === -1) {
           throw new Error(`natGatewayPlacement ${placement} contains non public subnet ${sub}`);
         }
       }