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.

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

(aws-ec2): Why are NAT gateways required for private subnets? #15929

Closed
ffxsam opened this issue Aug 7, 2021 · 8 comments · Fixed by #16348
Closed

(aws-ec2): Why are NAT gateways required for private subnets? #15929

ffxsam opened this issue Aug 7, 2021 · 8 comments · Fixed by #16348
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@ffxsam
Copy link

ffxsam commented Aug 7, 2021

❓ General Issue

The Question

I'm very curious why this check is in place:

if (count === 0 && hasPrivateSubnets) {
// eslint-disable-next-line max-len
throw new Error('If you do not want NAT gateways (natGateways=0), make sure you don\'t configure any PRIVATE subnets in \'subnetConfiguration\' (make them PUBLIC or ISOLATED instead)');
}

NAT gateways are, if I'm not mistaken, only required if resources within your private subnets need to access the Internet through an Internet gateway. In this case, I'm unable to use CDK to create a ServerlessCluster, because the SubnetGroup can't be defined without giving it a VPC with private subnets, which strangely require NAT Gateways. If I try to create a SubnetGroup using a VPC with public & isolated subnets, it complains and says I need to use private subnets: Error: There are no 'Private' subnet groups in this VPC. Available types: Isolated,Public

@ffxsam ffxsam added guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Aug 7, 2021
@github-actions github-actions bot added the @aws-cdk/aws-rds Related to Amazon Relational Database label Aug 7, 2021
@peterwoodworth peterwoodworth added @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud and removed @aws-cdk/aws-rds Related to Amazon Relational Database labels Aug 9, 2021
@peterwoodworth peterwoodworth changed the title (aws-rds): Why are NAT gateways required for private subnets? (aws-ec2): Why are NAT gateways required for private subnets? Aug 9, 2021
@njlynch
Copy link
Contributor

njlynch commented Aug 10, 2021

It's a great question.

It looks like there has been some question about this since the original implementation:

* Do we want to require that there are private subnets if there are NatGateways?
* They seem pointless but I see no reason to prevent it.

It also looks like there was an unaddressed comment on that PR: "Perhaps provide the rationale for this in the error message or an accompanying link to an issue or doc".

While I think it's likely that in the most common scenario, this check makes sense, I can also see infrastructure setups where it doesn't. Given that there's no work-around here (short of using some rather ugly escape hatches), I'm re-labelling this as a P1 bug.

We welcome community contributions! If you are able, we encourage you to contribute the bug fix. I think it's safe to just remove this check (and associated test(s)).

@njlynch njlynch added bug This issue is a bug. effort/small Small work item – less than a day of effort p1 and removed guidance Question that needs advice or information. needs-triage This issue or PR still needs to be triaged. labels Aug 10, 2021
@njlynch njlynch removed their assignment Aug 10, 2021
@ffxsam
Copy link
Author

ffxsam commented Aug 10, 2021

This is what I find interesting/confusing:

Referencing that comment above:
* Do we want to require that there are private subnets if there are NatGateways?

The thing is, that code check does the opposite of this: it requires there to be a NAT Gateway if there are private subnets, which makes no sense.

My ultimate concern here is that people new to AWS might mistakenly think the NAT Gateway is required (and potentially 3 of them!), costing roughly $90/mo for no reason.

Totally happy to open a PR and remove that check! I've already done so in my local node_modules folder which isn't ideal.

Sidebar: The concept of "isolated" subnets is not a thing, if I'm not mistaken. There are only private (not in the route table, connected to an IGW) or public. Seems like in the context of CDK, "isolated" means a private subnet that isn't connected to the external Internet in any way, and "private" means a private subnet that will go into the route table and be connected to a NAT Gateway. IMO (and I know this is maybe controversial), "isolated" should be phased out completely.

Reference: https://docs.aws.amazon.com/vpc/latest/userguide/VPC_Subnets.html

Note there's no mention of "isolated subnet." This creates a conceptual rift between CDK & AWS documentation, which could cause confusion as people new to AWS search the docs for info on isolated subnets.

My vote would be to align more closely with AWS docs and conventions, therefore:

  • Subnets can only be private or public
  • NAT Gateways are always optional

