Addressed PR comments round 2

Author: rainkwan

internal/fromproto5/upgraderesourceidentity.go

@@ -15,7 +15,7 @@ import (
 
 // UpgradeResourceIdentityRequest returns the *fwserver.UpgradeResourceIdentityRequest
 // equivalent of a *tfprotov5.UpgradeResourceIdentityRequest.
-func UpgradeResourceIdentityRequest(ctx context.Context, proto5 *tfprotov5.UpgradeResourceIdentityRequest, resource resource.Resource, identitySchema fwschema.Schema) (*fwserver.UpgradeResourceIdentityRequest, diag.Diagnostics) {
+func UpgradeIdentityRequest(ctx context.Context, proto5 *tfprotov5.UpgradeResourceIdentityRequest, resource resource.Resource, identitySchema fwschema.Schema) (*fwserver.UpgradeIdentityRequest, diag.Diagnostics) {
 	if proto5 == nil {
 		return nil, nil
 	}
@@ -36,7 +36,7 @@ func UpgradeResourceIdentityRequest(ctx context.Context, proto5 *tfprotov5.Upgra
 		return nil, diags
 	}
 
-	fw := &fwserver.UpgradeResourceIdentityRequest{
+	fw := &fwserver.UpgradeIdentityRequest{
 		RawState:       (*tfprotov6.RawState)(proto5.RawIdentity),
 		IdentitySchema: identitySchema,
 		Resource:       resource,

internal/fromproto5/upgraderesourceidentity_test.go

@@ -33,7 +33,7 @@ func TestUpgradeResourceIdentityRequest(t *testing.T) {
 		input               *tfprotov5.UpgradeResourceIdentityRequest
 		identitySchema      fwschema.Schema
 		resource            resource.Resource
-		expected            *fwserver.UpgradeResourceIdentityRequest
+		expected            *fwserver.UpgradeIdentityRequest
 		expectedDiagnostics diag.Diagnostics
 	}{
 		"nil": {
@@ -47,7 +47,7 @@ func TestUpgradeResourceIdentityRequest(t *testing.T) {
 				}),
 			},
 			identitySchema: testIdentitySchema,
-			expected: &fwserver.UpgradeResourceIdentityRequest{
+			expected: &fwserver.UpgradeIdentityRequest{
 				RawState: testNewTfprotov6RawState(t, map[string]interface{}{
 					"test_attribute": "test-value",
 				}),
@@ -57,7 +57,7 @@ func TestUpgradeResourceIdentityRequest(t *testing.T) {
 		"resourceschema": {
 			input:          &tfprotov5.UpgradeResourceIdentityRequest{},
 			identitySchema: testIdentitySchema,
-			expected: &fwserver.UpgradeResourceIdentityRequest{
+			expected: &fwserver.UpgradeIdentityRequest{
 				IdentitySchema: testIdentitySchema,
 			},
 		},
@@ -79,7 +79,7 @@ func TestUpgradeResourceIdentityRequest(t *testing.T) {
 				Version: 123,
 			},
 			identitySchema: testIdentitySchema,
-			expected: &fwserver.UpgradeResourceIdentityRequest{
+			expected: &fwserver.UpgradeIdentityRequest{
 				IdentitySchema: testIdentitySchema,
 				Version:        123,
 			},
@@ -90,7 +90,7 @@ func TestUpgradeResourceIdentityRequest(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
-			got, diags := fromproto5.UpgradeResourceIdentityRequest(context.Background(), testCase.input, testCase.resource, testCase.identitySchema)
+			got, diags := fromproto5.UpgradeIdentityRequest(context.Background(), testCase.input, testCase.resource, testCase.identitySchema)
 
 			if diff := cmp.Diff(got, testCase.expected); diff != "" {
 				t.Errorf("unexpected difference: %s", diff)

internal/fromproto6/upgraderesourceidentity.go

@@ -12,9 +12,9 @@ import (
 	"github.com/hashicorp/terraform-plugin-go/tfprotov6"
 )
 
-// UpgradeResourceIdentityRequest returns the *fwserver.UpgradeResourceIdentityRequest
-// equivalent of a *tfprotov6.UpgradeResourceIdentityRequest.
-func UpgradeResourceIdentityRequest(ctx context.Context, proto6 *tfprotov6.UpgradeResourceIdentityRequest, resource resource.Resource, identitySchema fwschema.Schema) (*fwserver.UpgradeResourceIdentityRequest, diag.Diagnostics) {
+// UpgradeIdentityRequest returns the *fwserver.UpgradeIdentityRequest
+// equivalent of a *tfprotov6.UpgradeIdentityRequest.
+func UpgradeIdentityRequest(ctx context.Context, proto6 *tfprotov6.UpgradeResourceIdentityRequest, resource resource.Resource, identitySchema fwschema.Schema) (*fwserver.UpgradeIdentityRequest, diag.Diagnostics) {
 	if proto6 == nil {
 		return nil, nil
 	}
@@ -35,7 +35,7 @@ func UpgradeResourceIdentityRequest(ctx context.Context, proto6 *tfprotov6.Upgra
 		return nil, diags
 	}
 
-	fw := &fwserver.UpgradeResourceIdentityRequest{
+	fw := &fwserver.UpgradeIdentityRequest{
 		RawState:       proto6.RawIdentity,
 		IdentitySchema: identitySchema,
 		Resource:       resource,

internal/fromproto6/upgraderesourceidentity_test.go

@@ -18,7 +18,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-go/tfprotov6"
 )
 
-func TestUpgradeResourceIdentityRequest(t *testing.T) {
+func TestUpgradeIdentityRequest(t *testing.T) {
 	t.Parallel()
 
 	testIdentitySchema := identityschema.Schema{
@@ -33,7 +33,7 @@ func TestUpgradeResourceIdentityRequest(t *testing.T) {
 		input               *tfprotov6.UpgradeResourceIdentityRequest
 		identitySchema      fwschema.Schema
 		resource            resource.Resource
-		expected            *fwserver.UpgradeResourceIdentityRequest
+		expected            *fwserver.UpgradeIdentityRequest
 		expectedDiagnostics diag.Diagnostics
 	}{
 		"nil": {
@@ -47,7 +47,7 @@ func TestUpgradeResourceIdentityRequest(t *testing.T) {
 				}),
 			},
 			identitySchema: testIdentitySchema,
-			expected: &fwserver.UpgradeResourceIdentityRequest{
+			expected: &fwserver.UpgradeIdentityRequest{
 				RawState: testNewRawState(t, map[string]interface{}{
 					"test_attribute": "test-value",
 				}),
@@ -57,7 +57,7 @@ func TestUpgradeResourceIdentityRequest(t *testing.T) {
 		"resourceschema": {
 			input:          &tfprotov6.UpgradeResourceIdentityRequest{},
 			identitySchema: testIdentitySchema,
-			expected: &fwserver.UpgradeResourceIdentityRequest{
+			expected: &fwserver.UpgradeIdentityRequest{
 				IdentitySchema: testIdentitySchema,
 			},
 		},
@@ -79,7 +79,7 @@ func TestUpgradeResourceIdentityRequest(t *testing.T) {
 				Version: 123,
 			},
 			identitySchema: testIdentitySchema,
-			expected: &fwserver.UpgradeResourceIdentityRequest{
+			expected: &fwserver.UpgradeIdentityRequest{
 				IdentitySchema: testIdentitySchema,
 				Version:        123,
 			},
@@ -90,7 +90,7 @@ func TestUpgradeResourceIdentityRequest(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
-			got, diags := fromproto6.UpgradeResourceIdentityRequest(context.Background(), testCase.input, testCase.resource, testCase.identitySchema)
+			got, diags := fromproto6.UpgradeIdentityRequest(context.Background(), testCase.input, testCase.resource, testCase.identitySchema)
 
 			if diff := cmp.Diff(got, testCase.expected); diff != "" {
 				t.Errorf("unexpected difference: %s", diff)

internal/fwserver/server_upgraderesourceidentity.go

@@ -16,9 +16,9 @@ import (
 	"github.com/hashicorp/terraform-plugin-framework/tfsdk"
 )
 
-// UpgradeResourceIdentityRequest is the framework server request for the
-// UpgradeResourceIdentity RPC.
-type UpgradeResourceIdentityRequest struct {
+// UpgradeIdentityRequest is the framework server request for the
+// UpgradeIdentity RPC.
+type UpgradeIdentityRequest struct {
 	Resource       resource.Resource
 	IdentitySchema fwschema.Schema
 	// TypeName is the type of resource that Terraform needs to upgrade the
@@ -28,10 +28,6 @@ type UpgradeResourceIdentityRequest struct {
 	// Version is the version of the identity state the resource currently has.
 	Version int64
 
-	// Using the tfprotov6 type here was a pragmatic effort decision around when
-	// the framework introduced compatibility promises. This type was chosen as
-	// it was readily available and trivial to convert between tfprotov5.
-	//
 	// Using a terraform-plugin-go type is not ideal for the framework as almost
 	// all terraform-plugin-go types have framework abstractions, but if there
 	// is ever a time where it makes sense to re-evaluate this decision, such as
@@ -40,15 +36,15 @@ type UpgradeResourceIdentityRequest struct {
 	RawState *tfprotov6.RawState
 }
 
-// UpgradeResourceIdentityResponse is the framework server response for the
-// UpgradeResourceIdentity RPC.
-type UpgradeResourceIdentityResponse struct {
+// UpgradeIdentityResponse is the framework server response for the
+// UpgradeIdentity RPC.
+type UpgradeIdentityResponse struct {
 	UpgradedIdentity *tfsdk.ResourceIdentity
 	Diagnostics      diag.Diagnostics
 }
 
-// UpgradeResourceIdentity implements the framework server UpgradeResourceIdentity RPC.
-func (s *Server) UpgradeResourceIdentity(ctx context.Context, req *UpgradeResourceIdentityRequest, resp *UpgradeResourceIdentityResponse) {
+// UpgradeIdentity implements the framework server UpgradeIdentity RPC.
+func (s *Server) UpgradeIdentity(ctx context.Context, req *UpgradeIdentityRequest, resp *UpgradeIdentityResponse) {
 	if req == nil {
 		return
 	}
@@ -71,11 +67,10 @@ func (s *Server) UpgradeResourceIdentity(ctx context.Context, req *UpgradeResour
 
 	if req.Version == req.IdentitySchema.GetVersion() {
 		resp.Diagnostics.AddError(
-			"Unable to Read Previously Saved Identity for UpgradeResourceIdentity",
-			"There was an error reading the saved resource identity using the current resource identity schema.\n\n"+
-				"If this resource Identity was last refreshed with Terraform CLI 0.11 and earlier, it must be refreshed or applied with an older provider version first. "+
-				"If you manually modified the resource Identity, you will need to manually modify it to match the current resource identity schema. "+
-				"Otherwise, please report this to the provider developer:\n\n",
+			"Unexpected Identity Upgrade Request",
+			"Terraform Core invoked UpgradeResourceIdentity even though the stored identity schema version matches the current version. "+
+				"This likely indicates a bug in the Terraform provider framework or Terraform Core. "+
+				"Please report this issue to the provider developer.",
 		)
 		return
 	}
@@ -114,7 +109,7 @@ func (s *Server) UpgradeResourceIdentity(ctx context.Context, req *UpgradeResour
 	logging.FrameworkTrace(ctx, "Resource implements ResourceWithUpgradeIdentity")
 
 	logging.FrameworkTrace(ctx, "Calling provider defined Resource UpgradeIdentity")
-	resourceIdentityUpgraders := resourceWithUpgradeIdentity.UpgradeResourceIdentity(ctx)
+	resourceIdentityUpgraders := resourceWithUpgradeIdentity.UpgradeIdentity(ctx)
 	logging.FrameworkTrace(ctx, "Called provider defined Resource UpgradeIdentity")
 
 	// Panic prevention
@@ -134,20 +129,20 @@ func (s *Server) UpgradeResourceIdentity(ctx context.Context, req *UpgradeResour
 		return
 	}
 
-	upgradeResourceIdentityRequest := resource.UpgradeResourceIdentityRequest{
-		RawState: req.RawState,
+	upgradeResourceIdentityRequest := resource.UpgradeIdentityRequest{
+		RawIdentity: req.RawState,
 	}
 
 	if resourceIdentityUpgrader.PriorSchema != nil {
-		logging.FrameworkTrace(ctx, "Initializing populated UpgradeResourceIdentityRequest Identity from provider defined prior schema and request RawState")
+		logging.FrameworkTrace(ctx, "Initializing populated UpgradeIdentityRequest Identity from provider defined prior schema and request RawState")
 
 		priorSchemaType := resourceIdentityUpgrader.PriorSchema.Type().TerraformType(ctx)
 
 		rawIdentityValue, err := req.RawState.UnmarshalWithOpts(priorSchemaType, unmarshalOpts)
 
 		if err != nil {
 			resp.Diagnostics.AddError(
-				"Unable to Read Previously Saved Identity for UpgradeResourceIdentity",
+				"Unable to Read Previously Saved Identity for UpgradeIdentity",
 				fmt.Sprintf("There was an error reading the saved resource Identity using the prior resource schema defined for version %d upgrade.\n\n", req.Version)+
 					"Please report this to the provider developer:\n\n"+err.Error(),
 			)
@@ -161,7 +156,7 @@ func (s *Server) UpgradeResourceIdentity(ctx context.Context, req *UpgradeResour
 
 	}
 
-	upgradeResourceIdentityResponse := resource.UpgradeResourceIdentityResponse{
+	upgradeResourceIdentityResponse := resource.UpgradeIdentityResponse{
 		Identity: &tfsdk.ResourceIdentity{
 			Schema: req.IdentitySchema,
 			// Raw is intentionally not set.
@@ -193,16 +188,5 @@ func (s *Server) UpgradeResourceIdentity(ctx context.Context, req *UpgradeResour
 		return
 	}
 
-	if upgradeResourceIdentityResponse.Identity.Raw.Type() != nil {
-		logging.FrameworkTrace(ctx, "UpgradeResourceIdentityResponse RawState set, overriding State")
-
-		resp.UpgradedIdentity = &tfsdk.ResourceIdentity{
-			Schema: req.IdentitySchema,
-			Raw:    upgradeResourceIdentityResponse.Identity.Raw,
-		}
-
-		return
-	}
-
 	resp.UpgradedIdentity = upgradeResourceIdentityResponse.Identity
 }