Skip to content

Commit

Permalink
fix(vpc): Vpc.fromLookup should throw if subnet group name tag is exp…
Browse files Browse the repository at this point in the history
…licitly given and does not exist

Currently if `subnetGroupNameTag` is provided in `Vpc.fromLookup()` and
a tag with that key does not exist, the error that is returned is very
generic and just indicates that the VPC could not be found. This makes
it very hard to troubleshoot what the real issue is (invalid
subnetGroupNameTag).

Now if the user provides a `subnetGroupNameTag` and a tag with that Key
does not exist an error is thrown indicating that an invalid
`subnetGroupNameTag` was provided

fixes #13962
  • Loading branch information
corymhall committed Jan 28, 2022
1 parent d93e091 commit 93fd68d
Show file tree
Hide file tree
Showing 2 changed files with 142 additions and 0 deletions.
4 changes: 4 additions & 0 deletions packages/aws-cdk/lib/context-providers/vpcs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ export class VpcNetworkContextProviderPlugin implements ContextProviderPlugin {
throw new Error(`Subnet ${subnet.SubnetArn} has invalid subnet type ${type} (must be ${SubnetType.Public}, ${SubnetType.Private} or ${SubnetType.Isolated})`);
}

if (args.subnetGroupNameTag && !getTag(args.subnetGroupNameTag, subnet.Tags)) {
throw new Error(`Invalid subnetGroupNameTag: Subnet ${subnet.SubnetArn} does not have an associated tag with Key='${args.subnetGroupNameTag}'`);
}

const name = getTag(args.subnetGroupNameTag || 'aws-cdk:subnet-name', subnet.Tags) || type;
const routeTableId = routeTables.routeTableIdForSubnetId(subnet.SubnetId);

Expand Down
138 changes: 138 additions & 0 deletions packages/aws-cdk/test/context-providers/vpcs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,144 @@ test('throws when no such VPC is found', async () => {
})).rejects.toThrow(/Could not find any VPCs matching/);
});

test('throws when subnet with subnetGroupNameTag not found', async () => {
// GIVEN
const filter = { foo: 'bar' };
const provider = new VpcNetworkContextProviderPlugin(mockSDK);

mockVpcLookup({
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',
Routes: [
{
DestinationCidrBlock: '1.1.1.1/24',
GatewayId: 'local',
Origin: 'CreateRouteTable',
State: 'active',
},
{
DestinationCidrBlock: '0.0.0.0/0',
GatewayId: 'igw-xxxxxx',
Origin: 'CreateRoute',
State: 'active',
},
],
},
{
Associations: [{ SubnetId: 'sub-789012' }],
RouteTableId: 'rtb-789012',
Routes: [
{
DestinationCidrBlock: '1.1.2.1/24',
GatewayId: 'local',
Origin: 'CreateRouteTable',
State: 'active',
},
{
DestinationCidrBlock: '0.0.0.0/0',
NatGatewayId: 'nat-xxxxxx',
Origin: 'CreateRoute',
State: 'active',
},
],
},
],
vpnGateways: [{ VpnGatewayId: 'gw-abcdef' }],
});

// WHEN
await expect(provider.getValue({
account: '1234',
region: 'us-east-1',
subnetGroupNameTag: 'DOES_NOT_EXIST',
filter,
})).rejects.toThrow(/Invalid subnetGroupNameTag: Subnet .* does not have an associated tag with Key='DOES_NOT_EXIST'/);
});

test('does not throw when subnet with subnetGroupNameTag is found', async () => {
// GIVEN
const filter = { foo: 'bar' };
const provider = new VpcNetworkContextProviderPlugin(mockSDK);

mockVpcLookup({
subnets: [
{ SubnetId: 'sub-123456', AvailabilityZone: 'bermuda-triangle-1337', MapPublicIpOnLaunch: true, Tags: [{ Key: 'DOES_EXIST', Value: 'SubnetName1' }] },
{ SubnetId: 'sub-789012', AvailabilityZone: 'bermuda-triangle-1337', MapPublicIpOnLaunch: false, Tags: [{ Key: 'DOES_EXIST', Value: 'SubnetName2' }] },
],
routeTables: [
{
Associations: [{ SubnetId: 'sub-123456' }],
RouteTableId: 'rtb-123456',
Routes: [
{
DestinationCidrBlock: '1.1.1.1/24',
GatewayId: 'local',
Origin: 'CreateRouteTable',
State: 'active',
},
{
DestinationCidrBlock: '0.0.0.0/0',
GatewayId: 'igw-xxxxxx',
Origin: 'CreateRoute',
State: 'active',
},
],
},
{
Associations: [{ SubnetId: 'sub-789012' }],
RouteTableId: 'rtb-789012',
Routes: [
{
DestinationCidrBlock: '1.1.2.1/24',
GatewayId: 'local',
Origin: 'CreateRouteTable',
State: 'active',
},
{
DestinationCidrBlock: '0.0.0.0/0',
NatGatewayId: 'nat-xxxxxx',
Origin: 'CreateRoute',
State: 'active',
},
],
},
],
vpnGateways: [{ VpnGatewayId: 'gw-abcdef' }],
});

// WHEN
const result = await provider.getValue({
account: '1234',
region: 'us-east-1',
subnetGroupNameTag: 'DOES_EXIST',
filter,
});

// THEN
expect(result).toEqual({
vpcId: 'vpc-1234567',
vpcCidrBlock: '1.1.1.1/16',
availabilityZones: ['bermuda-triangle-1337'],
isolatedSubnetIds: undefined,
isolatedSubnetNames: undefined,
isolatedSubnetRouteTableIds: undefined,
privateSubnetIds: ['sub-789012'],
privateSubnetNames: ['SubnetName2'],
privateSubnetRouteTableIds: ['rtb-789012'],
publicSubnetIds: ['sub-123456'],
publicSubnetNames: ['SubnetName1'],
publicSubnetRouteTableIds: ['rtb-123456'],
vpnGatewayId: 'gw-abcdef',
subnetGroups: undefined,
});
});

test('throws when multiple VPCs are found', async () => {
// GIVEN
const filter = { foo: 'bar' };
Expand Down

0 comments on commit 93fd68d

Please sign in to comment.