From dbe22181ae7f8225372df5b5e75db36754714ffb Mon Sep 17 00:00:00 2001
From: James Bardin <j.bardin@gmail.com>
Date: Wed, 12 Jun 2019 18:27:50 -0400
Subject: [PATCH] Ensure all object attrs & empty blocks in upgrade

When upgrading from a flatmap state, unset blocks would not exist in the
state, while they will represented as empty in the new cty.Value. This
will cause an unexpected diff in the first plan after upgrade. This
situation may normally be applied with no impact, but some providers may
have unexpected behavior, and if the attributes force replacement it may
require manual alteration of the state to complete the upgrade.
---
 helper/plugin/grpc_provider.go      | 12 ++++++++++++
 helper/plugin/grpc_provider_test.go | 17 +++++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/helper/plugin/grpc_provider.go b/helper/plugin/grpc_provider.go
index 161af4d461..2175484789 100644
--- a/helper/plugin/grpc_provider.go
+++ b/helper/plugin/grpc_provider.go
@@ -15,6 +15,7 @@ import (
 	"github.com/hashicorp/terraform/configs/configschema"
 	"github.com/hashicorp/terraform/helper/schema"
 	proto "github.com/hashicorp/terraform/internal/tfplugin5"
+	"github.com/hashicorp/terraform/plans/objchange"
 	"github.com/hashicorp/terraform/plugin/convert"
 	"github.com/hashicorp/terraform/terraform"
 )
@@ -283,6 +284,17 @@ func (s *GRPCProviderServer) UpgradeResourceState(_ context.Context, req *proto.
 		return resp, nil
 	}
 
+	// Now we need to make sure blocks are represented correctly, which means
+	// that missing blocks are empty collections, rather than null.
+	// First we need to CoerceValue to ensure that all object types match.
+	val, err = schemaBlock.CoerceValue(val)
+	if err != nil {
+		resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
+		return resp, nil
+	}
+	// Normalize the value and fill in any missing blocks.
+	val = objchange.NormalizeObjectFromLegacySDK(val, schemaBlock)
+
 	// encode the final state to the expected msgpack format
 	newStateMP, err := msgpack.Marshal(val, schemaBlock.ImpliedType())
 	if err != nil {
diff --git a/helper/plugin/grpc_provider_test.go b/helper/plugin/grpc_provider_test.go
index 89858913f2..91b205a59a 100644
--- a/helper/plugin/grpc_provider_test.go
+++ b/helper/plugin/grpc_provider_test.go
@@ -262,6 +262,18 @@ func TestUpgradeState_flatmapState(t *testing.T) {
 				Type:     schema.TypeInt,
 				Required: true,
 			},
+			"block": {
+				Type:     schema.TypeList,
+				Optional: true,
+				Elem: &schema.Resource{
+					Schema: map[string]*schema.Schema{
+						"attr": {
+							Type:     schema.TypeString,
+							Optional: true,
+						},
+					},
+				},
+			},
 		},
 		// this MigrateState will take the state to version 2
 		MigrateState: func(v int, is *terraform.InstanceState, _ interface{}) (*terraform.InstanceState, error) {
@@ -426,8 +438,9 @@ func TestUpgradeState_flatmapState(t *testing.T) {
 			}
 
 			expected := cty.ObjectVal(map[string]cty.Value{
-				"id":   cty.StringVal("bar"),
-				"four": cty.NumberIntVal(4),
+				"block": cty.ListValEmpty(cty.Object(map[string]cty.Type{"attr": cty.String})),
+				"id":    cty.StringVal("bar"),
+				"four":  cty.NumberIntVal(4),
 			})
 
 			if !cmp.Equal(expected, val, valueComparer, equateEmpty) {