Fix string representation of sets during interpolation #10920
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The change in hashicorp#10787 used flatmap.Expand to fix interpolation of nested maps, but it broke interpolation of sets such that their elements were not represented. For example, the expected string representation of a splatted aws_network_interface.whatever.*.private_ips should be: ``` [{Variable (TypeList): [{Variable (TypeString): 10.41.17.25}]} {Variable (TypeList): [{Variable (TypeString): 10.41.22.236}]}] ``` But instead it became: ``` [{Variable (TypeList): [{Variable (TypeString): }]} {Variable (TypeList): [{Variable (TypeString): }]}] ``` This is because the expandArray function of expand.go treated arrays to exclusively be lists, e.g. not sets. The old code used to match for numeric keys, so it would work for sets, whereas expandArray just assumed keys started at 0 and ascended incrementally. Remember that sets' keys are numeric, but since they are hashes, they can be any integer. The result of assuming that the keys start at 0 led to the recursive call to flatmap.Expand not matching any keys of the set, and returning nil, which is why the above example has nothing where the IP addresses used to be. So we bring back that matching behavior, but we move it to expandArray instead. We've modified it to not reconstruct the data structures like it used to when it was in the Interpolator, and to use the standard int sorter rather than implementing a custom sorter since a custom one is no longer necessary thanks to the use of flatmap.Expand. Fixes hashicorp#10908, and restores the viability of the workaround I posted in hashicorp#8696. Big thanks to @jszwedko for helping me with this fix. I was able to diagnose the problem along, but couldn't fix it without his help.
2rs2ts
force-pushed
the
feature/fix-interpolated-string-repr-of-sets
branch
from
b34f9d1 to
497010c
Compare
|
Cool, I dig it. |
jbardin
added a commit
that referenced
this pull request
Jan 9, 2017
fix flatmap.Expand to work with the schema.Set representation
|
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. |
ghost
locked and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The change in #10787 used
flatmap.Expandto fix interpolation of nested maps, but it broke interpolation of sets such that their elements were not represented. For example, the expected string representation of a splattedaws_network_interface.whatever.*.private_ipsshould be:But instead it became:
This is because the
expandArrayfunction of expand.go treated arrays to exclusively be lists, e.g. not sets. The old code used to match for numeric keys, so it would work for sets, whereasexpandArrayjust assumed keys started at 0 and ascended incrementally. Remember that sets' keys are numeric, but since they are hashes, they can be any integer. The result of assuming that the keys start at 0 led to the recursive call toflatmap.Expandnot matching any keys of the set, and returningnil, which is why the above example has nothing where the IP addresses used to be.So we bring back that matching behavior, but we move it to
expandArrayinstead. We've modified it to not reconstruct the data structures like it used to when it was in theInterpolator, and to use the standard int sorter rather than implementing a custom sorter since a custom one is no longer necessary thanks to the use offlatmap.Expand.Fixes #10908, and restores the viability of the workaround I posted in #8696.
Big thanks to @jszwedko for helping me with this fix. I was able to diagnose the problem alone, but couldn't fix it without his help.