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

Better user experience when using for_each with lists #29904

Closed
acdha opened this issue Nov 8, 2021 · 6 comments
Closed

Better user experience when using for_each with lists #29904

acdha opened this issue Nov 8, 2021 · 6 comments
Labels
enhancement new new issue not yet triaged waiting-response An issue/pull request is waiting for a response from the community

Comments

@acdha
Copy link

acdha commented Nov 8, 2021

Terraform somewhat recently added stricter validation to resource's for_each arguments. This is generally good for avoiding bugs but I've found one area where the experience could be more ergonomic:

In many cases, the data I'm using for the for_each argument is provided in a list. This causes an error, of course, so the easiest option is to wrap it in toset():

resource "aws_iam_user_group_membership" "deployment_users" {
  for_each = toset([
    aws_iam_user.user1.name,
    …
  ])

This works in many cases, especially on existing projects, but use of the function has the subtle effect of breaking the dependency analysis. This means that if you use any resource which does not already exist you'll get an error like this:

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.

This is somewhat slow going — 30 seconds to get the lock + state from Terraform cloud, a couple of minutes to refresh resources, repeat to add -target …, etc. — and it feels like a flaw that Terraform can't tell the length of a static input. This is notably different from the experience with a map literal, which always works:

resource "aws_iam_user_group_membership" "deployment_users" {
  for_each = {
    user1 = aws_iam_user.user1.name,
    …
  }

Two ways to improve the experience come to mind:

  • Expose the dependency graph through functions like toset() so it would work like the previous experience with lists and current experience with maps.
  • Add a literal syntax for creating sets similar to what maps and lists have which would allow compile-time validation: for example, the Python-style syntax set(aws_iam_user.user1, aws_iam_user.user2) would have a known size and would also be able to return an error if you referenced the same value twice.
@acdha acdha added enhancement new new issue not yet triaged labels Nov 8, 2021
@jbardin
Copy link
Member

jbardin commented Nov 9, 2021

Hi @acdha, thanks for filing the issue. The premise here seems to be founded on some misunderstanding of the error condition, but perhaps we can try and find out how we could better communicate this with users.

If I understand correctly what you mean by "Expose the dependency graph", there is no missing dependency when you use the toset function, everything is ordered in the exact same manner as it is with the map literal example. This also means that a set literal (which functionally wouldn't work any differently than toset([])) would produce the same result. The proposed set example also does not allow the number of set elements to be known (which means the length also cannot be used for count), because there is no way to determine if all unknown aws_iam_user.userX.name values will be unique, otherwise changing the number of elements in the set.

All the error is trying to express, is that the keys for the for_each expression are not known, which is a fundamental requirement for planning a resource using for_each expansion. The map example works, not because of any difference in dependencies, but because you put a literal user1 key in the expression, allowing the corresponding resource instance to be planned with the user1 key name.

@jbardin jbardin added the waiting-response An issue/pull request is waiting for a response from the community label Nov 9, 2021
@acdha
Copy link
Author

acdha commented Nov 9, 2021

The proposed set example also does not allow the number of set elements to be known (which means the length also cannot be used for count), because there is no way to determine if all unknown aws_iam_user.userX.name values will be unique, otherwise changing the number of elements in the set.

This is what I was referring to with the dependency graph: in most of the cases I've encountered, all of the provided values are known when Terraform starts (i.e. using only variables or resource properties set from variables) so it seems like Terraform should be able count the number of unique values, just as it does with the map syntax, so the resource with the for_each could use the same values displayed in the plan.

I understand that there are cases where something like a data provider or a dependency on a value provided by a resource would not allow that optimization.

@jbardin
Copy link
Member

jbardin commented Nov 9, 2021

Thanks @acdha, in the original example above, if aws_iam_user.user1.name were known ahead of time there would be no error. Since that value is not known, there is no way to plan what the instance key will be for the resource. Note that even in the case where there is only a single value, which means that the number of values can be logically determined, terraform still cannot plan based on that unknown value since it must have a string key to address the resource, and be assured that the value will not be null, which is an invalid key altogether.

Terraform also cannot count the number of unique values, because the values are unknown, and there is no way to determine their uniqueness. A value of cty.SetVal(cty.UnknownVal(string), cty.UnknownVal(string), cty.UnknownVal(string)) may contain anywhere from 1 to 3 values, hence the entire set value must be treated as unknown.

@mgzenitech
Copy link

@jbardin so maybe something like Terraform introducing of auto targeting with multiple apply stages would be a workaround to this issue? Because leaving this problem to the users doing partial applies is not something we can script in the end. For example in the piplines which deploy infrastructure we would have to know beforehand and script all those targets.

@jbardin
Copy link
Member

jbardin commented Nov 12, 2021

We have many requests over the years for something like automatic targeting (or progressive apply, partial apply, "stacks", there have been many names), but no holistic workflow has yet to be devised that covers the desired use cases. You can start here for some background: #4149. This is definitely still something we're very interested in, but I think the final solution may take a bit different form.

Even with that feature in mind however, we would still recommend restructuring the configuration to avoid the error altogether, rather than working around it with multiple application stages. Perhaps the suggestion in the error output is a little misleading, since we don't necessarily recommend using -target, it's just the only way terraform could bypass the problem without changes to the configuration, and it should be used only as a last resort when restructuring is not possible.

Since we have other features in mind to cover similar use cases, I'm going to close this one out. Thanks for the feedback!

@jbardin jbardin closed this as completed Nov 12, 2021
@github-actions
Copy link
Contributor

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement new new issue not yet triaged waiting-response An issue/pull request is waiting for a response from the community
Projects
None yet
Development

No branches or pull requests

3 participants