HDDS-939. Add S3 access check to Ozone manager. Contributed by Ajay Kumar. #634
Conversation
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
bharatviswa504
requested changes
Mar 22, 2019
Contributor
There was a problem hiding this comment.
So instead of a md5Hex of Kerberos, we now store accessKey as original Kerberos user.
So that for Ozone S3, in OM when acl check happens, it will be a kerberos user. So, ACL check for ozone s3 happens. (Not sure if my understanding is completely correct here?)
But with this we have a issue, because internally when a bucket is created (S3 bucket), we consider volume name as awsaccessKeyID. With this, our volume name can have '/', '.' characters. The volume creation fails. (Because we do validate the name in RpcClient by calling verifyResourceName) We need to change the logic over there. Previously we don't see any issue because it md5Hex.
I think if the awsAccessKey will not have realm, if it has just name we shall not see the issue.
|
💔 -1 overall
This message was automatically generated. |
Contributor
That will not work, since there can be same user names in different domains who are completely different users in real life. |
Contributor
Thank You @anuengineer for info. |
Contributor
Author
@bharatviswa504 thanks for bringing this up. Updated PR to handle this by normalizing the userId if it is kerberos id. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
bharatviswa504
requested changes
Mar 23, 2019
| } | ||
|
|
||
| public static String getVolumeName(String userName) { | ||
| Objects.nonNull(userName); |
Contributor
There was a problem hiding this comment.
I think here, it should be requireNotNull(userName)
Contributor
Author
There was a problem hiding this comment.
thanks for catching this. fixed in new commit.
hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/util/OzoneS3Util.java
Show resolved
Hide resolved
Contributor
|
And also we can add few tests to verify the volumeName generated from the newly added function validates with the HddsUtils.verifyResourceName()? So, that in future if someone is changing the logic in this place, it can be caught immediately if it causes any issues. |
Contributor
Author
|
|
💔 -1 overall
This message was automatically generated. |
bharatviswa504
requested changes
Mar 25, 2019
Contributor
There was a problem hiding this comment.
Here we should call OzoneS3Util.getVolumeName()
Contributor
There was a problem hiding this comment.
Minor NIT: space after ,
Contributor
bharatviswa504
left a comment
bharatviswa504
left a comment
There was a problem hiding this comment.
Over all patch LGTM.
One minor NIT, I think Yetus will report that.
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Contributor
|
@ajayydv |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Contributor
Author
|
failure in TestOzoneRpcClient.testMultipartUpload seems unrelated, passes locally. |
Contributor
|
+1 LGTM. |
shanthoosh
pushed a commit
to shanthoosh/hadoop
that referenced
this pull request
Oct 15, 2019
…emDescriptor CollectionStream -> InMemoryInputDescriptor & InMemoryOutputDescriptor CollectionStreamSystemSpec -> InMemorySystemDescriptor Author: Sanil Jain <[email protected]> Reviewers: Prateek Maheshwari <[email protected]>, Cameron Lee <[email protected]> Closes apache#634 from Sanil15/SAMZA-1840
p-szucs
pushed a commit
to p-szucs/hadoop
that referenced
this pull request
Oct 22, 2025
…pache#634) This commit removes hardcoded 'source', 'target', and 'release' versions from the maven-compiler-plugin configuration. This change allows the build to automatically inherit the correct JDK version from the parent POM, ensuring consistency and alignment with project standards.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
No description provided.