diff --git a/terraform/diff.go b/terraform/diff.go index d50ec5b10..5cf1b78ce 100644 --- a/terraform/diff.go +++ b/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. diff --git a/terraform/diff_test.go b/terraform/diff_test.go index 8f29ad08d..a74cbbecd 100644 --- a/terraform/diff_test.go +++ b/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) + } + }) } }