Skip to content

Upgrade io.grpc library to 1.70.0#24475

Merged
yingsu00 merged 1 commit intoprestodb:masterfrom
Dilli-Babu-Godari:grpc_upgrade
Mar 12, 2025
Merged

Upgrade io.grpc library to 1.70.0#24475
yingsu00 merged 1 commit intoprestodb:masterfrom
Dilli-Babu-Godari:grpc_upgrade

Conversation

@Dilli-Babu-Godari
Copy link
Contributor

@Dilli-Babu-Godari Dilli-Babu-Godari commented Feb 3, 2025

Description

Upgraded io.grpc library from 1.68.0 to 1.70.0.

As part of this upgrade, the following dependencies were updated:

  • io.grpc:grpc-protobuf-lite: 1.68.0 → 1.70.0
  • io.grpc:grpc-protobuf: 1.53.0 → 1.70.0
  • io.grpc:grpc-stub: 1.53.0 → 1.70.0
  • io.grpc:grpc-core: 1.53.0 → 1.70.0
  • io.grpc:grpc-api: 1.53.0 → 1.70.0
  • io.grpc:grpc-netty-shaded:1.69.0 → 1.70.0

Additionally, to resolve upper bound issues, the following dependencies were updated:

  • com.google.errorprone:error_prone_annotations: 2.28.0 to 2.36.0
  • com.google.auth:google-auth-library-oauth2-http: 1.23.0 to 1.31.0
  • com.google.auth:google-auth-library-credentials: 1.23.0 to 1.31.0
  • com.google.auto.value:auto-value-annotations: 1.10.4 to 1.11.0
  • com.google.http-client:google-http-client: 1.43.3 to 1.45.3
  • com.google.http-client:google-http-client-gson: 1.43.3 to 1.45.3
  • com.google.j2objc:j2objc-annotations: 2.8 to 3.0.0.

Motivation and Context

Addresses below CVEs
CVE-2024-7254 for grpc-protobuf, grpc-protobuf-lite.
CVE-2020-8908 and
CVE-2023-2976 for grpc-core, grpc-stub, grpc-api.

Impact

Test Plan

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== RELEASE NOTES ==

General Changes
* Upgrade the io.grpc library from version 1.68.0 to 1.70.0 in response to `CVE-2024-7254<https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-7254>`, `CVE-2020-8908<https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-8908>`
* Upgrade the errrorprone dependency from version 2.28.0 to 2.36.0

@prestodb-ci prestodb-ci added the from:IBM PR from IBM label Feb 3, 2025
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 3, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: Dilli-Babu-Godari (bedfe72)

@steveburnett
Copy link
Contributor

New release note guidelines as of last week: PR #24354 automatically adds links to this PR to the release notes. Please remove the manual PR link in the following format from the release note entries for this PR.

:pr:`12345`

I have updated the Release Notes Guidelines to remove the examples of manually adding the PR link.

@Dilli-Babu-Godari Dilli-Babu-Godari marked this pull request as ready for review February 4, 2025 06:33
@Dilli-Babu-Godari Dilli-Babu-Godari requested a review from a team as a code owner February 4, 2025 06:33
@Dilli-Babu-Godari Dilli-Babu-Godari marked this pull request as draft February 4, 2025 07:05
@Dilli-Babu-Godari Dilli-Babu-Godari force-pushed the grpc_upgrade branch 2 times, most recently from de32bc2 to dfbddd3 Compare February 4, 2025 08:53
@Dilli-Babu-Godari Dilli-Babu-Godari marked this pull request as ready for review February 4, 2025 09:51
Copy link
Member

@agrawalreetika agrawalreetika left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I have few questions,

@Dilli-Babu-Godari
Copy link
Contributor Author

Dilli-Babu-Godari commented Feb 6, 2025

Thanks for the PR. I have few questions,

Thanks for the review. Below are my responses to the questions:

  1. The dependency grpc-protobuf comes from the master POM, while grpc-protobuf-lite and grpc-protobuf come frompresto-bigquery. Meanwhile, grpc-stub, grpc-api, and grpc-core come from presto-base-arrow-flight.

  2. Regarding CVEs:

    CVE-2024-7254 is primarily observed in grpc version 1.68.0 of grpc-protobuf-lite and grpc-protobuf.
    CVE-2020-8908 and CVE-2023-2976 are found in grpc version 1.53.0 in dependencies like grpc-core, grpc-stub,
    grpc-api, and others.

I have updated CVEs accordingly in PR as well.

Copy link
Member

@imjalpreet imjalpreet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dilli-Babu-Godari, since there are dependency changes in BigQuery, could you please run a round of testing to ensure there are no runtime issues? I’m asking because Presto doesn’t have any integration or product tests that connect to BigQuery and execute queries. In the past, we’ve seen dependency upgrades cause regressions in the BigQuery connector, so it’s best to validate before merging. If the tests have already been done, then please describe them in the PR description.

@Dilli-Babu-Godari
Copy link
Contributor Author

