Return-Path: X-Original-To: apmail-hadoop-common-commits-archive@www.apache.org Delivered-To: apmail-hadoop-common-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 641C610E9D for ; Fri, 21 Nov 2014 19:01:45 +0000 (UTC) Received: (qmail 20930 invoked by uid 500); 21 Nov 2014 19:01:45 -0000 Delivered-To: apmail-hadoop-common-commits-archive@hadoop.apache.org Received: (qmail 20863 invoked by uid 500); 21 Nov 2014 19:01:45 -0000 Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: common-dev@hadoop.apache.org Delivered-To: mailing list common-commits@hadoop.apache.org Received: (qmail 20854 invoked by uid 99); 21 Nov 2014 19:01:45 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 21 Nov 2014 19:01:45 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id D0FE09AE38E; Fri, 21 Nov 2014 19:01:44 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: wheat9@apache.org To: common-commits@hadoop.apache.org Message-Id: <5f6b85eee2224eb18bc60102e21af943@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: hadoop git commit: HDFS-7420. Delegate permission checks to FSDirectory. Contributed by Haohui Mai. Date: Fri, 21 Nov 2014 19:01:44 +0000 (UTC) Repository: hadoop Updated Branches: refs/heads/trunk 3114d4731 -> 23dacb389 HDFS-7420. Delegate permission checks to FSDirectory. Contributed by Haohui Mai. Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/23dacb38 Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/23dacb38 Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/23dacb38 Branch: refs/heads/trunk Commit: 23dacb38924e3ed6a456b1c526e71e13e3c8f30d Parents: 3114d47 Author: Haohui Mai Authored: Fri Nov 21 11:01:14 2014 -0800 Committer: Haohui Mai Committed: Fri Nov 21 11:01:14 2014 -0800 ---------------------------------------------------------------------- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 2 + .../hdfs/server/namenode/FSDirectory.java | 84 +++++++++++++++++++- .../hdfs/server/namenode/FSNamesystem.java | 37 +++------ .../namenode/TestFSPermissionChecker.java | 2 +- 4 files changed, 95 insertions(+), 30 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/23dacb38/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index db3d010..8ce7d8c 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -383,6 +383,8 @@ Release 2.7.0 - UNRELEASED HDFS-7415. Move FSNameSystem.resolvePath() to FSDirectory. (wheat9) + HDFS-7420. Delegate permission checks to FSDirectory. (wheat9) + OPTIMIZATIONS BUG FIXES http://git-wip-us.apache.org/repos/asf/hadoop/blob/23dacb38/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java index d8f4f75..84e5028 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirectory.java @@ -52,6 +52,7 @@ import org.apache.hadoop.fs.XAttr; import org.apache.hadoop.fs.XAttrSetFlag; import org.apache.hadoop.fs.permission.AclEntry; import org.apache.hadoop.fs.permission.AclStatus; +import org.apache.hadoop.fs.permission.FsAction; import org.apache.hadoop.fs.permission.FsPermission; import org.apache.hadoop.fs.permission.PermissionStatus; import org.apache.hadoop.hdfs.DFSConfigKeys; @@ -96,6 +97,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import org.apache.hadoop.security.AccessControlException; +import org.apache.hadoop.security.UserGroupInformation; /** * Both FSDirectory and FSNamesystem manage the state of the namespace. @@ -152,6 +154,8 @@ public class FSDirectory implements Closeable { private final ReentrantReadWriteLock dirLock; private final boolean isPermissionEnabled; + private final String fsOwnerShortUserName; + private final String supergroup; // utility methods to acquire and release read lock and write lock void readLock() { @@ -195,13 +199,19 @@ public class FSDirectory implements Closeable { */ private final NameCache nameCache; - FSDirectory(FSNamesystem ns, Configuration conf) { + FSDirectory(FSNamesystem ns, Configuration conf) throws IOException { this.dirLock = new ReentrantReadWriteLock(true); // fair rootDir = createRoot(ns); inodeMap = INodeMap.newInstance(rootDir); this.isPermissionEnabled = conf.getBoolean( DFSConfigKeys.DFS_PERMISSIONS_ENABLED_KEY, DFSConfigKeys.DFS_PERMISSIONS_ENABLED_DEFAULT); + this.fsOwnerShortUserName = + UserGroupInformation.getCurrentUser().getShortUserName(); + this.supergroup = conf.get( + DFSConfigKeys.DFS_PERMISSIONS_SUPERUSERGROUP_KEY, + DFSConfigKeys.DFS_PERMISSIONS_SUPERUSERGROUP_DEFAULT); + int configuredLimit = conf.getInt( DFSConfigKeys.DFS_LIST_LIMIT, DFSConfigKeys.DFS_LIST_LIMIT_DEFAULT); this.lsLimit = configuredLimit>0 ? @@ -3331,4 +3341,76 @@ public class FSDirectory implements Closeable { } return inodesInPath; } + + FSPermissionChecker getPermissionChecker() + throws AccessControlException { + try { + return new FSPermissionChecker(fsOwnerShortUserName, supergroup, + NameNode.getRemoteUser()); + } catch (IOException ioe) { + throw new AccessControlException(ioe); + } + } + + void checkOwner(FSPermissionChecker pc, String path) + throws AccessControlException, UnresolvedLinkException { + checkPermission(pc, path, true, null, null, null, null); + } + + void checkPathAccess(FSPermissionChecker pc, String path, + FsAction access) + throws AccessControlException, UnresolvedLinkException { + checkPermission(pc, path, false, null, null, access, null); + } + void checkParentAccess( + FSPermissionChecker pc, String path, FsAction access) + throws AccessControlException, UnresolvedLinkException { + checkPermission(pc, path, false, null, access, null, null); + } + + void checkAncestorAccess( + FSPermissionChecker pc, String path, FsAction access) + throws AccessControlException, UnresolvedLinkException { + checkPermission(pc, path, false, access, null, null, null); + } + + void checkTraverse(FSPermissionChecker pc, String path) + throws AccessControlException, UnresolvedLinkException { + checkPermission(pc, path, false, null, null, null, null); + } + + /** + * Check whether current user have permissions to access the path. For more + * details of the parameters, see + * {@link FSPermissionChecker#checkPermission}. + */ + private void checkPermission( + FSPermissionChecker pc, String path, boolean doCheckOwner, + FsAction ancestorAccess, FsAction parentAccess, FsAction access, + FsAction subAccess) + throws AccessControlException, UnresolvedLinkException { + checkPermission(pc, path, doCheckOwner, ancestorAccess, + parentAccess, access, subAccess, false, true); + } + + /** + * Check whether current user have permissions to access the path. For more + * details of the parameters, see + * {@link FSPermissionChecker#checkPermission}. + */ + void checkPermission( + FSPermissionChecker pc, String path, boolean doCheckOwner, + FsAction ancestorAccess, FsAction parentAccess, FsAction access, + FsAction subAccess, boolean ignoreEmptyDir, boolean resolveLink) + throws AccessControlException, UnresolvedLinkException { + if (!pc.isSuperUser()) { + readLock(); + try { + pc.checkPermission(path, this, doCheckOwner, ancestorAccess, + parentAccess, access, subAccess, ignoreEmptyDir, resolveLink); + } finally { + readUnlock(); + } + } + } } http://git-wip-us.apache.org/repos/asf/hadoop/blob/23dacb38/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java index 1cefd4e..582b620 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSNamesystem.java @@ -404,7 +404,6 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, static int BLOCK_DELETION_INCREMENT = 1000; private final boolean isPermissionEnabled; private final UserGroupInformation fsOwner; - private final String fsOwnerShortUserName; private final String supergroup; private final boolean standbyShouldCheckpoint; @@ -777,7 +776,6 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, DFS_STORAGE_POLICY_ENABLED_DEFAULT); this.fsOwner = UserGroupInformation.getCurrentUser(); - this.fsOwnerShortUserName = fsOwner.getShortUserName(); this.supergroup = conf.get(DFS_PERMISSIONS_SUPERUSERGROUP_KEY, DFS_PERMISSIONS_SUPERUSERGROUP_DEFAULT); this.isPermissionEnabled = conf.getBoolean(DFS_PERMISSIONS_ENABLED_KEY, @@ -3922,11 +3920,7 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, private FSPermissionChecker getPermissionChecker() throws AccessControlException { - try { - return new FSPermissionChecker(fsOwnerShortUserName, supergroup, getRemoteUser()); - } catch (IOException ioe) { - throw new AccessControlException(ioe); - } + return dir.getPermissionChecker(); } /** @@ -6411,13 +6405,13 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, private void checkOwner(FSPermissionChecker pc, String path) throws AccessControlException, UnresolvedLinkException { - checkPermission(pc, path, true, null, null, null, null); + dir.checkOwner(pc, path); } private void checkPathAccess(FSPermissionChecker pc, String path, FsAction access) throws AccessControlException, UnresolvedLinkException { - checkPermission(pc, path, false, null, null, access, null); + dir.checkPathAccess(pc, path, access); } private void checkUnreadableBySuperuser(FSPermissionChecker pc, @@ -6438,18 +6432,18 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, private void checkParentAccess(FSPermissionChecker pc, String path, FsAction access) throws AccessControlException, UnresolvedLinkException { - checkPermission(pc, path, false, null, access, null, null); + dir.checkParentAccess(pc, path, access); } private void checkAncestorAccess(FSPermissionChecker pc, String path, FsAction access) throws AccessControlException, UnresolvedLinkException { - checkPermission(pc, path, false, access, null, null, null); + dir.checkAncestorAccess(pc, path, access); } private void checkTraverse(FSPermissionChecker pc, String path) throws AccessControlException, UnresolvedLinkException { - checkPermission(pc, path, false, null, null, null, null); + dir.checkTraverse(pc, path); } @Override @@ -6470,30 +6464,17 @@ public class FSNamesystem implements Namesystem, FSNamesystemMBean, String path, boolean doCheckOwner, FsAction ancestorAccess, FsAction parentAccess, FsAction access, FsAction subAccess) throws AccessControlException, UnresolvedLinkException { - checkPermission(pc, path, doCheckOwner, ancestorAccess, + checkPermission(pc, path, doCheckOwner, ancestorAccess, parentAccess, access, subAccess, false, true); } - /** - * Check whether current user have permissions to access the path. For more - * details of the parameters, see - * {@link FSPermissionChecker#checkPermission}. - */ private void checkPermission(FSPermissionChecker pc, String path, boolean doCheckOwner, FsAction ancestorAccess, FsAction parentAccess, FsAction access, FsAction subAccess, boolean ignoreEmptyDir, boolean resolveLink) throws AccessControlException, UnresolvedLinkException { - if (!pc.isSuperUser()) { - waitForLoadingFSImage(); - readLock(); - try { - pc.checkPermission(path, dir, doCheckOwner, ancestorAccess, - parentAccess, access, subAccess, ignoreEmptyDir, resolveLink); - } finally { - readUnlock(); - } - } + dir.checkPermission(pc, path, doCheckOwner, ancestorAccess, parentAccess, + access, subAccess, ignoreEmptyDir, resolveLink); } /** http://git-wip-us.apache.org/repos/asf/hadoop/blob/23dacb38/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java ---------------------------------------------------------------------- diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java index c0e1e05..7f23af4 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSPermissionChecker.java @@ -75,7 +75,7 @@ public class TestFSPermissionChecker { private INodeDirectory inodeRoot; @Before - public void setUp() { + public void setUp() throws IOException { Configuration conf = new Configuration(); FSNamesystem fsn = mock(FSNamesystem.class); doAnswer(new Answer() {