-
Notifications
You must be signed in to change notification settings - Fork 10.1k
Upgrade protoc and protoc-gen-go-grpc versions to matching terraform-plugin-go #37647
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
Upgrade protoc and protoc-gen-go-grpc versions to matching terraform-plugin-go #37647
Conversation
|
Keeping this in draft until after HashiConf |
6122570 to
37a4101
Compare
|
Edit: deleting the contents of tools/protobuf-compile/.workdir allowed me to reproduce the problem. Maybe the dependency updates weren't reflected in the binaries in that folder? |
This matched terraform-plugin-go
7fc5c74 to
c140d96
Compare
Looks like this was added in v1.69.3 in grpc/grpc-go#7057 ?
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.
This file has manually-made diffs that need closer review
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.
Here's generated code that informed this change:
terraform/internal/cloudplugin/cloudproto1/cloudproto1_grpc.pb.go
Lines 63 to 64 in 6c41e77
| // This type alias is provided for backwards compatibility with existing code that references the prior non-generic stream type by name. | |
| type CommandService_ExecuteClient = grpc.ServerStreamingClient[CommandResponse] |
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.
This file has diffs that need closer review
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.
Here's generated code that informed this change:
terraform/internal/tfplugin5/tfplugin5_grpc.pb.go
Lines 1276 to 1277 in 6c41e77
| // This type alias is provided for backwards compatibility with existing code that references the prior non-generic stream type by name. | |
| type Provisioner_ProvisionResourceClient = grpc.ServerStreamingClient[ProvisionResource_Response] |
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.
This file has manually-made diffs that need closer review
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.
Here's generated code that informed this change:
terraform/internal/tfplugin6/tfplugin6_grpc.pb.go
Lines 443 to 444 in 6c41e77
| // This type alias is provided for backwards compatibility with existing code that references the prior non-generic stream type by name. | |
| type Provider_ReadStateBytesClient = grpc.ServerStreamingClient[ReadStateBytes_Response] |
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.
This file has manually-made diffs that need closer review
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.
Here's generated code that informed this change:
terraform/internal/stacksplugin/stacksproto1/stacksproto1_grpc.pb.go
Lines 63 to 64 in 6c41e77
| // This type alias is provided for backwards compatibility with existing code that references the prior non-generic stream type by name. | |
| type CommandService_ExecuteClient = grpc.ServerStreamingClient[CommandResponse] |
terraform/internal/cloudplugin/cloudproto1/cloudproto1_grpc.pb.go
Lines 63 to 64 in 6c41e77
| // This type alias is provided for backwards compatibility with existing code that references the prior non-generic stream type by name. | |
| type CommandService_ExecuteClient = grpc.ServerStreamingClient[CommandResponse] |
liamcervante
left a 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.
I think this looks fine 👍 I guess the mockgen package seems to not be keeping up with the generics used by the proto files but nothing we can do about it.
|
Thanks Liam! Yeah it'd be nice if the mocks had the correct type baked in, instead of requiring setup in the test code, but if that causes issues in future I guess we can set up more test helpers. FYI to any readers/future me: I'm planning to merge this to main tomorrow after the release, and I'm not planning to backport. Given that the main branch currently represents the first alpha release of 1.15, and alphas do not need Mend scans, merging this PR doesn't impact anyone's release rota. |
|
Going ahead with merge despite the release not going out; main branch doesn't affect the release code. |
Do we want these values to match terraform-plugin-go? Does it even matter?
Not sure, but it's still good to keep these dependencies updated, and updating to match terraform-plugin-go feels like a good thing to arbitrarily aim for right now.
This PR:
protocwe useprotoc-gen-go-grpcwe useinternal/rpcapi/dynrpcserver/generator/main.goto be able to handle the generic types that are now in use after the update.A lot of diffs in this PR are in generated code, so I've highlighted areas where manual changes were made with comments. See also diffs in
internal/rpcapi/dynrpcserver/generator/main.goand go.mod/sumTarget Release
N/A
Rollback Plan
Changes to Security Controls
Are there any changes to security controls (access controls, encryption, logging) in this pull request? If so, explain.
CHANGELOG entry