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

flatten must return unknown with dynamic vals #106

Merged
merged 1 commit into from May 6, 2021

Conversation

jbardin
Copy link
Contributor

@jbardin jbardin commented May 6, 2021

If any type with the flatten argument is unknown, then the result cannot be known.

If any type is unknown, then the result cannot be known.
@apparentlymart
Copy link
Collaborator

Ahh yes, this makes sense to me: the final length of the result depends on the lengths of all of the lists/tuples inside, and if we encounter a cty.DynamicVal then we can't know what length it will contribute to the result, and an unknown list is the closest thing we have to a list with an unknown length. Added to this that we can't determine the number of elements in a tuple result means that we can't predict the type either.

Thanks!

@apparentlymart apparentlymart merged commit e789782 into zclconf:main May 6, 2021
@apparentlymart
Copy link
Collaborator

Thinking about this a little more, I think this rule might be a little over-conservative with the following input:

cty.ListVal([]cty.Value{
  cty.ObjectVal(map[string]cty.Value{
    "whatever": cty.DynamicVal,
  }),
})

In that case we would ideally return an equivalent result rather than reducing the entire thing to cty.DynamicVal, because the only cty.DynamicVal in the input is inside something that we can prove would never be "flattened".

I'm not super concerned about this because the result is still correct, just less accurate than it could be. Might be worth trying to make it a bit more accurate for Terraform's benefit though, because I think this could arise when flattening lists of objects that represent resource instances of types that have attributes of cty.DynamicPseudoType.

@jbardin
Copy link
Contributor Author

jbardin commented May 7, 2021

Yes, I thought of the same thing last night. While it's technically still correct, I would like to fix it since there may be terraform users who have known values become unknown unexpectedly (and with the extensive combinations of flatten, compact, coalesce, etc. that are in some configs, I would not be surprised to see that immediately crop up)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants