feat(ec2): access gateways created by NatProvider #4948

fogfish · 2019-11-11T12:29:06Z

mergify · 2019-11-11T12:29:43Z

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

PR title type(scope): text
- type: fix, feat, refactor go into CHANGELOG, chore is hidden
- scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
- text: use all lower-case, do not end with a period, do not include issue refs
PR Description
- Rationale: describe rationale of change and approach taken
- Issues: indicate issues fixed via: fixes #xxx or closes #xxx
- Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
Testing
- Unit test added. Prefer to add a new test rather than modify existing tests
- CLI or init templates change? Re-run/add CLI integration tests
Documentation
- README: update module README to describe new features
- API docs: public APIs must be documented. Copy from official AWS docs when possible
- Design: for significant features, follow design process

aws-cdk-automation · 2019-11-11T12:35:49Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2019-11-11T13:05:19Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

fogfish · 2019-11-12T10:56:46Z

@rix0rrr any feedback?

fogfish · 2019-11-18T12:39:27Z

It would be great to get any feedback on this pull request. This is fine, if you'd like to keep list of NATs private. In this case, we should start a work around project.

aws-cdk-automation · 2019-11-18T13:02:16Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

fogfish · 2019-11-25T08:30:35Z

Could you please take a look into this PR? This is quite small changes that simplifies provision of PrivateSubnets.

rix0rrr

We can totally add this to unblock you, modulo that I think the change should be completely in the NAT provider and not touch the VPC class at all. I'm thinking:

const natGatewayProvider = NatProvider.gateway();
new Vpc(..., { natGatewayProvider });
use(natGatewayProvider.gateways);

However, this is only one aspect. I bet there's also a more holistic change we can do to make the whole process of progressively adding subnets to a VPC more convenient. I'm not exactly sure what that would look like yet, but maybe we can design it together.

rix0rrr · 2019-11-25T13:07:19Z

packages/@aws-cdk/aws-ec2/lib/nat.ts

@@ -44,7 +44,7 @@ export abstract class NatProvider {
  /**
   * Called by the VPC to configure NAT
   */
-  public abstract configureNat(options: ConfigureNatOptions): void;
+  public abstract configureNat(options: ConfigureNatOptions): { [az: string]: string };


Instead of returning from this method, I think it makes more sense to store the information you're looking for on the object itself, and make it accessible using public getters.

When you do this, it's probably better to have a list of { az, router } records than a map. That will also allow future extensions where there is more than one router per AZ (which might be desirable for redundancy or performance).

fogfish · 2019-11-26T13:50:10Z

Thank you very much for the comment! This is a really good idea to decouple Gateway Provider from VPC. You proposal looks very good. Let's re-work this pull request to address it.

…-nat-gw

Pull request has been modified.

fogfish · 2019-11-27T11:58:28Z

@rix0rrr I've adjusted PR based on your suggestion.

aws-cdk-automation · 2019-11-27T12:03:22Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2019-11-27T12:27:54Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

fogfish · 2019-11-27T12:54:04Z

Hmm, Do you know why Lambda breaks it

@aws-cdk/aws-lambda... CHANGES.
Original assembly: @aws-cdk/aws-lambda@1.18.0
Updated assembly:  @aws-cdk/aws-lambda@1.16.1
API elements with incompatible changes:
err  - PROP @aws-cdk/aws-lambda.Runtime.JAVA_11: has been removed [removed:@aws-cdk/aws-lambda.Runtime.JAVA_11]
err  - PROP @aws-cdk/aws-lambda.Runtime.NODEJS_12_X: has been removed [removed:@aws-cdk/aws-lambda.Runtime.NODEJS_12_X]
err  - PROP @aws-cdk/aws-lambda.Runtime.PYTHON_3_8: has been removed [removed:@aws-cdk/aws-lambda.Runtime.PYTHON_3_8]

aws-cdk-automation · 2019-11-27T13:22:58Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

packages/@aws-cdk/aws-ec2/lib/nat.ts

Pull request has been modified.

aws-cdk-automation · 2019-11-28T08:45:07Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2019-11-28T09:35:46Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2019-11-28T10:15:02Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2019-11-28T10:16:11Z

AWS CodeBuild CI Report

Result: FAILED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

aws-cdk-automation · 2019-11-28T10:51:29Z

AWS CodeBuild CI Report

Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

fogfish · 2019-11-28T12:26:31Z

💃

Nice! Thank you a lot for support and valuable feedback.

fogfish added 2 commits November 11, 2019 14:23

feat(ec2): provide readonly access to NAT gateways in VPC

2f7d9ba

feat(ec2): provide readonly access to NAT gateways in VPC

b8f5f76

fogfish requested a review from rix0rrr as a code owner November 11, 2019 12:29

feat(ec2): fix linter issues

8792175

SomayaB assigned rix0rrr Nov 11, 2019

Merge branch 'master' into access-to-nat-gw

06e7f22

rix0rrr previously requested changes Nov 25, 2019

View reviewed changes

fogfish added 2 commits November 27, 2019 13:52

feat(ec2): split NAT gateways from VPC, provide list of GW via api

755505c

Merge remote-tracking branch 'github/access-to-nat-gw' into access-to…

a264be3

…-nat-gw

feat(ec2): add unit test for nat provider

83200e6

Merge branch 'master' into access-to-nat-gw

7691acf

rix0rrr previously requested changes Nov 27, 2019

View reviewed changes

packages/@aws-cdk/aws-ec2/lib/nat.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/aws-ec2/lib/nat.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/aws-ec2/lib/nat.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/aws-ec2/lib/nat.ts Show resolved Hide resolved

feat(ec2): add utility function to config subnet

b3e8ba7

Update nat.ts

f9864af

rix0rrr added 2 commits November 28, 2019 11:08

Update integ.nat-instances.lit.ts

1323cee

Update test.vpc.ts

0553c7e

feat(ec2): fix lint error

da2852b

rix0rrr added the pr/do-not-merge This PR should not be merged at this time. label Nov 28, 2019

rix0rrr approved these changes Nov 28, 2019

View reviewed changes

rix0rrr changed the title ~~Access to nat gw~~ feat(ec2): access gateways created by NatProvider Nov 28, 2019

rix0rrr merged commit 5907055 into aws:master Nov 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ec2): access gateways created by NatProvider #4948

