From cc8e8a55de4ca3d2aaeaa8c3af64904740046ef9 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 17 Aug 2015 19:26:58 -0700 Subject: [PATCH] helper/schema: Default hashing function for sets A common issue with new resource implementations is not considering parts of a complex structure that's used inside a set, which causes quirky behavior. The schema helper has enough information to provide a default reasonable implementation of a set function that includes all non-computed attributes in a deterministic way. Here we implement such a function and use it when no explicit hashing function is provided. In order to achieve this we encapsulate the construction of the zero value for a schema in a new method schema.ZeroValue, which allows us to put the fallback logic to the new default function in a single spot. It is no longer valid to use &Set{F: schema.Set} and all uses of that construct should be replaced with schema.ZeroValue().(*Set) . --- helper/schema/field_reader.go | 10 +- helper/schema/field_reader_config.go | 2 +- helper/schema/field_reader_diff.go | 2 +- helper/schema/field_reader_map.go | 2 +- helper/schema/schema.go | 32 +++- helper/schema/schema_test.go | 2 +- helper/schema/serialize.go | 105 +++++++++++++ helper/schema/serialize_test.go | 214 +++++++++++++++++++++++++++ helper/schema/set.go | 23 +++ 9 files changed, 374 insertions(+), 18 deletions(-) create mode 100644 helper/schema/serialize.go create mode 100644 helper/schema/serialize_test.go diff --git a/helper/schema/field_reader.go b/helper/schema/field_reader.go index fc2a1e090280..c3a6c76fa08f 100644 --- a/helper/schema/field_reader.go +++ b/helper/schema/field_reader.go @@ -38,15 +38,7 @@ func (r *FieldReadResult) ValueOrZero(s *Schema) interface{} { return r.Value } - result := s.Type.Zero() - - // The zero value of a set is nil, but we want it - // to actually be an empty set object... - if set, ok := result.(*Set); ok && set.F == nil { - set.F = s.Set - } - - return result + return s.ZeroValue() } // addrToSchema finds the final element schema for the given address diff --git a/helper/schema/field_reader_config.go b/helper/schema/field_reader_config.go index 69b63eac74f1..76aeed2bd8e4 100644 --- a/helper/schema/field_reader_config.go +++ b/helper/schema/field_reader_config.go @@ -201,7 +201,7 @@ func (r *ConfigFieldReader) readSet( address []string, schema *Schema) (FieldReadResult, map[int]int, error) { indexMap := make(map[int]int) // Create the set that will be our result - set := &Set{F: schema.Set} + set := schema.ZeroValue().(*Set) raw, err := readListField(&nestedConfigFieldReader{r}, address, schema) if err != nil { diff --git a/helper/schema/field_reader_diff.go b/helper/schema/field_reader_diff.go index e17a6685ec98..dcb379436e78 100644 --- a/helper/schema/field_reader_diff.go +++ b/helper/schema/field_reader_diff.go @@ -141,7 +141,7 @@ func (r *DiffFieldReader) readSet( prefix := strings.Join(address, ".") + "." // Create the set that will be our result - set := &Set{F: schema.Set} + set := schema.ZeroValue().(*Set) // Go through the map and find all the set items for k, d := range r.Diff.Attributes { diff --git a/helper/schema/field_reader_map.go b/helper/schema/field_reader_map.go index 6dc76c4740c1..feb3fcc0a7e1 100644 --- a/helper/schema/field_reader_map.go +++ b/helper/schema/field_reader_map.go @@ -105,7 +105,7 @@ func (r *MapFieldReader) readSet( } // Create the set that will be our result - set := &Set{F: schema.Set} + set := schema.ZeroValue().(*Set) // If we have an empty list, then return an empty list if countRaw.Computed || countRaw.Value.(int) == 0 { diff --git a/helper/schema/schema.go b/helper/schema/schema.go index 59a3260fb9f7..da0bb38b3a38 100644 --- a/helper/schema/schema.go +++ b/helper/schema/schema.go @@ -207,6 +207,30 @@ func (s *Schema) DefaultValue() (interface{}, error) { return nil, nil } +// Returns a zero value for the schema. +func (s *Schema) ZeroValue() interface{} { + // If it's a set then we'll do a bit of extra work to provide the + // right hashing function in our empty value. + if s.Type == TypeSet { + setFunc := s.Set + if setFunc == nil { + // Default set function uses the schema to hash the whole value + elem := s.Elem + switch t := elem.(type) { + case *Schema: + setFunc = HashSchema(t) + case *Resource: + setFunc = HashResource(t) + default: + panic("invalid set element type") + } + } + return &Set{F: setFunc} + } else { + return s.Type.Zero() + } +} + func (s *Schema) finalizeDiff( d *terraform.ResourceAttrDiff) *terraform.ResourceAttrDiff { if d == nil { @@ -496,10 +520,8 @@ func (m schemaMap) InternalValidate(topSchemaMap schemaMap) error { return fmt.Errorf("%s: Default is not valid for lists or sets", k) } - if v.Type == TypeList && v.Set != nil { + if v.Type != TypeSet && v.Set != nil { return fmt.Errorf("%s: Set can only be set for TypeSet", k) - } else if v.Type == TypeSet && v.Set == nil { - return fmt.Errorf("%s: Set must be set", k) } switch t := v.Elem.(type) { @@ -782,10 +804,10 @@ func (m schemaMap) diffSet( } if o == nil { - o = &Set{F: schema.Set} + o = schema.ZeroValue().(*Set) } if n == nil { - n = &Set{F: schema.Set} + n = schema.ZeroValue().(*Set) } os := o.(*Set) ns := n.(*Set) diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go index 83aa72a5cab7..404d7286cfc7 100644 --- a/helper/schema/schema_test.go +++ b/helper/schema/schema_test.go @@ -2789,7 +2789,7 @@ func TestSchemaMap_InternalValidate(t *testing.T) { Optional: true, }, }, - true, + false, }, // Required but computed diff --git a/helper/schema/serialize.go b/helper/schema/serialize.go new file mode 100644 index 000000000000..78f5bfbd6a96 --- /dev/null +++ b/helper/schema/serialize.go @@ -0,0 +1,105 @@ +package schema + +import ( + "bytes" + "sort" + "strconv" +) + +func SerializeValueForHash(buf *bytes.Buffer, val interface{}, schema *Schema) { + if val == nil { + buf.WriteRune(';') + return + } + + switch schema.Type { + case TypeBool: + if val.(bool) { + buf.WriteRune('1') + } else { + buf.WriteRune('0') + } + case TypeInt: + buf.WriteString(strconv.Itoa(val.(int))) + case TypeFloat: + buf.WriteString(strconv.FormatFloat(val.(float64), 'g', -1, 64)) + case TypeString: + buf.WriteString(val.(string)) + case TypeList: + buf.WriteRune('(') + l := val.([]interface{}) + for _, innerVal := range l { + serializeCollectionMemberForHash(buf, innerVal, schema.Elem) + } + buf.WriteRune(')') + case TypeMap: + m := val.(map[string]interface{}) + var keys []string + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + buf.WriteRune('[') + for _, k := range keys { + innerVal := m[k] + buf.WriteString(k) + buf.WriteRune(':') + serializeCollectionMemberForHash(buf, innerVal, schema.Elem) + } + buf.WriteRune(']') + case TypeSet: + buf.WriteRune('{') + s := val.(*Set) + for _, innerVal := range s.List() { + serializeCollectionMemberForHash(buf, innerVal, schema.Elem) + } + buf.WriteRune('}') + default: + panic("unknown schema type to serialize") + } + buf.WriteRune(';') +} + +// SerializeValueForHash appends a serialization of the given resource config +// to the given buffer, guaranteeing deterministic results given the same value +// and schema. +// +// Its primary purpose is as input into a hashing function in order +// to hash complex substructures when used in sets, and so the serialization +// is not reversible. +func SerializeResourceForHash(buf *bytes.Buffer, val interface{}, resource *Resource) { + sm := resource.Schema + m := val.(map[string]interface{}) + var keys []string + for k := range sm { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + innerSchema := sm[k] + // Skip attributes that are not user-provided. Computed attributes + // do not contribute to the hash since their ultimate value cannot + // be known at plan/diff time. + if !(innerSchema.Required || innerSchema.Optional) { + continue + } + + buf.WriteString(k) + buf.WriteRune(':') + innerVal := m[k] + SerializeValueForHash(buf, innerVal, innerSchema) + } +} + +func serializeCollectionMemberForHash(buf *bytes.Buffer, val interface{}, elem interface{}) { + switch tElem := elem.(type) { + case *Schema: + SerializeValueForHash(buf, val, tElem) + case *Resource: + buf.WriteRune('<') + SerializeResourceForHash(buf, val, tElem) + buf.WriteString(">;") + default: + panic("invalid element type") + } +} diff --git a/helper/schema/serialize_test.go b/helper/schema/serialize_test.go new file mode 100644 index 000000000000..7fe9e20bf2d3 --- /dev/null +++ b/helper/schema/serialize_test.go @@ -0,0 +1,214 @@ +package schema + +import ( + "bytes" + "testing" +) + +func TestSerializeForHash(t *testing.T) { + type testCase struct { + Schema interface{} + Value interface{} + Expected string + } + + tests := []testCase{ + + testCase{ + Schema: &Schema{ + Type: TypeInt, + }, + Value: 0, + Expected: "0;", + }, + + testCase{ + Schema: &Schema{ + Type: TypeInt, + }, + Value: 200, + Expected: "200;", + }, + + testCase{ + Schema: &Schema{ + Type: TypeBool, + }, + Value: true, + Expected: "1;", + }, + + testCase{ + Schema: &Schema{ + Type: TypeBool, + }, + Value: false, + Expected: "0;", + }, + + testCase{ + Schema: &Schema{ + Type: TypeFloat, + }, + Value: 1.0, + Expected: "1;", + }, + + testCase{ + Schema: &Schema{ + Type: TypeFloat, + }, + Value: 1.54, + Expected: "1.54;", + }, + + testCase{ + Schema: &Schema{ + Type: TypeFloat, + }, + Value: 0.1, + Expected: "0.1;", + }, + + testCase{ + Schema: &Schema{ + Type: TypeString, + }, + Value: "hello", + Expected: "hello;", + }, + + testCase{ + Schema: &Schema{ + Type: TypeString, + }, + Value: "1", + Expected: "1;", + }, + + testCase{ + Schema: &Schema{ + Type: TypeList, + Elem: &Schema{ + Type: TypeString, + }, + }, + Value: []interface{}{}, + Expected: "();", + }, + + testCase{ + Schema: &Schema{ + Type: TypeList, + Elem: &Schema{ + Type: TypeString, + }, + }, + Value: []interface{}{"hello", "world"}, + Expected: "(hello;world;);", + }, + + testCase{ + Schema: &Schema{ + Type: TypeList, + Elem: &Resource{ + Schema: map[string]*Schema{ + "fo": &Schema{ + Type: TypeString, + Required: true, + }, + "fum": &Schema{ + Type: TypeString, + Required: true, + }, + }, + }, + }, + Value: []interface{}{ + map[string]interface{}{ + "fo": "bar", + }, + map[string]interface{}{ + "fo": "baz", + "fum": "boz", + }, + }, + Expected: "(;;);", + }, + + testCase{ + Schema: &Schema{ + Type: TypeSet, + Elem: &Schema{ + Type: TypeString, + }, + }, + Value: NewSet(func(i interface{}) int { return len(i.(string)) }, []interface{}{ + "hello", + "woo", + }), + Expected: "{woo;hello;};", + }, + + testCase{ + Schema: &Schema{ + Type: TypeMap, + Elem: &Schema{ + Type: TypeString, + }, + }, + Value: map[string]interface{}{ + "foo": "bar", + "baz": "foo", + }, + Expected: "[baz:foo;foo:bar;];", + }, + + testCase{ + Schema: &Resource{ + Schema: map[string]*Schema{ + "name": &Schema{ + Type: TypeString, + Required: true, + }, + "size": &Schema{ + Type: TypeInt, + Optional: true, + }, + "green": &Schema{ + Type: TypeBool, + Optional: true, + Computed: true, + }, + "upside_down": &Schema{ + Type: TypeBool, + Computed: true, + }, + }, + }, + Value: map[string]interface{}{ + "name": "my-fun-database", + "size": 12, + "green": true, + }, + Expected: "green:1;name:my-fun-database;size:12;", + }, + } + + for _, test := range tests { + var gotBuf bytes.Buffer + schema := test.Schema + + switch s := schema.(type) { + case *Schema: + SerializeValueForHash(&gotBuf, test.Value, s) + case *Resource: + SerializeResourceForHash(&gotBuf, test.Value, s) + } + + got := gotBuf.String() + if got != test.Expected { + t.Errorf("hash(%#v) got %#v, but want %#v", test.Value, got, test.Expected) + } + } +} diff --git a/helper/schema/set.go b/helper/schema/set.go index 8d21866df2e1..e070a1eb9f17 100644 --- a/helper/schema/set.go +++ b/helper/schema/set.go @@ -1,6 +1,7 @@ package schema import ( + "bytes" "fmt" "reflect" "sort" @@ -15,6 +16,28 @@ func HashString(v interface{}) int { return hashcode.String(v.(string)) } +// HashResource hashes complex structures that are described using +// a *Resource. This is the default set implementation used when a set's +// element type is a full resource. +func HashResource(resource *Resource) SchemaSetFunc { + return func(v interface{}) int { + var buf bytes.Buffer + SerializeResourceForHash(&buf, v, resource) + return hashcode.String(buf.String()) + } +} + +// HashSchema hashes values that are described using a *Schema. This is the +// default set implementation used when a set's element type is a single +// schema. +func HashSchema(schema *Schema) SchemaSetFunc { + return func(v interface{}) int { + var buf bytes.Buffer + SerializeValueForHash(&buf, v, schema) + return hashcode.String(buf.String()) + } +} + // Set is a set data structure that is returned for elements of type // TypeSet. type Set struct {