From f52a6630f5c60b45cadb941a43c3630bf48104f6 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Wed, 3 Apr 2019 17:36:08 -0400
Subject: [PATCH 1/2] create a downstream failure from a computed value

These are the largest source of the old "diffs didn't match after apply"
errors. It's almost always an upstream dependency that caused the final
error.
---
 builtin/providers/test/resource.go      |  4 +-
 builtin/providers/test/resource_test.go | 63 +++++++++++++++++++++++--
 2 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/builtin/providers/test/resource.go b/builtin/providers/test/resource.go
index 32f1049c70..355831046c 100644
--- a/builtin/providers/test/resource.go
+++ b/builtin/providers/test/resource.go
@@ -19,7 +19,7 @@ func testResource() *schema.Resource {
 		},
 
 		CustomizeDiff: func(d *schema.ResourceDiff, _ interface{}) error {
-			if d.HasChange("required") {
+			if d.HasChange("optional") {
 				d.SetNewComputed("planned_computed")
 			}
 			return nil
@@ -176,7 +176,7 @@ func testResourceRead(d *schema.ResourceData, meta interface{}) error {
 	d.Set("computed_list", []string{"listval1", "listval2"})
 	d.Set("computed_set", []string{"setval1", "setval2"})
 
-	d.Set("planned_computed", d.Get("required"))
+	d.Set("planned_computed", d.Get("optional"))
 
 	// if there is no "set" value, erroneously set it to an empty set. This
 	// might change a null value to an empty set, but we should be able to
diff --git a/builtin/providers/test/resource_test.go b/builtin/providers/test/resource_test.go
index 3c64dc5945..c85bb338e3 100644
--- a/builtin/providers/test/resource_test.go
+++ b/builtin/providers/test/resource_test.go
@@ -847,25 +847,27 @@ func TestResource_plannedComputed(t *testing.T) {
 			resource.TestStep{
 				Config: strings.TrimSpace(`
 resource "test_resource" "foo" {
-	required           = "ok"
+	required = "ok"
 	required_map = {
 	  key = "value"
 	}
+	optional = "hi"
 }
 				`),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(
-						"test_resource.foo", "planned_computed", "ok",
+						"test_resource.foo", "planned_computed", "hi",
 					),
 				),
 			},
 			resource.TestStep{
 				Config: strings.TrimSpace(`
 resource "test_resource" "foo" {
-	required           = "changed"
+	required = "ok"
 	required_map = {
 	  key = "value"
 	}
+	optional = "changed"
 }
 				`),
 				Check: resource.ComposeTestCheckFunc(
@@ -916,3 +918,58 @@ func TestDiffApply_map(t *testing.T) {
 		t.Fatalf("expected:%#v got:%#v", expect, newAttrs)
 	}
 }
+
+func TestResource_dependsComputed(t *testing.T) {
+	resource.UnitTest(t, resource.TestCase{
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckResourceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+variable "change" {
+	default = false
+}
+
+resource "test_resource" "foo" {
+	required = "ok"
+	required_map = {
+	    key = "value"
+	}
+	optional = var.change ? "after" : ""
+}
+
+resource "test_resource" "bar" {
+	count = var.change ? 1 : 0
+	required = test_resource.foo.planned_computed
+	required_map = {
+		key = "value"
+	}
+}
+				`),
+			},
+			resource.TestStep{
+				Config: strings.TrimSpace(`
+variable "change" {
+	default = true
+}
+
+resource "test_resource" "foo" {
+	required = "ok"
+	required_map = {
+	    key = "value"
+	}
+	optional = var.change ? "after" : ""
+}
+
+resource "test_resource" "bar" {
+	count = var.change ? 1 : 0
+	required = test_resource.foo.planned_computed
+	required_map = {
+		key = "value"
+	}
+}
+				`),
+			},
+		},
+	})
+}

From 7b6710540795f20b7d36affd735b9b0dcd3c7520 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Wed, 3 Apr 2019 17:37:58 -0400
Subject: [PATCH 2/2] don't strip new-computeds from plan diffs

Stripping these was a patch for some provider behavior which was fixed
in other ways, and is no longer needed.
Removing this allows us to implement correct CusomizeDiffFuncs in
providers so that they can mark fields with empty values as computed
during a plan.
---
 helper/plugin/grpc_provider.go | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go
index f7ace8b5a3..de90d88ac1 100644
--- a/helper/plugin/grpc_provider.go
+++ b/helper/plugin/grpc_provider.go
@@ -611,17 +611,6 @@ func (s *GRPCProviderServer) PlanResourceChange(_ context.Context, req *proto.Pl
 		priorState = &terraform.InstanceState{}
 	}
 
-	// if we're not creating a new resource, remove any new computed fields
-	if !create {
-		for attr, d := range diff.Attributes {
-			// If there's no change, then don't let this go through as NewComputed.
-			// This usually only happens when Old and New are both empty.
-			if d.NewComputed && d.Old == d.New {
-				delete(diff.Attributes, attr)
-			}
-		}
-	}
-
 	// now we need to apply the diff to the prior state, so get the planned state
 	plannedAttrs, err := diff.Apply(priorState.Attributes, res.CoreConfigSchemaWhenShimmed())
 	schema.FixupAsSingleInstanceStateOut(