feat(ec2): access gateways created by NatProvider #4948

fogfish commented Nov 11, 2019

mergify bot commented Nov 11, 2019

aws-cdk-automation commented Nov 11, 2019

aws-cdk-automation commented Nov 11, 2019

fogfish commented Nov 12, 2019

fogfish commented Nov 18, 2019

aws-cdk-automation commented Nov 18, 2019

fogfish commented Nov 25, 2019

rix0rrr left a comment

rix0rrr Nov 25, 2019

fogfish commented Nov 26, 2019

fogfish commented Nov 27, 2019

aws-cdk-automation commented Nov 27, 2019

aws-cdk-automation commented Nov 27, 2019

fogfish commented Nov 27, 2019

aws-cdk-automation commented Nov 27, 2019

aws-cdk-automation commented Nov 28, 2019

aws-cdk-automation commented Nov 28, 2019

aws-cdk-automation commented Nov 28, 2019

aws-cdk-automation commented Nov 28, 2019

aws-cdk-automation commented Nov 28, 2019

fogfish commented Nov 28, 2019

feat(ec2): access gateways created by NatProvider #4948

feat(ec2): access gateways created by NatProvider #4948

Conversation

fogfish commented Nov 11, 2019

mergify bot commented Nov 11, 2019

aws-cdk-automation commented Nov 11, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Nov 11, 2019

AWS CodeBuild CI Report

fogfish commented Nov 12, 2019

fogfish commented Nov 18, 2019

aws-cdk-automation commented Nov 18, 2019

AWS CodeBuild CI Report

fogfish commented Nov 25, 2019

rix0rrr left a comment

Choose a reason for hiding this comment

rix0rrr Nov 25, 2019

Choose a reason for hiding this comment

fogfish commented Nov 26, 2019

fogfish commented Nov 27, 2019

aws-cdk-automation commented Nov 27, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Nov 27, 2019

AWS CodeBuild CI Report

fogfish commented Nov 27, 2019

aws-cdk-automation commented Nov 27, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Nov 28, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Nov 28, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Nov 28, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Nov 28, 2019

AWS CodeBuild CI Report

aws-cdk-automation commented Nov 28, 2019

AWS CodeBuild CI Report

fogfish commented Nov 28, 2019