Updates following code review (#415)

.changelog/426.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+internal/fwserver: Ensured `UpgradeResourceState` calls from Terraform 0.12 properly ignored attributes not defined in the schema
+```

internal/fwserver/server_upgraderesourcestate.go

@@ -44,6 +44,15 @@ func (s *Server) UpgradeResourceState(ctx context.Context, req *UpgradeResourceS
 		return
 	}
 
+	// Define options to be used when unmarshalling raw state.
+	// IgnoreUndefinedAttributes will silently skip over fields in the JSON
+	// that do not have a matching entry in the schema.
+	unmarshalOpts := tfprotov6.UnmarshalOpts{
+		ValueFromJSONOpts: tftypes.ValueFromJSONOpts{
+			IgnoreUndefinedAttributes: true,
+		},
+	}
+
 	// Terraform CLI can call UpgradeResourceState even if the stored state
 	// version matches the current schema. Presumably this is to account for
 	// the previous terraform-plugin-sdk implementation, which handled some
@@ -54,7 +63,7 @@ func (s *Server) UpgradeResourceState(ctx context.Context, req *UpgradeResourceS
 	// detail for provider developers. Instead, the framework will attempt to
 	// roundtrip the prior RawState to a State matching the current Schema.
 	//
-	// TODO: To prevent provider developers from accidentially implementing
+	// TODO: To prevent provider developers from accidentally implementing
 	// ResourceWithUpgradeState with a version matching the current schema
 	// version which would never get called, the framework can introduce a
 	// unit test helper.
@@ -67,11 +76,7 @@ func (s *Server) UpgradeResourceState(ctx context.Context, req *UpgradeResourceS
 
 		resourceSchemaType := req.ResourceSchema.TerraformType(ctx)
 
-		rawStateValue, err := req.RawState.UnmarshalWithOpts(resourceSchemaType, tftypes.UnmarshalOpts{
-			JSONOpts: tftypes.JSONOpts{
-				IgnoreUndefinedAttributes: true,
-			},
-		})
+		rawStateValue, err := req.RawState.UnmarshalWithOpts(resourceSchemaType, unmarshalOpts)
 
 		if err != nil {
 			resp.Diagnostics.AddError(
@@ -147,7 +152,7 @@ func (s *Server) UpgradeResourceState(ctx context.Context, req *UpgradeResourceS
 
 		priorSchemaType := resourceStateUpgrader.PriorSchema.TerraformType(ctx)
 
-		rawStateValue, err := req.RawState.Unmarshal(priorSchemaType)
+		rawStateValue, err := req.RawState.UnmarshalWithOpts(priorSchemaType, unmarshalOpts)
 
 		if err != nil {
 			resp.Diagnostics.AddError(

internal/fwserver/server_upgraderesourcestate_test.go

@@ -466,6 +466,91 @@ func TestServerUpgradeResourceState(t *testing.T) {
 				},
 			},
 		},
+		"PriorSchema-and-State-json-mismatch": {
+			server: &fwserver.Server{
+				Provider: &testprovider.Provider{},
+			},
+			request: &fwserver.UpgradeResourceStateRequest{
+				RawState: testNewRawState(t, map[string]interface{}{
+					"id":                    "test-id-value",
+					"required_attribute":    true,
+					"nonexistent_attribute": "value",
+				}),
+				ResourceSchema: schema,
+				ResourceType: &testprovider.ResourceType{
+					GetSchemaMethod: func(_ context.Context) (tfsdk.Schema, diag.Diagnostics) {
+						return schema, nil
+					},
+					NewResourceMethod: func(_ context.Context, _ tfsdk.Provider) (tfsdk.Resource, diag.Diagnostics) {
+						return &testprovider.ResourceWithUpgradeState{
+							Resource: &testprovider.Resource{},
+							UpgradeStateMethod: func(ctx context.Context) map[int64]tfsdk.ResourceStateUpgrader {
+								return map[int64]tfsdk.ResourceStateUpgrader{
+									0: {
+										PriorSchema: &tfsdk.Schema{
+											Attributes: map[string]tfsdk.Attribute{
+												"id": {
+													Type:     types.StringType,
+													Computed: true,
+												},
+												"optional_attribute": {
+													Type:     types.BoolType,
+													Optional: true,
+												},
+												"required_attribute": {
+													Type:     types.BoolType,
+													Required: true,
+												},
+											},
+										},
+										StateUpgrader: func(ctx context.Context, req tfsdk.UpgradeResourceStateRequest, resp *tfsdk.UpgradeResourceStateResponse) {
+											var priorStateData struct {
+												Id                string `tfsdk:"id"`
+												OptionalAttribute *bool  `tfsdk:"optional_attribute"`
+												RequiredAttribute bool   `tfsdk:"required_attribute"`
+											}
+
+											resp.Diagnostics.Append(req.State.Get(ctx, &priorStateData)...)
+
+											if resp.Diagnostics.HasError() {
+												return
+											}
+
+											upgradedStateData := struct {
+												Id                string  `tfsdk:"id"`
+												OptionalAttribute *string `tfsdk:"optional_attribute"`
+												RequiredAttribute string  `tfsdk:"required_attribute"`
+											}{
+												Id:                priorStateData.Id,
+												RequiredAttribute: fmt.Sprintf("%t", priorStateData.RequiredAttribute),
+											}
+
+											if priorStateData.OptionalAttribute != nil {
+												v := fmt.Sprintf("%t", *priorStateData.OptionalAttribute)
+												upgradedStateData.OptionalAttribute = &v
+											}
+
+											resp.Diagnostics.Append(resp.State.Set(ctx, upgradedStateData)...)
+										},
+									},
+								}
+							},
+						}, nil
+					},
+				},
+				Version: 0,
+			},
+			expectedResponse: &fwserver.UpgradeResourceStateResponse{
+				UpgradedState: &tfsdk.State{
+					Raw: tftypes.NewValue(schemaType, map[string]tftypes.Value{
+						"id":                 tftypes.NewValue(tftypes.String, "test-id-value"),
+						"optional_attribute": tftypes.NewValue(tftypes.String, nil),
+						"required_attribute": tftypes.NewValue(tftypes.String, "true"),
+					}),
+					Schema: schema,
+				},
+			},
+		},
 		"UpgradedState-missing": {
 			server: &fwserver.Server{
 				Provider: &testprovider.Provider{},