Fix broken support for requester-pays buckets #3256

gargnitingoogle · 2025-04-29T11:58:45Z

Description

Pass billing-project in every GCS call, except LROs (Long Running Operations) from storage control-client (through "x-goog-user-project") to fix mounting and operations of requester-pays buckets . This functionality was broken since v2.9.0 .

This fixes the mount of requester-pays buckets, as well as the following operations which go through storage-control-client.

GetFolder,
CreateFolder,
RenameFolder ~~# failing even after passing the billing-project until the bug fix for GCS bug (b/414790551) completely rolls out.~~ Works as b/414790551 has been fixed.
DeleteFolder

In terms of code, this also required

Add a new implementation of the interface StorageControlClient for the case when a billing-project has been specified by the gcsfuse user.

Note: After this PR merged, E2e tests need to be added for requester-pays buckets. That task is captured in b/415244758

Link to the issue in case of a bug fix.

b/406090515, b/405913611, #3111

Testing details

Manual - Tested manually to see that mounting of requester_pays buckets worked after this change.

Before change

go run . --foreground --implicit-dirs --log-severity=trace --billing-project=gcs-fuse-test-ml ${bucket} ${mountpath}
fusermount: entry for ${mountpath} not found in /etc/mtab
{"timestamp":{"seconds":1745946878,"nanos":13683580},"severity":"INFO","message":"Start gcsfuse/unknown (Go version go1.24.0) for app \"\" using mount point: ${mountpath}\n"}
...
{"timestamp":{"seconds":1745946878,"nanos":13918813},"severity":"INFO","message":"Creating Storage handle..."}
{"timestamp":{"seconds":1745946878,"nanos":13930025},"severity":"INFO","message":"UserAgent = gcsfuse/unknown (Go version go1.24.0) (GPN:gcsfuse) (Cfg:0:0:0:0)\n"}
{"timestamp":{"seconds":1745946878,"nanos":16407768},"severity":"INFO","message":"Creating a mount at \"${mountpath}\"\n"}
{"timestamp":{"seconds":1745946878,"nanos":16455442},"severity":"INFO","message":"Creating a new server...\n"}
{"timestamp":{"seconds":1745946878,"nanos":16482274},"severity":"INFO","message":"Set up root directory for bucket ${bucket}"}
{"timestamp":{"seconds":1745946878,"nanos":16494007},"severity":"INFO","message":"GetStorageLayout <- (${bucket})"}
{"timestamp":{"seconds":1745946878,"nanos":184320600},"severity":"ERROR","message":"Error while mounting gcsfuse: mountWithStorageHandle: fs.NewServer: create file system: SetUpBucket: BucketHandle: storageLayout call failed: rpc error: code = InvalidArgument desc = Bucket is a requester pays bucket but no user project provided.\nerror details: name = BadRequest field =  desc = Bucket is a requester pays bucket but no user project provided.\n"}
Error: mountWithStorageHandle: fs.NewServer: create file system: SetUpBucket: BucketHandle: storageLayout call failed: rpc error: code = InvalidArgument desc = Bucket is a requester pays bucket but no user project provided. error details: name = BadRequest field =  desc = Bucket is a requester pays bucket but no user project provided.
{"timestamp":{"seconds":1745946878,"nanos":184366869},"severity":"INFO","message":"Error occurred during command execution: mountWithStorageHandle: fs.NewServer: create file system: SetUpBucket: BucketHandle: storageLayout call failed: rpc error: code = InvalidArgument desc = Bucket is a requester pays bucket but no user project provided.\nerror details: name = BadRequest field =  desc = Bucket is a requester pays bucket but no user project provided."}
   exit status 1      ```

After change

go run . --foreground --implicit-dirs --log-severity=trace --billing-project=gcs-fuse-test-ml ${bucket} ${mountpath}
fusermount: entry for ${mountpath} not found in /etc/mtab
{"timestamp":{"seconds":1745946919,"nanos":399534338},"severity":"INFO","message":"Start gcsfuse/unknown (Go version go1.24.0) for app \"\" using mount point: ${mountpath}\n"}
...
{"timestamp":{"seconds":1745946919,"nanos":399811490},"severity":"INFO","message":"Creating Storage handle..."}
{"timestamp":{"seconds":1745946919,"nanos":399817677},"severity":"INFO","message":"UserAgent = gcsfuse/unknown (Go version go1.24.0) (GPN:gcsfuse) (Cfg:0:0:0:0)\n"}
{"timestamp":{"seconds":1745946919,"nanos":401077298},"severity":"INFO","message":"Creating a mount at \"${mountpath}\"\n"}
{"timestamp":{"seconds":1745946919,"nanos":401108382},"severity":"INFO","message":"Creating a new server...\n"}
{"timestamp":{"seconds":1745946919,"nanos":401125238},"severity":"INFO","message":"Set up root directory for bucket ${bucket}"}
{"timestamp":{"seconds":1745946919,"nanos":401135109},"severity":"INFO","message":"GetStorageLayout <- (bucket_name=\"${bucket}\", billing_project=\"${project_id}\")"}
{"timestamp":{"seconds":1745946919,"nanos":461349555},"severity":"INFO","message":"GetStorageLayout -> (${bucket}) 60 msec"}
{"timestamp":{"seconds":1745946919,"nanos":461452000},"severity":"TRACE","message":"gcs: Req              0x0: <- ListObjects(\"\")"}
{"timestamp":{"seconds":1745946919,"nanos":514638707},"severity":"TRACE","message":"gcs: Req              0x0: -> ListObjects(\"\") (53.173086ms): OK"}
{"timestamp":{"seconds":1745946919,"nanos":514708601},"severity":"INFO","message":"Mounting file system \"${bucket}\"..."}
{"timestamp":{"seconds":1745946919,"nanos":514741935},"severity":"TRACE","message":"fuse_debug: Beginning the mounting kickoff process"}
{"timestamp":{"seconds":1745946919,"nanos":514751759},"severity":"TRACE","message":"fuse_debug: Parsing fuse file descriptor"}
{"timestamp":{"seconds":1745946919,"nanos":514757221},"severity":"TRACE","message":"fuse_debug: Preparing for direct mounting"}
{"timestamp":{"seconds":1745946919,"nanos":514775959},"severity":"TRACE","message":"fuse_debug: Successfully opened the /dev/fuse in blocking mode"}
{"timestamp":{"seconds":1745946919,"nanos":514787476},"severity":"TRACE","message":"fuse_debug: Starting the unix mounting"}
{"timestamp":{"seconds":1745946919,"nanos":514801604},"severity":"TRACE","message":"fuse_debug: Directmount failed. Trying fallback."}
{"timestamp":{"seconds":1745946919,"nanos":514846909},"severity":"TRACE","message":"fuse_debug: Creating a socket pair"}
{"timestamp":{"seconds":1745946919,"nanos":514866096},"severity":"TRACE","message":"fuse_debug: Creating files to wrap the sockets"}
{"timestamp":{"seconds":1745946919,"nanos":514874077},"severity":"TRACE","message":"fuse_debug: Starting fusermount/os mount"}
{"timestamp":{"seconds":1745946919,"nanos":523319858},"severity":"TRACE","message":"fuse_debug: Wrapping socket pair in a connection"}
{"timestamp":{"seconds":1745946919,"nanos":523355567},"severity":"TRACE","message":"fuse_debug: Checking that we have a unix domain socket"}
{"timestamp":{"seconds":1745946919,"nanos":523363096},"severity":"TRACE","message":"fuse_debug: Read a message from socket"}
{"timestamp":{"seconds":1745946919,"nanos":523373111},"severity":"TRACE","message":"fuse_debug: Successfully read the socket message."}
{"timestamp":{"seconds":1745946919,"nanos":523379735},"severity":"TRACE","message":"fuse_debug: Converting FD into os.File"}
{"timestamp":{"seconds":1745946919,"nanos":523394973},"severity":"TRACE","message":"fuse_debug: Completed the mounting kickoff process"}
{"timestamp":{"seconds":1745946919,"nanos":523403624},"severity":"TRACE","message":"fuse_debug: Creating a connection object"}
{"timestamp":{"seconds":1745946919,"nanos":523485383},"severity":"TRACE","message":"fuse_debug: Op 0x00000002        connection.go:426] <- init"}
{"timestamp":{"seconds":1745946919,"nanos":523504387},"severity":"TRACE","message":"fuse_debug: Op 0x00000002        connection.go:521] -> init ()"}
{"timestamp":{"seconds":1745946919,"nanos":523517943},"severity":"TRACE","message":"fuse_debug: Successfully created the connection"}
{"timestamp":{"seconds":1745946919,"nanos":523531577},"severity":"TRACE","message":"fuse_debug: Waiting for mounting process to complete"}
{"timestamp":{"seconds":1745946919,"nanos":523540819},"severity":"INFO","message":"File system has been successfully mounted."}

Tested folder listing/creation/deletion/rename operations, which worked on a requester-pays bucket with this change.

~~Tested folder rename operation, which failed with following error (captured in b/414790551)~~

error in renaming folder: rpc error: code = InvalidArgument desc = This operation does not support custom billing projects at this time.\nerror details: name = BadRequest field =  desc = This operation does not support custom billing projects at this time."}

Unit tests - NA
Integration tests - Passed as part of presubmit run1, run2 .

Any backward incompatible change? If so, please explain.

codecov · 2025-04-29T18:06:00Z

Codecov Report

Attention: Patch coverage is 39.13043% with 14 lines in your changes missing coverage. Please review.

Project coverage is 78.84%. Comparing base (abfec48) to head (82ce8dc).
Report is 6 commits behind head on master.

Files with missing lines	Patch %	Lines
internal/storage/control_client_wrapper.go	27.77%	13 Missing ⚠️
internal/storage/storage_handle.go	75.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3256      +/-   ##
==========================================
- Coverage   78.92%   78.84%   -0.08%     
==========================================
  Files         134      135       +1     
  Lines       18084    18110      +26     
==========================================
+ Hits        14272    14279       +7     
- Misses       3341     3358      +17     
- Partials      471      473       +2

Flag	Coverage Δ
unittests	`78.84% <39.13%> (-0.08%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

abhishek10004 · 2025-04-30T06:47:16Z

You would have to ensure that all control client API calls handle billing project. I tried getFolder call and it failed with the same error.

gargnitingoogle · 2025-04-30T07:02:12Z

This PR also needs changes for other APIs called through storage-control-client, so marking this draft for now. Will have to rethink this change.

gargnitingoogle · 2025-05-06T19:20:36Z

For some weird reason, this change is failing emulator_tests consistently (with the error "Failed to mount GCSFuse: cannot mount gcsfuse: exit status 1"), even though those tests don't pass --billing-project or use requester-pays buckets in any way. So, it's not sure what's wrong with those tests or this change.

gargnitingoogle · 2025-05-26T11:23:15Z

Have left a couple of minor comments.

Also, you mentioned the following in your PR description:

Moving the interface StorageControlClient from package internal/storage to internal/storage/storageutil to avoid cyclicity in package importing

I don't see this in the changes. Could you please check & update the description.

Yeah, my bad. That was an outdated description from a previous iteration of the change, not needed anymore.

This reverts commit 62ddbce.

This reverts commit 74fc891.

abhishek10004

LGTM

internal/storage/storage_handle.go

kislaykishore requested review from a team and Tulsishah and removed request for a team April 29, 2025 11:59

gargnitingoogle added the execute-integration-tests Run only integration tests label Apr 29, 2025

gargnitingoogle marked this pull request as ready for review April 29, 2025 11:59

gargnitingoogle requested a review from a team as a code owner April 29, 2025 11:59

gargnitingoogle requested a review from ashmeenkaur April 29, 2025 11:59

gargnitingoogle marked this pull request as draft April 29, 2025 16:46

gargnitingoogle marked this pull request as ready for review April 29, 2025 17:10

gargnitingoogle force-pushed the gargnitin/fix-requester-pays-bucket/v1 branch from ee64d6d to 74fc891 Compare April 29, 2025 17:12

gargnitingoogle removed the execute-integration-tests Run only integration tests label Apr 30, 2025

gargnitingoogle requested review from abhishek10004 and charith87 and removed request for Tulsishah and ashmeenkaur April 30, 2025 05:25

gargnitingoogle marked this pull request as draft April 30, 2025 07:01

gargnitingoogle force-pushed the gargnitin/fix-requester-pays-bucket/v1 branch from d4c0001 to e553e70 Compare April 30, 2025 09:59

gargnitingoogle added the execute-integration-tests Run only integration tests label Apr 30, 2025

gargnitingoogle changed the title ~~Pass "x-goog-user-project" to GetStorageLayout call to support requester-pays buckets~~ Pass "x-goog-user-project" in every call through storage-control-client Apr 30, 2025

gargnitingoogle changed the title ~~Pass "x-goog-user-project" in every call through storage-control-client~~ Pass billing-project in every gcs call through storage-control-client Apr 30, 2025

gargnitingoogle removed the execute-integration-tests Run only integration tests label Apr 30, 2025

gargnitingoogle changed the title ~~Pass billing-project in every gcs call through storage-control-client~~ Fix broken support for requester-pays buckets May 1, 2025

gargnitingoogle mentioned this pull request May 2, 2025

Requester pay bucket cannot be mounted even with billing project provided #3111

Closed

gargnitingoogle force-pushed the gargnitin/fix-requester-pays-bucket/v1 branch from 4d13e6e to 1df5cca Compare May 6, 2025 06:46

gargnitingoogle marked this pull request as ready for review May 6, 2025 08:18

gargnitingoogle added the execute-integration-tests Run only integration tests label May 6, 2025

gargnitingoogle added 16 commits May 27, 2025 08:35

pass x-goog-user-project to GetStorageLayout call

c502661

dont pass empty billingProject to GetStorageLayout

65c2643

Revert "dont pass empty billingProject to GetStorageLayout"

fa74554

This reverts commit 62ddbce.

Revert "pass x-goog-user-project to GetStorageLayout call"

e5d16b3

This reverts commit 74fc891.

[unstable] Use biling-project in control-client: part 1

1f49b1f

Use biling-project in control-client: part 2

300d84b

fix header

482b710

fix lint errors

c742d2d

empty commit

eae90bc

reverted all prev commits at once

f179e70

Add cleaner, shorter code

9d3d197

add unit test

db4475c

empty commit

c336f5a

Fix failure in emulator_tests

0dd22cb

dont pass billing-project in RenameFolder calll LRO

8bdca58

address review comments

68d3889

gargnitingoogle force-pushed the gargnitin/fix-requester-pays-bucket/v1 branch from d78fb90 to 68d3889 Compare May 27, 2025 08:58

gargnitingoogle requested a review from abhishek10004 May 27, 2025 09:01

abhishek10004 previously approved these changes May 27, 2025

View reviewed changes

charith87 reviewed May 27, 2025

View reviewed changes

internal/storage/storage_handle.go Outdated Show resolved Hide resolved

address review comments

82ce8dc

gargnitingoogle dismissed abhishek10004’s stale review via 82ce8dc May 27, 2025 11:59

gargnitingoogle requested review from abhishek10004 and charith87 May 27, 2025 14:33

charith87 approved these changes May 28, 2025

View reviewed changes

gargnitingoogle merged commit ec8022e into master May 28, 2025
13 checks passed

gargnitingoogle deleted the gargnitin/fix-requester-pays-bucket/v1 branch May 28, 2025 09:46

BrewTestBot mentioned this pull request Jun 12, 2025

gcsfuse 3.0.0 Homebrew/homebrew-core#226707

Merged

gromgit mentioned this pull request Jun 12, 2025

gcsfuse-mac 3.0.0 gromgit/homebrew-fuse#135

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix broken support for requester-pays buckets #3256

Fix broken support for requester-pays buckets #3256

Uh oh!

gargnitingoogle commented Apr 29, 2025 •

edited

Loading

Uh oh!

codecov bot commented Apr 29, 2025 •

edited

Loading

Uh oh!

abhishek10004 commented Apr 30, 2025

Uh oh!

gargnitingoogle commented Apr 30, 2025

Uh oh!

gargnitingoogle commented May 6, 2025 •

edited

Loading

Uh oh!

gargnitingoogle commented May 26, 2025 •

edited

Loading

Uh oh!

abhishek10004 left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Fix broken support for requester-pays buckets #3256

Fix broken support for requester-pays buckets #3256

Uh oh!

Conversation

gargnitingoogle commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Link to the issue in case of a bug fix.

Testing details

Any backward incompatible change? If so, please explain.

Uh oh!

codecov bot commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

abhishek10004 commented Apr 30, 2025

Uh oh!

gargnitingoogle commented Apr 30, 2025

Uh oh!

gargnitingoogle commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gargnitingoogle commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abhishek10004 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gargnitingoogle commented Apr 29, 2025 •

edited

Loading

codecov bot commented Apr 29, 2025 •

edited

Loading

gargnitingoogle commented May 6, 2025 •

edited

Loading

gargnitingoogle commented May 26, 2025 •

edited

Loading