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

appengine v2 and pre-v2 packages generate proto registry conflicts with one another #281

Closed
aqua opened this issue Apr 17, 2022 · 4 comments
Labels
bankrupt Feel free to reopen when necessary.

Comments

@aqua
Copy link

aqua commented Apr 17, 2022

google.golang.org/appengine/v2 is offered as a transition strategy for GAE apps to use some of the longer-term supported GAE services while escaping the go111 runtime. However, the two packages both internally use a remote_api subpackage generated from a protobuf, and the two versions share the same package name, which generates a conflict in the proto registry when the two packages are present anywhere in the same binary. The migration notes don't mention that the two can't be in the same import closure simultaneously, and the only way to figure out the problem is to match the offending proto package against the entire dependency closure, where one eventually finds it in the appengine/internal subpackage.

I get that the two versions aren't meant to be used simultaneously, since v2 is acting as an API bridge, but not even having it be possible to be present in the same binary makes an even bigger refactoring burden to the one already involved in getting off the legacy runtime. Since the appengine package contains common datatypes like GeoPoint, utility functions like IsDevAppServer(), and all the serving-setup calls, it tends to get used in a lot of different parts of a codebase, which may have dependencies on one another. To do an incremental migration in an established codebase requires a strategy like moving all the appengine dependencies to a swappable top-level package first, which is the sort of migration trouble it seemed the v2 package was meant to avoid.

There's a basic repro case at https://github.com/aqua/gae-version-conflict-bug (the thing1 tests pass, the thing2 tests panic; more recent protobuf versions panic, older ones log about undefined behavior) if you want one.

@dasper
Copy link

dasper commented May 1, 2023

What is even worse is there are proto conflicts even when there are zero appengine v1 calls at all.

config pulls

- cloud.google.com/go/secretmanager/apiv1
-- google.golang.org/api/transport/grpc
--- google.golang.org/appengine v1.6.7 // indirect

Content API for Shopping pulls

- google.golang.org/api/option
-- golang.org/x/oauth2/google
--- google.golang.org/appengine v1.6.7 // indirect

even with zero calls to appengine v1 these still pull them and result in panic: proto: message appengine.base.StringProto is already registered as both v1 and v2 share the same internal files

There needs to be better documentation on what is expected to change these dependencies.

@martinflorek
Copy link

even with zero calls to appengine v1 these still pull them and result in panic: proto: message appengine.base.StringProto is already registered as both v1 and v2 share the same internal files

Is there any kind of a workaround for this?

@dasper
Copy link

dasper commented May 5, 2023

I would presume if you migrated off of all appengine packages then the gcloud would only pull v1 packages through its dependency chain and therefore have no registration collisions but our project cannot do that yet so this is just conjecture.

The only thing I have found so far is to update your app.yaml file to include a env variable to downgrade the new protobuf behavior back down to a warning.

env_variables:
  GOLANG_PROTOBUF_REGISTRATION_CONFLICT: warn

This should not have many (or any) side effects as currently App Engine does not allow you to create your own grpc endpoints so the likelyhood of reducing this to a warn instead of an error causing any major issues is low. Still, it is my opinion that the appengine packages still need to be fixed as v1/v2 versions are supposed to be mutually exclusive for end developers so having the recomended gcloud packages for the standard and flex environments pull internal dependencies by the legacy v1 package by default is asinine. Even if this is a bug due to user error or improper package management then the documentation is so shoddy that there is no answer to how to resolve it. Even here; this is an open issue over a year old with no response from the maintainers. This should be a good warning to anyone using App Engine to consider making their code platform agnostic and look at other cloud vendors.

@martinflorek
Copy link

I have upgraded all my APIs to v2, actually I am using only the Memcache APIs, so it it was not a big task.

During the migration I have upgraded all Google libraries and that was the problem. They use the new protobuf repo google.golang.org/protobuf and GAE APIs v2 are still using the old github.com/golang/protobuf. Thus I have downgraded the libraries back, so they use the old repo -- problem gone.

There is an issue for the migration to the new protobuf repo, that is opened for more than two years #228 I think that we have to wait for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bankrupt Feel free to reopen when necessary.
Projects
None yet
Development

No branches or pull requests

4 participants