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

Update deprecated jsonpb and protobuf/proto dependencies #902

Closed
MartinPetkov opened this issue Apr 22, 2021 · 1 comment
Closed

Update deprecated jsonpb and protobuf/proto dependencies #902

MartinPetkov opened this issue Apr 22, 2021 · 1 comment
Labels

Comments

@MartinPetkov
Copy link
Contributor

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:

  • Install google.golang.org/protobuf/cmd/protoc-gen-go in the Dockerfile instead of github.com/golang/protobuf/protoc-gen-go.
  • Add option go_package = "github.com/forseti-security/config-validator/pkg/api/validator"; to validator.proto.
  • In the Makefile, build like so from /go/src:
protoc -I/proto -I./github.com/forseti-security/config-validator/api --go-grpc_out=. --go_out=. ./github.com/forseti-security/config-validator/api/validator.proto
  • Update go.mod and go.sum (i.e. go mod tidy).

At this point I ran into an error:

$ go mod tidy
...
go: finding module for package google.golang.org/grpc/naming
github.com/forseti-security/config-validator/pkg/gcv/configs imports
	cloud.google.com/go/storage imports
	google.golang.org/api/option imports
	google.golang.org/api/internal imports
	google.golang.org/grpc/naming: module google.golang.org/grpc@latest found (v1.37.0), but does not contain package google.golang.org/grpc/naming

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.

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.
@github-actions
Copy link
Contributor

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

@github-actions github-actions bot added the Stale label Mar 14, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 22, 2023
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
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant