feat!: Swap count for for_each on subnet IAM resources

[thatcoleyouknow]

As an engineer that works for an organization that adds new subnets on a regular basis, we find ourselves constantly having to review very lengthy Terraform plans that are full of google_compute_subnetwork_iam_member recreations. This is partly due to Terraform lists being unordered but this can now be solved for by replacing the count argument with for_each in each of those resources. This would be a one-time breaking change, but it would also make it the last time consumers have to deal subnet IAM grants getting recreated due to changes in the list of subnets passed to this module.

If you all have any questions, concerns, or suggestions, I'd be happy to talk through those. I am also an enterprise customer so I can open a feature request with our TAM to try and help prioritize this, if that would help you all.

Thank you!

[thatcoleyouknow]

FWIW this has been tested internally. If any issues arise once someone kicks off the checks, feel free to @ me.

[thatcoleyouknow]

Could someone tell me what failed in the terraform-google-project-factory-int-trigger check? I don't have permissions to view that.

[thatcoleyouknow]

@imrannayer - Forgive the ping but I see you are an active maintainer. Is there a preferable way to get the gcbrun command ran on this PR so I can try and work through any issues preventing this from being reviewed for merge?

[bharathkkb]

/gcbrun

[thatcoleyouknow]

Could someone tell me what failed in the terraform-google-project-factory-int-trigger check? I don't have permissions to view that.

Now that this job actually ran, could I get someone to share why this failed?

[bharathkkb]

Hi @thatcoleyouknow , it failed when trying to apply the example dynamic-shared-vpc-local. I suspect switching to for_each is causing some values to be unknown during plan. Could you try testing that example locally?

STDERR: Error: Invalid for_each argument

  on ../../../modules/core_project_factory/main.tf line 215, in resource "google_compute_subnetwork_iam_member" "service_account_role_to_vpc_subnets":
 215:   for_each = (var.grant_network_role && var.enable_shared_vpc_service_project && length(var.shared_vpc_subnets) > 0 && var.create_project_sa) ? toset(var.shared_vpc_subnets) : toset([])
    ├────────────────
    │ var.create_project_sa is true
    │ var.enable_shared_vpc_service_project is true
    │ var.grant_network_role is true
    │ var.shared_vpc_subnets is list of string with 2 elements

The "for_each" set includes values derived from resource attributes that
cannot be determined until apply, and so Terraform cannot determine the full
set of keys that will identify the instances of this resource.

When working with unknown values in for_each, it's better to use a map value
where the keys are defined statically in your configuration and where only
the values contain apply-time results.

Alternatively, you could use the -target planning option to first apply only
the resources that the for_each value depends on, and then apply a second
time to fully converge.

Error: Invalid for_each argument

  on ../../../modules/core_project_factory/main.tf line 265, in resource "google_compute_subnetwork_iam_member" "apis_service_account_role_to_vpc_subnets":
 265:   for_each = (var.grant_network_role && var.enable_shared_vpc_service_project && length(var.shared_vpc_subnets) > 0) ? toset(var.shared_vpc_subnets) : toset([])
    ├────────────────
    │ var.enable_shared_vpc_service_project is true
    │ var.grant_network_role is true
    │ var.shared_vpc_subnets is list of string with 2 elements

The "for_each" set includes values derived from resource attributes that
cannot be determined until apply, and so Terraform cannot determine the full
set of keys that will identify the instances of this resource.

When working with unknown values in for_each, it's better to use a map value
where the keys are defined statically in your configuration and where only
the values contain apply-time results.

Alternatively, you could use the -target planning option to first apply only
the resources that the for_each value depends on, and then apply a second
time to fully converge.

Error: Invalid for_each argument

  on ../../../modules/shared_vpc_access/main.tf line 98, in resource "google_compute_subnetwork_iam_member" "service_shared_vpc_subnet_users":
  98:   for_each = var.grant_network_role ? toset(local.subnetwork_api) : toset([])
    ├────────────────
    │ local.subnetwork_api is list of string with 6 elements
    │ var.grant_network_role is true

