From 12378b4ee204ce408b783abc8ff7431c55f12ee7 Mon Sep 17 00:00:00 2001
From: Chris Marchesi <chrism@vancluevertech.com>
Date: Mon, 3 Jul 2017 10:19:35 -0700
Subject: [PATCH] terraform: Add value check to diff.Same

A diff new needs to pass basic value checks to be considered the
"same". Several provisions have been added to ensure that the list, set,
and RequiresNew behaviours that have needed some exceptions in the past
are preserved in this new logic.

This ensures that we are checking for value equality as much as
possible, which will be more important when we transition to the
possibility of diffs being sourced from external data.
---
 terraform/diff.go      | 37 +++++++++++++++++++++++++++++++++++--
 terraform/diff_test.go | 20 ++++++++++++++++++++
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/terraform/diff.go b/terraform/diff.go
index 9cbd5d88f..6972cacd9 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)
 
-		_, ok := d2.GetAttribute(k)
+		diffNew, 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,7 +837,40 @@ func (d *InstanceDiff) Same(d2 *InstanceDiff) (bool, string) {
 			}
 		}
 
-		// TODO: check for the same value if not computed
+		// 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.
+		//
+		// Single-value NewComputed values pass
+		if diffOld.NewComputed {
+			continue
+		}
+
+		// Computed keys for sets and lists pass
+		if strings.Contains(k, "~") {
+			continue
+		}
+
+		// Counts for sets need to be skipped as well as we have determined that we
+		// may not know the full value due to interpolation
+		if strings.HasSuffix(k, "#") {
+			continue
+		}
+
+		// Lists can be skipped if they are being removed (going from n > 0 to 0)
+		if strings.HasSuffix(k, "%") && diffOld.New == "0" && diffOld.Old != "0" {
+			continue
+		}
+
+		// If both old and new are RequiresNew, we are okay if both new values
+		// match.
+		if diffOld.RequiresNew && diffNew.RequiresNew && diffOld.New == diffNew.New {
+			continue
+		}
+
+		// At this point, we should be able to just check for deep equality.
+		if !reflect.DeepEqual(diffOld, diffNew) {
+			return false, fmt.Sprintf("value mismatch: %s", k)
+		}
 	}
 
 	// Check for leftover attributes
diff --git a/terraform/diff_test.go b/terraform/diff_test.go
index a74cbbecd..0e64e5d85 100644
--- a/terraform/diff_test.go
+++ b/terraform/diff_test.go
@@ -1131,6 +1131,26 @@ 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",
+		},
 	}
 
 	for i, tc := range cases {