Skip to content

Commit

Permalink
fix(ec2): fix subnet selection on looked-up VPCs (#4090)
Browse files Browse the repository at this point in the history
Looked-up VPCs are created with a dummy configuration before the first
lookup happens; a `selectSubnets` call on the dummy configuration is
likely to fail.

Add an exception to make the initial lookup not fail.

Fixes #3650.
  • Loading branch information
rix0rrr committed Sep 18, 2019
1 parent 364fd56 commit 4a113e6
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 6 deletions.
21 changes: 15 additions & 6 deletions packages/@aws-cdk/aws-ec2/lib/vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,11 @@ abstract class VpcBase extends Resource implements IVpc {
*/
protected readonly natDependencies = new Array<IConstruct>();

/**
* If this is set to true, don't error out on trying to select subnets
*/
protected incompleteSubnetDefinition: boolean = false;

/**
* Returns IDs of selected subnets
*/
Expand Down Expand Up @@ -347,7 +352,7 @@ abstract class VpcBase extends Resource implements IVpc {
const allSubnets = [...this.publicSubnets, ...this.privateSubnets, ...this.isolatedSubnets];
const subnets = allSubnets.filter(s => subnetGroupNameFromConstructId(s) === groupName);

if (subnets.length === 0) {
if (subnets.length === 0 && !this.incompleteSubnetDefinition) {
const names = Array.from(new Set(allSubnets.map(subnetGroupNameFromConstructId)));
throw new Error(`There are no subnet groups with name '${groupName}' in this VPC. Available names: ${names}`);
}
Expand All @@ -369,7 +374,10 @@ abstract class VpcBase extends Resource implements IVpc {
subnets = subnets.filter(s => subnetGroupNameFromConstructId(s) === subnetGroupNameFromConstructId(subnets[0]));
}

if (subnets.length === 0) {
// Force merge conflict here with https://github.com/aws/aws-cdk/pull/4089
// see ImportedVpc

if (subnets.length === 0 && !this.incompleteSubnetDefinition) {
const availableTypes = Object.entries(allSubnets).filter(([_, subs]) => subs.length > 0).map(([typeName, _]) => typeName);
throw new Error(`There are no '${subnetType}' subnet groups in this VPC. Available types: ${availableTypes}`);
}
Expand Down Expand Up @@ -759,7 +767,7 @@ export class Vpc extends VpcBase {
* Import an exported VPC
*/
public static fromVpcAttributes(scope: Construct, id: string, attrs: VpcAttributes): IVpc {
return new ImportedVpc(scope, id, attrs);
return new ImportedVpc(scope, id, attrs, false);
}

/**
Expand Down Expand Up @@ -796,10 +804,10 @@ export class Vpc extends VpcBase {
const attributes = ContextProvider.getValue(scope, {
provider: cxapi.VPC_PROVIDER,
props: { filter } as cxapi.VpcContextQuery,
dummyValue: DUMMY_VPC_PROPS
dummyValue: undefined
});

return this.fromVpcAttributes(scope, id, attributes);
return new ImportedVpc(scope, id, attributes || DUMMY_VPC_PROPS, attributes === undefined);

/**
* Prefixes all keys in the argument with `tag:`.`
Expand Down Expand Up @@ -1418,12 +1426,13 @@ class ImportedVpc extends VpcBase {
public readonly vpnGatewayId?: string;
public readonly internetConnectivityEstablished: IDependable = new ConcreteDependable();

constructor(scope: Construct, id: string, props: VpcAttributes) {
constructor(scope: Construct, id: string, props: VpcAttributes, isIncomplete: boolean) {
super(scope, id);

this.vpcId = props.vpcId;
this.availabilityZones = props.availabilityZones;
this.vpnGatewayId = props.vpnGatewayId;
this.incompleteSubnetDefinition = isIncomplete;

// tslint:disable:max-line-length
const pub = new ImportSubnetGroup(props.publicSubnetIds, props.publicSubnetNames, props.publicSubnetRouteTableIds, SubnetType.PUBLIC, this.availabilityZones, 'publicSubnetIds', 'publicSubnetNames', 'publicSubnetRouteTableIds');
Expand Down
15 changes: 15 additions & 0 deletions packages/@aws-cdk/aws-ec2/test/test.vpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,21 @@ export = {

test.done();
},

'selecting subnets by name from a looked-up VPC does not throw'(test: Test) {
// GIVEN
const stack = new Stack(undefined, undefined, { env: { region: 'us-east-1', account: '123456789012' }});
const vpc = Vpc.fromLookup(stack, 'VPC', {
vpcId: 'vpc-1234'
});

// WHEN
vpc.selectSubnets({ subnetName: 'Bleep' });

// THEN: no exception

test.done();
},
};

function getTestStack(): Stack {
Expand Down

0 comments on commit 4a113e6

Please sign in to comment.