From a590deb58b9919be23d464a9ffec025d6d32d508 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Fri, 22 Mar 2024 15:59:02 +0100 Subject: [PATCH] Fix subnet configuration in cloud nat module (#2171) * support optional secondary ranges in net-cloudnat module * fix subnet configuration * fix packer blueprint --- .../packer-image-builder/main.tf | 26 +++---- modules/net-cloudnat/README.md | 72 ++++++++++--------- modules/net-cloudnat/main.tf | 70 ++++++++++++------ modules/net-cloudnat/variables.tf | 34 ++++----- .../net_cloudnat/examples/subnets.yaml | 68 ++++++++++++++++++ 5 files changed, 184 insertions(+), 86 deletions(-) create mode 100644 tests/modules/net_cloudnat/examples/subnets.yaml diff --git a/blueprints/cloud-operations/packer-image-builder/main.tf b/blueprints/cloud-operations/packer-image-builder/main.tf index f6de3af5..2f6b0ba7 100644 --- a/blueprints/cloud-operations/packer-image-builder/main.tf +++ b/blueprints/cloud-operations/packer-image-builder/main.tf @@ -82,19 +82,19 @@ module "firewall" { } module "nat" { - source = "../../../modules/net-cloudnat" - project_id = module.project.project_id - region = var.region - name = "default" - router_network = module.vpc.name - config_source_subnets = "LIST_OF_SUBNETWORKS" - subnetworks = [ - { - self_link = module.vpc.subnet_self_links["${var.region}/${local.compute_subnet_name}"] - config_source_ranges = ["ALL_IP_RANGES"] - secondary_ranges = null - } - ] + source = "../../../modules/net-cloudnat" + project_id = module.project.project_id + region = var.region + name = "default" + router_network = module.vpc.name + config_source_subnetworks = { + all = false + subnetworks = [ + { + self_link = module.vpc.subnet_self_links["${var.region}/${local.compute_subnet_name}"] + } + ] + } } resource "google_service_account_iam_binding" "sa-image-builder-token-creators" { diff --git a/modules/net-cloudnat/README.md b/modules/net-cloudnat/README.md index 1ece2547..407bce3f 100644 --- a/modules/net-cloudnat/README.md +++ b/modules/net-cloudnat/README.md @@ -4,7 +4,7 @@ Simple Cloud NAT management, with optional router creation. - [Basic Example](#basic-example) -- [Subnetwork ranges](#subnetwork-ranges) +- [Subnetwork configuration](#subnetwork-configuration) - [Reserved IPs and custom rules](#reserved-ips-and-custom-rules) - [Variables](#variables) - [Outputs](#outputs) @@ -23,11 +23,15 @@ module "nat" { # tftest modules=1 resources=2 e2e ``` -## Subnetwork ranges +## Subnetwork configuration -When specifying subnets the default for IP ranges is to consider all ranges (primary and secondaries). +Subnetwork configuration is defined via the `config_source_subnetworks` variable: -More control can be obtained via the `all_ip_ranges` subnetwork attribute: when set to `false` only the primary subnet range is considered, unless secondary ranges are specified via the `secondary_ranges` attribute. +- the default is to configure all ranges for all subnets +- to only configure primary ranges set `config_source_subnetworks.primary_ranges_only` to `true` +- to specify a list of subnets set `config_source_subnetworks.all` to `false` and provide a list of subnets in `config_source_subnetworks.subnetworks` + +When specifying subnets the default for IP ranges is to consider all ranges (primary and secondaries). More control can be obtained via the `all` subnetwork attribute: when set to `false` only the primary subnet range is considered, unless secondary ranges are specified via the `secondary_ranges` attribute. ```hcl module "nat" { @@ -36,23 +40,26 @@ module "nat" { region = var.region name = "default" router_network = var.vpc.self_link - subnetworks = [ - { - # all ip ranges - self_link = "projects/foo/regions/europe-west1/subnetworks/net" - }, - { - # primary range only - self_link = "projects/foo/regions/europe-west3/subnetworks/net" - all_ip_ranges = false - }, - { - # both primary and specified secondary ranges - self_link = "projects/foo/regions/europe-west8/subnetworks/net" - all_ip_ranges = false - secondary_ranges = ["pods"] - } - ] + config_source_subnetworks = { + all = false + subnetworks = [ + { + # all ip ranges + self_link = "projects/${var.project_id}/regions/${var.region}/subnetworks/net-0" + }, + { + # primary range only + self_link = "projects/${var.project_id}/regions/${var.region}/subnetworks/net-1" + all_ranges = false + }, + { + # both primary and specified secondary ranges + self_link = "projects/${var.project_id}/regions/${var.region}/subnetworks/net-2" + all_ranges = false + secondary_ranges = ["pods"] + } + ] + } } # tftest modules=1 resources=2 ``` @@ -100,20 +107,19 @@ module "nat" { | name | description | type | required | default | |---|---|:---:|:---:|:---:| -| [name](variables.tf#L64) | Name of the Cloud NAT resource. | string | ✓ | | -| [project_id](variables.tf#L69) | Project where resources will be created. | string | ✓ | | -| [region](variables.tf#L74) | Region where resources will be created. | string | ✓ | | +| [name](variables.tf#L77) | Name of the Cloud NAT resource. | string | ✓ | | +| [project_id](variables.tf#L82) | Project where resources will be created. | string | ✓ | | +| [region](variables.tf#L87) | Region where resources will be created. | string | ✓ | | | [addresses](variables.tf#L17) | Optional list of external address self links. | list(string) | | [] | | [config_port_allocation](variables.tf#L23) | Configuration for how to assign ports to virtual machines. min_ports_per_vm and max_ports_per_vm have no effect unless enable_dynamic_port_allocation is set to 'true'. | object({…}) | | {} | -| [config_source_subnets](variables.tf#L39) | Subnetwork configuration (ALL_SUBNETWORKS_ALL_IP_RANGES, ALL_SUBNETWORKS_ALL_PRIMARY_IP_RANGES, LIST_OF_SUBNETWORKS). | string | | "ALL_SUBNETWORKS_ALL_IP_RANGES" | -| [config_timeouts](variables.tf#L45) | Timeout configurations. | object({…}) | | {} | -| [logging_filter](variables.tf#L58) | Enables logging if not null, value is one of 'ERRORS_ONLY', 'TRANSLATIONS_ONLY', 'ALL'. | string | | null | -| [router_asn](variables.tf#L79) | Router ASN used for auto-created router. | number | | null | -| [router_create](variables.tf#L85) | Create router. | bool | | true | -| [router_name](variables.tf#L91) | Router name, leave blank if router will be created to use auto generated name. | string | | null | -| [router_network](variables.tf#L97) | Name of the VPC used for auto-created router. | string | | null | -| [rules](variables.tf#L103) | List of rules associated with this NAT. | list(object({…})) | | [] | -| [subnetworks](variables.tf#L114) | Subnetworks to NAT, only used when config_source_subnets equals LIST_OF_SUBNETWORKS. | list(object({…})) | | [] | +| [config_source_subnetworks](variables.tf#L39) | Subnetwork configuration. | object({…}) | | {} | +| [config_timeouts](variables.tf#L58) | Timeout configurations. | object({…}) | | {} | +| [logging_filter](variables.tf#L71) | Enables logging if not null, value is one of 'ERRORS_ONLY', 'TRANSLATIONS_ONLY', 'ALL'. | string | | null | +| [router_asn](variables.tf#L92) | Router ASN used for auto-created router. | number | | null | +| [router_create](variables.tf#L98) | Create router. | bool | | true | +| [router_name](variables.tf#L104) | Router name, leave blank if router will be created to use auto generated name. | string | | null | +| [router_network](variables.tf#L110) | Name of the VPC used for auto-created router. | string | | null | +| [rules](variables.tf#L116) | List of rules associated with this NAT. | list(object({…})) | | [] | ## Outputs diff --git a/modules/net-cloudnat/main.tf b/modules/net-cloudnat/main.tf index fbc32edc..7a537d8e 100644 --- a/modules/net-cloudnat/main.tf +++ b/modules/net-cloudnat/main.tf @@ -20,6 +20,15 @@ locals { ? try(google_compute_router.router[0].name, null) : var.router_name ) + subnet_config = ( + var.config_source_subnetworks.all != true + ? "LIST_OF_SUBNETWORKS" + : ( + var.config_source_subnetworks.primary_ranges_only == true + ? "ALL_SUBNETWORKS_ALL_PRIMARY_IP_RANGES" + : "ALL_SUBNETWORKS_ALL_IP_RANGES" + ) + ) } resource "google_compute_router" "router" { @@ -38,22 +47,32 @@ resource "google_compute_router" "router" { } resource "google_compute_router_nat" "nat" { - project = var.project_id - region = var.region - name = var.name - router = local.router_name - nat_ips = var.addresses - nat_ip_allocate_option = length(var.addresses) > 0 ? "MANUAL_ONLY" : "AUTO_ONLY" - source_subnetwork_ip_ranges_to_nat = var.config_source_subnets - icmp_idle_timeout_sec = var.config_timeouts.icmp - udp_idle_timeout_sec = var.config_timeouts.udp - tcp_established_idle_timeout_sec = var.config_timeouts.tcp_established - tcp_time_wait_timeout_sec = var.config_timeouts.tcp_time_wait - tcp_transitory_idle_timeout_sec = var.config_timeouts.tcp_transitory - enable_endpoint_independent_mapping = var.config_port_allocation.enable_endpoint_independent_mapping - enable_dynamic_port_allocation = var.config_port_allocation.enable_dynamic_port_allocation - min_ports_per_vm = var.config_port_allocation.min_ports_per_vm - max_ports_per_vm = var.config_port_allocation.max_ports_per_vm + project = var.project_id + region = var.region + name = var.name + router = local.router_name + nat_ips = var.addresses + nat_ip_allocate_option = ( + length(var.addresses) > 0 ? "MANUAL_ONLY" : "AUTO_ONLY" + ) + source_subnetwork_ip_ranges_to_nat = local.subnet_config + icmp_idle_timeout_sec = var.config_timeouts.icmp + udp_idle_timeout_sec = var.config_timeouts.udp + tcp_established_idle_timeout_sec = var.config_timeouts.tcp_established + tcp_time_wait_timeout_sec = var.config_timeouts.tcp_time_wait + tcp_transitory_idle_timeout_sec = var.config_timeouts.tcp_transitory + enable_endpoint_independent_mapping = ( + var.config_port_allocation.enable_endpoint_independent_mapping + ) + enable_dynamic_port_allocation = ( + var.config_port_allocation.enable_dynamic_port_allocation + ) + min_ports_per_vm = ( + var.config_port_allocation.min_ports_per_vm + ) + max_ports_per_vm = ( + var.config_port_allocation.max_ports_per_vm + ) log_config { enable = var.logging_filter == null ? false : true @@ -61,24 +80,29 @@ resource "google_compute_router_nat" "nat" { } dynamic "subnetwork" { - for_each = var.subnetworks - iterator = subnet + for_each = toset( + local.subnet_config == "LIST_OF_SUBNETWORKS" + ? var.config_source_subnetworks.subnetworks + : [] + ) content { - name = subnet.value.self_link + name = subnetwork.value.self_link source_ip_ranges_to_nat = ( - subnet.value.all_ip_ranges == true ? ["ALL_IP_RANGES"] : concat( + subnetwork.value.all_ranges == true + ? ["ALL_IP_RANGES"] + : concat( ["PRIMARY_IP_RANGE"], ( - subnet.value.secondary_ranges == null + subnetwork.value.secondary_ranges == null ? [] : ["LIST_OF_SECONDARY_IP_RANGES"] ) ) ) secondary_ip_range_names = ( - subnet.value.all_ip_ranges == true + subnetwork.value.all_ranges == true ? null - : subnet.value.secondary_ranges + : subnetwork.value.secondary_ranges ) } } diff --git a/modules/net-cloudnat/variables.tf b/modules/net-cloudnat/variables.tf index 2a14955b..0ec05e42 100644 --- a/modules/net-cloudnat/variables.tf +++ b/modules/net-cloudnat/variables.tf @@ -36,10 +36,23 @@ variable "config_port_allocation" { } } -variable "config_source_subnets" { - description = "Subnetwork configuration (ALL_SUBNETWORKS_ALL_IP_RANGES, ALL_SUBNETWORKS_ALL_PRIMARY_IP_RANGES, LIST_OF_SUBNETWORKS)." - type = string - default = "ALL_SUBNETWORKS_ALL_IP_RANGES" +variable "config_source_subnetworks" { + description = "Subnetwork configuration." + type = object({ + all = optional(bool, true) + primary_ranges_only = optional(bool) + subnetworks = optional(list(object({ + self_link = string + all_ranges = optional(bool, true) + secondary_ranges = optional(list(string)) + })), []) + }) + nullable = false + default = {} +} + +output "foo" { + value = var.config_source_subnetworks.subnetworks } variable "config_timeouts" { @@ -110,16 +123,3 @@ variable "rules" { default = [] nullable = false } - -variable "subnetworks" { - description = "Subnetworks to NAT, only used when config_source_subnets equals LIST_OF_SUBNETWORKS." - type = list(object({ - self_link = string - all_ip_ranges = optional(bool, true) - # secondary ranges are only meaningful when all_ip_ranges is false - # in that case we populate config with LIST_OF_SECONDARY_IP_RANGES - # only if secondary_ranges is not null - secondary_ranges = optional(list(string)) - })) - default = [] -} diff --git a/tests/modules/net_cloudnat/examples/subnets.yaml b/tests/modules/net_cloudnat/examples/subnets.yaml new file mode 100644 index 00000000..3d0d80d5 --- /dev/null +++ b/tests/modules/net_cloudnat/examples/subnets.yaml @@ -0,0 +1,68 @@ +# Copyright 2024 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +values: + module.nat.google_compute_router.router[0]: + bgp: [] + description: null + encrypted_interconnect_router: null + name: default-nat + network: projects/xxx/global/networks/aaa + project: project-id + region: europe-west8 + timeouts: null + module.nat.google_compute_router_nat.nat: + drain_nat_ips: null + enable_dynamic_port_allocation: false + enable_endpoint_independent_mapping: true + icmp_idle_timeout_sec: 30 + log_config: + - enable: false + filter: ALL + max_ports_per_vm: 65536 + min_ports_per_vm: 64 + name: default + nat_ip_allocate_option: AUTO_ONLY + nat_ips: null + project: project-id + region: europe-west8 + router: default-nat + rules: [] + source_subnetwork_ip_ranges_to_nat: LIST_OF_SUBNETWORKS + subnetwork: + - name: projects/project-id/regions/europe-west8/subnetworks/net-0 + secondary_ip_range_names: [] + source_ip_ranges_to_nat: + - ALL_IP_RANGES + - name: projects/project-id/regions/europe-west8/subnetworks/net-1 + secondary_ip_range_names: [] + source_ip_ranges_to_nat: + - PRIMARY_IP_RANGE + - name: projects/project-id/regions/europe-west8/subnetworks/net-2 + secondary_ip_range_names: + - pods + source_ip_ranges_to_nat: + - LIST_OF_SECONDARY_IP_RANGES + - PRIMARY_IP_RANGE + tcp_established_idle_timeout_sec: 1200 + tcp_time_wait_timeout_sec: 120 + tcp_transitory_idle_timeout_sec: 30 + timeouts: null + udp_idle_timeout_sec: 30 + +counts: + google_compute_router: 1 + google_compute_router_nat: 1 + modules: 1 + resources: 2