From 1e81a3e7fa61b4085dffe5edc869aead9a57cd09 Mon Sep 17 00:00:00 2001
From: Brian Flad <bflad417@gmail.com>
Date: Tue, 4 Dec 2018 22:48:30 -0500
Subject: [PATCH] helper/schema: Always propagate NewComputed for previously
 zero value primative type attributes

When the following conditions were met:
* Schema attribute with a primative type (e.g. Type: TypeString) and Computed: true
* Old state of attribute set to zero value for type (e.g. "")
* Old state ID of resource set to non-empty (e.g. existing resource)

Attempting to use CustomizeDiff with SetNewComputed() would result in the difference  previously being discarded. This update ensures that previous zero values or resource existence does not influence the propagation of the computed update.

Previously:

```
--- FAIL: TestSetNewComputed (0.00s)
    --- FAIL: TestSetNewComputed/NewComputed_should_always_propagate (0.00s)
        resource_diff_test.go:684: Expected (*terraform.InstanceDiff)(0xc00051cea0)({
             mu: (sync.Mutex) {
              state: (int32) 0,
              sema: (uint32) 0
             },
             Attributes: (map[string]*terraform.ResourceAttrDiff) (len=1) {
              (string) (len=3) "foo": (*terraform.ResourceAttrDiff)(0xc0003dcec0)({
               Old: (string) "",
               New: (string) "",
               NewComputed: (bool) true,
               NewRemoved: (bool) false,
               NewExtra: (interface {}) <nil>,
               RequiresNew: (bool) false,
               Sensitive: (bool) false,
               Type: (terraform.DiffAttrType) 0
              })
             },
             Destroy: (bool) false,
             DestroyDeposed: (bool) false,
             DestroyTainted: (bool) false,
             Meta: (map[string]interface {}) <nil>
            })
            , got (*terraform.InstanceDiff)(0xc00051ce80)({
             mu: (sync.Mutex) {
              state: (int32) 0,
              sema: (uint32) 0
             },
             Attributes: (map[string]*terraform.ResourceAttrDiff) {
             },
             Destroy: (bool) false,
             DestroyDeposed: (bool) false,
             DestroyTainted: (bool) false,
             Meta: (map[string]interface {}) <nil>
            })

--- FAIL: TestSchemaMap_Diff (0.01s)
    --- FAIL: TestSchemaMap_Diff/79-NewComputed_should_always_propagate_with_CustomizeDiff (0.00s)
        schema_test.go:3289: expected:
            *terraform.InstanceDiff{mu:sync.Mutex{state:0, sema:0x0}, Attributes:map[string]*terraform.ResourceAttrDiff{"foo":*terraform.ResourceAttrDiff{Old:"", New:"", NewComputed:true, NewRemoved:false, NewExtra:interface {}(nil), RequiresNew:false, Sensitive:false, Type:0x0}}, Destroy:false, DestroyDeposed:false, DestroyTainted:false, Meta:map[string]interface {}(nil)}

            got:
            <nil>

FAIL
FAIL  github.com/hashicorp/terraform/helper/schema  0.825s
```
---
 helper/schema/resource_diff_test.go | 31 ++++++++++++++++++++++++++
 helper/schema/schema.go             |  4 ++--
 helper/schema/schema_test.go        | 34 +++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/helper/schema/resource_diff_test.go b/helper/schema/resource_diff_test.go
index 52c02a6ab..aa3a4e7df 100644
--- a/helper/schema/resource_diff_test.go
+++ b/helper/schema/resource_diff_test.go
@@ -599,6 +599,37 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 				},
 			},
 		},
+		resourceDiffTestCase{
+			Name: "NewComputed should always propagate",
+			Schema: map[string]*Schema{
+				"foo": &Schema{
+					Type:     TypeString,
+					Computed: true,
+				},
+			},
+			State: &terraform.InstanceState{
+				Attributes: map[string]string{
+					"foo": "",
+				},
+				ID: "pre-existing",
+			},
+			Config:   testConfig(t, map[string]interface{}{}),
+			Diff:     &terraform.InstanceDiff{Attributes: map[string]*terraform.ResourceAttrDiff{}},
+			Key:      "foo",
+			NewValue: "",
+			Expected: &terraform.InstanceDiff{
+				Attributes: func() map[string]*terraform.ResourceAttrDiff {
+					if computed {
+						return map[string]*terraform.ResourceAttrDiff{
+							"foo": &terraform.ResourceAttrDiff{
+								NewComputed: computed,
+							},
+						}
+					}
+					return map[string]*terraform.ResourceAttrDiff{}
+				}(),
+			},
+		},
 	}
 }
 
diff --git a/helper/schema/schema.go b/helper/schema/schema.go
index 286762bb1..de05df66f 100644
--- a/helper/schema/schema.go
+++ b/helper/schema/schema.go
@@ -1185,7 +1185,7 @@ func (m schemaMap) diffString(
 		return fmt.Errorf("%s: %s", k, err)
 	}
 
-	if os == ns && !all {
+	if os == ns && !all && !computed {
 		// They're the same value. If there old value is not blank or we
 		// have an ID, then return right away since we're already setup.
 		if os != "" || d.Id() != "" {
@@ -1193,7 +1193,7 @@ func (m schemaMap) diffString(
 		}
 
 		// Otherwise, only continue if we're computed
-		if !schema.Computed && !computed {
+		if !schema.Computed {
 			return nil
 		}
 	}
diff --git a/helper/schema/schema_test.go b/helper/schema/schema_test.go
index 9c505255c..a82f8b2d5 100644
--- a/helper/schema/schema_test.go
+++ b/helper/schema/schema_test.go
@@ -3126,6 +3126,40 @@ func TestSchemaMap_Diff(t *testing.T) {
 			Err: false,
 		},
 
+		{
+			Name: "NewComputed should always propagate with CustomizeDiff",
+			Schema: map[string]*Schema{
+				"foo": &Schema{
+					Type:     TypeString,
+					Computed: true,
+				},
+			},
+
+			State: &terraform.InstanceState{
+				Attributes: map[string]string{
+					"foo": "",
+				},
+				ID: "pre-existing",
+			},
+
+			Config: map[string]interface{}{},
+
+			CustomizeDiff: func(d *ResourceDiff, meta interface{}) error {
+				d.SetNewComputed("foo")
+				return nil
+			},
+
+			Diff: &terraform.InstanceDiff{
+				Attributes: map[string]*terraform.ResourceAttrDiff{
+					"foo": &terraform.ResourceAttrDiff{
+						NewComputed: true,
+					},
+				},
+			},
+
+			Err: false,
+		},
+
 		{
 			Name: "vetoing a diff",
 			Schema: map[string]*Schema{