From c19f43b492e66e0c8dd36aa156604fcb0df5f97b Mon Sep 17 00:00:00 2001 From: Steph Date: Tue, 5 Aug 2025 13:30:34 +0200 Subject: [PATCH 01/12] framework changes support list in sdkv2 --- internal/fwserver/server_listresource.go | 81 ++++++++++++++++++-- internal/fwserver/server_listresources.go | 22 +++--- internal/proto5server/server_listresource.go | 33 ++++++++ list/list_resource.go | 5 +- list/metadata.go | 7 ++ 5 files changed, 132 insertions(+), 16 deletions(-) diff --git a/internal/fwserver/server_listresource.go b/internal/fwserver/server_listresource.go index 74b0955c1..602b3ffd5 100644 --- a/internal/fwserver/server_listresource.go +++ b/internal/fwserver/server_listresource.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/list" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/hashicorp/terraform-plugin-go/tfprotov5" "github.com/hashicorp/terraform-plugin-go/tftypes" ) @@ -49,6 +50,8 @@ type ListResultsStream struct { // Results is a function that emits [ListResult] values via its push // function argument. Results iter.Seq[ListResult] + + ResultsProtoV5 iter.Seq[tfprotov5.ListResourceResult] } func ListResultError(summary string, detail string) ListResult { @@ -59,6 +62,18 @@ func ListResultError(summary string, detail string) ListResult { } } +func ListResultErrorProto5(summary string, detail string) tfprotov5.ListResourceResult { + return tfprotov5.ListResourceResult{ + Diagnostics: []*tfprotov5.Diagnostic{ + { + Severity: 1, + Summary: summary, + Detail: detail, + }, + }, + } +} + // ListResult represents a listed managed resource instance. type ListResult struct { // Identity is the identity of the managed resource instance. A nil value @@ -106,8 +121,10 @@ func (s *Server) ListResource(ctx context.Context, fwReq *ListRequest, fwStream if listResourceWithConfigure, ok := listResource.(list.ListResourceWithConfigure); ok { logging.FrameworkTrace(ctx, "ListResource implements ListResourceWithConfigure") + // ListResourceConfigureData isn't populated in the ConfigureProvider RPC + // We can use ResourceConfigureData here for now or populate ListResourceConfigureData configureReq := resource.ConfigureRequest{ - ProviderData: s.ListResourceConfigureData, + ProviderData: s.ResourceConfigureData, } configureResp := resource.ConfigureResponse{} @@ -156,15 +173,24 @@ func (s *Server) ListResource(ctx context.Context, fwReq *ListRequest, fwStream logging.FrameworkTrace(ctx, "Called provider defined ListResource") // If the provider returned a nil results stream, we return an empty stream. - if stream.Results == nil { - stream.Results = list.NoListResults - } - if diagsStream.Results == nil { diagsStream.Results = list.NoListResults } - fwStream.Results = processListResults(req, stream.Results, diagsStream.Results) + if stream.Results == nil || stream.Proto5Results == nil { + fwStream.Results = processListResults(req, list.NoListResults, diagsStream.Results) + } + + if stream.Results != nil { + fwStream.Results = processListResults(req, stream.Results, diagsStream.Results) + } + + if stream.Proto5Results != nil { + // TODO merge with diagsStream if needed + fwStream.ResultsProtoV5 = processListResultsProto5(req, stream.Proto5Results) + } + + return } func processListResults(req list.ListRequest, streams ...iter.Seq[list.ListResult]) iter.Seq[ListResult] { @@ -213,3 +239,46 @@ func processListResult(req list.ListRequest, result list.ListResult) ListResult return ListResult(result) } + +// Duplicated functions to handle proto5 result type +func processListResultsProto5(req list.ListRequest, stream iter.Seq[tfprotov5.ListResourceResult]) iter.Seq[tfprotov5.ListResourceResult] { + return func(push func(tfprotov5.ListResourceResult) bool) { + for result := range stream { + if !push(processListResultProto5(req, result)) { + return + } + } + } +} + +func processListResultProto5(req list.ListRequest, result tfprotov5.ListResourceResult) tfprotov5.ListResourceResult { + if len(result.Diagnostics) > 0 { + return result + } + + if result.Identity == nil { + return ListResultErrorProto5( + "Incomplete List Result", + "When listing resources, an implementation issue was found. "+ + "This is always a problem with the provider. Please report this to the provider developers.\n\n"+ + "The \"Identity\" field is nil.\n\n", + ) + } + + if req.IncludeResource { + // We could do an IsNull check here on the DynamicValue as well + if result.Resource == nil { + result.Diagnostics = []*tfprotov5.Diagnostic{ + { + Severity: 0, + Summary: "Incomplete List Result", + Detail: "When listing resources, an implementation issue was found. " + + "This is always a problem with the provider. Please report this to the provider developers.\n\n" + + "The \"IncludeResource\" field in the ListRequest is true, but the \"Resource\" field in the ListResult is nil.\n\n", + }, + } + } + } + + return result +} diff --git a/internal/fwserver/server_listresources.go b/internal/fwserver/server_listresources.go index 94760afb3..47f15cfc8 100644 --- a/internal/fwserver/server_listresources.go +++ b/internal/fwserver/server_listresources.go @@ -12,7 +12,6 @@ import ( "github.com/hashicorp/terraform-plugin-framework/internal/logging" "github.com/hashicorp/terraform-plugin-framework/list" "github.com/hashicorp/terraform-plugin-framework/provider" - "github.com/hashicorp/terraform-plugin-framework/resource" ) func (s *Server) ListResourceType(ctx context.Context, typeName string) (list.ListResource, diag.Diagnostics) { @@ -57,10 +56,10 @@ func (s *Server) ListResourceFuncs(ctx context.Context) (map[string]func() list. for _, listResourceFunc := range listResourceFuncSlice { listResource := listResourceFunc() - metadataReq := resource.MetadataRequest{ + metadataReq := list.MetadataRequest{ ProviderTypeName: providerTypeName, } - metadataResp := resource.MetadataResponse{} + metadataResp := list.MetadataResponse{} listResource.Metadata(ctx, metadataReq, &metadataResp) typeName := metadataResp.TypeName @@ -87,12 +86,17 @@ func (s *Server) ListResourceFuncs(ctx context.Context) (map[string]func() list. resourceFuncs, _ := s.ResourceFuncs(ctx) if _, ok := resourceFuncs[typeName]; !ok { - s.listResourceFuncsDiags.AddError( - "ListResource Type Defined without a Matching Managed Resource Type", - fmt.Sprintf("The %s ListResource type name was returned, but no matching managed Resource type was defined. ", typeName)+ - "This is always an issue with the provider and should be reported to the provider developers.", - ) - continue + + // With the ProtoV5 info in Metadata we can keep this validation + // TODO update error message + if metadataResp.ProtoV5Schema == nil || metadataResp.ProtoV5IdentitySchema == nil { + s.listResourceFuncsDiags.AddError( + "ListResource Type Defined without a Matching Managed Resource Type", + fmt.Sprintf("The %s ListResource type name was returned, but no matching managed Resource type was defined. ", typeName)+ + "This is always an issue with the provider and should be reported to the provider developers.", + ) + continue + } } s.listResourceFuncs[typeName] = listResourceFunc diff --git a/internal/proto5server/server_listresource.go b/internal/proto5server/server_listresource.go index 54bc34b37..7826b18b5 100644 --- a/internal/proto5server/server_listresource.go +++ b/internal/proto5server/server_listresource.go @@ -5,6 +5,7 @@ package proto5server import ( "context" + "github.com/hashicorp/terraform-plugin-framework/list" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/internal/fromproto5" @@ -47,6 +48,38 @@ func (s *Server) ListResource(ctx context.Context, protoReq *tfprotov5.ListResou return ListRequestErrorDiagnostics(ctx, allDiags...) } + // This breaks consistency with other RPCs but this allows us to switch the logic without an interceptor in mux + metadataResp := list.MetadataResponse{} + listResource.Metadata(ctx, list.MetadataRequest{}, &metadataResp) + + // There's validation in xxx that ensures both are set if either is provided so perhaps it's sufficient to only nil check Identity + if metadataResp.ProtoV5IdentitySchema != nil { + identitySchema, _ := fromproto5.IdentitySchema(ctx, metadataResp.ProtoV5IdentitySchema()) + resourceSchema, _ := fromproto5.ResourceSchema(ctx, metadataResp.ProtoV5Schema()) + + req := &fwserver.ListRequest{ + Config: config, + ListResource: listResource, + IncludeResource: protoReq.IncludeResource, + ResourceIdentitySchema: identitySchema, + ResourceSchema: resourceSchema, + } + + stream := &fwserver.ListResultsStream{} + s.FrameworkServer.ListResource(ctx, req, stream) + + protoStream.Results = func(push func(tfprotov5.ListResourceResult) bool) { + for result := range stream.ResultsProtoV5 { + // We pass the result along as-is since it's already in ProtoV5 format + if !push(result) { + return + } + } + } + + return protoStream, nil + } + resourceSchema, diags := s.FrameworkServer.ResourceSchema(ctx, protoReq.TypeName) allDiags.Append(diags...) if diags.HasError() { diff --git a/list/list_resource.go b/list/list_resource.go index 3dbdbee17..84d52d2c6 100644 --- a/list/list_resource.go +++ b/list/list_resource.go @@ -31,7 +31,10 @@ type ListResource interface { // The method signature is intended to be compatible with the Metadata // method signature in the Resource interface. One implementation of // Metadata can satisfy both interfaces. - Metadata(context.Context, resource.MetadataRequest, *resource.MetadataResponse) + + // Since we're passing additional schema information around in here we should use the + // MetadataRequest and MetadataResponse for list + Metadata(context.Context, MetadataRequest, *MetadataResponse) // ListResourceConfigSchema should return the schema for list blocks. ListResourceConfigSchema(context.Context, ListResourceSchemaRequest, *ListResourceSchemaResponse) diff --git a/list/metadata.go b/list/metadata.go index 8efe0b780..61d8a7107 100644 --- a/list/metadata.go +++ b/list/metadata.go @@ -3,6 +3,8 @@ package list +import "github.com/hashicorp/terraform-plugin-go/tfprotov5" + // MetadataRequest represents a request for the ListResource to return metadata, // such as its type name. An instance of this request struct is supplied as // an argument to the ListResource type Metadata method. @@ -21,4 +23,9 @@ type MetadataResponse struct { // TypeName should be the full list resource type, including the provider // type prefix and an underscore. For example, examplecloud_thing. TypeName string + + // Could this also live in the Schema instead? Should it? + // If it's here we can do validation + ProtoV5Schema func() *tfprotov5.Schema + ProtoV5IdentitySchema func() *tfprotov5.ResourceIdentitySchema } From a28690bc65eb5c238fd90cfd6f3875cad9e12511 Mon Sep 17 00:00:00 2001 From: Steph Date: Wed, 20 Aug 2025 10:27:28 +0200 Subject: [PATCH 02/12] update Set and SetAtPath to accept values of tftypes.Value --- internal/fwschemadata/data_set.go | 22 ++++++- internal/fwschemadata/data_set_at_path.go | 30 ++++++++- internal/fwserver/server_listresource.go | 50 +-------------- .../proto5server/server_getmetadata_test.go | 4 +- .../server_getproviderschema_test.go | 4 +- internal/proto5server/server_listresource.go | 61 ++++++------------- .../proto5server/server_listresource_test.go | 2 +- .../server_validatelistresourceconfig_test.go | 6 +- internal/testing/testprovider/listresource.go | 5 +- list/list_resource_test.go | 2 +- list/no_op_resource_test.go | 3 + 11 files changed, 84 insertions(+), 105 deletions(-) diff --git a/internal/fwschemadata/data_set.go b/internal/fwschemadata/data_set.go index b53604b31..4306481a0 100644 --- a/internal/fwschemadata/data_set.go +++ b/internal/fwschemadata/data_set.go @@ -6,15 +6,35 @@ package fwschemadata import ( "context" "fmt" + "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/internal/reflect" "github.com/hashicorp/terraform-plugin-framework/path" ) -// Set replaces the entire value. The value should be a struct whose fields +// Set replaces the entire value. The value can be a tftypes.Value or a struct whose fields // have one of the attr.Value types. Each field must have the tfsdk field tag. func (d *Data) Set(ctx context.Context, val any) diag.Diagnostics { + var diags diag.Diagnostics + + if v, ok := val.(tftypes.Value); ok { + objType := d.Schema.Type().TerraformType(ctx) + + if !objType.Equal(v.Type()) { + diags.AddError( + d.Description.Title()+" Write Error", + "An unexpected error was encountered trying to write the "+d.Description.String()+". This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + fmt.Sprintf("Error: Type mismatch between provided value and type of %s", d.Description.String()), + ) + return diags + + } + d.TerraformValue = v + + return diags + } + attrValue, diags := reflect.FromValue(ctx, d.Schema.Type(), val, path.Empty()) if diags.HasError() { diff --git a/internal/fwschemadata/data_set_at_path.go b/internal/fwschemadata/data_set_at_path.go index 211e75c1c..ea57cdd0b 100644 --- a/internal/fwschemadata/data_set_at_path.go +++ b/internal/fwschemadata/data_set_at_path.go @@ -28,9 +28,37 @@ import ( // Lists can only have the next element added according to the current length. func (d *Data) SetAtPath(ctx context.Context, path path.Path, val interface{}) diag.Diagnostics { var diags diag.Diagnostics - ctx = logging.FrameworkWithAttributePath(ctx, path.String()) + if v, ok := val.(tftypes.Value); ok { + atPath, err := d.Schema.AttributeAtPath(ctx, path) + if err != nil { + diags.AddAttributeError( + path, + d.Description.Title()+" Write Error", + "An unexpected error was encountered trying to write the "+d.Description.String()+". This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + fmt.Sprintf("Error: %s does not exist at path", path.String()), + ) + return diags + } + + attrType := atPath.GetType().TerraformType(ctx) + + if !attrType.Equal(v.Type()) { + diags.AddAttributeError( + path, + d.Description.Title()+" Write Error", + "An unexpected error was encountered trying to write the "+d.Description.String()+". This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + fmt.Sprintf("Error: Type of provided value does not match type of %s", path.String()), + ) + return diags + } + + d.TerraformValue = v + + return diags + } + tftypesPath, tftypesPathDiags := totftypes.AttributePath(ctx, path) diags.Append(tftypesPathDiags...) diff --git a/internal/fwserver/server_listresource.go b/internal/fwserver/server_listresource.go index 602b3ffd5..1b02e9982 100644 --- a/internal/fwserver/server_listresource.go +++ b/internal/fwserver/server_listresource.go @@ -177,7 +177,7 @@ func (s *Server) ListResource(ctx context.Context, fwReq *ListRequest, fwStream diagsStream.Results = list.NoListResults } - if stream.Results == nil || stream.Proto5Results == nil { + if stream.Results == nil { fwStream.Results = processListResults(req, list.NoListResults, diagsStream.Results) } @@ -185,11 +185,6 @@ func (s *Server) ListResource(ctx context.Context, fwReq *ListRequest, fwStream fwStream.Results = processListResults(req, stream.Results, diagsStream.Results) } - if stream.Proto5Results != nil { - // TODO merge with diagsStream if needed - fwStream.ResultsProtoV5 = processListResultsProto5(req, stream.Proto5Results) - } - return } @@ -239,46 +234,3 @@ func processListResult(req list.ListRequest, result list.ListResult) ListResult return ListResult(result) } - -// Duplicated functions to handle proto5 result type -func processListResultsProto5(req list.ListRequest, stream iter.Seq[tfprotov5.ListResourceResult]) iter.Seq[tfprotov5.ListResourceResult] { - return func(push func(tfprotov5.ListResourceResult) bool) { - for result := range stream { - if !push(processListResultProto5(req, result)) { - return - } - } - } -} - -func processListResultProto5(req list.ListRequest, result tfprotov5.ListResourceResult) tfprotov5.ListResourceResult { - if len(result.Diagnostics) > 0 { - return result - } - - if result.Identity == nil { - return ListResultErrorProto5( - "Incomplete List Result", - "When listing resources, an implementation issue was found. "+ - "This is always a problem with the provider. Please report this to the provider developers.\n\n"+ - "The \"Identity\" field is nil.\n\n", - ) - } - - if req.IncludeResource { - // We could do an IsNull check here on the DynamicValue as well - if result.Resource == nil { - result.Diagnostics = []*tfprotov5.Diagnostic{ - { - Severity: 0, - Summary: "Incomplete List Result", - Detail: "When listing resources, an implementation issue was found. " + - "This is always a problem with the provider. Please report this to the provider developers.\n\n" + - "The \"IncludeResource\" field in the ListRequest is true, but the \"Resource\" field in the ListResult is nil.\n\n", - }, - } - } - } - - return result -} diff --git a/internal/proto5server/server_getmetadata_test.go b/internal/proto5server/server_getmetadata_test.go index ef1dc7f78..62577ebe3 100644 --- a/internal/proto5server/server_getmetadata_test.go +++ b/internal/proto5server/server_getmetadata_test.go @@ -225,14 +225,14 @@ func TestServerGetMetadata(t *testing.T) { return []func() list.ListResource{ func() list.ListResource { return &testprovider.ListResource{ - MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { + MetadataMethod: func(_ context.Context, _ list.MetadataRequest, resp *list.MetadataResponse) { resp.TypeName = "test_list_resource1" }, } }, func() list.ListResource { return &testprovider.ListResource{ - MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { + MetadataMethod: func(_ context.Context, _ list.MetadataRequest, resp *list.MetadataResponse) { resp.TypeName = "test_list_resource2" }, } diff --git a/internal/proto5server/server_getproviderschema_test.go b/internal/proto5server/server_getproviderschema_test.go index 409e06e9e..b4d61375a 100644 --- a/internal/proto5server/server_getproviderschema_test.go +++ b/internal/proto5server/server_getproviderschema_test.go @@ -381,7 +381,7 @@ func TestServerGetProviderSchema(t *testing.T) { }, } }, - MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { + MetadataMethod: func(_ context.Context, _ list.MetadataRequest, resp *list.MetadataResponse) { resp.TypeName = "test_list_resource1" }, } @@ -397,7 +397,7 @@ func TestServerGetProviderSchema(t *testing.T) { }, } }, - MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { + MetadataMethod: func(_ context.Context, _ list.MetadataRequest, resp *list.MetadataResponse) { resp.TypeName = "test_list_resource2" }, } diff --git a/internal/proto5server/server_listresource.go b/internal/proto5server/server_listresource.go index 7826b18b5..21cf69d7e 100644 --- a/internal/proto5server/server_listresource.go +++ b/internal/proto5server/server_listresource.go @@ -52,54 +52,31 @@ func (s *Server) ListResource(ctx context.Context, protoReq *tfprotov5.ListResou metadataResp := list.MetadataResponse{} listResource.Metadata(ctx, list.MetadataRequest{}, &metadataResp) + req := &fwserver.ListRequest{ + Config: config, + ListResource: listResource, + IncludeResource: protoReq.IncludeResource, + Limit: protoReq.Limit, + } + // There's validation in xxx that ensures both are set if either is provided so perhaps it's sufficient to only nil check Identity if metadataResp.ProtoV5IdentitySchema != nil { - identitySchema, _ := fromproto5.IdentitySchema(ctx, metadataResp.ProtoV5IdentitySchema()) - resourceSchema, _ := fromproto5.ResourceSchema(ctx, metadataResp.ProtoV5Schema()) - - req := &fwserver.ListRequest{ - Config: config, - ListResource: listResource, - IncludeResource: protoReq.IncludeResource, - ResourceIdentitySchema: identitySchema, - ResourceSchema: resourceSchema, + req.ResourceSchema, _ = fromproto5.ResourceSchema(ctx, metadataResp.ProtoV5Schema()) + req.ResourceIdentitySchema, _ = fromproto5.IdentitySchema(ctx, metadataResp.ProtoV5IdentitySchema()) + } else { + req.ResourceSchema, diags = s.FrameworkServer.ResourceSchema(ctx, protoReq.TypeName) + allDiags.Append(diags...) + if diags.HasError() { + return ListRequestErrorDiagnostics(ctx, allDiags...) } - stream := &fwserver.ListResultsStream{} - s.FrameworkServer.ListResource(ctx, req, stream) - - protoStream.Results = func(push func(tfprotov5.ListResourceResult) bool) { - for result := range stream.ResultsProtoV5 { - // We pass the result along as-is since it's already in ProtoV5 format - if !push(result) { - return - } - } + req.ResourceIdentitySchema, diags = s.FrameworkServer.ResourceIdentitySchema(ctx, protoReq.TypeName) + allDiags.Append(diags...) + if diags.HasError() { + return ListRequestErrorDiagnostics(ctx, allDiags...) } - - return protoStream, nil - } - - resourceSchema, diags := s.FrameworkServer.ResourceSchema(ctx, protoReq.TypeName) - allDiags.Append(diags...) - if diags.HasError() { - return ListRequestErrorDiagnostics(ctx, allDiags...) - } - - identitySchema, diags := s.FrameworkServer.ResourceIdentitySchema(ctx, protoReq.TypeName) - allDiags.Append(diags...) - if diags.HasError() { - return ListRequestErrorDiagnostics(ctx, allDiags...) - } - - req := &fwserver.ListRequest{ - Config: config, - ListResource: listResource, - ResourceSchema: resourceSchema, - ResourceIdentitySchema: identitySchema, - IncludeResource: protoReq.IncludeResource, - Limit: protoReq.Limit, } + stream := &fwserver.ListResultsStream{} s.FrameworkServer.ListResource(ctx, req, stream) diff --git a/internal/proto5server/server_listresource_test.go b/internal/proto5server/server_listresource_test.go index b9aeb5a99..27aead0c7 100644 --- a/internal/proto5server/server_listresource_test.go +++ b/internal/proto5server/server_listresource_test.go @@ -118,7 +118,7 @@ func TestServerListResource(t *testing.T) { } resp.Results = slices.Values(results) }, - MetadataMethod: func(ctx context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { + MetadataMethod: func(ctx context.Context, req list.MetadataRequest, resp *list.MetadataResponse) { resp.TypeName = "test_resource" }, } diff --git a/internal/proto5server/server_validatelistresourceconfig_test.go b/internal/proto5server/server_validatelistresourceconfig_test.go index 1e25b7885..ceed14a50 100644 --- a/internal/proto5server/server_validatelistresourceconfig_test.go +++ b/internal/proto5server/server_validatelistresourceconfig_test.go @@ -58,7 +58,7 @@ func TestServerValidateListResourceConfig(t *testing.T) { return []func() list.ListResource{ func() list.ListResource { return &testprovider.ListResource{ - MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { + MetadataMethod: func(_ context.Context, _ list.MetadataRequest, resp *list.MetadataResponse) { resp.TypeName = "test_resource" }, ListResourceConfigSchemaMethod: func(_ context.Context, _ list.ListResourceSchemaRequest, resp *list.ListResourceSchemaResponse) {}, @@ -96,7 +96,7 @@ func TestServerValidateListResourceConfig(t *testing.T) { ListResourceConfigSchemaMethod: func(_ context.Context, _ list.ListResourceSchemaRequest, resp *list.ListResourceSchemaResponse) { resp.Schema = testSchema }, - MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { + MetadataMethod: func(_ context.Context, _ list.MetadataRequest, resp *list.MetadataResponse) { resp.TypeName = "test_resource" }, } @@ -135,7 +135,7 @@ func TestServerValidateListResourceConfig(t *testing.T) { ListResourceConfigSchemaMethod: func(_ context.Context, _ list.ListResourceSchemaRequest, resp *list.ListResourceSchemaResponse) { resp.Schema = testSchema }, - MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { + MetadataMethod: func(_ context.Context, _ list.MetadataRequest, resp *list.MetadataResponse) { resp.TypeName = "test_resource" }, }, diff --git a/internal/testing/testprovider/listresource.go b/internal/testing/testprovider/listresource.go index 857ea0d97..2907d95c3 100644 --- a/internal/testing/testprovider/listresource.go +++ b/internal/testing/testprovider/listresource.go @@ -7,7 +7,6 @@ import ( "context" "github.com/hashicorp/terraform-plugin-framework/list" - "github.com/hashicorp/terraform-plugin-framework/resource" ) var _ list.ListResource = &ListResource{} @@ -15,13 +14,13 @@ var _ list.ListResource = &ListResource{} // Declarative list.ListResource for unit testing. type ListResource struct { // ListResource interface methods - MetadataMethod func(context.Context, resource.MetadataRequest, *resource.MetadataResponse) + MetadataMethod func(context.Context, list.MetadataRequest, *list.MetadataResponse) ListResourceConfigSchemaMethod func(context.Context, list.ListResourceSchemaRequest, *list.ListResourceSchemaResponse) ListMethod func(context.Context, list.ListRequest, *list.ListResultsStream) } // Metadata satisfies the list.ListResource interface. -func (r *ListResource) Metadata(ctx context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { +func (r *ListResource) Metadata(ctx context.Context, req list.MetadataRequest, resp *list.MetadataResponse) { if r.MetadataMethod == nil { return } diff --git a/list/list_resource_test.go b/list/list_resource_test.go index 7426f9309..656f18417 100644 --- a/list/list_resource_test.go +++ b/list/list_resource_test.go @@ -26,7 +26,7 @@ type ComputeInstanceWithListResourceConfigValidators struct { func (c *ComputeInstanceResource) Configure(_ context.Context, _ resource.ConfigureRequest, _ *resource.ConfigureResponse) { } -func (c *ComputeInstanceResource) Metadata(_ context.Context, _ resource.MetadataRequest, _ *resource.MetadataResponse) { +func (c *ComputeInstanceResource) Metadata(_ context.Context, _ list.MetadataRequest, _ *list.MetadataResponse) { } func (c *ComputeInstanceWithValidateListResourceConfig) ValidateListResourceConfig(_ context.Context, _ list.ValidateConfigRequest, _ *list.ValidateConfigResponse) { diff --git a/list/no_op_resource_test.go b/list/no_op_resource_test.go index 18c0d18f1..4a7ffe0ba 100644 --- a/list/no_op_resource_test.go +++ b/list/no_op_resource_test.go @@ -25,3 +25,6 @@ func (*NoOpResource) Update(_ context.Context, _ resource.UpdateRequest, _ *reso func (*NoOpResource) Delete(_ context.Context, _ resource.DeleteRequest, _ *resource.DeleteResponse) { } + +func (*NoOpResource) Metadata(_ context.Context, _ resource.MetadataRequest, _ *resource.MetadataResponse) { +} From d2063a11cf29bb17c7c027e311c81334321f2645 Mon Sep 17 00:00:00 2001 From: Steph Date: Wed, 20 Aug 2025 11:24:17 +0200 Subject: [PATCH 03/12] move protov5 information into the resource metadataresponse --- internal/fwserver/server_listresources.go | 7 +++---- internal/proto5server/server_getmetadata_test.go | 4 ++-- internal/proto5server/server_getproviderschema_test.go | 4 ++-- internal/proto5server/server_listresource.go | 9 ++++----- internal/proto5server/server_listresource_test.go | 2 +- .../server_validatelistresourceconfig_test.go | 6 +++--- internal/testing/testprovider/listresource.go | 5 +++-- list/list_resource.go | 5 +---- list/list_resource_test.go | 2 +- list/no_op_resource_test.go | 3 --- resource/metadata.go | 5 +++++ 11 files changed, 25 insertions(+), 27 deletions(-) diff --git a/internal/fwserver/server_listresources.go b/internal/fwserver/server_listresources.go index 47f15cfc8..f9deabe47 100644 --- a/internal/fwserver/server_listresources.go +++ b/internal/fwserver/server_listresources.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/internal/logging" "github.com/hashicorp/terraform-plugin-framework/list" "github.com/hashicorp/terraform-plugin-framework/provider" + "github.com/hashicorp/terraform-plugin-framework/resource" ) func (s *Server) ListResourceType(ctx context.Context, typeName string) (list.ListResource, diag.Diagnostics) { @@ -56,10 +57,10 @@ func (s *Server) ListResourceFuncs(ctx context.Context) (map[string]func() list. for _, listResourceFunc := range listResourceFuncSlice { listResource := listResourceFunc() - metadataReq := list.MetadataRequest{ + metadataReq := resource.MetadataRequest{ ProviderTypeName: providerTypeName, } - metadataResp := list.MetadataResponse{} + metadataResp := resource.MetadataResponse{} listResource.Metadata(ctx, metadataReq, &metadataResp) typeName := metadataResp.TypeName @@ -86,8 +87,6 @@ func (s *Server) ListResourceFuncs(ctx context.Context) (map[string]func() list. resourceFuncs, _ := s.ResourceFuncs(ctx) if _, ok := resourceFuncs[typeName]; !ok { - - // With the ProtoV5 info in Metadata we can keep this validation // TODO update error message if metadataResp.ProtoV5Schema == nil || metadataResp.ProtoV5IdentitySchema == nil { s.listResourceFuncsDiags.AddError( diff --git a/internal/proto5server/server_getmetadata_test.go b/internal/proto5server/server_getmetadata_test.go index 62577ebe3..ef1dc7f78 100644 --- a/internal/proto5server/server_getmetadata_test.go +++ b/internal/proto5server/server_getmetadata_test.go @@ -225,14 +225,14 @@ func TestServerGetMetadata(t *testing.T) { return []func() list.ListResource{ func() list.ListResource { return &testprovider.ListResource{ - MetadataMethod: func(_ context.Context, _ list.MetadataRequest, resp *list.MetadataResponse) { + MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { resp.TypeName = "test_list_resource1" }, } }, func() list.ListResource { return &testprovider.ListResource{ - MetadataMethod: func(_ context.Context, _ list.MetadataRequest, resp *list.MetadataResponse) { + MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { resp.TypeName = "test_list_resource2" }, } diff --git a/internal/proto5server/server_getproviderschema_test.go b/internal/proto5server/server_getproviderschema_test.go index b4d61375a..409e06e9e 100644 --- a/internal/proto5server/server_getproviderschema_test.go +++ b/internal/proto5server/server_getproviderschema_test.go @@ -381,7 +381,7 @@ func TestServerGetProviderSchema(t *testing.T) { }, } }, - MetadataMethod: func(_ context.Context, _ list.MetadataRequest, resp *list.MetadataResponse) { + MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { resp.TypeName = "test_list_resource1" }, } @@ -397,7 +397,7 @@ func TestServerGetProviderSchema(t *testing.T) { }, } }, - MetadataMethod: func(_ context.Context, _ list.MetadataRequest, resp *list.MetadataResponse) { + MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { resp.TypeName = "test_list_resource2" }, } diff --git a/internal/proto5server/server_listresource.go b/internal/proto5server/server_listresource.go index 21cf69d7e..136069d71 100644 --- a/internal/proto5server/server_listresource.go +++ b/internal/proto5server/server_listresource.go @@ -5,12 +5,12 @@ package proto5server import ( "context" - "github.com/hashicorp/terraform-plugin-framework/list" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/internal/fromproto5" "github.com/hashicorp/terraform-plugin-framework/internal/fwserver" "github.com/hashicorp/terraform-plugin-framework/internal/toproto5" + "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-go/tfprotov5" ) @@ -48,9 +48,8 @@ func (s *Server) ListResource(ctx context.Context, protoReq *tfprotov5.ListResou return ListRequestErrorDiagnostics(ctx, allDiags...) } - // This breaks consistency with other RPCs but this allows us to switch the logic without an interceptor in mux - metadataResp := list.MetadataResponse{} - listResource.Metadata(ctx, list.MetadataRequest{}, &metadataResp) + metadataResp := resource.MetadataResponse{} + listResource.Metadata(ctx, resource.MetadataRequest{}, &metadataResp) req := &fwserver.ListRequest{ Config: config, @@ -76,7 +75,7 @@ func (s *Server) ListResource(ctx context.Context, protoReq *tfprotov5.ListResou return ListRequestErrorDiagnostics(ctx, allDiags...) } } - + stream := &fwserver.ListResultsStream{} s.FrameworkServer.ListResource(ctx, req, stream) diff --git a/internal/proto5server/server_listresource_test.go b/internal/proto5server/server_listresource_test.go index 27aead0c7..b9aeb5a99 100644 --- a/internal/proto5server/server_listresource_test.go +++ b/internal/proto5server/server_listresource_test.go @@ -118,7 +118,7 @@ func TestServerListResource(t *testing.T) { } resp.Results = slices.Values(results) }, - MetadataMethod: func(ctx context.Context, req list.MetadataRequest, resp *list.MetadataResponse) { + MetadataMethod: func(ctx context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { resp.TypeName = "test_resource" }, } diff --git a/internal/proto5server/server_validatelistresourceconfig_test.go b/internal/proto5server/server_validatelistresourceconfig_test.go index ceed14a50..1e25b7885 100644 --- a/internal/proto5server/server_validatelistresourceconfig_test.go +++ b/internal/proto5server/server_validatelistresourceconfig_test.go @@ -58,7 +58,7 @@ func TestServerValidateListResourceConfig(t *testing.T) { return []func() list.ListResource{ func() list.ListResource { return &testprovider.ListResource{ - MetadataMethod: func(_ context.Context, _ list.MetadataRequest, resp *list.MetadataResponse) { + MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { resp.TypeName = "test_resource" }, ListResourceConfigSchemaMethod: func(_ context.Context, _ list.ListResourceSchemaRequest, resp *list.ListResourceSchemaResponse) {}, @@ -96,7 +96,7 @@ func TestServerValidateListResourceConfig(t *testing.T) { ListResourceConfigSchemaMethod: func(_ context.Context, _ list.ListResourceSchemaRequest, resp *list.ListResourceSchemaResponse) { resp.Schema = testSchema }, - MetadataMethod: func(_ context.Context, _ list.MetadataRequest, resp *list.MetadataResponse) { + MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { resp.TypeName = "test_resource" }, } @@ -135,7 +135,7 @@ func TestServerValidateListResourceConfig(t *testing.T) { ListResourceConfigSchemaMethod: func(_ context.Context, _ list.ListResourceSchemaRequest, resp *list.ListResourceSchemaResponse) { resp.Schema = testSchema }, - MetadataMethod: func(_ context.Context, _ list.MetadataRequest, resp *list.MetadataResponse) { + MetadataMethod: func(_ context.Context, _ resource.MetadataRequest, resp *resource.MetadataResponse) { resp.TypeName = "test_resource" }, }, diff --git a/internal/testing/testprovider/listresource.go b/internal/testing/testprovider/listresource.go index 2907d95c3..857ea0d97 100644 --- a/internal/testing/testprovider/listresource.go +++ b/internal/testing/testprovider/listresource.go @@ -7,6 +7,7 @@ import ( "context" "github.com/hashicorp/terraform-plugin-framework/list" + "github.com/hashicorp/terraform-plugin-framework/resource" ) var _ list.ListResource = &ListResource{} @@ -14,13 +15,13 @@ var _ list.ListResource = &ListResource{} // Declarative list.ListResource for unit testing. type ListResource struct { // ListResource interface methods - MetadataMethod func(context.Context, list.MetadataRequest, *list.MetadataResponse) + MetadataMethod func(context.Context, resource.MetadataRequest, *resource.MetadataResponse) ListResourceConfigSchemaMethod func(context.Context, list.ListResourceSchemaRequest, *list.ListResourceSchemaResponse) ListMethod func(context.Context, list.ListRequest, *list.ListResultsStream) } // Metadata satisfies the list.ListResource interface. -func (r *ListResource) Metadata(ctx context.Context, req list.MetadataRequest, resp *list.MetadataResponse) { +func (r *ListResource) Metadata(ctx context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) { if r.MetadataMethod == nil { return } diff --git a/list/list_resource.go b/list/list_resource.go index 84d52d2c6..3dbdbee17 100644 --- a/list/list_resource.go +++ b/list/list_resource.go @@ -31,10 +31,7 @@ type ListResource interface { // The method signature is intended to be compatible with the Metadata // method signature in the Resource interface. One implementation of // Metadata can satisfy both interfaces. - - // Since we're passing additional schema information around in here we should use the - // MetadataRequest and MetadataResponse for list - Metadata(context.Context, MetadataRequest, *MetadataResponse) + Metadata(context.Context, resource.MetadataRequest, *resource.MetadataResponse) // ListResourceConfigSchema should return the schema for list blocks. ListResourceConfigSchema(context.Context, ListResourceSchemaRequest, *ListResourceSchemaResponse) diff --git a/list/list_resource_test.go b/list/list_resource_test.go index 656f18417..7426f9309 100644 --- a/list/list_resource_test.go +++ b/list/list_resource_test.go @@ -26,7 +26,7 @@ type ComputeInstanceWithListResourceConfigValidators struct { func (c *ComputeInstanceResource) Configure(_ context.Context, _ resource.ConfigureRequest, _ *resource.ConfigureResponse) { } -func (c *ComputeInstanceResource) Metadata(_ context.Context, _ list.MetadataRequest, _ *list.MetadataResponse) { +func (c *ComputeInstanceResource) Metadata(_ context.Context, _ resource.MetadataRequest, _ *resource.MetadataResponse) { } func (c *ComputeInstanceWithValidateListResourceConfig) ValidateListResourceConfig(_ context.Context, _ list.ValidateConfigRequest, _ *list.ValidateConfigResponse) { diff --git a/list/no_op_resource_test.go b/list/no_op_resource_test.go index 4a7ffe0ba..18c0d18f1 100644 --- a/list/no_op_resource_test.go +++ b/list/no_op_resource_test.go @@ -25,6 +25,3 @@ func (*NoOpResource) Update(_ context.Context, _ resource.UpdateRequest, _ *reso func (*NoOpResource) Delete(_ context.Context, _ resource.DeleteRequest, _ *resource.DeleteResponse) { } - -func (*NoOpResource) Metadata(_ context.Context, _ resource.MetadataRequest, _ *resource.MetadataResponse) { -} diff --git a/resource/metadata.go b/resource/metadata.go index 2977f3057..c26710a6e 100644 --- a/resource/metadata.go +++ b/resource/metadata.go @@ -3,6 +3,8 @@ package resource +import "github.com/hashicorp/terraform-plugin-go/tfprotov5" + // MetadataRequest represents a request for the Resource to return metadata, // such as its type name. An instance of this request struct is supplied as // an argument to the Resource type Metadata method. @@ -25,6 +27,9 @@ type MetadataResponse struct { // ResourceBehavior is used to control framework-specific logic when // interacting with this resource. ResourceBehavior ResourceBehavior + + ProtoV5Schema func() *tfprotov5.Schema + ProtoV5IdentitySchema func() *tfprotov5.ResourceIdentitySchema } // ResourceBehavior controls framework-specific logic when interacting From b33d28728f6a02671f250f9bd1aedbd1d3cf9552 Mon Sep 17 00:00:00 2001 From: Steph Date: Wed, 20 Aug 2025 11:57:41 +0200 Subject: [PATCH 04/12] add tests for updated Set and SetAtPath functionality --- internal/fwschemadata/data_set_at_path.go | 31 ++++++++++++----- .../fwschemadata/data_set_at_path_test.go | 33 +++++++++++++++++++ internal/fwschemadata/data_set_test.go | 33 +++++++++++++++++++ 3 files changed, 89 insertions(+), 8 deletions(-) diff --git a/internal/fwschemadata/data_set_at_path.go b/internal/fwschemadata/data_set_at_path.go index ea57cdd0b..a136118d5 100644 --- a/internal/fwschemadata/data_set_at_path.go +++ b/internal/fwschemadata/data_set_at_path.go @@ -31,30 +31,45 @@ func (d *Data) SetAtPath(ctx context.Context, path path.Path, val interface{}) d ctx = logging.FrameworkWithAttributePath(ctx, path.String()) if v, ok := val.(tftypes.Value); ok { - atPath, err := d.Schema.AttributeAtPath(ctx, path) - if err != nil { + atPath, atPathDiags := d.Schema.AttributeAtPath(ctx, path) + + diags.Append(atPathDiags...) + + if diags.HasError() { + return diags + } + + attrType := atPath.GetType().TerraformType(ctx) + + if !attrType.Equal(v.Type()) { diags.AddAttributeError( path, d.Description.Title()+" Write Error", "An unexpected error was encountered trying to write the "+d.Description.String()+". This is always an error in the provider. Please report the following to the provider developer:\n\n"+ - fmt.Sprintf("Error: %s does not exist at path", path.String()), + fmt.Sprintf("Error: Type of provided value does not match type of %s", path.String()), ) return diags } - attrType := atPath.GetType().TerraformType(ctx) + transformFunc, transformFuncDiags := d.SetAtPathTransformFunc(ctx, path, v, nil) + diags.Append(transformFuncDiags...) - if !attrType.Equal(v.Type()) { + if diags.HasError() { + return diags + } + + tfVal, err := tftypes.Transform(d.TerraformValue, transformFunc) + if err != nil { diags.AddAttributeError( path, d.Description.Title()+" Write Error", - "An unexpected error was encountered trying to write the "+d.Description.String()+". This is always an error in the provider. Please report the following to the provider developer:\n\n"+ - fmt.Sprintf("Error: Type of provided value does not match type of %s", path.String()), + "An unexpected error was encountered trying to write an attribute to the "+d.Description.String()+". This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "Error: Cannot transform data: "+err.Error(), ) return diags } - d.TerraformValue = v + d.TerraformValue = tfVal return diags } diff --git a/internal/fwschemadata/data_set_at_path_test.go b/internal/fwschemadata/data_set_at_path_test.go index 95a41fee9..6cf40dbb6 100644 --- a/internal/fwschemadata/data_set_at_path_test.go +++ b/internal/fwschemadata/data_set_at_path_test.go @@ -2924,6 +2924,39 @@ func TestDataSetAtPath(t *testing.T) { "other": tftypes.NewValue(tftypes.DynamicPseudoType, nil), }), }, + "write-tftypes-value": { + data: fwschemadata.Data{ + TerraformValue: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "test": tftypes.String, + "other": tftypes.String, + }, + }, nil), + Schema: testschema.Schema{ + Attributes: map[string]fwschema.Attribute{ + "test": testschema.Attribute{ + Type: types.StringType, + Required: true, + }, + "other": testschema.Attribute{ + Type: types.StringType, + Required: true, + }, + }, + }, + }, + path: path.Root("test"), + val: tftypes.NewValue(tftypes.String, "newvalue"), + expected: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "test": tftypes.String, + "other": tftypes.String, + }, + }, map[string]tftypes.Value{ + "test": tftypes.NewValue(tftypes.String, "newvalue"), + "other": tftypes.NewValue(tftypes.String, nil), + }), + }, "AttrTypeWithValidateError": { data: fwschemadata.Data{ TerraformValue: tftypes.NewValue(tftypes.Object{ diff --git a/internal/fwschemadata/data_set_test.go b/internal/fwschemadata/data_set_test.go index 995923187..412fd1d58 100644 --- a/internal/fwschemadata/data_set_test.go +++ b/internal/fwschemadata/data_set_test.go @@ -139,6 +139,39 @@ func TestDataSet(t *testing.T) { ), }), }, + "write-tftypes-values": { + data: fwschemadata.Data{ + TerraformValue: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + }, + }, map[string]tftypes.Value{ + "name": tftypes.NewValue(tftypes.String, "oldvalue"), + }), + Schema: testschema.Schema{ + Attributes: map[string]fwschema.Attribute{ + "name": testschema.Attribute{ + Type: types.StringType, + Required: true, + }, + }, + }, + }, + val: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + }, + }, map[string]tftypes.Value{ + "name": tftypes.NewValue(tftypes.String, "newvalue"), + }), + expected: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "name": tftypes.String, + }, + }, map[string]tftypes.Value{ + "name": tftypes.NewValue(tftypes.String, "newvalue"), + }), + }, "overwrite": { data: fwschemadata.Data{ TerraformValue: tftypes.Value{}, From fd1fa4f0d62aa7d5a4ae8a09fd47f3b5a8f2d0e3 Mon Sep 17 00:00:00 2001 From: Steph Date: Wed, 20 Aug 2025 12:17:39 +0200 Subject: [PATCH 05/12] remove redundant return statement --- internal/fwserver/server_listresource.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/fwserver/server_listresource.go b/internal/fwserver/server_listresource.go index 1b02e9982..88397bf2a 100644 --- a/internal/fwserver/server_listresource.go +++ b/internal/fwserver/server_listresource.go @@ -124,7 +124,7 @@ func (s *Server) ListResource(ctx context.Context, fwReq *ListRequest, fwStream // ListResourceConfigureData isn't populated in the ConfigureProvider RPC // We can use ResourceConfigureData here for now or populate ListResourceConfigureData configureReq := resource.ConfigureRequest{ - ProviderData: s.ResourceConfigureData, + ProviderData: s.ListResourceConfigureData, } configureResp := resource.ConfigureResponse{} @@ -184,8 +184,6 @@ func (s *Server) ListResource(ctx context.Context, fwReq *ListRequest, fwStream if stream.Results != nil { fwStream.Results = processListResults(req, stream.Results, diagsStream.Results) } - - return } func processListResults(req list.ListRequest, streams ...iter.Seq[list.ListResult]) iter.Seq[ListResult] { From 29382ad21d4a6e27bee8cee3cd09c02400735a62 Mon Sep 17 00:00:00 2001 From: Steph Date: Wed, 20 Aug 2025 12:42:43 +0200 Subject: [PATCH 06/12] further clean up --- internal/fwschemadata/data_set.go | 2 +- internal/fwserver/server_getmetadata_test.go | 1 + internal/fwserver/server_listresource.go | 17 ----------------- internal/fwserver/server_listresources.go | 2 +- internal/proto5server/server_listresource.go | 2 +- list/metadata.go | 7 ------- resource/metadata.go | 9 ++++++++- 7 files changed, 12 insertions(+), 28 deletions(-) diff --git a/internal/fwschemadata/data_set.go b/internal/fwschemadata/data_set.go index 4306481a0..a80627c9d 100644 --- a/internal/fwschemadata/data_set.go +++ b/internal/fwschemadata/data_set.go @@ -6,11 +6,11 @@ package fwschemadata import ( "context" "fmt" - "github.com/hashicorp/terraform-plugin-go/tftypes" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/internal/reflect" "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-go/tftypes" ) // Set replaces the entire value. The value can be a tftypes.Value or a struct whose fields diff --git a/internal/fwserver/server_getmetadata_test.go b/internal/fwserver/server_getmetadata_test.go index 96aebfc22..25f601082 100644 --- a/internal/fwserver/server_getmetadata_test.go +++ b/internal/fwserver/server_getmetadata_test.go @@ -839,6 +839,7 @@ func TestServerGetMetadata(t *testing.T) { diag.NewErrorDiagnostic( "ListResource Type Defined without a Matching Managed Resource Type", "The test_resource_1 ListResource type name was returned, but no matching managed Resource type was defined. "+ + "If the matching managed Resource type is a legacy resource, ProtoV5Schema and ProtoV5IdentitySchema must be specified in the Metadata method. "+ "This is always an issue with the provider and should be reported to the provider developers.", ), }, diff --git a/internal/fwserver/server_listresource.go b/internal/fwserver/server_listresource.go index 88397bf2a..66393f654 100644 --- a/internal/fwserver/server_listresource.go +++ b/internal/fwserver/server_listresource.go @@ -13,7 +13,6 @@ import ( "github.com/hashicorp/terraform-plugin-framework/list" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/tfsdk" - "github.com/hashicorp/terraform-plugin-go/tfprotov5" "github.com/hashicorp/terraform-plugin-go/tftypes" ) @@ -50,8 +49,6 @@ type ListResultsStream struct { // Results is a function that emits [ListResult] values via its push // function argument. Results iter.Seq[ListResult] - - ResultsProtoV5 iter.Seq[tfprotov5.ListResourceResult] } func ListResultError(summary string, detail string) ListResult { @@ -62,18 +59,6 @@ func ListResultError(summary string, detail string) ListResult { } } -func ListResultErrorProto5(summary string, detail string) tfprotov5.ListResourceResult { - return tfprotov5.ListResourceResult{ - Diagnostics: []*tfprotov5.Diagnostic{ - { - Severity: 1, - Summary: summary, - Detail: detail, - }, - }, - } -} - // ListResult represents a listed managed resource instance. type ListResult struct { // Identity is the identity of the managed resource instance. A nil value @@ -121,8 +106,6 @@ func (s *Server) ListResource(ctx context.Context, fwReq *ListRequest, fwStream if listResourceWithConfigure, ok := listResource.(list.ListResourceWithConfigure); ok { logging.FrameworkTrace(ctx, "ListResource implements ListResourceWithConfigure") - // ListResourceConfigureData isn't populated in the ConfigureProvider RPC - // We can use ResourceConfigureData here for now or populate ListResourceConfigureData configureReq := resource.ConfigureRequest{ ProviderData: s.ListResourceConfigureData, } diff --git a/internal/fwserver/server_listresources.go b/internal/fwserver/server_listresources.go index f9deabe47..164c32620 100644 --- a/internal/fwserver/server_listresources.go +++ b/internal/fwserver/server_listresources.go @@ -87,11 +87,11 @@ func (s *Server) ListResourceFuncs(ctx context.Context) (map[string]func() list. resourceFuncs, _ := s.ResourceFuncs(ctx) if _, ok := resourceFuncs[typeName]; !ok { - // TODO update error message if metadataResp.ProtoV5Schema == nil || metadataResp.ProtoV5IdentitySchema == nil { s.listResourceFuncsDiags.AddError( "ListResource Type Defined without a Matching Managed Resource Type", fmt.Sprintf("The %s ListResource type name was returned, but no matching managed Resource type was defined. ", typeName)+ + "If the matching managed Resource type is a legacy resource, ProtoV5Schema and ProtoV5IdentitySchema must be specified in the Metadata method. "+ "This is always an issue with the provider and should be reported to the provider developers.", ) continue diff --git a/internal/proto5server/server_listresource.go b/internal/proto5server/server_listresource.go index 136069d71..91794f0bf 100644 --- a/internal/proto5server/server_listresource.go +++ b/internal/proto5server/server_listresource.go @@ -58,7 +58,7 @@ func (s *Server) ListResource(ctx context.Context, protoReq *tfprotov5.ListResou Limit: protoReq.Limit, } - // There's validation in xxx that ensures both are set if either is provided so perhaps it's sufficient to only nil check Identity + // There's validation in ListResources that ensures both are set if either is provided so it should be sufficient to only nil check Identity if metadataResp.ProtoV5IdentitySchema != nil { req.ResourceSchema, _ = fromproto5.ResourceSchema(ctx, metadataResp.ProtoV5Schema()) req.ResourceIdentitySchema, _ = fromproto5.IdentitySchema(ctx, metadataResp.ProtoV5IdentitySchema()) diff --git a/list/metadata.go b/list/metadata.go index 61d8a7107..8efe0b780 100644 --- a/list/metadata.go +++ b/list/metadata.go @@ -3,8 +3,6 @@ package list -import "github.com/hashicorp/terraform-plugin-go/tfprotov5" - // MetadataRequest represents a request for the ListResource to return metadata, // such as its type name. An instance of this request struct is supplied as // an argument to the ListResource type Metadata method. @@ -23,9 +21,4 @@ type MetadataResponse struct { // TypeName should be the full list resource type, including the provider // type prefix and an underscore. For example, examplecloud_thing. TypeName string - - // Could this also live in the Schema instead? Should it? - // If it's here we can do validation - ProtoV5Schema func() *tfprotov5.Schema - ProtoV5IdentitySchema func() *tfprotov5.ResourceIdentitySchema } diff --git a/resource/metadata.go b/resource/metadata.go index c26710a6e..75572ef8a 100644 --- a/resource/metadata.go +++ b/resource/metadata.go @@ -28,8 +28,15 @@ type MetadataResponse struct { // interacting with this resource. ResourceBehavior ResourceBehavior - ProtoV5Schema func() *tfprotov5.Schema + // ProtoV5IdentitySchema is the ProtoV5 representation of the resource identity + // schema. This should only be supplied if framework functionality is being used + // with a legacy resource. Currently, this only applies to list. ProtoV5IdentitySchema func() *tfprotov5.ResourceIdentitySchema + + // ProtoV5Schema is the ProtoV5 representation of the resource schema + // This should only be supplied if framework functionality is being used + // with a legacy resource. Currently, this only applies to list. + ProtoV5Schema func() *tfprotov5.Schema } // ResourceBehavior controls framework-specific logic when interacting From 733d992fd85186a68d1334060c1fc558756b7c24 Mon Sep 17 00:00:00 2001 From: Steph Date: Wed, 20 Aug 2025 17:22:35 +0200 Subject: [PATCH 07/12] add a schemas method for list --- internal/fwserver/server_listresources.go | 7 +++++- internal/proto5server/server_listresource.go | 16 +++++++------ list/list_resource.go | 25 ++++++++++++++++++++ resource/metadata.go | 12 ---------- 4 files changed, 40 insertions(+), 20 deletions(-) diff --git a/internal/fwserver/server_listresources.go b/internal/fwserver/server_listresources.go index 164c32620..60f2ad359 100644 --- a/internal/fwserver/server_listresources.go +++ b/internal/fwserver/server_listresources.go @@ -85,9 +85,14 @@ func (s *Server) ListResourceFuncs(ctx context.Context) (map[string]func() list. continue } + schemasResp := list.SchemaResponse{} + if listResourceWithSchemas, ok := listResource.(list.ListResourceWithProtoSchemas); ok { + listResourceWithSchemas.Schemas(ctx, &schemasResp) + } + resourceFuncs, _ := s.ResourceFuncs(ctx) if _, ok := resourceFuncs[typeName]; !ok { - if metadataResp.ProtoV5Schema == nil || metadataResp.ProtoV5IdentitySchema == nil { + if schemasResp.ProtoV5Schema == nil || schemasResp.ProtoV5IdentitySchema == nil { s.listResourceFuncsDiags.AddError( "ListResource Type Defined without a Matching Managed Resource Type", fmt.Sprintf("The %s ListResource type name was returned, but no matching managed Resource type was defined. ", typeName)+ diff --git a/internal/proto5server/server_listresource.go b/internal/proto5server/server_listresource.go index 91794f0bf..1c0e1c730 100644 --- a/internal/proto5server/server_listresource.go +++ b/internal/proto5server/server_listresource.go @@ -10,7 +10,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/internal/fromproto5" "github.com/hashicorp/terraform-plugin-framework/internal/fwserver" "github.com/hashicorp/terraform-plugin-framework/internal/toproto5" - "github.com/hashicorp/terraform-plugin-framework/resource" + "github.com/hashicorp/terraform-plugin-framework/list" "github.com/hashicorp/terraform-plugin-go/tfprotov5" ) @@ -48,9 +48,6 @@ func (s *Server) ListResource(ctx context.Context, protoReq *tfprotov5.ListResou return ListRequestErrorDiagnostics(ctx, allDiags...) } - metadataResp := resource.MetadataResponse{} - listResource.Metadata(ctx, resource.MetadataRequest{}, &metadataResp) - req := &fwserver.ListRequest{ Config: config, ListResource: listResource, @@ -58,10 +55,15 @@ func (s *Server) ListResource(ctx context.Context, protoReq *tfprotov5.ListResou Limit: protoReq.Limit, } + schemaResp := list.SchemaResponse{} + if listResourceWithProtoSchemas, ok := listResource.(list.ListResourceWithProtoSchemas); ok { + listResourceWithProtoSchemas.Schemas(ctx, &schemaResp) + } + // There's validation in ListResources that ensures both are set if either is provided so it should be sufficient to only nil check Identity - if metadataResp.ProtoV5IdentitySchema != nil { - req.ResourceSchema, _ = fromproto5.ResourceSchema(ctx, metadataResp.ProtoV5Schema()) - req.ResourceIdentitySchema, _ = fromproto5.IdentitySchema(ctx, metadataResp.ProtoV5IdentitySchema()) + if schemaResp.ProtoV5IdentitySchema != nil { + req.ResourceSchema, _ = fromproto5.ResourceSchema(ctx, schemaResp.ProtoV5Schema()) + req.ResourceIdentitySchema, _ = fromproto5.IdentitySchema(ctx, schemaResp.ProtoV5IdentitySchema()) } else { req.ResourceSchema, diags = s.FrameworkServer.ResourceSchema(ctx, protoReq.TypeName) allDiags.Append(diags...) diff --git a/list/list_resource.go b/list/list_resource.go index 3dbdbee17..17324bc7a 100644 --- a/list/list_resource.go +++ b/list/list_resource.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/internal/fwschema" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/tfsdk" + "github.com/hashicorp/terraform-plugin-go/tfprotov5" "github.com/hashicorp/terraform-plugin-go/tftypes" ) @@ -41,6 +42,16 @@ type ListResource interface { List(context.Context, ListRequest, *ListResultsStream) } +// ListResourceWithProtoSchemas is an interface type that extends ListResource to include a method +// which allows provider developers to supply the ProtoV5 representations of resource and resource identity +// schemas. This is necessary if list functionality is being used with a legacy resource. +type ListResourceWithProtoSchemas interface { + ListResource + + // Schemas is called to provide the ProtoV5 representations of the resource and resource identity schemas. + Schemas(context.Context, *SchemaResponse) +} + // ListResourceWithConfigure is an interface type that extends ListResource to include a method // which the framework will automatically call so provider developers have the // opportunity to setup any necessary provider-level data or clients. @@ -182,6 +193,20 @@ type ListResult struct { Diagnostics diag.Diagnostics } +// SchemaResponse represents a response that is populated by the Schemas method +// and is used to pass along the ProtoV5 representations of the resource and resource identity schemas. +type SchemaResponse struct { + // ProtoV5IdentitySchema is the ProtoV5 representation of the resource identity + // schema. This should only be supplied if framework functionality is being used + // with a legacy resource. Currently, this only applies to list. + ProtoV5IdentitySchema func() *tfprotov5.ResourceIdentitySchema + + // ProtoV5Schema is the ProtoV5 representation of the resource schema + // This should only be supplied if framework functionality is being used + // with a legacy resource. Currently, this only applies to list. + ProtoV5Schema func() *tfprotov5.Schema +} + // ValidateConfigRequest represents a request to validate the configuration of // a list resource. An instance of this request struct is supplied as an // argument to the [ListResourceWithValidateConfig.ValidateListResourceConfig] diff --git a/resource/metadata.go b/resource/metadata.go index 75572ef8a..2977f3057 100644 --- a/resource/metadata.go +++ b/resource/metadata.go @@ -3,8 +3,6 @@ package resource -import "github.com/hashicorp/terraform-plugin-go/tfprotov5" - // MetadataRequest represents a request for the Resource to return metadata, // such as its type name. An instance of this request struct is supplied as // an argument to the Resource type Metadata method. @@ -27,16 +25,6 @@ type MetadataResponse struct { // ResourceBehavior is used to control framework-specific logic when // interacting with this resource. ResourceBehavior ResourceBehavior - - // ProtoV5IdentitySchema is the ProtoV5 representation of the resource identity - // schema. This should only be supplied if framework functionality is being used - // with a legacy resource. Currently, this only applies to list. - ProtoV5IdentitySchema func() *tfprotov5.ResourceIdentitySchema - - // ProtoV5Schema is the ProtoV5 representation of the resource schema - // This should only be supplied if framework functionality is being used - // with a legacy resource. Currently, this only applies to list. - ProtoV5Schema func() *tfprotov5.Schema } // ResourceBehavior controls framework-specific logic when interacting From 75250ccc5469d8b19ef69020593b9a47541e9f6d Mon Sep 17 00:00:00 2001 From: Steph Date: Mon, 25 Aug 2025 15:40:49 +0200 Subject: [PATCH 08/12] return expected and received types in diags and add tests for failure states --- internal/fwschemadata/data_set.go | 2 +- internal/fwschemadata/data_set_at_path.go | 2 +- .../fwschemadata/data_set_at_path_test.go | 24 +++++++++++++ internal/fwschemadata/data_set_test.go | 34 +++++++++++++++---- 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/internal/fwschemadata/data_set.go b/internal/fwschemadata/data_set.go index a80627c9d..0fbdccdb8 100644 --- a/internal/fwschemadata/data_set.go +++ b/internal/fwschemadata/data_set.go @@ -25,7 +25,7 @@ func (d *Data) Set(ctx context.Context, val any) diag.Diagnostics { diags.AddError( d.Description.Title()+" Write Error", "An unexpected error was encountered trying to write the "+d.Description.String()+". This is always an error in the provider. Please report the following to the provider developer:\n\n"+ - fmt.Sprintf("Error: Type mismatch between provided value and type of %s", d.Description.String()), + fmt.Sprintf("Error: Type mismatch between provided value and type of %s, expected %+v, got %+v", d.Description.String(), objType.String(), v.Type().String()), ) return diags diff --git a/internal/fwschemadata/data_set_at_path.go b/internal/fwschemadata/data_set_at_path.go index a136118d5..0fdf47e98 100644 --- a/internal/fwschemadata/data_set_at_path.go +++ b/internal/fwschemadata/data_set_at_path.go @@ -46,7 +46,7 @@ func (d *Data) SetAtPath(ctx context.Context, path path.Path, val interface{}) d path, d.Description.Title()+" Write Error", "An unexpected error was encountered trying to write the "+d.Description.String()+". This is always an error in the provider. Please report the following to the provider developer:\n\n"+ - fmt.Sprintf("Error: Type of provided value does not match type of %s", path.String()), + fmt.Sprintf("Error: Type of provided value does not match type of %q, expected %s, got %s", path.String(), attrType.String(), v.Type().String()), ) return diags } diff --git a/internal/fwschemadata/data_set_at_path_test.go b/internal/fwschemadata/data_set_at_path_test.go index 6cf40dbb6..ca257f5da 100644 --- a/internal/fwschemadata/data_set_at_path_test.go +++ b/internal/fwschemadata/data_set_at_path_test.go @@ -2957,6 +2957,30 @@ func TestDataSetAtPath(t *testing.T) { "other": tftypes.NewValue(tftypes.String, nil), }), }, + "write-tftypes-value-MismatchedTypeError": { + data: fwschemadata.Data{ + TerraformValue: tftypes.Value{}, + Schema: testschema.Schema{ + Attributes: map[string]fwschema.Attribute{ + "test": testschema.Attribute{ + Type: types.StringType, + Required: true, + }, + "other": testschema.Attribute{ + Type: types.StringType, + Required: true, + }, + }, + }, + }, + path: path.Root("test"), + val: tftypes.NewValue(tftypes.Bool, false), + expected: tftypes.Value{}, + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic(path.Root("test"), "Data Write Error", "An unexpected error was encountered trying to write the data. This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "Error: Type of provided value does not match type of \"test\", expected tftypes.String, got tftypes.Bool"), + }, + }, "AttrTypeWithValidateError": { data: fwschemadata.Data{ TerraformValue: tftypes.NewValue(tftypes.Object{ diff --git a/internal/fwschemadata/data_set_test.go b/internal/fwschemadata/data_set_test.go index 412fd1d58..64998d39f 100644 --- a/internal/fwschemadata/data_set_test.go +++ b/internal/fwschemadata/data_set_test.go @@ -141,13 +141,7 @@ func TestDataSet(t *testing.T) { }, "write-tftypes-values": { data: fwschemadata.Data{ - TerraformValue: tftypes.NewValue(tftypes.Object{ - AttributeTypes: map[string]tftypes.Type{ - "name": tftypes.String, - }, - }, map[string]tftypes.Value{ - "name": tftypes.NewValue(tftypes.String, "oldvalue"), - }), + TerraformValue: tftypes.Value{}, Schema: testschema.Schema{ Attributes: map[string]fwschema.Attribute{ "name": testschema.Attribute{ @@ -172,6 +166,31 @@ func TestDataSet(t *testing.T) { "name": tftypes.NewValue(tftypes.String, "newvalue"), }), }, + "write-tftypes-values-MismatchedTypeError": { + data: fwschemadata.Data{ + TerraformValue: tftypes.Value{}, + Schema: testschema.Schema{ + Attributes: map[string]fwschema.Attribute{ + "name": testschema.Attribute{ + Type: types.StringType, + Required: true, + }, + }, + }, + }, + val: tftypes.NewValue(tftypes.Object{ + AttributeTypes: map[string]tftypes.Type{ + "not_name": tftypes.String, + }, + }, map[string]tftypes.Value{ + "not_name": tftypes.NewValue(tftypes.String, "newvalue"), + }), + expected: tftypes.Value{}, + expectedDiags: diag.Diagnostics{ + diag.NewErrorDiagnostic("Data Write Error", "An unexpected error was encountered trying to write the data. This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "Error: Type mismatch between provided value and type of data, expected tftypes.Object[\"name\":tftypes.String], got tftypes.Object[\"not_name\":tftypes.String]"), + }, + }, "overwrite": { data: fwschemadata.Data{ TerraformValue: tftypes.Value{}, @@ -196,6 +215,7 @@ func TestDataSet(t *testing.T) { }, map[string]tftypes.Value{ "name": tftypes.NewValue(tftypes.String, "newvalue"), }), + expectedDiags: diag.Diagnostics{}, }, "overwrite-dynamic": { data: fwschemadata.Data{ From b07a4ee87d15a6d2e73b08703f9c7da850760ac2 Mon Sep 17 00:00:00 2001 From: Steph Date: Tue, 26 Aug 2025 10:38:07 +0200 Subject: [PATCH 09/12] address review comments --- internal/fwserver/server_listresource.go | 6 ++-- internal/fwserver/server_listresource_test.go | 28 +++++++++++++++++++ internal/fwserver/server_listresources.go | 6 ++-- internal/proto5server/server_listresource.go | 22 ++++++++++++--- list/list_resource.go | 17 +++++++---- 5 files changed, 62 insertions(+), 17 deletions(-) diff --git a/internal/fwserver/server_listresource.go b/internal/fwserver/server_listresource.go index 66393f654..828382338 100644 --- a/internal/fwserver/server_listresource.go +++ b/internal/fwserver/server_listresource.go @@ -161,12 +161,10 @@ func (s *Server) ListResource(ctx context.Context, fwReq *ListRequest, fwStream } if stream.Results == nil { - fwStream.Results = processListResults(req, list.NoListResults, diagsStream.Results) + stream.Results = list.NoListResults } - if stream.Results != nil { - fwStream.Results = processListResults(req, stream.Results, diagsStream.Results) - } + fwStream.Results = processListResults(req, stream.Results, diagsStream.Results) } func processListResults(req list.ListRequest, streams ...iter.Seq[list.ListResult]) iter.Seq[ListResult] { diff --git a/internal/fwserver/server_listresource_test.go b/internal/fwserver/server_listresource_test.go index a4aeba813..5c4315b94 100644 --- a/internal/fwserver/server_listresource_test.go +++ b/internal/fwserver/server_listresource_test.go @@ -184,6 +184,34 @@ func TestServerListResource(t *testing.T) { expectedStreamEvents: []fwserver.ListResult{}, expectedError: "config cannot be nil", }, + "zero-results-with-warning-diagnostic": { + server: &fwserver.Server{ + Provider: &testprovider.Provider{}, + }, + request: &fwserver.ListRequest{ + Config: &tfsdk.Config{}, + ListResource: &testprovider.ListResourceWithConfigure{ + ConfigureMethod: func(ctx context.Context, req resource.ConfigureRequest, resp *resource.ConfigureResponse) { + resp.Diagnostics.AddWarning("Test Warning", "This is a test warning diagnostic") + }, + ListResource: &testprovider.ListResource{ + ListMethod: func(ctx context.Context, req list.ListRequest, resp *list.ListResultsStream) { + resp.Results = list.NoListResults + }, + }, + }, + }, + expectedStreamEvents: []fwserver.ListResult{ + { + Identity: nil, + Resource: nil, + DisplayName: "", + Diagnostics: diag.Diagnostics{ + diag.NewWarningDiagnostic("Test Warning", "This is a test warning diagnostic"), + }, + }, + }, + }, "listresource-configure-data": { server: &fwserver.Server{ ListResourceConfigureData: "test-provider-configure-value", diff --git a/internal/fwserver/server_listresources.go b/internal/fwserver/server_listresources.go index 60f2ad359..dc3577d86 100644 --- a/internal/fwserver/server_listresources.go +++ b/internal/fwserver/server_listresources.go @@ -86,8 +86,8 @@ func (s *Server) ListResourceFuncs(ctx context.Context) (map[string]func() list. } schemasResp := list.SchemaResponse{} - if listResourceWithSchemas, ok := listResource.(list.ListResourceWithProtoSchemas); ok { - listResourceWithSchemas.Schemas(ctx, &schemasResp) + if listResourceWithSchemas, ok := listResource.(list.ListResourceWithRawV5Schemas); ok { + listResourceWithSchemas.RawV5Schemas(ctx, list.SchemaRequest{}, &schemasResp) } resourceFuncs, _ := s.ResourceFuncs(ctx) @@ -96,7 +96,7 @@ func (s *Server) ListResourceFuncs(ctx context.Context) (map[string]func() list. s.listResourceFuncsDiags.AddError( "ListResource Type Defined without a Matching Managed Resource Type", fmt.Sprintf("The %s ListResource type name was returned, but no matching managed Resource type was defined. ", typeName)+ - "If the matching managed Resource type is a legacy resource, ProtoV5Schema and ProtoV5IdentitySchema must be specified in the Metadata method. "+ + "If the matching managed Resource type is a legacy resource, ProtoV5Schema and ProtoV5IdentitySchema must be specified in the RawV5Schemas method. "+ "This is always an issue with the provider and should be reported to the provider developers.", ) continue diff --git a/internal/proto5server/server_listresource.go b/internal/proto5server/server_listresource.go index 1c0e1c730..99a168a6d 100644 --- a/internal/proto5server/server_listresource.go +++ b/internal/proto5server/server_listresource.go @@ -56,14 +56,28 @@ func (s *Server) ListResource(ctx context.Context, protoReq *tfprotov5.ListResou } schemaResp := list.SchemaResponse{} - if listResourceWithProtoSchemas, ok := listResource.(list.ListResourceWithProtoSchemas); ok { - listResourceWithProtoSchemas.Schemas(ctx, &schemaResp) + if listResourceWithProtoSchemas, ok := listResource.(list.ListResourceWithRawV5Schemas); ok { + listResourceWithProtoSchemas.RawV5Schemas(ctx, list.SchemaRequest{}, &schemaResp) } // There's validation in ListResources that ensures both are set if either is provided so it should be sufficient to only nil check Identity if schemaResp.ProtoV5IdentitySchema != nil { - req.ResourceSchema, _ = fromproto5.ResourceSchema(ctx, schemaResp.ProtoV5Schema()) - req.ResourceIdentitySchema, _ = fromproto5.IdentitySchema(ctx, schemaResp.ProtoV5IdentitySchema()) + var err error + + req.ResourceSchema, err = fromproto5.ResourceSchema(ctx, schemaResp.ProtoV5Schema) + if err != nil { + diags.AddError("Converting Resource Schema", err.Error()) + allDiags.Append(diags...) + return ListRequestErrorDiagnostics(ctx, allDiags...) + } + + req.ResourceIdentitySchema, err = fromproto5.IdentitySchema(ctx, schemaResp.ProtoV5IdentitySchema) + if err != nil { + diags.AddError("Converting Resource Identity Schema", err.Error()) + allDiags.Append(diags...) + return ListRequestErrorDiagnostics(ctx, allDiags...) + } + } else { req.ResourceSchema, diags = s.FrameworkServer.ResourceSchema(ctx, protoReq.TypeName) allDiags.Append(diags...) diff --git a/list/list_resource.go b/list/list_resource.go index 17324bc7a..4bd932bcc 100644 --- a/list/list_resource.go +++ b/list/list_resource.go @@ -42,14 +42,14 @@ type ListResource interface { List(context.Context, ListRequest, *ListResultsStream) } -// ListResourceWithProtoSchemas is an interface type that extends ListResource to include a method +// ListResourceWithRawV5Schemas is an interface type that extends ListResource to include a method // which allows provider developers to supply the ProtoV5 representations of resource and resource identity // schemas. This is necessary if list functionality is being used with a legacy resource. -type ListResourceWithProtoSchemas interface { +type ListResourceWithRawV5Schemas interface { ListResource - // Schemas is called to provide the ProtoV5 representations of the resource and resource identity schemas. - Schemas(context.Context, *SchemaResponse) + // RawV5Schemas is called to provide the ProtoV5 representations of the resource and resource identity schemas. + RawV5Schemas(context.Context, SchemaRequest, *SchemaResponse) } // ListResourceWithConfigure is an interface type that extends ListResource to include a method @@ -193,18 +193,23 @@ type ListResult struct { Diagnostics diag.Diagnostics } +// SchemaRequest represents a request for the ListResource to return the +// ProtoV5 schemas. An instance of this request struct is supplied as an argument +// to the ListResource type RawV5Schemas method. +type SchemaRequest struct{} + // SchemaResponse represents a response that is populated by the Schemas method // and is used to pass along the ProtoV5 representations of the resource and resource identity schemas. type SchemaResponse struct { // ProtoV5IdentitySchema is the ProtoV5 representation of the resource identity // schema. This should only be supplied if framework functionality is being used // with a legacy resource. Currently, this only applies to list. - ProtoV5IdentitySchema func() *tfprotov5.ResourceIdentitySchema + ProtoV5IdentitySchema *tfprotov5.ResourceIdentitySchema // ProtoV5Schema is the ProtoV5 representation of the resource schema // This should only be supplied if framework functionality is being used // with a legacy resource. Currently, this only applies to list. - ProtoV5Schema func() *tfprotov5.Schema + ProtoV5Schema *tfprotov5.Schema } // ValidateConfigRequest represents a request to validate the configuration of From 13d0805293e8098f34abeb4c0b753e77d3798f94 Mon Sep 17 00:00:00 2001 From: Steph Date: Tue, 26 Aug 2025 10:44:33 +0200 Subject: [PATCH 10/12] updated expected error message --- internal/fwserver/server_getmetadata_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/fwserver/server_getmetadata_test.go b/internal/fwserver/server_getmetadata_test.go index 25f601082..c55beecb1 100644 --- a/internal/fwserver/server_getmetadata_test.go +++ b/internal/fwserver/server_getmetadata_test.go @@ -839,7 +839,7 @@ func TestServerGetMetadata(t *testing.T) { diag.NewErrorDiagnostic( "ListResource Type Defined without a Matching Managed Resource Type", "The test_resource_1 ListResource type name was returned, but no matching managed Resource type was defined. "+ - "If the matching managed Resource type is a legacy resource, ProtoV5Schema and ProtoV5IdentitySchema must be specified in the Metadata method. "+ + "If the matching managed Resource type is a legacy resource, ProtoV5Schema and ProtoV5IdentitySchema must be specified in the RawV5Schemas method. "+ "This is always an issue with the provider and should be reported to the provider developers.", ), }, From a39dc07ee2a6fc02a1fb40dbd6df2108edb422a4 Mon Sep 17 00:00:00 2001 From: Steph Date: Tue, 26 Aug 2025 11:10:22 +0200 Subject: [PATCH 11/12] extend support for supplying raw schema types to proto6 --- internal/fwserver/server_getmetadata_test.go | 3 +- internal/fwserver/server_listresources.go | 14 ++++-- internal/proto5server/server_listresource.go | 5 +- internal/proto6server/server_listresource.go | 53 ++++++++++++++------ list/list_resource.go | 40 +++++++++++++-- 5 files changed, 87 insertions(+), 28 deletions(-) diff --git a/internal/fwserver/server_getmetadata_test.go b/internal/fwserver/server_getmetadata_test.go index c55beecb1..bb3296175 100644 --- a/internal/fwserver/server_getmetadata_test.go +++ b/internal/fwserver/server_getmetadata_test.go @@ -839,7 +839,8 @@ func TestServerGetMetadata(t *testing.T) { diag.NewErrorDiagnostic( "ListResource Type Defined without a Matching Managed Resource Type", "The test_resource_1 ListResource type name was returned, but no matching managed Resource type was defined. "+ - "If the matching managed Resource type is a legacy resource, ProtoV5Schema and ProtoV5IdentitySchema must be specified in the RawV5Schemas method. "+ + "If the matching managed Resource type not a framework resource either ProtoV5Schema and ProtoV5IdentitySchema must be specified in the RawV5Schemas method, "+ + "or ProtoV6Schema and ProtoV6IdentitySchema must be specified in the RawV6Schemas method. "+ "This is always an issue with the provider and should be reported to the provider developers.", ), }, diff --git a/internal/fwserver/server_listresources.go b/internal/fwserver/server_listresources.go index dc3577d86..e85365bf6 100644 --- a/internal/fwserver/server_listresources.go +++ b/internal/fwserver/server_listresources.go @@ -85,18 +85,24 @@ func (s *Server) ListResourceFuncs(ctx context.Context) (map[string]func() list. continue } - schemasResp := list.SchemaResponse{} + rawV5SchemasResp := list.RawV5SchemaResponse{} if listResourceWithSchemas, ok := listResource.(list.ListResourceWithRawV5Schemas); ok { - listResourceWithSchemas.RawV5Schemas(ctx, list.SchemaRequest{}, &schemasResp) + listResourceWithSchemas.RawV5Schemas(ctx, list.RawV5SchemaRequest{}, &rawV5SchemasResp) + } + + rawV6SchemasResp := list.RawV6SchemaResponse{} + if listResourceWithSchemas, ok := listResource.(list.ListResourceWithRawV6Schemas); ok { + listResourceWithSchemas.RawV6Schemas(ctx, list.RawV6SchemaRequest{}, &rawV6SchemasResp) } resourceFuncs, _ := s.ResourceFuncs(ctx) if _, ok := resourceFuncs[typeName]; !ok { - if schemasResp.ProtoV5Schema == nil || schemasResp.ProtoV5IdentitySchema == nil { + if (rawV5SchemasResp.ProtoV5Schema == nil || rawV5SchemasResp.ProtoV5IdentitySchema == nil) && (rawV6SchemasResp.ProtoV6Schema == nil || rawV6SchemasResp.ProtoV6IdentitySchema == nil) { s.listResourceFuncsDiags.AddError( "ListResource Type Defined without a Matching Managed Resource Type", fmt.Sprintf("The %s ListResource type name was returned, but no matching managed Resource type was defined. ", typeName)+ - "If the matching managed Resource type is a legacy resource, ProtoV5Schema and ProtoV5IdentitySchema must be specified in the RawV5Schemas method. "+ + "If the matching managed Resource type not a framework resource either ProtoV5Schema and ProtoV5IdentitySchema must be specified in the RawV5Schemas method, "+ + "or ProtoV6Schema and ProtoV6IdentitySchema must be specified in the RawV6Schemas method. "+ "This is always an issue with the provider and should be reported to the provider developers.", ) continue diff --git a/internal/proto5server/server_listresource.go b/internal/proto5server/server_listresource.go index 99a168a6d..a817f20e5 100644 --- a/internal/proto5server/server_listresource.go +++ b/internal/proto5server/server_listresource.go @@ -55,9 +55,9 @@ func (s *Server) ListResource(ctx context.Context, protoReq *tfprotov5.ListResou Limit: protoReq.Limit, } - schemaResp := list.SchemaResponse{} + schemaResp := list.RawV5SchemaResponse{} if listResourceWithProtoSchemas, ok := listResource.(list.ListResourceWithRawV5Schemas); ok { - listResourceWithProtoSchemas.RawV5Schemas(ctx, list.SchemaRequest{}, &schemaResp) + listResourceWithProtoSchemas.RawV5Schemas(ctx, list.RawV5SchemaRequest{}, &schemaResp) } // There's validation in ListResources that ensures both are set if either is provided so it should be sufficient to only nil check Identity @@ -77,7 +77,6 @@ func (s *Server) ListResource(ctx context.Context, protoReq *tfprotov5.ListResou allDiags.Append(diags...) return ListRequestErrorDiagnostics(ctx, allDiags...) } - } else { req.ResourceSchema, diags = s.FrameworkServer.ResourceSchema(ctx, protoReq.TypeName) allDiags.Append(diags...) diff --git a/internal/proto6server/server_listresource.go b/internal/proto6server/server_listresource.go index b180c4fbc..53512effb 100644 --- a/internal/proto6server/server_listresource.go +++ b/internal/proto6server/server_listresource.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/internal/fromproto6" "github.com/hashicorp/terraform-plugin-framework/internal/fwserver" "github.com/hashicorp/terraform-plugin-framework/internal/toproto6" + "github.com/hashicorp/terraform-plugin-framework/list" "github.com/hashicorp/terraform-plugin-go/tfprotov6" ) @@ -47,26 +48,48 @@ func (s *Server) ListResource(ctx context.Context, protoReq *tfprotov6.ListResou return ListRequestErrorDiagnostics(ctx, allDiags...) } - resourceSchema, diags := s.FrameworkServer.ResourceSchema(ctx, protoReq.TypeName) - allDiags.Append(diags...) - if diags.HasError() { - return ListRequestErrorDiagnostics(ctx, allDiags...) + req := &fwserver.ListRequest{ + Config: config, + ListResource: listResource, + IncludeResource: protoReq.IncludeResource, + Limit: protoReq.Limit, } - identitySchema, diags := s.FrameworkServer.ResourceIdentitySchema(ctx, protoReq.TypeName) - allDiags.Append(diags...) - if diags.HasError() { - return ListRequestErrorDiagnostics(ctx, allDiags...) + schemaResp := list.RawV6SchemaResponse{} + if listResourceWithProtoSchemas, ok := listResource.(list.ListResourceWithRawV6Schemas); ok { + listResourceWithProtoSchemas.RawV6Schemas(ctx, list.RawV6SchemaRequest{}, &schemaResp) } - req := &fwserver.ListRequest{ - Config: config, - ListResource: listResource, - ResourceSchema: resourceSchema, - ResourceIdentitySchema: identitySchema, - IncludeResource: protoReq.IncludeResource, - Limit: protoReq.Limit, + if schemaResp.ProtoV6IdentitySchema != nil { + var err error + + req.ResourceSchema, err = fromproto6.ResourceSchema(ctx, schemaResp.ProtoV6Schema) + if err != nil { + diags.AddError("Converting Resource Schema", err.Error()) + allDiags.Append(diags...) + return ListRequestErrorDiagnostics(ctx, allDiags...) + } + + req.ResourceIdentitySchema, err = fromproto6.IdentitySchema(ctx, schemaResp.ProtoV6IdentitySchema) + if err != nil { + diags.AddError("Converting Resource Identity Schema", err.Error()) + allDiags.Append(diags...) + return ListRequestErrorDiagnostics(ctx, allDiags...) + } + } else { + req.ResourceSchema, diags = s.FrameworkServer.ResourceSchema(ctx, protoReq.TypeName) + allDiags.Append(diags...) + if diags.HasError() { + return ListRequestErrorDiagnostics(ctx, allDiags...) + } + + req.ResourceIdentitySchema, diags = s.FrameworkServer.ResourceIdentitySchema(ctx, protoReq.TypeName) + allDiags.Append(diags...) + if diags.HasError() { + return ListRequestErrorDiagnostics(ctx, allDiags...) + } } + stream := &fwserver.ListResultsStream{} s.FrameworkServer.ListResource(ctx, req, stream) diff --git a/list/list_resource.go b/list/list_resource.go index 4bd932bcc..c0c4f0645 100644 --- a/list/list_resource.go +++ b/list/list_resource.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/tfsdk" "github.com/hashicorp/terraform-plugin-go/tfprotov5" + "github.com/hashicorp/terraform-plugin-go/tfprotov6" "github.com/hashicorp/terraform-plugin-go/tftypes" ) @@ -49,7 +50,17 @@ type ListResourceWithRawV5Schemas interface { ListResource // RawV5Schemas is called to provide the ProtoV5 representations of the resource and resource identity schemas. - RawV5Schemas(context.Context, SchemaRequest, *SchemaResponse) + RawV5Schemas(context.Context, RawV5SchemaRequest, *RawV5SchemaResponse) +} + +// ListResourceWithRawV6Schemas is an interface type that extends ListResource to include a method +// which allows provider developers to supply the ProtoV5 representations of resource and resource identity +// schemas. This is necessary if list functionality is being used with a legacy resource. +type ListResourceWithRawV6Schemas interface { + ListResource + + // RawV6Schemas is called to provide the ProtoV5 representations of the resource and resource identity schemas. + RawV6Schemas(context.Context, RawV6SchemaRequest, *RawV6SchemaResponse) } // ListResourceWithConfigure is an interface type that extends ListResource to include a method @@ -193,14 +204,14 @@ type ListResult struct { Diagnostics diag.Diagnostics } -// SchemaRequest represents a request for the ListResource to return the +// RawV5SchemaRequest represents a request for the ListResource to return the // ProtoV5 schemas. An instance of this request struct is supplied as an argument // to the ListResource type RawV5Schemas method. -type SchemaRequest struct{} +type RawV5SchemaRequest struct{} -// SchemaResponse represents a response that is populated by the Schemas method +// RawV5SchemaResponse represents a response that is populated by the Schemas method // and is used to pass along the ProtoV5 representations of the resource and resource identity schemas. -type SchemaResponse struct { +type RawV5SchemaResponse struct { // ProtoV5IdentitySchema is the ProtoV5 representation of the resource identity // schema. This should only be supplied if framework functionality is being used // with a legacy resource. Currently, this only applies to list. @@ -212,6 +223,25 @@ type SchemaResponse struct { ProtoV5Schema *tfprotov5.Schema } +// RawV6SchemaRequest represents a request for the ListResource to return the +// ProtoV5 schemas. An instance of this request struct is supplied as an argument +// to the ListResource type RawV5Schemas method. +type RawV6SchemaRequest struct{} + +// RawV6SchemaResponse represents a response that is populated by the Schemas method +// and is used to pass along the ProtoV5 representations of the resource and resource identity schemas. +type RawV6SchemaResponse struct { + // ProtoV5IdentitySchema is the ProtoV5 representation of the resource identity + // schema. This should only be supplied if framework functionality is being used + // with a legacy resource. Currently, this only applies to list. + ProtoV6IdentitySchema *tfprotov6.ResourceIdentitySchema + + // ProtoV5Schema is the ProtoV5 representation of the resource schema + // This should only be supplied if framework functionality is being used + // with a legacy resource. Currently, this only applies to list. + ProtoV6Schema *tfprotov6.Schema +} + // ValidateConfigRequest represents a request to validate the configuration of // a list resource. An instance of this request struct is supplied as an // argument to the [ListResourceWithValidateConfig.ValidateListResourceConfig] From aea7a09f32f9fbea9b65b316c801ace930bf477b Mon Sep 17 00:00:00 2001 From: Steph Date: Thu, 28 Aug 2025 10:44:35 +0200 Subject: [PATCH 12/12] review comments --- internal/fwserver/server_getmetadata_test.go | 2 +- internal/fwserver/server_listresources.go | 2 +- list/list_resource.go | 22 ++++++++++---------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/internal/fwserver/server_getmetadata_test.go b/internal/fwserver/server_getmetadata_test.go index bb3296175..605e9fc49 100644 --- a/internal/fwserver/server_getmetadata_test.go +++ b/internal/fwserver/server_getmetadata_test.go @@ -839,7 +839,7 @@ func TestServerGetMetadata(t *testing.T) { diag.NewErrorDiagnostic( "ListResource Type Defined without a Matching Managed Resource Type", "The test_resource_1 ListResource type name was returned, but no matching managed Resource type was defined. "+ - "If the matching managed Resource type not a framework resource either ProtoV5Schema and ProtoV5IdentitySchema must be specified in the RawV5Schemas method, "+ + "If the matching managed Resource type is not a framework resource either ProtoV5Schema and ProtoV5IdentitySchema must be specified in the RawV5Schemas method, "+ "or ProtoV6Schema and ProtoV6IdentitySchema must be specified in the RawV6Schemas method. "+ "This is always an issue with the provider and should be reported to the provider developers.", ), diff --git a/internal/fwserver/server_listresources.go b/internal/fwserver/server_listresources.go index e85365bf6..b1819a6d4 100644 --- a/internal/fwserver/server_listresources.go +++ b/internal/fwserver/server_listresources.go @@ -101,7 +101,7 @@ func (s *Server) ListResourceFuncs(ctx context.Context) (map[string]func() list. s.listResourceFuncsDiags.AddError( "ListResource Type Defined without a Matching Managed Resource Type", fmt.Sprintf("The %s ListResource type name was returned, but no matching managed Resource type was defined. ", typeName)+ - "If the matching managed Resource type not a framework resource either ProtoV5Schema and ProtoV5IdentitySchema must be specified in the RawV5Schemas method, "+ + "If the matching managed Resource type is not a framework resource either ProtoV5Schema and ProtoV5IdentitySchema must be specified in the RawV5Schemas method, "+ "or ProtoV6Schema and ProtoV6IdentitySchema must be specified in the RawV6Schemas method. "+ "This is always an issue with the provider and should be reported to the provider developers.", ) diff --git a/list/list_resource.go b/list/list_resource.go index c0c4f0645..ea71cf49e 100644 --- a/list/list_resource.go +++ b/list/list_resource.go @@ -45,7 +45,7 @@ type ListResource interface { // ListResourceWithRawV5Schemas is an interface type that extends ListResource to include a method // which allows provider developers to supply the ProtoV5 representations of resource and resource identity -// schemas. This is necessary if list functionality is being used with a legacy resource. +// schemas. This is necessary if list functionality is being used with a resource that is not defined with Framework. type ListResourceWithRawV5Schemas interface { ListResource @@ -54,12 +54,12 @@ type ListResourceWithRawV5Schemas interface { } // ListResourceWithRawV6Schemas is an interface type that extends ListResource to include a method -// which allows provider developers to supply the ProtoV5 representations of resource and resource identity -// schemas. This is necessary if list functionality is being used with a legacy resource. +// which allows provider developers to supply the ProtoV6 representations of resource and resource identity +// schemas. This is necessary if list functionality is being used with a resource that is not defined with Framework. type ListResourceWithRawV6Schemas interface { ListResource - // RawV6Schemas is called to provide the ProtoV5 representations of the resource and resource identity schemas. + // RawV6Schemas is called to provide the ProtoV6 representations of the resource and resource identity schemas. RawV6Schemas(context.Context, RawV6SchemaRequest, *RawV6SchemaResponse) } @@ -209,7 +209,7 @@ type ListResult struct { // to the ListResource type RawV5Schemas method. type RawV5SchemaRequest struct{} -// RawV5SchemaResponse represents a response that is populated by the Schemas method +// RawV5SchemaResponse represents a response that is populated by the RawV5Schemas method // and is used to pass along the ProtoV5 representations of the resource and resource identity schemas. type RawV5SchemaResponse struct { // ProtoV5IdentitySchema is the ProtoV5 representation of the resource identity @@ -224,19 +224,19 @@ type RawV5SchemaResponse struct { } // RawV6SchemaRequest represents a request for the ListResource to return the -// ProtoV5 schemas. An instance of this request struct is supplied as an argument -// to the ListResource type RawV5Schemas method. +// ProtoV6 schemas. An instance of this request struct is supplied as an argument +// to the ListResource type RawV6Schemas method. type RawV6SchemaRequest struct{} -// RawV6SchemaResponse represents a response that is populated by the Schemas method -// and is used to pass along the ProtoV5 representations of the resource and resource identity schemas. +// RawV6SchemaResponse represents a response that is populated by the RawV6Schemas method +// and is used to pass along the ProtoV6 representations of the resource and resource identity schemas. type RawV6SchemaResponse struct { - // ProtoV5IdentitySchema is the ProtoV5 representation of the resource identity + // ProtoV6IdentitySchema is the ProtoV6 representation of the resource identity // schema. This should only be supplied if framework functionality is being used // with a legacy resource. Currently, this only applies to list. ProtoV6IdentitySchema *tfprotov6.ResourceIdentitySchema - // ProtoV5Schema is the ProtoV5 representation of the resource schema + // ProtoV6Schema is the ProtoV6 representation of the resource schema // This should only be supplied if framework functionality is being used // with a legacy resource. Currently, this only applies to list. ProtoV6Schema *tfprotov6.Schema