From 54861246684e9468df9d2b68b9103099ab48cdc6 Mon Sep 17 00:00:00 2001 From: cnauroth Date: Mon, 22 Dec 2014 13:59:10 -0800 Subject: [PATCH] HDFS-7560. ACLs removed by removeDefaultAcl() will be back after NameNode restart/failover. Contributed by Vinayakumar B. (cherry picked from commit 2cf90a2c338497a466bbad9e83966033bf14bfb7) --- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 ++ .../hdfs/server/namenode/FSDirAclOp.java | 12 +++--- .../hdfs/server/namenode/FSEditLogLoader.java | 3 +- .../hdfs/server/namenode/FSAclBaseTest.java | 39 +++++++++++++++---- 4 files changed, 44 insertions(+), 13 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 997552df2c8..d2d67e40172 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -370,6 +370,9 @@ Release 2.7.0 - UNRELEASED HDFS-7557. Fix spacing for a few keys in DFSConfigKeys.java (Colin P.McCabe) + HDFS-7560. ACLs removed by removeDefaultAcl() will be back after NameNode + restart/failover. (Vinayakumar B via cnauroth) + Release 2.6.1 - UNRELEASED INCOMPATIBLE CHANGES diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAclOp.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAclOp.java index 7aaa21c99a1..dff1c2e337d 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAclOp.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirAclOp.java @@ -143,7 +143,7 @@ static HdfsFileStatus setAcl( try { iip = fsd.getINodesInPath4Write(src); fsd.checkOwner(pc, iip); - List newAcl = unprotectedSetAcl(fsd, src, aclSpec); + List newAcl = unprotectedSetAcl(fsd, src, aclSpec, false); fsd.getEditLog().logSetAcl(src, newAcl); } finally { fsd.writeUnlock(); @@ -185,7 +185,7 @@ static AclStatus getAclStatus( } static List unprotectedSetAcl( - FSDirectory fsd, String src, List aclSpec) + FSDirectory fsd, String src, List aclSpec, boolean fromEdits) throws IOException { assert fsd.hasWriteLock(); final INodesInPath iip = fsd.getINodesInPath4Write( @@ -199,9 +199,11 @@ static List unprotectedSetAcl( INode inode = FSDirectory.resolveLastINode(iip); int snapshotId = iip.getLatestSnapshotId(); - List existingAcl = AclStorage.readINodeLogicalAcl(inode); - List newAcl = AclTransformation.replaceAclEntries(existingAcl, - aclSpec); + List newAcl = aclSpec; + if (!fromEdits) { + List existingAcl = AclStorage.readINodeLogicalAcl(inode); + newAcl = AclTransformation.replaceAclEntries(existingAcl, aclSpec); + } AclStorage.updateINodeAcl(inode, newAcl, snapshotId); return newAcl; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java index 9d79ffae68c..c275ea53cab 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSEditLogLoader.java @@ -827,7 +827,8 @@ fsDir, renameReservedPathsOnUpgrade(timesOp.path, logVersion), } case OP_SET_ACL: { SetAclOp setAclOp = (SetAclOp) op; - FSDirAclOp.unprotectedSetAcl(fsDir, setAclOp.src, setAclOp.aclEntries); + FSDirAclOp.unprotectedSetAcl(fsDir, setAclOp.src, setAclOp.aclEntries, + true); break; } case OP_SET_XATTR: { diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSAclBaseTest.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSAclBaseTest.java index b0275e8c7fc..528dfffab5f 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSAclBaseTest.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/FSAclBaseTest.java @@ -426,7 +426,7 @@ public void testRemoveAclEntriesPathNotFound() throws IOException { } @Test - public void testRemoveDefaultAcl() throws IOException { + public void testRemoveDefaultAcl() throws Exception { FileSystem.mkdirs(fs, path, FsPermission.createImmutable((short)0750)); List aclSpec = Lists.newArrayList( aclEntry(ACCESS, USER, ALL), @@ -443,10 +443,15 @@ public void testRemoveDefaultAcl() throws IOException { aclEntry(ACCESS, GROUP, READ_EXECUTE) }, returned); assertPermission((short)010770); assertAclFeature(true); + // restart of the cluster + restartCluster(); + s = fs.getAclStatus(path); + AclEntry[] afterRestart = s.getEntries().toArray(new AclEntry[0]); + assertArrayEquals(returned, afterRestart); } @Test - public void testRemoveDefaultAclOnlyAccess() throws IOException { + public void testRemoveDefaultAclOnlyAccess() throws Exception { fs.create(path).close(); fs.setPermission(path, FsPermission.createImmutable((short)0640)); List aclSpec = Lists.newArrayList( @@ -463,10 +468,15 @@ public void testRemoveDefaultAclOnlyAccess() throws IOException { aclEntry(ACCESS, GROUP, READ_EXECUTE) }, returned); assertPermission((short)010770); assertAclFeature(true); + // restart of the cluster + restartCluster(); + s = fs.getAclStatus(path); + AclEntry[] afterRestart = s.getEntries().toArray(new AclEntry[0]); + assertArrayEquals(returned, afterRestart); } @Test - public void testRemoveDefaultAclOnlyDefault() throws IOException { + public void testRemoveDefaultAclOnlyDefault() throws Exception { FileSystem.mkdirs(fs, path, FsPermission.createImmutable((short)0750)); List aclSpec = Lists.newArrayList( aclEntry(DEFAULT, USER, "foo", ALL)); @@ -477,10 +487,15 @@ public void testRemoveDefaultAclOnlyDefault() throws IOException { assertArrayEquals(new AclEntry[] { }, returned); assertPermission((short)0750); assertAclFeature(false); + // restart of the cluster + restartCluster(); + s = fs.getAclStatus(path); + AclEntry[] afterRestart = s.getEntries().toArray(new AclEntry[0]); + assertArrayEquals(returned, afterRestart); } @Test - public void testRemoveDefaultAclMinimal() throws IOException { + public void testRemoveDefaultAclMinimal() throws Exception { FileSystem.mkdirs(fs, path, FsPermission.createImmutable((short)0750)); fs.removeDefaultAcl(path); AclStatus s = fs.getAclStatus(path); @@ -488,10 +503,15 @@ public void testRemoveDefaultAclMinimal() throws IOException { assertArrayEquals(new AclEntry[] { }, returned); assertPermission((short)0750); assertAclFeature(false); + // restart of the cluster + restartCluster(); + s = fs.getAclStatus(path); + AclEntry[] afterRestart = s.getEntries().toArray(new AclEntry[0]); + assertArrayEquals(returned, afterRestart); } @Test - public void testRemoveDefaultAclStickyBit() throws IOException { + public void testRemoveDefaultAclStickyBit() throws Exception { FileSystem.mkdirs(fs, path, FsPermission.createImmutable((short)01750)); List aclSpec = Lists.newArrayList( aclEntry(ACCESS, USER, ALL), @@ -508,6 +528,11 @@ public void testRemoveDefaultAclStickyBit() throws IOException { aclEntry(ACCESS, GROUP, READ_EXECUTE) }, returned); assertPermission((short)011770); assertAclFeature(true); + // restart of the cluster + restartCluster(); + s = fs.getAclStatus(path); + AclEntry[] afterRestart = s.getEntries().toArray(new AclEntry[0]); + assertArrayEquals(returned, afterRestart); } @Test(expected=FileNotFoundException.class) @@ -1137,9 +1162,7 @@ public void testSkipAclEnforcementPermsDisabled() throws Exception { assertFilePermissionDenied(fsAsDiana, DIANA, bruceFile); try { conf.setBoolean(DFSConfigKeys.DFS_PERMISSIONS_ENABLED_KEY, false); - destroyFileSystems(); restartCluster(); - initFileSystems(); assertFilePermissionGranted(fsAsDiana, DIANA, bruceFile); } finally { conf.setBoolean(DFSConfigKeys.DFS_PERMISSIONS_ENABLED_KEY, true); @@ -1404,10 +1427,12 @@ private void initFileSystems() throws Exception { * @throws Exception if restart fails */ private void restartCluster() throws Exception { + destroyFileSystems(); shutdown(); cluster = new MiniDFSCluster.Builder(conf).numDataNodes(1).format(false) .build(); cluster.waitActive(); + initFileSystems(); } /**