Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(ec2): create NAT Gateways with fixed IPs #14250

Merged
merged 5 commits into from Apr 21, 2021
Merged

feat(ec2): create NAT Gateways with fixed IPs #14250

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1db070d
feat(ec2): create NAT Gateways with fixed IPs
rix0rrr Apr 19, 2021
c0ff207
Merge branch 'master' into huijbers/nat-gateway-alloc-ids
NetaNir Apr 21, 2021
fbdf643
remove @experimental
NetaNir Apr 21, 2021
c070151
Merge branch 'master' into huijbers/nat-gateway-alloc-ids
NetaNir Apr 21, 2021
423deaf
Merge branch 'master' into huijbers/nat-gateway-alloc-ids
mergify[bot] Apr 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 39 additions & 2 deletions packages/@aws-cdk/aws-ec2/lib/nat.ts
View file
Open in desktop
@@ -1,4 +1,5 @@
import * as iam from '@aws-cdk/aws-iam';
import { Fn, Token } from '@aws-cdk/core';
import { Connections, IConnectable } from './connections';
import { Instance } from './instance';
import { InstanceType } from './instance-types';
Expand Down Expand Up @@ -59,8 +60,8 @@ export abstract class NatProvider {
*
* @see https://docs.aws.amazon.com/vpc/latest/userguide/vpc-nat-gateway.html
*/
public static gateway(): NatProvider {
return new NatGatewayProvider();
public static gateway(props: NatGatewayProps = {}): NatProvider {
return new NatGatewayProvider(props);
}

/**
Expand Down Expand Up @@ -122,6 +123,20 @@ export interface ConfigureNatOptions {
readonly privateSubnets: PrivateSubnet[];
}

/**
* Properties for a NAT gateway
*
* @experimental
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is being banned. I believe, you have to either go with the new model of 'Pre' suffix or just take this as a stable API.

cc @NetaNir

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The APIs above and below also had @experimental still there, so seeing as no code change has been made here yet, I'd like to keep this in line with the rest.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

coordinate with @NetaNir on this. See #14071.

Copy link
Contributor

@NetaNir NetaNir Apr 21, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nija-at is right, we removed all @expiremental annotations and added a lint rule to disallow the use of this annotation. The PR for the removal was merged today, I was keeping it open to allow owners to make any adjustments before all APIs are marked as stable.

I pushed the change to remove the @experimetnal flag to your branch and merged in the latest changes from master.

If you want to keep this API non-final, you can add a preview suffix to both the method and the props. The preview API spec is not yet written but we had some discussion on this PR, in which it was decided to use the suffix beta1.

*/
export interface NatGatewayProps {
/**
* EIP allocation IDs for the NAT gateways
nija-at marked this conversation as resolved.
Show resolved Hide resolved
*
* @default - No fixed EIPs allocated for the NAT gateways
*/
readonly eipAllocationIds?: string[];
}

/**
* Properties for a NAT instance
*
Expand Down Expand Up @@ -203,11 +218,20 @@ export interface NatInstanceProps {
class NatGatewayProvider extends NatProvider {
private gateways: PrefSet<string> = new PrefSet<string>();

constructor(private readonly props: NatGatewayProps = {}) {
super();
}

public configureNat(options: ConfigureNatOptions) {
// Create the NAT gateways
let i = 0;
for (const sub of options.natSubnets) {
const gateway = sub.addNatGateway();
if (this.props.eipAllocationIds) {
gateway.allocationId = pickN(i, this.props.eipAllocationIds);
}
this.gateways.add(sub.availabilityZone, gateway.ref);
i++;
}

// Add routes to them in the private subnets
Expand Down Expand Up @@ -377,4 +401,17 @@ function isOutboundAllowed(direction: NatTrafficDirection) {

function isInboundAllowed(direction: NatTrafficDirection) {
return direction === NatTrafficDirection.INBOUND_AND_OUTBOUND;
}

/**
* Token-aware pick index function
*/
function pickN(i: number, xs: string[]) {
if (Token.isUnresolved(xs)) { return Fn.select(i, xs); }

if (i >= xs.length) {
throw new Error(`Cannot get element ${i} from ${xs}`);
}

return xs[i];
}
18 changes: 18 additions & 0 deletions packages/@aws-cdk/aws-ec2/test/vpc.test.ts
View file
Open in desktop
Expand Up @@ -855,6 +855,24 @@ nodeunitShim({

test.done();
},

'NAT gateway provider with EIP allocations'(test: Test) {
const stack = new Stack();
const natGatewayProvider = NatProvider.gateway({
eipAllocationIds: ['a', 'b', 'c', 'd'],
});
new Vpc(stack, 'VpcNetwork', { natGatewayProvider });

cdkExpect(stack).to(haveResource('AWS::EC2::NatGateway', {
AllocationId: 'a',
}));
cdkExpect(stack).to(haveResource('AWS::EC2::NatGateway', {
AllocationId: 'b',
}));

test.done();
},

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