Merge pull request #11732 from hashicorp/b-diff-map-removal

terraform: ignore RequiresNew for collection removal in diff.Same

terraform/diff.go

@@ -644,7 +644,45 @@ func (d *InstanceDiff) Same(d2 *InstanceDiff) (bool, string) {
 	newNew := d2.RequiresNew()
 	if oldNew && !newNew {
 		oldNew = false
-		for _, rd := range d.Attributes {
+
+		// This section builds a list of ignorable attributes for requiresNew
+		// by removing off any elements of collections going to zero elements.
+		// For collections going to zero, they may not exist at all in the
+		// new diff (and hence RequiresNew == false).
+		ignoreAttrs := make(map[string]struct{})
+		for k, diffOld := range d.Attributes {
+			if !strings.HasSuffix(k, ".%") && !strings.HasSuffix(k, ".#") {
+				continue
+			}
+
+			// This case is in here as a protection measure. The bug that this
+			// code originally fixed (GH-11349) didn't have to deal with computed
+			// so I'm not 100% sure what the correct behavior is. Best to leave
+			// the old behavior.
+			if diffOld.NewComputed {
+				continue
+			}
+
+			// We're looking for the case a map goes to exactly 0.
+			if diffOld.New != "0" {
+				continue
+			}
+
+			// Found it! Ignore all of these. The prefix here is stripping
+			// off the "%" so it is just "k."
+			prefix := k[:len(k)-1]
+			for k2, _ := range d.Attributes {
+				if strings.HasPrefix(k2, prefix) {
+					ignoreAttrs[k2] = struct{}{}
+				}
+			}
+		}
+
+		for k, rd := range d.Attributes {
+			if _, ok := ignoreAttrs[k]; ok {
+				continue
+			}
+
 			// If the field is requires new and NOT computed, then what
 			// we have is a diff mismatch for sure. We set that the old
 			// diff does REQUIRE a ForceNew.

terraform/diff_test.go

@@ -1,6 +1,7 @@
 package terraform
 
 import (
+	"fmt"
 	"reflect"
 	"strings"
 	"testing"
@@ -1085,18 +1086,65 @@ func TestInstanceDiffSame(t *testing.T) {
 			true,
 			"",
 		},
+
+		// When removing all collection items, the diff is allowed to contain
+		// nothing when re-creating the resource. This should be the "Same"
+		// since we said we were going from 1 to 0.
+		{
+			&InstanceDiff{
+				Attributes: map[string]*ResourceAttrDiff{
+					"foo.%": &ResourceAttrDiff{
+						Old:         "1",
+						New:         "0",
+						RequiresNew: true,
+					},
+					"foo.bar": &ResourceAttrDiff{
+						Old:         "baz",
+						New:         "",
+						NewRemoved:  true,
+						RequiresNew: true,
+					},
+				},
+			},
+			&InstanceDiff{},
+			true,
+			"",
+		},
+
+		{
+			&InstanceDiff{
+				Attributes: map[string]*ResourceAttrDiff{
+					"foo.#": &ResourceAttrDiff{
+						Old:         "1",
+						New:         "0",
+						RequiresNew: true,
+					},
+					"foo.0": &ResourceAttrDiff{
+						Old:         "baz",
+						New:         "",
+						NewRemoved:  true,
+						RequiresNew: true,
+					},
+				},
+			},
+			&InstanceDiff{},
+			true,
+			"",
+		},
 	}
 
 	for i, tc := range cases {
-		same, reason := tc.One.Same(tc.Two)
-		if same != tc.Same {
-			t.Fatalf("%d: expected same: %t, got %t (%s)\n\n one: %#v\n\ntwo: %#v",
-				i, tc.Same, same, reason, tc.One, tc.Two)
-		}
-		if reason != tc.Reason {
-			t.Fatalf(
-				"%d: bad reason\n\nexpected: %#v\n\ngot: %#v", i, tc.Reason, reason)
-		}
+		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
+			same, reason := tc.One.Same(tc.Two)
+			if same != tc.Same {
+				t.Fatalf("%d: expected same: %t, got %t (%s)\n\n one: %#v\n\ntwo: %#v",
+					i, tc.Same, same, reason, tc.One, tc.Two)
+			}
+			if reason != tc.Reason {
+				t.Fatalf(
+					"%d: bad reason\n\nexpected: %#v\n\ngot: %#v", i, tc.Reason, reason)
+			}
+		})
 	}
 }