HDFS-10711. Optimize FSPermissionChecker group membership check. Contributed by Daryn Sharp.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/2550371f Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/2550371f Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/2550371f Branch: refs/heads/HDFS-9806 Commit: 2550371f66c49fe0e40aadaa68744311270084ce Parents: 091dd19 Author: Kihwal Lee <kih...@apache.org> Authored: Fri Aug 19 09:12:17 2016 -0500 Committer: Kihwal Lee <kih...@apache.org> Committed: Fri Aug 19 09:12:17 2016 -0500 ---------------------------------------------------------------------- .../hdfs/server/namenode/FSDirAttrOp.java | 2 +- .../server/namenode/FSPermissionChecker.java | 23 ++++++-------------- 2 files changed, 8 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/2550371f/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java index e6d36b8..e19341c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAttrOp.java @@ -88,7 +88,7 @@ public class FSDirAttrOp { if (username != null && !pc.getUser().equals(username)) { throw new AccessControlException("Non-super user cannot change owner"); } - if (group != null && !pc.containsGroup(group)) { + if (group != null && !pc.isMemberOfGroup(group)) { throw new AccessControlException("User does not belong to " + group); } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/2550371f/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java index 726319f..c9b1c76 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSPermissionChecker.java @@ -17,10 +17,7 @@ */ package org.apache.hadoop.hdfs.server.namenode; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; -import java.util.Set; +import java.util.Collection; import java.util.Stack; import org.apache.commons.logging.Log; @@ -81,7 +78,7 @@ class FSPermissionChecker implements AccessControlEnforcer { private final UserGroupInformation callerUgi; private final String user; - private final Set<String> groups; + private final Collection<String> groups; private final boolean isSuper; private final INodeAttributeProvider attributeProvider; @@ -92,15 +89,13 @@ class FSPermissionChecker implements AccessControlEnforcer { this.fsOwner = fsOwner; this.supergroup = supergroup; this.callerUgi = callerUgi; - HashSet<String> s = - new HashSet<String>(Arrays.asList(callerUgi.getGroupNames())); - groups = Collections.unmodifiableSet(s); + this.groups = callerUgi.getGroups(); user = callerUgi.getShortUserName(); isSuper = user.equals(fsOwner) || groups.contains(supergroup); this.attributeProvider = attributeProvider; } - public boolean containsGroup(String group) { + public boolean isMemberOfGroup(String group) { return groups.contains(group); } @@ -108,10 +103,6 @@ class FSPermissionChecker implements AccessControlEnforcer { return user; } - public Set<String> getGroups() { - return groups; - } - public boolean isSuperUser() { return isSuper; } @@ -337,7 +328,7 @@ class FSPermissionChecker implements AccessControlEnforcer { final FsAction checkAction; if (getUser().equals(inode.getUserName())) { //user class checkAction = mode.getUserAction(); - } else if (getGroups().contains(inode.getGroupName())) { //group class + } else if (isMemberOfGroup(inode.getGroupName())) { //group class checkAction = mode.getGroupAction(); } else { //other class checkAction = mode.getOtherAction(); @@ -407,7 +398,7 @@ class FSPermissionChecker implements AccessControlEnforcer { // member of multiple groups that have entries that grant access, then // it doesn't matter which is chosen, so exit early after first match. String group = name == null ? inode.getGroupName() : name; - if (getGroups().contains(group)) { + if (isMemberOfGroup(group)) { FsAction masked = AclEntryStatusFormat.getPermission(entry).and( mode.getGroupAction()); if (masked.implies(access)) { @@ -470,7 +461,7 @@ class FSPermissionChecker implements AccessControlEnforcer { && mode.getUserAction().implies(access)) { return; } - if (getGroups().contains(pool.getGroupName()) + if (isMemberOfGroup(pool.getGroupName()) && mode.getGroupAction().implies(access)) { return; } --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
