module doesn't work with dynamically computed values any longer

[ideasculptor]

The _num variables were removed, but apparently not with any concern over whether things will break.

I call it as such:

module "projects-iam" {
  source = "terraform-google-modules/iam/google//modules/projects_iam"

  projects     = [ local.project_id ]

  mode         = "additive"
  bindings = zipmap(
    var.project_roles,
    [ for s in var.project_roles :
        ["serviceAccount:${module.gke.service_account}"]
    ]
  )
}

where var.project_roles is a list passed in as a variable (but which is actually statically defined by the default value, which has 3 roles. But when I attempt to run it, I get the following error:

Error: Invalid for_each argument

  on .terraform/modules/projects-iam/terraform-google-modules-terraform-google-iam-45700d3/modules/projects_iam/main.tf line 44, in resource "google_project_iam_member" "project_iam_additive":
  44:   for_each = module.helper.set_additive

The "for_each" value depends on resource attributes that cannot be determined
until apply, so Terraform cannot predict how many instances will be created.
To work around this, use the -target argument to first apply only the
resources that the for_each depends on.

It worked fine when I included the _num variables for projects and bindings. It would seem that it either doesn't like a zipmap made up of constants, or else it doesn't like that it includes a string constructed from an output from another module. Either way, there doesn't seem to be a fix that works. I've tried explicitly targeting things like the service account, but that doesn't work.

Note that this occurs when I have a working gke cluster, but I made a change to the name, which forces a new cluster. When I added the call to projects-iam module to my terraform template when the GKE cluster already existed, it didn't complain. But it doesn't seem able to update both the cluster and the iam assignment at the same time - not if the name of the service account will change, anyway.

[ideasculptor]

And it is certainly necessary to be able to apply role assignments to a service account created within another module. That's a very common pattern, allowing a user to add custom authorizations to a service account created by a 3rd party module. Without it, the 3rd party module will create a service account that lacks crucial authorizations in order to be able to function.

[ideasculptor]

Doesn't it make much more sense to only ditch *_num variables once hashicorp has actually fixed HCL so that it can deal with iteration over dynamically computed values? As far as I can tell, the update to 0.12 provided new syntax for iteration but didn't fix the fundamental flaw in computing plans without having static lists to iterate over. for_each will work without error so long as the iteration is over a static collection, but it blows up when it cannot determine the iteration values in advance. That's the exact same problem that occurs with count, hence the continued need for _num variables.

[morgante]

You're right that unfortunately there is no perfect solution here:

1. Using the num solution works okay with count but prevents us from keying based on anything besides the iterator number, meaning adding a new role or editing a member can cause updates to unrelated roles (ex. #38 and other similar bugs)
2. Using for_each allows us to key off the the member and prevent those bugs, but does lead to issues with dynamic members.

As discussed in caveats, we provide two compromises:

1. If you want to apply to a single entity, you can use project and this can be a computed value.
2. If you want to use dynamic bindings, you need to use authoritative mode.

If you have a suggestion for how to work around these without introducing bugs like #38, I'm definitely open to suggestions.

[ideasculptor]

I guess I don't have a good answer. I did solve the problem in an etcd provider I built where I played some tricks to make plan comparison work correctly, but I know no one is going to approve the hacks I used in the official google provider. I don't really remember the details and it would take some work to even unwind what I did from that old source code. My Go skills are limited and my whole provider was a hack that really centered on solving this problem for dynamic lists of config values passed to my etcd provider.

I wouldn't mind a version of the module which expects a single value for just the recipient of the roles - since a very common use case will be assigning multiple roles to a single service account created by another module. Such a list of roles is pretty much always going to be a statically defined list in a vars file somewhere or else in a default value for a variable, and terraform doesn't barf on those. Projects can already be specified as singleton or list. So that just leaves the recipients list to blow the module up because of dynamic values, and that list is very likely to have a length of 1. A version of the module which takes that into account would eliminate this problem for me.

module "mymodule" {
  create_service_account = true
  ...
}

module "project-iam" {
  recipient = module.mymodule.service_account_address

  roles_list = var.roles_list
  project = module.mymodule.project

  # or pass a map of project->roles_list to eliminate multiple calls to module
  roles_map = var.roles_map
}

[morgante]

I think that could make sense as a separate submodule, with an interface like:

module "member-roles" {
  source   = "terraform-google-modules/iam/google//modules/member_iam"
  service_account = module.mymodule.service_account_address
  project = my-project
  project_roles = ["roles/owner", "roles/editor"]
}

[ideasculptor]

Technically, it's not doing anything that you couldn't do in the same number of lines of code without the module, but it appeals to my sense of order to use a module from this repo for all iam assignments.

resource "google_project_iam_member" "project_iam_additive" {
  for_each = var.project_roles
  project  = my-project
  role     = each.key
  member   = module.mymodule.service_account_address
}

That's actually why I suggested the map of project to roles to allow assignment of roles for multiple projects, since that can potentially be a useful thing to do (service account needs different roles in service project and network host project, for example), and it at least does something non-trivial for the user.

[morgante]

That's actually why I suggested the map of project to roles to allow assignment of roles for multiple projects, since that can potentially be a useful thing to do (service account needs different roles in service project and network host project, for example), and it at least does something non-trivial for the user.

That's true, but the problem with it is it would prevent using a dynamic project ID. I'm thinking we should keep the module simple (though admittedly not much additional value over implementing it yourself).

[ideasculptor]

Ah, right, and the other common use case for this module is populating the service account for a project in the project factory, which is about the only place a dynamic project id is a thing.

Fair enough, if I get to it before your team does, I’ll do it that way.

[torgeir]

For the record, if anyone else stumbles here;

I had similar issues with the subnets-iam module that also use the same type of loop that is not supported for the "additive" mode. Resolved by using the google_compute_subnetwork_iam_member directly instead.