From 88030764ff48410e4ce405c100f1ca22c6926762 Mon Sep 17 00:00:00 2001 From: Paul Hinze Date: Wed, 27 Jul 2016 12:47:44 -0500 Subject: [PATCH] config: Audit all interpolation functions for list/map behavior - `distinct()` - error on non-flat lists - `element()` - error on non-flat lists - `join()` - error on non-flat lists - `length()` - support maps - `lookup()` - error on non-flat maps - `values()` - error on non-flat maps --- config/interpolate_funcs.go | 58 ++++++++----- config/interpolate_funcs_test.go | 83 +++++++++++++++++-- .../docs/configuration/interpolation.html.md | 36 ++++---- 3 files changed, 136 insertions(+), 41 deletions(-) diff --git a/config/interpolate_funcs.go b/config/interpolate_funcs.go index 45f8e8915..87e9de778 100644 --- a/config/interpolate_funcs.go +++ b/config/interpolate_funcs.go @@ -180,13 +180,17 @@ func interpolationFuncCompact() ast.Function { var outputList []string for _, val := range inputList { - if strVal, ok := val.Value.(string); ok { - if strVal == "" { - continue - } - - outputList = append(outputList, strVal) + strVal, ok := val.Value.(string) + if !ok { + return nil, fmt.Errorf( + "compact() may only be used with flat lists, this list contains elements of %s", + val.Type.Printable()) } + if strVal == "" { + continue + } + + outputList = append(outputList, strVal) } return stringSliceToVariableValue(outputList), nil }, @@ -487,11 +491,16 @@ func interpolationFuncDistinct() ast.Function { var list []string if len(args) != 1 { - return nil, fmt.Errorf("distinct() excepts only one argument.") + return nil, fmt.Errorf("accepts only one argument.") } if argument, ok := args[0].([]ast.Variable); ok { for _, element := range argument { + if element.Type != ast.TypeString { + return nil, fmt.Errorf( + "only works for flat lists, this list contains elements of %s", + element.Type.Printable()) + } list = appendIfMissing(list, element.Value.(string)) } } @@ -527,15 +536,13 @@ func interpolationFuncJoin() ast.Function { } for _, arg := range args[1:] { - if parts, ok := arg.(ast.Variable); ok { - for _, part := range parts.Value.([]ast.Variable) { - list = append(list, part.Value.(string)) - } - } - if parts, ok := arg.([]ast.Variable); ok { - for _, part := range parts { - list = append(list, part.Value.(string)) + for _, part := range arg.([]ast.Variable) { + if part.Type != ast.TypeString { + return nil, fmt.Errorf( + "only works on flat lists, this list contains elements of %s", + part.Type.Printable()) } + list = append(list, part.Value.(string)) } } @@ -639,9 +646,11 @@ func interpolationFuncLength() ast.Function { return len(typedSubject), nil case []ast.Variable: return len(typedSubject), nil + case map[string]ast.Variable: + return len(typedSubject), nil } - return 0, fmt.Errorf("arguments to length() must be a string or list") + return 0, fmt.Errorf("arguments to length() must be a string, list, or map") }, } } @@ -740,9 +749,9 @@ func interpolationFuncLookup(vs map[string]ast.Variable) ast.Function { } } if v.Type != ast.TypeString { - return "", fmt.Errorf( - "lookup for '%s' has bad type %s", - args[1].(string), v.Type) + return nil, fmt.Errorf( + "lookup() may only be used with flat maps, this map contains elements of %s", + v.Type.Printable()) } return v.Value.(string), nil @@ -771,8 +780,13 @@ func interpolationFuncElement() ast.Function { resolvedIndex := index % len(list) - v := list[resolvedIndex].Value - return v, nil + v := list[resolvedIndex] + if v.Type != ast.TypeString { + return nil, fmt.Errorf( + "element() may only be used with flat lists, this list contains elements of %s", + v.Type.Printable()) + } + return v.Value, nil }, } } @@ -793,7 +807,7 @@ func interpolationFuncKeys(vs map[string]ast.Variable) ast.Function { sort.Strings(keys) - //Keys are guaranteed to be strings + // Keys are guaranteed to be strings return stringSliceToVariableValue(keys), nil }, } diff --git a/config/interpolate_funcs_test.go b/config/interpolate_funcs_test.go index dcc563ecb..11450f668 100644 --- a/config/interpolate_funcs_test.go +++ b/config/interpolate_funcs_test.go @@ -211,6 +211,13 @@ func TestInterpolateFuncCompact(t *testing.T) { []interface{}{}, false, }, + + // errrors on list of lists + { + `${compact(list(list("a"), list("b")))}`, + nil, + true, + }, }, }) } @@ -502,6 +509,12 @@ func TestInterpolateFuncDistinct(t *testing.T) { nil, true, }, + // non-flat list is an error + { + `${distinct(list(list("a"), list("a")))}`, + nil, + true, + }, }, }) } @@ -665,6 +678,7 @@ func TestInterpolateFuncJoin(t *testing.T) { Vars: map[string]ast.Variable{ "var.a_list": interfaceToVariableSwallowError([]string{"foo"}), "var.a_longer_list": interfaceToVariableSwallowError([]string{"foo", "bar", "baz"}), + "var.list_of_lists": interfaceToVariableSwallowError([]interface{}{[]string{"foo"}, []string{"bar"}, []string{"baz"}}), }, Cases: []testFunctionCase{ { @@ -684,6 +698,17 @@ func TestInterpolateFuncJoin(t *testing.T) { "foo.bar.baz", false, }, + + { + `${join(".", var.list_of_lists)}`, + nil, + true, + }, + { + `${join(".", list(list("nested")))}`, + nil, + true, + }, }, }) } @@ -878,6 +903,17 @@ func TestInterpolateFuncLength(t *testing.T) { "0", false, }, + // Works for maps + { + `${length(map("k", "v"))}`, + "1", + false, + }, + { + `${length(map("k1", "v1", "k2", "v2"))}`, + "2", + false, + }, }, }) } @@ -1003,15 +1039,29 @@ func TestInterpolateFuncSplit(t *testing.T) { func TestInterpolateFuncLookup(t *testing.T) { testFunction(t, testFunctionConfig{ Vars: map[string]ast.Variable{ - "var.foo": ast.Variable{ + "var.foo": { Type: ast.TypeMap, Value: map[string]ast.Variable{ - "bar": ast.Variable{ + "bar": { Type: ast.TypeString, Value: "baz", }, }, }, + "var.map_of_lists": ast.Variable{ + Type: ast.TypeMap, + Value: map[string]ast.Variable{ + "bar": { + Type: ast.TypeList, + Value: []ast.Variable{ + { + Type: ast.TypeString, + Value: "baz", + }, + }, + }, + }, + }, }, Cases: []testFunctionCase{ { @@ -1048,6 +1098,13 @@ func TestInterpolateFuncLookup(t *testing.T) { true, }, + // Cannot lookup into map of lists + { + `${lookup(var.map_of_lists, "bar")}`, + nil, + true, + }, + // Non-empty default { `${lookup(var.foo, "zap", "xyz")}`, @@ -1209,6 +1266,13 @@ func TestInterpolateFuncValues(t *testing.T) { nil, true, }, + + // Map of lists + { + `${values(map("one", list()))}`, + nil, + true, + }, }, }) } @@ -1221,9 +1285,10 @@ func interfaceToVariableSwallowError(input interface{}) ast.Variable { func TestInterpolateFuncElement(t *testing.T) { testFunction(t, testFunctionConfig{ Vars: map[string]ast.Variable{ - "var.a_list": interfaceToVariableSwallowError([]string{"foo", "baz"}), - "var.a_short_list": interfaceToVariableSwallowError([]string{"foo"}), - "var.empty_list": interfaceToVariableSwallowError([]interface{}{}), + "var.a_list": interfaceToVariableSwallowError([]string{"foo", "baz"}), + "var.a_short_list": interfaceToVariableSwallowError([]string{"foo"}), + "var.empty_list": interfaceToVariableSwallowError([]interface{}{}), + "var.a_nested_list": interfaceToVariableSwallowError([]interface{}{[]string{"foo"}, []string{"baz"}}), }, Cases: []testFunctionCase{ { @@ -1265,6 +1330,13 @@ func TestInterpolateFuncElement(t *testing.T) { nil, true, }, + + // Only works on single-level lists + { + `${element(var.a_nested_list, "0")}`, + nil, + true, + }, }, }) } @@ -1466,6 +1538,7 @@ func testFunction(t *testing.T, config testFunctionConfig) { } result, err := hil.Eval(ast, langEvalConfig(config.Vars)) + t.Logf("err: %s", err) if err != nil != tc.Error { t.Fatalf("Case #%d:\ninput: %#v\nerr: %s", i, tc.Input, err) } diff --git a/website/source/docs/configuration/interpolation.html.md b/website/source/docs/configuration/interpolation.html.md index ff555f2e6..71b9e150b 100644 --- a/website/source/docs/configuration/interpolation.html.md +++ b/website/source/docs/configuration/interpolation.html.md @@ -115,15 +115,15 @@ The supported built-in functions are: Example: `concat(aws_instance.db.*.tags.Name, aws_instance.web.*.tags.Name)` * `distinct(list)` - Removes duplicate items from a list. Keeps the first - occurrence of each element, and removes subsequent occurences. - Example: `distinct(var.usernames)` + occurrence of each element, and removes subsequent occurences. This + function is only valid for flat lists. Example: `distinct(var.usernames)` * `element(list, index)` - Returns a single element from a list at the given index. If the index is greater than the number of elements, this function will wrap using a standard mod algorithm. - A list is only possible with splat variables from resources with - a count greater than one. - Example: `element(aws_subnet.foo.*.id, count.index)` + This function only works on flat lists. Examples: + * `element(aws_subnet.foo.*.id, count.index)` + * `element(var.list_of_strings, 2)` * `file(path)` - Reads the contents of a file into the string. Variables in this file are _not_ interpolated. The contents of the file are @@ -149,24 +149,28 @@ The supported built-in functions are: `formatlist("instance %v has private ip %v", aws_instance.foo.*.id, aws_instance.foo.*.private_ip)`. Passing lists with different lengths to formatlist results in an error. - * `index(list, elem)` - Finds the index of a given element in a list. Example: - `index(aws_instance.foo.*.tags.Name, "foo-test")` + * `index(list, elem)` - Finds the index of a given element in a list. + This function only works on flat lists. + Example: `index(aws_instance.foo.*.tags.Name, "foo-test")` - * `join(delim, list)` - Joins the list with the delimiter for a resultant string. A list is - only possible with splat variables from resources with a count - greater than one. Example: `join(",", aws_instance.foo.*.id)` + * `join(delim, list)` - Joins the list with the delimiter for a resultant string. + This function works only on flat lists. + Examples: + * `join(",", aws_instance.foo.*.id)` + * `join(",", var.ami_list)` * `jsonencode(item)` - Returns a JSON-encoded representation of the given item, which may be a string, list of strings, or map from string to string. Note that if the item is a string, the return value includes the double quotes. - * `keys(map)` - Returns a lexically sorted, JSON-encoded list of the map keys. + * `keys(map)` - Returns a lexically sorted list of the map keys. - * `length(list)` - Returns a number of members in a given list + * `length(list)` - Returns a number of members in a given list, map, or string. or a number of characters in a given string. * `${length(split(",", "a,b,c"))}` = 3 * `${length("a,b,c")}` = 5 + * `${length(map("key", "val"))}` = 1 * `list(items...)` - Returns a list consisting of the arguments to the function. This function provides a way of representing list literals in interpolation. @@ -177,7 +181,9 @@ The supported built-in functions are: variable. The `map` parameter should be another variable, such as `var.amis`. If `key` does not exist in `map`, the interpolation will fail unless you specify a third argument, `default`, which should be a - string value to return if no `key` is found in `map. + string value to return if no `key` is found in `map`. This function + only works on flat maps and will return an error for maps that + include nested lists or maps. * `lower(string)` - Returns a copy of the string with all Unicode letters mapped to their lower case. @@ -232,7 +238,9 @@ The supported built-in functions are: * `uuid()` - Returns a UUID string in RFC 4122 v4 format. This string will change with every invocation of the function, so in order to prevent diffs on every plan & apply, it must be used with the [`ignore_changes`](/docs/configuration/resources.html#ignore-changes) lifecycle attribute. - * `values(map)` - Returns a JSON-encoded list of the map values, in the order of the keys returned by the `keys` function. + * `values(map)` - Returns a list of the map values, in the order of the keys + returned by the `keys` function. This function only works on flat maps and + will return an error for maps that include nested lists or maps. ## Templates