From 892ee3f191f2349a7b67e5ce4401ec670cc60299 Mon Sep 17 00:00:00 2001 From: Xiao Chen Date: Thu, 8 Dec 2016 12:40:20 -0800 Subject: [PATCH] HDFS-11197. Listing encryption zones fails when deleting a EZ that is on a snapshotted directory. Contributed by Wellington Chevreuil. (cherry picked from commit 401c7318723d8d62c7fc29728f7f4e8d336b4d2f) --- .../namenode/EncryptionZoneManager.java | 8 +- .../apache/hadoop/cli/TestCryptoAdminCLI.java | 3 +- .../namenode/TestEncryptionZoneManager.java | 138 ++++++++++++++++++ .../src/test/resources/testCryptoConf.xml | 98 ++++++++++++- 4 files changed, 242 insertions(+), 5 deletions(-) create mode 100644 hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEncryptionZoneManager.java 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 323ebaba465..52c66c13dc6 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 @@ -372,8 +372,12 @@ public class EncryptionZoneManager { contain a reference INode. */ final String pathName = getFullPathName(ezi); - INodesInPath iip = dir.getINodesInPath(pathName, DirOp.READ_LINK); - INode lastINode = iip.getLastINode(); + INode inode = dir.getInode(ezi.getINodeId()); + INode lastINode = null; + if (inode.getParent() != null || inode.isRoot()) { + INodesInPath iip = dir.getINodesInPath(pathName, DirOp.READ_LINK); + lastINode = iip.getLastINode(); + } if (lastINode == null || lastINode.getId() != ezi.getINodeId()) { continue; } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/cli/TestCryptoAdminCLI.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/cli/TestCryptoAdminCLI.java index 050daec0735..b008219a9cc 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/cli/TestCryptoAdminCLI.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/cli/TestCryptoAdminCLI.java @@ -62,6 +62,7 @@ public class TestCryptoAdminCLI extends CLITestHelperDFS { conf.setClass(PolicyProvider.POLICY_PROVIDER_CONFIG, HDFSPolicyProvider.class, PolicyProvider.class); conf.setInt(DFSConfigKeys.DFS_REPLICATION_KEY, 1); + conf.setLong(CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY, 10); tmpDir = new File(System.getProperty("test.build.data", "target"), UUID.randomUUID().toString()).getAbsoluteFile(); @@ -127,7 +128,7 @@ public class TestCryptoAdminCLI extends CLITestHelperDFS { } private class TestConfigFileParserCryptoAdmin extends - CLITestHelper.TestConfigFileParser { + CLITestHelperDFS.TestConfigFileParserDFS { @Override public void endElement(String uri, String localName, String qName) throws SAXException { 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 new file mode 100644 index 00000000000..728e15bb729 --- /dev/null +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestEncryptionZoneManager.java @@ -0,0 +1,138 @@ +/** + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hdfs.server.namenode; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.crypto.CipherSuite; +import org.apache.hadoop.crypto.CryptoProtocolVersion; +import org.apache.hadoop.fs.BatchedRemoteIterator.BatchedListEntries; +import org.apache.hadoop.fs.permission.FsPermission; +import org.apache.hadoop.fs.permission.PermissionStatus; +import org.apache.hadoop.hdfs.protocol.EncryptionZone; +import org.apache.hadoop.hdfs.server.namenode.FSDirectory.DirOp; +import org.junit.Before; +import org.junit.Test; + +/** + * Test class for EncryptionZoneManager methods. Added tests for + * listEncryptionZones method, for cases where inode can and cannot have a + * parent inode. + */ +public class TestEncryptionZoneManager { + + private FSDirectory mockedDir; + private INodesInPath mockedINodesInPath; + private INodeDirectory firstINode; + private INodeDirectory secondINode; + private INodeDirectory rootINode; + private PermissionStatus defaultPermission; + private EncryptionZoneManager ezManager; + + @Before + public void setup() { + this.mockedDir = mock(FSDirectory.class); + this.mockedINodesInPath = mock(INodesInPath.class); + this.defaultPermission = new PermissionStatus("test", "test", + new FsPermission((short) 755)); + this.rootINode = + new INodeDirectory(0L, "".getBytes(), defaultPermission, + System.currentTimeMillis()); + this.firstINode = + new INodeDirectory(1L, "first".getBytes(), defaultPermission, + System.currentTimeMillis()); + this.secondINode = + new INodeDirectory(2L, "second".getBytes(), defaultPermission, + System.currentTimeMillis()); + when(this.mockedDir.hasReadLock()).thenReturn(true); + when(this.mockedDir.hasWriteLock()).thenReturn(true); + when(this.mockedDir.getInode(0L)).thenReturn(rootINode); + when(this.mockedDir.getInode(1L)).thenReturn(firstINode); + when(this.mockedDir.getInode(2L)).thenReturn(secondINode); + } + + @Test + public void testListEncryptionZonesOneValidOnly() throws Exception{ + this.ezManager = new EncryptionZoneManager(mockedDir, new Configuration()); + this.ezManager.addEncryptionZone(1L, CipherSuite.AES_CTR_NOPADDING, + CryptoProtocolVersion.ENCRYPTION_ZONES, "test_key"); + this.ezManager.addEncryptionZone(2L, CipherSuite.AES_CTR_NOPADDING, + CryptoProtocolVersion.ENCRYPTION_ZONES, "test_key"); + // sets root as proper parent for firstINode only + 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()); + } + + @Test + public void testListEncryptionZonesTwoValids() throws Exception { + this.ezManager = new EncryptionZoneManager(mockedDir, new Configuration()); + this.ezManager.addEncryptionZone(1L, CipherSuite.AES_CTR_NOPADDING, + CryptoProtocolVersion.ENCRYPTION_ZONES, "test_key"); + this.ezManager.addEncryptionZone(2L, CipherSuite.AES_CTR_NOPADDING, + CryptoProtocolVersion.ENCRYPTION_ZONES, "test_key"); + // sets root as proper parent for both inodes + this.firstINode.setParent(rootINode); + this.secondINode.setParent(rootINode); + when(mockedDir.getINodesInPath("/first", DirOp.READ_LINK)). + thenReturn(mockedINodesInPath); + when(mockedINodesInPath.getLastINode()). + thenReturn(firstINode); + INodesInPath mockedINodesInPathForSecond = + mock(INodesInPath.class); + when(mockedDir.getINodesInPath("/second", DirOp.READ_LINK)). + thenReturn(mockedINodesInPathForSecond); + when(mockedINodesInPathForSecond.getLastINode()). + thenReturn(secondINode); + BatchedListEntries result = + ezManager.listEncryptionZones(0); + assertEquals(2, result.size()); + assertEquals(1L, result.get(0).getId()); + assertEquals("/first", result.get(0).getPath()); + assertEquals(2L, result.get(1).getId()); + assertEquals("/second", result.get(1).getPath()); + } + + @Test + public void testListEncryptionZonesForRoot() throws Exception{ + this.ezManager = new EncryptionZoneManager(mockedDir, new Configuration()); + this.ezManager.addEncryptionZone(0L, CipherSuite.AES_CTR_NOPADDING, + CryptoProtocolVersion.ENCRYPTION_ZONES, "test_key"); + // sets root as proper parent for firstINode only + when(mockedDir.getINodesInPath("/", DirOp.READ_LINK)). + thenReturn(mockedINodesInPath); + when(mockedINodesInPath.getLastINode()). + thenReturn(rootINode); + BatchedListEntries result = ezManager. + listEncryptionZones(-1); + assertEquals(1, result.size()); + assertEquals(0L, result.get(0).getId()); + assertEquals("/", 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 0294368754f..7e372becaf8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testCryptoConf.xml +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testCryptoConf.xml @@ -278,8 +278,8 @@ -fs NAMENODE -rmdir /src/.Trash - -fs NAMENODE -rmdir /src - -fs NAMENODE -rmdir /dst + -fs NAMENODE -rm -r /src + -fs NAMENODE -rm -r /dst @@ -478,5 +478,99 @@ + + + Test list encryption zones when no zone has been deleted + + -fs NAMENODE -rm -r .Trash/Current/* + -fs NAMENODE -mkdir /test1 + -createZone -path /test1 -keyName myKey + -listZones + + + -fs NAMENODE -rm -r /test1 + -fs NAMENODE -rm -r .Trash/Current/* + + + + RegexpAcrossOutputComparator + (/test1)\s*(myKey)\s* + + + + + + Test adding two zones, then deleting one and listing zones. The deleted zone should still be listed, as it's under user's Trash folder' + + -fs NAMENODE -rm -r .Trash/Current/* + -fs NAMENODE -mkdir /test1 + -fs NAMENODE -mkdir /test2 + -createZone -path /test1 -keyName myKey + -createZone -path /test2 -keyName myKey + -fs NAMENODE -rm -r /test2 + -listZones + + + -fs NAMENODE -rm -r /test1 + -fs NAMENODE -rm -r .Trash/Current/* + + + + RegexpAcrossOutputComparator + (/test1)\s*(myKey)\s*(/user/).*(/.Trash/Current/test2)\s*(myKey)\s* + + + + + + Test adding two zones, then permanently deleting one and listing zones. + + -fs NAMENODE -rm -r .Trash/Current/* + -fs NAMENODE -mkdir /test1 + -fs NAMENODE -mkdir /test2 + -createZone -path /test1 -keyName myKey + -createZone -path /test2 -keyName myKey + -fs NAMENODE -rm -r /test2 + -fs NAMENODE -rm -r .Trash/Current/* + -listZones + + + -fs NAMENODE -rm -r /test1 + -fs NAMENODE -rm -r .Trash/Current/* + + + + RegexpAcrossOutputComparator + (/test1)\s*(myKey)\s* + + + + + + Test adding two zones to a snapshotable directory, take snapshot, permanently delete one of the EZs, then list zones + + -fs NAMENODE -rm -r .Trash/Current/* + -fs NAMENODE -mkdir /snapshotable + -fs NAMENODE -mkdir /snapshotable/test1 + -fs NAMENODE -mkdir /snapshotable/test2 + -fs NAMENODE -allowSnapshot /snapshotable + -fs NAMENODE -createSnapshot /snapshotable snapshot1 + -createZone -path /snapshotable/test1 -keyName myKey + -createZone -path /snapshotable/test2 -keyName myKey + -fs NAMENODE -rm -r /snapshotable/test2 + -fs NAMENODE -rm -r .Trash/Current/* + -listZones + + + -fs NAMENODE -rm -r /snapshotable + -fs NAMENODE -rm -r .Trash/Current/* + + + + RegexpAcrossOutputComparator + (/test1)\s*(myKey)\s* + + +