I'm totally open to hearing any reasons why this may not be a great idea.

@rix0rrr
Copy link
Contributor

rix0rrr commented Aug 18, 2021

CDK currently has the following conventions:

  • Public subnet: subnet directly routing to the internet
  • Private subnet: subnet routing to the internet via NAT (either a NAT gateway or NAT instance)
  • Isolated subnet: subnet not routing to the internet

This is explained pretty much at the top of the EC2 README: https://docs.aws.amazon.com/cdk/api/latest/docs/aws-ec2-readme.html#subnet-types

So to answer your original question: why is NAT required for a private subnet? because by definition, a private subnet is one that routes through NAT.


As to why your ServerlessCluster cluster doesn't create: you need to tell it which subnets to use:

{
  vpcSubnets: { subnetType: SubnetType.ISOLATED }
}

As explained in https://docs.aws.amazon.com/cdk/api/latest/docs/aws-ec2-readme.html#choosing-subnets-for-resources

The default here is somewhat unfortunate: ServerlessCluster is imposing its own default of PRIVATE. If it hadn't done that, the VPC construct would have automatically picked your ISOLATED subnets for you.

C'lest la vie. This is worth a bug report to the RDS library.


As for if it's worth CDK making a distinction between PRIVATE and ISOLATED that EC2 itself does not... that's a fair question. This decision was based on some internal conventions we had inside the organization, but perhaps we should not have done that. Whatever the case, it will be a tough choice to undo with lots of implications and complications, so the upside should be very clearly worth it.

@ffxsam
Copy link
Author

ffxsam commented Aug 18, 2021

So to answer your original question: why is NAT required for a private subnet? because by definition, a private subnet is one that routes through NAT.

It seems like CDK conventions & AWS conventions are at odds, which is confusing, because CDK is an Amazon tool.

AWS docs state:

The instances in the public subnet can send outbound traffic directly to the internet, whereas the instances in the private subnet can't. Instead, the instances in the private subnet can [emphasis mine] access the internet by using a network address translation (NAT) gateway that resides in the public subnet.

