From 95d37ea79ca60ecac2e8141e663e7b0fea53e990 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Tue, 25 Oct 2016 22:36:59 -0400
Subject: [PATCH] helper/schema,terraform: handle computed primtives in diffs

Fixes #3309

There are two primary changes, one to how helper/schema creates diffs
and one to how Terraform compares diffs. Both require careful
understanding.

== 1. helper/schema Changes

helper/schema, given any primitive field (string, int, bool, etc.)
_used to_ create a basic diff when given a computed new value (i.e. from
an unkown interpolation). This would put in the plan that the old value
is whatever the old value was, and the new value was the actual
interpolation. For example, from #3309, the diff showed the following:

```
~ module.test.aws_eip.test-instance.0
    instance: "<INSTANCE ID>" => "${element(aws_instance.test-instance.*.id, count.index)}"
```

Then, when running `apply`, the diff would be realized and you would get
a diff mismatch error because it would realize the final value is the
same and remove it from the diff.

**The change:** `helper/schema` now marks unknown primitive values with
`NewComputed` set to true. Semantically this is correct for the diff to
have this information.

== 2. Terraform Diff.Same Changes

Next, the way Terraform compares diffs needed to be updated

Specifically, the case where the diff from the plan had a NewComputed
primitive and the diff from the apply _no longer has that value_. This
is possible if the computed value ended up being the same as the old
value. This is allowed to pass through.

Together, these fix #3309.
---
 helper/schema/schema.go      | 13 ++++++------
 helper/schema/schema_test.go | 10 +++++----
 terraform/diff.go            |  7 ++++++
 terraform/diff_test.go       | 41 ++++++++++++++++++++++++++++++++++++
 4 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/helper/schema/schema.go b/helper/schema/schema.go
index 452f0e08d..1a56ad5a3 100644
--- a/helper/schema/schema.go
+++ b/helper/schema/schema.go
@@ -995,7 +995,7 @@ func (m schemaMap) diffString(
 	all bool) error {
 	var originalN interface{}
 	var os, ns string
-	o, n, _, _ := d.diffChange(k)
+	o, n, _, computed := d.diffChange(k)
 	if schema.StateFunc != nil && n != nil {
 		originalN = n
 		n = schema.StateFunc(n)
@@ -1019,7 +1019,7 @@ func (m schemaMap) diffString(
 		}
 
 		// Otherwise, only continue if we're computed
-		if !schema.Computed {
+		if !schema.Computed && !computed {
 			return nil
 		}
 	}
@@ -1033,10 +1033,11 @@ func (m schemaMap) diffString(
 	}
 
 	diff.Attributes[k] = schema.finalizeDiff(&terraform.ResourceAttrDiff{
-		Old:        os,
-		New:        ns,
-		NewExtra:   originalN,
-		NewRemoved: removed,
+		Old:         os,
+		New:         ns,
+		NewExtra:    originalN,
+		NewRemoved:  removed,
+		NewComputed: computed,
 	})
 
 	return nil
diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go
index bb96c6a71..9511f363b 100644
--- a/helper/schema/schema_test.go
+++ b/helper/schema/schema_test.go
@@ -439,8 +439,9 @@ func TestSchemaMap_Diff(t *testing.T) {
 			Diff: &terraform.InstanceDiff{
 				Attributes: map[string]*terraform.ResourceAttrDiff{
 					"availability_zone": &terraform.ResourceAttrDiff{
-						Old: "",
-						New: "${var.foo}",
+						Old:         "",
+						New:         "${var.foo}",
+						NewComputed: true,
 					},
 				},
 			},
@@ -1675,8 +1676,9 @@ func TestSchemaMap_Diff(t *testing.T) {
 						New: "1",
 					},
 					"route.~1.gateway": &terraform.ResourceAttrDiff{
-						Old: "",
-						New: "${var.foo}",
+						Old:         "",
+						New:         "${var.foo}",
+						NewComputed: true,
 					},
 				},
 			},
diff --git a/terraform/diff.go b/terraform/diff.go
index 86be4bb5a..8cdde0a21 100644
--- a/terraform/diff.go
+++ b/terraform/diff.go
@@ -603,6 +603,13 @@ func (d *InstanceDiff) Same(d2 *InstanceDiff) (bool, string) {
 				continue
 			}
 
+			// If the last diff was a computed value then the absense of
+			// that value is allowed since it may mean the value ended up
+			// being the same.
+			if diffOld.NewComputed {
+				continue
+			}
+
 			// No exact match, but maybe this is a set containing computed
 			// values. So check if there is an approximate hash in the key
 			// and if so, try to match the key.
diff --git a/terraform/diff_test.go b/terraform/diff_test.go
index a9cb8b47b..a014787da 100644
--- a/terraform/diff_test.go
+++ b/terraform/diff_test.go
@@ -553,6 +553,47 @@ func TestInstanceDiffSame(t *testing.T) {
 			"diff RequiresNew; old: true, new: false",
 		},
 
+		// NewComputed on primitive
+		{
+			&InstanceDiff{
+				Attributes: map[string]*ResourceAttrDiff{
+					"foo": &ResourceAttrDiff{
+						Old:         "",
+						New:         "${var.foo}",
+						NewComputed: true,
+					},
+				},
+			},
+			&InstanceDiff{
+				Attributes: map[string]*ResourceAttrDiff{
+					"foo": &ResourceAttrDiff{
+						Old: "0",
+						New: "1",
+					},
+				},
+			},
+			true,
+			"",
+		},
+
+		// NewComputed on primitive, removed
+		{
+			&InstanceDiff{
+				Attributes: map[string]*ResourceAttrDiff{
+					"foo": &ResourceAttrDiff{
+						Old:         "",
+						New:         "${var.foo}",
+						NewComputed: true,
+					},
+				},
+			},
+			&InstanceDiff{
+				Attributes: map[string]*ResourceAttrDiff{},
+			},
+			true,
+			"",
+		},
+
 		{
 			&InstanceDiff{
 				Attributes: map[string]*ResourceAttrDiff{