From 35388c6eb546e0c3457cedf89c22c7a1346ed732 Mon Sep 17 00:00:00 2001 From: Rain Date: Tue, 25 Mar 2025 00:43:39 -0400 Subject: [PATCH 01/14] Add initial updates for identity to MoveResourceState RPC --- internal/fromproto5/moveresourcestate.go | 1 + internal/fwserver/server_moveresourcestate.go | 36 ++++++++++++++----- resource/move_state.go | 11 ++++++ 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/internal/fromproto5/moveresourcestate.go b/internal/fromproto5/moveresourcestate.go index f5e836b7e..23c582dca 100644 --- a/internal/fromproto5/moveresourcestate.go +++ b/internal/fromproto5/moveresourcestate.go @@ -45,6 +45,7 @@ func MoveResourceStateRequest(ctx context.Context, proto5 *tfprotov5.MoveResourc TargetResource: resource, TargetResourceSchema: resourceSchema, TargetTypeName: proto5.TargetTypeName, + SourceIdentity: (*tfprotov6.RawState)(proto5.SourceIdentity), } sourcePrivate, sourcePrivateDiags := privatestate.NewData(ctx, proto5.SourcePrivate) diff --git a/internal/fwserver/server_moveresourcestate.go b/internal/fwserver/server_moveresourcestate.go index 1a832f3fe..a76c0c7e2 100644 --- a/internal/fwserver/server_moveresourcestate.go +++ b/internal/fwserver/server_moveresourcestate.go @@ -65,14 +65,23 @@ type MoveResourceStateRequest struct { // TargetTypeName is the type name of the target resource as given by // Terraform across the protocol. TargetTypeName string + + // SourceIdentity is the identity of the source resource. + // + // Only the underlying JSON field is populated. + SourceIdentity *tfprotov6.RawState + + // SourceIdentitySchemaVersion is the version of the source resource state. + SourceIdentitySchemaVersion int64 } // MoveResourceStateResponse is the framework server response for the // MoveResourceState RPC. type MoveResourceStateResponse struct { - Diagnostics diag.Diagnostics - TargetPrivate *privatestate.Data - TargetState *tfsdk.State + Diagnostics diag.Diagnostics + TargetPrivate *privatestate.Data + TargetState *tfsdk.State + TargetIdentity *tfsdk.ResourceIdentity } // MoveResourceState implements the framework server MoveResourceState RPC. @@ -125,11 +134,13 @@ func (s *Server) MoveResourceState(ctx context.Context, req *MoveResourceStateRe for _, resourceStateMover := range resourceStateMovers { moveStateReq := resource.MoveStateRequest{ - SourcePrivate: sourcePrivate, - SourceProviderAddress: req.SourceProviderAddress, - SourceRawState: req.SourceRawState, - SourceSchemaVersion: req.SourceSchemaVersion, - SourceTypeName: req.SourceTypeName, + SourcePrivate: sourcePrivate, + SourceProviderAddress: req.SourceProviderAddress, + SourceRawState: req.SourceRawState, + SourceSchemaVersion: req.SourceSchemaVersion, + SourceTypeName: req.SourceTypeName, + SourceIdentity: req.SourceIdentity, + SourceIdentitySchemaVersion: req.SourceIdentitySchemaVersion, } moveStateResp := resource.MoveStateResponse{ TargetPrivate: privatestate.EmptyProviderData(ctx), @@ -137,6 +148,11 @@ func (s *Server) MoveResourceState(ctx context.Context, req *MoveResourceStateRe Schema: req.TargetResourceSchema, Raw: tftypes.NewValue(req.TargetResourceSchema.Type().TerraformType(ctx), nil), }, + // TODO: See if we need to change this to identity + TargetIdentity: &tfsdk.ResourceIdentity{ + Raw: tftypes.NewValue(req.TargetResourceSchema.Type().TerraformType(ctx), nil), + Schema: req.TargetResourceSchema, + }, } if resourceStateMover.SourceSchema != nil { @@ -237,6 +253,8 @@ 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, + "Target Resource Type: "+req.TargetTypeName, //+"\n"+ + // "Source Identity Schema Version: "+strconv.FormatInt(req.SourceSchemaVersion, 10), + ) } diff --git a/resource/move_state.go b/resource/move_state.go index 6918f5b76..49e0a56c3 100644 --- a/resource/move_state.go +++ b/resource/move_state.go @@ -75,6 +75,14 @@ type MoveStateRequest struct { // other request fields, to determine whether the request should be handled // by this particular implementation. SourceTypeName string + + // SourceIdentity is the identity of the source resource. + // + // Only the underlying JSON field is populated. + SourceIdentity *tfprotov6.RawState + + // SourceIdentitySchemaVersion is the version of the source resource state. + SourceIdentitySchemaVersion int64 } // MoveStateResponse represents a response to a MoveStateRequest. @@ -107,4 +115,7 @@ type MoveStateResponse struct { // operation. This field is not pre-populated as it is up to implementations // whether the source private data is relevant for the target resource. TargetPrivate *privatestate.ProviderData + + // TargetIdentity is the identity of the target resource. + TargetIdentity *tfsdk.ResourceIdentity } From 6efa63b39e8594bcd7764559f9e4900991290ed7 Mon Sep 17 00:00:00 2001 From: Rain Date: Tue, 25 Mar 2025 08:40:22 -0400 Subject: [PATCH 02/14] Add toproto updates for identity to MoveResourceState RPC --- internal/fromproto5/moveresourcestate.go | 17 +++++++++-------- internal/fromproto6/moveresourcestate.go | 17 +++++++++-------- internal/toproto5/moveresourcestate.go | 5 +++++ internal/toproto6/moveresourcestate.go | 5 +++++ 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/internal/fromproto5/moveresourcestate.go b/internal/fromproto5/moveresourcestate.go index 23c582dca..75845a03f 100644 --- a/internal/fromproto5/moveresourcestate.go +++ b/internal/fromproto5/moveresourcestate.go @@ -38,14 +38,15 @@ func MoveResourceStateRequest(ctx context.Context, proto5 *tfprotov5.MoveResourc } fw := &fwserver.MoveResourceStateRequest{ - SourceProviderAddress: proto5.SourceProviderAddress, - SourceRawState: (*tfprotov6.RawState)(proto5.SourceState), - SourceSchemaVersion: proto5.SourceSchemaVersion, - SourceTypeName: proto5.SourceTypeName, - TargetResource: resource, - TargetResourceSchema: resourceSchema, - TargetTypeName: proto5.TargetTypeName, - SourceIdentity: (*tfprotov6.RawState)(proto5.SourceIdentity), + SourceProviderAddress: proto5.SourceProviderAddress, + SourceRawState: (*tfprotov6.RawState)(proto5.SourceState), + SourceSchemaVersion: proto5.SourceSchemaVersion, + SourceTypeName: proto5.SourceTypeName, + TargetResource: resource, + TargetResourceSchema: resourceSchema, + TargetTypeName: proto5.TargetTypeName, + SourceIdentity: (*tfprotov6.RawState)(proto5.SourceIdentity), + SourceIdentitySchemaVersion: proto5.SourceIdentitySchemaVersion, } sourcePrivate, sourcePrivateDiags := privatestate.NewData(ctx, proto5.SourcePrivate) diff --git a/internal/fromproto6/moveresourcestate.go b/internal/fromproto6/moveresourcestate.go index 6b31a2cc0..cc4c3ad4d 100644 --- a/internal/fromproto6/moveresourcestate.go +++ b/internal/fromproto6/moveresourcestate.go @@ -5,7 +5,6 @@ package fromproto6 import ( "context" - "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/internal/fwschema" "github.com/hashicorp/terraform-plugin-framework/internal/fwserver" @@ -37,13 +36,15 @@ func MoveResourceStateRequest(ctx context.Context, proto6 *tfprotov6.MoveResourc } fw := &fwserver.MoveResourceStateRequest{ - SourceProviderAddress: proto6.SourceProviderAddress, - SourceRawState: proto6.SourceState, - SourceSchemaVersion: proto6.SourceSchemaVersion, - SourceTypeName: proto6.SourceTypeName, - TargetResource: resource, - TargetResourceSchema: resourceSchema, - TargetTypeName: proto6.TargetTypeName, + SourceProviderAddress: proto6.SourceProviderAddress, + SourceRawState: proto6.SourceState, + SourceSchemaVersion: proto6.SourceSchemaVersion, + SourceTypeName: proto6.SourceTypeName, + TargetResource: resource, + TargetResourceSchema: resourceSchema, + TargetTypeName: proto6.TargetTypeName, + SourceIdentity: proto6.SourceIdentity, + SourceIdentitySchemaVersion: proto6.SourceIdentitySchemaVersion, } sourcePrivate, sourcePrivateDiags := privatestate.NewData(ctx, proto6.SourcePrivate) diff --git a/internal/toproto5/moveresourcestate.go b/internal/toproto5/moveresourcestate.go index bfffadf1b..2c598c46c 100644 --- a/internal/toproto5/moveresourcestate.go +++ b/internal/toproto5/moveresourcestate.go @@ -27,6 +27,11 @@ func MoveResourceStateResponse(ctx context.Context, fw *fwserver.MoveResourceSta proto5.Diagnostics = append(proto5.Diagnostics, Diagnostics(ctx, diags)...) proto5.TargetPrivate = targetPrivate + targetIdentity, diags := ResourceIdentity(ctx, fw.TargetIdentity) + + proto5.Diagnostics = append(proto5.Diagnostics, Diagnostics(ctx, diags)...) + proto5.TargetIdentity = targetIdentity + targetState, diags := State(ctx, fw.TargetState) proto5.Diagnostics = append(proto5.Diagnostics, Diagnostics(ctx, diags)...) diff --git a/internal/toproto6/moveresourcestate.go b/internal/toproto6/moveresourcestate.go index 86c2a55a8..83a81d446 100644 --- a/internal/toproto6/moveresourcestate.go +++ b/internal/toproto6/moveresourcestate.go @@ -27,6 +27,11 @@ func MoveResourceStateResponse(ctx context.Context, fw *fwserver.MoveResourceSta proto6.Diagnostics = append(proto6.Diagnostics, Diagnostics(ctx, diags)...) proto6.TargetPrivate = targetPrivate + targetIdentity, diags := ResourceIdentity(ctx, fw.TargetIdentity) + + proto6.Diagnostics = append(proto6.Diagnostics, Diagnostics(ctx, diags)...) + proto6.TargetIdentity = targetIdentity + targetState, diags := State(ctx, fw.TargetState) proto6.Diagnostics = append(proto6.Diagnostics, Diagnostics(ctx, diags)...) From b56b2fbaee882eff279974b38c48cb31542d4014 Mon Sep 17 00:00:00 2001 From: Rain Date: Thu, 27 Mar 2025 15:02:45 -0400 Subject: [PATCH 03/14] Trying to start testing --- internal/fromproto5/moveresourcestate_test.go | 24 ++++ internal/fromproto6/moveresourcestate_test.go | 24 ++++ .../fwserver/server_moveresourcestate_test.go | 120 ++++++++++++++++++ 3 files changed, 168 insertions(+) diff --git a/internal/fromproto5/moveresourcestate_test.go b/internal/fromproto5/moveresourcestate_test.go index 2122edb75..2d47f54c4 100644 --- a/internal/fromproto5/moveresourcestate_test.go +++ b/internal/fromproto5/moveresourcestate_test.go @@ -162,6 +162,30 @@ func TestMoveResourceStateRequest(t *testing.T) { TargetTypeName: "examplecloud_thing", }, }, + "SourceIdentity": { + input: &tfprotov5.MoveResourceStateRequest{ + SourceIdentity: testNewTfprotov5RawState(t, map[string]interface{}{ + "test_identity_attribute": "test-value", + }), + }, + resourceSchema: testFwSchema, + expected: &fwserver.MoveResourceStateRequest{ + SourceIdentity: testNewTfprotov6RawState(t, map[string]interface{}{ + "test_identity_attribute": "test-value", + }), + TargetResourceSchema: testFwSchema, + }, + }, + "SourceIdentitySchemaVersion": { + input: &tfprotov5.MoveResourceStateRequest{ + SourceIdentitySchemaVersion: 123, + }, + resourceSchema: testFwSchema, + expected: &fwserver.MoveResourceStateRequest{ + SourceIdentitySchemaVersion: 123, + TargetResourceSchema: testFwSchema, + }, + }, } for name, testCase := range testCases { diff --git a/internal/fromproto6/moveresourcestate_test.go b/internal/fromproto6/moveresourcestate_test.go index 881ab3a49..00f33f750 100644 --- a/internal/fromproto6/moveresourcestate_test.go +++ b/internal/fromproto6/moveresourcestate_test.go @@ -162,6 +162,30 @@ func TestMoveResourceStateRequest(t *testing.T) { TargetTypeName: "examplecloud_thing", }, }, + "SourceIdentity": { + input: &tfprotov6.MoveResourceStateRequest{ + SourceIdentity: testNewRawState(t, map[string]interface{}{ + "test_identity_attribute": "test-value", + }), + }, + resourceSchema: testFwSchema, + expected: &fwserver.MoveResourceStateRequest{ + SourceIdentity: testNewRawState(t, map[string]interface{}{ + "test_identity_attribute": "test-value", + }), + TargetResourceSchema: testFwSchema, + }, + }, + "SourceIdentitySchemaVersion": { + input: &tfprotov6.MoveResourceStateRequest{ + SourceIdentitySchemaVersion: 123, + }, + resourceSchema: testFwSchema, + expected: &fwserver.MoveResourceStateRequest{ + SourceIdentitySchemaVersion: 123, + TargetResourceSchema: testFwSchema, + }, + }, } for name, testCase := range testCases { diff --git a/internal/fwserver/server_moveresourcestate_test.go b/internal/fwserver/server_moveresourcestate_test.go index 8e2445ba9..0b9a8e575 100644 --- a/internal/fwserver/server_moveresourcestate_test.go +++ b/internal/fwserver/server_moveresourcestate_test.go @@ -6,6 +6,7 @@ package fwserver_test import ( "context" "fmt" + "github.com/hashicorp/terraform-plugin-framework/resource/identityschema" "testing" "github.com/google/go-cmp/cmp" @@ -39,8 +40,21 @@ func TestServerMoveResourceState(t *testing.T) { }, }, } + + testIdentitySchema := identityschema.Schema{ + Attributes: map[string]identityschema.Attribute{ + "optionalforimport_attribute": identityschema.StringAttribute{ + OptionalForImport: true, + }, + "requiredforimport_attribute": identityschema.StringAttribute{ + RequiredForImport: true, + }, + }, + } schemaType := testSchema.Type().TerraformType(ctx) + schemaIdentityType := testIdentitySchema.Type().TerraformType(ctx) + testSchemaWriteOnly := schema.Schema{ Attributes: map[string]schema.Attribute{ "id": schema.StringAttribute{ @@ -812,6 +826,112 @@ func TestServerMoveResourceState(t *testing.T) { }, }, }, + "request-SourceIdentitySchemaVersion": { + server: &fwserver.Server{ + Provider: &testprovider.Provider{}, + }, + request: &fwserver.MoveResourceStateRequest{ + SourceIdentitySchemaVersion: 123, + // SourceRawState required to prevent error + 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) { + expected := int64(123) + + if diff := cmp.Diff(req.SourceIdentitySchemaVersion, expected); diff != "" { + resp.Diagnostics.AddError("Unexpected req.SourceIdentitySchemaVersion difference", diff) + } + + // 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{ + TargetPrivate: privatestate.EmptyData(ctx), + TargetState: &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: testSchema, + }, + }, + }, + "request-SourceIdentity": { + server: &fwserver.Server{ + Provider: &testprovider.Provider{}, + }, + request: &fwserver.MoveResourceStateRequest{ + // SourceRawState required to prevent error + SourceIdentity: testNewRawState(t, map[string]interface{}{ + "optionalforimport_attribute": false, + "requiredforimport_attribute": true, + }), + 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) { + expectedSourceIdentity := testNewRawState(t, map[string]interface{}{ + "optionalforimport_attribute": false, + "requiredforimport_attribute": true, + }) + + if diff := cmp.Diff(req.SourceIdentity, expectedSourceIdentity); diff != "" { + resp.Diagnostics.AddError("Unexpected req.SourceIdentity difference", diff) + } + + resp.Diagnostics.Append(resp.TargetIdentity.SetAttribute(ctx, path.Root("optionalforimport_attribute"), "false")...) + resp.Diagnostics.Append(resp.TargetIdentity.SetAttribute(ctx, path.Root("requiredforimport_attribute"), "true")...) + + // 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{ + TargetPrivate: privatestate.EmptyData(ctx), + TargetState: &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: testSchema, + }, + TargetIdentity: &tfsdk.ResourceIdentity{ + Raw: tftypes.NewValue(schemaIdentityType, map[string]tftypes.Value{ + "optionalforimport_attribute": tftypes.NewValue(tftypes.String, "false"), + "requiredforimport_attribute": tftypes.NewValue(tftypes.String, "true"), + }), + Schema: testIdentitySchema, + }, + }, + }, } for name, testCase := range testCases { From d192ef7b00ea70a96448067096b03744fd8e025f Mon Sep 17 00:00:00 2001 From: Rain Date: Tue, 1 Apr 2025 14:55:39 -0400 Subject: [PATCH 04/14] Added more tests for identity to move rpc and added identitySchema to MoveState Request --- internal/fromproto5/moveresourcestate.go | 4 +- internal/fromproto5/moveresourcestate_test.go | 3 +- internal/fromproto6/moveresourcestate.go | 3 +- internal/fwserver/server_moveresourcestate.go | 20 ++++- .../fwserver/server_moveresourcestate_test.go | 19 ++--- .../proto5server/server_moveresourcestate.go | 10 ++- .../proto6server/server_moveresourcestate.go | 11 ++- internal/toproto5/moveresourcestate_test.go | 80 ++++++++++++++++++ internal/toproto6/moveresourcestate_test.go | 82 ++++++++++++++++++- 9 files changed, 209 insertions(+), 23 deletions(-) diff --git a/internal/fromproto5/moveresourcestate.go b/internal/fromproto5/moveresourcestate.go index 75845a03f..988bb0ab1 100644 --- a/internal/fromproto5/moveresourcestate.go +++ b/internal/fromproto5/moveresourcestate.go @@ -5,7 +5,6 @@ package fromproto5 import ( "context" - "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/internal/fwschema" "github.com/hashicorp/terraform-plugin-framework/internal/fwserver" @@ -17,7 +16,7 @@ import ( // MoveResourceStateRequest returns the *fwserver.MoveResourceStateRequest // equivalent of a *tfprotov5.MoveResourceStateRequest. -func MoveResourceStateRequest(ctx context.Context, proto5 *tfprotov5.MoveResourceStateRequest, resource resource.Resource, resourceSchema fwschema.Schema) (*fwserver.MoveResourceStateRequest, diag.Diagnostics) { +func MoveResourceStateRequest(ctx context.Context, proto5 *tfprotov5.MoveResourceStateRequest, resource resource.Resource, resourceSchema fwschema.Schema, identitySchema fwschema.Schema) (*fwserver.MoveResourceStateRequest, diag.Diagnostics) { if proto5 == nil { return nil, nil } @@ -47,6 +46,7 @@ func MoveResourceStateRequest(ctx context.Context, proto5 *tfprotov5.MoveResourc TargetTypeName: proto5.TargetTypeName, SourceIdentity: (*tfprotov6.RawState)(proto5.SourceIdentity), SourceIdentitySchemaVersion: proto5.SourceIdentitySchemaVersion, + IdentitySchema: identitySchema, } sourcePrivate, sourcePrivateDiags := privatestate.NewData(ctx, proto5.SourcePrivate) diff --git a/internal/fromproto5/moveresourcestate_test.go b/internal/fromproto5/moveresourcestate_test.go index 2d47f54c4..78182cd0e 100644 --- a/internal/fromproto5/moveresourcestate_test.go +++ b/internal/fromproto5/moveresourcestate_test.go @@ -32,6 +32,7 @@ func TestMoveResourceStateRequest(t *testing.T) { testCases := map[string]struct { input *tfprotov5.MoveResourceStateRequest resourceSchema fwschema.Schema + identitySchema fwschema.Schema resource resource.Resource expected *fwserver.MoveResourceStateRequest expectedDiagnostics diag.Diagnostics @@ -192,7 +193,7 @@ func TestMoveResourceStateRequest(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - got, diags := fromproto5.MoveResourceStateRequest(context.Background(), testCase.input, testCase.resource, testCase.resourceSchema) + got, diags := fromproto5.MoveResourceStateRequest(context.Background(), testCase.input, testCase.resource, testCase.resourceSchema, testCase.identitySchema) if diff := cmp.Diff(got, testCase.expected); diff != "" { t.Errorf("unexpected difference: %s", diff) diff --git a/internal/fromproto6/moveresourcestate.go b/internal/fromproto6/moveresourcestate.go index cc4c3ad4d..7e85ce9f6 100644 --- a/internal/fromproto6/moveresourcestate.go +++ b/internal/fromproto6/moveresourcestate.go @@ -15,7 +15,7 @@ import ( // MoveResourceStateRequest returns the *fwserver.MoveResourceStateRequest // equivalent of a *tfprotov6.MoveResourceStateRequest. -func MoveResourceStateRequest(ctx context.Context, proto6 *tfprotov6.MoveResourceStateRequest, resource resource.Resource, resourceSchema fwschema.Schema) (*fwserver.MoveResourceStateRequest, diag.Diagnostics) { +func MoveResourceStateRequest(ctx context.Context, proto6 *tfprotov6.MoveResourceStateRequest, resource resource.Resource, resourceSchema fwschema.Schema, identitySchema fwschema.Schema) (*fwserver.MoveResourceStateRequest, diag.Diagnostics) { if proto6 == nil { return nil, nil } @@ -45,6 +45,7 @@ func MoveResourceStateRequest(ctx context.Context, proto6 *tfprotov6.MoveResourc TargetTypeName: proto6.TargetTypeName, SourceIdentity: proto6.SourceIdentity, SourceIdentitySchemaVersion: proto6.SourceIdentitySchemaVersion, + IdentitySchema: identitySchema, } sourcePrivate, sourcePrivateDiags := privatestate.NewData(ctx, proto6.SourcePrivate) diff --git a/internal/fwserver/server_moveresourcestate.go b/internal/fwserver/server_moveresourcestate.go index a76c0c7e2..85a15dd5b 100644 --- a/internal/fwserver/server_moveresourcestate.go +++ b/internal/fwserver/server_moveresourcestate.go @@ -73,6 +73,8 @@ type MoveResourceStateRequest struct { // SourceIdentitySchemaVersion is the version of the source resource state. SourceIdentitySchemaVersion int64 + + IdentitySchema fwschema.Schema } // MoveResourceStateResponse is the framework server response for the @@ -148,10 +150,9 @@ func (s *Server) MoveResourceState(ctx context.Context, req *MoveResourceStateRe Schema: req.TargetResourceSchema, Raw: tftypes.NewValue(req.TargetResourceSchema.Type().TerraformType(ctx), nil), }, - // TODO: See if we need to change this to identity TargetIdentity: &tfsdk.ResourceIdentity{ - Raw: tftypes.NewValue(req.TargetResourceSchema.Type().TerraformType(ctx), nil), - Schema: req.TargetResourceSchema, + Raw: tftypes.NewValue(req.IdentitySchema.Type().TerraformType(ctx), nil), + Schema: req.IdentitySchema, }, } @@ -221,6 +222,16 @@ 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 { @@ -237,6 +248,9 @@ func (s *Server) MoveResourceState(ctx context.Context, req *MoveResourceStateRe if !moveStateResp.TargetState.Raw.Equal(tftypes.NewValue(req.TargetResourceSchema.Type().TerraformType(ctx), nil)) { resp.Diagnostics = moveStateResp.Diagnostics resp.TargetState = &moveStateResp.TargetState + if moveStateResp.TargetIdentity != nil { + resp.TargetIdentity = moveStateResp.TargetIdentity + } if moveStateResp.TargetPrivate != nil { resp.TargetPrivate.Provider = moveStateResp.TargetPrivate diff --git a/internal/fwserver/server_moveresourcestate_test.go b/internal/fwserver/server_moveresourcestate_test.go index 0b9a8e575..8f9703a2e 100644 --- a/internal/fwserver/server_moveresourcestate_test.go +++ b/internal/fwserver/server_moveresourcestate_test.go @@ -43,14 +43,12 @@ func TestServerMoveResourceState(t *testing.T) { testIdentitySchema := identityschema.Schema{ Attributes: map[string]identityschema.Attribute{ - "optionalforimport_attribute": identityschema.StringAttribute{ - OptionalForImport: true, - }, - "requiredforimport_attribute": identityschema.StringAttribute{ + "test_id": identityschema.StringAttribute{ RequiredForImport: true, }, }, } + schemaType := testSchema.Type().TerraformType(ctx) schemaIdentityType := testIdentitySchema.Type().TerraformType(ctx) @@ -878,8 +876,7 @@ func TestServerMoveResourceState(t *testing.T) { request: &fwserver.MoveResourceStateRequest{ // SourceRawState required to prevent error SourceIdentity: testNewRawState(t, map[string]interface{}{ - "optionalforimport_attribute": false, - "requiredforimport_attribute": true, + "test_id": "test_id_value", }), SourceRawState: testNewRawState(t, map[string]interface{}{ "id": "test-id-value", @@ -891,16 +888,14 @@ func TestServerMoveResourceState(t *testing.T) { { StateMover: func(_ context.Context, req resource.MoveStateRequest, resp *resource.MoveStateResponse) { expectedSourceIdentity := testNewRawState(t, map[string]interface{}{ - "optionalforimport_attribute": false, - "requiredforimport_attribute": true, + "test_id": "test_id_value", }) if diff := cmp.Diff(req.SourceIdentity, expectedSourceIdentity); diff != "" { resp.Diagnostics.AddError("Unexpected req.SourceIdentity difference", diff) } - resp.Diagnostics.Append(resp.TargetIdentity.SetAttribute(ctx, path.Root("optionalforimport_attribute"), "false")...) - resp.Diagnostics.Append(resp.TargetIdentity.SetAttribute(ctx, path.Root("requiredforimport_attribute"), "true")...) + resp.Diagnostics.Append(resp.TargetIdentity.SetAttribute(ctx, path.Root("test_id"), "test_id_value")...) // 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")...) @@ -912,6 +907,7 @@ func TestServerMoveResourceState(t *testing.T) { }, TargetResourceSchema: testSchema, TargetTypeName: "test_resource", + IdentitySchema: testIdentitySchema, }, expectedResponse: &fwserver.MoveResourceStateResponse{ TargetPrivate: privatestate.EmptyData(ctx), @@ -925,8 +921,7 @@ func TestServerMoveResourceState(t *testing.T) { }, TargetIdentity: &tfsdk.ResourceIdentity{ Raw: tftypes.NewValue(schemaIdentityType, map[string]tftypes.Value{ - "optionalforimport_attribute": tftypes.NewValue(tftypes.String, "false"), - "requiredforimport_attribute": tftypes.NewValue(tftypes.String, "true"), + "test_id": tftypes.NewValue(tftypes.String, "test_id_value"), }), Schema: testIdentitySchema, }, diff --git a/internal/proto5server/server_moveresourcestate.go b/internal/proto5server/server_moveresourcestate.go index efa1b118c..23b570ed9 100644 --- a/internal/proto5server/server_moveresourcestate.go +++ b/internal/proto5server/server_moveresourcestate.go @@ -36,11 +36,19 @@ func (s *Server) MoveResourceState(ctx context.Context, proto5Req *tfprotov5.Mov fwResp.Diagnostics.Append(diags...) + identitySchema, diags := s.FrameworkServer.ResourceIdentitySchema(ctx, proto5Req.TargetTypeName) + + fwResp.Diagnostics.Append(diags...) + + if fwResp.Diagnostics.HasError() { + return toproto5.MoveResourceStateResponse(ctx, fwResp), nil + } + if fwResp.Diagnostics.HasError() { return toproto5.MoveResourceStateResponse(ctx, fwResp), nil } - fwReq, diags := fromproto5.MoveResourceStateRequest(ctx, proto5Req, resource, resourceSchema) + fwReq, diags := fromproto5.MoveResourceStateRequest(ctx, proto5Req, resource, resourceSchema, identitySchema) fwResp.Diagnostics.Append(diags...) diff --git a/internal/proto6server/server_moveresourcestate.go b/internal/proto6server/server_moveresourcestate.go index 1010b5d76..96e3a96cb 100644 --- a/internal/proto6server/server_moveresourcestate.go +++ b/internal/proto6server/server_moveresourcestate.go @@ -5,7 +5,6 @@ package proto6server import ( "context" - "github.com/hashicorp/terraform-plugin-framework/internal/fromproto6" "github.com/hashicorp/terraform-plugin-framework/internal/fwserver" "github.com/hashicorp/terraform-plugin-framework/internal/logging" @@ -36,11 +35,19 @@ func (s *Server) MoveResourceState(ctx context.Context, proto6Req *tfprotov6.Mov fwResp.Diagnostics.Append(diags...) + identitySchema, diags := s.FrameworkServer.ResourceIdentitySchema(ctx, proto6Req.TargetTypeName) + + fwResp.Diagnostics.Append(diags...) + + if fwResp.Diagnostics.HasError() { + return toproto6.MoveResourceStateResponse(ctx, fwResp), nil + } + if fwResp.Diagnostics.HasError() { return toproto6.MoveResourceStateResponse(ctx, fwResp), nil } - fwReq, diags := fromproto6.MoveResourceStateRequest(ctx, proto6Req, resource, resourceSchema) + fwReq, diags := fromproto6.MoveResourceStateRequest(ctx, proto6Req, resource, resourceSchema, identitySchema) fwResp.Diagnostics.Append(diags...) diff --git a/internal/toproto5/moveresourcestate_test.go b/internal/toproto5/moveresourcestate_test.go index 36075c0bf..948ec6150 100644 --- a/internal/toproto5/moveresourcestate_test.go +++ b/internal/toproto5/moveresourcestate_test.go @@ -5,6 +5,7 @@ package toproto5_test import ( "context" + "github.com/hashicorp/terraform-plugin-framework/resource/identityschema" "testing" "github.com/google/go-cmp/cmp" @@ -33,6 +34,44 @@ func TestMoveResourceStateResponse(t *testing.T) { }, ) + testIdentityProto5Type := tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "test_id": tftypes.String, + }, + } + + testIdentityProto5Value := tftypes.NewValue(testIdentityProto5Type, map[string]tftypes.Value{ + "test_id": tftypes.NewValue(tftypes.String, "id-123"), + }) + + testIdentityProto5DynamicValue, err := tfprotov5.NewDynamicValue(testIdentityProto5Type, testIdentityProto5Value) + + if err != nil { + t.Fatalf("unexpected error calling tfprotov5.NewDynamicValue(): %s", err) + } + + testIdentity := &tfsdk.ResourceIdentity{ + Raw: testIdentityProto5Value, + Schema: identityschema.Schema{ + Attributes: map[string]identityschema.Attribute{ + "test_id": identityschema.StringAttribute{ + RequiredForImport: true, + }, + }, + }, + } + + testIdentityInvalid := &tfsdk.ResourceIdentity{ + Raw: testIdentityProto5Value, + Schema: identityschema.Schema{ + Attributes: map[string]identityschema.Attribute{ + "test_id": identityschema.BoolAttribute{ + RequiredForImport: true, + }, + }, + }, + } + testCases := map[string]struct { input *fwserver.MoveResourceStateResponse expected *tfprotov5.MoveResourceStateResponse @@ -152,6 +191,47 @@ func TestMoveResourceStateResponse(t *testing.T) { }, }, }, + "TargetIdentity": { + input: &fwserver.MoveResourceStateResponse{ + TargetIdentity: testIdentity, + }, + expected: &tfprotov5.MoveResourceStateResponse{ + TargetIdentity: &tfprotov5.ResourceIdentityData{ + IdentityData: &testIdentityProto5DynamicValue, + }, + }, + }, + "TargetIdentity-invalid": { + input: &fwserver.MoveResourceStateResponse{ + Diagnostics: diag.Diagnostics{ + diag.NewWarningDiagnostic("test warning summary", "test warning details"), + diag.NewErrorDiagnostic("test error summary", "test error details"), + }, + TargetIdentity: testIdentityInvalid, + }, + expected: &tfprotov5.MoveResourceStateResponse{ + Diagnostics: []*tfprotov5.Diagnostic{ + { + Severity: tfprotov5.DiagnosticSeverityWarning, + Summary: "test warning summary", + Detail: "test warning details", + }, + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "test error summary", + Detail: "test error details", + }, + { + Severity: tfprotov5.DiagnosticSeverityError, + Summary: "Unable to Convert Resource Identity", + Detail: "An unexpected error was encountered when converting the resource identity to the protocol type. " + + "This is always an issue in terraform-plugin-framework used to implement the provider and should be reported to the provider developers.\n\n" + + "Please report this to the provider developer:\n\n" + + "Unable to create DynamicValue: AttributeName(\"test_id\"): unexpected value type string, tftypes.Bool values must be of type bool", + }, + }, + }, + }, } for name, testCase := range testCases { diff --git a/internal/toproto6/moveresourcestate_test.go b/internal/toproto6/moveresourcestate_test.go index d5281c970..dfed88c69 100644 --- a/internal/toproto6/moveresourcestate_test.go +++ b/internal/toproto6/moveresourcestate_test.go @@ -5,10 +5,11 @@ package toproto6_test import ( "context" + "github.com/hashicorp/terraform-plugin-framework/resource/identityschema" + "github.com/hashicorp/terraform-plugin-go/tfprotov6" "testing" "github.com/google/go-cmp/cmp" - "github.com/hashicorp/terraform-plugin-go/tfprotov6" "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/hashicorp/terraform-plugin-framework/diag" @@ -33,6 +34,44 @@ func TestMoveResourceStateResponse(t *testing.T) { }, ) + testIdentityProto6Type := tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "test_id": tftypes.String, + }, + } + + testIdentityProto6Value := tftypes.NewValue(testIdentityProto6Type, map[string]tftypes.Value{ + "test_id": tftypes.NewValue(tftypes.String, "id-123"), + }) + + testIdentityProto6DynamicValue, err := tfprotov6.NewDynamicValue(testIdentityProto6Type, testIdentityProto6Value) + + if err != nil { + t.Fatalf("unexpected error calling tfprotov6.NewDynamicValue(): %s", err) + } + + testIdentity := &tfsdk.ResourceIdentity{ + Raw: testIdentityProto6Value, + Schema: identityschema.Schema{ + Attributes: map[string]identityschema.Attribute{ + "test_id": identityschema.StringAttribute{ + RequiredForImport: true, + }, + }, + }, + } + + testIdentityInvalid := &tfsdk.ResourceIdentity{ + Raw: testIdentityProto6Value, + Schema: identityschema.Schema{ + Attributes: map[string]identityschema.Attribute{ + "test_id": identityschema.BoolAttribute{ + RequiredForImport: true, + }, + }, + }, + } + testCases := map[string]struct { input *fwserver.MoveResourceStateResponse expected *tfprotov6.MoveResourceStateResponse @@ -152,6 +191,47 @@ func TestMoveResourceStateResponse(t *testing.T) { }, }, }, + "TargetIdentity": { + input: &fwserver.MoveResourceStateResponse{ + TargetIdentity: testIdentity, + }, + expected: &tfprotov6.MoveResourceStateResponse{ + TargetIdentity: &tfprotov6.ResourceIdentityData{ + IdentityData: &testIdentityProto6DynamicValue, + }, + }, + }, + "TargetIdentity-invalid": { + input: &fwserver.MoveResourceStateResponse{ + Diagnostics: diag.Diagnostics{ + diag.NewWarningDiagnostic("test warning summary", "test warning details"), + diag.NewErrorDiagnostic("test error summary", "test error details"), + }, + TargetIdentity: testIdentityInvalid, + }, + expected: &tfprotov6.MoveResourceStateResponse{ + Diagnostics: []*tfprotov6.Diagnostic{ + { + Severity: tfprotov6.DiagnosticSeverityWarning, + Summary: "test warning summary", + Detail: "test warning details", + }, + { + Severity: tfprotov6.DiagnosticSeverityError, + Summary: "test error summary", + Detail: "test error details", + }, + { + Severity: tfprotov6.DiagnosticSeverityError, + Summary: "Unable to Convert Resource Identity", + Detail: "An unexpected error was encountered when converting the resource identity to the protocol type. " + + "This is always an issue in terraform-plugin-framework used to implement the provider and should be reported to the provider developers.\n\n" + + "Please report this to the provider developer:\n\n" + + "Unable to create DynamicValue: AttributeName(\"test_id\"): unexpected value type string, tftypes.Bool values must be of type bool", + }, + }, + }, + }, } for name, testCase := range testCases { From 4512e0cbb11b2601fb4465ea63b6dee672269b9a Mon Sep 17 00:00:00 2001 From: Rain Date: Tue, 1 Apr 2025 18:36:41 -0400 Subject: [PATCH 05/14] Trying out a solution for proto5server/server_moveresourcestate_test, still failing tests --- internal/fwserver/server_moveresourcestate.go | 9 +- .../server_moveresourcestate_test.go | 193 ++++++++++++++++++ .../resourcewithidentityandmovestate.go | 43 ++++ internal/toproto5/resource_identity.go | 2 +- 4 files changed, 245 insertions(+), 2 deletions(-) create mode 100644 internal/testing/testprovider/resourcewithidentityandmovestate.go diff --git a/internal/fwserver/server_moveresourcestate.go b/internal/fwserver/server_moveresourcestate.go index 85a15dd5b..5eeefa6fc 100644 --- a/internal/fwserver/server_moveresourcestate.go +++ b/internal/fwserver/server_moveresourcestate.go @@ -144,6 +144,13 @@ func (s *Server) MoveResourceState(ctx context.Context, req *MoveResourceStateRe SourceIdentity: req.SourceIdentity, SourceIdentitySchemaVersion: req.SourceIdentitySchemaVersion, } + + var taridentity tftypes.Type + + if req.IdentitySchema != nil { + taridentity = req.IdentitySchema.Type().TerraformType(ctx) + } + moveStateResp := resource.MoveStateResponse{ TargetPrivate: privatestate.EmptyProviderData(ctx), TargetState: tfsdk.State{ @@ -151,7 +158,7 @@ func (s *Server) MoveResourceState(ctx context.Context, req *MoveResourceStateRe Raw: tftypes.NewValue(req.TargetResourceSchema.Type().TerraformType(ctx), nil), }, TargetIdentity: &tfsdk.ResourceIdentity{ - Raw: tftypes.NewValue(req.IdentitySchema.Type().TerraformType(ctx), nil), + Raw: tftypes.NewValue(taridentity, nil), Schema: req.IdentitySchema, }, } diff --git a/internal/proto5server/server_moveresourcestate_test.go b/internal/proto5server/server_moveresourcestate_test.go index 5b5e53c97..e4973649e 100644 --- a/internal/proto5server/server_moveresourcestate_test.go +++ b/internal/proto5server/server_moveresourcestate_test.go @@ -6,6 +6,7 @@ package proto5server import ( "context" "fmt" + "github.com/hashicorp/terraform-plugin-framework/resource/identityschema" "testing" "github.com/google/go-cmp/cmp" @@ -39,6 +40,14 @@ func TestServerMoveResourceState(t *testing.T) { } schemaType := testSchema.Type().TerraformType(ctx) + testIdentitySchema := identityschema.Schema{ + Attributes: map[string]identityschema.Attribute{ + "test_id": identityschema.StringAttribute{ + RequiredForImport: true, + }, + }, + } + testCases := map[string]struct { server *Server request *tfprotov5.MoveResourceStateRequest @@ -413,6 +422,132 @@ func TestServerMoveResourceState(t *testing.T) { }), }, }, + "request-SourceIdentitySchemaVersion": { + server: &Server{ + FrameworkServer: fwserver.Server{ + Provider: &testprovider.Provider{ + ResourcesMethod: func(_ context.Context) []func() resource.Resource { + return []func() resource.Resource{ + func() resource.Resource { + return &testprovider.ResourceWithMoveState{ + Resource: &testprovider.Resource{ + SchemaMethod: func(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) { + resp.Schema = testSchema + }, + MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { + resp.TypeName = "test_resource" + }, + }, + MoveStateMethod: func(ctx context.Context) []resource.StateMover { + return []resource.StateMover{ + { + StateMover: func(_ context.Context, req resource.MoveStateRequest, resp *resource.MoveStateResponse) { + expected := int64(123) + + if diff := cmp.Diff(req.SourceIdentitySchemaVersion, expected); diff != "" { + resp.Diagnostics.AddError("Unexpected req.SourceIdentitySchemaVersion difference", diff) + } + + // 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")...) + }, + }, + } + }, + } + }, + } + }, + }, + }, + }, + request: &tfprotov5.MoveResourceStateRequest{ + SourceIdentitySchemaVersion: 123, + // SourceState required to prevent error + SourceState: testNewTfprotov5RawState(t, map[string]interface{}{ + "id": "test-id-value", + "required_attribute": true, + }), + TargetTypeName: "test_resource", + }, + expectedResponse: &tfprotov5.MoveResourceStateResponse{ + TargetState: testNewDynamicValue(t, 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"), + }), + }, + }, + "request-SourceIdentity": { + server: &Server{ + FrameworkServer: fwserver.Server{ + Provider: &testprovider.Provider{ + ResourcesMethod: func(_ context.Context) []func() resource.Resource { + return []func() resource.Resource{ + func() resource.Resource { + return &testprovider.ResourceWithIdentityAndMoveState{ + Resource: &testprovider.Resource{ + SchemaMethod: func(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) { + resp.Schema = testSchema + }, + MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { + resp.TypeName = "test_resource" + }, + }, + MoveStateMethod: func(ctx context.Context) []resource.StateMover { + return []resource.StateMover{ + { + StateMover: func(_ context.Context, req resource.MoveStateRequest, resp *resource.MoveStateResponse) { + expectedSourceRawState := testNewTfprotov6RawState(t, map[string]interface{}{ + "id": "test-id-value", + "required_attribute": true, + }) + + if diff := cmp.Diff(req.SourceRawState, expectedSourceRawState); diff != "" { + resp.Diagnostics.AddError("Unexpected req.SourceRawState difference", diff) + } + + resp.Diagnostics.Append(resp.TargetIdentity.SetAttribute(ctx, path.Root("test_id"), "test_id_value")...) + + // 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")...) + }, + }, + } + }, + IdentitySchemaMethod: func(ctx context.Context, req resource.IdentitySchemaRequest, resp *resource.IdentitySchemaResponse) { + resp.IdentitySchema = testIdentitySchema + }, + } + }, + } + }, + }, + }, + }, + request: &tfprotov5.MoveResourceStateRequest{ + SourceState: testNewTfprotov5RawState(t, map[string]interface{}{ + "id": "test-id-value", + "required_attribute": true, + }), + SourceIdentity: testNewTfprotov5RawState(t, map[string]interface{}{ + "test_id": "test-id-value", + }), + TargetTypeName: "test_resource", + }, + expectedResponse: &tfprotov5.MoveResourceStateResponse{ + TargetIdentity: &tfprotov5.ResourceIdentityData{IdentityData: testNewDynamicValue(t, schemaType, map[string]tftypes.Value{ + "test_id": tftypes.NewValue(tftypes.String, "test-id-value"), + })}, + TargetState: testNewDynamicValue(t, 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"), + }), + }, + }, "request-TargetTypeName-missing": { server: &Server{ FrameworkServer: fwserver.Server{ @@ -705,6 +840,64 @@ func TestServerMoveResourceState(t *testing.T) { }), }, }, + "response-TargetIdentity": { + server: &Server{ + FrameworkServer: fwserver.Server{ + Provider: &testprovider.Provider{ + ResourcesMethod: func(_ context.Context) []func() resource.Resource { + return []func() resource.Resource{ + func() resource.Resource { + return &testprovider.ResourceWithIdentityAndMoveState{ + Resource: &testprovider.Resource{ + SchemaMethod: func(_ context.Context, _ resource.SchemaRequest, resp *resource.SchemaResponse) { + resp.Schema = testSchema + }, + MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { + resp.TypeName = "test_resource" + }, + }, + MoveStateMethod: func(ctx context.Context) []resource.StateMover { + return []resource.StateMover{ + { + StateMover: func(_ context.Context, req resource.MoveStateRequest, resp *resource.MoveStateResponse) { + resp.Diagnostics.Append(resp.TargetIdentity.SetAttribute(ctx, path.Root("test_id"), "test_id_value")...) + 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")...) + }, + }, + } + }, + IdentitySchemaMethod: func(ctx context.Context, req resource.IdentitySchemaRequest, resp *resource.IdentitySchemaResponse) { + resp.IdentitySchema = testIdentitySchema + }, + } + }, + } + }, + }, + }, + }, + request: &tfprotov5.MoveResourceStateRequest{ + SourceState: testNewTfprotov5RawState(t, map[string]interface{}{ + "id": "test-id-value", + "required_attribute": true, + }), + SourceIdentity: testNewTfprotov5RawState(t, map[string]interface{}{ + "test_id": "test-id-value", + }), + TargetTypeName: "test_resource", + }, + expectedResponse: &tfprotov5.MoveResourceStateResponse{ + TargetState: testNewDynamicValue(t, 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"), + }), + TargetIdentity: &tfprotov5.ResourceIdentityData{IdentityData: testNewDynamicValue(t, schemaType, map[string]tftypes.Value{ + "test_id": tftypes.NewValue(tftypes.String, "test-id-value"), + })}, + }, + }, } for name, testCase := range testCases { diff --git a/internal/testing/testprovider/resourcewithidentityandmovestate.go b/internal/testing/testprovider/resourcewithidentityandmovestate.go new file mode 100644 index 000000000..60f4d0676 --- /dev/null +++ b/internal/testing/testprovider/resourcewithidentityandmovestate.go @@ -0,0 +1,43 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package testprovider + +import ( + "context" + + "github.com/hashicorp/terraform-plugin-framework/resource" +) + +var _ resource.Resource = &ResourceWithIdentityAndMoveState{} +var _ resource.ResourceWithIdentity = &ResourceWithIdentityAndMoveState{} +var _ resource.ResourceWithMoveState = &ResourceWithIdentityAndMoveState{} + +// Declarative resource.ResourceWithIdentityAndMoveState for unit testing. +type ResourceWithIdentityAndMoveState struct { + *Resource + + // ResourceWithIdentity interface methods + IdentitySchemaMethod func(context.Context, resource.IdentitySchemaRequest, *resource.IdentitySchemaResponse) + + // ResourceWithMoveState interface methods + MoveStateMethod func(context.Context) []resource.StateMover +} + +// IdentitySchema implements resource.ResourceWithIdentity. +func (p *ResourceWithIdentityAndMoveState) IdentitySchema(ctx context.Context, req resource.IdentitySchemaRequest, resp *resource.IdentitySchemaResponse) { + if p.IdentitySchemaMethod == nil { + return + } + + p.IdentitySchemaMethod(ctx, req, resp) +} + +// MoveState satisfies the resource.ResourceWithMoveState interface. +func (r *ResourceWithIdentityAndMoveState) MoveState(ctx context.Context) []resource.StateMover { + if r.MoveStateMethod == nil { + return nil + } + + return r.MoveStateMethod(ctx) +} diff --git a/internal/toproto5/resource_identity.go b/internal/toproto5/resource_identity.go index eea046ac9..671fa0c9f 100644 --- a/internal/toproto5/resource_identity.go +++ b/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 { + if fw == nil || fw.Schema == nil { return nil, nil } From 25adb9990101417bfe81b4eef149745d696be1f6 Mon Sep 17 00:00:00 2001 From: Rain Date: Wed, 2 Apr 2025 14:34:45 -0400 Subject: [PATCH 06/14] Fixed failing tests --- internal/fromproto6/moveresourcestate_test.go | 3 ++- internal/fwserver/server_moveresourcestate.go | 15 +++++-------- .../server_moveresourcestate_test.go | 22 +++++++++++++++---- 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/internal/fromproto6/moveresourcestate_test.go b/internal/fromproto6/moveresourcestate_test.go index 00f33f750..d53449a72 100644 --- a/internal/fromproto6/moveresourcestate_test.go +++ b/internal/fromproto6/moveresourcestate_test.go @@ -32,6 +32,7 @@ func TestMoveResourceStateRequest(t *testing.T) { testCases := map[string]struct { input *tfprotov6.MoveResourceStateRequest resourceSchema fwschema.Schema + identitySchema fwschema.Schema resource resource.Resource expected *fwserver.MoveResourceStateRequest expectedDiagnostics diag.Diagnostics @@ -192,7 +193,7 @@ func TestMoveResourceStateRequest(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - got, diags := fromproto6.MoveResourceStateRequest(context.Background(), testCase.input, testCase.resource, testCase.resourceSchema) + got, diags := fromproto6.MoveResourceStateRequest(context.Background(), testCase.input, testCase.resource, testCase.resourceSchema, testCase.identitySchema) if diff := cmp.Diff(got, testCase.expected); diff != "" { t.Errorf("unexpected difference: %s", diff) diff --git a/internal/fwserver/server_moveresourcestate.go b/internal/fwserver/server_moveresourcestate.go index 5eeefa6fc..4028a864c 100644 --- a/internal/fwserver/server_moveresourcestate.go +++ b/internal/fwserver/server_moveresourcestate.go @@ -145,22 +145,19 @@ func (s *Server) MoveResourceState(ctx context.Context, req *MoveResourceStateRe SourceIdentitySchemaVersion: req.SourceIdentitySchemaVersion, } - var taridentity tftypes.Type - - if req.IdentitySchema != nil { - taridentity = req.IdentitySchema.Type().TerraformType(ctx) - } - moveStateResp := resource.MoveStateResponse{ TargetPrivate: privatestate.EmptyProviderData(ctx), TargetState: tfsdk.State{ Schema: req.TargetResourceSchema, Raw: tftypes.NewValue(req.TargetResourceSchema.Type().TerraformType(ctx), nil), }, - TargetIdentity: &tfsdk.ResourceIdentity{ - Raw: tftypes.NewValue(taridentity, nil), + } + + if req.IdentitySchema != nil { + moveStateResp.TargetIdentity = &tfsdk.ResourceIdentity{ + Raw: tftypes.NewValue(req.IdentitySchema.Type().TerraformType(ctx), nil), Schema: req.IdentitySchema, - }, + } } if resourceStateMover.SourceSchema != nil { diff --git a/internal/proto5server/server_moveresourcestate_test.go b/internal/proto5server/server_moveresourcestate_test.go index e4973649e..17b11fc47 100644 --- a/internal/proto5server/server_moveresourcestate_test.go +++ b/internal/proto5server/server_moveresourcestate_test.go @@ -48,6 +48,12 @@ func TestServerMoveResourceState(t *testing.T) { }, } + testIdentityType := tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "test_id": tftypes.String, + }, + } + testCases := map[string]struct { server *Server request *tfprotov5.MoveResourceStateRequest @@ -508,7 +514,15 @@ func TestServerMoveResourceState(t *testing.T) { resp.Diagnostics.AddError("Unexpected req.SourceRawState difference", diff) } - resp.Diagnostics.Append(resp.TargetIdentity.SetAttribute(ctx, path.Root("test_id"), "test_id_value")...) + expectedSourceIdentity := testNewTfprotov6RawState(t, map[string]interface{}{ + "test_id": "test-id-value", + }) + + if diff := cmp.Diff(req.SourceIdentity, expectedSourceIdentity); diff != "" { + resp.Diagnostics.AddError("Unexpected req.SourceIdentity difference", diff) + } + + resp.Diagnostics.Append(resp.TargetIdentity.SetAttribute(ctx, path.Root("test_id"), "test-id-value")...) // 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")...) @@ -538,7 +552,7 @@ func TestServerMoveResourceState(t *testing.T) { TargetTypeName: "test_resource", }, expectedResponse: &tfprotov5.MoveResourceStateResponse{ - TargetIdentity: &tfprotov5.ResourceIdentityData{IdentityData: testNewDynamicValue(t, schemaType, map[string]tftypes.Value{ + TargetIdentity: &tfprotov5.ResourceIdentityData{IdentityData: testNewDynamicValue(t, testIdentityType, map[string]tftypes.Value{ "test_id": tftypes.NewValue(tftypes.String, "test-id-value"), })}, TargetState: testNewDynamicValue(t, schemaType, map[string]tftypes.Value{ @@ -860,7 +874,7 @@ func TestServerMoveResourceState(t *testing.T) { return []resource.StateMover{ { StateMover: func(_ context.Context, req resource.MoveStateRequest, resp *resource.MoveStateResponse) { - resp.Diagnostics.Append(resp.TargetIdentity.SetAttribute(ctx, path.Root("test_id"), "test_id_value")...) + resp.Diagnostics.Append(resp.TargetIdentity.SetAttribute(ctx, path.Root("test_id"), "test-id-value")...) 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")...) }, @@ -893,7 +907,7 @@ func TestServerMoveResourceState(t *testing.T) { "optional_attribute": tftypes.NewValue(tftypes.String, nil), "required_attribute": tftypes.NewValue(tftypes.String, "true"), }), - TargetIdentity: &tfprotov5.ResourceIdentityData{IdentityData: testNewDynamicValue(t, schemaType, map[string]tftypes.Value{ + TargetIdentity: &tfprotov5.ResourceIdentityData{IdentityData: testNewDynamicValue(t, testIdentityType, map[string]tftypes.Value{ "test_id": tftypes.NewValue(tftypes.String, "test-id-value"), })}, }, From 6ac71cf36bb075090b43116160d5ed32a6ee192a Mon Sep 17 00:00:00 2001 From: Rain Kwan <91649079+rainkwan@users.noreply.github.com> Date: Tue, 15 Apr 2025 09:42:37 -0400 Subject: [PATCH 07/14] Update resource/move_state.go Co-authored-by: Austin Valle --- resource/move_state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resource/move_state.go b/resource/move_state.go index 49e0a56c3..977f565d9 100644 --- a/resource/move_state.go +++ b/resource/move_state.go @@ -81,7 +81,7 @@ type MoveStateRequest struct { // Only the underlying JSON field is populated. SourceIdentity *tfprotov6.RawState - // SourceIdentitySchemaVersion is the version of the source resource state. + // SourceIdentitySchemaVersion is the version of the source resource identity. SourceIdentitySchemaVersion int64 } From 21f096c68f429d309698f6e5c2de3d40c056ae27 Mon Sep 17 00:00:00 2001 From: Rain Date: Wed, 16 Apr 2025 14:06:07 -0400 Subject: [PATCH 08/14] Added a response-invalid-identity test to test invalid identity responses --- internal/fwserver/server_moveresourcestate.go | 14 +------ .../fwserver/server_moveresourcestate_test.go | 39 +++++++++++++++++++ internal/toproto5/resource_identity.go | 2 +- tfsdk/resource_identity.go | 11 +++++- 4 files changed, 51 insertions(+), 15 deletions(-) diff --git a/internal/fwserver/server_moveresourcestate.go b/internal/fwserver/server_moveresourcestate.go index 4028a864c..4937fa042 100644 --- a/internal/fwserver/server_moveresourcestate.go +++ b/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, ) } diff --git a/internal/fwserver/server_moveresourcestate_test.go b/internal/fwserver/server_moveresourcestate_test.go index 8f9703a2e..d7d134009 100644 --- a/internal/fwserver/server_moveresourcestate_test.go +++ b/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 { diff --git a/internal/toproto5/resource_identity.go b/internal/toproto5/resource_identity.go index 671fa0c9f..eea046ac9 100644 --- a/internal/toproto5/resource_identity.go +++ b/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 } diff --git a/tfsdk/resource_identity.go b/tfsdk/resource_identity.go index d02f1e8e3..b42416c7a 100644 --- a/tfsdk/resource_identity.go +++ b/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) From b52253dedbf35c3af3192e29386cfa859ed922ed Mon Sep 17 00:00:00 2001 From: Rain Kwan <91649079+rainkwan@users.noreply.github.com> Date: Thu, 17 Apr 2025 10:23:52 -0400 Subject: [PATCH 09/14] Update tfsdk/resource_identity.go Co-authored-by: Austin Valle --- tfsdk/resource_identity.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tfsdk/resource_identity.go b/tfsdk/resource_identity.go index b42416c7a..fd9f30579 100644 --- a/tfsdk/resource_identity.go +++ b/tfsdk/resource_identity.go @@ -73,8 +73,8 @@ func (s *ResourceIdentity) SetAttribute(ctx context.Context, path path.Path, val 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"+ + "Missing Identity Definition", + "An unexpected error was encountered when attempting to set a resource identity attribute. The resource does not indicate support via a resource identity schema.\n\n"+ "This is always a problem with the provider and should be reported to the provider developer."), } } From 6ad98d710c1fb1883af689ba73bd89ecd8df9790 Mon Sep 17 00:00:00 2001 From: Rain Kwan <91649079+rainkwan@users.noreply.github.com> Date: Thu, 17 Apr 2025 10:24:20 -0400 Subject: [PATCH 10/14] Update internal/fwserver/server_moveresourcestate_test.go Co-authored-by: Austin Valle --- internal/fwserver/server_moveresourcestate_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/internal/fwserver/server_moveresourcestate_test.go b/internal/fwserver/server_moveresourcestate_test.go index d7d134009..a92fecd81 100644 --- a/internal/fwserver/server_moveresourcestate_test.go +++ b/internal/fwserver/server_moveresourcestate_test.go @@ -941,8 +941,13 @@ func TestServerMoveResourceState(t *testing.T) { 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")...) + // This resource doesn't indicate identity support (via a schema), so this should raise a diagnostic. + resp.TargetIdentity = &tfsdk.ResourceIdentity{ + Raw: tftypes.NewValue(testIdentitySchema.Type().TerraformType(ctx), map[string]tftypes.Value{ + "test_id": tftypes.NewValue(tftypes.String, "should-not-be-set"), + }), + Schema: testIdentitySchema, + } // 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")...) From 3226b918686027f6619f17d0ec05e36176ce34fe Mon Sep 17 00:00:00 2001 From: Rain Date: Thu, 17 Apr 2025 11:28:20 -0400 Subject: [PATCH 11/14] Fixing the pipeline --- internal/fwserver/server_moveresourcestate.go | 10 ++++++++++ .../fwserver/server_moveresourcestate_test.go | 19 +++++++++++++------ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/internal/fwserver/server_moveresourcestate.go b/internal/fwserver/server_moveresourcestate.go index 4937fa042..32063fff9 100644 --- a/internal/fwserver/server_moveresourcestate.go +++ b/internal/fwserver/server_moveresourcestate.go @@ -226,6 +226,16 @@ 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 { diff --git a/internal/fwserver/server_moveresourcestate_test.go b/internal/fwserver/server_moveresourcestate_test.go index a92fecd81..39e6cdfcb 100644 --- a/internal/fwserver/server_moveresourcestate_test.go +++ b/internal/fwserver/server_moveresourcestate_test.go @@ -961,12 +961,19 @@ func TestServerMoveResourceState(t *testing.T) { 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.", - ), + TargetState: &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: testSchema, + }, + TargetIdentity: &tfsdk.ResourceIdentity{ + Raw: tftypes.NewValue(schemaIdentityType, map[string]tftypes.Value{ + "test_id": tftypes.NewValue(tftypes.String, "should-not-be-set"), + }), + Schema: testIdentitySchema, }, TargetPrivate: privatestate.EmptyData(ctx), }, From d7d1d94151898a11bf9180befddc20e58e45bb19 Mon Sep 17 00:00:00 2001 From: Rain Kwan <91649079+rainkwan@users.noreply.github.com> Date: Fri, 18 Apr 2025 16:07:45 -0400 Subject: [PATCH 12/14] Update internal/fwserver/server_moveresourcestate.go Co-authored-by: Austin Valle --- internal/fwserver/server_moveresourcestate.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/internal/fwserver/server_moveresourcestate.go b/internal/fwserver/server_moveresourcestate.go index 32063fff9..4937fa042 100644 --- a/internal/fwserver/server_moveresourcestate.go +++ b/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 { From fc058a85a8ba28610931cf5bbe791dae73e1a97a Mon Sep 17 00:00:00 2001 From: Rain Kwan <91649079+rainkwan@users.noreply.github.com> Date: Fri, 18 Apr 2025 16:08:10 -0400 Subject: [PATCH 13/14] Update internal/fwserver/server_moveresourcestate.go Co-authored-by: Austin Valle --- internal/fwserver/server_moveresourcestate.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/internal/fwserver/server_moveresourcestate.go b/internal/fwserver/server_moveresourcestate.go index 4937fa042..b838687d3 100644 --- a/internal/fwserver/server_moveresourcestate.go +++ b/internal/fwserver/server_moveresourcestate.go @@ -246,6 +246,16 @@ func (s *Server) MoveResourceState(ctx context.Context, req *MoveResourceStateRe resp.TargetIdentity = moveStateResp.TargetIdentity } + 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 + } + if moveStateResp.TargetPrivate != nil { resp.TargetPrivate.Provider = moveStateResp.TargetPrivate } From 097c894187db5b8d990fd5b9766b3c5e392c9e9c Mon Sep 17 00:00:00 2001 From: Rain Date: Fri, 18 Apr 2025 16:26:09 -0400 Subject: [PATCH 14/14] Updating the response-invalid-identity test output --- internal/fwserver/server_moveresourcestate_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/fwserver/server_moveresourcestate_test.go b/internal/fwserver/server_moveresourcestate_test.go index 39e6cdfcb..90e32aeb4 100644 --- a/internal/fwserver/server_moveresourcestate_test.go +++ b/internal/fwserver/server_moveresourcestate_test.go @@ -961,6 +961,13 @@ func TestServerMoveResourceState(t *testing.T) { 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. 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.", + ), + }, TargetState: &tfsdk.State{ Raw: tftypes.NewValue(schemaType, map[string]tftypes.Value{ "id": tftypes.NewValue(tftypes.String, "test-id-value"),