-
Notifications
You must be signed in to change notification settings - Fork 97
Add ListResource RPC #1157
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
Add ListResource RPC #1157
Changes from all commits
d07f61a
ce0467f
fe9a7fa
a193b8d
a5a3d3f
3ff5bfa
eecfd76
7f3191d
f6b4307
38660df
d18edd0
7e41955
9590b11
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,147 @@ | ||
// Copyright (c) HashiCorp, Inc. | ||
// SPDX-License-Identifier: MPL-2.0 | ||
|
||
package fwserver | ||
|
||
import ( | ||
"context" | ||
"errors" | ||
"iter" | ||
|
||
"github.com/hashicorp/terraform-plugin-framework/diag" | ||
"github.com/hashicorp/terraform-plugin-framework/internal/fwschema" | ||
"github.com/hashicorp/terraform-plugin-framework/internal/logging" | ||
"github.com/hashicorp/terraform-plugin-framework/list" | ||
"github.com/hashicorp/terraform-plugin-framework/tfsdk" | ||
) | ||
|
||
// ListRequest is the framework server request for the ListResource RPC. | ||
type ListRequest struct { | ||
// ListResource is an instance of the provider's list resource | ||
// 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 | ||
|
||
ResourceSchema fwschema.Schema | ||
ResourceIdentitySchema fwschema.Schema | ||
} | ||
|
||
// 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]. | ||
type ListResultsStream struct { | ||
// Results is a function that emits [ListResult] values via its push | ||
// function argument. | ||
Results iter.Seq[ListResult] | ||
} | ||
|
||
func ListResultError(summary string, detail string) ListResult { | ||
return ListResult{ | ||
Diagnostics: diag.Diagnostics{ | ||
diag.NewErrorDiagnostic(summary, detail), | ||
}, | ||
} | ||
} | ||
|
||
// ListResult represents a listed managed resource instance. | ||
type ListResult struct { | ||
// Identity is the identity of the managed resource instance. A nil value | ||
// will raise an error 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 | ||
} | ||
|
||
var NoListResults = func(func(ListResult) bool) {} | ||
|
||
// ListResource implements the framework server ListResource RPC. | ||
func (s *Server) ListResource(ctx context.Context, fwReq *ListRequest, fwStream *ListResultsStream) error { | ||
listResource := fwReq.ListResource | ||
|
||
if fwReq.Config == nil { | ||
fwStream.Results = NoListResults | ||
return errors.New("Invalid ListResource request: Config cannot be nil") | ||
} | ||
|
||
req := list.ListRequest{ | ||
Config: *fwReq.Config, | ||
IncludeResource: fwReq.IncludeResource, | ||
ResourceSchema: fwReq.ResourceSchema, | ||
ResourceIdentitySchema: fwReq.ResourceIdentitySchema, | ||
} | ||
|
||
stream := &list.ListResultsStream{} | ||
|
||
logging.FrameworkTrace(ctx, "Calling provider defined ListResource") | ||
listResource.List(ctx, req, stream) | ||
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 | ||
} | ||
|
||
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.Diagnostics.HasError() { | ||
return ListResult(result) | ||
} | ||
|
||
if result.Identity == nil { // TODO: is result.Identity.Raw.IsNull() a practical concern? | ||
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. I guess the question would: does Terraform core care if the provider returns a In which case we should just check both:
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. 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. Terraform currently raises an error when a list result has no identity |
||
return ListResultError( | ||
"Incomplete List Result", | ||
"The provider did not populate the Identity field in the ListResourceResult. This may be due to an error in the provider's implementation.", | ||
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. nit: We can probably use more guided language since it's almost certainly a bug in the provider and should be reported there first: Something to the tune of -> 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. +1. Updating. |
||
) | ||
} | ||
|
||
if req.IncludeResource { | ||
if result.Resource == nil { // TODO: is result.Resource.Raw.IsNull() a practical concern? | ||
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. Same as the above comment 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. 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. GitHub notifications aren't working well today, I hope I don't miss any mentions. Ideally a provider should always send a resource when include resource is true, but Terraform doesn't enforce that |
||
result.Diagnostics.AddWarning( | ||
"Incomplete List Result", | ||
"The provider did not populate the Resource field in the ListResourceResult. This may be due to the provider not supporting this functionality or an error in the provider's implementation.", | ||
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. Perhaps future PR, but if this is a warning that practitioners are likely to see often, maybe we should tailor it specifically to them (i.e. not using the Go types since they have no reference of what these fields mean and won't be reporting to provider developers) 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. I agree, and +1 to a future PR for this. 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. 💭 I'm less certain of the guidance for a nil Resource warning. It's a warning, after all. Is it actionable by the user? Open to suggestions. |
||
) | ||
} | ||
} | ||
|
||
return ListResult(result) | ||
} |
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.
TODO: add a doc comment
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.
Since it's not a "publicly" exported field, no need to add a doc comment unless you feel this helper needs more explanation for maintainers.
Although I'm wondering if this function even needs to be exported (or even exist, it's only used once ATM, maybe it will be used more later?)