HDFS-10711. Optimize FSPermissionChecker group membership check. Contributed by Daryn Sharp.

This commit is contained in:
Kihwal Lee 2016-08-19 09:12:17 -05:00
parent 091dd19e86
commit 2550371f66
2 changed files with 8 additions and 17 deletions

View File

@ -88,7 +88,7 @@ static HdfsFileStatus setOwner(
if (username != null && !pc.getUser().equals(username)) { if (username != null && !pc.getUser().equals(username)) {
throw new AccessControlException("Non-super user cannot change owner"); 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); throw new AccessControlException("User does not belong to " + group);
} }
} }

View File

@ -17,10 +17,7 @@
*/ */
package org.apache.hadoop.hdfs.server.namenode; package org.apache.hadoop.hdfs.server.namenode;
import java.util.Arrays; import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.Stack; import java.util.Stack;
import org.apache.commons.logging.Log; import org.apache.commons.logging.Log;
@ -81,7 +78,7 @@ private String toAccessControlString(INodeAttributes inodeAttrib,
private final UserGroupInformation callerUgi; private final UserGroupInformation callerUgi;
private final String user; private final String user;
private final Set<String> groups; private final Collection<String> groups;
private final boolean isSuper; private final boolean isSuper;
private final INodeAttributeProvider attributeProvider; private final INodeAttributeProvider attributeProvider;
@ -92,15 +89,13 @@ private String toAccessControlString(INodeAttributes inodeAttrib,
this.fsOwner = fsOwner; this.fsOwner = fsOwner;
this.supergroup = supergroup; this.supergroup = supergroup;
this.callerUgi = callerUgi; this.callerUgi = callerUgi;
HashSet<String> s = this.groups = callerUgi.getGroups();
new HashSet<String>(Arrays.asList(callerUgi.getGroupNames()));
groups = Collections.unmodifiableSet(s);
user = callerUgi.getShortUserName(); user = callerUgi.getShortUserName();
isSuper = user.equals(fsOwner) || groups.contains(supergroup); isSuper = user.equals(fsOwner) || groups.contains(supergroup);
this.attributeProvider = attributeProvider; this.attributeProvider = attributeProvider;
} }
public boolean containsGroup(String group) { public boolean isMemberOfGroup(String group) {
return groups.contains(group); return groups.contains(group);
} }
@ -108,10 +103,6 @@ public String getUser() {
return user; return user;
} }
public Set<String> getGroups() {
return groups;
}
public boolean isSuperUser() { public boolean isSuperUser() {
return isSuper; return isSuper;
} }
@ -337,7 +328,7 @@ private boolean hasPermission(INodeAttributes inode, FsAction access) {
final FsAction checkAction; final FsAction checkAction;
if (getUser().equals(inode.getUserName())) { //user class if (getUser().equals(inode.getUserName())) { //user class
checkAction = mode.getUserAction(); checkAction = mode.getUserAction();
} else if (getGroups().contains(inode.getGroupName())) { //group class } else if (isMemberOfGroup(inode.getGroupName())) { //group class
checkAction = mode.getGroupAction(); checkAction = mode.getGroupAction();
} else { //other class } else { //other class
checkAction = mode.getOtherAction(); checkAction = mode.getOtherAction();
@ -407,7 +398,7 @@ private boolean hasAclPermission(INodeAttributes inode,
// member of multiple groups that have entries that grant access, then // member of multiple groups that have entries that grant access, then
// it doesn't matter which is chosen, so exit early after first match. // it doesn't matter which is chosen, so exit early after first match.
String group = name == null ? inode.getGroupName() : name; String group = name == null ? inode.getGroupName() : name;
if (getGroups().contains(group)) { if (isMemberOfGroup(group)) {
FsAction masked = AclEntryStatusFormat.getPermission(entry).and( FsAction masked = AclEntryStatusFormat.getPermission(entry).and(
mode.getGroupAction()); mode.getGroupAction());
if (masked.implies(access)) { if (masked.implies(access)) {
@ -470,7 +461,7 @@ public void checkPermission(CachePool pool, FsAction access)
&& mode.getUserAction().implies(access)) { && mode.getUserAction().implies(access)) {
return; return;
} }
if (getGroups().contains(pool.getGroupName()) if (isMemberOfGroup(pool.getGroupName())
&& mode.getGroupAction().implies(access)) { && mode.getGroupAction().implies(access)) {
return; return;
} }