Added a response-invalid-identity test to test invalid identity responses

Author: rainkwan

internal/fwserver/server_moveresourcestate.go

@@ -226,16 +226,6 @@ func (s *Server) MoveResourceState(ctx context.Context, req *MoveResourceStateRe
 			return
 		}
 
-		if resp.TargetIdentity != nil && req.IdentitySchema == nil {
-			resp.Diagnostics.AddError(
-				"Unexpected Move State Response",
-				"An unexpected error was encountered when creating the move state response. New identity data was returned by the provider move state operation, but the resource does not indicate identity support.\n\n"+
-					"This is always a problem with the provider and should be reported to the provider developer.",
-			)
-
-			return
-		}
-
 		// Set any write-only attributes in the move resource state to null
 		modifiedState, err := tftypes.Transform(moveStateResp.TargetState.Raw, NullifyWriteOnlyAttributes(ctx, moveStateResp.TargetState.Schema))
 		if err != nil {
@@ -271,8 +261,6 @@ func (s *Server) MoveResourceState(ctx context.Context, req *MoveResourceStateRe
 			"Source Provider Address: "+req.SourceProviderAddress+"\n"+
 			"Source Resource Type: "+req.SourceTypeName+"\n"+
 			"Source Resource Schema Version: "+strconv.FormatInt(req.SourceSchemaVersion, 10)+"\n"+
-			"Target Resource Type: "+req.TargetTypeName, //+"\n"+
-		// "Source Identity Schema Version: "+strconv.FormatInt(req.SourceSchemaVersion, 10),
-
+			"Target Resource Type: "+req.TargetTypeName,
 	)
 }

internal/fwserver/server_moveresourcestate_test.go

@@ -927,6 +927,45 @@ func TestServerMoveResourceState(t *testing.T) {
 				},
 			},
 		},
+		"response-invalid-identity": {
+			server: &fwserver.Server{
+				Provider: &testprovider.Provider{},
+			},
+			request: &fwserver.MoveResourceStateRequest{
+				SourceRawState: testNewRawState(t, map[string]interface{}{
+					"id":                 "test-id-value",
+					"required_attribute": true,
+				}),
+				TargetResource: &testprovider.ResourceWithMoveState{
+					MoveStateMethod: func(ctx context.Context) []resource.StateMover {
+						return []resource.StateMover{
+							{
+								StateMover: func(_ context.Context, req resource.MoveStateRequest, resp *resource.MoveStateResponse) {
+									// Try to set TargetIdentity even though req.IdentitySchema is nil
+									resp.Diagnostics.Append(resp.TargetIdentity.SetAttribute(ctx, path.Root("test_id"), "should-not-be-set")...)
+
+									// Prevent missing implementation error, the values do not matter except for response assertion
+									resp.Diagnostics.Append(resp.TargetState.SetAttribute(ctx, path.Root("id"), "test-id-value")...)
+									resp.Diagnostics.Append(resp.TargetState.SetAttribute(ctx, path.Root("required_attribute"), "true")...)
+								},
+							},
+						}
+					},
+				},
+				TargetResourceSchema: testSchema,
+				TargetTypeName:       "test_resource",
+			},
+			expectedResponse: &fwserver.MoveResourceStateResponse{
+				Diagnostics: diag.Diagnostics{
+					diag.NewErrorDiagnostic(
+						"Unexpected Move State Response",
+						"An unexpected error was encountered when creating the move state response. Identity data was returned by the provider move state operation, but the resource does not indicate identity support.\n\n"+
+							"This is always a problem with the provider and should be reported to the provider developer.",
+					),
+				},
+				TargetPrivate: privatestate.EmptyData(ctx),
+			},
+		},
 	}
 
 	for name, testCase := range testCases {

internal/toproto5/resource_identity.go

@@ -14,7 +14,7 @@ import (
 
 // ResourceIdentity returns the *tfprotov5.ResourceIdentityData for a *tfsdk.ResourceIdentity.
 func ResourceIdentity(ctx context.Context, fw *tfsdk.ResourceIdentity) (*tfprotov5.ResourceIdentityData, diag.Diagnostics) {
-	if fw == nil || fw.Schema == nil {
+	if fw == nil {
 		return nil, nil
 	}
 

tfsdk/resource_identity.go

@@ -5,7 +5,6 @@ package tfsdk
 
 import (
 	"context"
-
 	"github.com/hashicorp/terraform-plugin-framework/diag"
 	"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
 	"github.com/hashicorp/terraform-plugin-framework/internal/fwschemadata"
@@ -70,6 +69,16 @@ func (s *ResourceIdentity) Set(ctx context.Context, val interface{}) diag.Diagno
 //
 // Lists can only have the next element added according to the current length.
 func (s *ResourceIdentity) SetAttribute(ctx context.Context, path path.Path, val interface{}) diag.Diagnostics {
+	// If s is nil, then calling s.data triggers a nil pointer error so we return the error diag here
+	if s == nil {
+		return diag.Diagnostics{
+			diag.NewErrorDiagnostic(
+				"Unexpected Move State Response",
+				"An unexpected error was encountered when creating the move state response. Identity data was returned by the provider move state operation, but the resource does not indicate identity support.\n\n"+
+					"This is always a problem with the provider and should be reported to the provider developer."),
+		}
+	}
+
 	data := s.data()
 	diags := data.SetAtPath(ctx, path, val)