From 01be1a5ecd4a9788bd540cc1287b7d52a9cfb292 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 23 Nov 2016 13:43:56 -0500 Subject: [PATCH] Check for interpolated values when reading a map Accessing an interpolated value in a map through ConfigFieldReader can fail, because GetRaw can't access interpolated values, so check if the value exists at all by looking in the config. If the config has a value, assume our map's value is interpolated and proceed as such. We also need to lookup the correct schema to properly read a field from a nested structure. - Maps previously always defaulted to TypeString. Now check if Elem is a ValueType and use that if applicable - Lists now return the schema for nested element types, defaulting to a TypeString like maps. This only allows maps and lists to be nested one level deep, and the inner map or list must only contain string values. --- helper/schema/field_reader.go | 38 ++++- helper/schema/field_reader_config.go | 13 +- helper/schema/field_reader_config_test.go | 178 +++++++++++++++++++--- 3 files changed, 204 insertions(+), 25 deletions(-) diff --git a/helper/schema/field_reader.go b/helper/schema/field_reader.go index 5606e6d8b..1660a6702 100644 --- a/helper/schema/field_reader.go +++ b/helper/schema/field_reader.go @@ -75,6 +75,8 @@ func addrToSchema(addr []string, schemaMap map[string]*Schema) []*Schema { return nil } case TypeList, TypeSet: + isIndex := len(addr) > 0 && addr[0] == "#" + switch v := current.Elem.(type) { case *Resource: current = &Schema{ @@ -83,20 +85,52 @@ func addrToSchema(addr []string, schemaMap map[string]*Schema) []*Schema { } case *Schema: current = v + case ValueType: + current = &Schema{Type: v} default: + // we may not know the Elem type and are just looking for the + // index + if isIndex { + break + } + + if len(addr) == 0 { + // we've processed the address, so return what we've + // collected + return result + } + + if len(addr) == 1 { + if _, err := strconv.Atoi(addr[0]); err == nil { + // we're indexing a value without a schema. This can + // happen if the list is nested in another schema type. + // Default to a TypeString like we do with a map + current = &Schema{Type: TypeString} + break + } + } + return nil } // If we only have one more thing and the next thing // is a #, then we're accessing the index which is always // an int. - if len(addr) > 0 && addr[0] == "#" { + if isIndex { current = &Schema{Type: TypeInt} break } + case TypeMap: if len(addr) > 0 { - current = &Schema{Type: TypeString} + switch v := current.Elem.(type) { + case ValueType: + current = &Schema{Type: v} + default: + // maps default to string values. This is all we can have + // if this is nested in another list or map. + current = &Schema{Type: TypeString} + } } case typeObject: // If we're already in the object, then we want to handle Sets diff --git a/helper/schema/field_reader_config.go b/helper/schema/field_reader_config.go index 042fe9d0e..53ff5208f 100644 --- a/helper/schema/field_reader_config.go +++ b/helper/schema/field_reader_config.go @@ -104,7 +104,18 @@ func (r *ConfigFieldReader) readMap(k string, schema *Schema) (FieldReadResult, // the type of the raw value. mraw, ok := r.Config.GetRaw(k) if !ok { - return FieldReadResult{}, nil + // check if this is from an interpolated field by seeing if it exists + // in the config + _, ok := r.Config.Get(k) + if !ok { + // this really doesn't exist + return FieldReadResult{}, nil + } + + // We couldn't fetch the value from a nested data structure, so treat the + // raw value as an interpolation string. The mraw value is only used + // for the type switch below. + mraw = "${INTERPOLATED}" } result := make(map[string]interface{}) diff --git a/helper/schema/field_reader_config_test.go b/helper/schema/field_reader_config_test.go index 0b68cce22..5db643d75 100644 --- a/helper/schema/field_reader_config_test.go +++ b/helper/schema/field_reader_config_test.go @@ -219,15 +219,27 @@ func TestConfigFieldReader_ComputedMap(t *testing.T) { Type: TypeMap, Computed: true, }, + "listmap": &Schema{ + Type: TypeMap, + Computed: true, + Elem: TypeList, + }, + "maplist": &Schema{ + Type: TypeList, + Computed: true, + Elem: TypeMap, + }, } - cases := map[string]struct { + cases := []struct { + Name string Addr []string Result FieldReadResult Config *terraform.ResourceConfig Err bool }{ - "set, normal": { + { + "set, normal", []string{"map"}, FieldReadResult{ Value: map[string]interface{}{ @@ -244,7 +256,8 @@ func TestConfigFieldReader_ComputedMap(t *testing.T) { false, }, - "computed element": { + { + "computed element", []string{"map"}, FieldReadResult{ Exists: true, @@ -263,7 +276,8 @@ func TestConfigFieldReader_ComputedMap(t *testing.T) { false, }, - "native map": { + { + "native map", []string{"map"}, FieldReadResult{ Value: map[string]interface{}{ @@ -292,27 +306,147 @@ func TestConfigFieldReader_ComputedMap(t *testing.T) { }), false, }, + + { + "map-from-list-of-maps", + []string{"maplist", "0"}, + FieldReadResult{ + Value: map[string]interface{}{ + "key": "bar", + }, + Exists: true, + Computed: false, + }, + testConfigInterpolate(t, map[string]interface{}{ + "maplist": "${var.foo}", + }, map[string]ast.Variable{ + "var.foo": ast.Variable{ + Type: ast.TypeList, + Value: []ast.Variable{ + { + Type: ast.TypeMap, + Value: map[string]ast.Variable{ + "key": ast.Variable{ + Type: ast.TypeString, + Value: "bar", + }, + }, + }, + }, + }, + }), + false, + }, + + { + "value-from-list-of-maps", + []string{"maplist", "0", "key"}, + FieldReadResult{ + Value: "bar", + Exists: true, + Computed: false, + }, + testConfigInterpolate(t, map[string]interface{}{ + "maplist": "${var.foo}", + }, map[string]ast.Variable{ + "var.foo": ast.Variable{ + Type: ast.TypeList, + Value: []ast.Variable{ + { + Type: ast.TypeMap, + Value: map[string]ast.Variable{ + "key": ast.Variable{ + Type: ast.TypeString, + Value: "bar", + }, + }, + }, + }, + }, + }), + false, + }, + + { + "list-from-map-of-lists", + []string{"listmap", "key"}, + FieldReadResult{ + Value: []interface{}{"bar"}, + Exists: true, + Computed: false, + }, + testConfigInterpolate(t, map[string]interface{}{ + "listmap": "${var.foo}", + }, map[string]ast.Variable{ + "var.foo": ast.Variable{ + Type: ast.TypeMap, + Value: map[string]ast.Variable{ + "key": ast.Variable{ + Type: ast.TypeList, + Value: []ast.Variable{ + ast.Variable{ + Type: ast.TypeString, + Value: "bar", + }, + }, + }, + }, + }, + }), + false, + }, + + { + "value-from-map-of-lists", + []string{"listmap", "key", "0"}, + FieldReadResult{ + Value: "bar", + Exists: true, + Computed: false, + }, + testConfigInterpolate(t, map[string]interface{}{ + "listmap": "${var.foo}", + }, map[string]ast.Variable{ + "var.foo": ast.Variable{ + Type: ast.TypeMap, + Value: map[string]ast.Variable{ + "key": ast.Variable{ + Type: ast.TypeList, + Value: []ast.Variable{ + ast.Variable{ + Type: ast.TypeString, + Value: "bar", + }, + }, + }, + }, + }, + }), + false, + }, } - for name, tc := range cases { - r := &ConfigFieldReader{ - Schema: schema, - Config: tc.Config, - } - out, err := r.ReadField(tc.Addr) - if err != nil != tc.Err { - t.Fatalf("%s: err: %s", name, err) - } - if s, ok := out.Value.(*Set); ok { - // If it is a set, convert to the raw map - out.Value = s.m - if len(s.m) == 0 { - out.Value = nil + for i, tc := range cases { + t.Run(fmt.Sprintf("%d-%s", i, tc.Name), func(t *testing.T) { + r := &ConfigFieldReader{ + Schema: schema, + Config: tc.Config, } - } - if !reflect.DeepEqual(tc.Result, out) { - t.Fatalf("%s: bad: %#v", name, out) - } + out, err := r.ReadField(tc.Addr) + if err != nil != tc.Err { + t.Fatal(err) + } + if s, ok := out.Value.(*Set); ok { + // If it is a set, convert to the raw map + out.Value = s.m + if len(s.m) == 0 { + out.Value = nil + } + } + if !reflect.DeepEqual(tc.Result, out) { + t.Fatalf("\nexpected: %#v\ngot: %#v", tc.Result, out) + } + }) } }