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.

Sign up for GitHub

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

lang/funcs: lookup() can work with maps of lists, maps and objects #22269

Merged
merged 3 commits into from
Aug 1, 2019
Merged

lang/funcs: lookup() can work with maps of lists, maps and objects #22269

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bd311de
lang/funcs: lookup() can work with maps of lists, maps and objects
mildwonkey Jul 31, 2019
f1d295e
lang/funcs: lookup() must validate the the default value and map element
mildwonkey Jul 31, 2019
8326537
pr feedback
mildwonkey Aug 1, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion lang/funcs/collection.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,12 @@ var LookupFunc = function.New(&function.Spec{
}
return cty.DynamicPseudoType, function.NewArgErrorf(0, "the given object has no attribute %q", key)
case ty.IsMapType():
if len(args) == 3 {
_, err = convert.Convert(args[2], ty.ElementType())
if err != nil {
return cty.NilType, function.NewArgErrorf(0, "the default value and map elements must have the same type")
Copy link
Contributor

Choose a reason for hiding this comment

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

The 0 here indicates that the problem is with the zeroth argument; perhaps it would be clearer to indicate 2 so that Terraform will indicate the default value as the subject of the error when it prints out diagnostics, as a subtle additional indication that it's probably the default value that ought to change to match the map, rather than the other way around.

}
}
return ty.ElementType(), nil
default:
return cty.NilType, function.NewArgErrorf(0, "lookup() requires a map as the first argument")
Expand Down Expand Up @@ -695,8 +701,10 @@ var LookupFunc = function.New(&function.Spec{
return cty.NumberVal(v.AsBigFloat()), nil
case ty.Equals(cty.Bool):
return cty.BoolVal(v.True()), nil
case ty.IsObjectType() || ty.IsListType() || ty.IsMapType():
return v, nil
default:
return cty.NilVal, errors.New("lookup() can only be used with maps of primitive types")
return cty.NilVal, fmt.Errorf("lookup() cannot be used with type %s", ty.FriendlyName())
mildwonkey marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down
54 changes: 54 additions & 0 deletions lang/funcs/collection_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1469,6 +1469,22 @@ func TestLookup(t *testing.T) {
cty.StringVal("baz"),
}),
})
mapOfMaps := cty.MapVal(map[string]cty.Value{
"foo": cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("bar"),
}),
"baz": cty.MapVal(map[string]cty.Value{
"b": cty.StringVal("bat"),
}),
})
mapOfObjects := cty.ObjectVal(map[string]cty.Value{
mildwonkey marked this conversation as resolved.
Show resolved Hide resolved
"foo": cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("bar"),
}),
"baz": cty.MapVal(map[string]cty.Value{
"b": cty.StringVal("bat"),
}),
})
mildwonkey marked this conversation as resolved.
Show resolved Hide resolved
mapWithUnknowns := cty.MapVal(map[string]cty.Value{
"foo": cty.StringVal("bar"),
"baz": cty.UnknownVal(cty.String),
Expand Down Expand Up @@ -1507,6 +1523,26 @@ func TestLookup(t *testing.T) {
cty.NumberIntVal(42),
false,
},
{
[]cty.Value{
mapOfMaps,
cty.StringVal("foo"),
},
cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("bar"),
}),
false,
},
{
[]cty.Value{
mapOfObjects,
cty.StringVal("foo"),
},
cty.MapVal(map[string]cty.Value{
"a": cty.StringVal("bar"),
}),
false,
},
{ // Invalid key
[]cty.Value{
simpleMap,
Expand Down Expand Up @@ -1541,6 +1577,15 @@ func TestLookup(t *testing.T) {
cty.StringVal("bar"),
false,
},
{ // Supplied default with valid (int) key
[]cty.Value{
simpleMap,
cty.StringVal("foobar"),
cty.NumberIntVal(-1),
},
cty.StringVal("-1"),
false,
},
{ // Supplied default with valid key
[]cty.Value{
mapWithObjects,
Expand All @@ -1559,6 +1604,15 @@ func TestLookup(t *testing.T) {
cty.StringVal(""),
false,
},
{ // Supplied default with type mismatch: expects a map return
[]cty.Value{
mapOfMaps,
cty.StringVal("foo"),
cty.StringVal(""),
},
cty.NilVal,
true,
},
{ // Supplied non-empty default with invalid key
[]cty.Value{
simpleMap,
Expand Down