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

Inteprolate list of maps #10787

Merged
merged 4 commits into from
Dec 16, 2016
Merged

Inteprolate list of maps #10787

merged 4 commits into from
Dec 16, 2016

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Dec 16, 2016

Fix flatmap.Expand to work correctly with nested maps. This can then be used to unpack all nested values for the Interpolator, rather than parsing the keys and building the data structures in the Interpolator itself.

Fixes #9693

flatmap.Expand was adding `%` as a value in nested maps. Removing that
allows us properly expand objects other than a simple map.
Interpolation of a map from a list of maps was not working. Add a
provider example test to cover this.
Now that flatmap.Expand will properly expand nested maps, we can use
that to extract any lists and maps when interpolating.
Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, one change request: flatmap has pretty extensive unit tests if I recall, can you add a test for your simple change? Then its good!

@jbardin
Copy link
Member Author

jbardin commented Dec 16, 2016

Oh yes, spent too much time trying to make TF tests, forgot about the easy ones.

Pushed

@jbardin jbardin merged commit 4f625af into master Dec 16, 2016
@jbardin jbardin deleted the jbardin/inteprolate-list-of-maps branch December 16, 2016 21:41
2rs2ts added a commit to 2rs2ts/terraform that referenced this pull request Dec 23, 2016
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.

Signed-off-by: Jesse Szwedko <jesse.szwedko@getbraintree.com>
2rs2ts added a commit to 2rs2ts/terraform that referenced this pull request Dec 23, 2016
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 added a commit to 2rs2ts/terraform that referenced this pull request Dec 23, 2016
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.
@ghost
Copy link

ghost commented Apr 18, 2020

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 ghost locked and limited conversation to collaborators Apr 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't index from schema.Set
2 participants