From c003e8f9a62cde8e4d9a00e71f1b9c55943bb63f Mon Sep 17 00:00:00 2001
From: Martin Atkins <mart@degeneration.co.uk>
Date: Fri, 3 Nov 2017 15:33:40 -0700
Subject: [PATCH] core: don't compare attribute values in Diff.Same

We previously didn't compare values but had a TODO to start doing so,
which we then recently did. Unfortunately it turns out that we _depend_
on not comparing values here, because when we use EvalCompareDiff (a key
user of Diff.Same) we pass in a diff made from a fresh re-interpolation
of the configuration and so any non-pure function results (timestamp,
uuid) have produced different values.
---
 terraform/diff.go      | 35 +++++++++--------------------------
 terraform/diff_test.go | 20 --------------------
 2 files changed, 9 insertions(+), 46 deletions(-)

diff --git a/terraform/diff.go b/terraform/diff.go
index 5e51ff36b..d6dc55061 100644
--- a/terraform/diff.go
+++ b/terraform/diff.go
@@ -748,7 +748,7 @@ func (d *InstanceDiff) Same(d2 *InstanceDiff) (bool, string) {
 		delete(checkOld, k)
 		delete(checkNew, k)
 
-		diffNew, ok := d2.GetAttribute(k)
+		_, ok := d2.GetAttribute(k)
 		if !ok {
 			// If there's no new attribute, and the old diff expected the attribute
 			// to be removed, that's just fine.
@@ -837,31 +837,14 @@ func (d *InstanceDiff) Same(d2 *InstanceDiff) (bool, string) {
 			}
 		}
 
-		// If our attributes are not computed, then there is no reason why we can't
-		// check to make sure the diff values are the same. Do that now.
-		//
-		// There are several conditions that we need to pass here as they are
-		// allowed cases even if values don't match, so let's check those first.
-		switch {
-		case diffOld.NewComputed:
-			// NewComputed values pass
-		case strings.Contains(k, "~"):
-			// Computed keys for sets and lists
-		case strings.HasSuffix(k, "#"):
-			// Counts for sets need to be skipped as well as we have determined that
-			// we may not know the full value due to interpolation
-		case strings.HasSuffix(k, "%") && diffOld.New == "0" && diffOld.Old != "0":
-			// Lists can be skipped if they are being removed (going from n > 0 to 0)
-		case d.DestroyTainted && d2.GetDestroyTainted() && diffOld.New == diffNew.New:
-			// Same for DestoryTainted
-		case d.requiresNew() && d2.RequiresNew() && diffOld.New == diffNew.New:
-			// Same for RequiresNew
-		default:
-			// Anything that gets here should be able to be checked for deep equality.
-			if !reflect.DeepEqual(diffOld, diffNew) {
-				return false, fmt.Sprintf("value mismatch: %s", k)
-			}
-		}
+		// We don't compare the values because we can't currently actually
+		// guarantee to generate the same value two two diffs created from
+		// the same state+config: we have some pesky interpolation functions
+		// that do not behave as pure functions (uuid, timestamp) and so they
+		// can be different each time a diff is produced.
+		// FIXME: Re-organize our config handling so that we don't re-evaluate
+		// expressions when we produce a second comparison diff during
+		// apply (for EvalCompareDiff).
 	}
 
 	// Check for leftover attributes
diff --git a/terraform/diff_test.go b/terraform/diff_test.go
index 90bf49038..d88f383c5 100644
--- a/terraform/diff_test.go
+++ b/terraform/diff_test.go
@@ -1131,26 +1131,6 @@ func TestInstanceDiffSame(t *testing.T) {
 			true,
 			"",
 		},
-		{
-			&InstanceDiff{
-				Attributes: map[string]*ResourceAttrDiff{
-					"foo": &ResourceAttrDiff{
-						Old: "1",
-						New: "2",
-					},
-				},
-			},
-			&InstanceDiff{
-				Attributes: map[string]*ResourceAttrDiff{
-					"foo": &ResourceAttrDiff{
-						Old: "1",
-						New: "3",
-					},
-				},
-			},
-			false,
-			"value mismatch: foo",
-		},
 
 		// Make sure that DestroyTainted diffs pass as well, especially when diff
 		// two works off of no state.