Skip to content
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 GetMetadata RPC to provider protocol #33739

Merged
merged 3 commits into from
Aug 24, 2023
Merged

Add GetMetadata RPC to provider protocol #33739

merged 3 commits into from
Aug 24, 2023

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Aug 24, 2023

Reference: #33486

This is a followup to the new provider server capability to make the GetProviderSchema RPC optional. While this server capability would perform its intended function when directly talking to a single provider server SDK implementation, provider servers using terraform-plugin-mux need a methodology for the mux server to determine the available resource types of each underlying provider server to properly route resource-specific RPCs. Since the only methodology available to the mux server would be calling the GetProviderSchema RPC to each of underlying provider servers, any memory optimization of core caching would be lost.

The choice of adding a new RPC instead of adjusting the existing GetProviderSchema RPC with additional request information, such as "only list the type names and not the schema information in the response", is two-fold:

  • Prevents the introduction of conditional logic for the existing RPC.
  • Clearly delineates the purpose of the RPC and can be documented easier.

The choice of adding this to the existing provider service is two-fold:

  • Implementing a separate protocol and/or service only on the provider side of the protocol would be a novel design change. This small of a change does not warrant the potential research and testing effort that would be associated with that implementation.
  • While the core implementation will not use the new RPC immediately, there is no reason why it should be restricted from doing so in the future if a valid use case surfaces. Other ecosystem tools, beyond terraform-plugin-mux, can also potentially benefit from the lightweight RPC now.

This is changing the 5.4 and 6.4 protocol versions following the guidance of this comment in the definition files, since it directly relates to the prior intention of the new minor versions:

// Note that only the proto files included in a release tag of Terraform are
// official protocol releases. Proto files taken from other commits may include
// incomplete changes or features that did not make it into a final release.
// In all reasonable cases, plugin developers should take the proto file from
// the tag of the most recent release of Terraform, and not from the main
// branch or any other development branch.

As with any Protocol Buffers definition update, protocol compatibility is guaranteed within a major version, however generated protocol source code compatibility is not guaranteed. In this case, implementing the new RPC method in protocol wrapper types and the moving of the ServerCapabilities message to the top namespace are considered acceptable changes.

Target Release

1.6.x

Draft CHANGELOG entry

N/A

Reference: #33486

This is a followup to the new provider server capability to make the `GetProviderSchema` RPC optional. While this server capability would perform its intended function when directly talking to a single provider server SDK implementation, provider servers using terraform-plugin-mux need a methodology for the mux server to determine the available resource types of each underlying provider server to properly route resource-specific RPCs. Since the only methodology available to the mux server would be calling the `GetProviderSchema` RPC to each of underlying provider servers, any memory optimization of core caching would be lost.

The choice of adding a new RPC instead of adjusting the existing `GetProviderSchema` RPC with additional request information, such as "only list the type names and not the schema information in the response", is two-fold:

- Prevents the introduction of conditional logic for the existing RPC.
- Clearly delineates the purpose of the RPC and can be documented easier.

The choice of adding this to the existing provider service is two-fold:

- Implementing a separate protocol and/or service only on the provider side of the protocol would be a novel design change. This small of a change does not warrant the potential research and testing effort that would be associated with that implementation.
- While the core implementation will not use the new RPC immediately, there is no reason why it should be restricted from doing so in the future if a valid use case surfaces. Other ecosystem tools, beyond terraform-plugin-mux, can also potentially benefit from the lightweight RPC now.

This is changing the 5.4 and 6.4 protocol versions following the guidance of this comment in the definition files, since it directly relates to the prior intention of the new minor versions:

```protobuf
// Note that only the proto files included in a release tag of Terraform are
// official protocol releases. Proto files taken from other commits may include
// incomplete changes or features that did not make it into a final release.
// In all reasonable cases, plugin developers should take the proto file from
// the tag of the most recent release of Terraform, and not from the main
// branch or any other development branch.
```

As with any Protocol Buffers definition update, protocol compatibility is guaranteed within a major version, however generated protocol source code compatibility is not guaranteed. In this case, implementing the new RPC method in protocol wrapper types and the moving of the `ServerCapabilities` message to the top namespace are considered acceptable changes.
@bflad bflad merged commit 7094517 into main Aug 24, 2023
4 checks passed
@bflad bflad deleted the bflad/metadata-rpc branch August 24, 2023 18:35
@github-actions
Copy link

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants