Repository navigation
HDDS-17361. S3 Gateway can set another user as the owner of a new key - #11707
Merged
Merged
Conversation
S3 Gateway shares one RpcClient between all request threads. RpcClient kept the S3 user of the latest request in the shared field s3gUgi, and getRealUserInfo() returned it as the owner for new keys, files, directories and multipart uploads. A concurrent request from another user could therefore change the owner of a key. Create the UserGroupInformation from the thread-local S3Auth when it is needed instead, as createBucket() and getDEK() already do.
smengcl
approved these changes
Oct 23, 2026
ivandika3
reviewed
Oct 23, 2026
ivandika3
left a comment
ivandika3
left a comment
Contributor
There was a problem hiding this comment.
@peterxcli Thanks for the fix. Is it possible to make s3gUgi to be ThreadLocal instead? The argument of the review #4188 (comment) is that UGI creation is wasteful.
Co-authored-by: Siyao Meng <[email protected]>
peterxcli
commented
Oct 24, 2026
| public void setThreadLocalS3Auth( | ||
| S3Auth ozoneSharedSecretAuth) { | ||
| ozoneManagerClient.setThreadLocalS3Auth(ozoneSharedSecretAuth); | ||
| this.s3gUgi = UserGroupInformation.createRemoteUser(getThreadLocalS3Auth().getUserPrincipal()); |
Member
Author
There was a problem hiding this comment.
this removal actually help to reduce ugi creation, as GET, HEAD, list and delete request dont call getRealUserInfo()
peterxcli
marked this pull request as ready for review
October 24, 2026 18:48
ivandika3
approved these changes
Oct 25, 2026
ivandika3
left a comment
ivandika3
left a comment
Contributor
There was a problem hiding this comment.
Thanks @peterxcli , LGTM +1.
Member
Author
|
Thanks @ivandika3 @smengcl! |
errose28
added a commit
to errose28/ozone
that referenced
this pull request
Oct 27, 2026
* master: (114 commits) HDDS-17411. Show one initialization notice on the Namespace Usage page (apache#11737) HDDS-17375. Create new submodule ozone-integration-test-tools (apache#11754) HDDS-17452. Introduce ClientProtocol delegation for S3 Gateway (apache#11757) HDDS-16661. Validate unsigned S3 checksum trailer completion and framing (apache#11586) HDDS-17450. Speed up TestDirectoryDeletingService (apache#11765) HDDS-17308. ListObjectsV2 Incorrectly Filters CommonPrefixes When StartAfter Is Before the Matching Prefixes (apache#11715). Clamp Echo sleep time and response payload size. (apache#44) HDDS-17433. Add a freon command for positioned reads on shared input streams (apache#11741) HDDS-17239. Initialize usage counters on bucket create (apache#11742) HDDS-17357. Initialize Netty in background when Ozone client is created (apache#11726) HDDS-17426. Disable ozone.tracing.client.application-aware in acceptance tests (apache#11758) HDDS-16737. Share OM follower read logic between the Hadoop RPC and gRPC clients (apache#11418) HDDS-17446. Copy datanode container set in NodeStateMap.updateNode only once (apache#11746) HDDS-17454. Bump asm to 9.11.1 (apache#11750) HDDS-17453. Bump jackson to 2.21.8 (apache#11749) HDDS-17376. Scale repeated-defrag footprint tolerance with the number of OM tables (apache#11708) HDDS-17380. Fix deadlock when BackgroundContainerDataScanner shutdown during volume failure (apache#11729) HDDS-17251. List open multipart uploads in ListOpenFiles (apache#11739) HDDS-17361. S3 Gateway can set another user as the owner of a new key (apache#11707) HDDS-17238. Initialize refCount and usedNamespace on volume create (apache#11720) ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Background
The S3 Gateway serves all users with one
RpcClient. Each request thread sets its ownS3Authas a thread-local, andRpcClientuses that S3 user as the owner of new keys, files, directories and multipart uploads.Problem
RpcClient#setThreadLocalS3Authalso stores the user in the fields3gUgi, which HDDS-7791 added. All request threads share this field, andgetRealUserInfo()returns it whenever the calling thread has anS3Auth. OM stores the owner that the client sends.Take two users, Alice and Bob, on the same S3 Gateway. Alice uploads an object. While the gateway handles the upload, Bob sends any request, for example ListObjects. Bob's request overwrites
s3gUgi, so Alice's new key is stored with Bob as its owner.ListObjects,ozone sh key infoand ofs then show Bob as the owner of Alice's key.In a stress run on current master with one S3 Gateway, two users write from 8 threads each and a third user only lists objects. 330 of the 960 new keys get the wrong owner, and 236 of them get the user who only lists.
Access checks don't use the key owner, so this doesn't change who can read or write a key.
Changes
s3gUgi.getRealUserInfo()creates theUserGroupInformationfrom theS3Authof the calling thread.createBucket()andgetDEK()already do the same.setThreadLocalS3Auth()no longer creates aUserGroupInformation. It used to do that once or twice for every S3 request. Now one is created only when a write needs the owner.What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-17361
How was this patch tested?
New unit test
TestRpcClient#testKeyOwnerComesFromS3AuthOfCallingThread. One thread sets theS3Authofalice, a second thread then sets theS3Authofbob, and the first thread creates a key. The test checks that theCreateKeyrequest carriesaliceas the owner. Without the fix, it fails withexpected: <[alice]> but was: <[bob]>.