From 8dcbc0b0a0d6b985c9fa4c096c9f9214cacdfa7b Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 19 Jul 2016 17:21:14 -0400 Subject: [PATCH] Add concat to accept lists of lists and maps This will allow the concat interpolation function to accept lists of lists, and lists of maps as well as strings. We still allow bare strings for backwards compatibility, but remove some of the old comment wording as it could cause confusion of this function with actual string concatenation. Since maps are now supported in the config, this removes the superfluous (and failing) TestInterpolationFuncConcatListOfMaps. --- config/interpolate_funcs.go | 51 ++++++++------ config/interpolate_funcs_test.go | 113 +++++++++++++++++++++---------- 2 files changed, 107 insertions(+), 57 deletions(-) diff --git a/config/interpolate_funcs.go b/config/interpolate_funcs.go index a3cdf828b..91aa66da7 100644 --- a/config/interpolate_funcs.go +++ b/config/interpolate_funcs.go @@ -258,10 +258,8 @@ func interpolationFuncCoalesce() ast.Function { } } -// interpolationFuncConcat implements the "concat" function that -// concatenates multiple strings. This isn't actually necessary anymore -// since our language supports string concat natively, but for backwards -// compat we do this. +// interpolationFuncConcat implements the "concat" function that concatenates +// multiple lists. func interpolationFuncConcat() ast.Function { return ast.Function{ ArgTypes: []ast.Type{ast.TypeAny}, @@ -269,33 +267,42 @@ func interpolationFuncConcat() ast.Function { Variadic: true, VariadicType: ast.TypeAny, Callback: func(args []interface{}) (interface{}, error) { - var finalListElements []string + var outputList []ast.Variable for _, arg := range args { - // Append strings for backward compatibility - if argument, ok := arg.(string); ok { - finalListElements = append(finalListElements, argument) - continue - } - - // Otherwise variables - if argument, ok := arg.([]ast.Variable); ok { - for _, element := range argument { - t := element.Type - switch t { + switch arg := arg.(type) { + case string: + outputList = append(outputList, ast.Variable{Type: ast.TypeString, Value: arg}) + case []ast.Variable: + for _, v := range arg { + switch v.Type { case ast.TypeString: - finalListElements = append(finalListElements, element.Value.(string)) + outputList = append(outputList, v) + case ast.TypeList: + outputList = append(outputList, v) + case ast.TypeMap: + outputList = append(outputList, v) default: - return nil, fmt.Errorf("concat() does not support lists of %s", t.Printable()) + return nil, fmt.Errorf("concat() does not support lists of %s", v.Type.Printable()) } } - continue - } - return nil, fmt.Errorf("arguments to concat() must be a string or list of strings") + default: + return nil, fmt.Errorf("concat() does not support %T", arg) + } } - return stringSliceToVariableValue(finalListElements), nil + // we don't support heterogeneous types, so make sure all types match the first + if len(outputList) > 0 { + firstType := outputList[0].Type + for _, v := range outputList[1:] { + if v.Type != firstType { + return nil, fmt.Errorf("unexpected %s in list of %s", v.Type.Printable(), firstType.Printable()) + } + } + } + + return outputList, nil }, } } diff --git a/config/interpolate_funcs_test.go b/config/interpolate_funcs_test.go index 5b9800c14..541bcffab 100644 --- a/config/interpolate_funcs_test.go +++ b/config/interpolate_funcs_test.go @@ -5,7 +5,6 @@ import ( "io/ioutil" "os" "reflect" - "strings" "testing" "github.com/hashicorp/hil" @@ -325,44 +324,88 @@ func TestInterpolateFuncConcat(t *testing.T) { []interface{}{"a", "b", "c", "d", "e", "f", "0", "1"}, false, }, + + // list vars + { + `${concat("${var.list}", "${var.list}")}`, + []interface{}{"a", "b", "a", "b"}, + false, + }, + // lists of lists + { + `${concat("${var.lists}", "${var.lists}")}`, + []interface{}{[]interface{}{"c", "d"}, []interface{}{"c", "d"}}, + false, + }, + + // lists of maps + { + `${concat("${var.maps}", "${var.maps}")}`, + []interface{}{map[string]interface{}{"key1": "a", "key2": "b"}, map[string]interface{}{"key1": "a", "key2": "b"}}, + false, + }, + + // mismatched types + { + `${concat("${var.lists}", "${var.maps}")}`, + nil, + true, + }, + }, + Vars: map[string]ast.Variable{ + "var.list": { + Type: ast.TypeList, + Value: []ast.Variable{ + { + Type: ast.TypeString, + Value: "a", + }, + { + Type: ast.TypeString, + Value: "b", + }, + }, + }, + "var.lists": { + Type: ast.TypeList, + Value: []ast.Variable{ + { + Type: ast.TypeList, + Value: []ast.Variable{ + { + Type: ast.TypeString, + Value: "c", + }, + { + Type: ast.TypeString, + Value: "d", + }, + }, + }, + }, + }, + "var.maps": { + Type: ast.TypeList, + Value: []ast.Variable{ + { + Type: ast.TypeMap, + Value: map[string]ast.Variable{ + "key1": { + Type: ast.TypeString, + Value: "a", + }, + "key2": { + Type: ast.TypeString, + Value: "b", + }, + }, + }, + }, + }, }, }) } -// TODO: This test is split out and calls a private function -// because there's no good way to get a list of maps into the unit -// tests due to GH-7142 - once lists of maps can be expressed properly as -// literals this unit test can be wrapped back into the suite above. -// -// Reproduces crash reported in GH-7030. -func TestInterpolationFuncConcatListOfMaps(t *testing.T) { - listOfMapsOne := ast.Variable{ - Type: ast.TypeList, - Value: []ast.Variable{ - { - Type: ast.TypeMap, - Value: map[string]interface{}{"one": "foo"}, - }, - }, - } - listOfMapsTwo := ast.Variable{ - Type: ast.TypeList, - Value: []ast.Variable{ - { - Type: ast.TypeMap, - Value: map[string]interface{}{"two": "bar"}, - }, - }, - } - args := []interface{}{listOfMapsOne.Value, listOfMapsTwo.Value} - - _, err := interpolationFuncConcat().Callback(args) - - if err == nil || !strings.Contains(err.Error(), "concat() does not support lists of type map") { - t.Fatalf("Expected err, got: %v", err) - } -} - func TestInterpolateFuncDistinct(t *testing.T) { testFunction(t, testFunctionConfig{ Cases: []testFunctionCase{