fix(requester pays): Fix requester pays for gRPC and also setup integration tests with gRPC with explicit SA.#4611
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4611 +/- ##
==========================================
+ Coverage 83.64% 83.67% +0.02%
==========================================
Files 164 164
Lines 20272 20278 +6
==========================================
+ Hits 16956 16967 +11
+ Misses 2676 2672 -4
+ Partials 640 639 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
50e35d9 to
fd71118
Compare
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a bug in the gRPC protocol implementation where the billing project was not correctly propagated, leading to failures in requester pays scenarios. Additionally, it enhances the integration testing suite by introducing a dedicated service account and improved credential management, ensuring that tests correctly validate requester pays behavior by avoiding reliance on overly privileged accounts that previously masked potential issues. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces support for requester-pays buckets by allowing a billing project to be specified and propagated through the storage and bucket handles. Key changes include the addition of a billingProject field, manual metadata injection for gRPC ListObjects calls, and updated integration tests that handle service account credentials. A review comment points out a high-severity issue where using option.WithQuotaProject on a shared gRPC client could lead to incorrect billing attribution across different buckets, recommending that this configuration be handled at the bucket level instead.
raj-prince
left a comment
There was a problem hiding this comment.
Looks good overall.
Added couple of minor comments.
…tegration test utilities
…ays handling in integration tests
…in integration tests
1460fb2 to
5b95b0f
Compare
Description
This PR fixes a bug in gRPC protocol requester pays feature. Where the billing project is not populated correctly causing errors of the following types.
Testing
Created separate SA with the required permission to test the requester pays feature correctly. Previously ran tests used overly Privileged account which had
resourcemanager.projects.createBillingAssignmentpermission on the project to which the bucket in test belonged. This causes the the requester pays tests to pass even if the billing project is not provided thus masking the failures.The SA and relevant permissions have been setup accordingly for the
gcs-fuse-testandgcs-fuse-test-mlprjoects.Tests are passing on GKE as well: https://b.corp.google.com/issues/500550846#comment19
Link to the issue in case of a bug fix.
b/500550846
Testing details
Any backward incompatible change? If so, please explain.
NONE