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

gcp-libraries-bom and protobuf-java compatibility #3420

Closed
edudar-chwy opened this issue Dec 16, 2024 · 14 comments
Closed

gcp-libraries-bom and protobuf-java compatibility #3420

edudar-chwy opened this issue Dec 16, 2024 · 14 comments
Assignees
Labels
priority: p1 type: bug Something isn't working

Comments

@edudar-chwy
Copy link

In the 5.9.0 update, the version of gcp-libraries-bom has been bumped to 26.51.0 in #3350. That bom switched to a new major version of protobuf-java starting 26.50.0. Because the new protobuf is not backward compatible, they even added a new BOM for protobuf3: googleapis/java-cloud-bom#6764. As a result, if a code needs protobuf3, it starts failing with a bunch of NoSuchMethod and similar exceptions.

It would be good to either follow a similar path and provide a second BOM or to lower the dependency version to pre-protobuf. All other options are welcome.

Lowering the version via maven/gradle properties also works, but it took quite some time to trace the root of the issue, as it was not obvious that a minor version upgrade would bring a major incompatible version of a core library.

@zhumin8
Copy link
Contributor

zhumin8 commented Dec 19, 2024

Can you provide a bit more details on which method is giving NoSuchMethod in your case?

@zhumin8 zhumin8 added the type: bug Something isn't working label Dec 19, 2024
@burkedavison burkedavison self-assigned this Dec 19, 2024
@lqiu96
Copy link
Contributor

lqiu96 commented Dec 19, 2024

Hi, I believe makeExtensionsImmutable() gencode should have been removed from Protobuf starting with Protoc 21.7+. IIRC, it was removed as part of the FootMitten vulnerability.

Do you have any user protos generated with Protoc 21.6 or earlier? Also, do you have a repro or any code snippets that you could help showcase how you're seeing the NoMethodError? I believe our Java SDK should be compatible with 3.x and 4.x Protobuf-Java. Thanks!

@edudar-chwy
Copy link
Author

edudar-chwy commented Dec 19, 2024

It was not even in our own code but a 3p dependency that we use. The latest version of temporal-sdk uses relatively old grpc which probably uses protoc built prior 21.7. I'll bring this up with temporal maintainers as well.

To clarify, I posted this issue here because the 5.9 release enforced the version bump to 4.x, and that didn't play nicely with some other dependencies we have that are not as compatible with both 3.x and 4.x...

@lqiu96
Copy link
Contributor

lqiu96 commented Dec 19, 2024

I see this also reported in Protobuf repo Issue #19540.

(As you noted above) We do offer the libraries-bom-protobuf3 which does set the Protobuf-Java runtime to 3.25.x.

it took quite some time to trace the root of the issue, as it was not obvious that a minor version upgrade would bring a major incompatible version of a core library.

Understood. We do have notes detailing the upgrade in the libraries-bom repo, but it may not be obvious to look there. I'll see to try and also add the notes in the spring-cloud-gcp repo and hopefully this may help others that come across this issue.

@burkedavison
Copy link
Member

burkedavison commented Dec 26, 2024

Closing this issue as resolved. To summarize:

Release notes have been amended to note the update and potential issues.

Although a major version dependency update is not something we would normally perform during a minor version release, this situation was an exception due to its backward compatibility with the prior major version.

An exception to this backward compatibility is a change made to resolve a CVE.

@burkedavison
Copy link
Member

@edudar-chwy : If you continue to have issues beyond the NoSuchMethod error related to makeExtensionsImmutable(), please let us know.

@wleese
Copy link

wleese commented Dec 30, 2024

The link to the page of the CVE is not public, could more details be provided?

