Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Erroneous 'Invalid for_each argument' when interpolating const map #22735

Closed
OJFord opened this issue Sep 7, 2019 · 15 comments · Fixed by #23999
Closed

Erroneous 'Invalid for_each argument' when interpolating const map #22735

OJFord opened this issue Sep 7, 2019 · 15 comments · Fixed by #23999
Labels
bug config v0.12 Issues (primarily bugs) reported against v0.12 releases

Comments

@OJFord
Copy link
Contributor

OJFord commented Sep 7, 2019

Terraform Version

0.12.8

Terraform Configuration Files

locals {
  server_count = {
    location1 = {
        type1 = 3
    }
  }

  server_set = toset(flatten([
    for loc in keys(local.server_count) : [
      for type in keys(local.server_count[loc]) : [
        for i in range(local.server_count[loc][type]) : "${loc}_${type}_${i}"
      ]
    ]
  ]))
}

resource "" "" {
  for_each = local.server_set

  # ...
}

Expected Behavior

Successful plan.

Actual Behavior

Error: Invalid for_each argument

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.

Steps to Reproduce

  1. Config as described
  2. terraform plan

Additional Context

As expected, terraform apply -target=locals.server_set does not help:

Apply complete! Resources: 0 added, 0 changed, 0 destroyed.

@venky999
Copy link

venky999 commented Sep 9, 2019

I am seeing same issue with terraform 0.12.8

/******************************* module foo ***********************/
//main.tf

resource "null_resource" "foo" {
   for_each = {"one" : "1","two" : "2","three" : "3"}
   triggers = {
       key = each.key
       value = each.value
       id = uuid()
   }
}

//output.tf

output "ids" {
  value =  [for item in null_resource.foo : item.triggers.id ]
}

/*************************************** ***********************/

//main.tf

module "foo" {
  source       = "./modules/foo"
}

locals {
  server_count = {
    location1 = {
        type1 = 3
    }
  }

  server_set = toset(flatten([
    for loc in keys(local.server_count) : [
      for type in keys(local.server_count[loc]) : [
        for i in range(local.server_count[loc][type]) : join(",", module.foo.ids )
      ]
    ]
  ]))
}
resource "null_resource" "test" {
  for_each = local.server_set

  triggers = {
    key   = each.key

  }
}

@teamterraform
Copy link
Contributor

Hi @OJFord! Thanks for reporting this.

Was this working for you before in Terraform 0.12.7, or did you try this configuration for the first time in Terraform 0.12.8? We made some changes in this area in 0.12.8 to address some related bugs, so we'd like to try to figure out if this is a regression from those changes or a separate issue. Thanks!

@OJFord
Copy link
Contributor Author

OJFord commented Sep 9, 2019

@terraformbot I believe it is actually resolved (perhaps by the changes in .7->.8) - the server_set (and it's implied separation from a bigger map with 'not known before apply' data) is the key. However, I think the remaining issue (and perhaps@venky999 can confirm) is that the error appears quite far away in the output (with or without trace) from the problematic for_each.

So you can get output like:

# ...
creating <some resource that has been fixed with a plan-time for_each>

Error: Invalid for_each argument

where it looks like the fix hasn't worked, but actually it relates to a for_each in a different resource, listed much further up, that was missed in fixing for_eachs.

However, what would be really great is if only the each.key needed to be known before apply; then this hack isn't needed anyway. For example:

resource "server_resource" "servers" {
  count = 3
  name = "server-${count.index}"
  # ...
}

locals {
  servers = { for s in server_resource : s.name => s.id } # or whatever post-apply values
}

# *Not* possible today, don't copy-paste!
resource "null_resource" "n" {
  for_each = local.servers  # each.key is known at plan, each.value isn't

  # use `each.value`
}

@venky999
Copy link

venky999 commented Sep 9, 2019

@teamterraform @OJFord @pselle I have observed https://github.com/hashicorp/terraform/pull/22597 in terraform 0.12.7 and terraform 0.12.8 fixed few of the issues but strangely in some cases like the above one I have posted it is still happening.. I am not quite sure why this is the case....

@venky999
Copy link

venky999 commented Sep 10, 2019

Any update on this issue.. I am kind of stuck because of this issue...

@pselle
Copy link
Contributor

pselle commented Sep 10, 2019

Thanks for the report @OJFord!

where it looks like the fix hasn't worked, but actually it relates to a for_each in a different resource, listed much further up, that was missed in fixing for_eachs.

I've opened #22760 to add error source addressing for this error, which will make it much more useful, and easier to debug.

I tested the initial issue's case and was able to successfully apply (which it sounds like you did as well), so that case looks fixed to me. In regards to the most recent comment, what you said should be true:

However, what would be really great is if only the each.key needed to be known before apply; then this hack isn't needed anyway.

When I try your example on a map with known keys (but unknown values), things work smoothly for me. In general, this should be true (that map keys must be known, but values need not necessarily).

Thanks so much for including examples, they are very helpful!

Here's my working example (on 0.12.8):

resource "random_pet" "a" {
  count = 2
  prefix = "a-${count.index}"
}

locals {
  pets = { for p in random_pet.a : p.prefix => p.id }
}

resource "random_pet" "b" {
  for_each = local.pets
  prefix = each.value
}

@venky999
Copy link

@pselle Thank you for clarifying.. I am still not sure why the we are seeing this issue in the example I have posted above.

@venky999
Copy link

venky999 commented Sep 12, 2019

I am able to reproduce the above error in the following examples as well ....

/// not working

 dogs = [
     {
       name = "tommy"
     },
     {
       name = "sneeky"
     }
  ]

  cats = [
     {
       name = "toffle"
     },
     {
       name = "ruffle"
     }
  ]
  
  pets = {
    dogs = [ for item in local.dogs : merge(item, map("id", uuid() )) ]
    cats = [ for item in local.cats : merge(item, map("id", uuid() )) ]
  }
}

