diff --git a/config/interpolate_funcs.go b/config/interpolate_funcs.go index 23cc9dfb0210..9b26c24ff2d8 100644 --- a/config/interpolate_funcs.go +++ b/config/interpolate_funcs.go @@ -54,12 +54,13 @@ func interpolationFuncConcat() ast.Function { continue } - if strings.Contains(argument, InterpSplitDelim) { + if IsStringList(argument) { isDeprecated = false + finalList = append(finalList, StringList(argument).Slice()...) + } else { + finalList = append(finalList, argument) } - finalList = append(finalList, argument) - // Deprecated concat behaviour b.WriteString(argument) } @@ -68,7 +69,7 @@ func interpolationFuncConcat() ast.Function { return b.String(), nil } - return strings.Join(finalList, InterpSplitDelim), nil + return NewStringList(finalList).String(), nil }, } } @@ -131,11 +132,16 @@ func interpolationFuncFormatList() ast.Function { if !ok { continue } - parts := strings.Split(s, InterpSplitDelim) - if len(parts) == 1 { + if !IsStringList(s) { continue } + + parts := StringList(s).Slice() + + // otherwise the list is sent down to be indexed varargs[i-1] = parts + + // Check length if n == 0 { // first list we've seen n = len(parts) @@ -167,7 +173,7 @@ func interpolationFuncFormatList() ast.Function { } list[i] = fmt.Sprintf(format, fmtargs...) } - return strings.Join(list, InterpSplitDelim), nil + return NewStringList(list).String(), nil }, } } @@ -181,7 +187,7 @@ func interpolationFuncJoin() ast.Function { Callback: func(args []interface{}) (interface{}, error) { var list []string for _, arg := range args[1:] { - parts := strings.Split(arg.(string), InterpSplitDelim) + parts := StringList(arg.(string)).Slice() list = append(list, parts...) } @@ -223,18 +229,15 @@ func interpolationFuncLength() ast.Function { ReturnType: ast.TypeInt, Variadic: false, Callback: func(args []interface{}) (interface{}, error) { - if !strings.Contains(args[0].(string), InterpSplitDelim) { + if !IsStringList(args[0].(string)) { return len(args[0].(string)), nil } - var list []string + length := 0 for _, arg := range args { - parts := strings.Split(arg.(string), InterpSplitDelim) - for _, part := range parts { - list = append(list, part) - } + length += StringList(arg.(string)).Length() } - return len(list), nil + return length, nil }, } } @@ -246,7 +249,9 @@ func interpolationFuncSplit() ast.Function { ArgTypes: []ast.Type{ast.TypeString, ast.TypeString}, ReturnType: ast.TypeString, Callback: func(args []interface{}) (interface{}, error) { - return strings.Replace(args[1].(string), args[0].(string), InterpSplitDelim, -1), nil + sep := args[0].(string) + s := args[1].(string) + return NewStringList(strings.Split(s, sep)).String(), nil }, } } @@ -284,7 +289,7 @@ func interpolationFuncElement() ast.Function { ArgTypes: []ast.Type{ast.TypeString, ast.TypeString}, ReturnType: ast.TypeString, Callback: func(args []interface{}) (interface{}, error) { - list := strings.Split(args[0].(string), InterpSplitDelim) + list := StringList(args[0].(string)) index, err := strconv.Atoi(args[1].(string)) if err != nil { @@ -292,7 +297,7 @@ func interpolationFuncElement() ast.Function { "invalid number for index, got %s", args[1]) } - v := list[index%len(list)] + v := list.Element(index) return v, nil }, } @@ -323,7 +328,7 @@ func interpolationFuncKeys(vs map[string]ast.Variable) ast.Function { sort.Strings(keys) - return strings.Join(keys, InterpSplitDelim), nil + return NewStringList(keys).String(), nil }, } } @@ -363,7 +368,7 @@ func interpolationFuncValues(vs map[string]ast.Variable) ast.Function { vals = append(vals, vs[k].Value.(string)) } - return strings.Join(vals, InterpSplitDelim), nil + return NewStringList(vals).String(), nil }, } } diff --git a/config/interpolate_funcs_test.go b/config/interpolate_funcs_test.go index 830a0154cfcd..3258bda5c449 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/terraform/config/lang" @@ -42,74 +41,46 @@ func TestInterpolateFuncConcat(t *testing.T) { // String + list { `${concat("a", split(",", "b,c"))}`, - fmt.Sprintf( - "%s%s%s%s%s", - "a", - InterpSplitDelim, - "b", - InterpSplitDelim, - "c"), + NewStringList([]string{"a", "b", "c"}).String(), false, }, // List + string { `${concat(split(",", "a,b"), "c")}`, - fmt.Sprintf( - "%s%s%s%s%s", - "a", - InterpSplitDelim, - "b", - InterpSplitDelim, - "c"), + NewStringList([]string{"a", "b", "c"}).String(), false, }, // Single list { `${concat(split(",", ",foo,"))}`, - fmt.Sprintf( - "%s%s%s", - InterpSplitDelim, - "foo", - InterpSplitDelim), + NewStringList([]string{"", "foo", ""}).String(), false, }, { `${concat(split(",", "a,b,c"))}`, - fmt.Sprintf( - "%s%s%s%s%s", - "a", - InterpSplitDelim, - "b", - InterpSplitDelim, - "c"), + NewStringList([]string{"a", "b", "c"}).String(), false, }, // Two lists { `${concat(split(",", "a,b,c"), split(",", "d,e"))}`, - strings.Join([]string{ - "a", "b", "c", "d", "e", - }, InterpSplitDelim), + NewStringList([]string{"a", "b", "c", "d", "e"}).String(), false, }, // Two lists with different separators { `${concat(split(",", "a,b,c"), split(" ", "d e"))}`, - strings.Join([]string{ - "a", "b", "c", "d", "e", - }, InterpSplitDelim), + NewStringList([]string{"a", "b", "c", "d", "e"}).String(), false, }, // More lists { `${concat(split(",", "a,b"), split(",", "c,d"), split(",", "e,f"), split(",", "0,1"))}`, - strings.Join([]string{ - "a", "b", "c", "d", "e", "f", "0", "1", - }, InterpSplitDelim), + NewStringList([]string{"a", "b", "c", "d", "e", "f", "0", "1"}).String(), false, }, }, @@ -204,7 +175,7 @@ func TestInterpolateFuncFormatList(t *testing.T) { // formatlist applies to each list element in turn { `${formatlist("<%s>", split(",", "A,B"))}`, - "" + InterpSplitDelim + "", + NewStringList([]string{"", ""}).String(), false, }, // formatlist repeats scalar elements @@ -219,23 +190,18 @@ func TestInterpolateFuncFormatList(t *testing.T) { "A=1, B=2, C=3", false, }, - // formatlist of lists of length zero/one are repeated, just as scalars are - { - `${join(", ", formatlist("%s=%s", split(",", ""), split(",", "1,2,3")))}`, - "=1, =2, =3", - false, - }, - { - `${join(", ", formatlist("%s=%s", split(",", "A"), split(",", "1,2,3")))}`, - "A=1, A=2, A=3", - false, - }, // Mismatched list lengths generate an error { `${formatlist("%s=%2s", split(",", "A,B,C,D"), split(",", "1,2,3"))}`, nil, true, }, + // Works with lists of length 1 [GH-2240] + { + `${formatlist("%s.id", split(",", "demo-rest-elb"))}`, + NewStringList([]string{"demo-rest-elb.id"}).String(), + false, + }, }, }) } @@ -250,7 +216,8 @@ func TestInterpolateFuncJoin(t *testing.T) { }, { - `${join(",", "foo")}`, + fmt.Sprintf(`${join(",", "%s")}`, + NewStringList([]string{"foo"}).String()), "foo", false, }, @@ -266,10 +233,7 @@ func TestInterpolateFuncJoin(t *testing.T) { { fmt.Sprintf(`${join(".", "%s")}`, - fmt.Sprintf( - "foo%sbar%sbaz", - InterpSplitDelim, - InterpSplitDelim)), + NewStringList([]string{"foo", "bar", "baz"}).String()), "foo.bar.baz", false, }, @@ -386,47 +350,39 @@ func TestInterpolateFuncSplit(t *testing.T) { true, }, + { + `${split(",", "")}`, + NewStringList([]string{""}).String(), + false, + }, + { `${split(",", "foo")}`, - "foo", + NewStringList([]string{"foo"}).String(), false, }, { `${split(",", ",,,")}`, - fmt.Sprintf( - "%s%s%s", - InterpSplitDelim, - InterpSplitDelim, - InterpSplitDelim), + NewStringList([]string{"", "", "", ""}).String(), false, }, { `${split(",", "foo,")}`, - fmt.Sprintf( - "%s%s", - "foo", - InterpSplitDelim), + NewStringList([]string{"foo", ""}).String(), false, }, { `${split(",", ",foo,")}`, - fmt.Sprintf( - "%s%s%s", - InterpSplitDelim, - "foo", - InterpSplitDelim), + NewStringList([]string{"", "foo", ""}).String(), false, }, { `${split(".", "foo.bar.baz")}`, - fmt.Sprintf( - "foo%sbar%sbaz", - InterpSplitDelim, - InterpSplitDelim), + NewStringList([]string{"foo", "bar", "baz"}).String(), false, }, }, @@ -484,9 +440,7 @@ func TestInterpolateFuncKeys(t *testing.T) { Cases: []testFunctionCase{ { `${keys("foo")}`, - fmt.Sprintf( - "bar%squx", - InterpSplitDelim), + NewStringList([]string{"bar", "qux"}).String(), false, }, @@ -533,9 +487,7 @@ func TestInterpolateFuncValues(t *testing.T) { Cases: []testFunctionCase{ { `${values("foo")}`, - fmt.Sprintf( - "quack%sbaz", - InterpSplitDelim), + NewStringList([]string{"quack", "baz"}).String(), false, }, @@ -568,13 +520,14 @@ func TestInterpolateFuncElement(t *testing.T) { Cases: []testFunctionCase{ { fmt.Sprintf(`${element("%s", "1")}`, - "foo"+InterpSplitDelim+"baz"), + NewStringList([]string{"foo", "baz"}).String()), "baz", false, }, { - `${element("foo", "0")}`, + fmt.Sprintf(`${element("%s", "0")}`, + NewStringList([]string{"foo"}).String()), "foo", false, }, @@ -582,7 +535,7 @@ func TestInterpolateFuncElement(t *testing.T) { // Invalid index should wrap vs. out-of-bounds { fmt.Sprintf(`${element("%s", "2")}`, - "foo"+InterpSplitDelim+"baz"), + NewStringList([]string{"foo", "baz"}).String()), "foo", false, }, @@ -590,7 +543,7 @@ func TestInterpolateFuncElement(t *testing.T) { // Too many args { fmt.Sprintf(`${element("%s", "0", "2")}`, - "foo"+InterpSplitDelim+"baz"), + NewStringList([]string{"foo", "baz"}).String()), nil, true, }, diff --git a/config/interpolate_walk.go b/config/interpolate_walk.go index faacb572621e..adcb5e32cf4d 100644 --- a/config/interpolate_walk.go +++ b/config/interpolate_walk.go @@ -10,10 +10,6 @@ import ( "github.com/mitchellh/reflectwalk" ) -// InterpSplitDelim is the delimeter that is looked for to split when -// it is returned. -const InterpSplitDelim = `B780FFEC-B661-4EB8-9236-A01737AD98B6` - // interpolationWalker implements interfaces for the reflectwalk package // (github.com/mitchellh/reflectwalk) that can be used to automatically // execute a callback for an interpolation. @@ -148,8 +144,8 @@ func (w *interpolationWalker) Primitive(v reflect.Value) error { // set if it is computed. This behavior is different if we're // splitting (in a SliceElem) or not. remove := false - if w.loc == reflectwalk.SliceElem { - parts := strings.Split(replaceVal, InterpSplitDelim) + if w.loc == reflectwalk.SliceElem && IsStringList(replaceVal) { + parts := StringList(replaceVal).Slice() for _, p := range parts { if p == UnknownVariableValue { remove = true @@ -247,7 +243,7 @@ func (w *interpolationWalker) splitSlice() { if !ok { continue } - if idx := strings.Index(sv, InterpSplitDelim); idx >= 0 { + if IsStringList(sv) { split = true break } @@ -269,10 +265,15 @@ func (w *interpolationWalker) splitSlice() { continue } - // Split on the delimiter - for _, p := range strings.Split(sv, InterpSplitDelim) { - result = append(result, p) + if IsStringList(sv) { + for _, p := range StringList(sv).Slice() { + result = append(result, p) + } + continue } + + // Not a string list, so just set it + result = append(result, sv) } // Our slice is now done, we have to replace the slice now diff --git a/config/interpolate_walk_test.go b/config/interpolate_walk_test.go index 9b2c34133fc6..fc7c8b5492f3 100644 --- a/config/interpolate_walk_test.go +++ b/config/interpolate_walk_test.go @@ -157,7 +157,7 @@ func TestInterpolationWalker_replace(t *testing.T) { "bing", }, }, - Value: "bar" + InterpSplitDelim + "baz", + Value: NewStringList([]string{"bar", "baz"}).String(), }, { @@ -168,7 +168,7 @@ func TestInterpolationWalker_replace(t *testing.T) { }, }, Output: map[string]interface{}{}, - Value: UnknownVariableValue + InterpSplitDelim + "baz", + Value: NewStringList([]string{UnknownVariableValue, "baz"}).String(), }, } @@ -183,7 +183,7 @@ func TestInterpolationWalker_replace(t *testing.T) { } if !reflect.DeepEqual(tc.Input, tc.Output) { - t.Fatalf("%d: bad:\n\n%#v", i, tc.Input) + t.Fatalf("%d: bad:\n\nexpected:%#v\ngot:%#v", i, tc.Output, tc.Input) } } } diff --git a/config/string_list.go b/config/string_list.go new file mode 100644 index 000000000000..70d43d1e4bc4 --- /dev/null +++ b/config/string_list.go @@ -0,0 +1,76 @@ +package config + +import ( + "fmt" + "strings" +) + +// StringList represents the "poor man's list" that terraform uses +// internally +type StringList string + +// This is the delimiter used to recognize and split StringLists +// +// It plays two semantic roles: +// * It introduces a list +// * It terminates each element +// +// Example representations: +// [] => SLD +// [""] => SLDSLD +// [" "] => SLD SLD +// ["foo"] => SLDfooSLD +// ["foo", "bar"] => SLDfooSLDbarSLD +// ["", ""] => SLDSLDSLD +const stringListDelim = `B780FFEC-B661-4EB8-9236-A01737AD98B6` + +// Build a StringList from a slice +func NewStringList(parts []string) StringList { + // We have to special case the empty list representation + if len(parts) == 0 { + return StringList(stringListDelim) + } + return StringList(fmt.Sprintf("%s%s%s", + stringListDelim, + strings.Join(parts, stringListDelim), + stringListDelim, + )) +} + +// Returns an element at the index, wrapping around the length of the string +// when index > list length +func (sl StringList) Element(index int) string { + if sl.Length() == 0 { + return "" + } + return sl.Slice()[index%sl.Length()] +} + +// Returns the length of the StringList +func (sl StringList) Length() int { + return len(sl.Slice()) +} + +// Returns a slice of strings as represented by this StringList +func (sl StringList) Slice() []string { + parts := strings.Split(string(sl), stringListDelim) + + switch len(parts) { + case 0, 1: + return []string{} + case 2: + return []string{""} + } + + // strip empty elements generated by leading and trailing delimiters + return parts[1 : len(parts)-1] +} + +func (sl StringList) String() string { + return string(sl) +} + +// Determines if a given string represents a StringList +func IsStringList(s string) bool { + return strings.Contains(s, stringListDelim) +} diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 3fb368ccac9c..bf5a3a19bdc2 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -582,7 +582,7 @@ func TestSchemaMap_Diff(t *testing.T) { }, ConfigVariables: map[string]string{ - "var.foo": "2" + config.InterpSplitDelim + "5", + "var.foo": config.NewStringList([]string{"2", "5"}).String(), }, Diff: &terraform.InstanceDiff{ @@ -626,8 +626,8 @@ func TestSchemaMap_Diff(t *testing.T) { }, ConfigVariables: map[string]string{ - "var.foo": config.UnknownVariableValue + - config.InterpSplitDelim + "5", + "var.foo": config.NewStringList([]string{ + config.UnknownVariableValue, "5"}).String(), }, Diff: &terraform.InstanceDiff{ @@ -905,7 +905,7 @@ func TestSchemaMap_Diff(t *testing.T) { }, ConfigVariables: map[string]string{ - "var.foo": "2" + config.InterpSplitDelim + "5", + "var.foo": config.NewStringList([]string{"2", "5"}).String(), }, Diff: &terraform.InstanceDiff{ @@ -952,8 +952,8 @@ func TestSchemaMap_Diff(t *testing.T) { }, ConfigVariables: map[string]string{ - "var.foo": config.UnknownVariableValue + - config.InterpSplitDelim + "5", + "var.foo": config.NewStringList([]string{ + config.UnknownVariableValue, "5"}).String(), }, Diff: &terraform.InstanceDiff{ diff --git a/terraform/context_test.go b/terraform/context_test.go index 55580cb9a8e9..53092d72d21c 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -1672,29 +1672,6 @@ func TestContext2Plan_provider(t *testing.T) { } } -func TestContext2Plan_varMultiCountOne(t *testing.T) { - m := testModule(t, "plan-var-multi-count-one") - p := testProvider("aws") - p.DiffFn = testDiffFn - ctx := testContext2(t, &ContextOpts{ - Module: m, - Providers: map[string]ResourceProviderFactory{ - "aws": testProviderFuncFixed(p), - }, - }) - - plan, err := ctx.Plan() - if err != nil { - t.Fatalf("err: %s", err) - } - - actual := strings.TrimSpace(plan.String()) - expected := strings.TrimSpace(testTerraformPlanVarMultiCountOneStr) - if actual != expected { - t.Fatalf("bad:\n%s", actual) - } -} - func TestContext2Plan_varListErr(t *testing.T) { m := testModule(t, "plan-var-list-err") p := testProvider("aws") diff --git a/terraform/interpolate.go b/terraform/interpolate.go index a066c0a5f351..83f6265e1eb5 100644 --- a/terraform/interpolate.go +++ b/terraform/interpolate.go @@ -453,7 +453,7 @@ func (i *Interpolater) computeResourceMultiVariable( v.FullKey()) } - return strings.Join(values, config.InterpSplitDelim), nil + return config.NewStringList(values).String(), nil } func (i *Interpolater) resourceVariableInfo( diff --git a/terraform/test-fixtures/plan-var-multi-count-one/main.tf b/terraform/test-fixtures/plan-var-multi-count-one/main.tf deleted file mode 100644 index 6fd84932f105..000000000000 --- a/terraform/test-fixtures/plan-var-multi-count-one/main.tf +++ /dev/null @@ -1,9 +0,0 @@ -resource "aws_instance" "foo" { - num = "2" - - count = 1 -} - -resource "aws_instance" "bar" { - foo = "${aws_instance.foo.*.num}" -}