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
Member

@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
Member 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
Member 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
Member 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
Member 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change This will impact or improve our compatibility posture
Projects
None yet
2 participants