Dilli-Babu-Godari commented Feb 14, 2025

@Dilli-Babu-Godari, since there are dependency changes in BigQuery, could you please run a round of testing to ensure there are no runtime issues? I’m asking because Presto doesn’t have any integration or product tests that connect to BigQuery and execute queries. In the past, we’ve seen dependency upgrades cause regressions in the BigQuery connector, so it’s best to validate before merging. If the tests have already been done, then please describe them in the PR description.

I am able to run a SELECT query with BigQuery, as verified in the images attached below.

Screenshot 2025-02-14 at 12 24 07 PM Screenshot 2025-02-14 at 12 24 15 PM Screenshot 2025-02-14 at 12 25 03 PM Screenshot 2025-02-14 at 12 25 10 PM

I perfomed shadow testing in CPD as well in wxd.

Screenshot 2025-02-14 at 10 29 04 AM Screenshot 2025-02-14 at 10 31 16 AM Screenshot 2025-02-14 at 11 36 51 AM

Could you please review once again ?

Copy link
Member

@agrawalreetika agrawalreetika left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LTGTM, please rebase the branch on latest master.

agrawalreetika
agrawalreetika previously approved these changes Feb 17, 2025
Copy link
Member

@imjalpreet imjalpreet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dilli-Babu-Godari Currently, we have the gRPC dependencies scattered across 3 modules(root, bigquery and pinot-toolkit). Since we use the same grpc.version across all modules, let’s move all gRPC dependencies to the DependencyManagement section of the root POM and remove <grpc.version> from presto-bigquery, presto-pinot-toolkit, and presto-pinot. This will allow us to update the version in a single place.

@Dilli-Babu-Godari
Copy link
Contributor Author

@imjalpreet, should we follow the same approach as @aaneja suggested in PR #24507 for this PR as well?

@aaneja
Copy link
Contributor

aaneja commented Feb 24, 2025

@imjalpreet, should we follow the same approach as @aaneja suggested in PR #24507 for this PR as well?

No, what you have here is good. Log4J is a special case, that IMO should be handled differently

Copy link
Member

@imjalpreet imjalpreet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dilli-Babu-Godari thank you for the changes, mostly LGTM apart from a few nits.

Can you also confirm that you verified the bigquery connector works fine after the latest changes?

@Dilli-Babu-Godari Dilli-Babu-Godari force-pushed the grpc_upgrade branch 2 times, most recently from 2018a6a to 4d4217d Compare February 28, 2025 10:02
@Dilli-Babu-Godari
Copy link
Contributor Author

@Dilli-Babu-Godari thank you for the changes, mostly LGTM apart from a few nits.

Can you also confirm that you verified the bigquery connector works fine after the latest changes?

I ran a SELECT query with bigquery connector, as verified in the images attached below.

Screenshot 2025-02-28 at 1 52 17 PM Screenshot 2025-02-28 at 1 51 53 PM

@Dilli-Babu-Godari
Copy link
Contributor Author

@imjalpreet could you please review the changes whenever you get a chance ? Thanks!

imjalpreet
imjalpreet previously approved these changes Mar 4, 2025
Copy link
Member

@imjalpreet imjalpreet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks.

@imjalpreet
Copy link
Member

@jaystarshot / @yingsu00 can you please have a look whenever you get a chance? Thanks!

agrawalreetika
agrawalreetika previously approved these changes Mar 6, 2025
Upgraded io.grpc library from 1.68.0 to 1.70.0.

As part of this upgrade, the following dependencies were updated:
- io.grpc:grpc-protobuf-lite: 1.68.0 → 1.70.0
- io.grpc:grpc-protobuf: 1.53.0 → 1.70.0
- io.grpc:grpc-stub: 1.53.0 → 1.70.0
- io.grpc:grpc-core: 1.53.0 → 1.70.0
- io.grpc:grpc-api: 1.53.0 → 1.70.0
Additionally, to resolve upper bound issues, the following dependencies were updated:
- com.google.errorprone:error_prone_annotations: 2.28.0 to 2.36.0
- com.google.auth:google-auth-library-oauth2-http: 1.23.0 to 1.31.0
- com.google.auth:google-auth-library-credentials: 1.23.0 to 1.31.0
- com.google.auto.value:auto-value-annotations: 1.10.4 to 1.11.0
- com.google.http-client:google-http-client: 1.43.3 to 1.45.3
- com.google.http-client:google-http-client-gson: 1.43.3 to 1.45.3
- com.google.j2objc:j2objc-annotations: 2.8 to 3.0.0
@yingsu00 yingsu00 merged commit 8f487f6 into prestodb:master Mar 12, 2025
92 checks passed
@prestodb-ci prestodb-ci mentioned this pull request Mar 28, 2025
30 tasks
@prestodb-ci prestodb-ci requested review from a team, auden-woolfson and imsayari404 and removed request for a team April 3, 2025 04:08
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 this pull request may close these issues.

8 participants