Skip to content

Commit caa518b

Browse files
rainkwanrainkwan
committed
Added more tests for identity to move rpc and added identitySchema to MoveState Request
1 parent f674e29 commit caa518b

File tree

9 files changed

+209
-23
lines changed

9 files changed

+209
-23
lines changed

‎internal/fromproto5/moveresourcestate.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package fromproto5
55

66
import (
77
"context"
8-
98
"github.com/hashicorp/terraform-plugin-framework/diag"
109
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema"
1110
"github.com/hashicorp/terraform-plugin-framework/internal/fwserver"
@@ -17,7 +16,7 @@ import (
1716

1817
// MoveResourceStateRequest returns the *fwserver.MoveResourceStateRequest
1918
// equivalent of a *tfprotov5.MoveResourceStateRequest.
20-
func MoveResourceStateRequest(ctx context.Context, proto5 *tfprotov5.MoveResourceStateRequest, resource resource.Resource, resourceSchema fwschema.Schema) (*fwserver.MoveResourceStateRequest, diag.Diagnostics) {
19+
func MoveResourceStateRequest(ctx context.Context, proto5 *tfprotov5.MoveResourceStateRequest, resource resource.Resource, resourceSchema fwschema.Schema, identitySchema fwschema.Schema) (*fwserver.MoveResourceStateRequest, diag.Diagnostics) {
2120
if proto5 == nil {
2221
return nil, nil
2322
}
@@ -47,6 +46,7 @@ func MoveResourceStateRequest(ctx context.Context, proto5 *tfprotov5.MoveResourc
4746
TargetTypeName: proto5.TargetTypeName,
4847
SourceIdentity: (*tfprotov6.RawState)(proto5.SourceIdentity),
4948
SourceIdentitySchemaVersion: proto5.SourceIdentitySchemaVersion,
49+
IdentitySchema: identitySchema,
5050
}
5151

5252
sourcePrivate, sourcePrivateDiags := privatestate.NewData(ctx, proto5.SourcePrivate)

‎internal/fromproto5/moveresourcestate_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ func TestMoveResourceStateRequest(t *testing.T) {
3232
testCases := map[string]struct {
3333
input *tfprotov5.MoveResourceStateRequest
3434
resourceSchema fwschema.Schema
35+
identitySchema fwschema.Schema
3536
resource resource.Resource
3637
expected *fwserver.MoveResourceStateRequest
3738
expectedDiagnostics diag.Diagnostics
@@ -192,7 +193,7 @@ func TestMoveResourceStateRequest(t *testing.T) {
192193
t.Run(name, func(t *testing.T) {
193194
t.Parallel()
194195

195-
got, diags := fromproto5.MoveResourceStateRequest(context.Background(), testCase.input, testCase.resource, testCase.resourceSchema)
196+
got, diags := fromproto5.MoveResourceStateRequest(context.Background(), testCase.input, testCase.resource, testCase.resourceSchema, testCase.identitySchema)
196197

197198
if diff := cmp.Diff(got, testCase.expected); diff != "" {
198199
t.Errorf("unexpected difference: %s", diff)

‎internal/fromproto6/moveresourcestate.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515

1616
// MoveResourceStateRequest returns the *fwserver.MoveResourceStateRequest
1717
// equivalent of a *tfprotov6.MoveResourceStateRequest.
18-
func MoveResourceStateRequest(ctx context.Context, proto6 *tfprotov6.MoveResourceStateRequest, resource resource.Resource, resourceSchema fwschema.Schema) (*fwserver.MoveResourceStateRequest, diag.Diagnostics) {
18+
func MoveResourceStateRequest(ctx context.Context, proto6 *tfprotov6.MoveResourceStateRequest, resource resource.Resource, resourceSchema fwschema.Schema, identitySchema fwschema.Schema) (*fwserver.MoveResourceStateRequest, diag.Diagnostics) {
1919
if proto6 == nil {
2020
return nil, nil
2121
}
@@ -45,6 +45,7 @@ func MoveResourceStateRequest(ctx context.Context, proto6 *tfprotov6.MoveResourc
4545
TargetTypeName: proto6.TargetTypeName,
4646
SourceIdentity: proto6.SourceIdentity,
4747
SourceIdentitySchemaVersion: proto6.SourceIdentitySchemaVersion,
48+
IdentitySchema: identitySchema,
4849
}
4950

5051
sourcePrivate, sourcePrivateDiags := privatestate.NewData(ctx, proto6.SourcePrivate)

‎internal/fwserver/server_moveresourcestate.go‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ type MoveResourceStateRequest struct {
7373

7474
// SourceIdentitySchemaVersion is the version of the source resource state.
7575
SourceIdentitySchemaVersion int64
76+
77+
IdentitySchema fwschema.Schema
7678
}
7779

7880
// MoveResourceStateResponse is the framework server response for the
@@ -148,10 +150,9 @@ func (s *Server) MoveResourceState(ctx context.Context, req *MoveResourceStateRe
148150
Schema: req.TargetResourceSchema,
149151
Raw: tftypes.NewValue(req.TargetResourceSchema.Type().TerraformType(ctx), nil),
150152
},
151-
// TODO: See if we need to change this to identity
152153
TargetIdentity: &tfsdk.ResourceIdentity{
153-
Raw: tftypes.NewValue(req.TargetResourceSchema.Type().TerraformType(ctx), nil),
154-
Schema: req.TargetResourceSchema,
154+
Raw: tftypes.NewValue(req.IdentitySchema.Type().TerraformType(ctx), nil),
155+
Schema: req.IdentitySchema,
155156
},
156157
}
157158

@@ -221,6 +222,16 @@ func (s *Server) MoveResourceState(ctx context.Context, req *MoveResourceStateRe
221222
return
222223
}
223224

225+
if resp.TargetIdentity != nil && req.IdentitySchema == nil {
226+
resp.Diagnostics.AddError(
227+
"Unexpected Move State Response",
228+
"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"+
229+
"This is always a problem with the provider and should be reported to the provider developer.",
230+
)
231+
232+
return
233+
}
234+
224235
// Set any write-only attributes in the move resource state to null
225236
modifiedState, err := tftypes.Transform(moveStateResp.TargetState.Raw, NullifyWriteOnlyAttributes(ctx, moveStateResp.TargetState.Schema))
226237
if err != nil {
@@ -237,6 +248,9 @@ func (s *Server) MoveResourceState(ctx context.Context, req *MoveResourceStateRe
237248
if !moveStateResp.TargetState.Raw.Equal(tftypes.NewValue(req.TargetResourceSchema.Type().TerraformType(ctx), nil)) {
238249
resp.Diagnostics = moveStateResp.Diagnostics
239250
resp.TargetState = &moveStateResp.TargetState
251+
if moveStateResp.TargetIdentity != nil {
252+
resp.TargetIdentity = moveStateResp.TargetIdentity
253+
}
240254

241255
if moveStateResp.TargetPrivate != nil {
242256
resp.TargetPrivate.Provider = moveStateResp.TargetPrivate

‎internal/fwserver/server_moveresourcestate_test.go‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,12 @@ func TestServerMoveResourceState(t *testing.T) {
4343

4444
testIdentitySchema := identityschema.Schema{
4545
Attributes: map[string]identityschema.Attribute{
46-
"optionalforimport_attribute": identityschema.StringAttribute{
47-
OptionalForImport: true,
48-
},
49-
"requiredforimport_attribute": identityschema.StringAttribute{
46+
"test_id": identityschema.StringAttribute{
5047
RequiredForImport: true,
5148
},
5249
},
5350
}
51+
5452
schemaType := testSchema.Type().TerraformType(ctx)
5553

5654
schemaIdentityType := testIdentitySchema.Type().TerraformType(ctx)
@@ -878,8 +876,7 @@ func TestServerMoveResourceState(t *testing.T) {
878876
request: &fwserver.MoveResourceStateRequest{
879877
// SourceRawState required to prevent error
880878
SourceIdentity: testNewRawState(t, map[string]interface{}{
881-
"optionalforimport_attribute": false,
882-
"requiredforimport_attribute": true,
879+
"test_id": "test_id_value",
883880
}),
884881
SourceRawState: testNewRawState(t, map[string]interface{}{
885882
"id": "test-id-value",
@@ -891,16 +888,14 @@ func TestServerMoveResourceState(t *testing.T) {
891888
{
892889
StateMover: func(_ context.Context, req resource.MoveStateRequest, resp *resource.MoveStateResponse) {
893890
expectedSourceIdentity := testNewRawState(t, map[string]interface{}{
894-
"optionalforimport_attribute": false,
895-
"requiredforimport_attribute": true,
891+
"test_id": "test_id_value",
896892
})
897893

898894
if diff := cmp.Diff(req.SourceIdentity, expectedSourceIdentity); diff != "" {
899895
resp.Diagnostics.AddError("Unexpected req.SourceIdentity difference", diff)
900896
}
901897

902-
resp.Diagnostics.Append(resp.TargetIdentity.SetAttribute(ctx, path.Root("optionalforimport_attribute"), "false")...)
903-
resp.Diagnostics.Append(resp.TargetIdentity.SetAttribute(ctx, path.Root("requiredforimport_attribute"), "true")...)
898+
resp.Diagnostics.Append(resp.TargetIdentity.SetAttribute(ctx, path.Root("test_id"), "test_id_value")...)
904899

905900
// Prevent missing implementation error, the values do not matter except for response assertion
906901
resp.Diagnostics.Append(resp.TargetState.SetAttribute(ctx, path.Root("id"), "test-id-value")...)
@@ -912,6 +907,7 @@ func TestServerMoveResourceState(t *testing.T) {
912907
},
913908
TargetResourceSchema: testSchema,
914909
TargetTypeName: "test_resource",
910+
IdentitySchema: testIdentitySchema,
915911
},
916912
expectedResponse: &fwserver.MoveResourceStateResponse{
917913
TargetPrivate: privatestate.EmptyData(ctx),
@@ -925,8 +921,7 @@ func TestServerMoveResourceState(t *testing.T) {
925921
},
926922
TargetIdentity: &tfsdk.ResourceIdentity{
927923
Raw: tftypes.NewValue(schemaIdentityType, map[string]tftypes.Value{
928-
"optionalforimport_attribute": tftypes.NewValue(tftypes.String, "false"),
929-
"requiredforimport_attribute": tftypes.NewValue(tftypes.String, "true"),
924+
"test_id": tftypes.NewValue(tftypes.String, "test_id_value"),
930925
}),
931926
Schema: testIdentitySchema,
932927
},

‎internal/proto5server/server_moveresourcestate.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,19 @@ func (s *Server) MoveResourceState(ctx context.Context, proto5Req *tfprotov5.Mov
3636

3737
fwResp.Diagnostics.Append(diags...)
3838

39+
identitySchema, diags := s.FrameworkServer.ResourceIdentitySchema(ctx, proto5Req.TargetTypeName)
40+
41+
fwResp.Diagnostics.Append(diags...)
42+
43+
if fwResp.Diagnostics.HasError() {
44+
return toproto5.MoveResourceStateResponse(ctx, fwResp), nil
45+
}
46+
3947
if fwResp.Diagnostics.HasError() {
4048
return toproto5.MoveResourceStateResponse(ctx, fwResp), nil
4149
}
4250

43-
fwReq, diags := fromproto5.MoveResourceStateRequest(ctx, proto5Req, resource, resourceSchema)
51+
fwReq, diags := fromproto5.MoveResourceStateRequest(ctx, proto5Req, resource, resourceSchema, identitySchema)
4452

4553
fwResp.Diagnostics.Append(diags...)
4654

‎internal/proto6server/server_moveresourcestate.go‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package proto6server
55

66
import (
77
"context"
8-
98
"github.com/hashicorp/terraform-plugin-framework/internal/fromproto6"
109
"github.com/hashicorp/terraform-plugin-framework/internal/fwserver"
1110
"github.com/hashicorp/terraform-plugin-framework/internal/logging"
@@ -36,11 +35,19 @@ func (s *Server) MoveResourceState(ctx context.Context, proto6Req *tfprotov6.Mov
3635

3736
fwResp.Diagnostics.Append(diags...)
3837

38+
identitySchema, diags := s.FrameworkServer.ResourceIdentitySchema(ctx, proto6Req.TargetTypeName)
39+
40+
fwResp.Diagnostics.Append(diags...)
41+
42+
if fwResp.Diagnostics.HasError() {
43+
return toproto6.MoveResourceStateResponse(ctx, fwResp), nil
44+
}
45+
3946
if fwResp.Diagnostics.HasError() {
4047
return toproto6.MoveResourceStateResponse(ctx, fwResp), nil
4148
}
4249

43-
fwReq, diags := fromproto6.MoveResourceStateRequest(ctx, proto6Req, resource, resourceSchema)
50+
fwReq, diags := fromproto6.MoveResourceStateRequest(ctx, proto6Req, resource, resourceSchema, identitySchema)
4451

4552
fwResp.Diagnostics.Append(diags...)
4653

‎internal/toproto5/moveresourcestate_test.go‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package toproto5_test
55

66
import (
77
"context"
8+
"github.com/hashicorp/terraform-plugin-framework/resource/identityschema"
89
"testing"
910

1011
"github.com/google/go-cmp/cmp"
@@ -33,6 +34,44 @@ func TestMoveResourceStateResponse(t *testing.T) {
3334
},
3435
)
3536

37+
testIdentityProto5Type := tftypes.Object{
38+
AttributeTypes: map[string]tftypes.Type{
39+
"test_id": tftypes.String,
40+
},
41+
}
42+
43+
testIdentityProto5Value := tftypes.NewValue(testIdentityProto5Type, map[string]tftypes.Value{
44+
"test_id": tftypes.NewValue(tftypes.String, "id-123"),
45+
})
46+
47+
testIdentityProto5DynamicValue, err := tfprotov5.NewDynamicValue(testIdentityProto5Type, testIdentityProto5Value)
48+
49+
if err != nil {
50+
t.Fatalf("unexpected error calling tfprotov5.NewDynamicValue(): %s", err)
51+
}
52+
53+
testIdentity := &tfsdk.ResourceIdentity{
54+
Raw: testIdentityProto5Value,
55+
Schema: identityschema.Schema{
56+
Attributes: map[string]identityschema.Attribute{
57+
"test_id": identityschema.StringAttribute{
58+
RequiredForImport: true,
59+
},
60+
},
61+
},
62+
}
63+
64+
testIdentityInvalid := &tfsdk.ResourceIdentity{
65+
Raw: testIdentityProto5Value,
66+
Schema: identityschema.Schema{
67+
Attributes: map[string]identityschema.Attribute{
68+
"test_id": identityschema.BoolAttribute{
69+
RequiredForImport: true,
70+
},
71+
},
72+
},
73+
}
74+
3675
testCases := map[string]struct {
3776
input *fwserver.MoveResourceStateResponse
3877
expected *tfprotov5.MoveResourceStateResponse
@@ -152,6 +191,47 @@ func TestMoveResourceStateResponse(t *testing.T) {
152191
},
153192
},
154193
},
194+
"TargetIdentity": {
195+
input: &fwserver.MoveResourceStateResponse{
196+
TargetIdentity: testIdentity,
197+
},
198+
expected: &tfprotov5.MoveResourceStateResponse{
199+
TargetIdentity: &tfprotov5.ResourceIdentityData{
200+
IdentityData: &testIdentityProto5DynamicValue,
201+
},
202+
},
203+
},
204+
"TargetIdentity-invalid": {
205+
input: &fwserver.MoveResourceStateResponse{
206+
Diagnostics: diag.Diagnostics{
207+
diag.NewWarningDiagnostic("test warning summary", "test warning details"),
208+
diag.NewErrorDiagnostic("test error summary", "test error details"),
209+
},
210+
TargetIdentity: testIdentityInvalid,
211+
},
212+
expected: &tfprotov5.MoveResourceStateResponse{
213+
Diagnostics: []*tfprotov5.Diagnostic{
214+
{
215+
Severity: tfprotov5.DiagnosticSeverityWarning,
216+
Summary: "test warning summary",
217+
Detail: "test warning details",
218+
},
219+
{
220+
Severity: tfprotov5.DiagnosticSeverityError,
221+
Summary: "test error summary",
222+
Detail: "test error details",
223+
},
224+
{
225+
Severity: tfprotov5.DiagnosticSeverityError,
226+
Summary: "Unable to Convert Resource Identity",
227+
Detail: "An unexpected error was encountered when converting the resource identity to the protocol type. " +
228+
"This is always an issue in terraform-plugin-framework used to implement the provider and should be reported to the provider developers.\n\n" +
229+
"Please report this to the provider developer:\n\n" +
230+
"Unable to create DynamicValue: AttributeName(\"test_id\"): unexpected value type string, tftypes.Bool values must be of type bool",
231+
},
232+
},
233+
},
234+
},
155235
}
156236

157237
for name, testCase := range testCases {

0 commit comments

Comments
 (0)