for_each and splat

[omeid]

Using for_each with a map in resources results in a weird behaviour:

Input:

provider "aws" {
  version = "~> 2"
}

resource "aws_eip" "this" {
  for_each = {
    "a": 1,
    "b": 2,
  }
}


output "eips" {
  description = "Subnet ID"
  value = aws_eip.this[*].id
}

Output:

Error: Unsupported attribute

  on main.tf line 17, in output "eips":
  17:   value = aws_eip.this[*].id

This object does not have an attribute named "id".

Using values(aws_eip.this).*.id works with aws_eip, however, with aws_subnet, I get the following:

Output:

Error: Error in function call

  on main.tf line 17, in output "ids":
  17:   value = values(aws_subnet.this).*.id
    |----------------
    | aws_subnet.this is tuple with 2 elements

Call to function "values" failed: values() requires a map as the first
argument.

Happens on terraform 0.12.6 as well as master.

[teamterraform]

Hi @omeid!

As you've figured out here, a resource with for_each set appears in expressions as a map rather than list, so the splat operators are not directly applicable to it.

You didn't show the source code for aws_subnet.this, so we're not sure what's going on with that. Does that resource also have for_each set? Our first thought was that maybe you were hitting the bug from #22407, but that's currently fixed in master ready for inclusing in the 0.12.7 release, so perhaps this is something a bit different.

[omeid]

Hello @teamterraform

As you've figured out here, a resource with for_each set appears in expressions as a map rather than list, so the splat operators are not directly applicable to it.

This is somewhat unexpected, they should work.

You didn't show the source code for aws_subnet.this, so we're not sure what's going on with that. Does that resource also have for_each set?

Yes. It is pretty much a standard aws_subnet with for_each.

Our first thought was that maybe you were hitting the bug from #22407, but that's currently fixed in master ready.

I believe it is not directly related and the issue happens on the master still.

[omeid]

update, I was not able to reproduce the same problem using a minimal version, and the problem went away once I recreated the subnets. I suppose the problem may be related to moving from count to for_each.

[omeid]

While I am unable to create a small code to reproduce this, it is happening on a large deployment randomly. Run terraform apply multiple time makes it go away. So I suppose this is some kind of race condition.

[omeid]

Still unable to replicate this, what is even bizarre is that changing unrelated resources makes the error go away.

[teamterraform]

Randomly sounds a lot like #22407 -- this will go out in the next release of Terraform (which is soon). Does the randomness happen on master? Or we leave this open until 0.12.7 is out, and then validate that it's not happening anymore.

[omeid]

Okay, I figured out, the error happens when create_before_destroy is set to true. This causes the state collection holding the resources in question, for example aws_subnet.this in this case, to include mixed index_keys, string for the actual resources created by the map, and numerical for deposed items.

[mattolenik]

That is NOT the only way this happens! I'm getting this when using a plain simple module that has nothing but subnets. No transitioning from something else, no create_before_destroy. This seems like very obvious behavior and I can't imagine how for_each was envisioned as being useful without it.

[brianpham]

I am hitting this same issue when I am trying to get output

locals {
  amis = {
    default = "ami-075deac7b5ecf3241",
    latest = "ami-075deac7b5ecf9901"
  }
}

resource "aws_ami_copy" "this" {
  for_each = local.amis

  name              = each.key
  description       = "${each.key} ami copied from ${each.value}"
  source_ami_id     = each.value
  source_ami_region = "us-east-1"
}


output "ami_id" {
  value = zipmap(aws_ami_copy.this[*].name, aws_ami_copy.this[*].id)
}

The error is 

Error: Unsupported attribute

  on ../../modules/aws/ami_copy/main.tf line 24, in output "ami_id":
  24:   value = zipmap(aws_ami_copy.this[*].name, aws_ami_copy.this[*].id)

This object does not have an attribute named "name".


Error: Unsupported attribute

  on ../../modules/aws/ami_copy/main.tf line 24, in output "ami_id":
  24:   value = zipmap(aws_ami_copy.this[*].name, aws_ami_copy.this[*].id)

