From b0bc896a6800ca41cd8d455e5228aed614b05bc9 Mon Sep 17 00:00:00 2001 From: Ludovico Magnocavallo Date: Wed, 22 Oct 2025 18:51:06 +0200 Subject: [PATCH] Allow null project id in service account module when reusing service account (#3452) * allow null project id for service account reuse * fix pf --- modules/iam-service-account/README.md | 54 +++++++++++++++---- modules/iam-service-account/iam.tf | 20 ++++--- modules/iam-service-account/main.tf | 18 +++++-- modules/iam-service-account/outputs.tf | 2 +- modules/iam-service-account/variables.tf | 27 ++++++++-- modules/project-factory/automation.tf | 3 -- .../projects-service-accounts.tf | 3 -- .../fast/stages/s0_org_setup/not-simple.yaml | 18 +------ .../iam_service_account/examples/reuse-0.yaml | 29 ++++++++++ .../project_factory/examples/example.yaml | 4 +- 10 files changed, 125 insertions(+), 53 deletions(-) create mode 100644 tests/modules/iam_service_account/examples/reuse-0.yaml diff --git a/modules/iam-service-account/README.md b/modules/iam-service-account/README.md index cd4e100e6..8df4621ff 100644 --- a/modules/iam-service-account/README.md +++ b/modules/iam-service-account/README.md @@ -4,12 +4,10 @@ This module allows simplified creation and management of one a service account a Note that outputs have no dependencies on IAM bindings to prevent resource cycles. -## TOC - -- [TOC](#toc) - [Simple Example](#simple-example) - [IAM](#iam) +- [Reusing Existing Service Accounts](#reusing-existing-service-accounts) - [Tag Bindings](#tag-bindings) - [Files](#files) - [Variables](#variables) @@ -85,6 +83,40 @@ module "service-account-with-tags" { # tftest modules=1 resources=3 inventory=iam.yaml ``` +## Reusing Existing Service Accounts + +Like other modules in this repository, this module allows reusing existing service accounts where only IAM or tag bindings management is needed, via the `service_account_reuse` variable. + +When reusing service accounts, the `name` variable can be set to the fully fledged service account email. In such cases the `project_id` variable can be ignored as the project id is derived from the email. + +The `service_account_reuse.use_data_source` flag also allows to skip the data source used to fetch the service account unique id (numeric), which is only used when setting tag bindings. If those are needed while still skipping the data source, populate the additional attributes `service_account_reuse.attributes`. + +```hcl +module "service-account" { + source = "./fabric/modules/iam-service-account" + name = "test-0@myproject.iam.gserviceaccount.com" + context = { + folder_ids = { + test = "folders/1234567890" + } + } + iam_billing_roles = { + "ABCDE-12345-ABCDE" = [ + "roles/billing.user" + ] + } + iam_folder_roles = { + "$folder_ids:test" = [ + "roles/resourcemanager.folderAdmin" + ] + } + service_account_reuse = { + use_data_source = false + } +} +# tftest modules=1 resources=2 inventory=reuse-0.yaml +``` + ## Tag Bindings Use the `tag_bindings` variable to attach tags to the service account. Provide `project_number` to prevent potential permadiffs with the tag binding resource. @@ -119,12 +151,11 @@ module "service-account-with-tags" { | name | description | type | required | default | |---|---|:---:|:---:|:---:| -| [name](variables.tf#L55) | Name of the service account to create. | string | ✓ | | -| [project_id](variables.tf#L70) | Project id where service account will be created. | string | ✓ | | +| [name](variables.tf#L58) | Name of the service account to create. | string | ✓ | | | [context](variables.tf#L17) | External context used in replacements. | object({…}) | | {} | | [create_ignore_already_exists](variables.tf#L33) | If set to true, skip service account creation if a service account with the same email already exists. | bool | | null | -| [description](variables.tf#L43) | Optional description. | string | | null | -| [display_name](variables.tf#L49) | Display name of the service account to create. | string | | "Terraform-managed." | +| [description](variables.tf#L44) | Optional description. | string | | null | +| [display_name](variables.tf#L51) | Display name of the service account to create. | string | | "Terraform-managed." | | [iam](variables-iam.tf#L17) | IAM bindings in {ROLE => [MEMBERS]} format. | map(list(string)) | | {} | | [iam_billing_roles](variables-iam.tf#L24) | Billing account roles granted to this service account, by billing account id. Non-authoritative. | map(list(string)) | | {} | | [iam_bindings](variables-iam.tf#L31) | Authoritative IAM bindings in {KEY => {role = ROLE, members = [], condition = {}}}. Keys are arbitrary. | map(object({…})) | | {} | @@ -136,10 +167,11 @@ module "service-account-with-tags" { | [iam_project_roles](variables-iam.tf#L89) | Project roles granted to this service account, by project id. | map(list(string)) | | {} | | [iam_sa_roles](variables-iam.tf#L96) | Service account roles granted to this service account, by service account name. | map(list(string)) | | {} | | [iam_storage_roles](variables-iam.tf#L103) | Storage roles granted to this service account, by bucket name. | map(list(string)) | | {} | -| [prefix](variables.tf#L60) | Prefix applied to service account names. | string | | null | -| [project_number](variables.tf#L75) | Project number of var.project_id. Set this to avoid permadiffs when creating tag bindings. | string | | null | -| [service_account_reuse](variables.tf#L81) | Reuse existing service account if not null. Data source can be forced disabled if tag bindings are not used, or unique id is set. | object({…}) | | null | -| [tag_bindings](variables.tf#L92) | Tag bindings for this service accounts, in key => tag value id format. | map(string) | | {} | +| [prefix](variables.tf#L64) | Prefix applied to service account names. | string | | null | +| [project_id](variables.tf#L75) | Project id where service account will be created. This can be left null when reusing service accounts. | string | | null | +| [project_number](variables.tf#L89) | Project number of var.project_id. Set this to avoid permadiffs when creating tag bindings. This can be left null when reusing service accounts and tags are not used. | string | | null | +| [service_account_reuse](variables.tf#L96) | Reuse existing service account if not null. Data source can be forced disabled if tag bindings are not used, or unique id is set. | object({…}) | | null | +| [tag_bindings](variables.tf#L109) | Tag bindings for this service accounts, in key => tag value id format. | map(string) | | {} | ## Outputs diff --git a/modules/iam-service-account/iam.tf b/modules/iam-service-account/iam.tf index f993764d6..f3a452c17 100644 --- a/modules/iam-service-account/iam.tf +++ b/modules/iam-service-account/iam.tf @@ -89,9 +89,11 @@ locals { } resource "google_service_account_iam_binding" "authoritative" { - for_each = local.iam - service_account_id = local.service_account.name - role = lookup(local.ctx.custom_roles, each.key, each.key) + for_each = local.iam + service_account_id = try( + local.service_account.name, local.static_id + ) + role = lookup(local.ctx.custom_roles, each.key, each.key) members = [ for v in each.value : lookup(local.ctx.iam_principals, v, v) @@ -99,8 +101,10 @@ resource "google_service_account_iam_binding" "authoritative" { } resource "google_service_account_iam_binding" "bindings" { - for_each = var.iam_bindings - service_account_id = local.service_account.name + for_each = var.iam_bindings + service_account_id = try( + local.service_account.name, local.static_id + ) role = lookup( local.ctx.custom_roles, each.value.role, each.value.role ) @@ -120,8 +124,10 @@ resource "google_service_account_iam_binding" "bindings" { } resource "google_service_account_iam_member" "bindings" { - for_each = local.iam_bindings_additive - service_account_id = local.service_account.name + for_each = local.iam_bindings_additive + service_account_id = try( + local.service_account.name, local.static_id + ) role = lookup( local.ctx.custom_roles, each.value.role, each.value.role ) diff --git a/modules/iam-service-account/main.tf b/modules/iam-service-account/main.tf index 8725ee40c..737e7d13f 100644 --- a/modules/iam-service-account/main.tf +++ b/modules/iam-service-account/main.tf @@ -26,11 +26,19 @@ locals { ? "serviceAccount:${local.service_account.email}" : local.static_iam_email ) - name = split("@", var.name)[0] - prefix = var.prefix == null ? "" : "${var.prefix}-" - project_id = lookup(local.ctx.project_ids, var.project_id, var.project_id) + name = split("@", var.name)[0] + prefix = var.prefix == null ? "" : "${var.prefix}-" + project_id = ( + var.project_id == null + # if no project ID is passed we're reusing and can infer it from the email + ? try(regex("^[^@]+@([^.]+)", var.name)[0], null) + # otherwise check if we need context expansion + : lookup(local.ctx.project_ids, var.project_id, var.project_id) + ) static_email = ( - "${local.prefix}${local.name}@${local.sa_domain}.iam.gserviceaccount.com" + var.project_id == null + ? var.name + : "${local.prefix}${local.name}@${local.sa_domain}.iam.gserviceaccount.com" ) static_iam_email = "serviceAccount:${local.static_email}" static_id = ( @@ -70,7 +78,7 @@ data "google_service_account" "service_account" { } resource "google_service_account" "service_account" { - count = local.use_data_source ? 0 : 1 + count = var.service_account_reuse == null ? 1 : 0 project = local.project_id account_id = "${local.prefix}${local.name}" display_name = var.display_name diff --git a/modules/iam-service-account/outputs.tf b/modules/iam-service-account/outputs.tf index 1c80760e6..16b856f89 100644 --- a/modules/iam-service-account/outputs.tf +++ b/modules/iam-service-account/outputs.tf @@ -53,5 +53,5 @@ output "service_account" { output "unique_id" { description = "Fully qualified service account id." - value = local.service_account.unique_id + value = try(local.service_account.unique_id, null) } diff --git a/modules/iam-service-account/variables.tf b/modules/iam-service-account/variables.tf index 47845be48..eac757aae 100644 --- a/modules/iam-service-account/variables.tf +++ b/modules/iam-service-account/variables.tf @@ -26,13 +26,14 @@ variable "context" { storage_buckets = optional(map(string), {}) tag_values = optional(map(string), {}) }) - default = {} nullable = false + default = {} } variable "create_ignore_already_exists" { description = "If set to true, skip service account creation if a service account with the same email already exists." type = bool + nullable = true default = null validation { condition = !(var.create_ignore_already_exists == true && var.service_account_reuse == null) @@ -43,23 +44,27 @@ variable "create_ignore_already_exists" { variable "description" { description = "Optional description." type = string + nullable = true default = null } variable "display_name" { description = "Display name of the service account to create." type = string + nullable = true default = "Terraform-managed." } variable "name" { description = "Name of the service account to create." + nullable = false type = string } variable "prefix" { description = "Prefix applied to service account names." type = string + nullable = true default = null validation { condition = var.prefix != "" @@ -68,13 +73,23 @@ variable "prefix" { } variable "project_id" { - description = "Project id where service account will be created." + description = "Project id where service account will be created. This can be left null when reusing service accounts." type = string + nullable = true + default = null + validation { + condition = ( + var.project_id != null || + var.service_account_reuse != null && strcontains(var.name, "@") + ) + error_message = "Project id can only be null when reusing service accounts and a fully qualified email is passed as name." + } } variable "project_number" { - description = "Project number of var.project_id. Set this to avoid permadiffs when creating tag bindings." + description = "Project number of var.project_id. Set this to avoid permadiffs when creating tag bindings. This can be left null when reusing service accounts and tags are not used." type = string + nullable = true default = null } @@ -83,10 +98,12 @@ variable "service_account_reuse" { type = object({ use_data_source = optional(bool, true) attributes = optional(object({ - unique_id = string + project_number = number + unique_id = string })) }) - default = null + nullable = true + default = null } variable "tag_bindings" { diff --git a/modules/project-factory/automation.tf b/modules/project-factory/automation.tf index 3cd06fc7d..9cefd5f7e 100644 --- a/modules/project-factory/automation.tf +++ b/modules/project-factory/automation.tf @@ -159,9 +159,6 @@ module "automation-service-accounts-iam" { name = module.automation-service-accounts[each.key].name service_account_reuse = { use_data_source = false - attributes = { - unique_id = module.automation-service-accounts[each.key].unique_id - } } context = merge(local.ctx, { service_account_ids = local.project_sas_ids diff --git a/modules/project-factory/projects-service-accounts.tf b/modules/project-factory/projects-service-accounts.tf index 9488f1fb8..15fc35af0 100644 --- a/modules/project-factory/projects-service-accounts.tf +++ b/modules/project-factory/projects-service-accounts.tf @@ -87,9 +87,6 @@ module "service_accounts-iam" { name = each.value.name service_account_reuse = { use_data_source = false - attributes = { - unique_id = module.service-accounts[each.key].unique_id - } } context = merge(local.ctx, { project_ids = local.ctx_project_ids diff --git a/tests/fast/stages/s0_org_setup/not-simple.yaml b/tests/fast/stages/s0_org_setup/not-simple.yaml index 4e1f7d057..d7b8f47ec 100644 --- a/tests/fast/stages/s0_org_setup/not-simple.yaml +++ b/tests/fast/stages/s0_org_setup/not-simple.yaml @@ -1634,13 +1634,6 @@ values: member: serviceAccount:iac-vpcsc-rw@ft0-prod-iac-core-0.iam.gserviceaccount.com project: ft0-prod-iac-core-0 timeouts: null - module.factory.module.service_accounts-iam["iac-0/iac-org-cicd-ro"].google_service_account.service_account[0]: - account_id: iac-org-cicd-ro - create_ignore_already_exists: null - description: null - disabled: false - display_name: Terraform-managed. - timeouts: null ? module.factory.module.service_accounts-iam["iac-0/iac-org-cicd-ro"].google_service_account_iam_member.additive["$service_account_ids:iac-0/iac-org-ro-roles/iam.serviceAccountTokenCreator"] : condition: [] role: roles/iam.serviceAccountTokenCreator @@ -1649,13 +1642,6 @@ values: : condition: [] role: roles/iam.workloadIdentityUser service_account_id: projects/ft0-prod-iac-core-0/serviceAccounts/iac-org-ro@ft0-prod-iac-core-0.iam.gserviceaccount.com - module.factory.module.service_accounts-iam["iac-0/iac-org-cicd-rw"].google_service_account.service_account[0]: - account_id: iac-org-cicd-rw - create_ignore_already_exists: null - description: null - disabled: false - display_name: Terraform-managed. - timeouts: null ? module.factory.module.service_accounts-iam["iac-0/iac-org-cicd-rw"].google_service_account_iam_member.additive["$service_account_ids:iac-0/iac-org-rw-roles/iam.serviceAccountTokenCreator"] : condition: [] role: roles/iam.serviceAccountTokenCreator @@ -2859,7 +2845,7 @@ counts: google_project_iam_member: 15 google_project_service: 33 google_project_service_identity: 9 - google_service_account: 16 + google_service_account: 14 google_service_account_iam_member: 4 google_storage_bucket: 3 google_storage_bucket_iam_binding: 4 @@ -2873,5 +2859,5 @@ counts: google_tags_tag_value_iam_binding: 4 local_file: 9 modules: 46 - resources: 311 + resources: 309 terraform_data: 2 diff --git a/tests/modules/iam_service_account/examples/reuse-0.yaml b/tests/modules/iam_service_account/examples/reuse-0.yaml new file mode 100644 index 000000000..8e27dfdf3 --- /dev/null +++ b/tests/modules/iam_service_account/examples/reuse-0.yaml @@ -0,0 +1,29 @@ +# Copyright 2023 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.service-account.google_billing_account_iam_member.billing-roles["ABCDE-12345-ABCDE-roles/billing.user"]: + billing_account_id: ABCDE-12345-ABCDE + condition: [] + role: roles/billing.user + module.service-account.google_folder_iam_member.folder-roles["$folder_ids:test-roles/resourcemanager.folderAdmin"]: + condition: [] + folder: folders/1234567890 + role: roles/resourcemanager.folderAdmin + +counts: + google_billing_account_iam_member: 1 + google_folder_iam_member: 1 + modules: 1 + resources: 2 diff --git a/tests/modules/project_factory/examples/example.yaml b/tests/modules/project_factory/examples/example.yaml index 49359822f..d3bbc0842 100644 --- a/tests/modules/project_factory/examples/example.yaml +++ b/tests/modules/project_factory/examples/example.yaml @@ -620,7 +620,7 @@ counts: google_project_iam_member: 21 google_project_service: 13 google_project_service_identity: 4 - google_service_account: 7 + google_service_account: 6 google_service_account_iam_binding: 1 google_storage_bucket: 1 google_storage_bucket_iam_binding: 2 @@ -630,5 +630,5 @@ counts: google_tags_tag_value: 2 google_tags_tag_value_iam_binding: 1 modules: 23 - resources: 90 + resources: 89 terraform_data: 1