Accept JSON encoded dynamic values from providers

Core was previously ignoring JSON-encoded dynamic values, but these are
technically supported, so we must either error or accept the value.
Since we already have the decoder for Json state, it's minimal effort to
support this on all plugin methods too.

This change also gives providers an easy way to implement the
UpgradeResourceState method. The obvious implementation of returning the same
JSON-encoded value has tripped up a few providers not using the legacy
SDK already, and we should have at least indicated that the value was
being lost.

plugin/grpc_provider.go

@@ -12,6 +12,7 @@ import (
 	proto "github.com/hashicorp/terraform/internal/tfplugin5"
 	"github.com/hashicorp/terraform/plugin/convert"
 	"github.com/hashicorp/terraform/providers"
+	ctyjson "github.com/zclconf/go-cty/cty/json"
 	"github.com/zclconf/go-cty/cty/msgpack"
 	"google.golang.org/grpc"
 )
@@ -185,13 +186,10 @@ func (p *GRPCProvider) PrepareProviderConfig(r providers.PrepareProviderConfigRe
 		return resp
 	}
 
-	config := cty.NullVal(ty)
+	config, err := decodeDynamicValue(protoResp.PreparedConfig, ty)
-	if protoResp.PreparedConfig != nil {
+	if err != nil {
-		config, err = msgpack.Unmarshal(protoResp.PreparedConfig.Msgpack, ty)
+		resp.Diagnostics = resp.Diagnostics.Append(err)
-		if err != nil {
+		return resp
-			resp.Diagnostics = resp.Diagnostics.Append(err)
-			return resp
-		}
 	}
 	resp.PreparedConfig = config
 
@@ -270,16 +268,19 @@ func (p *GRPCProvider) UpgradeResourceState(r providers.UpgradeResourceStateRequ
 	}
 	resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics))
 
-	state := cty.NullVal(resSchema.Block.ImpliedType())
+	ty := resSchema.Block.ImpliedType()
-	if protoResp.UpgradedState != nil {
+	resp.UpgradedState = cty.NullVal(ty)
-		state, err = msgpack.Unmarshal(protoResp.UpgradedState.Msgpack, resSchema.Block.ImpliedType())
+	if protoResp.UpgradedState == nil {
-		if err != nil {
+		return resp
-			resp.Diagnostics = resp.Diagnostics.Append(err)
-			return resp
-		}
 	}
 
+	state, err := decodeDynamicValue(protoResp.UpgradedState, ty)
+	if err != nil {
+		resp.Diagnostics = resp.Diagnostics.Append(err)
+		return resp
+	}
 	resp.UpgradedState = state
+
 	return resp
 }
 
@@ -361,13 +362,10 @@ func (p *GRPCProvider) ReadResource(r providers.ReadResourceRequest) (resp provi
 	}
 	resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics))
 
-	state := cty.NullVal(resSchema.Block.ImpliedType())
+	state, err := decodeDynamicValue(protoResp.NewState, resSchema.Block.ImpliedType())
-	if protoResp.NewState != nil {
+	if err != nil {
-		state, err = msgpack.Unmarshal(protoResp.NewState.Msgpack, resSchema.Block.ImpliedType())
+		resp.Diagnostics = resp.Diagnostics.Append(err)
-		if err != nil {
+		return resp
-			resp.Diagnostics = resp.Diagnostics.Append(err)
-			return resp
-		}
 	}
 	resp.NewState = state
 	resp.Private = protoResp.Private
@@ -423,13 +421,10 @@ func (p *GRPCProvider) PlanResourceChange(r providers.PlanResourceChangeRequest)
 	}
 	resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics))
 
