Merge pull request #7834 from hashicorp/f-interp-funcs-list-map-audit

config: Audit all interpolation functions for list/map behavior

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 {
+				strVal, ok := val.Value.(string)
-					if strVal == "" {
+				if !ok {
-						continue
+					return nil, fmt.Errorf(
-					}
+						"compact() may only be used with flat lists, this list contains elements of %s",
-
+						val.Type.Printable())
-					outputList = append(outputList, strVal)
 				}
+				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 arg.([]ast.Variable) {
-					for _, part := range parts.Value.([]ast.Variable) {
+					if part.Type != ast.TypeString {
-						list = append(list, part.Value.(string))
+						return nil, fmt.Errorf(
-					}
+							"only works on flat lists, this list contains elements of %s",
-				}
+							part.Type.Printable())
-				if parts, ok := arg.([]ast.Variable); ok {
-					for _, part := range parts {
-						list = append(list, part.Value.(string))
 					}
+					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(
+				return nil, fmt.Errorf(
-					"lookup for '%s' has bad type %s",
+					"lookup() may only be used with flat maps, this map contains elements of %s",
-					args[1].(string), v.Type)
+					v.Type.Printable())
 			}
 
 			return v.Value.(string), nil
@@ -771,8 +780,13 @@ func interpolationFuncElement() ast.Function {
 
 			resolvedIndex := index % len(list)
 
-			v := list[resolvedIndex].Value
+			v := list[resolvedIndex]
-			return v, nil
+			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
 		},
 	}

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_list":        interfaceToVariableSwallowError([]string{"foo", "baz"}),
-			"var.a_short_list": interfaceToVariableSwallowError([]string{"foo"}),
+			"var.a_short_list":  interfaceToVariableSwallowError([]string{"foo"}),
-			"var.empty_list":   interfaceToVariableSwallowError([]interface{}{}),
+			"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)
 		}

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.
+     occurrence of each element, and removes subsequent occurences. This
-     Example: `distinct(var.usernames)`
+     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
+      This function only works on flat lists. Examples:
-      a count greater than one.
+      * `element(aws_subnet.foo.*.id, count.index)`
-      Example: `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(list, elem)` - Finds the index of a given element in a list.
-      `index(aws_instance.foo.*.tags.Name, "foo-test")`
+      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
+  * `join(delim, list)` - Joins the list with the delimiter for a resultant string.
-      only possible with splat variables from resources with a count
+      This function works only on flat lists.
-      greater than one. Example: `join(",", aws_instance.foo.*.id)`
+      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