This object does not have an attribute named "id".

[teamterraform]

Hi all!

Just wanted to clarify that it is by design that the splat operators don't work with maps. Splat operators are for lists only, as described in the documentation. Note in particular the paragraph describing their other "useful effect":

Splat expressions also have another useful effect: if they are applied to a value that is not a list or tuple then the value is automatically wrapped in a single-element list before processing. That is, var.single_object[*].id is equivalent to [var.single_object][*].id, or effectively [var.single_object.id]. This behavior is not interesting in most cases, but it is particularly useful when referring to resources that may or may not have count set, and thus may or may not produce a tuple value.

So, the reason applying [*] to a map doesn't work is that it causes the map to be wrapped in a single-element list, per the rule above. This is the 0.12 equivalent of the Terraform 0.11 behavior where you could apply .* to a resource that doesn't have count set at all and get a single-element list. It's not possible to change that behavior because existing configurations are relying on it.

Aside from that special behavior (which isn't useful when using for_each), a splat expression is just a shorthand for a for expression that applies only to lists. When using for_each we must use a full for expression:

  value = { for k, v in aws_eip.this : k => v.id }

or, if you want the result to be a list (in lexical order by the keys, discarding the keys themselves):

  value = [for v in aws_eip.this : v.id]

or, if you want the result to be a set of strings, reflecting that the ids are not ordered and more convenient to use with for_each elsewhere in the configuration:

  value = toset([for v in aws_eip.this : v.id])

As mentioned above, we cannot and do not intend to change this behavior to make splat expressions work with maps because that is incompatible with existing configurations. Based on the comments so far it's not clear to us whether everyone is just reporting that splat expressions don't work with for_each (which is by design, not a bug) or if some of you are reporting a bug. For that reason, we're going to leave this open for now in case someone wants to add a bug report about Terraform behaving inconsistently to what we've described above in this comment.

[joshuaspence]

Thanks for the explanation. What is the expected way to access a list of property from a resource created using for_each? I'm currently using values(resource)[*].attribute and it works just fine but it could certainly be made more succinct with a language construct to simplify this use case. Here's a snippet of my Terraform configuration.

resource "aws_route53_record" "acm_validation" {
  zone_id  = aws_route53_zone.main[each.key].zone_id
  name     = each.value.resource_record_name
  type     = each.value.resource_record_type
  ttl      = 60
  records  = [each.value.resource_record_value]
  for_each = { for certificate in aws_acm_certificate.main.domain_validation_opt
}

resource "aws_acm_certificate_validation" "main" {
  certificate_arn         = aws_acm_certificate.main.arn
  validation_record_fqdns = values(aws_route53_record.acm_validation)[*].fqdn
}

Edit: I suppose I could also do this: [for record in aws_route53_record.acm_validation : record.fqdn].

[Sanyambansal76]

@joshuaspence thanks values(aws_route53_record.acm_validation)[*].fqdn it worked for me

[dmrzzz]

@teamterraform thanks for the explanation!

An explicit hint about maps in the documentation would be a big help here, to counteract the strong visual intuition that aws_instance.example[*].id really looks like it could apply to for_each as well as count resources. Perhaps something like:

---

Splat expressions are not directly applicable to maps (including for_each resource blocks). You can apply a splat expression to the values of the map

values(aws_instance.example)[*].id

or use an equivalent for expression

[for k, v in aws_instance.example : v.id]

---

Incidentally, is [for v in var.map : ...] (discarding keys) an official acceptable usage of for with maps? Your answer seems to imply this, but currently https://www.terraform.io/docs/configuration/expressions.html#for-expressions only mentions [for k, v in var.map : ...]

If yes, then the equivalent for expression above could instead be

[for o in aws_instance.example : o.id]

which is shorter, though also potentially even more counterintuitive since that's literally the same equivalent for expression you would get from aws_instance.example[*].id if the resource block used count rather than for_each.

[pselle]

Thank you for the suggestions @dmrzzz, I've looked in the docs and found we have docs regarding for_each and splat in https://www.terraform.io/docs/configuration/expressions.html#splat-expressions, but I've added one to https://www.terraform.io/docs/configuration/expressions.html#references-to-resource-attributes as well.

As to your question about for, we do have an active reference to that in the docs that uses values: https://www.terraform.io/docs/configuration/expressions.html#for-value-in-aws_instance-example-value-id-.

I'd also note that this is valid in the same way that if you wanted to use the index values of a list, you could use for i, v in list : .... That is, if only one temporary variable is used, then that variable will apply to the values in a given object, rather than its keys (be they list indices or map keys), and if two are available (k, v ...) then key is available.

I'm closing this issue as we've clarified that the splat operator with for_each resources are incompatible, because for_each resources are a map, not a list. We do appreciate the feedback on hoping for a way to make this more concise in the future.

[joshuaspence]

I understand that the documentation is correct, but is there any reason to not change the language so that the splat operator can be used with maps or introducing an equivalent syntax for maps?

[dmrzzz]

@pselle thanks for the doc updates, and sorry I didn't see the very useful "Splat expressions are for lists only..." that you had already added in 67e314d prior to my earlier comment; my browser must still have had the old page cached.

Regarding for: you just blew my mind a second time! I love the symmetry, and both possibilities now make sense to me based on your comments here; it would be great to see them at least briefly mentioned in the authoritative section on how to write for expressions (https://github.com/hashicorp/terraform/blob/master/website/docs/configuration/expressions.html.md#for-expressions). Here's one attempt based on your text above:

---

If only one temporary variable is provided for an object or map source (for v in var.map), it will be set to the value of each element (in lexical order by keys, but discarding the keys themselves). If two temporary variables are provided for a list source (for i, v in var.list), then i will be set to the list index and v to the value.

[pselle]

@joshuaspence We can consider this for future language design, around the issue of "I want all map'd instances", but personal opinion is splat is not a great fit for this without guards, as it is ambiguous as to if you will get the values or keys of a map (since both may be meaningful data, whereas indicies less so). "Get all the instances" for resources represented by a map returns two values, the keys and the values, and so using keys() or values() to convert to the list before using splat is the most explicit way to be clear at present -- I'm not against us designing another means, but that's a little more "why".

@dmrzzz Thanks for the suggestion! I'll ponder this a little more, because I want to make sure we're providing useful data in the docs/not handing people lines that might confuse them ("why do I need the index?"). I might also float this up to our documentation lead to see if he has some more suggestions on clarifying for expressions; these are a tough concept!

[omeid]

@pselle While it is true that both the key and value of a map has meaning, generally the key is a meaningful-reference (opposite to merely sequential reference in case of lists) and the value is well, the value. So treating var.map.* same as values(var.map).* would be generally useful.

Of course, if someone wants the keys, they can always still do that with keys(var.map).

[apparentlymart]

As we noted in the longer comment above, the splat operator in its current form cannot be changed to work directly with maps because the splat operator already has a defined meaning for working with non-list/non-set values: it wraps the value in a single-element list, or produces an empty list if the value is null. Existing configurations are relying on this behavior because in Terraform 0.11 it was somewhat common to use the .*. syntax with resources that don't have count set, producing a single-element list.

For the foreseeable future, keys(map) and values(map)[*] are the most succinct forms we intend to support, making it explicit whether it's the keys or the values that are being used as the input. As others have noted, you can also express these as a for expression, which might be clearer to future readers at the expense of being a little longer:

[ for v in aws_instance.example : v.id ]

Since it's likely that when for_each is being used the keys are somehow meaningful identifiers themselves, another reasonable thing to do is to produce a map result like this:

{ for k, v in aws_instance.example : k => v.id }

Since all three of these results would be reasonable things to compute in different situations, we think it's important that it's clear to a reader which of these forms is the result of a particular expression. The Terraform language generally prioritizes being explicit over being terse.

[omeid]

Thanks for the explanation @apparentlymart. I think it is worth documenting the backwards compatibility issue to help users understand the rationale for the somewhat surprising behaviour.

[ghost]

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.