Skip to content

Commit

Permalink
fix(ec2): opaque error when insufficient NAT EIPs are configured (#16040
Browse files Browse the repository at this point in the history
)

When manually configuring NAT gateways for a VPC, a relatively opaque
error is emitted when insufficient EIP allocation IDs are provided,
compared to the subnet count.

Added a proactive validation of the allocation IDs to improve the
actionability of the error message.

Fixes #16039


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
RomainMuller committed Aug 13, 2021
1 parent fdee43c commit a308cac
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 12 deletions.
10 changes: 9 additions & 1 deletion packages/@aws-cdk/aws-ec2/lib/nat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,14 @@ class NatGatewayProvider extends NatProvider {
}

public configureNat(options: ConfigureNatOptions) {
if (
this.props.eipAllocationIds != null
&& !Token.isUnresolved(this.props.eipAllocationIds)
&& this.props.eipAllocationIds.length < options.natSubnets.length
) {
throw new Error(`Not enough NAT gateway EIP allocation IDs (${this.props.eipAllocationIds.length} provided) for the requested subnet count (${options.natSubnets.length} needed).`);
}

// Create the NAT gateways
let i = 0;
for (const sub of options.natSubnets) {
Expand Down Expand Up @@ -413,4 +421,4 @@ function pickN(i: number, xs: string[]) {
}

return xs[i];
}
}
46 changes: 35 additions & 11 deletions packages/@aws-cdk/aws-ec2/test/vpc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ nodeunitShim({
'vpc.vpcId returns a token to the VPC ID'(test: Test) {
const stack = getTestStack();
const vpc = new Vpc(stack, 'TheVPC');
test.deepEqual(stack.resolve(vpc.vpcId), { Ref: 'TheVPC92636AB0' } );
test.deepEqual(stack.resolve(vpc.vpcId), { Ref: 'TheVPC92636AB0' });
test.done();
},

Expand All @@ -55,11 +55,11 @@ nodeunitShim({
new Vpc(stack, 'TheVPC');
cdkExpect(stack).to(
haveResource('AWS::EC2::VPC',
hasTags( [{ Key: 'Name', Value: 'TestStack/TheVPC' }])),
hasTags([{ Key: 'Name', Value: 'TestStack/TheVPC' }])),
);
cdkExpect(stack).to(
haveResource('AWS::EC2::InternetGateway',
hasTags( [{ Key: 'Name', Value: 'TestStack/TheVPC' }])),
hasTags([{ Key: 'Name', Value: 'TestStack/TheVPC' }])),
);
test.done();
},
Expand All @@ -86,7 +86,7 @@ nodeunitShim({

'dns getters correspond to CFN properties': (() => {

const tests: any = { };
const tests: any = {};

const inputs = [
{ dnsSupport: false, dnsHostnames: false },
Expand Down Expand Up @@ -173,8 +173,7 @@ nodeunitShim({
},
],
});
cdkExpect(stack).to(countResources('AWS::EC2::InternetGateway', 1))
;
cdkExpect(stack).to(countResources('AWS::EC2::InternetGateway', 1));
cdkExpect(stack).notTo(haveResource('AWS::EC2::NatGateway'));
test.done();
},
Expand Down Expand Up @@ -223,7 +222,7 @@ nodeunitShim({
'with no subnets defined, the VPC should have an IGW, and a NAT Gateway per AZ'(test: Test) {
const stack = getTestStack();
const zones = stack.availabilityZones.length;
new Vpc(stack, 'TheVPC', { });
new Vpc(stack, 'TheVPC', {});
cdkExpect(stack).to(countResources('AWS::EC2::InternetGateway', 1));
cdkExpect(stack).to(countResources('AWS::EC2::NatGateway', zones));
test.done();
Expand Down Expand Up @@ -251,7 +250,7 @@ nodeunitShim({
cdkExpect(stack).to(haveResource('AWS::EC2::InternetGateway'));
cdkExpect(stack).to(haveResourceLike('AWS::EC2::Route', {
DestinationCidrBlock: '8.8.8.8/32',
GatewayId: { },
GatewayId: {},
}));
test.done();
},
Expand Down Expand Up @@ -457,7 +456,7 @@ nodeunitShim({
}
cdkExpect(stack).to(haveResourceLike('AWS::EC2::Route', {
DestinationCidrBlock: '0.0.0.0/0',
NatGatewayId: { },
NatGatewayId: {},
}));

test.done();
Expand All @@ -475,7 +474,7 @@ nodeunitShim({
}
cdkExpect(stack).to(haveResourceLike('AWS::EC2::Route', {
DestinationCidrBlock: '0.0.0.0/0',
NatGatewayId: { },
NatGatewayId: {},
}));
test.done();
},
Expand All @@ -489,7 +488,7 @@ nodeunitShim({
cdkExpect(stack).to(countResources('AWS::EC2::NatGateway', 1));
cdkExpect(stack).to(haveResourceLike('AWS::EC2::Route', {
DestinationCidrBlock: '0.0.0.0/0',
NatGatewayId: { },
NatGatewayId: {},
}));
test.done();
},
Expand Down Expand Up @@ -873,6 +872,31 @@ nodeunitShim({
test.done();
},

'NAT gateway provider with insufficient EIP allocations'(test: Test) {
const stack = new Stack();
const natGatewayProvider = NatProvider.gateway({ eipAllocationIds: ['a'] });
expect(() => new Vpc(stack, 'VpcNetwork', { natGatewayProvider }))
.toThrow(/Not enough NAT gateway EIP allocation IDs \(1 provided\) for the requested subnet count \(\d+ needed\)/);

test.done();
},

'NAT gateway provider with token EIP allocations'(test: Test) {
const stack = new Stack();
const eipAllocationIds = Fn.split(',', Fn.importValue('myVpcId'));
const natGatewayProvider = NatProvider.gateway({ eipAllocationIds });
new Vpc(stack, 'VpcNetwork', { natGatewayProvider });

cdkExpect(stack).to(haveResource('AWS::EC2::NatGateway', {
AllocationId: stack.resolve(Fn.select(0, eipAllocationIds)),
}));
cdkExpect(stack).to(haveResource('AWS::EC2::NatGateway', {
AllocationId: stack.resolve(Fn.select(1, eipAllocationIds)),
}));

test.done();
},

'Can add an IPv6 route'(test: Test) {
// GIVEN
const stack = getTestStack();
Expand Down

0 comments on commit a308cac

Please sign in to comment.