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

tfprotov5+tfprotov6: Require FunctionServer in ProviderServer and MoveResourceState in ResourceServer #388

Merged
merged 4 commits into from
Apr 22, 2024

Conversation

bflad
Copy link
Contributor

@bflad bflad commented Mar 11, 2024

Closes #353
Closes #363

This removes the temporary handling to smoothly release the new Terraform 1.8 and later RPC handling for provider servers. These changes codify this Go module's desired design that it reflects the protocol via its required implementations.

…eResourceState in ResourceServer

Reference: #353
Reference: #363

This removes the temporary handling to smoothly release the new Terraform 1.8 and later RPC handling for provider servers. These changes codify this Go module's desired design that it reflects the protocol via its required implementations.
@bflad bflad added the breaking-change This will impact or improve our compatibility posture label Mar 11, 2024
@bflad bflad added this to the v0.23.0 milestone Mar 11, 2024
@bflad bflad requested a review from a team as a code owner March 11, 2024 17:12
bflad added a commit to hashicorp/terraform-provider-corner that referenced this pull request Mar 11, 2024
Reference: hashicorp/terraform-plugin-go#388

These changes are so the testing provider is compliant with the upcoming `tfprotov5` and `tfprotov6` packages which require provider implementations to fully implement function server handling (already covered) and MoveResourceState (needs covering, hence this change). Almost the entire ecosystem will not have this issue as the higher level SDKs (terraform-plugin-framework, terraform-plugin-sdk, etc.) handle these protocol operations manually, but this test provider is written directly in the low level SDK, so it must implement the necessary changes itself prior to that upstream breaking change.
@bflad
Copy link
Contributor Author

bflad commented Mar 11, 2024

Submitted hashicorp/terraform-provider-corner#223 to make CI happier 😄

Copy link
Contributor

@bendbennett bendbennett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

bflad added a commit to hashicorp/terraform-provider-corner that referenced this pull request Mar 12, 2024
Reference: hashicorp/terraform-plugin-go#388

These changes are so the testing provider is compliant with the upcoming `tfprotov5` and `tfprotov6` packages which require provider implementations to fully implement function server handling (already covered) and MoveResourceState (needs covering, hence this change). Almost the entire ecosystem will not have this issue as the higher level SDKs (terraform-plugin-framework, terraform-plugin-sdk, etc.) handle these protocol operations manually, but this test provider is written directly in the low level SDK, so it must implement the necessary changes itself prior to that upstream breaking change.
@bflad
Copy link
Contributor Author

bflad commented Mar 12, 2024

Looks like this will need to be done over two releases because terraform-plugin-mux required calling MoveResourceState, which required directly referencing the temporary ResourceServerWithMoveResourceState interface. This PR will be updated to contain MoveResourceState method in both interfaces, then we can remove ResourceServerWithMoveResourceState later.

Maybe it can be a takeaway for future prerelease RPCs to only implement them in mux if absolutely necessary. The problem is that hashicorp/aws and other large providers tend to be a target for those features and use mux. We could consider whether mux should still remain a separate Go module, instead moving all those packages under the scope of this Go module. Mux is stable enough, low level enough, and also allows breaking changes. We would just need to sort out with the web platform team the website documentation since it references the terraform-plugin-mux repository directly.

@bflad
Copy link
Contributor Author

bflad commented Mar 12, 2024

Updated this PR to reinstate ResourceServerWithMoveResourceServer a little longer. Created #389 for eventual followup once this is released and terraform-plugin-mux is upgraded and released.

@bflad
Copy link
Contributor Author

bflad commented Apr 22, 2024

Given this functionality is now GA in Terraform 1.8 and the interfaces have been out for a month+, going to update and merge this for release at any time.

@bflad bflad merged commit 62ca775 into main Apr 22, 2024
72 checks passed
@bflad bflad deleted the bflad/require-interfaces branch April 22, 2024 13:49
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 May 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking-change This will impact or improve our compatibility posture
Projects
None yet
2 participants