Initial Setup for ValidateListResourceConfig in List RPCs #514
Conversation
rainkwan
changed the title
| @@ -348,6 +359,7 @@ func TestGetProviderSchema_Response(t *testing.T) { | |||
| DataSourceSchemas: map[string]*tfplugin5.Schema{}, | |||
| Diagnostics: []*tfplugin5.Diagnostic{}, | |||
| EphemeralResourceSchemas: map[string]*tfplugin5.Schema{}, | |||
| ListResourceSchemas: map[string]*tfplugin5.Schema{}, | |||
There was a problem hiding this comment.
💭 I'm also finding myself repeating the setup of empty maps in table-driven tests. An optional follow-up: how do we feel about using cmptopts.EquateEmpty to remove the repetitive setup of empty maps? I have never used it, but it seems useful here.
| "context" | ||
| ) | ||
|
|
||
| // ListResourceMetadata describes metadata for an list resource in the GetMetadata |
There was a problem hiding this comment.
| // ListResourceMetadata describes metadata for an list resource in the GetMetadata | |
| // ListResourceMetadata describes metadata for a list resource in the GetMetadata |
& find-replace similar usages :)
| // list resource configuration is valid. It is guaranteed to have types | ||
| // conforming to your schema, but it is not guaranteed that all values | ||
| // will be known. This is your opportunity to do custom or advanced | ||
| // validation prior to an list resource being opened. |
There was a problem hiding this comment.
| // validation prior to an list resource being opened. | |
| // validation prior to a list resource being used. |
| // ValidateListResourceConfigRequest is the request Terraform sends when it | ||
| // wants to validate an list resource's configuration. | ||
| type ValidateListResourceConfigRequest struct { | ||
| // TypeName is the type of resource Terraform is validating. |
There was a problem hiding this comment.
| // TypeName is the type of resource Terraform is validating. | |
| // TypeName is the type of list resource Terraform is validating. |
| // list resource configuration is valid. It is guaranteed to have types | ||
| // conforming to your schema, but it is not guaranteed that all values | ||
| // will be known. This is your opportunity to do custom or advanced | ||
| // validation prior to an list resource being opened. |
There was a problem hiding this comment.
| // validation prior to an list resource being opened. | |
| // validation prior to a list resource being used. |
| // - ListResource | ||
| // - ValidateListResourceConfig | ||
| // | ||
| // Deprecated: All methods will be moved into the |
| rpc := "ValidateListResourceConfig" | ||
| ctx = s.loggingContext(ctx) | ||
| ctx = logging.RpcContext(ctx, rpc) | ||
| ctx = logging.ResourceContext(ctx, protoReq.TypeName) |
There was a problem hiding this comment.
Let's define a new logging.ListResourceContext method, for consistency with DataSourceContext and EphemeralResourceContext.
|
|
||
| ctx = tf5serverlogging.DownstreamRequest(ctx) | ||
|
|
||
| // TODO: Remove this check and error in preference of |
| // TODO: Remove this check and error in preference of | ||
| // s.downstream.ValidateListResourceConfig below once ProviderServer interface | ||
| // implements this RPC method. | ||
| // nolint:staticcheck |
| if !ok { | ||
| logging.ProtocolError(ctx, "ProviderServer does not implement ValidateListResourceConfig") | ||
|
|
||
| protoResp := &tfplugin5.ValidateListResourceConfig_Response{ |
There was a problem hiding this comment.
A non-blocking thought for follow-up: in light of recent experience with unimplemented RPCs, I'm curious if Terraform prefers the provider to return an error instead of a diagnostic.
https://pkg.go.dev/google.golang.org/grpc/status#Error and https://pkg.go.dev/google.golang.org/[email protected]/codes#Code – Unimplemented Code = 12.
Related Issue
Fixes #
Description
Includes support for RPCs ValidateListResourceConfig outlined by TF in https://github.com/hashicorp/terraform/blob/main/docs/plugin-protocol/tfplugin5.proto and https://github.com/hashicorp/terraform/blob/main/docs/plugin-protocol/tfplugin6.proto
Rollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
No