Skip to content

Commit

Permalink
fix(vpc): recognize Public subnets by Internet Gateway (#3784)
Browse files Browse the repository at this point in the history 
The VPC importer used to look at `MapPublicIpOnLaunch` as a shortcut
for determining whether a Subnet was public or private.

The correct way to do it is to look at the route table and see if
it includes an Internet Gateway.

Fixes #3706.
  • Loading branch information
@rix0rrr
rix0rrr authored Aug 28, 2019
1 parent 4afd40e commit 54599e5
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 72 deletions.
42 changes: 32 additions & 10 deletions packages/aws-cdk/lib/context-providers/vpcs.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ export class VpcNetworkContextProviderPlugin implements ContextProviderPlugin {
const listedSubnets = subnetsResponse.Subnets || [];

const routeTablesResponse = await ec2.describeRouteTables(filters).promise();
const listedRouteTables = routeTablesResponse.RouteTables || [];

const mainRouteTable = listedRouteTables.find(table => table.Associations && !!table.Associations.find(assoc => assoc.Main));
const routeTables = new RouteTables(routeTablesResponse.RouteTables || []);

// Now comes our job to separate these subnets out into AZs and subnet groups (Public, Private, Isolated)
// We have the following attributes to go on:
Expand All @@ -64,19 +62,17 @@ export class VpcNetworkContextProviderPlugin implements ContextProviderPlugin {

const subnets: Subnet[] = listedSubnets.map(subnet => {
let type = getTag('aws-cdk:subnet-type', subnet.Tags);
if (type === undefined) {
type = subnet.MapPublicIpOnLaunch ? SubnetType.Public : SubnetType.Private;
}
if (type === undefined && subnet.MapPublicIpOnLaunch) { type = SubnetType.Public; }
if (type === undefined && routeTables.hasRouteToIgw(subnet.SubnetId)) { type = SubnetType.Public; }
if (type === undefined) { type = SubnetType.Private; }

if (!isValidSubnetType(type)) {
// tslint:disable-next-line: max-line-length
throw new Error(`Subnet ${subnet.SubnetArn} has invalid subnet type ${type} (must be ${SubnetType.Public}, ${SubnetType.Private} or ${SubnetType.Isolated})`);
}

const name = getTag('aws-cdk:subnet-name', subnet.Tags) || type;
const specificRouteTable =
listedRouteTables.find(table => table.Associations && !!table.Associations.find(assoc => assoc.SubnetId === subnet.SubnetId));
const routeTableId = (specificRouteTable && specificRouteTable.RouteTableId)
|| (mainRouteTable && mainRouteTable.RouteTableId);
const routeTableId = routeTables.routeTableIdForSubnetId(subnet.SubnetId);

if (!routeTableId) {
throw new Error(`Subnet ${subnet.SubnetArn} does not have an associated route table (and there is no "main" table)`);
Expand Down Expand Up @@ -131,6 +127,32 @@ export class VpcNetworkContextProviderPlugin implements ContextProviderPlugin {
}
}

class RouteTables {
public readonly mainRouteTable?: AWS.EC2.RouteTable;

constructor(private readonly tables: AWS.EC2.RouteTable[]) {
this.mainRouteTable = this.tables.find(table => !!table.Associations && table.Associations.some(assoc => !!assoc.Main));
}

public routeTableIdForSubnetId(subnetId: string | undefined): string | undefined {
const table = this.tableForSubnet(subnetId);
return (table && table.RouteTableId) || (this.mainRouteTable && this.mainRouteTable.RouteTableId);
}

/**
* Whether the given subnet has a route to an IGW
*/
public hasRouteToIgw(subnetId: string | undefined): boolean {
const table = this.tableForSubnet(subnetId);

return !!table && !!table.Routes && table.Routes.some(route => !!route.GatewayId && route.GatewayId.startsWith('igw-'));
}

public tableForSubnet(subnetId: string | undefined) {
return this.tables.find(table => !!table.Associations && table.Associations.some(assoc => assoc.SubnetId === subnetId));
}
}

/**
* Return the value of a tag from a set of tags
*/
Expand Down
181 changes: 119 additions & 62 deletions packages/aws-cdk/test/context-providers/test.vpcs.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,35 +25,17 @@ export = nodeunit.testCase({
const filter = { foo: 'bar' };
const provider = new VpcNetworkContextProviderPlugin(mockSDK);

AWS.mock('EC2', 'describeVpcs', (params: aws.EC2.DescribeVpcsRequest, cb: AwsCallback<aws.EC2.DescribeVpcsResult>) => {
test.deepEqual(params.Filters, [{ Name: 'foo', Values: ['bar'] }]);
return cb(null, { Vpcs: [{ VpcId: 'vpc-1234567' }] });
});
AWS.mock('EC2', 'describeSubnets', (params: aws.EC2.DescribeSubnetsRequest, cb: AwsCallback<aws.EC2.DescribeSubnetsResult>) => {
test.deepEqual(params.Filters, [{ Name: 'vpc-id', Values: ['vpc-1234567'] }]);
return cb(null, {
Subnets: [
{ SubnetId: 'sub-123456', AvailabilityZone: 'bermuda-triangle-1337', MapPublicIpOnLaunch: true },
{ SubnetId: 'sub-789012', AvailabilityZone: 'bermuda-triangle-1337', MapPublicIpOnLaunch: false }
]
});
});
AWS.mock('EC2', 'describeRouteTables', (params: aws.EC2.DescribeRouteTablesRequest, cb: AwsCallback<aws.EC2.DescribeRouteTablesResult>) => {
test.deepEqual(params.Filters, [{ Name: 'vpc-id', Values: ['vpc-1234567'] }]);
return cb(null, {
RouteTables: [
{ Associations: [{ SubnetId: 'sub-123456' }], RouteTableId: 'rtb-123456', },
{ Associations: [{ SubnetId: 'sub-789012' }], RouteTableId: 'rtb-789012', }
]
});
});
AWS.mock('EC2', 'describeVpnGateways', (params: aws.EC2.DescribeVpnGatewaysRequest, cb: AwsCallback<aws.EC2.DescribeVpnGatewaysResult>) => {
test.deepEqual(params.Filters, [
{ Name: 'attachment.vpc-id', Values: [ 'vpc-1234567' ] },
{ Name: 'attachment.state', Values: [ 'attached' ] },
{ Name: 'state', Values: [ 'available' ] }
]);
return cb(null, { VpnGateways: [{ VpnGatewayId: 'gw-abcdef' }] });
mockVpcLookup(test, {
subnets: [
{ SubnetId: 'sub-123456', AvailabilityZone: 'bermuda-triangle-1337', MapPublicIpOnLaunch: true },
{ SubnetId: 'sub-789012', AvailabilityZone: 'bermuda-triangle-1337', MapPublicIpOnLaunch: false }
],
routeTables: [
{ Associations: [{ SubnetId: 'sub-123456' }], RouteTableId: 'rtb-123456', },
{ Associations: [{ SubnetId: 'sub-789012' }], RouteTableId: 'rtb-789012', }
],
vpnGateways: [{ VpnGatewayId: 'gw-abcdef' }]

});

// WHEN
Expand All @@ -75,8 +57,8 @@ export = nodeunit.testCase({
vpnGatewayId: 'gw-abcdef'
});

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

async 'throws when no such VPC is found'(test: nodeunit.Test) {
Expand All @@ -97,8 +79,8 @@ export = nodeunit.testCase({
test.throws(() => { throw e; }, /Could not find any VPCs matching/);
}

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

async 'throws when multiple VPCs are found'(test: nodeunit.Test) {
Expand All @@ -119,44 +101,25 @@ export = nodeunit.testCase({
test.throws(() => { throw e; }, /Found 2 VPCs matching/);
}

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

async 'uses the VPC main route table when a subnet has no specific association'(test: nodeunit.Test) {
// GIVEN
const filter = { foo: 'bar' };
const provider = new VpcNetworkContextProviderPlugin(mockSDK);

AWS.mock('EC2', 'describeVpcs', (params: aws.EC2.DescribeVpcsRequest, cb: AwsCallback<aws.EC2.DescribeVpcsResult>) => {
test.deepEqual(params.Filters, [{ Name: 'foo', Values: ['bar'] }]);
return cb(null, { Vpcs: [{ VpcId: 'vpc-1234567' }] });
});
AWS.mock('EC2', 'describeSubnets', (params: aws.EC2.DescribeSubnetsRequest, cb: AwsCallback<aws.EC2.DescribeSubnetsResult>) => {
test.deepEqual(params.Filters, [{ Name: 'vpc-id', Values: ['vpc-1234567'] }]);
return cb(null, {
Subnets: [
{ SubnetId: 'sub-123456', AvailabilityZone: 'bermuda-triangle-1337', MapPublicIpOnLaunch: true },
{ SubnetId: 'sub-789012', AvailabilityZone: 'bermuda-triangle-1337', MapPublicIpOnLaunch: false }
]
});
});
AWS.mock('EC2', 'describeRouteTables', (params: aws.EC2.DescribeRouteTablesRequest, cb: AwsCallback<aws.EC2.DescribeRouteTablesResult>) => {
test.deepEqual(params.Filters, [{ Name: 'vpc-id', Values: ['vpc-1234567'] }]);
return cb(null, {
RouteTables: [
{ Associations: [{ SubnetId: 'sub-123456' }], RouteTableId: 'rtb-123456', },
{ Associations: [{ Main: true }], RouteTableId: 'rtb-789012', }
]
});
});
AWS.mock('EC2', 'describeVpnGateways', (params: aws.EC2.DescribeVpnGatewaysRequest, cb: AwsCallback<aws.EC2.DescribeVpnGatewaysResult>) => {
test.deepEqual(params.Filters, [
{ Name: 'attachment.vpc-id', Values: [ 'vpc-1234567' ] },
{ Name: 'attachment.state', Values: [ 'attached' ] },
{ Name: 'state', Values: [ 'available' ] }
]);
return cb(null, { VpnGateways: [{ VpnGatewayId: 'gw-abcdef' }] });
mockVpcLookup(test, {
subnets: [
{ SubnetId: 'sub-123456', AvailabilityZone: 'bermuda-triangle-1337', MapPublicIpOnLaunch: true },
{ SubnetId: 'sub-789012', AvailabilityZone: 'bermuda-triangle-1337', MapPublicIpOnLaunch: false }
],
routeTables: [
{ Associations: [{ SubnetId: 'sub-123456' }], RouteTableId: 'rtb-123456', },
{ Associations: [{ Main: true }], RouteTableId: 'rtb-789012', }
],
vpnGateways: [{ VpnGatewayId: 'gw-abcdef' }]
});

// WHEN
Expand All @@ -180,5 +143,99 @@ export = nodeunit.testCase({

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

async 'Recognize public subnet by route table'(test: nodeunit.Test) {
// GIVEN
const filter = { foo: 'bar' };
const provider = new VpcNetworkContextProviderPlugin(mockSDK);

mockVpcLookup(test, {
subnets: [
{ SubnetId: 'sub-123456', AvailabilityZone: 'bermuda-triangle-1337', MapPublicIpOnLaunch: false },
],
routeTables: [
{
Associations: [{ SubnetId: 'sub-123456' }],
RouteTableId: 'rtb-123456',
Routes: [
{
DestinationCidrBlock: "10.0.2.0/26",
Origin: "CreateRoute",
State: "active",
VpcPeeringConnectionId: "pcx-xxxxxx"
},
{
DestinationCidrBlock: "10.0.1.0/24",
GatewayId: "local",
Origin: "CreateRouteTable",
State: "active"
},
{
DestinationCidrBlock: "0.0.0.0/0",
GatewayId: "igw-xxxxxx",
Origin: "CreateRoute",
State: "active"
}
],
},
],
});

// WHEN
const result = await provider.getValue({ filter });

// THEN
test.deepEqual(result, {
vpcId: 'vpc-1234567',
availabilityZones: ['bermuda-triangle-1337'],
isolatedSubnetIds: undefined,
isolatedSubnetNames: undefined,
isolatedSubnetRouteTableIds: undefined,
privateSubnetIds: undefined,
privateSubnetNames: undefined,
privateSubnetRouteTableIds: undefined,
publicSubnetIds: ['sub-123456'],
publicSubnetNames: ['Public'],
publicSubnetRouteTableIds: ['rtb-123456'],
vpnGatewayId: undefined,
});

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

interface VpcLookupOptions {
subnets: aws.EC2.Subnet[];
routeTables: aws.EC2.RouteTable[];
vpnGateways?: aws.EC2.VpnGateway[];
}

function mockVpcLookup(test: nodeunit.Test, options: VpcLookupOptions) {
const VpcId = 'vpc-1234567';

AWS.mock('EC2', 'describeVpcs', (params: aws.EC2.DescribeVpcsRequest, cb: AwsCallback<aws.EC2.DescribeVpcsResult>) => {
test.deepEqual(params.Filters, [{ Name: 'foo', Values: ['bar'] }]);
return cb(null, { Vpcs: [{ VpcId }] });
});

AWS.mock('EC2', 'describeSubnets', (params: aws.EC2.DescribeSubnetsRequest, cb: AwsCallback<aws.EC2.DescribeSubnetsResult>) => {
test.deepEqual(params.Filters, [{ Name: 'vpc-id', Values: [VpcId] }]);
return cb(null, { Subnets: options.subnets });
});

AWS.mock('EC2', 'describeRouteTables', (params: aws.EC2.DescribeRouteTablesRequest, cb: AwsCallback<aws.EC2.DescribeRouteTablesResult>) => {
test.deepEqual(params.Filters, [{ Name: 'vpc-id', Values: [VpcId] }]);
return cb(null, { RouteTables: options.routeTables });
});

AWS.mock('EC2', 'describeVpnGateways', (params: aws.EC2.DescribeVpnGatewaysRequest, cb: AwsCallback<aws.EC2.DescribeVpnGatewaysResult>) => {
test.deepEqual(params.Filters, [
{ Name: 'attachment.vpc-id', Values: [ VpcId ] },
{ Name: 'attachment.state', Values: [ 'attached' ] },
{ Name: 'state', Values: [ 'available' ] }
]);
return cb(null, { VpnGateways: options.vpnGateways });
});
}

0 comments on commit 54599e5

Please sign in to comment.