We noticed that this change breaks applications that use Spring Cloud GCP & Beam (which in turn pulls in hadoop-connectors that hasn't been made compatible with protobuf 4.x).

Without a new release of hadoop-connectors (and preferably a new release of the Beam SDK), apps with Spring Cloud GCP 5.9.0 are stuck.

@lqiu96
Copy link
Contributor

lqiu96 commented Dec 30, 2024

The link to the page of the CVE is not public, could more details be provided?

Apologies, I didn't realize the link was not public. I believe it was this CVE: GHSA-h4h5-3hr4-j3g2

We noticed that this change breaks applications that use Spring Cloud GCP & Beam (which in turn pulls in GoogleCloudDataproc/hadoop-connectors#1270 that hasn't been made compatible with protobuf 4.x). Without a new release of hadoop-connectors (and preferably a new release of the Beam SDK), apps with Spring Cloud GCP 5.9.0 are stuck.

Thanks for bringing this up. I wasn't aware of this connection. I'll need to have a discussion internally about this given Beam's release cycle cadence. For now, I believe a temporary workaround would be to manually import the Protobuf-Java v3.25.5 bom (manually downgrade the protobuf-java runtime version to what it was before this spring release).

@lqiu96
Copy link
Contributor

lqiu96 commented Dec 30, 2024

Hadoop-Connectors has been made Protobuf 3.x and 4.x compatible: GoogleCloudDataproc/hadoop-connectors#1282 and this change was released as part of v2.2.26.

Beam has been updates to reference this version of hadoop-connectors: apache/beam@4bac374, but hasn't been released yet. Should be part of the next Beam release.

In the meantime, I believe there are a two ways to workaround this until the next Beam release (let me know if there are others or if things don't work):

  1. Override Hadoop-Connectors itself to v2.2.26+. This may have potential issues with Beam
  2. Override the Protobuf-Java version to use the 3.x runtime. This should align the Protobuf version in Spring and Beam to both be Protobuf 3.x

@wleese
Copy link

wleese commented Dec 31, 2024

I'll poke some people to try the workarounds. They may be lazy and prefer to hold off on Spring Cloud GCP 5.9.0 until a new version of Beam is released, and it's holidays so no promises :)

I do wonder however, why was the choice made not to go with using libraries-bom-protobuf3 and by extension Protobuf-Java v3.25.5?

I see the same suggestion being made in this issue, but no reasoning why breaking compatibility was deemed the right choice here.

@lqiu96
Copy link
Contributor

lqiu96 commented Jan 6, 2025

Protobuf guarantees forward compatibility and ensures that Protoc gen code X runs with Protobuf-Java runtimes X and X+1. However, for Protoc gen code 3.x and Protobuf-Java runtime 4.x, we know that it is not fully compatible. As noted in the release notes, we know this new runtime version brings changes that may impact a few customers with source, binary, or Protoc gen code incompatibilities (i.e. hadoop-connectors and temporal-sdk from the earlier comments). We believe that these cases are not common and that a vast majority of customers should not be impacted.

The Google Cloud Java SDK has been updated to be source and binary compatible with Protobuf-Java Runtime 4.x. Since Spring-Cloud-GCP uses these client libraries, we believe Spring-Cloud-GCP is fully compatible with Protobuf-Java runtimes 3.x and 4.x.

We chose the default to be libraries-bom (Protobuf-Java 4.x) since we aim to keep the Java SDK up to date with the latest versions. We expect that a majority of users will be able to use the latest stable features without any issue. Customers who do experience issues can use libraries-bom-protobuf3 or use Protobuf-Java 3.25.5 until the incompatibilities are resolved.

@breun
Copy link
Contributor

breun commented Jan 6, 2025

We chose the default to be libraries-bom (Protobuf-Java 4.x) since we aim to keep the Java SDK up to date with the latest versions.

It's of course a good principle to keep dependencies as current as possible, but I think in principle using the latest version should not be preferred over maintaining backwards compatibility within a major version of a library. Semantic versioning and the Tip & Tail model are still the best models out there for users of libraries, and I'd love for Spring Cloud GCP (and all other libraries in existence) to use those, because they make the life of integrators and users so much easier. Spring Cloud GCP 5 could have gotten the Protobuf 3 update with the CVE fix, and the upcoming Spring Cloud GCP 6 release could have gotten the latest Protobuf 4 version with the latest features, and nobody would have gotten stuck while upgrading to the latest minor version, and users could have dealt with the breaking changes when they upgraded to a new major version.

If Protobuf 4 was presented as being fully backwards compatible with 3, and only after adoption turned out to not be, I can kind of understand the choice to upgrade to Protobuf 4, but there is always a higher risk of accidental backwards incompatibilities with major version upgrades, so even when backwards compatibility is promised, I would recommend always being very hesitant with major version updates of dependencies during the lifetime of a particular major version of a library, especially when there is a patch update available. I just see breaking changes like these creeping into libraries due to broken compatibility promises way too often (not necessarily just in Spring Cloud GCP, but in general).

@lqiu96
Copy link
Contributor

lqiu96 commented Jan 7, 2025

I think in principle using the latest version should not be preferred over maintaining backwards compatibility within a major version of a library

Compatibility is something that we do try to maintain in the SDK and in Spring Cloud GCP. I know it doesn't sounds true given the above messages and the issue we're commenting on. Major version updates for our deps are scrutinized and we don't just pull in those versions just because it's new.

There were additional internal factors and limitations that led us to this and it was a tradeoff given our findings during testing (i.e. not fully backwards compatible). Ideally, Protobuf 4.x runtime upgrade should have been as major version bump of the libraries-bom (+ probably as part of a new version of Spring-Cloud-GCP). This would clearly signal the incompatibilities that are mentioned above and in the release notes.

Semantic versioning and the Tip & Tail model are still the best models out there for users of libraries

Thanks for the link to the model. That's something we'll look into.

I just see breaking changes like these creeping into libraries due to broken compatibility promises way too often

Yep, understood. This is something that we are looking to improve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p1 type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants