Skip to content

Commit

Permalink
fix(ec2): new Instance fails in lookup Vpc
Browse files Browse the repository at this point in the history
Selecting by `subnetGroupName` in a looked up VPC may produce 0 subnets
on the first run of the app, if the actual lookup hasn't been performed
yet.

Rather than throwing an exception (which will fail the execution
outright and never even get to perform the lookup), add a construct
error. On the next run the lookup will have been performed and
the validation will no longer fail.

Fixes #7580.
  • Loading branch information
rix0rrr authored May 4, 2020
1 parent be6666b commit 3161de8
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 4 deletions.
18 changes: 15 additions & 3 deletions packages/@aws-cdk/aws-ec2/lib/instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { IMachineImage, OperatingSystemType } from './machine-image';
import { ISecurityGroup, SecurityGroup } from './security-group';
import { UserData } from './user-data';
import { BlockDevice, synthesizeBlockDeviceMappings } from './volume';
import { IVpc, SubnetSelection } from './vpc';
import { IVpc, Subnet, SubnetSelection } from './vpc';

/**
* Name tag constant
Expand Down Expand Up @@ -291,10 +291,22 @@ export class Instance extends Resource implements IInstance {
if (selected.length === 1) {
subnet = selected[0];
} else {
throw new Error(`Need exactly 1 subnet to match AZ '${props.availabilityZone}', found ${selected.length}. Use a different availabilityZone.`);
this.node.addError(`Need exactly 1 subnet to match AZ '${props.availabilityZone}', found ${selected.length}. Use a different availabilityZone.`);
}
} else {
subnet = subnets[0];
if (subnets.length > 0) {
subnet = subnets[0];
} else {
this.node.addError(`Did not find any subnets matching '${JSON.stringify(props.vpcSubnets)}', please use a different selection.`);
}
}
if (!subnet) {
// We got here and we don't have a subnet because of validation errors.
// Invent one on the spot so the code below doesn't fail.
subnet = Subnet.fromSubnetAttributes(this, 'DummySubnet', {
subnetId: 's-notfound',
availabilityZone: 'az-notfound',
});
}

this.instance = new CfnInstance(this, 'Resource', {
Expand Down
22 changes: 21 additions & 1 deletion packages/@aws-cdk/aws-ec2/test/test.vpc.from-lookup.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Construct, ContextProvider, GetContextValueOptions, GetContextValueResult, Lazy, Stack } from '@aws-cdk/core';
import * as cxapi from '@aws-cdk/cx-api';
import { Test } from 'nodeunit';
import { SubnetType, Vpc } from '../lib';
import { GenericLinuxImage, Instance, InstanceType, SubnetType, Vpc } from '../lib';

export = {
'Vpc.fromLookup()': {
Expand Down Expand Up @@ -188,6 +188,26 @@ export = {

test.done();
},

'don\'t crash when using subnetgroup name in lookup VPC'(test: Test) {
// GIVEN
const stack = new Stack(undefined, 'MyTestStack', { env: { account: '1234567890', region: 'dummy' } });
const vpc = Vpc.fromLookup(stack, 'vpc', { isDefault: true });

// WHEN
new Instance(stack, 'Instance', {
vpc,
instanceType: new InstanceType('t2.large'),
machineImage: new GenericLinuxImage({ dummy: 'ami-1234' }),
vpcSubnets: {
subnetGroupName: 'application_layer',
},
});

// THEN -- no exception occurred

test.done();
},
},
};

Expand Down

0 comments on commit 3161de8

Please sign in to comment.