fix(vpc): correct NAT gateway AZ mapping logic (#1257)

Author: Manoj-Kumar-Selvaraj

README.md

@@ -336,6 +336,7 @@ No modules.
 | [aws_vpn_gateway_route_propagation.intra](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/vpn_gateway_route_propagation) | resource |
 | [aws_vpn_gateway_route_propagation.private](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/vpn_gateway_route_propagation) | resource |
 | [aws_vpn_gateway_route_propagation.public](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/vpn_gateway_route_propagation) | resource |
+| [aws_availability_zones.available](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/availability_zones) | data source |
 | [aws_caller_identity.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/caller_identity) | data source |
 | [aws_iam_policy_document.flow_log_cloudwatch_assume_role](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.vpc_flow_log_cloudwatch](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
@@ -488,6 +489,7 @@ No modules.
 | <a name="input_name"></a> [name](#input\_name) | Name to be used on all the resources as identifier | `string` | `""` | no |
 | <a name="input_nat_eip_tags"></a> [nat\_eip\_tags](#input\_nat\_eip\_tags) | Additional tags for the NAT EIP | `map(string)` | `{}` | no |
 | <a name="input_nat_gateway_destination_cidr_block"></a> [nat\_gateway\_destination\_cidr\_block](#input\_nat\_gateway\_destination\_cidr\_block) | Used to pass a custom destination route for private NAT Gateway. If not specified, the default 0.0.0.0/0 is used as a destination route | `string` | `"0.0.0.0/0"` | no |
+| <a name="input_nat_gateway_subnet_ids"></a> [nat\_gateway\_subnet\_ids](#input\_nat\_gateway\_subnet\_ids) | List of subnet IDs to use for NAT Gateways. If provided, these subnet IDs will be used (in order). If empty, the module will automatically select the public subnet that matches each Availability Zone (AZ). | `list(string)` | `[]` | no |
 | <a name="input_nat_gateway_tags"></a> [nat\_gateway\_tags](#input\_nat\_gateway\_tags) | Additional tags for the NAT gateways | `map(string)` | `{}` | no |
 | <a name="input_one_nat_gateway_per_az"></a> [one\_nat\_gateway\_per\_az](#input\_one\_nat\_gateway\_per\_az) | Should be true if you want only one NAT Gateway per availability zone. Requires `var.azs` to be set, and the number of `public_subnets` created to be greater than or equal to the number of availability zones specified in `var.azs` | `bool` | `false` | no |
 | <a name="input_outpost_acl_tags"></a> [outpost\_acl\_tags](#input\_outpost\_acl\_tags) | Additional tags for the outpost subnets network ACL | `map(string)` | `{}` | no |

examples/complete/main.tf

@@ -9,8 +9,8 @@ locals {
   region = "eu-west-1"
 
   vpc_cidr = "10.0.0.0/16"
-  azs      = slice(data.aws_availability_zones.available.names, 0, 3)
-
+  azs      = slice(sort(data.aws_availability_zones.available.names), 0, 3)
+  #azs      = slice(sort(data.aws_availability_zones.available.names), 0, 2) #Tested with 2 AZs
   tags = {
     Example    = local.name
     GithubRepo = "terraform-aws-vpc"
@@ -28,9 +28,11 @@ module "vpc" {
   name = local.name
   cidr = local.vpc_cidr
 
-  azs                 = local.azs
-  private_subnets     = [for k, v in local.azs : cidrsubnet(local.vpc_cidr, 8, k)]
-  public_subnets      = [for k, v in local.azs : cidrsubnet(local.vpc_cidr, 8, k + 4)]
+  azs             = local.azs
+  private_subnets = [for k, v in local.azs : cidrsubnet(local.vpc_cidr, 8, k)]
+  #4 Subnets created for 3 AZs to test the NAT gateways are created in correct availability zone
+  public_subnets = [for i in range(4) : cidrsubnet(local.vpc_cidr, 8, i + 4)]
+  #public_subnets      = [for k, v in local.azs : cidrsubnet(local.vpc_cidr, 8, k + 4)]
   database_subnets    = [for k, v in local.azs : cidrsubnet(local.vpc_cidr, 8, k + 8)]
   elasticache_subnets = [for k, v in local.azs : cidrsubnet(local.vpc_cidr, 8, k + 12)]
   redshift_subnets    = [for k, v in local.azs : cidrsubnet(local.vpc_cidr, 8, k + 16)]
@@ -41,19 +43,20 @@ module "vpc" {
   database_subnet_names    = ["DB Subnet One"]
   elasticache_subnet_names = ["Elasticache Subnet One", "Elasticache Subnet Two"]
   redshift_subnet_names    = ["Redshift Subnet One", "Redshift Subnet Two", "Redshift Subnet Three"]
-  intra_subnet_names       = []
 
   create_database_subnet_group  = false
   manage_default_network_acl    = false
   manage_default_route_table    = false
   manage_default_security_group = false
 
+  # NAT Gateway Configuration
+  enable_nat_gateway     = true
+  one_nat_gateway_per_az = true # Module will automatically map NAT Gateways to first public subnet in each AZ
+  single_nat_gateway     = false
+
   enable_dns_hostnames = true
   enable_dns_support   = true
 
-  enable_nat_gateway = true
-  single_nat_gateway = true
-
   customer_gateways = {
     IP1 = {
       bgp_asn     = 65112

variables.tf

@@ -1245,20 +1245,6 @@ variable "one_nat_gateway_per_az" {
   default     = false
 }
 
-variable "nat_gateway_subnet_ids" {
-  description = <<EOT
-Optional list of subnet IDs to use for NAT Gateways. If provided, these
-subnet IDs will be used (in order). If empty, the module will automatically
-select the public subnet that matches each Availability Zone (AZ).
-EOT
-  validation {
-    condition = alltrue([for id in var.nat_gateway_subnet_ids : id != ""])
-    error_message = "nat_gateway_subnet_ids must not contain empty strings."
-  }
-  type    = list(string)
-  default = []
-}
-
 variable "reuse_nat_ips" {
   description = "Should be true if you don't want EIPs to be created for your NAT Gateways and will instead pass them in via the 'external_nat_ip_ids' variable"
   type        = bool

wrappers/main.tf

@@ -233,6 +233,7 @@ module "wrapper" {
   name                                                        = try(each.value.name, var.defaults.name, "")
   nat_eip_tags                                                = try(each.value.nat_eip_tags, var.defaults.nat_eip_tags, {})
   nat_gateway_destination_cidr_block                          = try(each.value.nat_gateway_destination_cidr_block, var.defaults.nat_gateway_destination_cidr_block, "0.0.0.0/0")
+  nat_gateway_subnet_ids                                      = try(each.value.nat_gateway_subnet_ids, var.defaults.nat_gateway_subnet_ids, [])
   nat_gateway_tags                                            = try(each.value.nat_gateway_tags, var.defaults.nat_gateway_tags, {})
   one_nat_gateway_per_az                                      = try(each.value.one_nat_gateway_per_az, var.defaults.one_nat_gateway_per_az, false)
   outpost_acl_tags                                            = try(each.value.outpost_acl_tags, var.defaults.outpost_acl_tags, {})