From c6cdc9c98ccb2871c4fde3abcc467406ff0d8d26 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Tue, 16 Sep 2025 23:21:18 +0200 Subject: [PATCH] Incremental improvements to project factory and underlying modules (#3325) * incremental improvements to project factory and underlying modules * fix org policies diff test --- fast/stages/2-project-factory/README.md | 12 +- fast/stages/2-project-factory/main.tf | 41 ++++-- .../2-project-factory/variables-fast.tf | 8 + fast/stages/2-project-factory/variables.tf | 138 ++++++++++++++++++ modules/folder/organization-policies.tf | 16 +- modules/organization/organization-policies.tf | 16 +- modules/project/organization-policies.tf | 16 +- .../test_plan_org_policies_modules.py | 2 +- 8 files changed, 221 insertions(+), 28 deletions(-) diff --git a/fast/stages/2-project-factory/README.md b/fast/stages/2-project-factory/README.md index ea30ea5ad..cc72ad40a 100644 --- a/fast/stages/2-project-factory/README.md +++ b/fast/stages/2-project-factory/README.md @@ -362,9 +362,12 @@ automation: | [automation](variables-fast.tf#L17) | Automation resources created by the bootstrap stage. | object({…}) | ✓ | | 0-org-setup | | [billing_account](variables-fast.tf#L26) | Billing account id. | object({…}) | ✓ | | 0-org-setup | | [prefix](variables-fast.tf#L92) | Prefix used for resources that need unique names. Use a maximum of 9 chars for organizations, and 11 chars for tenants. | string | ✓ | | 0-org-setup | -| [context](variables.tf#L17) | Context-specific interpolations. | object({…}) | | {} | | +| [context](variables.tf#L17) | Context-specific interpolations. | object({…}) | | {} | | | [custom_roles](variables-fast.tf#L34) | Custom roles defined at the org level, in key => id format. | map(string) | | {} | 0-org-setup | -| [factories_config](variables.tf#L35) | Path to folder with YAML resource description data files. | object({…}) | | {} | | +| [data_defaults](variables.tf#L36) | Optional default values used when corresponding project or folder data from files are missing. | object({…}) | | {} | | +| [data_merges](variables.tf#L108) | Optional values that will be merged with corresponding data from files. Combines with `data_defaults`, file data, and `data_overrides`. | object({…}) | | {} | | +| [data_overrides](variables.tf#L127) | Optional values that override corresponding data from files. Takes precedence over file data and `data_defaults`. | object({…}) | | {} | | +| [factories_config](variables.tf#L173) | Path to folder with YAML resource description data files. | object({…}) | | {} | | | [folder_ids](variables-fast.tf#L42) | Folders created in the bootstrap stage. | map(string) | | {} | 0-org-setup | | [host_project_ids](variables-fast.tf#L58) | Host project for the shared VPC. | map(string) | | {} | 2-networking | | [iam_principals](variables-fast.tf#L50) | IAM-format principals. | map(string) | | {} | 0-org-setup | @@ -373,8 +376,9 @@ automation: | [perimeters](variables-fast.tf#L84) | Optional VPC-SC perimeter ids. | map(string) | | {} | 1-vpcsc | | [project_ids](variables-fast.tf#L102) | Projects created in the bootstrap stage. | map(string) | | {} | 0-org-setup | | [service_accounts](variables-fast.tf#L110) | Service accounts created in the bootstrap stage. | map(string) | | {} | 0-org-setup | -| [stage_name](variables.tf#L55) | FAST stage name. Used to separate output files across different factories. | string | | "2-project-factory" | | -| [tag_values](variables-fast.tf#L118) | FAST-managed resource manager tag values. | map(string) | | {} | 0-org-setup | +| [stage_name](variables.tf#L193) | FAST stage name. Used to separate output files across different factories. | string | | "2-project-factory" | | +| [subnet_self_links](variables-fast.tf#L118) | Shared VPC subnet IDs. | map(map(string)) | | {} | 2-networking | +| [tag_values](variables-fast.tf#L126) | FAST-managed resource manager tag values. | map(string) | | {} | 0-org-setup | ## Outputs diff --git a/fast/stages/2-project-factory/main.tf b/fast/stages/2-project-factory/main.tf index 5de1109bc..941103f56 100644 --- a/fast/stages/2-project-factory/main.tf +++ b/fast/stages/2-project-factory/main.tf @@ -16,9 +16,25 @@ # tfdoc:file:description Project factory. +locals { + subnet_self_links = flatten([ + for net, subnets in var.subnet_self_links : [ + for subnet_name, subnet_link in subnets : { + key = "${net}/${subnet_name}" + link = subnet_link + } + ] + ]) +} + module "factory" { source = "../../../modules/project-factory" context = { + condition_vars = merge({ + subnet_self_links = { + for v in local.subnet_self_links : v.key => v.link + } + }, var.context.condition_vars) custom_roles = merge( var.custom_roles, var.context.custom_roles ) @@ -50,20 +66,23 @@ module "factory" { var.perimeters, var.context.vpc_sc_perimeters ) } - data_defaults = { - # more defaults are available, check the project factory variables - billing_account = var.billing_account.id - storage_location = var.locations.storage - } - data_merges = { - services = [ + data_defaults = merge(var.data_defaults, { + billing_account = coalesce( + var.data_defaults.billing_account, var.billing_account.id + ) + prefix = coalesce(var.data_defaults.prefix, var.prefix) + storage_location = coalesce( + var.data_defaults.storage_location, var.locations.storage + ) + }) + data_merges = merge(var.data_merges, { + services = length(var.data_merges.services) > 0 ? var.data_merges.services : [ "logging.googleapis.com", "monitoring.googleapis.com" ] - } - data_overrides = { - prefix = var.prefix - } + } + ) + data_overrides = var.data_overrides factories_config = merge(var.factories_config, { budgets = { billing_account_id = try( diff --git a/fast/stages/2-project-factory/variables-fast.tf b/fast/stages/2-project-factory/variables-fast.tf index c1b3aaa92..b39b799f4 100644 --- a/fast/stages/2-project-factory/variables-fast.tf +++ b/fast/stages/2-project-factory/variables-fast.tf @@ -115,6 +115,14 @@ variable "service_accounts" { default = {} } +variable "subnet_self_links" { + # tfdoc:variable:source 2-networking + description = "Shared VPC subnet IDs." + type = map(map(string)) + nullable = false + default = {} +} + variable "tag_values" { # tfdoc:variable:source 0-org-setup description = "FAST-managed resource manager tag values." diff --git a/fast/stages/2-project-factory/variables.tf b/fast/stages/2-project-factory/variables.tf index ec57ebdfe..2c86fed85 100644 --- a/fast/stages/2-project-factory/variables.tf +++ b/fast/stages/2-project-factory/variables.tf @@ -17,6 +17,7 @@ variable "context" { description = "Context-specific interpolations." type = object({ + condition_vars = optional(map(map(string)), {}) custom_roles = optional(map(string), {}) folder_ids = optional(map(string), {}) iam_principals = optional(map(string), {}) @@ -32,6 +33,143 @@ variable "context" { nullable = false } +variable "data_defaults" { + description = "Optional default values used when corresponding project or folder data from files are missing." + type = object({ + billing_account = optional(string) + bucket = optional(object({ + force_destroy = optional(bool) + }), {}) + contacts = optional(map(list(string)), {}) + deletion_policy = optional(string) + factories_config = optional(object({ + custom_roles = optional(string) + observability = optional(string) + org_policies = optional(string) + quotas = optional(string) + }), {}) + labels = optional(map(string), {}) + metric_scopes = optional(list(string), []) + parent = optional(string) + prefix = optional(string) + project_reuse = optional(object({ + use_data_source = optional(bool, true) + attributes = optional(object({ + name = string + number = number + services_enabled = optional(list(string), []) + })) + })) + service_encryption_key_ids = optional(map(list(string)), {}) + services = optional(list(string), []) + shared_vpc_service_config = optional(object({ + host_project = string + iam_bindings_additive = optional(map(object({ + member = string + role = string + condition = optional(object({ + expression = string + title = string + description = optional(string) + })) + })), {}) + network_users = optional(list(string), []) + service_agent_iam = optional(map(list(string)), {}) + service_agent_subnet_iam = optional(map(list(string)), {}) + service_iam_grants = optional(list(string), []) + network_subnet_users = optional(map(list(string)), {}) + })) + storage_location = optional(string) + tag_bindings = optional(map(string), {}) + # non-project resources + service_accounts = optional(map(object({ + display_name = optional(string, "Terraform-managed.") + iam_self_roles = optional(list(string)) + })), {}) + universe = optional(object({ + prefix = string + unavailable_service_identities = optional(list(string), []) + unavailable_services = optional(list(string), []) + })) + vpc_sc = optional(object({ + perimeter_name = string + is_dry_run = optional(bool, false) + })) + logging_data_access = optional(map(object({ + ADMIN_READ = optional(object({ exempted_members = optional(list(string)) })), + DATA_READ = optional(object({ exempted_members = optional(list(string)) })), + DATA_WRITE = optional(object({ exempted_members = optional(list(string)) })) + })), {}) + }) + nullable = false + default = {} +} + +variable "data_merges" { + description = "Optional values that will be merged with corresponding data from files. Combines with `data_defaults`, file data, and `data_overrides`." + type = object({ + contacts = optional(map(list(string)), {}) + labels = optional(map(string), {}) + metric_scopes = optional(list(string), []) + service_encryption_key_ids = optional(map(list(string)), {}) + services = optional(list(string), []) + tag_bindings = optional(map(string), {}) + # non-project resources + service_accounts = optional(map(object({ + display_name = optional(string, "Terraform-managed.") + iam_self_roles = optional(list(string)) + })), {}) + }) + nullable = false + default = {} +} + +variable "data_overrides" { + description = "Optional values that override corresponding data from files. Takes precedence over file data and `data_defaults`." + type = object({ + # data overrides default to null to mark that they should not override + billing_account = optional(string) + bucket = optional(object({ + force_destroy = optional(bool) + }), {}) + contacts = optional(map(list(string))) + deletion_policy = optional(string) + factories_config = optional(object({ + custom_roles = optional(string) + observability = optional(string) + org_policies = optional(string) + quotas = optional(string) + }), {}) + parent = optional(string) + prefix = optional(string) + service_encryption_key_ids = optional(map(list(string))) + storage_location = optional(string) + tag_bindings = optional(map(string)) + services = optional(list(string)) + # non-project resources + service_accounts = optional(map(object({ + display_name = optional(string, "Terraform-managed.") + iam_self_roles = optional(list(string)) + }))) + universe = optional(object({ + prefix = string + unavailable_service_identities = optional(list(string), []) + unavailable_services = optional(list(string), []) + })) + vpc_sc = optional(object({ + perimeter_name = string + is_dry_run = optional(bool, false) + })) + logging_data_access = optional(map(object({ + ADMIN_READ = optional(object({ exempted_members = optional(list(string)) })), + DATA_READ = optional(object({ exempted_members = optional(list(string)) })), + DATA_WRITE = optional(object({ exempted_members = optional(list(string)) })) + }))) + }) + nullable = false + default = {} +} + variable "factories_config" { description = "Path to folder with YAML resource description data files." type = object({ diff --git a/modules/folder/organization-policies.tf b/modules/folder/organization-policies.tf index 0bfa8074f..40cc11e4c 100644 --- a/modules/folder/organization-policies.tf +++ b/modules/folder/organization-policies.tf @@ -138,8 +138,12 @@ resource "google_org_policy_policy" "default" { dynamic "values" { for_each = rule.value.has_values ? [1] : [] content { - allowed_values = try(rule.value.allow.values, null) - denied_values = try(rule.value.deny.values, null) + allowed_values = try(rule.value.allow.values, null) == null ? null : [ + for v in rule.value.allow.values : templatestring(v, var.context.condition_vars) + ] + denied_values = try(rule.value.deny.values, null) == null ? null : [ + for v in rule.value.deny.values : templatestring(v, var.context.condition_vars) + ] } } } @@ -177,8 +181,12 @@ resource "google_org_policy_policy" "default" { dynamic "values" { for_each = rule.value.has_values ? [1] : [] content { - allowed_values = try(rule.value.allow.values, null) - denied_values = try(rule.value.deny.values, null) + allowed_values = try(rule.value.allow.values, null) == null ? null : [ + for v in rule.value.allow.values : templatestring(v, var.context.condition_vars) + ] + denied_values = try(rule.value.deny.values, null) == null ? null : [ + for v in rule.value.deny.values : templatestring(v, var.context.condition_vars) + ] } } } diff --git a/modules/organization/organization-policies.tf b/modules/organization/organization-policies.tf index 5adcf9403..fa0efdcda 100644 --- a/modules/organization/organization-policies.tf +++ b/modules/organization/organization-policies.tf @@ -138,8 +138,12 @@ resource "google_org_policy_policy" "default" { dynamic "values" { for_each = rule.value.has_values ? [1] : [] content { - allowed_values = try(rule.value.allow.values, null) - denied_values = try(rule.value.deny.values, null) + allowed_values = try(rule.value.allow.values, null) == null ? null : [ + for v in rule.value.allow.values : templatestring(v, var.context.condition_vars) + ] + denied_values = try(rule.value.deny.values, null) == null ? null : [ + for v in rule.value.deny.values : templatestring(v, var.context.condition_vars) + ] } } } @@ -177,8 +181,12 @@ resource "google_org_policy_policy" "default" { dynamic "values" { for_each = rule.value.has_values ? [1] : [] content { - allowed_values = try(rule.value.allow.values, null) - denied_values = try(rule.value.deny.values, null) + allowed_values = try(rule.value.allow.values, null) == null ? null : [ + for v in rule.value.allow.values : templatestring(v, var.context.condition_vars) + ] + denied_values = try(rule.value.deny.values, null) == null ? null : [ + for v in rule.value.deny.values : templatestring(v, var.context.condition_vars) + ] } } } diff --git a/modules/project/organization-policies.tf b/modules/project/organization-policies.tf index ac688bed3..358e54f8c 100644 --- a/modules/project/organization-policies.tf +++ b/modules/project/organization-policies.tf @@ -138,8 +138,12 @@ resource "google_org_policy_policy" "default" { dynamic "values" { for_each = rule.value.has_values ? [1] : [] content { - allowed_values = try(rule.value.allow.values, null) - denied_values = try(rule.value.deny.values, null) + allowed_values = try(rule.value.allow.values, null) == null ? null : [ + for v in rule.value.allow.values : templatestring(v, var.context.condition_vars) + ] + denied_values = try(rule.value.deny.values, null) == null ? null : [ + for v in rule.value.deny.values : templatestring(v, var.context.condition_vars) + ] } } } @@ -177,8 +181,12 @@ resource "google_org_policy_policy" "default" { dynamic "values" { for_each = rule.value.has_values ? [1] : [] content { - allowed_values = try(rule.value.allow.values, null) - denied_values = try(rule.value.deny.values, null) + allowed_values = try(rule.value.allow.values, null) == null ? null : [ + for v in rule.value.allow.values : templatestring(v, var.context.condition_vars) + ] + denied_values = try(rule.value.deny.values, null) == null ? null : [ + for v in rule.value.deny.values : templatestring(v, var.context.condition_vars) + ] } } } diff --git a/tests/modules/organization/test_plan_org_policies_modules.py b/tests/modules/organization/test_plan_org_policies_modules.py index 5315da34a..708c0ce03 100644 --- a/tests/modules/organization/test_plan_org_policies_modules.py +++ b/tests/modules/organization/test_plan_org_policies_modules.py @@ -54,7 +54,7 @@ def test_policy_implementation(): '- parent = local.folder_id\n', '+ name = "${var.organization_id}/policies/${each.value}"\n', '+ parent = var.organization_id\n', - '@@ -187,0 +188,9 @@\n', + '@@ -195,0 +196,9 @@\n', '+ depends_on = [\n', '+ google_organization_iam_binding.authoritative,\n', '+ google_organization_iam_binding.bindings,\n',