Merge pull request #17463 from hashicorp/b-helper-r-diff-nested-keys

helper/schema: Allow ResourceDiff.ForceNew on nested fields (avoid crash)

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.

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))
+			}
+		})
+	}
+}