-
Notifications
You must be signed in to change notification settings - Fork 97
fwserver: add ListResource
method
#1153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
bbasata
wants to merge
3
commits into
add-list-to-fwserver-part-ii
Choose a base branch
from
add-list-to-fwserver-part-iii
base: add-list-to-fwserver-part-ii
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+327
−249
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,125 @@ | ||
// Copyright (c) HashiCorp, Inc. | ||
// SPDX-License-Identifier: MPL-2.0 | ||
|
||
package fwserver | ||
|
||
import ( | ||
"context" | ||
"iter" | ||
|
||
"github.com/hashicorp/terraform-plugin-framework/diag" | ||
"github.com/hashicorp/terraform-plugin-framework/internal/logging" | ||
"github.com/hashicorp/terraform-plugin-framework/list" | ||
"github.com/hashicorp/terraform-plugin-framework/tfsdk" | ||
) | ||
|
||
// ListRequest represents a request for the provider to list instances of a | ||
// managed resource type that satisfy a user-defined request. An instance of | ||
// this reqeuest struct is passed as an argument to the provider's List | ||
// function implementation. | ||
type ListRequest struct { | ||
// ListResource is an instance of the provider's ListResource | ||
// implementation for a specific managed resource type. | ||
ListResource list.ListResource | ||
|
||
// Config is the configuration the user supplied for listing resource | ||
// instances. | ||
Config tfsdk.Config | ||
|
||
// IncludeResource indicates whether the provider should populate the | ||
// Resource field in the ListResult struct. | ||
IncludeResource bool | ||
} | ||
|
||
// ListResultsStream represents a streaming response to a ListRequest. An | ||
// instance of this struct is supplied as an argument to the provider's List | ||
// function. The provider should set a Results iterator function that pushes | ||
// zero or more results of type ListResult. | ||
// | ||
// For convenience, a provider implementation may choose to convert a slice of | ||
// results into an iterator using [slices.Values]. | ||
// | ||
// [slices.Values]: https://pkg.go.dev/slices#Values | ||
type ListResourceStream struct { | ||
// Results is a function that emits ListResult values via its push | ||
// function argument. | ||
Results iter.Seq[ListResult] | ||
} | ||
|
||
// ListResult represents a listed managed resource instance. | ||
type ListResult struct { | ||
// Identity is the identity of the managed resource instance. A nil value | ||
// will raise will raise a diagnostic. | ||
Identity *tfsdk.ResourceIdentity | ||
|
||
// Resource is the provider's representation of the attributes of the | ||
// listed managed resource instance. | ||
// | ||
// If ListRequest.IncludeResource is true, a nil value will raise | ||
// a warning diagnostic. | ||
Resource *tfsdk.Resource | ||
|
||
// DisplayName is a provider-defined human-readable description of the | ||
// listed managed resource instance, intended for CLI and browser UIs. | ||
DisplayName string | ||
|
||
// Diagnostics report errors or warnings related to the listed managed | ||
// resource instance. An empty slice indicates a successful operation with | ||
// no warnings or errors generated. | ||
Diagnostics diag.Diagnostics | ||
} | ||
|
||
// ListResource implements the framework server ListResource RPC. | ||
func (s *Server) ListResource(ctx context.Context, fwReq *ListRequest, fwStream *ListResourceStream) error { | ||
listResource := fwReq.ListResource | ||
|
||
req := list.ListRequest{ | ||
Config: fwReq.Config, | ||
IncludeResource: fwReq.IncludeResource, | ||
} | ||
|
||
stream := &list.ListResultsStream{} | ||
|
||
logging.FrameworkTrace(ctx, "Calling provider defined ListResource") | ||
listResource.List(ctx, req, stream) | ||
logging.FrameworkTrace(ctx, "Called provider defined ListResource") | ||
|
||
if stream.Results == nil { | ||
// If the provider returned a nil results stream, we return an empty stream. | ||
stream.Results = list.NoListResults | ||
} | ||
|
||
fwStream.Results = processListResults(req, stream.Results) | ||
return nil | ||
} | ||
|
||
func processListResults(req list.ListRequest, stream iter.Seq[list.ListResult]) iter.Seq[ListResult] { | ||
return func(push func(ListResult) bool) { | ||
for result := range stream { | ||
if !push(processListResult(req, result)) { | ||
return | ||
} | ||
} | ||
} | ||
} | ||
|
||
// processListResult validates the content of a list.ListResult and returns a | ||
// ListResult | ||
func processListResult(req list.ListRequest, result list.ListResult) ListResult { | ||
if result.Identity == nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO: needs tests |
||
return ListResult{ | ||
Diagnostics: diag.Diagnostics{ | ||
diag.NewErrorDiagnostic("Incomplete List Result", "ListResult.Identity is nil."), | ||
}, | ||
} | ||
} | ||
|
||
if req.IncludeResource && result.Resource == nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TODO: needs tests |
||
result.Diagnostics.AddWarning( | ||
"Incomplete List Result", | ||
"ListRequest.IncludeResource is true and ListResult.Resource is nil.", | ||
) | ||
} | ||
|
||
return ListResult(result) | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,184 @@ | ||
// Copyright (c) HashiCorp, Inc. | ||
// SPDX-License-Identifier: MPL-2.0 | ||
|
||
package fwserver_test | ||
|
||
import ( | ||
"context" | ||
"slices" | ||
"testing" | ||
|
||
"github.com/google/go-cmp/cmp" | ||
"github.com/hashicorp/terraform-plugin-framework/diag" | ||
"github.com/hashicorp/terraform-plugin-framework/internal/fwserver" | ||
"github.com/hashicorp/terraform-plugin-framework/internal/testing/testprovider" | ||
"github.com/hashicorp/terraform-plugin-framework/list" | ||
"github.com/hashicorp/terraform-plugin-framework/resource/identityschema" | ||
"github.com/hashicorp/terraform-plugin-framework/resource/schema" | ||
"github.com/hashicorp/terraform-plugin-framework/tfsdk" | ||
"github.com/hashicorp/terraform-plugin-go/tftypes" | ||
) | ||
|
||
func TestServerListResource(t *testing.T) { | ||
t.Parallel() | ||
|
||
testSchema := schema.Schema{ | ||
Attributes: map[string]schema.Attribute{ | ||
"test_computed": schema.StringAttribute{ | ||
Computed: true, | ||
}, | ||
"test_required": schema.StringAttribute{ | ||
Required: true, | ||
}, | ||
}, | ||
} | ||
|
||
testType := tftypes.Object{ | ||
AttributeTypes: map[string]tftypes.Type{ | ||
"test_attribute": tftypes.String, | ||
}, | ||
} | ||
|
||
testResourceValue1 := tftypes.NewValue(testType, map[string]tftypes.Value{ | ||
"test_attribute": tftypes.NewValue(tftypes.String, "test-value-1"), | ||
}) | ||
|
||
testResourceValue2 := tftypes.NewValue(testType, map[string]tftypes.Value{ | ||
"test_attribute": tftypes.NewValue(tftypes.String, "test-value-2"), | ||
}) | ||
|
||
testIdentitySchema := identityschema.Schema{ | ||
Attributes: map[string]identityschema.Attribute{ | ||
"test_id": identityschema.StringAttribute{ | ||
RequiredForImport: true, | ||
}, | ||
}, | ||
} | ||
|
||
testIdentityType := tftypes.Object{ | ||
AttributeTypes: map[string]tftypes.Type{ | ||
"test_id": tftypes.String, | ||
}, | ||
} | ||
|
||
testIdentityValue1 := tftypes.NewValue(testIdentityType, map[string]tftypes.Value{ | ||
"test_id": tftypes.NewValue(tftypes.String, "new-id-123"), | ||
}) | ||
|
||
testIdentityValue2 := tftypes.NewValue(testIdentityType, map[string]tftypes.Value{ | ||
"test_id": tftypes.NewValue(tftypes.String, "new-id-456"), | ||
}) | ||
|
||
// nilIdentityValue := tftypes.NewValue(testIdentityType, nil) | ||
|
||
testCases := map[string]struct { | ||
server *fwserver.Server | ||
request *fwserver.ListRequest | ||
expectedStreamEvents []fwserver.ListResult | ||
}{ | ||
"success-with-zero-results": { | ||
server: &fwserver.Server{ | ||
Provider: &testprovider.Provider{}, | ||
}, | ||
request: &fwserver.ListRequest{ | ||
ListResource: &testprovider.ListResource{ | ||
ListMethod: func(ctx context.Context, req list.ListRequest, resp *list.ListResultsStream) { // TODO | ||
resp.Results = list.NoListResults | ||
}, | ||
}, | ||
}, | ||
expectedStreamEvents: []fwserver.ListResult{}, | ||
}, | ||
"success-with-nil-results": { | ||
server: &fwserver.Server{ | ||
Provider: &testprovider.Provider{}, | ||
}, | ||
request: &fwserver.ListRequest{ | ||
ListResource: &testprovider.ListResource{ | ||
ListMethod: func(ctx context.Context, req list.ListRequest, resp *list.ListResultsStream) { // TODO | ||
// Do nothing, so that resp.Results is nil | ||
}, | ||
}, | ||
}, | ||
expectedStreamEvents: []fwserver.ListResult{}, | ||
}, | ||
|
||
"success-with-multiple-results": { | ||
server: &fwserver.Server{ | ||
Provider: &testprovider.Provider{}, | ||
}, | ||
request: &fwserver.ListRequest{ | ||
ListResource: &testprovider.ListResource{ | ||
ListMethod: func(ctx context.Context, req list.ListRequest, resp *list.ListResultsStream) { // TODO | ||
resp.Results = slices.Values([]list.ListResult{ | ||
{ | ||
Identity: &tfsdk.ResourceIdentity{ | ||
Schema: testIdentitySchema, | ||
Raw: testIdentityValue1, | ||
}, | ||
Resource: &tfsdk.Resource{ | ||
Schema: testSchema, | ||
Raw: testResourceValue1, | ||
}, | ||
DisplayName: "Test Resource 1", | ||
Diagnostics: diag.Diagnostics{}, | ||
}, | ||
{ | ||
Identity: &tfsdk.ResourceIdentity{ | ||
Schema: testIdentitySchema, | ||
Raw: testIdentityValue2, | ||
}, | ||
Resource: &tfsdk.Resource{ | ||
Schema: testSchema, | ||
Raw: testResourceValue2, | ||
}, | ||
DisplayName: "Test Resource 2", | ||
Diagnostics: diag.Diagnostics{}, | ||
}, | ||
}) | ||
}, | ||
}, | ||
}, | ||
expectedStreamEvents: []fwserver.ListResult{ | ||
{ | ||
Identity: &tfsdk.ResourceIdentity{ | ||
Schema: testIdentitySchema, | ||
Raw: testIdentityValue1, | ||
}, | ||
Resource: &tfsdk.Resource{ | ||
Schema: testSchema, | ||
Raw: testResourceValue1, | ||
}, | ||
DisplayName: "Test Resource 1", | ||
Diagnostics: diag.Diagnostics{}, | ||
}, | ||
{ | ||
Identity: &tfsdk.ResourceIdentity{ | ||
Schema: testIdentitySchema, | ||
Raw: testIdentityValue2, | ||
}, | ||
Resource: &tfsdk.Resource{ | ||
Schema: testSchema, | ||
Raw: testResourceValue2, | ||
}, | ||
DisplayName: "Test Resource 2", | ||
Diagnostics: diag.Diagnostics{}, | ||
}, | ||
}, | ||
}, | ||
} | ||
|
||
for name, testCase := range testCases { | ||
t.Run(name, func(t *testing.T) { | ||
t.Parallel() | ||
|
||
response := &fwserver.ListResourceStream{} | ||
testCase.server.ListResource(context.Background(), testCase.request, response) | ||
|
||
events := slices.AppendSeq([]fwserver.ListResult{}, response.Results) | ||
if diff := cmp.Diff(events, testCase.expectedStreamEvents); diff != "" { | ||
t.Errorf("unexpected difference: %s", diff) | ||
} | ||
}) | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 A follow-up enhancement: what if a (List) Resource uses its data model instead of
ListResult
here? and Framework usesfwschemadata
to transform the data model into aListResult
Terraform value?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... and also, what if a List Resource "encapsulates" an SDKv2 managed resource, so it has no existing data model? is this still useful?