HDFS-11197. Listing encryption zones fails when deleting a EZ that is on a snapshotted directory. Contributed by Wellington Chevreuil.

hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/EncryptionZoneManager.java

@@ -371,8 +371,12 @@ BatchedListEntries<EncryptionZone> listEncryptionZones(long prevId)
        contain a reference INode.
       */
       final String pathName = getFullPathName(ezi);
-      INodesInPath iip = dir.getINodesInPath(pathName, DirOp.READ_LINK);
+      INode inode = dir.getInode(ezi.getINodeId());
-      INode lastINode = iip.getLastINode();
+      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;
       }

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/cli/TestCryptoAdminCLI.java

@@ -63,6 +63,7 @@ public void setUp() throws Exception {
     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 = GenericTestUtils.getTestDir(UUID.randomUUID().toString());
     final Path jksPath = new Path(tmpDir.toString(), "test.jks");
@@ -127,7 +128,7 @@ protected TestConfigFileParser getConfigParser() {
   }
 
   private class TestConfigFileParserCryptoAdmin extends
-      CLITestHelper.TestConfigFileParser {
+      CLITestHelperDFS.TestConfigFileParserDFS {
     @Override
     public void endElement(String uri, String localName, String qName)
         throws SAXException {

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<EncryptionZone> 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<EncryptionZone> 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<EncryptionZone> result = ezManager.
+        listEncryptionZones(-1);
+    assertEquals(1, result.size());
+    assertEquals(0L, result.get(0).getId());
+    assertEquals("/", result.get(0).getPath());
+  }
+}

hadoop-hdfs-project/hadoop-hdfs/src/test/resources/testCryptoConf.xml

@@ -278,8 +278,8 @@
       </test-commands>
       <cleanup-commands>
         <command>-fs NAMENODE -rmdir /src/.Trash</command>
-        <command>-fs NAMENODE -rmdir /src</command>
+        <command>-fs NAMENODE -rm -r /src</command>
-        <command>-fs NAMENODE -rmdir /dst</command>
+        <command>-fs NAMENODE -rm -r /dst</command>
       </cleanup-commands>
       <comparators>
         <comparator>
@@ -478,5 +478,99 @@
         </comparator>
       </comparators>
     </test>
+
+    <test>
+      <description>Test list encryption zones when no zone has been deleted</description>
+      <test-commands>
+        <command>-fs NAMENODE -rm -r .Trash/Current/*</command>
+        <command>-fs NAMENODE -mkdir /test1</command>
+        <crypto-admin-command>-createZone -path /test1 -keyName myKey</crypto-admin-command>
+        <crypto-admin-command>-listZones</crypto-admin-command>
+      </test-commands>
+      <cleanup-commands>
+        <command>-fs NAMENODE -rm -r /test1</command>
+        <command>-fs NAMENODE -rm -r .Trash/Current/*</command>
+      </cleanup-commands>
+      <comparators>
+        <comparator>
+          <type>RegexpAcrossOutputComparator</type>
+          <expected-output>(/test1)\s*(myKey)\s*</expected-output>
+        </comparator>
+      </comparators>
+    </test>
+
+    <test>
+      <description>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'</description>
+      <test-commands>
+        <command>-fs NAMENODE -rm -r .Trash/Current/*</command>
+        <command>-fs NAMENODE -mkdir /test1</command>
+        <command>-fs NAMENODE -mkdir /test2</command>
+        <crypto-admin-command>-createZone -path /test1 -keyName myKey</crypto-admin-command>
+        <crypto-admin-command>-createZone -path /test2 -keyName myKey</crypto-admin-command>
+        <command>-fs NAMENODE -rm -r /test2</command>
+        <crypto-admin-command>-listZones</crypto-admin-command>
+      </test-commands>
+      <cleanup-commands>
+        <command>-fs NAMENODE -rm -r /test1</command>
+        <command>-fs NAMENODE -rm -r .Trash/Current/*</command>
+      </cleanup-commands>
+      <comparators>
+        <comparator>
+          <type>RegexpAcrossOutputComparator</type>
+          <expected-output>(/test1)\s*(myKey)\s*(/user/).*(/.Trash/Current/test2)\s*(myKey)\s*</expected-output>
+        </comparator>
+      </comparators>
+    </test>
+
+    <test>
+      <description>Test adding two zones, then permanently deleting one and listing zones.</description>
+      <test-commands>
+        <command>-fs NAMENODE -rm -r .Trash/Current/*</command>
+        <command>-fs NAMENODE -mkdir /test1</command>
+        <command>-fs NAMENODE -mkdir /test2</command>
+        <crypto-admin-command>-createZone -path /test1 -keyName myKey</crypto-admin-command>
+        <crypto-admin-command>-createZone -path /test2 -keyName myKey</crypto-admin-command>
+        <command>-fs NAMENODE -rm -r /test2</command>
+        <command>-fs NAMENODE -rm -r .Trash/Current/*</command>
+        <crypto-admin-command>-listZones</crypto-admin-command>
+      </test-commands>
+      <cleanup-commands>
+        <command>-fs NAMENODE -rm -r /test1</command>
+        <command>-fs NAMENODE -rm -r .Trash/Current/*</command>
+      </cleanup-commands>
+      <comparators>
+        <comparator>
+          <type>RegexpAcrossOutputComparator</type>
+          <expected-output>(/test1)\s*(myKey)\s*</expected-output>
+        </comparator>
+      </comparators>
+    </test>
+
+    <test>
+      <description>Test adding two zones to a snapshotable directory, take snapshot, permanently delete one of the EZs, then list zones</description>
+      <test-commands>
+        <command>-fs NAMENODE -rm -r .Trash/Current/*</command>
+        <command>-fs NAMENODE -mkdir /snapshotable</command>
+        <command>-fs NAMENODE -mkdir /snapshotable/test1</command>
+        <command>-fs NAMENODE -mkdir /snapshotable/test2</command>
+        <dfs-admin-command>-fs NAMENODE -allowSnapshot /snapshotable</dfs-admin-command>
+        <command>-fs NAMENODE -createSnapshot /snapshotable snapshot1</command>
+        <crypto-admin-command>-createZone -path /snapshotable/test1 -keyName myKey</crypto-admin-command>
+        <crypto-admin-command>-createZone -path /snapshotable/test2 -keyName myKey</crypto-admin-command>
+        <command>-fs NAMENODE -rm -r /snapshotable/test2</command>
+        <command>-fs NAMENODE -rm -r .Trash/Current/*</command>
+        <crypto-admin-command>-listZones</crypto-admin-command>
+      </test-commands>
+      <cleanup-commands>
+        <command>-fs NAMENODE -rm -r /snapshotable</command>
+        <command>-fs NAMENODE -rm -r .Trash/Current/*</command>
+      </cleanup-commands>
+      <comparators>
+        <comparator>
+          <type>RegexpAcrossOutputComparator</type>
+          <expected-output>(/test1)\s*(myKey)\s*</expected-output>
+        </comparator>
+      </comparators>
+    </test>
   </tests>
 </configuration>