terraform icon indicating copy to clipboard operation
terraform copied to clipboard

Issues with combining `try` and `for_each`

Open ibacalu opened this issue 3 years ago • 4 comments

Terraform Version

Terraform v1.2.*
Terraform v1.3.0

Terraform Configuration Files

Really silly setup with the sole purpose of showing the issue

resource "aws_kms_key" "this" {}

locals {
  echo = {
    AWS_KMS_KEY = aws_kms_key.this.id
  }
}

resource "null_resource" "echo" {
  for_each = {
    for k, v in try(local.echo, {}) :
    k => v
  }
  provisioner "local-exec" {
    command = "/bin/echo [${each.key}] ${each.value}"
  }
}

Debug Output

Debug not really needed

Expected Behavior

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # aws_kms_key.this will be created
  + resource "aws_kms_key" "this" {
      + arn                                = (known after apply)
      + bypass_policy_lockout_safety_check = false
      + customer_master_key_spec           = "SYMMETRIC_DEFAULT"
      + description                        = (known after apply)
      + enable_key_rotation                = false
      + id                                 = (known after apply)
      + is_enabled                         = true
      + key_id                             = (known after apply)
      + key_usage                          = "ENCRYPT_DECRYPT"
      + multi_region                       = (known after apply)
      + policy                             = (known after apply)
      + tags_all                           = (known after apply)
    }

  # null_resource.echo["AWS_KMS_KEY"] will be created
  + resource "null_resource" "echo" {
      + id = (known after apply)
    }

Plan: 2 to add, 0 to change, 0 to destroy.

Actual Behavior

╷
│ Error: Invalid for_each argument
│ 
│   on main.tf line 10, in resource "null_resource" "echo":
│   10:   for_each = {
│   11:     for k, v in try(local.echo, {}) :
│   12:     k => v
│   13:   }
│     ├────────────────
│     │ local.echo is object with 1 attribute "AWS_KMS_KEY"
│ 
│ The "for_each" map includes keys 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 define the map keys statically in your configuration and place apply-time results only in the map values.
│ 
│ 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.

Steps to Reproduce

  1. terraform init
  2. terraform plan

Additional Context

If I remove the try from local.echo definition, or replace it with coalesce it will work.

P.S.

  • Updated configuration to make it even simpler
  • local.echo is detected as object, however it behaves the same even if I set it manually to map.
  • You can see even in the error it correctly identifies that there's exactly 1 attribute in the local.echo object.

References

Might be related to:

  • #31295

ibacalu avatar Sep 26 '22 09:09 ibacalu

Thanks for the minimal example - very clear to see what you're talking about. The behaviour of try is similar to can here in the sense described in https://github.com/hashicorp/terraform/issues/31646, with similar consequences. I'm leaving this open in case there is an improvement to be made here in making try a little more intelligent with respect to its contents - perhaps it can try a little harder.

kmoe avatar Sep 27 '22 14:09 kmoe

FYI - most of the time you can swap a try() for a lookup() when a computed value is involved to avoid this error. try() produces a cleaner syntax and allow for multiple fallback values, but you can achieve the same with nested lookup()s to achieve the same functionality (albeit, a bit more verbose)

bryantbiggs avatar Sep 28 '22 20:09 bryantbiggs

FYI - most of the time you can swap a try() for a lookup() when a computed value is involved to avoid this error. try() produces a cleaner syntax and allow for multiple fallback values, but you can achieve the same with nested lookup()s to achieve the same functionality (albeit, a bit more verbose)

Unfortunately lookup() fails if the parent key doesn't exist and my real use-case would have to account for non-existing keys. I have to add though that the latest optional([type], [default]) is allowing me to use coalesce() in most scenarios.

ibacalu avatar Sep 28 '22 21:09 ibacalu

Hello, I'm changing this to an enhancement request, since the language is working as designed here. While the language may someday be able to resolve simple examples as shown here, the general case is much harder. For example:

resource "terraform_data" "this" {
  input = {
    key = "value"
  }
}

locals {
  attempt = {
    key1 = terraform_data.this.output["will_not_exist"]
    key2 = "ok"
  }
  fallback = {
    key1 = "ok"
    key2 = "fail"
  }
}

resource "terraform_data" "echo" {
  for_each = { for k, v in try(local.attempt, local.fallback) : k => v }
  input = each.value
}

Even though local.attempt has known keys, if the try function returns anything other than a completely dynamic value, The key2 value will change once the map is known to not contain "will_not_exist" causing a failure during apply. Now we could check that all keys match and also determine that key2 should be unknown as well, but the recursive analysis gets much more difficult and will take some more research.

jbardin avatar Aug 14 '23 18:08 jbardin