resource "random_pet" "dogs" {
  for_each = { for item in local.pets["dogs"] : item["name"] => item }
  prefix = each.key
}

but if I change

resource "random_pet" "dogs" {
  for_each = { for item in local.pets["dogs"] : item["name"] => item }
  prefix = each.key
}

to

resource "random_pet" "pets" {
  for_each =  local.pets
  prefix = each.key
}

the error disappears....

and even if change the above code to use maps... the error is gone ...

working example ..

locals {
  dogs = [
     {
       name = "tommy"
     },
     {
       name = "sneeky"
     }
  ]

  cats = [
     {
       name = "toffle"
     },
     {
       name = "ruffle"
     }
  ]
  
  pets = {
    dogs = { for item in local.dogs : item["name"] => merge(item, map("id", uuid() )) }
    cats = { for item in local.cats : item["name"] => merge(item, map("id", uuid() )) }
  }
}

resource "random_pet" "dogs" {
  for_each = local.pets["dogs"]
  prefix = each.key
}

and the error returns again when I change to

resource "random_pet" "dogs" {
  for_each = { for item, value in local.pets["dogs"] : item => value if value["name"] != "" }
  prefix = each.key
}

or

resource "random_pet" "dogs" {
  for_each = { for item, value in local.pets["dogs"] : item => value... if value["name"] != "" }
  prefix = each.key
}

it looks to me as if for_each is failing when we try to construct a dynamic map using for from an object that is constructed dynamically .... and assign the constructed map to for_each ....

@pselle
Copy link
Contributor

pselle commented Sep 15, 2019

This is the line that seems like the best candidate to me, @venky999:

    for i in range(local.server_count[loc][type]) : join(",", module.foo.ids )

In the set definition, that's creating a dependency on the output of the module, thus leading me to believe the set contains unknown values (a set for for_each cannot consist of unknown values; this is because the values of the set are used to create keys for the instances, thus they must be known).

@venky999
Copy link

venky999 commented Sep 16, 2019

thank you @pselle ... do you think the same issue might be the reason for the error in the pets examples I have posted ...

@pselle
Copy link
Contributor

pselle commented Sep 17, 2019

@venky999 Sort of -- it has to do with the dynamism of the data, and when the values are known. Just because an expression contains "constant" values, depending on when things are evaluated, it may still be unknown from Terraform's perspective, and if values are unknown when TF is assigning keys to instances, that's a problem (from TF's perception of the world).

I'm going to keep thinking about this and leave this open for now.

@venky999
Copy link

thank you @pselle

@nik-shornikov
Copy link

nik-shornikov commented Jan 28, 2020

In the set definition, that's creating a dependency on the output of the module

I believe this is the bug. I have a module that uses for_each on a set(string) parameter functions. Meaning, I only use the key, and there can never be anything unknown about the number of string keys (not even before the dependencies are planned).

I parametrize the module like this: functions = [ "${module.a_function.function_name}", "${module.another_function.function_name}" ]. It fails with above error.

I parametrize it like this: [ "actualstring", "another" ]. It works.

(0.12.8 and 0.12.20)

@pselle
Copy link
Contributor

pselle commented Jan 30, 2020

I've opened a documentation PR after further investigating this issue and am closing this one -- the original example is fixed, and further examples in this thread lead to investigation (thanks @jbardin!) leading to seeing that this is an expected behavior if using impure functions, of which uuid is one. The keys in for_each maps must be known, as they are used to key the resource instances. So even if it looks like a constant, if we're talking about something that is computed, that may be an unknown value, and resource instances cannot be keyed on unknown values.

As to @nik-shornikov -- the number of string keys is known, so you could use count here, but the values are not, without a -target to ensure those module outputs are completed, so unknowns would be expected here. If you want to use those values in your resource (the function names), you could use a map as your for_each argument with static string keys and the values being those function names. If you need further assistance on this particular case, I suggest the forums where more discussion is appropriate, as this is issue in particular is not a bug.

Thank you everyone for your examples, and for your patience.

@ghost
Copy link

ghost commented Mar 28, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Mar 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug config v0.12 Issues (primarily bugs) reported against v0.12 releases
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants