From 858715d054e38fa98add5df8fc089737f31a0218 Mon Sep 17 00:00:00 2001 From: Xiao Chen Date: Wed, 30 Aug 2017 09:42:04 -0700 Subject: [PATCH] HDFS-12336. Listing encryption zones still fails when deleted EZ is not a direct child of snapshottable directory. Contributed by Wellington Chevreuil. --- .../namenode/EncryptionZoneManager.java | 2 +- .../hadoop/hdfs/server/namenode/INode.java | 11 ++++- .../hadoop/hdfs/TestEncryptionZones.java | 47 +++++++++++++++++++ .../namenode/TestEncryptionZoneManager.java | 26 ++++++++++ .../src/test/resources/testCryptoConf.xml | 33 +++++++++++++ 5 files changed, 117 insertions(+), 2 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java index 3e670cea1c1..40bc36a153a 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java @@ -373,7 +373,7 @@ public class EncryptionZoneManager { final String pathName = getFullPathName(ezi); INode inode = dir.getInode(ezi.getINodeId()); INode lastINode = null; - if (inode.getParent() != null || inode.isRoot()) { + if (INode.isValidAbsolutePath(pathName)) { INodesInPath iip = dir.getINodesInPath(pathName, DirOp.READ_LINK); lastINode = iip.getLastINode(); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java index fcae42ef977..91b11548d27 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/INode.java @@ -782,8 +782,17 @@ public abstract class INode implements INodeAttributes, Diff.Element { return StringUtils.split(path, Path.SEPARATOR_CHAR); } + /** + * Verifies if the path informed is a valid absolute path. + * @param path the absolute path to validate. + * @return true if the path is valid. + */ + static boolean isValidAbsolutePath(final String path){ + return path != null && path.startsWith(Path.SEPARATOR); + } + private static void checkAbsolutePath(final String path) { - if (path == null || !path.startsWith(Path.SEPARATOR)) { + if (!isValidAbsolutePath(path)) { throw new AssertionError("Absolute path required, but got '" + path + "'"); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptionZones.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptionZones.java index 195f3bd6099..1dda8b3dc35 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptionZones.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestEncryptionZones.java @@ -1885,4 +1885,51 @@ public class TestEncryptionZones { // Read them back in and compare byte-by-byte verifyFilesEqual(fs, baseFile, encFile1, len); } + + /** + * Test listing encryption zones after zones had been deleted, + * but still exist under snapshots. This test first moves EZs + * to trash folder, so that an inodereference is created for the EZ, + * then it removes the EZ from trash folder to emulate condition where + * the EZ inode will not be complete. + */ + @Test + public void testListEncryptionZonesWithSnapshots() throws Exception { + final Path snapshottable = new Path("/zones"); + final Path zoneDirectChild = new Path(snapshottable, "zone1"); + final Path snapshottableChild = new Path(snapshottable, "child"); + final Path zoneSubChild = new Path(snapshottableChild, "zone2"); + fsWrapper.mkdir(zoneDirectChild, FsPermission.getDirDefault(), + true); + fsWrapper.mkdir(zoneSubChild, FsPermission.getDirDefault(), + true); + dfsAdmin.allowSnapshot(snapshottable); + dfsAdmin.createEncryptionZone(zoneDirectChild, TEST_KEY, NO_TRASH); + dfsAdmin.createEncryptionZone(zoneSubChild, TEST_KEY, NO_TRASH); + final Path snap1 = fs.createSnapshot(snapshottable, "snap1"); + Configuration clientConf = new Configuration(conf); + clientConf.setLong(FS_TRASH_INTERVAL_KEY, 1); + FsShell shell = new FsShell(clientConf); + //will "trash" the zone under subfolder of snapshottable directory + verifyShellDeleteWithTrash(shell, snapshottableChild); + //permanently remove zone under subfolder of snapshottable directory + fsWrapper.delete(shell.getCurrentTrashDir(snapshottableChild), + true); + final RemoteIterator it = dfsAdmin.listEncryptionZones(); + boolean match = false; + while (it.hasNext()) { + EncryptionZone ez = it.next(); + assertNotEquals("EncryptionZone " + zoneSubChild.toString() + + " should not be listed.", + ez.getPath(), zoneSubChild.toString()); + } + //will "trash" the zone direct child of snapshottable directory + verifyShellDeleteWithTrash(shell, zoneDirectChild); + //permanently remove zone direct child of snapshottable directory + fsWrapper.delete(shell.getCurrentTrashDir(zoneDirectChild), true); + assertFalse("listEncryptionZones should not return anything, " + + "since both EZs were deleted.", + dfsAdmin.listEncryptionZones().hasNext()); + } + } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEncryptionZoneManager.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEncryptionZoneManager.java index 728e15bb729..fecbbfa9786 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEncryptionZoneManager.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEncryptionZoneManager.java @@ -135,4 +135,30 @@ public class TestEncryptionZoneManager { assertEquals(0L, result.get(0).getId()); assertEquals("/", result.get(0).getPath()); } + + @Test + public void testListEncryptionZonesSubDirInvalid() throws Exception{ + INodeDirectory thirdINode = new INodeDirectory(3L, "third".getBytes(), + defaultPermission, System.currentTimeMillis()); + when(this.mockedDir.getInode(3L)).thenReturn(thirdINode); + //sets "second" as parent + thirdINode.setParent(this.secondINode); + this.ezManager = new EncryptionZoneManager(mockedDir, new Configuration()); + this.ezManager.addEncryptionZone(1L, CipherSuite.AES_CTR_NOPADDING, + CryptoProtocolVersion.ENCRYPTION_ZONES, "test_key"); + this.ezManager.addEncryptionZone(3L, CipherSuite.AES_CTR_NOPADDING, + CryptoProtocolVersion.ENCRYPTION_ZONES, "test_key"); + // sets root as proper parent for firstINode only, + // leave secondINode with no parent + this.firstINode.setParent(rootINode); + when(mockedDir.getINodesInPath("/first", DirOp.READ_LINK)). + thenReturn(mockedINodesInPath); + when(mockedINodesInPath.getLastINode()). + thenReturn(firstINode); + BatchedListEntries result = ezManager. + listEncryptionZones(0); + assertEquals(1, result.size()); + assertEquals(1L, result.get(0).getId()); + assertEquals("/first", result.get(0).getPath()); + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testCryptoConf.xml b/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testCryptoConf.xml index f9bb29eb9e4..8ab79105753 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testCryptoConf.xml +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testCryptoConf.xml @@ -593,6 +593,7 @@ -listZones + -fs NAMENODE -deleteSnapshot /snapshotable snapshot1 -fs NAMENODE -rm -r /snapshotable -fs NAMENODE -rm -r .Trash/Current/* @@ -603,5 +604,37 @@ + + + Test adding two zones to a snapshotable directory; + The second zone is not a direct child of snapshottable directory; + Take snapshot, permanently delete second EZ, then list zones; + + -fs NAMENODE -rm -r .Trash/Current/* + -fs NAMENODE -mkdir /snapshotable + -fs NAMENODE -mkdir /snapshotable/test1 + -fs NAMENODE -mkdir /snapshotable/test1/test2 + -fs NAMENODE -mkdir /snapshotable/test3 + -fs NAMENODE -allowSnapshot /snapshotable + -createZone -path /snapshotable/test1/test2 -keyName myKey + -createZone -path /snapshotable/test3 -keyName myKey + -fs NAMENODE -createSnapshot /snapshotable snapshot1 + -fs NAMENODE -rm -r /snapshotable/test1 + -fs NAMENODE -rm -r .Trash/Current/* + -listZones + + + -fs NAMENODE -deleteSnapshot /snapshotable snapshot1 + -fs NAMENODE -rm -r /snapshotable + -fs NAMENODE -rm -r .Trash/Current/* + + + + RegexpAcrossOutputComparator + (/test3)\s*(myKey)\s* + + + +