-
Notifications
You must be signed in to change notification settings - Fork 464
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
Update deprecated jsonpb and protobuf/proto dependencies #902
Labels
Comments
This was referenced Apr 26, 2021
Merged
MartinPetkov
added a commit
to MartinPetkov/cloud-foundation-toolkit
that referenced
this issue
May 3, 2021
In github.com/GoogleCloudPlatform/issues/902, I suggested eventually moving to protojson. According to golang/protobuf#1121 (comment), that library does not support a stable serialization output. That means that the test in proto_test.go would become flaky. The suggestion there is "Instead of syntactically comparing the serialized form of a message, the test should semantically compare the structured form of a message", so I've done that here by loading the got and want strings as JSON and comparing the structured JSON.
morgante
pushed a commit
that referenced
this issue
May 3, 2021
In github.com//issues/902, I suggested eventually moving to protojson. According to golang/protobuf#1121 (comment), that library does not support a stable serialization output. That means that the test in proto_test.go would become flaky. The suggestion there is "Instead of syntactically comparing the serialized form of a message, the test should semantically compare the structured form of a message", so I've done that here by loading the got and want strings as JSON and comparing the structured JSON.
This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 7 days |
SnowmanSeniorDev
added a commit
to SnowmanSeniorDev/cloud-foundation-toolkit
that referenced
this issue
Apr 17, 2023
In github.com/GoogleCloudPlatform/cloud-foundation-toolkit/issues/902, I suggested eventually moving to protojson. According to golang/protobuf#1121 (comment), that library does not support a stable serialization output. That means that the test in proto_test.go would become flaky. The suggestion there is "Instead of syntactically comparing the serialized form of a message, the test should semantically compare the structured form of a message", so I've done that here by loading the got and want strings as JSON and comparing the structured JSON.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The following dependencies are deprecated:
github.com/golang/protobuf/jsonpb
->google.golang.org/protobuf/encoding/protojson
github.com/golang/protobuf/proto
->google.golang.org/protobuf/proto
I spent some time investigating replacing these, but unfortunately it turned out to be non-trivial.
The code can be changed easily enough. However, CFT uses the
*validator.Asset
type from config-validator's validator.pb.go, which must be updated to use the new dependencies too. This is how far I got with attempting to do that:google.golang.org/protobuf/cmd/protoc-gen-go
in the Dockerfile instead ofgithub.com/golang/protobuf/protoc-gen-go
.option go_package = "github.com/forseti-security/config-validator/pkg/api/validator";
to validator.proto.go mod tidy
).At this point I ran into an error:
This seems related to micro/go-micro#2091 and duo-labs/webauthn#76. The generated validator.pb.go asserts that the minimum version of grpc-go is v1.32.0. I'm not sure if the fix here is to use an older version of protoc-gen-go or to refactor the code to not use the
naming
package.As well, other repositories that depend on validator.pb.go may need to be updated to use the newer
google.golang.org/protobuf/proto
package, which is additional work.At the end of the day, I'm not sure these deprecated libraries pose a large risk, but I wanted to record my investigative work for anyone who can pick this up in the future.
The text was updated successfully, but these errors were encountered: