From b4bf8131516c80a0b3b980924afde5d510458935 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Thu, 8 Jan 2015 18:02:19 -0800 Subject: [PATCH] helper/schema: too big to fail --- helper/schema/field_reader.go | 51 +++-- helper/schema/field_reader_config.go | 131 +++++++++++-- helper/schema/field_reader_config_test.go | 3 + helper/schema/field_reader_diff.go | 10 +- helper/schema/field_reader_diff_test.go | 20 +- helper/schema/field_reader_map.go | 124 +++++++++--- helper/schema/field_reader_map_test.go | 19 +- helper/schema/field_reader_multi.go | 23 +-- helper/schema/field_reader_multi_test.go | 32 ++-- helper/schema/field_reader_test.go | 135 ++++++++++++- helper/schema/resource_data.go | 150 +++++++++++---- helper/schema/resource_data_test.go | 223 ++-------------------- helper/schema/schema.go | 32 +++- helper/schema/schema_test.go | 25 ++- 14 files changed, 639 insertions(+), 339 deletions(-) diff --git a/helper/schema/field_reader.go b/helper/schema/field_reader.go index ce5f38543..911ff219f 100644 --- a/helper/schema/field_reader.go +++ b/helper/schema/field_reader.go @@ -19,8 +19,8 @@ type FieldReadResult struct { // or the items that should be removed (if they existed). NegValue // doesn't make sense for primitives but is important for any // container types such as maps, sets, lists. - Value interface{} - NegValue interface{} + Value interface{} + ValueProcessed interface{} // Exists is true if the field was found in the data. False means // it wasn't found if there was no error. @@ -32,26 +32,28 @@ type FieldReadResult struct { } // addrToSchema finds the final element schema for the given address -// and the given schema. -func addrToSchema(addr []string, schemaMap map[string]*Schema) *Schema { - var result *Schema - var lastType ValueType +// and the given schema. It returns all the schemas that led to the final +// schema. These are in order of the address (out to in). +func addrToSchema(addr []string, schemaMap map[string]*Schema) []*Schema { current := &Schema{ Type: typeObject, Elem: schemaMap, } + result := make([]*Schema, 0, len(addr)) for len(addr) > 0 { k := addr[0] addr = addr[1:] REPEAT: - if len(addr) == 0 { - result = current + // We want to trim off the first "typeObject" since its not a + // real lookup that people do. i.e. []string{"foo"} in a structure + // isn't {typeObject, typeString}, its just a {typeString}. + if len(result) > 0 || current.Type != typeObject { + result = append(result, current) } - currentType := current.Type - switch current.Type { + switch t := current.Type; t { case TypeBool: fallthrough case TypeInt: @@ -75,18 +77,33 @@ func addrToSchema(addr []string, schemaMap map[string]*Schema) *Schema { return nil } + // If we only have one more thing and the the next thing + // is a #, then we're accessing the index which is always + // an int. if len(addr) > 0 && addr[0] == "#" { current = &Schema{Type: TypeInt} + break } case TypeMap: if len(addr) > 0 { current = &Schema{Type: TypeString} } case typeObject: - if lastType == TypeSet || lastType == TypeList { - // We just ignore sets/lists since they don't access - // objects the same way. - break + // If we're already in the object, then we want to handle Sets + // and Lists specially. Basically, their next key is the lookup + // key (the set value or the list element). For these scenarios, + // we just want to skip it and move to the next element if there + // is one. + if len(result) > 0 { + lastType := result[len(result)-2].Type + if lastType == TypeSet || lastType == TypeList { + if len(addr) == 0 { + break + } + + k = addr[0] + addr = addr[1:] + } } m := current.Elem.(map[string]*Schema) @@ -98,8 +115,6 @@ func addrToSchema(addr []string, schemaMap map[string]*Schema) *Schema { current = val goto REPEAT } - - lastType = currentType } return result @@ -129,7 +144,7 @@ func readListField( if countResult.Computed || countResult.Value.(int) == 0 { return FieldReadResult{ Value: []interface{}{}, - Exists: true, + Exists: countResult.Exists, Computed: countResult.Computed, }, nil } @@ -184,7 +199,7 @@ func readObjectField( return FieldReadResult{ Value: result, - Exists: true, + Exists: len(schema) > 0 && len(result) > 0, }, nil } diff --git a/helper/schema/field_reader_config.go b/helper/schema/field_reader_config.go index 167d8bf71..215c0957e 100644 --- a/helper/schema/field_reader_config.go +++ b/helper/schema/field_reader_config.go @@ -2,7 +2,9 @@ package schema import ( "fmt" + "strconv" "strings" + "sync" "github.com/hashicorp/terraform/terraform" "github.com/mitchellh/mapstructure" @@ -13,15 +15,68 @@ import ( type ConfigFieldReader struct { Config *terraform.ResourceConfig Schema map[string]*Schema + + lock sync.Mutex } func (r *ConfigFieldReader) ReadField(address []string) (FieldReadResult, error) { - k := strings.Join(address, ".") - schema := addrToSchema(address, r.Schema) - if schema == nil { + return r.readField(address, false) +} + +func (r *ConfigFieldReader) readField( + address []string, nested bool) (FieldReadResult, error) { + schemaList := addrToSchema(address, r.Schema) + if len(schemaList) == 0 { return FieldReadResult{}, nil } + if !nested { + // If we have a set anywhere in the address, then we need to + // read that set out in order and actually replace that part of + // the address with the real list index. i.e. set.50 might actually + // map to set.12 in the config, since it is in list order in the + // config, not indexed by set value. + for i, v := range schemaList { + // Sets are the only thing that cause this issue. + if v.Type != TypeSet { + continue + } + + // If we're at the end of the list, then we don't have to worry + // about this because we're just requesting the whole set. + if i == len(schemaList)-1 { + continue + } + + // If we're looking for the count, then ignore... + if address[i+1] == "#" { + continue + } + + // Get the code + code, err := strconv.ParseInt(address[i+1], 0, 0) + if err != nil { + return FieldReadResult{}, err + } + + // Get the set so we can get the index map that tells us the + // mapping of the hash code to the list index + _, indexMap, err := r.readSet(address[:i+1], v) + if err != nil { + return FieldReadResult{}, err + } + + index, ok := indexMap[int(code)] + if !ok { + return FieldReadResult{}, nil + } + + address[i+1] = strconv.FormatInt(int64(index), 10) + } + } + + k := strings.Join(address, ".") + schema := schemaList[len(schemaList)-1] switch schema.Type { case TypeBool: fallthrough @@ -30,13 +85,16 @@ func (r *ConfigFieldReader) ReadField(address []string) (FieldReadResult, error) case TypeString: return r.readPrimitive(k, schema) case TypeList: - return readListField(r, address, schema) + return readListField(&nestedConfigFieldReader{r}, address, schema) case TypeMap: return r.readMap(k) case TypeSet: - return r.readSet(address, schema) + result, _, err := r.readSet(address, schema) + return result, err case typeObject: - return readObjectField(r, address, schema.Elem.(map[string]*Schema)) + return readObjectField( + &nestedConfigFieldReader{r}, + address, schema.Elem.(map[string]*Schema)) default: panic(fmt.Sprintf("Unknown type: %#v", schema.Type)) } @@ -100,13 +158,14 @@ func (r *ConfigFieldReader) readPrimitive( } func (r *ConfigFieldReader) readSet( - address []string, schema *Schema) (FieldReadResult, error) { - raw, err := readListField(r, address, schema) + address []string, schema *Schema) (FieldReadResult, map[int]int, error) { + indexMap := make(map[int]int) + raw, err := readListField(&nestedConfigFieldReader{r}, address, schema) if err != nil { - return FieldReadResult{}, err + return FieldReadResult{}, indexMap, err } if !raw.Exists { - return FieldReadResult{}, nil + return FieldReadResult{}, indexMap, nil } // Create the set that will be our result @@ -118,16 +177,60 @@ func (r *ConfigFieldReader) readSet( Value: set, Exists: true, Computed: raw.Computed, - }, nil + }, indexMap, nil } // Build up the set from the list elements - for _, v := range raw.Value.([]interface{}) { - set.Add(v) + for i, v := range raw.Value.([]interface{}) { + // Check if any of the keys in this item are computed + computed := r.hasComputedSubKeys( + fmt.Sprintf("%s.%d", strings.Join(address, "."), i), schema) + + code := set.add(v) + indexMap[code] = i + if computed { + set.m[-code] = set.m[code] + delete(set.m, code) + code = -code + } } return FieldReadResult{ Value: set, Exists: true, - }, nil + }, indexMap, nil +} + +// hasComputedSubKeys walks through a schema and returns whether or not the +// given key contains any subkeys that are computed. +func (r *ConfigFieldReader) hasComputedSubKeys(key string, schema *Schema) bool { + prefix := key + "." + + switch t := schema.Elem.(type) { + case *Resource: + for k, schema := range t.Schema { + if r.Config.IsComputed(prefix + k) { + return true + } + + if r.hasComputedSubKeys(prefix+k, schema) { + return true + } + } + } + + return false +} + +// nestedConfigFieldReader is a funny little thing that just wraps a +// ConfigFieldReader to call readField when ReadField is called so that +// we don't recalculate the set rewrites in the address, which leads to +// an infinite loop. +type nestedConfigFieldReader struct { + Reader *ConfigFieldReader +} + +func (r *nestedConfigFieldReader) ReadField( + address []string) (FieldReadResult, error) { + return r.Reader.readField(address, true) } diff --git a/helper/schema/field_reader_config_test.go b/helper/schema/field_reader_config_test.go index 5eda033b2..f194b3fa0 100644 --- a/helper/schema/field_reader_config_test.go +++ b/helper/schema/field_reader_config_test.go @@ -184,6 +184,9 @@ func TestConfigFieldReader(t *testing.T) { } for name, tc := range cases { + if name != "list" { + continue + } out, err := r.ReadField(tc.Addr) if (err != nil) != tc.OutErr { t.Fatalf("%s: err: %s", name, err) diff --git a/helper/schema/field_reader_diff.go b/helper/schema/field_reader_diff.go index 337bcc5c9..f627040aa 100644 --- a/helper/schema/field_reader_diff.go +++ b/helper/schema/field_reader_diff.go @@ -32,11 +32,12 @@ type DiffFieldReader struct { } func (r *DiffFieldReader) ReadField(address []string) (FieldReadResult, error) { - schema := addrToSchema(address, r.Schema) - if schema == nil { + schemaList := addrToSchema(address, r.Schema) + if len(schemaList) == 0 { return FieldReadResult{}, nil } + schema := schemaList[len(schemaList)-1] switch schema.Type { case TypeBool: fallthrough @@ -117,14 +118,15 @@ func (r *DiffFieldReader) readPrimitive( if !attrD.NewComputed { resultVal = attrD.New if attrD.NewExtra != nil { + result.ValueProcessed = resultVal if err := mapstructure.WeakDecode(attrD.NewExtra, &resultVal); err != nil { return FieldReadResult{}, err } } } - result.Exists = true result.Computed = attrD.NewComputed + result.Exists = true result.Value, err = stringToPrimitive(resultVal, false, schema) if err != nil { return FieldReadResult{}, err @@ -167,6 +169,6 @@ func (r *DiffFieldReader) readSet( return FieldReadResult{ Value: set, - Exists: true, + Exists: set.Len() > 0, }, nil } diff --git a/helper/schema/field_reader_diff_test.go b/helper/schema/field_reader_diff_test.go index 0e64e5b5c..adcdbe18e 100644 --- a/helper/schema/field_reader_diff_test.go +++ b/helper/schema/field_reader_diff_test.go @@ -73,6 +73,13 @@ func TestDiffFieldReader(t *testing.T) { return a.(map[string]interface{})["index"].(int) }, }, + "setEmpty": &Schema{ + Type: TypeSet, + Elem: &Schema{Type: TypeInt}, + Set: func(a interface{}) int { + return a.(int) + }, + }, } r := &DiffFieldReader{ @@ -197,7 +204,7 @@ func TestDiffFieldReader(t *testing.T) { Source: &MapFieldReader{ Schema: schema, - Map: map[string]string{ + Map: BasicMapReader(map[string]string{ "listMap.#": "2", "listMap.0.foo": "bar", "listMap.0.bar": "baz", @@ -209,7 +216,7 @@ func TestDiffFieldReader(t *testing.T) { "setChange.#": "1", "setChange.10.index": "10", "setChange.10.value": "50", - }, + }), }, } @@ -387,6 +394,15 @@ func TestDiffFieldReader(t *testing.T) { }, false, }, + + "setEmpty": { + []string{"setEmpty"}, + FieldReadResult{ + Value: []interface{}{}, + Exists: false, + }, + false, + }, } for name, tc := range cases { diff --git a/helper/schema/field_reader_map.go b/helper/schema/field_reader_map.go index f06a7170a..ad1a5f3dc 100644 --- a/helper/schema/field_reader_map.go +++ b/helper/schema/field_reader_map.go @@ -8,30 +8,31 @@ import ( // MapFieldReader reads fields out of an untyped map[string]string to // the best of its ability. type MapFieldReader struct { - Map map[string]string + Map MapReader Schema map[string]*Schema } func (r *MapFieldReader) ReadField(address []string) (FieldReadResult, error) { k := strings.Join(address, ".") - schema := addrToSchema(address, r.Schema) - if schema == nil { + schemaList := addrToSchema(address, r.Schema) + if len(schemaList) == 0 { return FieldReadResult{}, nil } + schema := schemaList[len(schemaList)-1] switch schema.Type { case TypeBool: fallthrough case TypeInt: fallthrough case TypeString: - return r.readPrimitive(k, schema) + return r.readPrimitive(address, schema) case TypeList: return readListField(r, address, schema) case TypeMap: return r.readMap(k) case TypeSet: - return r.readSet(k, schema) + return r.readSet(address, schema) case typeObject: return readObjectField(r, address, schema.Elem.(map[string]*Schema)) default: @@ -44,14 +45,14 @@ func (r *MapFieldReader) readMap(k string) (FieldReadResult, error) { resultSet := false prefix := k + "." - for k, v := range r.Map { - if !strings.HasPrefix(k, prefix) { - continue + r.Map.Range(func(k, v string) bool { + if strings.HasPrefix(k, prefix) { + result[k[len(prefix):]] = v + resultSet = true } - result[k[len(prefix):]] = v - resultSet = true - } + return true + }) var resultVal interface{} if resultSet { @@ -65,8 +66,9 @@ func (r *MapFieldReader) readMap(k string) (FieldReadResult, error) { } func (r *MapFieldReader) readPrimitive( - k string, schema *Schema) (FieldReadResult, error) { - result, ok := r.Map[k] + address []string, schema *Schema) (FieldReadResult, error) { + k := strings.Join(address, ".") + result, ok := r.Map.Access(k) if !ok { return FieldReadResult{}, nil } @@ -83,9 +85,10 @@ func (r *MapFieldReader) readPrimitive( } func (r *MapFieldReader) readSet( - k string, schema *Schema) (FieldReadResult, error) { + address []string, schema *Schema) (FieldReadResult, error) { // Get the number of elements in the list - countRaw, err := r.readPrimitive(k+".#", &Schema{Type: TypeInt}) + countRaw, err := r.readPrimitive( + append(address, "#"), &Schema{Type: TypeInt}) if err != nil { return FieldReadResult{}, err } @@ -101,29 +104,32 @@ func (r *MapFieldReader) readSet( if countRaw.Computed || countRaw.Value.(int) == 0 { return FieldReadResult{ Value: set, - Exists: true, + Exists: countRaw.Exists, Computed: countRaw.Computed, }, nil } // Go through the map and find all the set items - prefix := k + "." - for k, _ := range r.Map { + prefix := strings.Join(address, ".") + "." + countExpected := countRaw.Value.(int) + countActual := make(map[string]struct{}) + completed := r.Map.Range(func(k, _ string) bool { if !strings.HasPrefix(k, prefix) { - continue + return true } if strings.HasPrefix(k, prefix+"#") { // Ignore the count field - continue + return true } // Split the key, since it might be a sub-object like "idx.field" parts := strings.Split(k[len(prefix):], ".") idx := parts[0] - raw, err := r.ReadField([]string{prefix[:len(prefix)-1], idx}) + var raw FieldReadResult + raw, err = r.ReadField(append(address, idx)) if err != nil { - return FieldReadResult{}, err + return false } if !raw.Exists { // This shouldn't happen because we just verified it does exist @@ -131,6 +137,21 @@ func (r *MapFieldReader) readSet( } set.Add(raw.Value) + + // Due to the way multimap readers work, if we've seen the number + // of fields we expect, then exit so that we don't read later values. + // For example: the "set" map might have "ports.#", "ports.0", and + // "ports.1", but the "state" map might have those plus "ports.2". + // We don't want "ports.2" + countActual[idx] = struct{}{} + if len(countActual) >= countExpected { + return false + } + + return true + }) + if !completed && err != nil { + return FieldReadResult{}, err } return FieldReadResult{ @@ -138,3 +159,62 @@ func (r *MapFieldReader) readSet( Exists: true, }, nil } + +// MapReader is an interface that is given to MapFieldReader for accessing +// a "map". This can be used to have alternate implementations. For a basic +// map[string]string, use BasicMapReader. +type MapReader interface { + Access(string) (string, bool) + Range(func(string, string) bool) bool +} + +// BasicMapReader implements MapReader for a single map. +type BasicMapReader map[string]string + +func (r BasicMapReader) Access(k string) (string, bool) { + v, ok := r[k] + return v, ok +} + +func (r BasicMapReader) Range(f func(string, string) bool) bool { + for k, v := range r { + if cont := f(k, v); !cont { + return false + } + } + + return true +} + +// MultiMapReader reads over multiple maps, preferring keys that are +// founder earlier (lower number index) vs. later (higher number index) +type MultiMapReader []map[string]string + +func (r MultiMapReader) Access(k string) (string, bool) { + for _, m := range r { + if v, ok := m[k]; ok { + return v, ok + } + } + + return "", false +} + +func (r MultiMapReader) Range(f func(string, string) bool) bool { + done := make(map[string]struct{}) + for _, m := range r { + for k, v := range m { + if _, ok := done[k]; ok { + continue + } + + if cont := f(k, v); !cont { + return false + } + + done[k] = struct{}{} + } + } + + return true +} diff --git a/helper/schema/field_reader_map_test.go b/helper/schema/field_reader_map_test.go index 9734d7f7a..5c4081c36 100644 --- a/helper/schema/field_reader_map_test.go +++ b/helper/schema/field_reader_map_test.go @@ -43,9 +43,16 @@ func TestMapFieldReader(t *testing.T) { return a.(map[string]interface{})["index"].(int) }, }, + "setEmpty": &Schema{ + Type: TypeSet, + Elem: &Schema{Type: TypeInt}, + Set: func(a interface{}) int { + return a.(int) + }, + }, }, - Map: map[string]string{ + Map: BasicMapReader(map[string]string{ "bool": "true", "int": "42", "string": "string", @@ -70,7 +77,7 @@ func TestMapFieldReader(t *testing.T) { "setDeep.10.value": "foo", "setDeep.50.index": "50", "setDeep.50.value": "bar", - }, + }), } cases := map[string]struct { @@ -161,6 +168,14 @@ func TestMapFieldReader(t *testing.T) { false, }, + "setEmpty": { + []string{"setEmpty"}, + []interface{}{}, + false, + false, + false, + }, + "setDeep": { []string{"setDeep"}, []interface{}{ diff --git a/helper/schema/field_reader_multi.go b/helper/schema/field_reader_multi.go index 3e2134708..89ad3a86f 100644 --- a/helper/schema/field_reader_multi.go +++ b/helper/schema/field_reader_multi.go @@ -41,20 +41,17 @@ func (r *MultiLevelFieldReader) ReadFieldMerge( address []string, level string) (FieldReadResult, error) { var result FieldReadResult for _, l := range r.Levels { - r, ok := r.Readers[l] - if !ok { - continue - } + if r, ok := r.Readers[l]; ok { + out, err := r.ReadField(address) + if err != nil { + return FieldReadResult{}, fmt.Errorf( + "Error reading level %s: %s", l, err) + } - out, err := r.ReadField(address) - if err != nil { - return FieldReadResult{}, fmt.Errorf( - "Error reading level %s: %s", l, err) - } - - // TODO: computed - if out.Exists { - result = out + // TODO: computed + if out.Exists { + result = out + } } if l == level { diff --git a/helper/schema/field_reader_multi_test.go b/helper/schema/field_reader_multi_test.go index 915fb1447..85286a66e 100644 --- a/helper/schema/field_reader_multi_test.go +++ b/helper/schema/field_reader_multi_test.go @@ -23,23 +23,23 @@ func TestMultiLevelFieldReaderReadFieldExact(t *testing.T) { Schema: map[string]*Schema{ "foo": &Schema{Type: TypeString}, }, - Map: map[string]string{ + Map: BasicMapReader(map[string]string{ "foo": "bar", - }, + }), }, &MapFieldReader{ Schema: map[string]*Schema{ "foo": &Schema{Type: TypeString}, }, - Map: map[string]string{ + Map: BasicMapReader(map[string]string{ "foo": "baz", - }, + }), }, &MapFieldReader{ Schema: map[string]*Schema{ "foo": &Schema{Type: TypeString}, }, - Map: map[string]string{}, + Map: BasicMapReader(map[string]string{}), }, }, @@ -95,9 +95,9 @@ func TestMultiLevelFieldReaderReadFieldMerge(t *testing.T) { Schema: map[string]*Schema{ "availability_zone": &Schema{Type: TypeString}, }, - Map: map[string]string{ + Map: BasicMapReader(map[string]string{ "availability_zone": "foo", - }, + }), }, Diff: &terraform.InstanceDiff{ @@ -127,9 +127,9 @@ func TestMultiLevelFieldReaderReadFieldMerge(t *testing.T) { "availability_zone": &Schema{Type: TypeString}, }, - Map: map[string]string{ + Map: BasicMapReader(map[string]string{ "availability_zone": "foo", - }, + }), }, &DiffFieldReader{ @@ -142,9 +142,9 @@ func TestMultiLevelFieldReaderReadFieldMerge(t *testing.T) { "availability_zone": &Schema{Type: TypeString}, }, - Map: map[string]string{ + Map: BasicMapReader(map[string]string{ "availability_zone": "foo", - }, + }), }, Diff: &terraform.InstanceDiff{ @@ -186,12 +186,12 @@ func TestMultiLevelFieldReaderReadFieldMerge(t *testing.T) { }, }, - Map: map[string]string{ + Map: BasicMapReader(map[string]string{ "config_vars.#": "2", "config_vars.0.foo": "bar", "config_vars.0.bar": "bar", "config_vars.1.bar": "baz", - }, + }), }, Diff: &terraform.InstanceDiff{ @@ -225,15 +225,15 @@ func TestMultiLevelFieldReaderReadFieldMerge(t *testing.T) { Schema: map[string]*Schema{ "foo": &Schema{Type: TypeString}, }, - Map: map[string]string{ + Map: BasicMapReader(map[string]string{ "foo": "bar", - }, + }), }, &MapFieldReader{ Schema: map[string]*Schema{ "foo": &Schema{Type: TypeString}, }, - Map: map[string]string{}, + Map: BasicMapReader(map[string]string{}), }, }, diff --git a/helper/schema/field_reader_test.go b/helper/schema/field_reader_test.go index 7498b05f6..64d2c3f2c 100644 --- a/helper/schema/field_reader_test.go +++ b/helper/schema/field_reader_test.go @@ -9,14 +9,134 @@ func TestAddrToSchema(t *testing.T) { cases := map[string]struct { Addr []string Schema map[string]*Schema - Result *Schema + Result []ValueType }{ + "list": { + []string{"list"}, + map[string]*Schema{ + "list": &Schema{ + Type: TypeList, + Elem: &Schema{Type: TypeInt}, + }, + }, + []ValueType{TypeList}, + }, + + "list.#": { + []string{"list", "#"}, + map[string]*Schema{ + "list": &Schema{ + Type: TypeList, + Elem: &Schema{Type: TypeInt}, + }, + }, + []ValueType{TypeList, TypeInt}, + }, + + "list.0": { + []string{"list", "0"}, + map[string]*Schema{ + "list": &Schema{ + Type: TypeList, + Elem: &Schema{Type: TypeInt}, + }, + }, + []ValueType{TypeList, TypeInt}, + }, + + "list.0 with resource": { + []string{"list", "0"}, + map[string]*Schema{ + "list": &Schema{ + Type: TypeList, + Elem: &Resource{ + Schema: map[string]*Schema{ + "field": &Schema{Type: TypeString}, + }, + }, + }, + }, + []ValueType{TypeList, typeObject}, + }, + + "list.0.field": { + []string{"list", "0", "field"}, + map[string]*Schema{ + "list": &Schema{ + Type: TypeList, + Elem: &Resource{ + Schema: map[string]*Schema{ + "field": &Schema{Type: TypeString}, + }, + }, + }, + }, + []ValueType{TypeList, typeObject, TypeString}, + }, + + "set": { + []string{"set"}, + map[string]*Schema{ + "set": &Schema{ + Type: TypeSet, + Elem: &Schema{Type: TypeInt}, + Set: func(a interface{}) int { + return a.(int) + }, + }, + }, + []ValueType{TypeSet}, + }, + + "set.#": { + []string{"set", "#"}, + map[string]*Schema{ + "set": &Schema{ + Type: TypeSet, + Elem: &Schema{Type: TypeInt}, + Set: func(a interface{}) int { + return a.(int) + }, + }, + }, + []ValueType{TypeSet, TypeInt}, + }, + + "set.0": { + []string{"set", "0"}, + map[string]*Schema{ + "set": &Schema{ + Type: TypeSet, + Elem: &Schema{Type: TypeInt}, + Set: func(a interface{}) int { + return a.(int) + }, + }, + }, + []ValueType{TypeSet, TypeInt}, + }, + + "set.0 with resource": { + []string{"set", "0"}, + map[string]*Schema{ + "set": &Schema{ + Type: TypeSet, + Elem: &Resource{ + Schema: map[string]*Schema{ + "field": &Schema{Type: TypeString}, + }, + }, + }, + }, + []ValueType{TypeSet, typeObject}, + }, + "mapElem": { []string{"map", "foo"}, map[string]*Schema{ "map": &Schema{Type: TypeMap}, }, - &Schema{Type: TypeString}, + []ValueType{TypeMap, TypeString}, }, "setDeep": { @@ -35,14 +155,19 @@ func TestAddrToSchema(t *testing.T) { }, }, }, - &Schema{Type: TypeInt}, + []ValueType{TypeSet, typeObject, TypeInt}, }, } for name, tc := range cases { result := addrToSchema(tc.Addr, tc.Schema) - if !reflect.DeepEqual(result, tc.Result) { - t.Fatalf("%s: %#v", name, result) + types := make([]ValueType, len(result)) + for i, v := range result { + types[i] = v.Type + } + + if !reflect.DeepEqual(types, tc.Result) { + t.Fatalf("%s: %#v", name, types) } } } diff --git a/helper/schema/resource_data.go b/helper/schema/resource_data.go index ee6728735..2f0082be5 100644 --- a/helper/schema/resource_data.go +++ b/helper/schema/resource_data.go @@ -28,11 +28,12 @@ type ResourceData struct { diffing bool // Don't set - setMap map[string]string - newState *terraform.InstanceState - partial bool - partialMap map[string]struct{} - once sync.Once + multiReader *MultiLevelFieldReader + setMap map[string]string + newState *terraform.InstanceState + partial bool + partialMap map[string]struct{} + once sync.Once } // getSource represents the level we want to get for a value (internally). @@ -153,10 +154,7 @@ func (d *ResourceData) Partial(on bool) { // If the key is invalid or the value is not a correct type, an error // will be returned. func (d *ResourceData) Set(key string, value interface{}) error { - if d.setMap == nil { - d.setMap = make(map[string]string) - } - + d.once.Do(d.init) parts := strings.Split(key, ".") return d.setObject("", parts, d.schema, value) } @@ -236,12 +234,57 @@ func (d *ResourceData) State() *terraform.InstanceState { } func (d *ResourceData) init() { + // Initialize the field that will store our new state var copyState terraform.InstanceState if d.state != nil { copyState = *d.state } - d.newState = ©State + + // Initialize the map for storing set data + d.setMap = make(map[string]string) + + // Initialize the reader for getting data from the + // underlying sources (config, diff, etc.) + readers := make(map[string]FieldReader) + var stateAttributes map[string]string + if d.state != nil { + stateAttributes = d.state.Attributes + readers["state"] = &MapFieldReader{ + Schema: d.schema, + Map: BasicMapReader(stateAttributes), + } + } + if d.config != nil { + readers["config"] = &ConfigFieldReader{ + Schema: d.schema, + Config: d.config, + } + } + if d.diff != nil { + readers["diff"] = &DiffFieldReader{ + Schema: d.schema, + Diff: d.diff, + Source: &MultiLevelFieldReader{ + Levels: []string{"state", "config"}, + Readers: readers, + }, + } + } + readers["set"] = &MapFieldReader{ + Schema: d.schema, + Map: BasicMapReader(d.setMap), + } + d.multiReader = &MultiLevelFieldReader{ + Levels: []string{ + "state", + "config", + "diff", + "set", + }, + + Readers: readers, + } } func (d *ResourceData) diffChange( @@ -279,21 +322,57 @@ func (d *ResourceData) get( parts []string, schema *Schema, source getSource) getResult { - switch schema.Type { - case TypeList: - return d.getList(k, parts, schema, source) - case TypeMap: - return d.getMap(k, parts, schema, source) - case TypeSet: - return d.getSet(k, parts, schema, source) - case TypeBool: - fallthrough - case TypeInt: - fallthrough - case TypeString: - return d.getPrimitive(k, parts, schema, source) - default: - panic(fmt.Sprintf("%s: unknown type %#v", k, schema.Type)) + d.once.Do(d.init) + + level := "set" + flags := source & ^getSourceLevelMask + diff := flags&getSourceDiff != 0 + exact := flags&getSourceExact != 0 + source = source & getSourceLevelMask + if source >= getSourceSet { + level = "set" + } else if diff { + level = "diff" + } else if source >= getSourceConfig { + level = "config" + } else { + level = "state" + } + + // Build the address of the key we're looking for and ask the FieldReader + addr := append(strings.Split(k, "."), parts...) + for i, v := range addr { + if v[0] == '~' { + addr[i] = v[1:] + } + } + + var result FieldReadResult + var err error + if exact { + result, err = d.multiReader.ReadFieldExact(addr, level) + } else { + result, err = d.multiReader.ReadFieldMerge(addr, level) + } + if err != nil { + panic(err) + } + + // If the result doesn't exist, then we set the value to the zero value + if result.Value == nil { + if schema := addrToSchema(addr, d.schema); len(schema) > 0 { + result.Value = schema[len(schema)-1].Type.Zero() + } + } + + // Transform the FieldReadResult into a getResult. It might be worth + // merging these two structures one day. + return getResult{ + Value: result.Value, + ValueProcessed: result.ValueProcessed, + Computed: result.Computed, + Exists: result.Exists, + Schema: schema, } } @@ -831,11 +910,10 @@ func (d *ResourceData) setList( schema *Schema, value interface{}) error { if len(parts) > 0 { - // We're setting a specific element - idx := parts[0] - parts = parts[1:] + return fmt.Errorf("%s: can only set the full list, not elements", k) + } - // Special case if we're accessing the count of the list + setElement := func(k string, idx string, value interface{}) error { if idx == "#" { return fmt.Errorf("%s: can't set count of list", k) } @@ -843,10 +921,12 @@ func (d *ResourceData) setList( key := fmt.Sprintf("%s.%s", k, idx) switch t := schema.Elem.(type) { case *Resource: - return d.setObject(key, parts, t.Schema, value) + return d.setObject(key, nil, t.Schema, value) case *Schema: - return d.set(key, parts, t, value) + return d.set(key, nil, t, value) } + + return nil } var vs []interface{} @@ -858,7 +938,7 @@ func (d *ResourceData) setList( var err error for i, elem := range vs { is := strconv.FormatInt(int64(i), 10) - err = d.setList(k, []string{is}, schema, elem) + err = setElement(k, is, elem) if err != nil { break } @@ -866,7 +946,7 @@ func (d *ResourceData) setList( if err != nil { for i, _ := range vs { is := strconv.FormatInt(int64(i), 10) - d.setList(k, []string{is}, schema, nil) + setElement(k, is, nil) } return err @@ -1015,7 +1095,9 @@ func (d *ResourceData) setSet( // Build a temp *ResourceData to use for the conversion tempD := &ResourceData{ setMap: make(map[string]string), + schema: map[string]*Schema{k: schema}, } + tempD.once.Do(tempD.init) // Set the entire list, this lets us get sane values out of it if err := tempD.setList(k, nil, schema, value); err != nil { @@ -1031,7 +1113,7 @@ func (d *ResourceData) setSet( source := getSourceSet | getSourceExact for i := 0; i < v.Len(); i++ { is := strconv.FormatInt(int64(i), 10) - result := tempD.getList(k, []string{is}, schema, source) + result := tempD.get(k, []string{is}, schema, source) if !result.Exists { panic("just set item doesn't exist") } diff --git a/helper/schema/resource_data_test.go b/helper/schema/resource_data_test.go index e66a681e8..0151a45a9 100644 --- a/helper/schema/resource_data_test.go +++ b/helper/schema/resource_data_test.go @@ -606,7 +606,7 @@ func TestResourceDataGet(t *testing.T) { } if !reflect.DeepEqual(v, tc.Value) { - t.Fatalf("Bad: %d\n\n%#v", i, v) + t.Fatalf("Bad: %d\n\n%#v\n\nExpected: %#v", i, v, tc.Value) } } } @@ -816,7 +816,7 @@ func TestResourceDataGetOk(t *testing.T) { Diff: nil, Key: "ports", - Value: []interface{}{}, + Value: nil, Ok: false, }, @@ -1082,34 +1082,6 @@ func TestResourceDataSet(t *testing.T) { GetValue: "", }, - // List of primitives, set element - { - Schema: map[string]*Schema{ - "ports": &Schema{ - Type: TypeList, - Computed: true, - Elem: &Schema{Type: TypeInt}, - }, - }, - - State: &terraform.InstanceState{ - Attributes: map[string]string{ - "ports.#": "3", - "ports.0": "1", - "ports.1": "2", - "ports.2": "5", - }, - }, - - Diff: nil, - - Key: "ports.1", - Value: 3, - - GetKey: "ports", - GetValue: []interface{}{1, 3, 5}, - }, - // List of primitives, set list { Schema: map[string]*Schema{ @@ -1153,139 +1125,6 @@ func TestResourceDataSet(t *testing.T) { GetValue: []interface{}{}, }, - // List of resource, set element - { - Schema: map[string]*Schema{ - "ingress": &Schema{ - Type: TypeList, - Computed: true, - Elem: &Resource{ - Schema: map[string]*Schema{ - "from": &Schema{ - Type: TypeInt, - }, - }, - }, - }, - }, - - State: &terraform.InstanceState{ - Attributes: map[string]string{ - "ingress.#": "2", - "ingress.0.from": "80", - "ingress.1.from": "8080", - }, - }, - - Diff: nil, - - Key: "ingress.1.from", - Value: 9000, - - GetKey: "ingress", - GetValue: []interface{}{ - map[string]interface{}{ - "from": 80, - }, - map[string]interface{}{ - "from": 9000, - }, - }, - }, - - // List of resource, set full resource element - { - Schema: map[string]*Schema{ - "ingress": &Schema{ - Type: TypeList, - Computed: true, - Elem: &Resource{ - Schema: map[string]*Schema{ - "from": &Schema{ - Type: TypeInt, - }, - }, - }, - }, - }, - - State: &terraform.InstanceState{ - Attributes: map[string]string{ - "ingress.#": "2", - "ingress.0.from": "80", - "ingress.1.from": "8080", - }, - }, - - Diff: nil, - - Key: "ingress.1", - Value: map[string]interface{}{ - "from": 9000, - }, - - GetKey: "ingress", - GetValue: []interface{}{ - map[string]interface{}{ - "from": 80, - }, - map[string]interface{}{ - "from": 9000, - }, - }, - }, - - // List of resource, set full resource element, with error - { - Schema: map[string]*Schema{ - "ingress": &Schema{ - Type: TypeList, - Computed: true, - Elem: &Resource{ - Schema: map[string]*Schema{ - "from": &Schema{ - Type: TypeInt, - }, - "to": &Schema{ - Type: TypeInt, - }, - }, - }, - }, - }, - - State: &terraform.InstanceState{ - Attributes: map[string]string{ - "ingress.#": "2", - "ingress.0.from": "80", - "ingress.0.to": "10", - "ingress.1.from": "8080", - "ingress.1.to": "8080", - }, - }, - - Diff: nil, - - Key: "ingress.1", - Value: map[string]interface{}{ - "from": 9000, - "to": "bar", - }, - Err: true, - - GetKey: "ingress", - GetValue: []interface{}{ - map[string]interface{}{ - "from": 80, - "to": 10, - }, - map[string]interface{}{ - "from": 8080, - "to": 8080, - }, - }, - }, - // Set a list of maps { Schema: map[string]*Schema{ @@ -1325,45 +1164,6 @@ func TestResourceDataSet(t *testing.T) { }, }, - // Set a list of maps - { - Schema: map[string]*Schema{ - "config_vars": &Schema{ - Type: TypeList, - Optional: true, - Computed: true, - Elem: &Schema{ - Type: TypeMap, - }, - }, - }, - - State: nil, - - Diff: nil, - - Key: "config_vars", - Value: []interface{}{ - map[string]string{ - "foo": "bar", - }, - map[string]string{ - "bar": "baz", - }, - }, - Err: false, - - GetKey: "config_vars", - GetValue: []interface{}{ - map[string]interface{}{ - "foo": "bar", - }, - map[string]interface{}{ - "bar": "baz", - }, - }, - }, - // Set, with list { Schema: map[string]*Schema{ @@ -1727,8 +1527,13 @@ func TestResourceDataState(t *testing.T) { }, Set: map[string]interface{}{ - "config_vars.1": map[string]interface{}{ - "baz": "bang", + "config_vars": []map[string]interface{}{ + map[string]interface{}{ + "foo": "bar", + }, + map[string]interface{}{ + "baz": "bang", + }, }, }, @@ -2141,8 +1946,13 @@ func TestResourceDataState(t *testing.T) { }, Set: map[string]interface{}{ - "config_vars.1": map[string]interface{}{ - "baz": "bang", + "config_vars": []map[string]interface{}{ + map[string]interface{}{ + "foo": "bar", + }, + map[string]interface{}{ + "baz": "bang", + }, }, }, @@ -2150,6 +1960,7 @@ func TestResourceDataState(t *testing.T) { Result: &terraform.InstanceState{ Attributes: map[string]string{ + // TODO: broken, shouldn't bar be removed? "config_vars.#": "2", "config_vars.0.foo": "bar", "config_vars.0.bar": "bar", diff --git a/helper/schema/schema.go b/helper/schema/schema.go index e875028bd..218de7851 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -36,6 +36,28 @@ const ( typeObject ) +// Zero returns the zero value for a type. +func (t ValueType) Zero() interface{} { + switch t { + case TypeInvalid: + return nil + case TypeBool: + return false + case TypeInt: + return 0 + case TypeString: + return "" + case TypeList: + return []interface{}{} + case TypeMap: + return map[string]interface{}{} + case TypeSet: + return nil + default: + panic(fmt.Sprintf("unknown type %#v", t)) + } +} + // Schema is used to describe the structure of a value. // // Read the documentation of the struct elements for important details. @@ -221,8 +243,10 @@ func (m schemaMap) Diff( result2 := new(terraform.InstanceDiff) result2.Attributes = make(map[string]*terraform.ResourceAttrDiff) - // Reset the data to not contain state + // Reset the data to not contain state. We have to call init() + // again in order to reset the FieldReaders. d.state = nil + d.init() // Perform the diff again for k, schema := range m { @@ -458,6 +482,9 @@ func (m schemaMap) diffList( d *ResourceData, all bool) error { o, n, _, computedList := d.diffChange(k) + if computedList { + n = nil + } nSet := n != nil // If we have an old value and no new value is set or will be @@ -655,6 +682,9 @@ func (m schemaMap) diffSet( d *ResourceData, all bool) error { o, n, _, computedSet := d.diffChange(k) + if computedSet { + n = nil + } nSet := n != nil // If we have an old value and no new value is set or will be diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 9f24f8b76..f741ad56c 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -8,6 +8,27 @@ import ( "github.com/hashicorp/terraform/terraform" ) +func TestValueType_Zero(t *testing.T) { + cases := []struct { + Type ValueType + Value interface{} + }{ + {TypeBool, false}, + {TypeInt, 0}, + {TypeString, ""}, + {TypeList, []interface{}{}}, + {TypeMap, map[string]interface{}{}}, + {TypeSet, nil}, + } + + for i, tc := range cases { + actual := tc.Type.Zero() + if !reflect.DeepEqual(actual, tc.Value) { + t.Fatalf("%d: %#v != %#v", i, actual, tc.Value) + } + } +} + func TestSchemaMap_Diff(t *testing.T) { cases := []struct { Schema map[string]*Schema @@ -844,7 +865,7 @@ func TestSchemaMap_Diff(t *testing.T) { Diff: &terraform.InstanceDiff{ Attributes: map[string]*terraform.ResourceAttrDiff{ "ports.#": &terraform.ResourceAttrDiff{ - Old: "", + Old: "0", New: "", NewComputed: true, }, @@ -1664,7 +1685,7 @@ func TestSchemaMap_Diff(t *testing.T) { New: "1", }, "route.~1.gateway.#": &terraform.ResourceAttrDiff{ - Old: "", + Old: "0", NewComputed: true, }, },