Does it make sense to suggest `toset()` on lists in `for_each`?

Terraform
1

We’ve all gotten this error at some point:

The “for_each” value depends on resource attributes that cannot be determined until apply

There are countless Google results of people frustrated with this. I think that part of the blame lies with the documentation and its suggestion to use toset() as the way to use lists with for_each.

Why use a set?

The only thing terraform needs is a pre-determined key in for_each during the plan. It turns out that the list data type always has a unique key: the index. For every scenario where the list length is fixed, for_each should work just fine if you convert the list to a map. You just have to avoid falling into the toset() trap.

This won’t work:

locals {
  ssh_sg_ids = [
    module.private.alb_security_group_id,
    module.public.alb_security_group_id,
  ]
}

resource "aws_security_group_rule" "allow_ssh" {
  for_each                 = toset(local.ssh_sg_ids)
  source_security_group_id = each.value
  ...
}

But this will:

resource "aws_security_group_rule" "allow_ssh" {
  for_each                 = {for i, val in local.ssh_sg_ids: i => val}
  source_security_group_id = each.value
  ...
}

I can’t think of a situation where toset() would be better than making a map from the list. So why suggest toset() at all?

Furthermore, the error message is very misleading by talking about

The “for_each” value

The problem is decidedly the keys being used in for_each, not the values

2

If you are just looping through a list then count is a much better option than for_each. However the big advantage of for_each (which is only true if using a map rather than a list) is that changes to the variable only affects the values which are adjusted. For a list removing an entry from the middle will cause all subsequent resources to be destroyed & recreated (as the index has changed for each of those). Your suggestion of using the index position with for_each has the same limitation. If instead you use a key that isn’t based in the position you can add or remove entries without having to recreate anything that hasn’t actually changed.

1 Like
3

Your suggestion of using the index position with for_each has the same limitation.

Ah, you’re right, that would be a scenario where toset() would be superior to my suggestion

4

With your original example I’d not use a list at all. Instead I’d have something like:

locals {
  ssh_sg_ids = {
    private = module.private.alb_security_group_id
    public  = module.public.alb_security_group_id
  }
}

And then use for_each with that map.

5

Yes, the original example can be modified easily by explicitly declaring the keys of a map. There are plenty of scenarios though where the list length is known at apply-time but the module writer still doesn’t know the list length. If you had a var.ssh_sg_ids instead of a local.ssh_sg_ids, presumably that would have a type of list(string), like is standard in most modules.

6

I find trying to use map(string) or map(object({...})) much more than lists where possible, mostly because it suggests the use of for_each instead of count.

7

In any case, you’ve convinced me that using indexes as keys does remove one of the main benefits of for_each over count, which is dealing with changes in the source list.