-	state := cty.NullVal(resSchema.Block.ImpliedType())
+	state, err := decodeDynamicValue(protoResp.PlannedState, resSchema.Block.ImpliedType())
-	if protoResp.PlannedState != nil {
+	if err != nil {
-		state, err = msgpack.Unmarshal(protoResp.PlannedState.Msgpack, resSchema.Block.ImpliedType())
+		resp.Diagnostics = resp.Diagnostics.Append(err)
-		if err != nil {
+		return resp
-			resp.Diagnostics = resp.Diagnostics.Append(err)
-			return resp
-		}
 	}
 	resp.PlannedState = state
 
@@ -492,13 +487,10 @@ func (p *GRPCProvider) ApplyResourceChange(r providers.ApplyResourceChangeReques
 
 	resp.Private = protoResp.Private
 
-	state := cty.NullVal(resSchema.Block.ImpliedType())
+	state, err := decodeDynamicValue(protoResp.NewState, resSchema.Block.ImpliedType())
-	if protoResp.NewState != nil {
+	if err != nil {
-		state, err = msgpack.Unmarshal(protoResp.NewState.Msgpack, resSchema.Block.ImpliedType())
+		resp.Diagnostics = resp.Diagnostics.Append(err)
-		if err != nil {
+		return resp
-			resp.Diagnostics = resp.Diagnostics.Append(err)
-			return resp
-		}
 	}
 	resp.NewState = state
 
@@ -529,13 +521,10 @@ func (p *GRPCProvider) ImportResourceState(r providers.ImportResourceStateReques
 		}
 
 		resSchema := p.getResourceSchema(resource.TypeName)
-		state := cty.NullVal(resSchema.Block.ImpliedType())
+		state, err := decodeDynamicValue(imported.State, resSchema.Block.ImpliedType())
-		if imported.State != nil {
+		if err != nil {
-			state, err = msgpack.Unmarshal(imported.State.Msgpack, resSchema.Block.ImpliedType())
+			resp.Diagnostics = resp.Diagnostics.Append(err)
-			if err != nil {
+			return resp
-				resp.Diagnostics = resp.Diagnostics.Append(err)
-				return resp
-			}
 		}
 		resource.State = state
 		resp.ImportedResources = append(resp.ImportedResources, resource)
@@ -579,13 +568,10 @@ func (p *GRPCProvider) ReadDataSource(r providers.ReadDataSourceRequest) (resp p
 	}
 	resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics))
 
-	state := cty.NullVal(dataSchema.Block.ImpliedType())
+	state, err := decodeDynamicValue(protoResp.State, dataSchema.Block.ImpliedType())
-	if protoResp.State != nil {
+	if err != nil {
-		state, err = msgpack.Unmarshal(protoResp.State.Msgpack, dataSchema.Block.ImpliedType())
+		resp.Diagnostics = resp.Diagnostics.Append(err)
-		if err != nil {
+		return resp
-			resp.Diagnostics = resp.Diagnostics.Append(err)
-			return resp
-		}
 	}
 	resp.State = state
 
@@ -613,3 +599,18 @@ func (p *GRPCProvider) Close() error {
 	p.PluginClient.Kill()
 	return nil
 }
+
+// Decode a DynamicValue from either the JSON or MsgPack encoding.
+func decodeDynamicValue(v *proto.DynamicValue, ty cty.Type) (cty.Value, error) {
+	// always return a valid value
+	res := cty.NullVal(ty)
+	if v == nil {
+		return res, nil
+	}
+
+	if len(v.Msgpack) > 0 {
+		return msgpack.Unmarshal(v.Msgpack, ty)
+	}
+
+	return ctyjson.Unmarshal(v.Json, ty)
+}

plugin/grpc_provider_test.go

@@ -177,6 +177,37 @@ func TestGRPCProvider_UpgradeResourceState(t *testing.T) {
 	}
 }
 
+func TestGRPCProvider_UpgradeResourceStateJSON(t *testing.T) {
+	client := mockProviderClient(t)
+	p := &GRPCProvider{
+		client: client,
+	}
+
+	client.EXPECT().UpgradeResourceState(
+		gomock.Any(),
+		gomock.Any(),
+	).Return(&proto.UpgradeResourceState_Response{
+		UpgradedState: &proto.DynamicValue{
+			Json: []byte(`{"attr":"bar"}`),
+		},
+	}, nil)
+
+	resp := p.UpgradeResourceState(providers.UpgradeResourceStateRequest{
+		TypeName:     "resource",
+		Version:      0,
+		RawStateJSON: []byte(`{"old_attr":"bar"}`),
+	})
+	checkDiags(t, resp.Diagnostics)
+
+	expected := cty.ObjectVal(map[string]cty.Value{
+		"attr": cty.StringVal("bar"),
+	})
+
+	if !cmp.Equal(expected, resp.UpgradedState, typeComparer, valueComparer, equateEmpty) {
+		t.Fatal(cmp.Diff(expected, resp.UpgradedState, typeComparer, valueComparer, equateEmpty))
+	}
+}
+
 func TestGRPCProvider_Configure(t *testing.T) {
 	client := mockProviderClient(t)
 	p := &GRPCProvider{
@@ -246,6 +277,39 @@ func TestGRPCProvider_ReadResource(t *testing.T) {
 	}
 }
 
+func TestGRPCProvider_ReadResourceJSON(t *testing.T) {
+	client := mockProviderClient(t)
+	p := &GRPCProvider{
+		client: client,
+	}
+
+	client.EXPECT().ReadResource(
+		gomock.Any(),
+		gomock.Any(),
+	).Return(&proto.ReadResource_Response{
+		NewState: &proto.DynamicValue{
+			Json: []byte(`{"attr":"bar"}`),
+		},
+	}, nil)
+
+	resp := p.ReadResource(providers.ReadResourceRequest{
+		TypeName: "resource",
+		PriorState: cty.ObjectVal(map[string]cty.Value{
+			"attr": cty.StringVal("foo"),
+		}),
+	})
+
+	checkDiags(t, resp.Diagnostics)
+
+	expected := cty.ObjectVal(map[string]cty.Value{
+		"attr": cty.StringVal("bar"),
+	})
+
+	if !cmp.Equal(expected, resp.NewState, typeComparer, valueComparer, equateEmpty) {
+		t.Fatal(cmp.Diff(expected, resp.NewState, typeComparer, valueComparer, equateEmpty))
+	}
+}
+
 func TestGRPCProvider_PlanResourceChange(t *testing.T) {
 	client := mockProviderClient(t)
 	p := &GRPCProvider{
@@ -309,6 +373,69 @@ func TestGRPCProvider_PlanResourceChange(t *testing.T) {
 	}
 }
 
+func TestGRPCProvider_PlanResourceChangeJSON(t *testing.T) {
+	client := mockProviderClient(t)
+	p := &GRPCProvider{
+		client: client,
+	}
+
+	expectedPrivate := []byte(`{"meta": "data"}`)
+
+	client.EXPECT().PlanResourceChange(
+		gomock.Any(),
+		gomock.Any(),
+	).Return(&proto.PlanResourceChange_Response{
+		PlannedState: &proto.DynamicValue{
+			Json: []byte(`{"attr":"bar"}`),
+		},
+		RequiresReplace: []*proto.AttributePath{
+			{
+				Steps: []*proto.AttributePath_Step{
+					{
+						Selector: &proto.AttributePath_Step_AttributeName{
+							AttributeName: "attr",
+						},
+					},
+				},
+			},
+		},
+		PlannedPrivate: expectedPrivate,
+	}, nil)
+
+	resp := p.PlanResourceChange(providers.PlanResourceChangeRequest{
+		TypeName: "resource",
+		PriorState: cty.ObjectVal(map[string]cty.Value{
+			"attr": cty.StringVal("foo"),
+		}),
+		ProposedNewState: cty.ObjectVal(map[string]cty.Value{
+			"attr": cty.StringVal("bar"),
+		}),
+		Config: cty.ObjectVal(map[string]cty.Value{
+			"attr": cty.StringVal("bar"),
+		}),
+	})
+
+	checkDiags(t, resp.Diagnostics)
+
+	expectedState := cty.ObjectVal(map[string]cty.Value{
+		"attr": cty.StringVal("bar"),
+	})
+
+	if !cmp.Equal(expectedState, resp.PlannedState, typeComparer, valueComparer, equateEmpty) {
+		t.Fatal(cmp.Diff(expectedState, resp.PlannedState, typeComparer, valueComparer, equateEmpty))
+	}
+
+	expectedReplace := `[]cty.Path{cty.Path{cty.GetAttrStep{Name:"attr"}}}`
+	replace := fmt.Sprintf("%#v", resp.RequiresReplace)
+	if expectedReplace != replace {
+		t.Fatalf("expected %q, got %q", expectedReplace, replace)
+	}
+
+	if !bytes.Equal(expectedPrivate, resp.PlannedPrivate) {
+		t.Fatalf("expected %q, got %q", expectedPrivate, resp.PlannedPrivate)
+	}
+}
+
 func TestGRPCProvider_ApplyResourceChange(t *testing.T) {
 	client := mockProviderClient(t)
 	p := &GRPCProvider{
@@ -355,6 +482,52 @@ func TestGRPCProvider_ApplyResourceChange(t *testing.T) {
 		t.Fatalf("expected %q, got %q", expectedPrivate, resp.Private)
 	}
 }
+func TestGRPCProvider_ApplyResourceChangeJSON(t *testing.T) {
+	client := mockProviderClient(t)
+	p := &GRPCProvider{
+		client: client,
+	}
+
+	expectedPrivate := []byte(`{"meta": "data"}`)
+
+	client.EXPECT().ApplyResourceChange(
+		gomock.Any(),
+		gomock.Any(),
+	).Return(&proto.ApplyResourceChange_Response{
+		NewState: &proto.DynamicValue{
+			Json: []byte(`{"attr":"bar"}`),
+		},
+		Private: expectedPrivate,
+	}, nil)
+
+	resp := p.ApplyResourceChange(providers.ApplyResourceChangeRequest{
+		TypeName: "resource",
+		PriorState: cty.ObjectVal(map[string]cty.Value{
+			"attr": cty.StringVal("foo"),
+		}),
+		PlannedState: cty.ObjectVal(map[string]cty.Value{
+			"attr": cty.StringVal("bar"),
+		}),
+		Config: cty.ObjectVal(map[string]cty.Value{
+			"attr": cty.StringVal("bar"),
+		}),
+		PlannedPrivate: expectedPrivate,
+	})
+
+	checkDiags(t, resp.Diagnostics)
+
+	expectedState := cty.ObjectVal(map[string]cty.Value{
+		"attr": cty.StringVal("bar"),
+	})
+
+	if !cmp.Equal(expectedState, resp.NewState, typeComparer, valueComparer, equateEmpty) {
+		t.Fatal(cmp.Diff(expectedState, resp.NewState, typeComparer, valueComparer, equateEmpty))
+	}
+
+	if !bytes.Equal(expectedPrivate, resp.Private) {
+		t.Fatalf("expected %q, got %q", expectedPrivate, resp.Private)
+	}
+}
 
 func TestGRPCProvider_ImportResourceState(t *testing.T) {
 	client := mockProviderClient(t)
@@ -399,6 +572,49 @@ func TestGRPCProvider_ImportResourceState(t *testing.T) {
 		t.Fatal(cmp.Diff(expectedResource, imported, typeComparer, valueComparer, equateEmpty))
 	}
 }
+func TestGRPCProvider_ImportResourceStateJSON(t *testing.T) {
+	client := mockProviderClient(t)
+	p := &GRPCProvider{
+		client: client,
+	}
+
+	expectedPrivate := []byte(`{"meta": "data"}`)
+
+	client.EXPECT().ImportResourceState(
+		gomock.Any(),
+		gomock.Any(),
+	).Return(&proto.ImportResourceState_Response{
+		ImportedResources: []*proto.ImportResourceState_ImportedResource{
+			{
+				TypeName: "resource",
+				State: &proto.DynamicValue{
+					Json: []byte(`{"attr":"bar"}`),
+				},
+				Private: expectedPrivate,
+			},
+		},
+	}, nil)
+
+	resp := p.ImportResourceState(providers.ImportResourceStateRequest{
+		TypeName: "resource",
+		ID:       "foo",
+	})
+
+	checkDiags(t, resp.Diagnostics)
+
+	expectedResource := providers.ImportedResource{
+		TypeName: "resource",
+		State: cty.ObjectVal(map[string]cty.Value{
+			"attr": cty.StringVal("bar"),
+		}),
+		Private: expectedPrivate,
+	}
+
+	imported := resp.ImportedResources[0]
+	if !cmp.Equal(expectedResource, imported, typeComparer, valueComparer, equateEmpty) {
+		t.Fatal(cmp.Diff(expectedResource, imported, typeComparer, valueComparer, equateEmpty))
+	}
+}
 
 func TestGRPCProvider_ReadDataSource(t *testing.T) {
 	client := mockProviderClient(t)
@@ -432,3 +648,36 @@ func TestGRPCProvider_ReadDataSource(t *testing.T) {
 		t.Fatal(cmp.Diff(expected, resp.State, typeComparer, valueComparer, equateEmpty))
 	}
 }
+
+func TestGRPCProvider_ReadDataSourceJSON(t *testing.T) {
+	client := mockProviderClient(t)
+	p := &GRPCProvider{
+		client: client,
+	}
+
+	client.EXPECT().ReadDataSource(
+		gomock.Any(),
+		gomock.Any(),
+	).Return(&proto.ReadDataSource_Response{
+		State: &proto.DynamicValue{
+			Json: []byte(`{"attr":"bar"}`),
+		},
+	}, nil)
+
+	resp := p.ReadDataSource(providers.ReadDataSourceRequest{
+		TypeName: "data",
+		Config: cty.ObjectVal(map[string]cty.Value{
+			"attr": cty.StringVal("foo"),
+		}),
+	})
+
+	checkDiags(t, resp.Diagnostics)
+
+	expected := cty.ObjectVal(map[string]cty.Value{
+		"attr": cty.StringVal("bar"),
+	})
+
+	if !cmp.Equal(expected, resp.State, typeComparer, valueComparer, equateEmpty) {
+		t.Fatal(cmp.Diff(expected, resp.State, typeComparer, valueComparer, equateEmpty))
+	}
+}