From 2cf90a2c338497a466bbad9e83966033bf14bfb7 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. --- 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 c38b92ad14a..07a78c21889 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -627,6 +627,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 @@ class FSDirAclOp { 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 @@ class FSDirAclOp { } 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 @@ class FSDirAclOp { 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 9d08d4e3b4d..2ae5e033df3 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 @@ -823,7 +823,8 @@ public class FSEditLogLoader { } 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 abstract class FSAclBaseTest { } @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 abstract class FSAclBaseTest { 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 abstract class FSAclBaseTest { 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 abstract class FSAclBaseTest { 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 abstract class FSAclBaseTest { 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 abstract class FSAclBaseTest { 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 abstract class FSAclBaseTest { 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 @@ public abstract class FSAclBaseTest { * @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(); } /**