peterxcli opened a new pull request, #11707: URL: https://github.com/apache/ozone/pull/11707
## What changes were proposed in this pull request? ### Background The S3 Gateway serves all users with one `RpcClient`. Each request thread sets its own `S3Auth` as a thread-local, and `RpcClient` uses that S3 user as the owner of new keys, files, directories and multipart uploads. ### Problem `RpcClient#setThreadLocalS3Auth` also stores the user in the field `s3gUgi`, which HDDS-7791 added. All request threads share this field, and `getRealUserInfo()` returns it whenever the calling thread has an `S3Auth`. 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 info` and 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 - Remove the shared field `s3gUgi`. - `getRealUserInfo()` creates the `UserGroupInformation` from the `S3Auth` of the calling thread. `createBucket()` and `getDEK()` already do the same. - `setThreadLocalS3Auth()` no longer creates a `UserGroupInformation`. 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 the `S3Auth` of `alice`, a second thread then sets the `S3Auth` of `bob`, and the first thread creates a key. The test checks that the `CreateKey` request carries `alice` as the owner. Without the fix, it fails with `expected: <[alice]> but was: <[bob]>`. - The existing owner tests `testKeyOwner` and `testMultipartUploadOwner` pass in `TestOzoneRpcClient` and in `TestSecureOzoneRpcClient` (block tokens and ACL checks on). - With the fix, the stress run above (MiniOzoneCluster, one S3 Gateway, AWS SDK v1) gets no wrong owners in 960 keys. It is not part of this PR, because it depends on timing. - `TestRpcClient`, `TestOzoneClient`, `TestOzoneECClient` and checkstyle pass locally. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
