From c1248d328a59b901d33bb7f15b0bee4947d550fc Mon Sep 17 00:00:00 2001 From: Eric Zhao Date: Mon, 12 Jan 2026 19:34:18 +1000 Subject: [PATCH] Allow any VPC for (secure) network_tags (#3634) * feat: allow all for VPC networks * feat: add examples * feat: add header * feat: module test * fix: update network testing data to pass validation --------- Co-authored-by: Julio Castillo --- modules/organization/README.md | 8 +- modules/organization/tags.tf | 4 +- modules/organization/variables-tags.tf | 9 ++- modules/project/README.md | 42 ++++++++++- modules/project/tags.tf | 4 +- modules/project/variables-tags.tf | 9 ++- tests/modules/organization/tags.tfvars | 2 +- tests/modules/organization/tags.yaml | 2 +- .../organization/tags_force_context.tfvars | 2 +- .../modules/organization/tags_skip_iam.tfvars | 2 +- tests/modules/organization/tags_skip_iam.yaml | 2 +- .../project/examples/tags-network-all.yaml | 74 +++++++++++++++++++ 12 files changed, 143 insertions(+), 17 deletions(-) create mode 100644 tests/modules/project/examples/tags-network-all.yaml diff --git a/modules/organization/README.md b/modules/organization/README.md index d1f18c1ea..10f150175 100644 --- a/modules/organization/README.md +++ b/modules/organization/README.md @@ -824,14 +824,14 @@ module "org" { | [logging_exclusions](variables-logging.tf#L28) | Logging exclusions for this organization in the form {NAME -> FILTER}. | map(string) | | {} | | [logging_settings](variables-logging.tf#L35) | Default settings for logging resources. | object({…}) | | null | | [logging_sinks](variables-logging.tf#L45) | Logging sinks to create for the organization. | map(object({…})) | | {} | -| [network_tags](variables-tags.tf#L17) | Network tags by key name. If `id` is provided, key creation is skipped. The `iam` attribute behaves like the similarly named one at module level. | map(object({…})) | | {} | +| [network_tags](variables-tags.tf#L17) | Network tags by key name. If `id` is provided, key creation is skipped. The `iam` attribute behaves like the similarly named one at module level. | map(object({…})) | | {} | | [org_policies](variables.tf#L85) | Organization policies applied to this organization keyed by policy name. | map(object({…})) | | {} | | [org_policy_custom_constraints](variables.tf#L113) | Organization policy custom constraints keyed by constraint name. | map(object({…})) | | {} | | [pam_entitlements](variables-pam.tf#L17) | Privileged Access Manager entitlements for this resource, keyed by entitlement ID. | map(object({…})) | | {} | | [scc_sha_custom_modules](variables-scc.tf#L17) | SCC custom modules keyed by module name. | map(object({…})) | | {} | -| [tag_bindings](variables-tags.tf#L82) | Tag bindings for this organization, in key => tag value id format. | map(string) | | {} | -| [tags](variables-tags.tf#L89) | Tags by key name. If `id` is provided, key or value creation is skipped. The `iam` attribute behaves like the similarly named one at module level. | map(object({…})) | | {} | -| [tags_config](variables-tags.tf#L154) | Fine-grained control on tag resource and IAM creation. | object({…}) | | {} | +| [tag_bindings](variables-tags.tf#L89) | Tag bindings for this organization, in key => tag value id format. | map(string) | | {} | +| [tags](variables-tags.tf#L96) | Tags by key name. If `id` is provided, key or value creation is skipped. The `iam` attribute behaves like the similarly named one at module level. | map(object({…})) | | {} | +| [tags_config](variables-tags.tf#L161) | Fine-grained control on tag resource and IAM creation. | object({…}) | | {} | | [workforce_identity_config](variables-identity-providers.tf#L17) | Workforce Identity Federation pool and providers. | object({…}) | | null | ## Outputs diff --git a/modules/organization/tags.tf b/modules/organization/tags.tf index 5a5f36371..e7a33f69f 100644 --- a/modules/organization/tags.tf +++ b/modules/organization/tags.tf @@ -182,7 +182,9 @@ resource "google_tags_tag_key" "default" { lookup(each.value, "network", null) == null ? null : "GCE_FIREWALL" ) purpose_data = ( - lookup(each.value, "network", null) == null ? null : { network = each.value.network } + lookup(each.value, "network", null) == null ? null : ( + each.value.network == "ALL" ? { organization = "auto" } : { network = each.value.network } + ) ) short_name = each.key description = each.value.description diff --git a/modules/organization/variables-tags.tf b/modules/organization/variables-tags.tf index 91cf57ac6..dae95563d 100644 --- a/modules/organization/variables-tags.tf +++ b/modules/organization/variables-tags.tf @@ -19,7 +19,7 @@ variable "network_tags" { type = map(object({ description = optional(string, "Managed by the Terraform organization module.") id = optional(string) - network = string # project_id/vpc_name + network = string # project_id/vpc_name or "ALL" to toggle GCE_FIREWALL purpose iam = optional(map(list(string)), {}) iam_bindings = optional(map(object({ members = list(string) @@ -77,6 +77,13 @@ variable "network_tags" { ) error_message = "Use an empty map instead of null as value." } + validation { + condition = alltrue([ + for k, v in var.network_tags : + v.network == "ALL" || can(regex("^[a-z][a-z0-9-]{4,28}[a-z0-9]/[a-z](?:[-a-z0-9]*[a-z0-9])?$", v.network)) + ]) + error_message = "The network attribute must be 'ALL' or a valid VPC network URI (project_id/vpc_name)." + } } variable "tag_bindings" { diff --git a/modules/project/README.md b/modules/project/README.md index 7404bbed7..af104f8b8 100644 --- a/modules/project/README.md +++ b/modules/project/README.md @@ -1107,6 +1107,40 @@ module "project" { # tftest modules=1 resources=8 inventory=tags-network.yaml ``` +If you want to create a Tag Key with `GCE_FIREWALL` purpose that is valid for the whole organization (allowing the binding on any network within it), use `"ALL"` as the network value: + +```hcl +module "project" { + source = "./fabric/modules/project" + billing_account = var.billing_account_id + name = "project" + prefix = var.prefix + parent = var.folder_id + services = [ + "compute.googleapis.com" + ] + network_tags = { + net-environment = { + description = "This is a network tag." + network = "ALL" + iam = { + "roles/resourcemanager.tagAdmin" = ["group:${var.group_email}"] + } + values = { + dev = {} + prod = { + description = "Environment: production." + iam = { + "roles/resourcemanager.tagUser" = ["group:${var.group_email}"] + } + } + } + } + } +} +# tftest modules=1 resources=8 inventory=tags-network-all.yaml +``` + ### Tags Factory Tags can also be specified via a factory in a similar way to organization policies and policy constraints. Each file is mapped to tag key, where @@ -2133,7 +2167,7 @@ module "project" { | [logging_metrics](variables-observability.tf#L145) | Log-based metrics. | map(object({…})) | | {} | | [logging_sinks](variables-observability.tf#L185) | Logging sinks to create for this project. | map(object({…})) | | {} | | [metric_scopes](variables-observability.tf#L216) | List of projects that will act as metric scopes for this project. | list(string) | | [] | -| [network_tags](variables-tags.tf#L17) | Network tags by key name. If `id` is provided, key creation is skipped. The `iam` attribute behaves like the similarly named one at module level. | map(object({…})) | | {} | +| [network_tags](variables-tags.tf#L17) | Network tags by key name. If `id` is provided, key creation is skipped. The `iam` attribute behaves like the similarly named one at module level. | map(object({…})) | | {} | | [notification_channels](variables-observability.tf#L223) | Monitoring notification channels. | map(object({…})) | | {} | | [org_policies](variables.tf#L205) | Organization policies applied to this project keyed by policy name. | map(object({…})) | | {} | | [pam_entitlements](variables-pam.tf#L17) | Privileged Access Manager entitlements for this resource, keyed by entitlement ID. | map(object({…})) | | {} | @@ -2149,9 +2183,9 @@ module "project" { | [shared_vpc_host_config](variables.tf#L313) | Configures this project as a Shared VPC host project (mutually exclusive with shared_vpc_service_project). | object({…}) | | null | | [shared_vpc_service_config](variables.tf#L323) | Configures this project as a Shared VPC service project (mutually exclusive with shared_vpc_host_config). | object({…}) | | {…} | | [skip_delete](variables.tf#L360) | Deprecated. Use deletion_policy. | bool | | null | -| [tag_bindings](variables-tags.tf#L82) | Tag bindings for this project, in key => tag value id format. | map(string) | | null | -| [tags](variables-tags.tf#L89) | Tags by key name. If `id` is provided, key or value creation is skipped. The `iam` attribute behaves like the similarly named one at module level. | map(object({…})) | | {} | -| [tags_config](variables-tags.tf#L154) | Fine-grained control on tag resource and IAM creation. | object({…}) | | {} | +| [tag_bindings](variables-tags.tf#L89) | Tag bindings for this project, in key => tag value id format. | map(string) | | null | +| [tags](variables-tags.tf#L96) | Tags by key name. If `id` is provided, key or value creation is skipped. The `iam` attribute behaves like the similarly named one at module level. | map(object({…})) | | {} | +| [tags_config](variables-tags.tf#L161) | Fine-grained control on tag resource and IAM creation. | object({…}) | | {} | | [universe](variables.tf#L372) | GCP universe where to deploy the project. The prefix will be prepended to the project id. | object({…}) | | null | | [vpc_sc](variables.tf#L383) | VPC-SC configuration for the project, use when `ignore_changes` for resources is set in the VPC-SC module. | object({…}) | | null | | [workload_identity_pools](variables-identity-providers.tf#L17) | Workload Identity Federation pools and providers. | map(object({…})) | | {} | diff --git a/modules/project/tags.tf b/modules/project/tags.tf index 62a437d44..32910a81e 100644 --- a/modules/project/tags.tf +++ b/modules/project/tags.tf @@ -182,7 +182,9 @@ resource "google_tags_tag_key" "default" { lookup(each.value, "network", null) == null ? null : "GCE_FIREWALL" ) purpose_data = ( - lookup(each.value, "network", null) == null ? null : { network = each.value.network } + lookup(each.value, "network", null) == null ? null : ( + each.value.network == "ALL" ? { organization = "auto" } : { network = each.value.network } + ) ) short_name = each.key description = each.value.description diff --git a/modules/project/variables-tags.tf b/modules/project/variables-tags.tf index 581f8e03a..080a7a66f 100644 --- a/modules/project/variables-tags.tf +++ b/modules/project/variables-tags.tf @@ -19,7 +19,7 @@ variable "network_tags" { type = map(object({ id = optional(string) description = optional(string, "Managed by the Terraform project module.") - network = string # project_id/vpc_name + network = string # project_id/vpc_name or "ALL" to toggle GCE_FIREWALL purpose iam = optional(map(list(string)), {}) iam_bindings = optional(map(object({ members = list(string) @@ -77,6 +77,13 @@ variable "network_tags" { ) error_message = "Use an empty map instead of null as value." } + validation { + condition = alltrue([ + for k, v in var.network_tags : + v.network == "ALL" || can(regex("^[a-z][a-z0-9-]{4,28}[a-z0-9]/[a-z](?:[-a-z0-9]*[a-z0-9])?$", v.network)) + ]) + error_message = "The network attribute must be 'ALL' or a valid VPC network name (project_id/vpc_name)." + } } variable "tag_bindings" { diff --git a/tests/modules/organization/tags.tfvars b/tests/modules/organization/tags.tfvars index 5d9426741..2aba64393 100644 --- a/tests/modules/organization/tags.tfvars +++ b/tests/modules/organization/tags.tfvars @@ -1,6 +1,6 @@ network_tags = { net_environment = { - network = "foobar" + network = "test-project/test-vpc" } } tags = { diff --git a/tests/modules/organization/tags.yaml b/tests/modules/organization/tags.yaml index af2eafb51..7343d1f53 100644 --- a/tests/modules/organization/tags.yaml +++ b/tests/modules/organization/tags.yaml @@ -36,7 +36,7 @@ values: parent: organizations/1234567890 purpose: GCE_FIREWALL purpose_data: - network: foobar + network: test-project/test-vpc short_name: net_environment ? google_tags_tag_key_iam_binding.default["foobar:roles/resourcemanager.tagAdmin"] : condition: [] diff --git a/tests/modules/organization/tags_force_context.tfvars b/tests/modules/organization/tags_force_context.tfvars index 1fb914387..4093e7063 100644 --- a/tests/modules/organization/tags_force_context.tfvars +++ b/tests/modules/organization/tags_force_context.tfvars @@ -15,7 +15,7 @@ context = { } network_tags = { net_environment = { - network = "foobar" + network = "test-project/test-vpc" } } tags_config = { diff --git a/tests/modules/organization/tags_skip_iam.tfvars b/tests/modules/organization/tags_skip_iam.tfvars index fa66bf2b0..4edc0d43b 100644 --- a/tests/modules/organization/tags_skip_iam.tfvars +++ b/tests/modules/organization/tags_skip_iam.tfvars @@ -1,6 +1,6 @@ network_tags = { net_environment = { - network = "foobar" + network = "test-project/test-vpc" } } tags_config = { diff --git a/tests/modules/organization/tags_skip_iam.yaml b/tests/modules/organization/tags_skip_iam.yaml index 42b64fb8e..4133f676c 100644 --- a/tests/modules/organization/tags_skip_iam.yaml +++ b/tests/modules/organization/tags_skip_iam.yaml @@ -36,7 +36,7 @@ values: parent: organizations/1234567890 purpose: GCE_FIREWALL purpose_data: - network: foobar + network: test-project/test-vpc short_name: net_environment google_tags_tag_value.default["foobar/one"]: description: Managed by the Terraform organization module. diff --git a/tests/modules/project/examples/tags-network-all.yaml b/tests/modules/project/examples/tags-network-all.yaml new file mode 100644 index 000000000..08784da4e --- /dev/null +++ b/tests/modules/project/examples/tags-network-all.yaml @@ -0,0 +1,74 @@ +# Copyright 2025 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.project.google_project.project[0]: + auto_create_network: false + billing_account: 123456-123456-123456 + deletion_policy: 'DELETE' + folder_id: '1122334455' + labels: null + name: test-project + org_id: null + project_id: test-project + timeouts: null + module.project.google_project_iam_member.service_agents["compute-system"]: + condition: [] + project: test-project + role: roles/compute.serviceAgent + module.project.google_project_service.project_services["compute.googleapis.com"]: + disable_dependent_services: false + disable_on_destroy: false + project: test-project + service: compute.googleapis.com + timeouts: null + module.project.google_tags_tag_key.default["net-environment"]: + description: This is a network tag. + parent: projects/test-project + purpose: GCE_FIREWALL + purpose_data: + organization: auto + short_name: net-environment + timeouts: null + module.project.google_tags_tag_key_iam_binding.default["net-environment:roles/resourcemanager.tagAdmin"]: + condition: [] + members: + - group:organization-admins@example.org + role: roles/resourcemanager.tagAdmin + module.project.google_tags_tag_value.default["net-environment/dev"]: + description: Managed by the Terraform project module. + short_name: dev + timeouts: null + module.project.google_tags_tag_value.default["net-environment/prod"]: + description: 'Environment: production.' + short_name: prod + timeouts: null + module.project.google_tags_tag_value_iam_binding.default["net-environment/prod:roles/resourcemanager.tagUser"]: + condition: [] + members: + - group:organization-admins@example.org + role: roles/resourcemanager.tagUser + +counts: + google_project: 1 + google_project_iam_member: 1 + google_project_service: 1 + google_tags_tag_key: 1 + google_tags_tag_key_iam_binding: 1 + google_tags_tag_value: 2 + google_tags_tag_value_iam_binding: 1 + modules: 1 + resources: 8 + +outputs: {}