(Source: https://docs.aws.amazon.com/vpc/latest/userguide/VPC_Scenario2.html)

Having a NAT Gateway is optional. Resources on private subnets may not need to connect to the Internet at all. Another reference to NAT Gateway being optional:

The instance 2A can't reach the internet, but can reach other instances in the VPC. You can allow an instance in your VPC to initiate outbound connections to the internet over IPv4 but prevent unsolicited inbound connections from the internet using a network address translation (NAT) gateway or instance.

(Source: https://docs.aws.amazon.com/vpc/latest/userguide/VPC_Subnets.html)

Contrast the above with CDK's docs:

Private - instances in private subnets are not directly routable from the Internet, and connect out to the Internet via a NAT gateway. By default, a NAT gateway is created in every public subnet for maximum availability. Be aware that you will be charged for NAT gateways.

Again, my main concern is for people new to AWS who might mistakenly believe they need a NAT Gateway to use private subnets, and incur high monthly costs. The way CDK is set up and documented is misleading.

I did eventually arrive at a solution with the serverless cluster (before I saw your post):

  vpcSubnets: { subnetType: ec2.SubnetType.PUBLIC },

I'm using the default VPC which doesn't have private subnets, and there doesn't seem to be any security issue with using public subnets, since the RDS endpoint is still unreachable to the outside world. I suppose, to be on the safe side, I should figure out how to just tack on a few private subnets to my existing default VPC and use those instead.

@ffxsam
Copy link
Author

ffxsam commented Aug 25, 2021

Case in point:

CleanShot 2021-08-19 at 09 33 29

njlynch added a commit that referenced this issue Sep 2, 2021
Early on in the CDK history, a decision was made to delineate between subnets
with Internet access (i.e., those with a NAT) and those without. The convention
chosen at that time was to label the subnets as `PRIVATE` and `ISOLATED`,
respectively. The intent was to make it clear that subnets without a NAT were
completely isolated from the broader Internet (unless connected through another
subnet).

However, this introduction of a new subnet type that does not match EC2
documentation and naming conventions can cause confusion. Most critically, a
user may select a `PRIVATE` subnet without realizing that it automatically
requires one (or more) NAT gateways. As NAT gateways are not free, this can
lead to unintended charges.

To realign to the EC2 terminology -- while retaining the existing logic
surrounding SubnetTypes -- the existing types of `PRIVATE` and `ISOLATED` are
being renamed to `PRIVATE_WITH_NAT` and `PRIVATE_ISOLATED`, respectively.

fixes #15929
@njlynch
Copy link
Contributor

njlynch commented Sep 2, 2021

@ffxsam:

It's a fair criticism that our concept of ISOLATED may cause unintentional confusion. I've submitted a PR to rename PRIVATE and ISOLATED to PRIVATE_WITH_NAT and PRIVATE_ISOLATED to hopefully make this clearer.

@ffxsam
Copy link
Author

ffxsam commented Sep 2, 2021

@njlynch Thanks! I think that's much clearer. And I hope some modifications will be made so that CDK doesn't require NAT gateways in instances where they're not really required.

@mergify mergify bot closed this as completed in #16348 Sep 3, 2021
mergify bot pushed a commit that referenced this issue Sep 3, 2021
…#16348)

Early on in the CDK history, a decision was made to delineate between subnets
with Internet access (i.e., those with a NAT) and those without. The convention
chosen at that time was to label the subnets as `PRIVATE` and `ISOLATED`,
respectively. The intent was to make it clear that subnets without a NAT were
completely isolated from the broader Internet (unless connected through another
subnet).

However, this introduction of a new subnet type that does not match EC2
documentation and naming conventions can cause confusion. Most critically, a
user may select a `PRIVATE` subnet without realizing that it automatically
requires one (or more) NAT gateways. As NAT gateways are not free, this can
lead to unintended charges.

To realign to the EC2 terminology -- while retaining the existing logic
surrounding SubnetTypes -- the existing types of `PRIVATE` and `ISOLATED` are
being renamed to `PRIVATE_WITH_NAT` and `PRIVATE_ISOLATED`, respectively.

fixes #15929


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Sep 3, 2021

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Sep 6, 2021
…aws#16348)

Early on in the CDK history, a decision was made to delineate between subnets
with Internet access (i.e., those with a NAT) and those without. The convention
chosen at that time was to label the subnets as `PRIVATE` and `ISOLATED`,
respectively. The intent was to make it clear that subnets without a NAT were
completely isolated from the broader Internet (unless connected through another
subnet).

However, this introduction of a new subnet type that does not match EC2
documentation and naming conventions can cause confusion. Most critically, a
user may select a `PRIVATE` subnet without realizing that it automatically
requires one (or more) NAT gateways. As NAT gateways are not free, this can
lead to unintended charges.

To realign to the EC2 terminology -- while retaining the existing logic
surrounding SubnetTypes -- the existing types of `PRIVATE` and `ISOLATED` are
being renamed to `PRIVATE_WITH_NAT` and `PRIVATE_ISOLATED`, respectively.

fixes aws#15929


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
david-doyle-as24 pushed a commit to david-doyle-as24/aws-cdk that referenced this issue Sep 7, 2021
…aws#16348)

Early on in the CDK history, a decision was made to delineate between subnets
with Internet access (i.e., those with a NAT) and those without. The convention
chosen at that time was to label the subnets as `PRIVATE` and `ISOLATED`,
respectively. The intent was to make it clear that subnets without a NAT were
completely isolated from the broader Internet (unless connected through another
subnet).

However, this introduction of a new subnet type that does not match EC2
documentation and naming conventions can cause confusion. Most critically, a
user may select a `PRIVATE` subnet without realizing that it automatically
requires one (or more) NAT gateways. As NAT gateways are not free, this can
lead to unintended charges.

To realign to the EC2 terminology -- while retaining the existing logic
surrounding SubnetTypes -- the existing types of `PRIVATE` and `ISOLATED` are
being renamed to `PRIVATE_WITH_NAT` and `PRIVATE_ISOLATED`, respectively.

fixes aws#15929


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants