Function keys() to return set instead of list

[aaronsteers]

Current Terraform Version

Terraform v0.12.21

Use-cases

The primary use case this affects is when using the keys from a map variable as input to a for_each loop. Although the resulting list is unique and would be correctly defined as a set. However, since the compiler cannot detect that the result of keys() is in fact a unique set, terraform throws an error that for_each can only accept map or set as input.

Attempted Solutions

1. The workaround is to wrap keys() in toset(): for_each = toset(keys(var.my_map))

Proposal

Proposed solution is that keys() would always return a set object instead of list.

References

n/a

[jbardin]

Hi @aaronsteers

Unfortunately we can't change the return type of keys without possibly breaking existing configurations, so that requires a fairly high bar to initiate the change.

Can you help me better understand the use case here? The for_each feature is intended to work primarily with maps, and you have a map already, so extracting the keys is not needed. Using the map directly and only accessing each.key would have the exact same outcome.

[aaronsteers]

@jbardin - I completely understand this could potentially be a breaking change. For my info, are there many cases where set is not accepted in place of list? I understand why implicit conversion cannot go from list->set but I'm not well-versed on how Terraform handles implicit conversion the other way around, i.e. set->list.

Another specific use case is where, in a locals block, we'll take an input variable like secrets_map which maps secrets to a key vault location of some sort. Because there are multiple phases of transformation and handoff, the first step we take is to extract a new variable secrets_names which is a list (a set) of those to-be-declared secrets names. For readability if for no other reason, we don't want to be passing around extra secrets info to module declarations (even uri's to the secrets) if not necessary, so it has been our design practice to pass the more explicit secrets_names list to for_each loops that just need the name of the secret instead of the full secrets_map. Wrapping everything in toset() certainly resolves the compile issue, but again, it seems like this could be handled in the function definition itself, avoiding adding unnecessary decorators into the code.

Can you speak a little more to how much would break - or I guess, what are the rules around implicit set -> list conversions? Understanding this would be a very small improvement in functionality, I am trying to better weigh that against what would break. Apologies for borrowing OOP terms that probably don't apply here, but if set is something like a subclass of list, my hope is that most code would continue to run as-is.

[jbardin]

Thanks for the feedback @aaronsteers!

You're right that a set(string) can be converted to a list(string) is all cases. Terraform is using structural typing rather than OOP-style class typing. What this means is that while implicit conversions can be done, it is still necessary for the conversion to happen, which means there may currently be codepaths not expecting to require converting the type.

I think the impact would be minimal, we just want to make sure there is a good reason to make any impact at all. Looking at the keys function, it looks like the type signature itself can be simplified, and we can possibly change that to be a static set(string) for simpler evaluation.

[aaronsteers]

Awesome - thanks for the quick reply. For my part, I agree the relative priority of the change is minor. And to minimize risk, this might make the most sense as part an upcoming major release, where some number of potentially-breaking changes are already expected rather than a minor release.

Either way, I still think as a minor enhancement, it seems like a good overall investment and I am glad to have it logged and potentially able to get incorporated into the roadmap (whatever that might look like).

[jbardin]

Hi @aaronsteers

I've had a little time to look into this and I see a couple problems.
Because keys is guaranteed to return keys in lexicographical order, there are examples of configuration that (for better or worse) use the form keys(map)[n] to extract values and changing the type to a set would break that and require an intermediate for-expression.

The other point is that the keys function does not always return a list. If the argument is an object, a tuple is returned instead indicating the number of elements which is useful for other early interpolations. Because configuration often intermixes maps and objects for various reasons, having the keys function break within for_each when the inferred argument type becomes an object would be confusing to users. Since the set of valid operations is not equal for tuple and set, it impact all evaluations derived from the keys function.

Because not passing the full map value to for_each is already going to be the exception, I think we can live with the toset workaround to maintain greater consistency overall and we can close this out for now.

Thanks again for the request!

[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.