From e50be82da4e1fa4380f867d2b380807d1a897377 Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Sat, 2 Mar 2019 11:21:59 -0500
Subject: [PATCH 1/2] don't add empty blocks in ProposedNewObject

If the config contained a null block, don't convert it to a block with
empty values for the proposed new value.
---
 plans/objchange/objchange.go      |  7 ++++++
 plans/objchange/objchange_test.go | 39 +++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/plans/objchange/objchange.go b/plans/objchange/objchange.go
index 15565a400..ed0f4ba5e 100644
--- a/plans/objchange/objchange.go
+++ b/plans/objchange/objchange.go
@@ -25,6 +25,13 @@ import (
 // produce strange results with more "extreme" cases, such as a nested set
 // block where _all_ attributes are computed.
 func ProposedNewObject(schema *configschema.Block, prior, config cty.Value) cty.Value {
+	// If the config and prior are both null, return early here before
+	// populating the prior block. The prevents non-null blocks from appearing
+	// the proposed state value.
+	if config.IsNull() && prior.IsNull() {
+		return prior
+	}
+
 	if prior.IsNull() {
 		// In this case, we will construct a synthetic prior value that is
 		// similar to the result of decoding an empty configuration block,
diff --git a/plans/objchange/objchange_test.go b/plans/objchange/objchange_test.go
index 91f9df1f2..97f749aca 100644
--- a/plans/objchange/objchange_test.go
+++ b/plans/objchange/objchange_test.go
@@ -83,6 +83,45 @@ func TestProposedNewObject(t *testing.T) {
 				}),
 			}),
 		},
+		"null block remains null": {
+			&configschema.Block{
+				Attributes: map[string]*configschema.Attribute{
+					"foo": {
+						Type:     cty.String,
+						Optional: true,
+					},
+				},
+				BlockTypes: map[string]*configschema.NestedBlock{
+					"baz": {
+						Nesting: configschema.NestingSingle,
+						Block: configschema.Block{
+							Attributes: map[string]*configschema.Attribute{
+								"boz": {
+									Type:     cty.String,
+									Optional: true,
+									Computed: true,
+								},
+							},
+						},
+					},
+				},
+			},
+			cty.NullVal(cty.DynamicPseudoType),
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.StringVal("bar"),
+				"baz": cty.NullVal(cty.Object(map[string]cty.Type{
+					"boz": cty.String,
+				})),
+			}),
+			// The baz block does not exist in the config, and therefore
+			// shouldn't be planned.
+			cty.ObjectVal(map[string]cty.Value{
+				"foo": cty.StringVal("bar"),
+				"baz": cty.NullVal(cty.Object(map[string]cty.Type{
+					"boz": cty.String,
+				})),
+			}),
+		},
 		"no prior with set": {
 			// This one is here because our handling of sets is more complex
 			// than others (due to the fuzzy correlation heuristic) and

From 33d5ddf291d68ccaf6416b02f698ef362b5d01db Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Sat, 2 Mar 2019 11:30:37 -0500
Subject: [PATCH 2/2] remove empty timeouts blocks in copyTimeoutValues

The hcl2shims will always add in the timeouts block, because there's no
way to differentiate a null single block from an empty one in the
flatmapped state. Since we are only concerned with keeping the prior
timeouts value, always set the new value to null, and then copy over the
prior value if it exists.
---
 helper/plugin/grpc_provider.go | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go
index b1b4b5d3d..85e1a3d75 100644
--- a/helper/plugin/grpc_provider.go
+++ b/helper/plugin/grpc_provider.go
@@ -1125,22 +1125,34 @@ func normalizeFlatmapContainers(prior map[string]string, attrs map[string]string
 // the Private/Meta fields. we need to copy those values into the planned state
 // so that core doesn't see a perpetual diff with the timeout block.
 func copyTimeoutValues(to cty.Value, from cty.Value) cty.Value {
-	// if `from` is null, then there are no attributes, and if `to` is null we
-	// are planning to remove it altogether.
-	if from.IsNull() || to.IsNull() {
+	// if `to` is null we are planning to remove it altogether.
+	if to.IsNull() {
 		return to
 	}
+	toAttrs := to.AsValueMap()
+	// We need to remove the key since the hcl2shims will add a non-null block
+	// because we can't determine if a single block was null from the flatmapped
+	// values. This needs to conform to the correct schema for marshaling, so
+	// change the value to null rather than deleting it from the object map.
+	timeouts, ok := toAttrs[schema.TimeoutsConfigKey]
+	if ok {
+		toAttrs[schema.TimeoutsConfigKey] = cty.NullVal(timeouts.Type())
+	}
+
+	// if from is null then there are no timeouts to copy
+	if from.IsNull() {
+		return cty.ObjectVal(toAttrs)
+	}
 
 	fromAttrs := from.AsValueMap()
-	timeouts, ok := fromAttrs[schema.TimeoutsConfigKey]
+	timeouts, ok = fromAttrs[schema.TimeoutsConfigKey]
 
-	// no timeouts to copy
-	// timeouts shouldn't be unknown, but don't copy possibly invalid values
+	// timeouts shouldn't be unknown, but don't copy possibly invalid values either
 	if !ok || timeouts.IsNull() || !timeouts.IsWhollyKnown() {
-		return to
+		// no timeouts block to copy
+		return cty.ObjectVal(toAttrs)
 	}
 
-	toAttrs := to.AsValueMap()
 	toAttrs[schema.TimeoutsConfigKey] = timeouts
 
 	return cty.ObjectVal(toAttrs)