The "for_each" set includes values derived from resource attributes that
cannot be determined until apply, and so Terraform cannot determine the full
set of keys that will identify the instances of this resource.

When working with unknown values in for_each, it's better to use a map value
where the keys are defined statically in your configuration and where only
the values contain apply-time results.

Alternatively, you could use the -target planning option to first apply only
the resources that the for_each value depends on, and then apply a second
time to fully converge.

Error: Invalid for_each argument

  on ../../../modules/shared_vpc_access/main.tf line 124, in resource "google_compute_subnetwork_iam_member" "cloudservices_shared_vpc_subnet_users":
 124:   for_each = local.gke_shared_vpc_enabled && var.enable_shared_vpc_service_project && var.grant_network_role ? toset(local.subnetwork_api) : toset([])
    ├────────────────
    │ local.gke_shared_vpc_enabled is true
    │ local.subnetwork_api is list of string with 6 elements
    │ var.enable_shared_vpc_service_project is true
    │ var.grant_network_role is true

The "for_each" set includes values derived from resource attributes that
cannot be determined until apply, and so Terraform cannot determine the full
set of keys that will identify the instances of this resource.

When working with unknown values in for_each, it's better to use a map value
where the keys are defined statically in your configuration and where only
the values contain apply-time results.

Alternatively, you could use the -target planning option to first apply only
the resources that the for_each value depends on, and then apply a second
time to fully converge.
---- End output of terraform apply -auto-approve -lock=true -lock-timeout=0s -input=false -no-color -parallelism=10 -refresh=true 

[thatcoleyouknow]

@bharathkkb - Per Google's recommendation, we use the Google's Terraform example foundation[1] to manage each layer our foundation, which also uses the terraform-google-network module to provision the VPC and subnets. The terraform-google-network module uses lists for all outputs, leaving me with 2 sensible options here:

1. (Preferred) Adjust the shared_vpc example in this repo to pass formatted self_links to the service project modules so the host project, network, subnets, and service projects that use those subnets can all be deployed in a single apply.
2. Submit a PR to the terraform-google-network module to output formatted self_links based on inputs to solve for the Terraform limitation that requires the self_link to be known before the subnet IAM resources can be planned before apply.

While the approach used in the example[2] may make sense for a simple test, it doesn't align with Google's own Terraform best practices[3]. In most real world scenarios, one would not want to manage all layers of a foundation in a single root module.

Would you be open to me also changing how the subnets are passed from module.vpc to the service project modules (module.service-project*)? I'd like to format the subnets passed to the service project modules so the calculated value isn't needed during the plan phase. That would look something like this:

Lines 17-20 and 95-107 of shared_vpc example[4]...

locals {
  subnets = {
    "subnet_01" = {
      name         = "${var.network_name}-subnet-01"
      region       = "us-west1"
      ip_cidr_range = "10.10.10.0/24"
    }
    "subnet_02" = {
      name         = "${var.network_name}-subnet-02"
      region       = "us-west1"
      ip_cidr_range = "10.10.20.0/24"
    }
  }
  subnet_self_links = [
    for _, subnet in local.subnets :
    format(
      "projects/%s/regions/%s/subnetworks/%s",
      module.host-project.project_id,
      subnet.region,
      subnet.name
    )
  ]
}

...

module "service-project" {
  source  = "terraform-google-modules/project-factory/google//modules/svpc_service_project"
  version = "~> 18.0"

  name              = var.service_project_name
  random_project_id = false

  org_id          = var.organization_id
  folder_id       = var.folder_id
  billing_account = var.billing_account

  shared_vpc         = module.host-project.project_id
  shared_vpc_subnets = module.vpc.subnets_self_links
  shared_vpc_subnets = local.subnet_self_links
...

[1] https://github.com/terraform-google-modules/terraform-example-foundation
[2] https://github.com/terraform-google-modules/terraform-google-project-factory/blob/main/examples/shared_vpc/main.tf
[3] https://cloud.google.com/docs/terraform/best-practices
[4] https://github.com/terraform-google-modules/terraform-google-project-factory/blob/main/examples/shared_vpc/main.tf#L95-L107