From 55ef93f0f98f1b831d4326eb4540d79d73a38468 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Tue, 15 Nov 2016 11:26:42 -0800
Subject: [PATCH] terraform: Diff.Same should understand that Destroy might go
 false

---
 terraform/diff.go      | 33 +++++++++++++++++---
 terraform/diff_test.go | 70 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+), 5 deletions(-)

diff --git a/terraform/diff.go b/terraform/diff.go
index a474efad1..002f72921 100644
--- a/terraform/diff.go
+++ b/terraform/diff.go
@@ -606,14 +606,37 @@ func (d *InstanceDiff) Same(d2 *InstanceDiff) (bool, string) {
 	d.mu.Lock()
 	defer d.mu.Unlock()
 
-	if d.Destroy != d2.GetDestroy() {
+	// If we're going from requiring new to NOT requiring new, then we have
+	// to see if all required news were computed. If so, it is allowed since
+	// computed may also mean "same value and therefore not new".
+	oldNew := d.requiresNew()
+	newNew := d2.RequiresNew()
+	if oldNew && !newNew {
+		oldNew = false
+		for _, rd := range d.Attributes {
+			// 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.
+			if rd != nil && rd.RequiresNew && !rd.NewComputed {
+				oldNew = true
+				break
+			}
+		}
+	}
+
+	if oldNew != newNew {
+		return false, fmt.Sprintf(
+			"diff RequiresNew; old: %t, new: %t", oldNew, newNew)
+	}
+
+	// Verify that destroy matches. The second boolean here allows us to
+	// have mismatching Destroy if we're moving from RequiresNew true
+	// to false above. Therefore, the second boolean will only pass if
+	// we're moving from Destroy: true to false as well.
+	if d.Destroy != d2.GetDestroy() && d.requiresNew() == oldNew {
 		return false, fmt.Sprintf(
 			"diff: Destroy; old: %t, new: %t", d.Destroy, d2.GetDestroy())
 	}
-	if d.requiresNew() != d2.RequiresNew() {
-		return false, fmt.Sprintf(
-			"diff RequiresNew; old: %t, new: %t", d.requiresNew(), d2.RequiresNew())
-	}
 
 	// Go through the old diff and make sure the new diff has all the
 	// same attributes. To start, build up the check map to be all the keys.
diff --git a/terraform/diff_test.go b/terraform/diff_test.go
index 655b165f9..ae5ce6da4 100644
--- a/terraform/diff_test.go
+++ b/terraform/diff_test.go
@@ -750,6 +750,76 @@ func TestInstanceDiffSame(t *testing.T) {
 			"",
 		},
 
+		// Computed can change RequiresNew by removal, and that's okay
+		{
+			&InstanceDiff{
+				Attributes: map[string]*ResourceAttrDiff{
+					"foo.#": &ResourceAttrDiff{
+						Old:         "0",
+						NewComputed: true,
+						RequiresNew: true,
+					},
+				},
+			},
+			&InstanceDiff{
+				Attributes: map[string]*ResourceAttrDiff{},
+			},
+			true,
+			"",
+		},
+
+		// Computed can change Destroy by removal, and that's okay
+		{
+			&InstanceDiff{
+				Attributes: map[string]*ResourceAttrDiff{
+					"foo.#": &ResourceAttrDiff{
+						Old:         "0",
+						NewComputed: true,
+						RequiresNew: true,
+					},
+				},
+
+				Destroy: true,
+			},
+			&InstanceDiff{
+				Attributes: map[string]*ResourceAttrDiff{},
+			},
+			true,
+			"",
+		},
+
+		// Computed can change Destroy by elements
+		{
+			&InstanceDiff{
+				Attributes: map[string]*ResourceAttrDiff{
+					"foo.#": &ResourceAttrDiff{
+						Old:         "0",
+						NewComputed: true,
+						RequiresNew: true,
+					},
+				},
+
+				Destroy: true,
+			},
+			&InstanceDiff{
+				Attributes: map[string]*ResourceAttrDiff{
+					"foo.#": &ResourceAttrDiff{
+						Old: "1",
+						New: "1",
+					},
+					"foo.12": &ResourceAttrDiff{
+						Old:         "4",
+						New:         "12",
+						RequiresNew: true,
+					},
+				},
+
+				Destroy: true,
+			},
+			true,
+			"",
+		},
+
 		// Computed sets may not contain all fields in the original diff, and
 		// because multiple entries for the same set can compute to the same
 		// hash before the values are computed or interpolated, the overall