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

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

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.

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

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.

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.

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.

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.