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 d6302bae94..f4cf8f2be7 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 @@ -573,7 +573,7 @@ private boolean pathResolvesToId(final long zoneId, final String zonePath) return false; } INode lastINode = null; - if (inode.getParent() != null || inode.isRoot()) { + if (INode.isValidAbsolutePath(zonePath)) { INodesInPath iip = dir.getINodesInPath(zonePath, 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 874563db9f..34bfe10a59 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 @@ -783,8 +783,17 @@ public static String[] getPathNames(String path) { 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 bf02db3a09..870023b1dc 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 @@ -1867,4 +1867,51 @@ public void testEncryptedReadWriteUsingDiffKeyProvider() throws Exception { // 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 728e15bb72..fecbbfa978 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 void testListEncryptionZonesForRoot() throws Exception{ 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 a6980aac21..c109442dcc 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/* @@ -683,5 +684,36 @@ + + + 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* + + +