fix(ec2): 'encoded list token' error using Vpc imported from deploy-time lists

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

@@ -1351,6 +1351,45 @@ test('Can set autoScalingGroupName', () => {
   }));
 });
 
+test('can use Vpc imported from unparseable list tokens', () => {
+  // GIVEN
+  const stack = new cdk.Stack();
+
+  const vpcId = cdk.Fn.importValue('myVpcId');
+  const availabilityZones = cdk.Fn.split(',', cdk.Fn.importValue('myAvailabilityZones'));
+  const publicSubnetIds = cdk.Fn.split(',', cdk.Fn.importValue('myPublicSubnetIds'));
+  const privateSubnetIds = cdk.Fn.split(',', cdk.Fn.importValue('myPrivateSubnetIds'));
+  const isolatedSubnetIds = cdk.Fn.split(',', cdk.Fn.importValue('myIsolatedSubnetIds'));
+
+  const vpc = ec2.Vpc.fromVpcAttributes(stack, 'importedVpc', {
+    vpcId,
+    availabilityZones,
+    publicSubnetIds,
+    privateSubnetIds,
+    isolatedSubnetIds,
+  });
+
+  // WHEN
+  new autoscaling.AutoScalingGroup(stack, 'ecs-ec2-asg', {
+    instanceType: new ec2.InstanceType('t2.micro'),
+    machineImage: new ec2.AmazonLinuxImage(),
+    minCapacity: 1,
+    maxCapacity: 1,
+    desiredCapacity: 1,
+    vpc,
+    allowAllOutbound: false,
+    associatePublicIpAddress: false,
+    vpcSubnets: { subnetType: ec2.SubnetType.PRIVATE },
+  });
+
+  // THEN
+  expect(stack).to(haveResourceLike('AWS::AutoScaling::AutoScalingGroup', {
+    VPCZoneIdentifier: {
+      'Fn::Split': [',', { 'Fn::ImportValue': 'myPrivateSubnetIds' }],
+    },
+  }));
+});
+
 function mockSecurityGroup(stack: cdk.Stack) {
   return ec2.SecurityGroup.fromSecurityGroupId(stack, 'MySG', 'most-secure');
 }

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

@@ -1010,7 +1010,16 @@ export class Vpc extends VpcBase {
   ];
 
   /**
-   * Import an exported VPC
+   * Import a VPC by supplying all attributes directly
+   *
+   * NOTE: using `fromVpcAttributes()` with deploy-time parameters (like a `Fn.importValue()` or
+   * `CfnParameter` to represent a list of subnet IDs) sometimes accidentally works. It happens
+   * to work for constructs that need a list of subnets (like `AutoScalingGroup` and `eks.Cluster`)
+   * but it does not work for constructs that need individual subnets (like
+   * `Instance`). See https://github.com/aws/aws-cdk/issues/4118 for more
+   * information.
+   *
+   * Prefer to use `Vpc.fromLookup()` instead.
    */
   public static fromVpcAttributes(scope: Construct, id: string, attrs: VpcAttributes): IVpc {
     return new ImportedVpc(scope, id, attrs, false);
@@ -1927,7 +1936,21 @@ class ImportedSubnet extends Resource implements ISubnet, IPublicSubnet, IPrivat
     super(scope, id);
 
     if (!attrs.routeTableId) {
-      const ref = Token.isUnresolved(attrs.subnetId)
+      // The following looks a little weird, but comes down to:
+      //
+      // * Is the subnetId itself unresolved ({ Ref: Subnet }); or
+      // * Was it the accidentally extracted first element of a list-encoded
+      //   token? ({ Fn::ImportValue: Subnets } => ['#{Token[1234]}'] =>
+      //   '#{Token[1234]}'
+      //
+      // There's no other API to test for the second case than to the string back into
+      // a list and see if the combination is Unresolved.
+      //
+      // In both cases we can't output the subnetId literally into the metadata (because it'll
+      // be useless). In the 2nd case even, if we output it to metadata, the `resolve()` call
+      // that gets done on the metadata will even `throw`, because the '#{Token}' value will
+      // occur in an illegal position (not in a list context).
+      const ref = Token.isUnresolved(attrs.subnetId) || Token.isUnresolved([attrs.subnetId])
         ? `at '${Node.of(scope).path}/${id}'`
         : `'${attrs.subnetId}'`;
       // eslint-disable-next-line max-len

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

@@ -1,5 +1,5 @@
 import { countResources, expect, haveResource, haveResourceLike, isSuperObject, MatchStyle } from '@aws-cdk/assert';
-import { CfnOutput, Lazy, Stack, Tags } from '@aws-cdk/core';
+import { CfnOutput, CfnResource, Fn, Lazy, Stack, Tags } from '@aws-cdk/core';
 import { nodeunitShim, Test } from 'nodeunit-shim';
 import {
   AclCidr, AclTraffic, BastionHostLinux, CfnSubnet, CfnVPC, SubnetFilter, DefaultInstanceTenancy, GenericLinuxImage,
@@ -1227,6 +1227,36 @@ nodeunitShim({
       test.done();
     },
 
+    'fromVpcAttributes using imported refs'(test: Test) {
+      // GIVEN
+      const stack = getTestStack();
+
+      const vpcId = Fn.importValue('myVpcId');
+      const availabilityZones = Fn.split(',', Fn.importValue('myAvailabilityZones'));
+      const publicSubnetIds = Fn.split(',', Fn.importValue('myPublicSubnetIds'));
+
+      // WHEN
+      const vpc = Vpc.fromVpcAttributes(stack, 'VPC', {
+        vpcId,
+        availabilityZones,
+        publicSubnetIds,
+      });
+
+      new CfnResource(stack, 'Resource', {
+        type: 'Some::Resource',
+        properties: {
+          subnetIds: vpc.selectSubnets().subnetIds,
+        },
+      });
+
+      // THEN - No exception
+      expect(stack).to(haveResource('Some::Resource', {
+        subnetIds: { 'Fn::Split': [',', { 'Fn::ImportValue': 'myPublicSubnetIds' }] },
+      }));
+
+      test.done();
+    },
+
     'select explicitly defined subnets'(test: Test) {
       // GIVEN
       const stack = getTestStack();