diff --git a/config/config.go b/config/config.go index e3eb6530e..d00794022 100644 --- a/config/config.go +++ b/config/config.go @@ -161,6 +161,7 @@ type VariableType byte const ( VariableTypeUnknown VariableType = iota VariableTypeString + VariableTypeList VariableTypeMap ) @@ -170,6 +171,8 @@ func (v VariableType) Printable() string { return "string" case VariableTypeMap: return "map" + case VariableTypeList: + return "list" default: return "unknown" } @@ -351,16 +354,30 @@ func (c *Config) Validate() error { m.Id())) } - // Check that the configuration can all be strings + // Check that the configuration can all be strings, lists or maps raw := make(map[string]interface{}) for k, v := range m.RawConfig.Raw { var strVal string - if err := mapstructure.WeakDecode(v, &strVal); err != nil { - errs = append(errs, fmt.Errorf( - "%s: variable %s must be a string value", - m.Id(), k)) + if err := mapstructure.WeakDecode(v, &strVal); err == nil { + raw[k] = strVal + continue } - raw[k] = strVal + + var mapVal map[string]interface{} + if err := mapstructure.WeakDecode(v, &mapVal); err == nil { + raw[k] = mapVal + continue + } + + var sliceVal []interface{} + if err := mapstructure.WeakDecode(v, &sliceVal); err == nil { + raw[k] = sliceVal + continue + } + + errs = append(errs, fmt.Errorf( + "%s: variable %s must be a string, list or map value", + m.Id(), k)) } // Check for invalid count variables @@ -721,7 +738,8 @@ func (c *Config) validateVarContextFn( if rv.Multi && rv.Index == -1 { *errs = append(*errs, fmt.Errorf( - "%s: multi-variable must be in a slice", source)) + "%s: use of the splat ('*') operator must be wrapped in a list declaration", + source)) } } } @@ -829,6 +847,7 @@ func (v *Variable) Merge(v2 *Variable) *Variable { var typeStringMap = map[string]VariableType{ "string": VariableTypeString, "map": VariableTypeMap, + "list": VariableTypeList, } // Type returns the type of variable this is. @@ -888,9 +907,9 @@ func (v *Variable) inferTypeFromDefault() VariableType { return VariableTypeString } - var strVal string - if err := mapstructure.WeakDecode(v.Default, &strVal); err == nil { - v.Default = strVal + var s string + if err := mapstructure.WeakDecode(v.Default, &s); err == nil { + v.Default = s return VariableTypeString } @@ -900,5 +919,11 @@ func (v *Variable) inferTypeFromDefault() VariableType { return VariableTypeMap } + var l []string + if err := mapstructure.WeakDecode(v.Default, &l); err == nil { + v.Default = l + return VariableTypeList + } + return VariableTypeUnknown } diff --git a/config/config_test.go b/config/config_test.go index 8e2565ff4..2ab314f35 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -215,8 +215,15 @@ func TestConfigValidate_moduleVarInt(t *testing.T) { func TestConfigValidate_moduleVarMap(t *testing.T) { c := testConfig(t, "validate-module-var-map") - if err := c.Validate(); err == nil { - t.Fatal("should be invalid") + if err := c.Validate(); err != nil { + t.Fatalf("should be valid: %s", err) + } +} + +func TestConfigValidate_moduleVarList(t *testing.T) { + c := testConfig(t, "validate-module-var-list") + if err := c.Validate(); err != nil { + t.Fatalf("should be valid: %s", err) } } @@ -367,10 +374,10 @@ func TestConfigValidate_varDefault(t *testing.T) { } } -func TestConfigValidate_varDefaultBadType(t *testing.T) { - c := testConfig(t, "validate-var-default-bad-type") - if err := c.Validate(); err == nil { - t.Fatal("should not be valid") +func TestConfigValidate_varDefaultListType(t *testing.T) { + c := testConfig(t, "validate-var-default-list-type") + if err := c.Validate(); err != nil { + t.Fatal("should be valid: %s", err) } } diff --git a/config/interpolate.go b/config/interpolate.go index bfdd114c6..14e70bfcc 100644 --- a/config/interpolate.go +++ b/config/interpolate.go @@ -284,18 +284,35 @@ func DetectVariables(root ast.Node) ([]InterpolatedVariable, error) { return n } - vn, ok := n.(*ast.VariableAccess) - if !ok { + switch vn := n.(type) { + case *ast.VariableAccess: + v, err := NewInterpolatedVariable(vn.Name) + if err != nil { + resultErr = err + return n + } + result = append(result, v) + case *ast.Index: + if va, ok := vn.Target.(*ast.VariableAccess); ok { + v, err := NewInterpolatedVariable(va.Name) + if err != nil { + resultErr = err + return n + } + result = append(result, v) + } + if va, ok := vn.Key.(*ast.VariableAccess); ok { + v, err := NewInterpolatedVariable(va.Name) + if err != nil { + resultErr = err + return n + } + result = append(result, v) + } + default: return n } - v, err := NewInterpolatedVariable(vn.Name) - if err != nil { - resultErr = err - return n - } - - result = append(result, v) return n } diff --git a/config/interpolate_funcs.go b/config/interpolate_funcs.go index 53d8ccccb..e13c75486 100644 --- a/config/interpolate_funcs.go +++ b/config/interpolate_funcs.go @@ -1,7 +1,6 @@ package config import ( - "bytes" "crypto/md5" "crypto/sha1" "crypto/sha256" @@ -24,6 +23,31 @@ import ( "github.com/mitchellh/go-homedir" ) +// stringSliceToVariableValue converts a string slice into the value +// required to be returned from interpolation functions which return +// TypeList. +func stringSliceToVariableValue(values []string) []ast.Variable { + output := make([]ast.Variable, len(values)) + for index, value := range values { + output[index] = ast.Variable{ + Type: ast.TypeString, + Value: value, + } + } + return output +} + +func listVariableValueToStringSlice(values []ast.Variable) ([]string, error) { + output := make([]string, len(values)) + for index, value := range values { + if value.Type != ast.TypeString { + return []string{}, fmt.Errorf("list has non-string element (%T)", value.Type.String()) + } + output[index] = value.Value.(string) + } + return output, nil +} + // Funcs is the mapping of built-in functions for configuration. func Funcs() map[string]ast.Function { return map[string]ast.Function{ @@ -61,14 +85,23 @@ func Funcs() map[string]ast.Function { // (e.g. as returned by "split") of any empty strings. func interpolationFuncCompact() ast.Function { return ast.Function{ - ArgTypes: []ast.Type{ast.TypeString}, - ReturnType: ast.TypeString, + ArgTypes: []ast.Type{ast.TypeList}, + ReturnType: ast.TypeList, Variadic: false, Callback: func(args []interface{}) (interface{}, error) { - if !IsStringList(args[0].(string)) { - return args[0].(string), nil + inputList := args[0].([]ast.Variable) + + var outputList []string + for _, val := range inputList { + if strVal, ok := val.Value.(string); ok { + if strVal == "" { + continue + } + + outputList = append(outputList, strVal) + } } - return StringList(args[0].(string)).Compact().String(), nil + return stringSliceToVariableValue(outputList), nil }, } } @@ -189,39 +222,32 @@ func interpolationFuncCoalesce() ast.Function { // compat we do this. func interpolationFuncConcat() ast.Function { return ast.Function{ - ArgTypes: []ast.Type{ast.TypeString}, - ReturnType: ast.TypeString, + ArgTypes: []ast.Type{ast.TypeAny}, + ReturnType: ast.TypeList, Variadic: true, - VariadicType: ast.TypeString, + VariadicType: ast.TypeAny, Callback: func(args []interface{}) (interface{}, error) { - var b bytes.Buffer - var finalList []string - - var isDeprecated = true + var finalListElements []string for _, arg := range args { - argument := arg.(string) - - if len(argument) == 0 { + // Append strings for backward compatibility + if argument, ok := arg.(string); ok { + finalListElements = append(finalListElements, argument) continue } - if IsStringList(argument) { - isDeprecated = false - finalList = append(finalList, StringList(argument).Slice()...) - } else { - finalList = append(finalList, argument) + // Otherwise variables + if argument, ok := arg.([]ast.Variable); ok { + for _, element := range argument { + finalListElements = append(finalListElements, element.Value.(string)) + } + continue } - // Deprecated concat behaviour - b.WriteString(argument) + return nil, fmt.Errorf("arguments to concat() must be a string or list") } - if isDeprecated { - return b.String(), nil - } - - return NewStringList(finalList).String(), nil + return stringSliceToVariableValue(finalListElements), nil }, } } @@ -266,10 +292,10 @@ func interpolationFuncFormat() ast.Function { // string formatting on lists. func interpolationFuncFormatList() ast.Function { return ast.Function{ - ArgTypes: []ast.Type{ast.TypeString}, + ArgTypes: []ast.Type{ast.TypeAny}, Variadic: true, VariadicType: ast.TypeAny, - ReturnType: ast.TypeString, + ReturnType: ast.TypeList, Callback: func(args []interface{}) (interface{}, error) { // Make a copy of the variadic part of args // to avoid modifying the original. @@ -280,15 +306,15 @@ func interpolationFuncFormatList() ast.Function { // Confirm along the way that all lists have the same length (n). var n int for i := 1; i < len(args); i++ { - s, ok := args[i].(string) + s, ok := args[i].([]ast.Variable) if !ok { continue } - if !IsStringList(s) { - continue - } - parts := StringList(s).Slice() + parts, err := listVariableValueToStringSlice(s) + if err != nil { + return nil, err + } // otherwise the list is sent down to be indexed varargs[i-1] = parts @@ -325,7 +351,7 @@ func interpolationFuncFormatList() ast.Function { } list[i] = fmt.Sprintf(format, fmtargs...) } - return NewStringList(list).String(), nil + return stringSliceToVariableValue(list), nil }, } } @@ -334,13 +360,13 @@ func interpolationFuncFormatList() ast.Function { // find the index of a specific element in a list func interpolationFuncIndex() ast.Function { return ast.Function{ - ArgTypes: []ast.Type{ast.TypeString, ast.TypeString}, + ArgTypes: []ast.Type{ast.TypeList, ast.TypeString}, ReturnType: ast.TypeInt, Callback: func(args []interface{}) (interface{}, error) { - haystack := StringList(args[0].(string)).Slice() + haystack := args[0].([]ast.Variable) needle := args[1].(string) for index, element := range haystack { - if needle == element { + if needle == element.Value { return index, nil } } @@ -353,13 +379,28 @@ func interpolationFuncIndex() ast.Function { // multi-variable values to be joined by some character. func interpolationFuncJoin() ast.Function { return ast.Function{ - ArgTypes: []ast.Type{ast.TypeString, ast.TypeString}, - ReturnType: ast.TypeString, + ArgTypes: []ast.Type{ast.TypeString}, + Variadic: true, + VariadicType: ast.TypeList, + ReturnType: ast.TypeString, Callback: func(args []interface{}) (interface{}, error) { var list []string + + if len(args) < 2 { + return nil, fmt.Errorf("not enough arguments to join()") + } + for _, arg := range args[1:] { - parts := StringList(arg.(string)).Slice() - list = append(list, parts...) + 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)) + } + } } return strings.Join(list, args[0].(string)), nil @@ -413,19 +454,20 @@ func interpolationFuncReplace() ast.Function { func interpolationFuncLength() ast.Function { return ast.Function{ - ArgTypes: []ast.Type{ast.TypeString}, + ArgTypes: []ast.Type{ast.TypeAny}, ReturnType: ast.TypeInt, Variadic: false, Callback: func(args []interface{}) (interface{}, error) { - if !IsStringList(args[0].(string)) { - return len(args[0].(string)), nil + subject := args[0] + + switch typedSubject := subject.(type) { + case string: + return len(typedSubject), nil + case []ast.Variable: + return len(typedSubject), nil } - length := 0 - for _, arg := range args { - length += StringList(arg.(string)).Length() - } - return length, nil + return 0, fmt.Errorf("arguments to length() must be a string or list") }, } } @@ -454,11 +496,12 @@ func interpolationFuncSignum() ast.Function { func interpolationFuncSplit() ast.Function { return ast.Function{ ArgTypes: []ast.Type{ast.TypeString, ast.TypeString}, - ReturnType: ast.TypeString, + ReturnType: ast.TypeList, Callback: func(args []interface{}) (interface{}, error) { sep := args[0].(string) s := args[1].(string) - return NewStringList(strings.Split(s, sep)).String(), nil + elements := strings.Split(s, sep) + return stringSliceToVariableValue(elements), nil }, } } @@ -495,10 +538,10 @@ func interpolationFuncLookup(vs map[string]ast.Variable) ast.Function { // wrap if the index is larger than the number of elements in the multi-variable value. func interpolationFuncElement() ast.Function { return ast.Function{ - ArgTypes: []ast.Type{ast.TypeString, ast.TypeString}, + ArgTypes: []ast.Type{ast.TypeList, ast.TypeString}, ReturnType: ast.TypeString, Callback: func(args []interface{}) (interface{}, error) { - list := StringList(args[0].(string)) + list := args[0].([]ast.Variable) index, err := strconv.Atoi(args[1].(string)) if err != nil || index < 0 { @@ -506,7 +549,9 @@ func interpolationFuncElement() ast.Function { "invalid number for index, got %s", args[1]) } - v := list.Element(index) + resolvedIndex := index % len(list) + + v := list[resolvedIndex].Value return v, nil }, } @@ -528,12 +573,8 @@ func interpolationFuncKeys(vs map[string]ast.Variable) ast.Function { sort.Strings(keys) - variable, err := hil.InterfaceToVariable(keys) - if err != nil { - return nil, err - } - - return variable.Value, nil + //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 e7b2cfa84..d5f44d498 100644 --- a/config/interpolate_funcs_test.go +++ b/config/interpolate_funcs_test.go @@ -17,21 +17,21 @@ func TestInterpolateFuncCompact(t *testing.T) { // empty string within array { `${compact(split(",", "a,,b"))}`, - NewStringList([]string{"a", "b"}).String(), + []interface{}{"a", "b"}, false, }, // empty string at the end of array { `${compact(split(",", "a,b,"))}`, - NewStringList([]string{"a", "b"}).String(), + []interface{}{"a", "b"}, false, }, // single empty string { `${compact(split(",", ""))}`, - NewStringList([]string{}).String(), + []interface{}{}, false, }, }, @@ -174,76 +174,52 @@ func TestInterpolateFuncCoalesce(t *testing.T) { }) } -func TestInterpolateFuncDeprecatedConcat(t *testing.T) { - testFunction(t, testFunctionConfig{ - Cases: []testFunctionCase{ - { - `${concat("foo", "bar")}`, - "foobar", - false, - }, - - { - `${concat("foo")}`, - "foo", - false, - }, - - { - `${concat()}`, - nil, - true, - }, - }, - }) -} - func TestInterpolateFuncConcat(t *testing.T) { testFunction(t, testFunctionConfig{ Cases: []testFunctionCase{ // String + list { `${concat("a", split(",", "b,c"))}`, - NewStringList([]string{"a", "b", "c"}).String(), + []interface{}{"a", "b", "c"}, false, }, // List + string { `${concat(split(",", "a,b"), "c")}`, - NewStringList([]string{"a", "b", "c"}).String(), + []interface{}{"a", "b", "c"}, false, }, // Single list { `${concat(split(",", ",foo,"))}`, - NewStringList([]string{"", "foo", ""}).String(), + []interface{}{"", "foo", ""}, false, }, { `${concat(split(",", "a,b,c"))}`, - NewStringList([]string{"a", "b", "c"}).String(), + []interface{}{"a", "b", "c"}, false, }, // Two lists { `${concat(split(",", "a,b,c"), split(",", "d,e"))}`, - NewStringList([]string{"a", "b", "c", "d", "e"}).String(), + []interface{}{"a", "b", "c", "d", "e"}, false, }, // Two lists with different separators { `${concat(split(",", "a,b,c"), split(" ", "d e"))}`, - NewStringList([]string{"a", "b", "c", "d", "e"}).String(), + []interface{}{"a", "b", "c", "d", "e"}, false, }, // More lists { `${concat(split(",", "a,b"), split(",", "c,d"), split(",", "e,f"), split(",", "0,1"))}`, - NewStringList([]string{"a", "b", "c", "d", "e", "f", "0", "1"}).String(), + []interface{}{"a", "b", "c", "d", "e", "f", "0", "1"}, false, }, }, @@ -338,7 +314,7 @@ func TestInterpolateFuncFormatList(t *testing.T) { // formatlist applies to each list element in turn { `${formatlist("<%s>", split(",", "A,B"))}`, - NewStringList([]string{"", ""}).String(), + []interface{}{"", ""}, false, }, // formatlist repeats scalar elements @@ -362,7 +338,7 @@ func TestInterpolateFuncFormatList(t *testing.T) { // Works with lists of length 1 [GH-2240] { `${formatlist("%s.id", split(",", "demo-rest-elb"))}`, - NewStringList([]string{"demo-rest-elb.id"}).String(), + []interface{}{"demo-rest-elb.id"}, false, }, }, @@ -371,6 +347,11 @@ func TestInterpolateFuncFormatList(t *testing.T) { func TestInterpolateFuncIndex(t *testing.T) { testFunction(t, testFunctionConfig{ + Vars: map[string]ast.Variable{ + "var.list1": interfaceToVariableSwallowError([]string{"notfoo", "stillnotfoo", "bar"}), + "var.list2": interfaceToVariableSwallowError([]string{"foo"}), + "var.list3": interfaceToVariableSwallowError([]string{"foo", "spam", "bar", "eggs"}), + }, Cases: []testFunctionCase{ { `${index("test", "")}`, @@ -379,22 +360,19 @@ func TestInterpolateFuncIndex(t *testing.T) { }, { - fmt.Sprintf(`${index("%s", "foo")}`, - NewStringList([]string{"notfoo", "stillnotfoo", "bar"}).String()), + `${index(var.list1, "foo")}`, nil, true, }, { - fmt.Sprintf(`${index("%s", "foo")}`, - NewStringList([]string{"foo"}).String()), + `${index(var.list2, "foo")}`, "0", false, }, { - fmt.Sprintf(`${index("%s", "bar")}`, - NewStringList([]string{"foo", "spam", "bar", "eggs"}).String()), + `${index(var.list3, "bar")}`, "2", false, }, @@ -404,6 +382,10 @@ func TestInterpolateFuncIndex(t *testing.T) { func TestInterpolateFuncJoin(t *testing.T) { testFunction(t, testFunctionConfig{ + Vars: map[string]ast.Variable{ + "var.a_list": interfaceToVariableSwallowError([]string{"foo"}), + "var.a_longer_list": interfaceToVariableSwallowError([]string{"foo", "bar", "baz"}), + }, Cases: []testFunctionCase{ { `${join(",")}`, @@ -412,24 +394,13 @@ func TestInterpolateFuncJoin(t *testing.T) { }, { - fmt.Sprintf(`${join(",", "%s")}`, - NewStringList([]string{"foo"}).String()), + `${join(",", var.a_list)}`, "foo", false, }, - /* - TODO - { - `${join(",", "foo", "bar")}`, - "foo,bar", - false, - }, - */ - { - fmt.Sprintf(`${join(".", "%s")}`, - NewStringList([]string{"foo", "bar", "baz"}).String()), + `${join(".", var.a_longer_list)}`, "foo.bar.baz", false, }, @@ -632,37 +603,37 @@ func TestInterpolateFuncSplit(t *testing.T) { { `${split(",", "")}`, - NewStringList([]string{""}).String(), + []interface{}{""}, false, }, { `${split(",", "foo")}`, - NewStringList([]string{"foo"}).String(), + []interface{}{"foo"}, false, }, { `${split(",", ",,,")}`, - NewStringList([]string{"", "", "", ""}).String(), + []interface{}{"", "", "", ""}, false, }, { `${split(",", "foo,")}`, - NewStringList([]string{"foo", ""}).String(), + []interface{}{"foo", ""}, false, }, { `${split(",", ",foo,")}`, - NewStringList([]string{"", "foo", ""}).String(), + []interface{}{"", "foo", ""}, false, }, { `${split(".", "foo.bar.baz")}`, - NewStringList([]string{"foo", "bar", "baz"}).String(), + []interface{}{"foo", "bar", "baz"}, false, }, }, @@ -810,43 +781,47 @@ func TestInterpolateFuncValues(t *testing.T) { }) } +func interfaceToVariableSwallowError(input interface{}) ast.Variable { + variable, _ := hil.InterfaceToVariable(input) + return 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"}), + }, Cases: []testFunctionCase{ { - fmt.Sprintf(`${element("%s", "1")}`, - NewStringList([]string{"foo", "baz"}).String()), + `${element(var.a_list, "1")}`, "baz", false, }, { - fmt.Sprintf(`${element("%s", "0")}`, - NewStringList([]string{"foo"}).String()), + `${element(var.a_short_list, "0")}`, "foo", false, }, // Invalid index should wrap vs. out-of-bounds { - fmt.Sprintf(`${element("%s", "2")}`, - NewStringList([]string{"foo", "baz"}).String()), + `${element(var.a_list, "2")}`, "foo", false, }, // Negative number should fail { - fmt.Sprintf(`${element("%s", "-1")}`, - NewStringList([]string{"foo"}).String()), + `${element(var.a_short_list, "-1")}`, nil, true, }, // Too many args { - fmt.Sprintf(`${element("%s", "0", "2")}`, - NewStringList([]string{"foo", "baz"}).String()), + `${element(var.a_list, "0", "2")}`, nil, true, }, diff --git a/config/interpolate_walk.go b/config/interpolate_walk.go index eb5c2480a..143b96131 100644 --- a/config/interpolate_walk.go +++ b/config/interpolate_walk.go @@ -150,12 +150,15 @@ 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 && IsStringList(replaceVal.(string)) { - parts := StringList(replaceVal.(string)).Slice() - for _, p := range parts { - if p == UnknownVariableValue { + if w.loc == reflectwalk.SliceElem { + switch typedReplaceVal := replaceVal.(type) { + case string: + if typedReplaceVal == UnknownVariableValue { + remove = true + } + case []interface{}: + if hasUnknownValue(typedReplaceVal) { remove = true - break } } } else if replaceVal == UnknownVariableValue { @@ -226,63 +229,63 @@ func (w *interpolationWalker) replaceCurrent(v reflect.Value) { } } +func hasUnknownValue(variable []interface{}) bool { + for _, value := range variable { + if strVal, ok := value.(string); ok { + if strVal == UnknownVariableValue { + return true + } + } + } + return false +} + func (w *interpolationWalker) splitSlice() { - // Get the []interface{} slice so we can do some operations on - // it without dealing with reflection. We'll document each step - // here to be clear. - var s []interface{} raw := w.cs[len(w.cs)-1] + + var s []interface{} switch v := raw.Interface().(type) { case []interface{}: s = v case []map[string]interface{}: return - default: - panic("Unknown kind: " + raw.Kind().String()) } - // Check if we have any elements that we need to split. If not, then - // just return since we're done. split := false - for _, v := range s { - sv, ok := v.(string) - if !ok { - continue - } - if IsStringList(sv) { + for _, val := range s { + if varVal, ok := val.(ast.Variable); ok && varVal.Type == ast.TypeList { + split = true + } + if _, ok := val.([]interface{}); ok { split = true - break } } + if !split { return } - // Make a new result slice that is twice the capacity to fit our growth. - result := make([]interface{}, 0, len(s)*2) - - // Go over each element of the original slice and start building up - // the resulting slice by splitting where we have to. + result := make([]interface{}, 0) for _, v := range s { - sv, ok := v.(string) - if !ok { - // Not a string, so just set it - result = append(result, v) - continue - } - - if IsStringList(sv) { - for _, p := range StringList(sv).Slice() { - result = append(result, p) + switch val := v.(type) { + case ast.Variable: + switch val.Type { + case ast.TypeList: + elements := val.Value.([]ast.Variable) + for _, element := range elements { + result = append(result, element.Value) + } + default: + result = append(result, val.Value) } - continue + case []interface{}: + for _, element := range val { + result = append(result, element) + } + default: + result = append(result, v) } - - // Not a string list, so just set it - result = append(result, sv) } - // Our slice is now done, we have to replace the slice now - // with this new one that we have. w.replaceCurrent(reflect.ValueOf(result)) } diff --git a/config/interpolate_walk_test.go b/config/interpolate_walk_test.go index e00eefe72..70067a99c 100644 --- a/config/interpolate_walk_test.go +++ b/config/interpolate_walk_test.go @@ -109,7 +109,7 @@ func TestInterpolationWalker_replace(t *testing.T) { cases := []struct { Input interface{} Output interface{} - Value string + Value interface{} }{ { Input: map[string]interface{}{ @@ -159,7 +159,7 @@ func TestInterpolationWalker_replace(t *testing.T) { "bing", }, }, - Value: NewStringList([]string{"bar", "baz"}).String(), + Value: []interface{}{"bar", "baz"}, }, { @@ -170,7 +170,7 @@ func TestInterpolationWalker_replace(t *testing.T) { }, }, Output: map[string]interface{}{}, - Value: NewStringList([]string{UnknownVariableValue, "baz"}).String(), + Value: []interface{}{UnknownVariableValue, "baz"}, }, } diff --git a/config/test-fixtures/validate-module-var-list/main.tf b/config/test-fixtures/validate-module-var-list/main.tf new file mode 100644 index 000000000..e3a97b7de --- /dev/null +++ b/config/test-fixtures/validate-module-var-list/main.tf @@ -0,0 +1,4 @@ +module "foo" { + source = "./foo" + nodes = [1,2,3] +} diff --git a/config/test-fixtures/validate-module-var-map/main.tf b/config/test-fixtures/validate-module-var-map/main.tf index e3a97b7de..b42ff010d 100644 --- a/config/test-fixtures/validate-module-var-map/main.tf +++ b/config/test-fixtures/validate-module-var-map/main.tf @@ -1,4 +1,7 @@ module "foo" { source = "./foo" - nodes = [1,2,3] + nodes = { + key1 = "value1" + key2 = "value2" + } } diff --git a/config/test-fixtures/validate-var-default-bad-type/main.tf b/config/test-fixtures/validate-var-default-list-type/main.tf similarity index 100% rename from config/test-fixtures/validate-var-default-bad-type/main.tf rename to config/test-fixtures/validate-var-default-list-type/main.tf diff --git a/helper/schema/field_reader_config_test.go b/helper/schema/field_reader_config_test.go index 2defb0e02..ba7c0f1a6 100644 --- a/helper/schema/field_reader_config_test.go +++ b/helper/schema/field_reader_config_test.go @@ -305,6 +305,7 @@ func testConfigInterpolate( t *testing.T, raw map[string]interface{}, vs map[string]ast.Variable) *terraform.ResourceConfig { + rc, err := config.NewRawConfig(raw) if err != nil { t.Fatalf("err: %s", err) diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index dba10ee34..2ec1aa78a 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -8,6 +8,7 @@ import ( "strconv" "testing" + "github.com/hashicorp/hil" "github.com/hashicorp/hil/ast" "github.com/hashicorp/terraform/config" "github.com/hashicorp/terraform/helper/hashcode" @@ -123,12 +124,17 @@ func TestValueType_Zero(t *testing.T) { } } +func interfaceToVariableSwallowError(input interface{}) ast.Variable { + variable, _ := hil.InterfaceToVariable(input) + return variable +} + func TestSchemaMap_Diff(t *testing.T) { cases := map[string]struct { Schema map[string]*Schema State *terraform.InstanceState Config map[string]interface{} - ConfigVariables map[string]string + ConfigVariables map[string]ast.Variable Diff *terraform.InstanceDiff Err bool }{ @@ -396,8 +402,8 @@ func TestSchemaMap_Diff(t *testing.T) { "availability_zone": "${var.foo}", }, - ConfigVariables: map[string]string{ - "var.foo": "bar", + ConfigVariables: map[string]ast.Variable{ + "var.foo": interfaceToVariableSwallowError("bar"), }, Diff: &terraform.InstanceDiff{ @@ -426,8 +432,8 @@ func TestSchemaMap_Diff(t *testing.T) { "availability_zone": "${var.foo}", }, - ConfigVariables: map[string]string{ - "var.foo": config.UnknownVariableValue, + ConfigVariables: map[string]ast.Variable{ + "var.foo": interfaceToVariableSwallowError(config.UnknownVariableValue), }, Diff: &terraform.InstanceDiff{ @@ -576,8 +582,8 @@ func TestSchemaMap_Diff(t *testing.T) { "ports": []interface{}{1, "${var.foo}"}, }, - ConfigVariables: map[string]string{ - "var.foo": config.NewStringList([]string{"2", "5"}).String(), + ConfigVariables: map[string]ast.Variable{ + "var.foo": interfaceToVariableSwallowError([]interface{}{"2", "5"}), }, Diff: &terraform.InstanceDiff{ @@ -619,9 +625,9 @@ func TestSchemaMap_Diff(t *testing.T) { "ports": []interface{}{1, "${var.foo}"}, }, - ConfigVariables: map[string]string{ - "var.foo": config.NewStringList([]string{ - config.UnknownVariableValue, "5"}).String(), + ConfigVariables: map[string]ast.Variable{ + "var.foo": interfaceToVariableSwallowError([]interface{}{ + config.UnknownVariableValue, "5"}), }, Diff: &terraform.InstanceDiff{ @@ -886,8 +892,8 @@ func TestSchemaMap_Diff(t *testing.T) { "ports": []interface{}{"${var.foo}", 1}, }, - ConfigVariables: map[string]string{ - "var.foo": config.NewStringList([]string{"2", "5"}).String(), + ConfigVariables: map[string]ast.Variable{ + "var.foo": interfaceToVariableSwallowError([]interface{}{"2", "5"}), }, Diff: &terraform.InstanceDiff{ @@ -932,9 +938,9 @@ func TestSchemaMap_Diff(t *testing.T) { "ports": []interface{}{1, "${var.foo}"}, }, - ConfigVariables: map[string]string{ - "var.foo": config.NewStringList([]string{ - config.UnknownVariableValue, "5"}).String(), + ConfigVariables: map[string]ast.Variable{ + "var.foo": interfaceToVariableSwallowError([]interface{}{ + config.UnknownVariableValue, "5"}), }, Diff: &terraform.InstanceDiff{ @@ -1603,8 +1609,8 @@ func TestSchemaMap_Diff(t *testing.T) { "instances": []interface{}{"${var.foo}"}, }, - ConfigVariables: map[string]string{ - "var.foo": config.UnknownVariableValue, + ConfigVariables: map[string]ast.Variable{ + "var.foo": interfaceToVariableSwallowError(config.UnknownVariableValue), }, Diff: &terraform.InstanceDiff{ @@ -1654,8 +1660,8 @@ func TestSchemaMap_Diff(t *testing.T) { }, }, - ConfigVariables: map[string]string{ - "var.foo": config.UnknownVariableValue, + ConfigVariables: map[string]ast.Variable{ + "var.foo": interfaceToVariableSwallowError(config.UnknownVariableValue), }, Diff: &terraform.InstanceDiff{ @@ -1720,8 +1726,8 @@ func TestSchemaMap_Diff(t *testing.T) { }, }, - ConfigVariables: map[string]string{ - "var.foo": config.UnknownVariableValue, + ConfigVariables: map[string]ast.Variable{ + "var.foo": interfaceToVariableSwallowError(config.UnknownVariableValue), }, Diff: &terraform.InstanceDiff{ @@ -1787,8 +1793,8 @@ func TestSchemaMap_Diff(t *testing.T) { }, }, - ConfigVariables: map[string]string{ - "var.foo": config.UnknownVariableValue, + ConfigVariables: map[string]ast.Variable{ + "var.foo": interfaceToVariableSwallowError(config.UnknownVariableValue), }, Diff: &terraform.InstanceDiff{ @@ -2134,8 +2140,8 @@ func TestSchemaMap_Diff(t *testing.T) { "ports": []interface{}{1, "${var.foo}32"}, }, - ConfigVariables: map[string]string{ - "var.foo": config.UnknownVariableValue, + ConfigVariables: map[string]ast.Variable{ + "var.foo": interfaceToVariableSwallowError(config.UnknownVariableValue), }, Diff: &terraform.InstanceDiff{ @@ -2403,12 +2409,7 @@ func TestSchemaMap_Diff(t *testing.T) { } if len(tc.ConfigVariables) > 0 { - vars := make(map[string]ast.Variable) - for k, v := range tc.ConfigVariables { - vars[k] = ast.Variable{Value: v, Type: ast.TypeString} - } - - if err := c.Interpolate(vars); err != nil { + if err := c.Interpolate(tc.ConfigVariables); err != nil { t.Fatalf("#%q err: %s", tn, err) } } @@ -2420,7 +2421,7 @@ func TestSchemaMap_Diff(t *testing.T) { } if !reflect.DeepEqual(tc.Diff, d) { - t.Fatalf("#%q:\n\nexpected: %#v\n\ngot:\n\n%#v", tn, tc.Diff, d) + t.Fatalf("#%q:\n\nexpected:\n%#v\n\ngot:\n%#v", tn, tc.Diff, d) } } } diff --git a/terraform/context.go b/terraform/context.go index 90947aebf..8ddad3331 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -225,6 +225,8 @@ func (c *Context) Input(mode InputMode) error { continue case config.VariableTypeMap: continue + case config.VariableTypeList: + continue case config.VariableTypeString: // Good! default: diff --git a/terraform/eval_variable.go b/terraform/eval_variable.go index f5f60f0e9..40e953d7c 100644 --- a/terraform/eval_variable.go +++ b/terraform/eval_variable.go @@ -60,6 +60,10 @@ func (n *EvalTypeCheckVariable) Eval(ctx EvalContext) (interface{}, error) { continue } + if proposedValue == config.UnknownVariableValue { + continue + } + switch declaredType { case config.VariableTypeString: // This will need actual verification once we aren't dealing with @@ -80,6 +84,14 @@ func (n *EvalTypeCheckVariable) Eval(ctx EvalContext) (interface{}, error) { return nil, fmt.Errorf("variable %s%s should be type %s, got %T", name, modulePathDescription, declaredType.Printable(), proposedValue) } + case config.VariableTypeList: + switch proposedValue.(type) { + case []interface{}: + continue + default: + return nil, fmt.Errorf("variable %s%s should be type %s, got %T", + name, modulePathDescription, declaredType.Printable(), proposedValue) + } default: // This will need the actual type substituting when we have more than // just strings and maps. diff --git a/terraform/interpolate.go b/terraform/interpolate.go index 66f36246c..81c6fc21e 100644 --- a/terraform/interpolate.go +++ b/terraform/interpolate.go @@ -63,7 +63,7 @@ func (i *Interpolater) Values( if err != nil { return nil, fmt.Errorf("invalid default map value for %s: %v", v.Name, v.Default) } - // Potentially TODO(jen20): check against declared type + result[n] = variable } } @@ -230,21 +230,26 @@ func (i *Interpolater) valueResourceVar( return nil } - var attr string - var err error if v.Multi && v.Index == -1 { - attr, err = i.computeResourceMultiVariable(scope, v) + variable, err := i.computeResourceMultiVariable(scope, v) + if err != nil { + return err + } + if variable == nil { + return fmt.Errorf("no error reported by variable %q is nil", v.Name) + } + result[n] = *variable } else { - attr, err = i.computeResourceVariable(scope, v) - } - if err != nil { - return err + variable, err := i.computeResourceVariable(scope, v) + if err != nil { + return err + } + if variable == nil { + return fmt.Errorf("no error reported by variable %q is nil", v.Name) + } + result[n] = *variable } - result[n] = ast.Variable{ - Value: attr, - Type: ast.TypeString, - } return nil } @@ -334,7 +339,7 @@ func (i *Interpolater) valueUserVar( func (i *Interpolater) computeResourceVariable( scope *InterpolationScope, - v *config.ResourceVariable) (string, error) { + v *config.ResourceVariable) (*ast.Variable, error) { id := v.ResourceId() if v.Multi { id = fmt.Sprintf("%s.%d", id, v.Index) @@ -343,16 +348,18 @@ func (i *Interpolater) computeResourceVariable( i.StateLock.RLock() defer i.StateLock.RUnlock() + unknownVariable := unknownVariable() + // Get the information about this resource variable, and verify // that it exists and such. module, _, err := i.resourceVariableInfo(scope, v) if err != nil { - return "", err + return nil, err } // If we have no module in the state yet or count, return empty if module == nil || len(module.Resources) == 0 { - return "", nil + return nil, nil } // Get the resource out from the state. We know the state exists @@ -374,12 +381,13 @@ func (i *Interpolater) computeResourceVariable( } if attr, ok := r.Primary.Attributes[v.Field]; ok { - return attr, nil + return &ast.Variable{Type: ast.TypeString, Value: attr}, nil } // computed list attribute if _, ok := r.Primary.Attributes[v.Field+".#"]; ok { - return i.interpolateListAttribute(v.Field, r.Primary.Attributes) + variable, err := i.interpolateListAttribute(v.Field, r.Primary.Attributes) + return &variable, err } // At apply time, we can't do the "maybe has it" check below @@ -402,13 +410,13 @@ func (i *Interpolater) computeResourceVariable( // Lists and sets make this key := fmt.Sprintf("%s.#", strings.Join(parts[:i], ".")) if attr, ok := r.Primary.Attributes[key]; ok { - return attr, nil + return &ast.Variable{Type: ast.TypeString, Value: attr}, nil } // Maps make this key = fmt.Sprintf("%s", strings.Join(parts[:i], ".")) if attr, ok := r.Primary.Attributes[key]; ok { - return attr, nil + return &ast.Variable{Type: ast.TypeString, Value: attr}, nil } } } @@ -418,7 +426,7 @@ MISSING: // semantic level. If we reached this point and don't have variables, // just return the computed value. if scope == nil && scope.Resource == nil { - return config.UnknownVariableValue, nil + return &unknownVariable, nil } // If the operation is refresh, it isn't an error for a value to @@ -432,10 +440,10 @@ MISSING: // For an input walk, computed values are okay to return because we're only // looking for missing variables to prompt the user for. if i.Operation == walkRefresh || i.Operation == walkPlanDestroy || i.Operation == walkDestroy || i.Operation == walkInput { - return config.UnknownVariableValue, nil + return &unknownVariable, nil } - return "", fmt.Errorf( + return nil, fmt.Errorf( "Resource '%s' does not have attribute '%s' "+ "for variable '%s'", id, @@ -445,21 +453,23 @@ MISSING: func (i *Interpolater) computeResourceMultiVariable( scope *InterpolationScope, - v *config.ResourceVariable) (string, error) { + v *config.ResourceVariable) (*ast.Variable, error) { i.StateLock.RLock() defer i.StateLock.RUnlock() + unknownVariable := unknownVariable() + // Get the information about this resource variable, and verify // that it exists and such. module, cr, err := i.resourceVariableInfo(scope, v) if err != nil { - return "", err + return nil, err } // Get the count so we know how many to iterate over count, err := cr.Count() if err != nil { - return "", fmt.Errorf( + return nil, fmt.Errorf( "Error reading %s count: %s", v.ResourceId(), err) @@ -467,7 +477,7 @@ func (i *Interpolater) computeResourceMultiVariable( // If we have no module in the state yet or count, return empty if module == nil || len(module.Resources) == 0 || count == 0 { - return "", nil + return &ast.Variable{Type: ast.TypeString, Value: ""}, nil } var values []string @@ -489,32 +499,37 @@ func (i *Interpolater) computeResourceMultiVariable( continue } - attr, ok := r.Primary.Attributes[v.Field] - if !ok { - // computed list attribute - _, ok := r.Primary.Attributes[v.Field+".#"] - if !ok { - continue + if singleAttr, ok := r.Primary.Attributes[v.Field]; ok { + if singleAttr == config.UnknownVariableValue { + return &unknownVariable, nil } - attr, err = i.interpolateListAttribute(v.Field, r.Primary.Attributes) - if err != nil { - return "", err - } - } - if config.IsStringList(attr) { - for _, s := range config.StringList(attr).Slice() { - values = append(values, s) - } + values = append(values, singleAttr) continue } - // If any value is unknown, the whole thing is unknown - if attr == config.UnknownVariableValue { - return config.UnknownVariableValue, nil + // computed list attribute + _, ok = r.Primary.Attributes[v.Field+".#"] + if !ok { + continue + } + multiAttr, err := i.interpolateListAttribute(v.Field, r.Primary.Attributes) + if err != nil { + return nil, err } - values = append(values, attr) + if multiAttr == unknownVariable { + return &ast.Variable{Type: ast.TypeString, Value: ""}, nil + } + + for _, element := range multiAttr.Value.([]ast.Variable) { + strVal := element.Value.(string) + if strVal == config.UnknownVariableValue { + return &unknownVariable, nil + } + + values = append(values, strVal) + } } if len(values) == 0 { @@ -529,10 +544,10 @@ func (i *Interpolater) computeResourceMultiVariable( // For an input walk, computed values are okay to return because we're only // looking for missing variables to prompt the user for. if i.Operation == walkRefresh || i.Operation == walkPlanDestroy || i.Operation == walkDestroy || i.Operation == walkInput { - return config.UnknownVariableValue, nil + return &unknownVariable, nil } - return "", fmt.Errorf( + return nil, fmt.Errorf( "Resource '%s' does not have attribute '%s' "+ "for variable '%s'", v.ResourceId(), @@ -540,12 +555,13 @@ func (i *Interpolater) computeResourceMultiVariable( v.FullKey()) } - return config.NewStringList(values).String(), nil + variable, err := hil.InterfaceToVariable(values) + return &variable, err } func (i *Interpolater) interpolateListAttribute( resourceID string, - attributes map[string]string) (string, error) { + attributes map[string]string) (ast.Variable, error) { attr := attributes[resourceID+".#"] log.Printf("[DEBUG] Interpolating computed list attribute %s (%s)", @@ -556,7 +572,7 @@ func (i *Interpolater) interpolateListAttribute( // unknown". We must honor that meaning here so computed references can be // treated properly during the plan phase. if attr == config.UnknownVariableValue { - return attr, nil + return unknownVariable(), nil } // Otherwise we gather the values from the list-like attribute and return @@ -570,7 +586,7 @@ func (i *Interpolater) interpolateListAttribute( } sort.Strings(members) - return config.NewStringList(members).String(), nil + return hil.InterfaceToVariable(members) } func (i *Interpolater) resourceVariableInfo( diff --git a/terraform/interpolate_test.go b/terraform/interpolate_test.go index afdbd5f37..e3777ae4a 100644 --- a/terraform/interpolate_test.go +++ b/terraform/interpolate_test.go @@ -7,6 +7,7 @@ import ( "sync" "testing" + "github.com/hashicorp/hil" "github.com/hashicorp/hil/ast" "github.com/hashicorp/terraform/config" ) @@ -210,6 +211,11 @@ func TestInterpolater_resourceVariableMulti(t *testing.T) { }) } +func interfaceToVariableSwallowError(input interface{}) ast.Variable { + variable, _ := hil.InterfaceToVariable(input) + return variable +} + func TestInterpolator_resourceMultiAttributes(t *testing.T) { lock := new(sync.RWMutex) state := &State{ @@ -251,31 +257,24 @@ func TestInterpolator_resourceMultiAttributes(t *testing.T) { Path: rootModulePath, } - name_servers := []string{ + name_servers := []interface{}{ "ns-1334.awsdns-38.org", "ns-1680.awsdns-18.co.uk", "ns-498.awsdns-62.com", "ns-601.awsdns-11.net", } - expectedNameServers := config.NewStringList(name_servers).String() // More than 1 element - testInterpolate(t, i, scope, "aws_route53_zone.yada.name_servers", ast.Variable{ - Value: expectedNameServers, - Type: ast.TypeString, - }) + testInterpolate(t, i, scope, "aws_route53_zone.yada.name_servers", + interfaceToVariableSwallowError(name_servers)) // Exactly 1 element - testInterpolate(t, i, scope, "aws_route53_zone.yada.listeners", ast.Variable{ - Value: config.NewStringList([]string{"red"}).String(), - Type: ast.TypeString, - }) + testInterpolate(t, i, scope, "aws_route53_zone.yada.listeners", + interfaceToVariableSwallowError([]interface{}{"red"})) // Zero elements - testInterpolate(t, i, scope, "aws_route53_zone.yada.nothing", ast.Variable{ - Value: config.NewStringList([]string{}).String(), - Type: ast.TypeString, - }) + testInterpolate(t, i, scope, "aws_route53_zone.yada.nothing", + interfaceToVariableSwallowError([]interface{}{})) // Maps still need to work testInterpolate(t, i, scope, "aws_route53_zone.yada.tags.Name", ast.Variable{ @@ -290,7 +289,7 @@ func TestInterpolator_resourceMultiAttributesWithResourceCount(t *testing.T) { Path: rootModulePath, } - name_servers := []string{ + name_servers := []interface{}{ "ns-1334.awsdns-38.org", "ns-1680.awsdns-18.co.uk", "ns-498.awsdns-62.com", @@ -302,50 +301,38 @@ func TestInterpolator_resourceMultiAttributesWithResourceCount(t *testing.T) { } // More than 1 element - expectedNameServers := config.NewStringList(name_servers[0:4]).String() - testInterpolate(t, i, scope, "aws_route53_zone.terra.0.name_servers", ast.Variable{ - Value: expectedNameServers, - Type: ast.TypeString, - }) + testInterpolate(t, i, scope, "aws_route53_zone.terra.0.name_servers", + interfaceToVariableSwallowError(name_servers[0:4])) + // More than 1 element in both - expectedNameServers = config.NewStringList(name_servers).String() - testInterpolate(t, i, scope, "aws_route53_zone.terra.*.name_servers", ast.Variable{ - Value: expectedNameServers, - Type: ast.TypeString, - }) + testInterpolate(t, i, scope, "aws_route53_zone.terra.*.name_servers", + interfaceToVariableSwallowError(name_servers)) // Exactly 1 element - testInterpolate(t, i, scope, "aws_route53_zone.terra.0.listeners", ast.Variable{ - Value: config.NewStringList([]string{"red"}).String(), - Type: ast.TypeString, - }) + testInterpolate(t, i, scope, "aws_route53_zone.terra.0.listeners", + interfaceToVariableSwallowError([]interface{}{"red"})) + // Exactly 1 element in both - testInterpolate(t, i, scope, "aws_route53_zone.terra.*.listeners", ast.Variable{ - Value: config.NewStringList([]string{"red", "blue"}).String(), - Type: ast.TypeString, - }) + testInterpolate(t, i, scope, "aws_route53_zone.terra.*.listeners", + interfaceToVariableSwallowError([]interface{}{"red", "blue"})) // Zero elements - testInterpolate(t, i, scope, "aws_route53_zone.terra.0.nothing", ast.Variable{ - Value: config.NewStringList([]string{}).String(), - Type: ast.TypeString, - }) + testInterpolate(t, i, scope, "aws_route53_zone.terra.0.nothing", + interfaceToVariableSwallowError([]interface{}{})) + // Zero + 1 element - testInterpolate(t, i, scope, "aws_route53_zone.terra.*.special", ast.Variable{ - Value: config.NewStringList([]string{"extra"}).String(), - Type: ast.TypeString, - }) + testInterpolate(t, i, scope, "aws_route53_zone.terra.*.special", + interfaceToVariableSwallowError([]interface{}{"extra"})) // Maps still need to work testInterpolate(t, i, scope, "aws_route53_zone.terra.0.tags.Name", ast.Variable{ Value: "reindeer", Type: ast.TypeString, }) + // Maps still need to work in both - testInterpolate(t, i, scope, "aws_route53_zone.terra.*.tags.Name", ast.Variable{ - Value: config.NewStringList([]string{"reindeer", "white-hart"}).String(), - Type: ast.TypeString, - }) + testInterpolate(t, i, scope, "aws_route53_zone.terra.*.tags.Name", + interfaceToVariableSwallowError([]interface{}{"reindeer", "white-hart"})) } func TestInterpolator_resourceMultiAttributesComputed(t *testing.T) {