From 2d92a2218789418857875a167e131d36873c5c80 Mon Sep 17 00:00:00 2001 From: Nuru Date: Thu, 19 May 2022 14:03:07 -0700 Subject: [PATCH] Fix public-only subnets (#162) --- README.md | 2 +- docs/terraform.md | 2 +- nat-gateway.tf | 6 ++++++ nat-instance.tf | 2 ++ public.tf | 16 ++++++++-------- variables.tf | 5 ++++- 6 files changed, 22 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 23a99992..44cf1a5b 100644 --- a/README.md +++ b/README.md @@ -357,7 +357,7 @@ Available targets: | [label\_value\_case](#input\_label\_value\_case) | Controls the letter case of ID elements (labels) as included in `id`,
set as tag values, and output by this module individually.
Does not affect values of tags passed in via the `tags` input.
Possible values: `lower`, `title`, `upper` and `none` (no transformation).
Set this to `title` and set `delimiter` to `""` to yield Pascal Case IDs.
Default value: `lower`. | `string` | `null` | no | | [labels\_as\_tags](#input\_labels\_as\_tags) | Set of labels (ID elements) to include as tags in the `tags` output.
Default is to include all labels.
Tags with empty values will not be included in the `tags` output.
Set to `[]` to suppress all generated tags.
**Notes:**
The value of the `name` tag, if included, will be the `id`, not the `name`.
Unlike other `null-label` inputs, the initial setting of `labels_as_tags` cannot be
changed in later chained modules. Attempts to change it will be silently ignored. | `set(string)` | 
[
  "default"
]
| no | | [map\_public\_ip\_on\_launch](#input\_map\_public\_ip\_on\_launch) | If `true`, instances launched into a public subnet will be assigned a public IPv4 address | `bool` | `true` | no | -| [max\_nats](#input\_max\_nats) | Maximum number of NAT Gateways or NAT instances to create | `number` | `999` | no | +| [max\_nats](#input\_max\_nats) | Upper limit on number of NAT Gateways/Instances to create.
Set to 1 or 2 for cost savings at the expense of availability. | `number` | `999` | no | | [max\_subnet\_count](#input\_max\_subnet\_count) | Sets the maximum number of each type (public or private) of subnet to deploy.
0 will reserve a CIDR for every Availability Zone (excluding Local Zones) in the region, and
deploy a subnet in each availability zone specified in `availability_zones` or `availability_zone_ids`,
or every zone if none are specified. We recommend setting this equal to the maximum number of AZs you anticipate using,
to avoid causing subnets to be destroyed and recreated with smaller IPv4 CIDRs when AWS adds an availability zone.
Due to Terraform limitations, you can not set `max_subnet_count` from a computed value, you have to set it
from an explicit constant. For most cases, `3` is a good choice. | `number` | `0` | no | | [metadata\_http\_endpoint\_enabled](#input\_metadata\_http\_endpoint\_enabled) | Whether the metadata service is available on the created NAT instances | `bool` | `true` | no | | [metadata\_http\_put\_response\_hop\_limit](#input\_metadata\_http\_put\_response\_hop\_limit) | The desired HTTP PUT response hop limit (between 1 and 64) for instance metadata requests on the created NAT instances | `number` | `1` | no | diff --git a/docs/terraform.md b/docs/terraform.md index 91f413c5..d8f9b18c 100644 --- a/docs/terraform.md +++ b/docs/terraform.md @@ -98,7 +98,7 @@ | [label\_value\_case](#input\_label\_value\_case) | Controls the letter case of ID elements (labels) as included in `id`,
set as tag values, and output by this module individually.
Does not affect values of tags passed in via the `tags` input.
Possible values: `lower`, `title`, `upper` and `none` (no transformation).
Set this to `title` and set `delimiter` to `""` to yield Pascal Case IDs.
Default value: `lower`. | `string` | `null` | no | | [labels\_as\_tags](#input\_labels\_as\_tags) | Set of labels (ID elements) to include as tags in the `tags` output.
Default is to include all labels.
Tags with empty values will not be included in the `tags` output.
Set to `[]` to suppress all generated tags.
**Notes:**
The value of the `name` tag, if included, will be the `id`, not the `name`.
Unlike other `null-label` inputs, the initial setting of `labels_as_tags` cannot be
changed in later chained modules. Attempts to change it will be silently ignored. | `set(string)` | 
[
  "default"
]
| no | | [map\_public\_ip\_on\_launch](#input\_map\_public\_ip\_on\_launch) | If `true`, instances launched into a public subnet will be assigned a public IPv4 address | `bool` | `true` | no | -| [max\_nats](#input\_max\_nats) | Maximum number of NAT Gateways or NAT instances to create | `number` | `999` | no | +| [max\_nats](#input\_max\_nats) | Upper limit on number of NAT Gateways/Instances to create.
Set to 1 or 2 for cost savings at the expense of availability. | `number` | `999` | no | | [max\_subnet\_count](#input\_max\_subnet\_count) | Sets the maximum number of each type (public or private) of subnet to deploy.
0 will reserve a CIDR for every Availability Zone (excluding Local Zones) in the region, and
deploy a subnet in each availability zone specified in `availability_zones` or `availability_zone_ids`,
or every zone if none are specified. We recommend setting this equal to the maximum number of AZs you anticipate using,
to avoid causing subnets to be destroyed and recreated with smaller IPv4 CIDRs when AWS adds an availability zone.
Due to Terraform limitations, you can not set `max_subnet_count` from a computed value, you have to set it
from an explicit constant. For most cases, `3` is a good choice. | `number` | `0` | no | | [metadata\_http\_endpoint\_enabled](#input\_metadata\_http\_endpoint\_enabled) | Whether the metadata service is available on the created NAT instances | `bool` | `true` | no | | [metadata\_http\_put\_response\_hop\_limit](#input\_metadata\_http\_put\_response\_hop\_limit) | The desired HTTP PUT response hop limit (between 1 and 64) for instance metadata requests on the created NAT instances | `number` | `1` | no | diff --git a/nat-gateway.tf b/nat-gateway.tf index 5897dda4..617f6763 100644 --- a/nat-gateway.tf +++ b/nat-gateway.tf @@ -23,6 +23,8 @@ resource "aws_nat_gateway" "default" { depends_on = [aws_eip_association.nat_instance] } +# If private IPv4 subnets and NAT Gateway are both enabled, create a +# default route from private subnet to NAT Gateway in each subnet resource "aws_route" "nat4" { count = local.nat_gateway_enabled && local.private4_enabled ? local.private_route_table_count : 0 @@ -37,6 +39,8 @@ resource "aws_route" "nat4" { } } +# If private IPv6 subnet needs NAT64 and NAT Gateway is enabled, create a +# NAT64 route from private subnet to NAT Gateway in each subnet resource "aws_route" "private_nat64" { count = local.nat_gateway_enabled && local.private_dns64_enabled ? local.private_route_table_count : 0 @@ -51,6 +55,8 @@ resource "aws_route" "private_nat64" { } } +# If public IPv6 subnet needs NAT64 and NAT Gateway is enabled, create a +# NAT64 route from private subnet to NAT Gateway in each subnet resource "aws_route" "public_nat64" { count = local.nat_gateway_enabled && local.public_dns64_enabled ? local.public_route_table_count : 0 diff --git a/nat-instance.tf b/nat-instance.tf index b9d27d04..a6f781a4 100644 --- a/nat-instance.tf +++ b/nat-instance.tf @@ -124,6 +124,8 @@ resource "aws_eip_association" "nat_instance" { allocation_id = local.nat_eip_allocations[count.index] } +# If private IPv4 subnets and NAT Instance are both enabled, create a +# default route from private subnet to NAT Instance in each subnet resource "aws_route" "nat_instance" { count = local.nat_instance_enabled ? local.private_route_table_count : 0 diff --git a/public.tf b/public.tf index 45067991..a43ed8a7 100644 --- a/public.tf +++ b/public.tf @@ -21,18 +21,18 @@ resource "aws_subnet" "public" { # Use element()'s wrap-around behavior to handle the case where we are only provisioning public subnets. cidr_block = local.public4_enabled ? element(local.ipv4_public_subnet_cidrs, count.index) : null ipv6_cidr_block = local.public6_enabled ? element(local.ipv6_public_subnet_cidrs, count.index) : null - ipv6_native = local.public6_enabled && !local.private4_enabled + ipv6_native = local.public6_enabled && !local.public4_enabled #bridgecrew:skip=BC_AWS_NETWORKING_53:Public VPCs should be allowed to default to public IPs - map_public_ip_on_launch = local.private4_enabled ? var.map_public_ip_on_launch : null + map_public_ip_on_launch = local.public4_enabled ? var.map_public_ip_on_launch : null assign_ipv6_address_on_creation = local.public6_enabled ? var.public_assign_ipv6_address_on_creation : null enable_dns64 = local.public6_enabled ? local.public_dns64_enabled : null - enable_resource_name_dns_a_record_on_launch = local.private4_enabled ? var.ipv4_public_instance_hostnames_enabled : null - enable_resource_name_dns_aaaa_record_on_launch = local.public6_enabled ? var.ipv6_public_instance_hostnames_enabled || !local.private4_enabled : null + enable_resource_name_dns_a_record_on_launch = local.public4_enabled ? var.ipv4_public_instance_hostnames_enabled : null + enable_resource_name_dns_aaaa_record_on_launch = local.public6_enabled ? var.ipv6_public_instance_hostnames_enabled || !local.public4_enabled : null - private_dns_hostname_type_on_launch = local.private4_enabled ? var.ipv4_public_instance_hostname_type : null + private_dns_hostname_type_on_launch = local.public4_enabled ? var.ipv4_public_instance_hostname_type : null tags = merge( @@ -62,7 +62,7 @@ resource "aws_route_table" "public" { } resource "aws_route" "public" { - count = local.private4_enabled && local.igw_configured ? local.public_route_table_count : 0 + count = local.public4_enabled && local.igw_configured ? local.public_route_table_count : 0 route_table_id = local.public_route_table_ids[count.index] destination_cidr_block = "0.0.0.0/0" @@ -104,7 +104,7 @@ resource "aws_network_acl" "public" { } resource "aws_network_acl_rule" "public4_ingress" { - count = local.public_open_network_acl_enabled && local.private4_enabled ? 1 : 0 + count = local.public_open_network_acl_enabled && local.public4_enabled ? 1 : 0 network_acl_id = aws_network_acl.public[0].id rule_action = "allow" @@ -118,7 +118,7 @@ resource "aws_network_acl_rule" "public4_ingress" { } resource "aws_network_acl_rule" "public4_egress" { - count = local.public_open_network_acl_enabled && local.private4_enabled ? 1 : 0 + count = local.public_open_network_acl_enabled && local.public4_enabled ? 1 : 0 network_acl_id = aws_network_acl.public[0].id rule_action = "allow" diff --git a/variables.tf b/variables.tf index 56148e4c..f41dcefa 100644 --- a/variables.tf +++ b/variables.tf @@ -45,7 +45,10 @@ variable "max_subnet_count" { variable "max_nats" { type = number - description = "Maximum number of NAT Gateways or NAT instances to create" + description = <<-EOT + Upper limit on number of NAT Gateways/Instances to create. + Set to 1 or 2 for cost savings at the expense of availability. + EOT # Default should be MAX_INT, but Terraform does not provide that. 999 is big enough. default = 999 }