From bd311debbb61e9c2bb18e0398d0454ef2ac2d428 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Wed, 31 Jul 2019 10:16:24 -0400 Subject: [PATCH 1/3] lang/funcs: lookup() can work with maps of lists, maps and objects lookup() can already handle aribtrary objects of (whatever) and should handle maps of (whatever) similarly. --- lang/funcs/collection.go | 4 +++- lang/funcs/collection_test.go | 36 +++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/lang/funcs/collection.go b/lang/funcs/collection.go index bcccc1fd2994..ed0362d03074 100644 --- a/lang/funcs/collection.go +++ b/lang/funcs/collection.go @@ -695,8 +695,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()) } } } diff --git a/lang/funcs/collection_test.go b/lang/funcs/collection_test.go index 71a805e7bf33..367d071be33e 100644 --- a/lang/funcs/collection_test.go +++ b/lang/funcs/collection_test.go @@ -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{ + "foo": cty.MapVal(map[string]cty.Value{ + "a": cty.StringVal("bar"), + }), + "baz": cty.MapVal(map[string]cty.Value{ + "b": cty.StringVal("bat"), + }), + }) mapWithUnknowns := cty.MapVal(map[string]cty.Value{ "foo": cty.StringVal("bar"), "baz": cty.UnknownVal(cty.String), @@ -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, From f1d295e942c97c2409927dd4f5fb33475c75a576 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Wed, 31 Jul 2019 12:21:45 -0400 Subject: [PATCH 2/3] lang/funcs: lookup() must validate the the default value and map element types match or can be converted. --- lang/funcs/collection.go | 6 ++++++ lang/funcs/collection_test.go | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/lang/funcs/collection.go b/lang/funcs/collection.go index ed0362d03074..f1d39fc401a4 100644 --- a/lang/funcs/collection.go +++ b/lang/funcs/collection.go @@ -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") + } + } return ty.ElementType(), nil default: return cty.NilType, function.NewArgErrorf(0, "lookup() requires a map as the first argument") diff --git a/lang/funcs/collection_test.go b/lang/funcs/collection_test.go index 367d071be33e..fe1c4d873931 100644 --- a/lang/funcs/collection_test.go +++ b/lang/funcs/collection_test.go @@ -1577,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, @@ -1595,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, From 8326537acdc45af1b66591a435773288712d245c Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Thu, 1 Aug 2019 09:46:00 -0400 Subject: [PATCH 3/3] pr feedback --- lang/funcs/collection.go | 18 ++---------------- lang/funcs/collection_test.go | 16 ++++++++++++++-- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/lang/funcs/collection.go b/lang/funcs/collection.go index f1d39fc401a4..e6898457b9dd 100644 --- a/lang/funcs/collection.go +++ b/lang/funcs/collection.go @@ -663,7 +663,7 @@ var LookupFunc = function.New(&function.Spec{ 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") + return cty.NilType, function.NewArgErrorf(2, "the default value must have the same type as the map elements") } } return ty.ElementType(), nil @@ -692,21 +692,7 @@ var LookupFunc = function.New(&function.Spec{ return mapVar.GetAttr(lookupKey), nil } } else if mapVar.HasIndex(cty.StringVal(lookupKey)) == cty.True { - v := mapVar.Index(cty.StringVal(lookupKey)) - if ty := v.Type(); !ty.Equals(cty.NilType) { - switch { - case ty.Equals(cty.String): - return cty.StringVal(v.AsString()), nil - case ty.Equals(cty.Number): - 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, fmt.Errorf("lookup() cannot be used with type %s", ty.FriendlyName()) - } - } + return mapVar.Index(cty.StringVal(lookupKey)), nil } if defaultValueSet { diff --git a/lang/funcs/collection_test.go b/lang/funcs/collection_test.go index fe1c4d873931..cec18c04962f 100644 --- a/lang/funcs/collection_test.go +++ b/lang/funcs/collection_test.go @@ -1477,7 +1477,11 @@ func TestLookup(t *testing.T) { "b": cty.StringVal("bat"), }), }) - mapOfObjects := cty.ObjectVal(map[string]cty.Value{ + mapOfTuples := cty.MapVal(map[string]cty.Value{ + "foo": cty.TupleVal([]cty.Value{cty.StringVal("bar")}), + "baz": cty.TupleVal([]cty.Value{cty.StringVal("bat")}), + }) + objectOfMaps := cty.ObjectVal(map[string]cty.Value{ "foo": cty.MapVal(map[string]cty.Value{ "a": cty.StringVal("bar"), }), @@ -1535,7 +1539,7 @@ func TestLookup(t *testing.T) { }, { []cty.Value{ - mapOfObjects, + objectOfMaps, cty.StringVal("foo"), }, cty.MapVal(map[string]cty.Value{ @@ -1543,6 +1547,14 @@ func TestLookup(t *testing.T) { }), false, }, + { + []cty.Value{ + mapOfTuples, + cty.StringVal("foo"), + }, + cty.TupleVal([]cty.Value{cty.StringVal("bar")}), + false, + }, { // Invalid key []cty.Value{ simpleMap,