From 616c6fbc4875b410c1030bd2188e34e306d98658 Mon Sep 17 00:00:00 2001 From: Julio Castillo Date: Fri, 15 Sep 2023 14:19:33 +0200 Subject: [PATCH 1/2] Fix subnet iam_bindings to use arbitrary keys --- modules/net-vpc/README.md | 13 ++++--- modules/net-vpc/subnets.tf | 39 +++++++++---------- modules/net-vpc/variables.tf | 3 ++ .../modules/net_vpc/examples/subnet-iam.yaml | 2 +- 4 files changed, 30 insertions(+), 27 deletions(-) diff --git a/modules/net-vpc/README.md b/modules/net-vpc/README.md index 976dd3fd23..ea86930ead 100644 --- a/modules/net-vpc/README.md +++ b/modules/net-vpc/README.md @@ -118,8 +118,9 @@ module "vpc" { ] } iam_bindings = { - "roles/compute.networkUser" = { + subnet-1-iam = { members = ["group:group2@example.com"] + role = "roles/compute.networkUser" condition = { expression = "resource.matchTag('123456789012/env', 'prod')" title = "test_condition" @@ -132,7 +133,7 @@ module "vpc" { region = "europe-west1" ip_cidr_range = "10.0.1.0/24" iam_bindings_additive = { - subnet-2-am1 = { + subnet-2-iam = { member = "user:am1@example.com" role = "roles/compute.networkUser" subnet = "europe-west1/subnet-2" @@ -553,10 +554,10 @@ module "vpc" { | [routing_mode](variables.tf#L147) | The network routing mode (default 'GLOBAL'). | string | | "GLOBAL" | | [shared_vpc_host](variables.tf#L157) | Enable shared VPC for this project. | bool | | false | | [shared_vpc_service_projects](variables.tf#L163) | Shared VPC service projects to register with this host. | list(string) | | [] | -| [subnets](variables.tf#L169) | Subnet configuration. | list(object({…})) | | [] | -| [subnets_proxy_only](variables.tf#L215) | List of proxy-only subnets for Regional HTTPS or Internal HTTPS load balancers. Note: Only one proxy-only subnet for each VPC network in each region can be active. | list(object({…})) | | [] | -| [subnets_psc](variables.tf#L248) | List of subnets for Private Service Connect service producers. | list(object({…})) | | [] | -| [vpc_create](variables.tf#L279) | Create VPC. When set to false, uses a data source to reference existing VPC. | bool | | true | +| [subnets](variables.tf#L169) | Subnet configuration. | list(object({…})) | | [] | +| [subnets_proxy_only](variables.tf#L216) | List of proxy-only subnets for Regional HTTPS or Internal HTTPS load balancers. Note: Only one proxy-only subnet for each VPC network in each region can be active. | list(object({…})) | | [] | +| [subnets_psc](variables.tf#L250) | List of subnets for Private Service Connect service producers. | list(object({…})) | | [] | +| [vpc_create](variables.tf#L282) | Create VPC. When set to false, uses a data source to reference existing VPC. | bool | | true | ## Outputs diff --git a/modules/net-vpc/subnets.tf b/modules/net-vpc/subnets.tf index f006503945..fe5abea929 100644 --- a/modules/net-vpc/subnets.tf +++ b/modules/net-vpc/subnets.tf @@ -36,36 +36,37 @@ locals { } : null global = try(v.global, false) ip_cidr_range = v.ip_cidr_range - ipv6 = can(v.ipv6) ? { + ipv6 = !can(v.ipv6) ? null : { access_type = try(v.ipv6.access_type, "INTERNAL") - } : null + } name = try(v.name, k) region = v.region secondary_ip_ranges = try(v.secondary_ip_ranges, null) iam = try(v.iam, {}) - iam_bindings = can(v.iam_bindings) ? { + iam_bindings = !can(v.iam_bindings) ? {} : { for k2, v2 in v.iam_bindings : k2 => { + role = v2.role members = v2.members - condition = can(v2.condition) ? { + condition = !can(v2.condition) ? null : { expression = v2.condition.expression title = v2.condition.title description = try(v2.condition.description, null) - } : null + } } - } : {} - iam_bindings_additive = can(v.iam_bindings_additive) ? { + } + iam_bindings_additive = !can(v.iam_bindings_additive) ? {} : { for k2, v2 in v.iam_bindings_additive : k2 => { member = v2.member role = v2.role - condition = can(v2.condition) ? { + condition = !can(v2.condition) ? null : { expression = v2.condition.expression title = v2.condition.title description = try(v2.condition.description, null) - } : null + } } - } : {} + } _is_regular = !try(v.psc == true, false) && !try(v.proxy_only == true, false) _is_psc = try(v.psc == true, false) _is_proxy_only = try(v.proxy_only == true, false) @@ -89,16 +90,17 @@ locals { ] ], )) - subnet_iam_bindings = flatten([ - for s in concat(var.subnets, var.subnets_psc, var.subnets_proxy_only, values(local._factory_subnets)) : [ - for role, data in s.iam_bindings : { - role = role + subnet_iam_bindings = merge([ + for s in concat(var.subnets, var.subnets_psc, var.subnets_proxy_only, values(local._factory_subnets)) : { + for key, data in s.iam_bindings : + key => { + role = data.role subnet = "${s.region}/${s.name}" members = data.members condition = data.condition } - ] - ]) + } + ]...) # note: all additive bindings share a single namespace for the key. # In other words, if you have multiple additive bindings with the # same name, only one will be used @@ -210,10 +212,7 @@ resource "google_compute_subnetwork_iam_binding" "authoritative" { } resource "google_compute_subnetwork_iam_binding" "bindings" { - for_each = { - for binding in local.subnet_iam_bindings : - "${binding.subnet}.${binding.role}.${try(binding.condition.title, "")}" => binding - } + for_each = local.subnet_iam_bindings project = var.project_id subnetwork = local.all_subnets[each.value.subnet].name region = local.all_subnets[each.value.subnet].region diff --git a/modules/net-vpc/variables.tf b/modules/net-vpc/variables.tf index 439b5a56f1..5c4cc692d3 100644 --- a/modules/net-vpc/variables.tf +++ b/modules/net-vpc/variables.tf @@ -191,6 +191,7 @@ variable "subnets" { iam = optional(map(list(string)), {}) iam_bindings = optional(map(object({ + role = string members = list(string) condition = optional(object({ expression = string @@ -224,6 +225,7 @@ variable "subnets_proxy_only" { iam = optional(map(list(string)), {}) iam_bindings = optional(map(object({ + role = string members = list(string) condition = optional(object({ expression = string @@ -255,6 +257,7 @@ variable "subnets_psc" { iam = optional(map(list(string)), {}) iam_bindings = optional(map(object({ + role = string members = list(string) condition = optional(object({ expression = string diff --git a/tests/modules/net_vpc/examples/subnet-iam.yaml b/tests/modules/net_vpc/examples/subnet-iam.yaml index 68b0341867..9fcf160a0c 100644 --- a/tests/modules/net_vpc/examples/subnet-iam.yaml +++ b/tests/modules/net_vpc/examples/subnet-iam.yaml @@ -80,7 +80,7 @@ values: region: europe-west1 role: roles/compute.networkUser subnetwork: subnet-1 - module.vpc.google_compute_subnetwork_iam_binding.bindings["europe-west1/subnet-1.roles/compute.networkUser.test_condition"]: + module.vpc.google_compute_subnetwork_iam_binding.bindings["subnet-1-iam"]: condition: - description: null expression: resource.matchTag('123456789012/env', 'prod') From 4d5df5aeb249b53ae0b960aeb0f163a6f47cede1 Mon Sep 17 00:00:00 2001 From: Julio Castillo Date: Fri, 15 Sep 2023 14:57:32 +0200 Subject: [PATCH 2/2] Fix tests --- tests/modules/net_vpc/examples/subnet-iam.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/modules/net_vpc/examples/subnet-iam.yaml b/tests/modules/net_vpc/examples/subnet-iam.yaml index 9fcf160a0c..1b925f48e2 100644 --- a/tests/modules/net_vpc/examples/subnet-iam.yaml +++ b/tests/modules/net_vpc/examples/subnet-iam.yaml @@ -91,7 +91,7 @@ values: region: europe-west1 role: roles/compute.networkUser subnetwork: subnet-1 - module.vpc.google_compute_subnetwork_iam_member.bindings["subnet-2-am1"]: + module.vpc.google_compute_subnetwork_iam_member.bindings["subnet-2-iam"]: condition: [] member: user:am1@example.com project: my-project