From ccf8a31cbb12f5d56398cdd0524a519726f7cc54 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Wed, 28 Feb 2018 12:55:14 +0000 Subject: [PATCH] helper/schema: Allow ResourceDiff.ForceNew on nested fields (avoid crash) --- helper/schema/resource_diff.go | 34 +++++- helper/schema/resource_diff_test.go | 183 ++++++++++++++++++++++++++++ 2 files changed, 213 insertions(+), 4 deletions(-) diff --git a/helper/schema/resource_diff.go b/helper/schema/resource_diff.go index 822d0dc4d..7b3716dd7 100644 --- a/helper/schema/resource_diff.go +++ b/helper/schema/resource_diff.go @@ -223,9 +223,11 @@ func (d *ResourceDiff) Clear(key string) error { func (d *ResourceDiff) clear(key string) error { // Check the schema to make sure that this key exists first. - if _, ok := d.schema[key]; !ok { + schemaL := addrToSchema(strings.Split(key, "."), d.schema) + if len(schemaL) == 0 { return fmt.Errorf("%s is not a valid key", key) } + for k := range d.diff.Attributes { if strings.HasPrefix(k, key) { delete(d.diff.Attributes, k) @@ -234,6 +236,19 @@ func (d *ResourceDiff) clear(key string) error { return nil } +// GetChangedKeysPrefix helps to implement Resource.CustomizeDiff +// where we need to act on all nested fields +// without calling out each one separately +func (d *ResourceDiff) GetChangedKeysPrefix(prefix string) []string { + keys := make([]string, 0) + for k := range d.diff.Attributes { + if strings.HasPrefix(k, prefix) { + keys = append(keys, k) + } + } + return keys +} + // diffChange helps to implement resourceDiffer and derives its change values // from ResourceDiff's own change data, in addition to existing diff, config, and state. func (d *ResourceDiff) diffChange(key string) (interface{}, interface{}, bool, bool, bool) { @@ -309,9 +324,20 @@ func (d *ResourceDiff) ForceNew(key string) error { return fmt.Errorf("ForceNew: No changes for %s", key) } - _, new := d.GetChange(key) - d.schema[key].ForceNew = true - return d.setDiff(key, new, false) + keyParts := strings.Split(key, ".") + var schema *Schema + schemaL := addrToSchema(keyParts, d.schema) + if len(schemaL) > 0 { + schema = schemaL[len(schemaL)-1] + } else { + return fmt.Errorf("ForceNew: %s is not a valid key", key) + } + + schema.ForceNew = true + + // We need to set whole lists/sets/maps here + _, new := d.GetChange(keyParts[0]) + return d.setDiff(keyParts[0], new, false) } // Get hands off to ResourceData.Get. diff --git a/helper/schema/resource_diff_test.go b/helper/schema/resource_diff_test.go index 18ba40935..f5f14a125 100644 --- a/helper/schema/resource_diff_test.go +++ b/helper/schema/resource_diff_test.go @@ -2,6 +2,7 @@ package schema import ( "reflect" + "sort" "testing" "github.com/davecgh/go-spew/spew" @@ -29,6 +30,7 @@ type resourceDiffTestCase struct { OldValue interface{} NewValue interface{} Expected *terraform.InstanceDiff + ExpectedKeys []string ExpectedError bool } @@ -697,6 +699,69 @@ func TestForceNew(t *testing.T) { }, }, }, + resourceDiffTestCase{ + Name: "nested field", + Schema: map[string]*Schema{ + "foo": &Schema{ + Type: TypeList, + Required: true, + MaxItems: 1, + Elem: &Resource{ + Schema: map[string]*Schema{ + "bar": { + Type: TypeString, + Optional: true, + }, + "baz": { + Type: TypeString, + Optional: true, + }, + }, + }, + }, + }, + State: &terraform.InstanceState{ + Attributes: map[string]string{ + "foo.#": "1", + "foo.0.bar": "abc", + "foo.0.baz": "xyz", + }, + }, + Config: testConfig(t, map[string]interface{}{ + "foo": []map[string]interface{}{ + map[string]interface{}{ + "bar": "abcdefg", + "baz": "changed", + }, + }, + }), + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "foo.0.bar": &terraform.ResourceAttrDiff{ + Old: "abc", + New: "abcdefg", + }, + "foo.0.baz": &terraform.ResourceAttrDiff{ + Old: "xyz", + New: "changed", + }, + }, + }, + Key: "foo.0.baz", + Expected: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "foo.0.bar": &terraform.ResourceAttrDiff{ + Old: "abc", + New: "abcdefg", + }, + "foo.0.baz": &terraform.ResourceAttrDiff{ + Old: "xyz", + New: "changed", + RequiresNew: true, + }, + }, + }, + }, } for _, tc := range cases { t.Run(tc.Name, func(t *testing.T) { @@ -851,3 +916,121 @@ func TestClear(t *testing.T) { }) } } + +func TestGetChangedKeysPrefix(t *testing.T) { + cases := []resourceDiffTestCase{ + resourceDiffTestCase{ + Name: "basic primitive diff", + Schema: map[string]*Schema{ + "foo": &Schema{ + Type: TypeString, + Optional: true, + Computed: true, + }, + }, + State: &terraform.InstanceState{ + Attributes: map[string]string{ + "foo": "bar", + }, + }, + Config: testConfig(t, map[string]interface{}{ + "foo": "baz", + }), + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "foo": &terraform.ResourceAttrDiff{ + Old: "bar", + New: "baz", + }, + }, + }, + Key: "foo", + ExpectedKeys: []string{ + "foo", + }, + }, + resourceDiffTestCase{ + Name: "nested field filtering", + Schema: map[string]*Schema{ + "testfield": &Schema{ + Type: TypeString, + Required: true, + }, + "foo": &Schema{ + Type: TypeList, + Required: true, + MaxItems: 1, + Elem: &Resource{ + Schema: map[string]*Schema{ + "bar": { + Type: TypeString, + Optional: true, + }, + "baz": { + Type: TypeString, + Optional: true, + }, + }, + }, + }, + }, + State: &terraform.InstanceState{ + Attributes: map[string]string{ + "testfield": "blablah", + "foo.#": "1", + "foo.0.bar": "abc", + "foo.0.baz": "xyz", + }, + }, + Config: testConfig(t, map[string]interface{}{ + "testfield": "modified", + "foo": []map[string]interface{}{ + map[string]interface{}{ + "bar": "abcdefg", + "baz": "changed", + }, + }, + }), + Diff: &terraform.InstanceDiff{ + Attributes: map[string]*terraform.ResourceAttrDiff{ + "testfield": &terraform.ResourceAttrDiff{ + Old: "blablah", + New: "modified", + }, + "foo.0.bar": &terraform.ResourceAttrDiff{ + Old: "abc", + New: "abcdefg", + }, + "foo.0.baz": &terraform.ResourceAttrDiff{ + Old: "xyz", + New: "changed", + }, + }, + }, + Key: "foo", + ExpectedKeys: []string{ + "foo.0.bar", + "foo.0.baz", + }, + }, + } + for _, tc := range cases { + t.Run(tc.Name, func(t *testing.T) { + m := schemaMap(tc.Schema) + d := newResourceDiff(m, tc.Config, tc.State, tc.Diff) + keys := d.GetChangedKeysPrefix(tc.Key) + + for _, k := range d.UpdatedKeys() { + if err := m.diff(k, m[k], tc.Diff, d, false); err != nil { + t.Fatalf("bad: %s", err) + } + } + + sort.Strings(keys) + + if !reflect.DeepEqual(tc.ExpectedKeys, keys) { + t.Fatalf("Expected %s, got %s", spew.Sdump(tc.ExpectedKeys), spew.Sdump(keys)) + } + }) + } +}