HDFS-15850. Superuser actions should be reported to external enforcers #2784
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
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
xiaoyuyao
reviewed
Mar 20, 2021
There was a problem hiding this comment.
Thanks @vivekratnavel for working on this. The patch LGTM overall, a few comments added inline. Can you also double check the Jenkins results and confirm if any of the test failures are related to this change?
There was a problem hiding this comment.
Can you rephrase the comments to make it clear what super user can do and what a non-super user owner can do w.r.t. SetOwner operation:
superuser: change owner to a different user, change owner group to any group
owner: can't change owner to a different user but can change owner group to different group that the user belongs to.
...fs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
should we add the cache pool name as part of the context info as normal HDFS audit has that?
There was a problem hiding this comment.
That's a good suggestion @xiaoyuyao. We cannot add the cache pool name for "addCachePool()" method since the superuser check needs to happen before adding the cache pool and we get the name only after it's added. But, I will set the cache pool name in context for modify and remove methods.
There was a problem hiding this comment.
Can we get it from other calls after Line 7670?
String poolNameStr = "{poolName: " +
(req == null ? null : req.getPoolName()) + "}";
There was a problem hiding this comment.
Can we add path info as the context for better auditing?
There was a problem hiding this comment.
Can we add the zone as context info for better auditing?
...s-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Can we rephrase "Checks if the user belongs to superuser group." to "Checks if the user is a superuser or belongs to superuser group."
There was a problem hiding this comment.
can we pass the path parameter to add more context for better auditing?
|
Hi @xiaoyuyao, thanks for the review. I have updated the patch addressing all your comments. Please take another look. I also checked the unit test failures in the previous patch and none seemed to be related to my changes. I will check them again after CI finishes for the latest patch. |
|
💔 -1 overall
This message was automatically generated. |
xiaoyuyao
reviewed
Mar 22, 2021
There was a problem hiding this comment.
Thanks @vivekratnavel for the update. The latest patch LGTM, just have one minor comments.
There was a problem hiding this comment.
Can we get it from other calls after Line 7670?
String poolNameStr = "{poolName: " +
(req == null ? null : req.getPoolName()) + "}";
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
xiaoyuyao
reviewed
Mar 23, 2021
...s-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
Outdated
Show resolved
Hide resolved
xiaoyuyao
reviewed
Mar 23, 2021
...hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INodeAttributeProvider.java
Outdated
Show resolved
Hide resolved
xiaoyuyao
reviewed
Mar 23, 2021
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
xiaoyuyao
reviewed
Mar 29, 2021
There was a problem hiding this comment.
Thanks @vivekratnavel for the update. The latest patch LGTM, +1 pending CI.
...s-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java
Outdated
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
xiaoyuyao
reviewed
Mar 31, 2021
hadoop-hdfs-project/hadoop-hdfs/dev-support/findbugsExcludeFile.xml
Outdated
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
The unit test failures reported are not related to this patch. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
vivekratnavel
force-pushed
the
HDFS-15850
branch
from
d443432 to
1c6a130
Compare
|
Thanks @vivekratnavel for update. The latest change LGTM, +1 pending CI. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
Thanks @vivekratnavel for the update. The latest change LGTM, +1. |
jojochuang
pushed a commit
to jojochuang/hadoop
that referenced
this pull request
May 22, 2021
apache#2784) (cherry picked from commit c821008) Conflicts: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java Change-Id: I7843b7bdbbf43956cf1ceb47fc94cb99ae901468
kiran-maturi
pushed a commit
to kiran-maturi/hadoop
that referenced
this pull request
Nov 24, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
https://issues.apache.org/jira/browse/HDFS-15850