HDFS-5925. ACL configuration flag must only reject ACL API calls, not ACLs present in fsimage or edits. Contributed by Chris Nauroth.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/HDFS-4685@1567450 13f79535-47bb-0310-9956-ffa450edef68

hadoop-hdfs-project/hadoop-hdfs/CHANGES-HDFS-4685.txt

@@ -72,6 +72,9 @@ HDFS-4685 (Unreleased)
 
     HDFS-5625. Write end user documentation for HDFS ACLs. (cnauroth)
 
+    HDFS-5925. ACL configuration flag must only reject ACL API calls, not ACLs
+    present in fsimage or edits. (cnauroth)
+
   OPTIMIZATIONS
 
   BUG FIXES

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

@@ -24,8 +24,7 @@ import org.apache.hadoop.hdfs.protocol.AclException;
 
 /**
  * Support for ACLs is controlled by a configuration flag.  If the configuration
- * flag is false, then the NameNode will reject all ACL-related operations and
- * refuse to load an fsimage or edit log containing ACLs.
+ * flag is false, then the NameNode will reject all ACL-related operations.
  */
 final class AclConfigFlag {
   private final boolean enabled;
@@ -47,28 +46,11 @@ final class AclConfigFlag {
    * @throws AclException if ACLs are disabled
    */
   public void checkForApiCall() throws AclException {
-    check("The ACL operation has been rejected.");
-  }
-
-  /**
-   * Checks the flag on behalf of edit log loading.
-   *
-   * @throws AclException if ACLs are disabled
-   */
-  public void checkForEditLog() throws AclException {
-    check("Cannot load edit log containing an ACL.");
-  }
-
-  /**
-   * Common check method.
-   *
-   * @throws AclException if ACLs are disabled
-   */
-  private void check(String reason) throws AclException {
     if (!enabled) {
       throw new AclException(String.format(
-        "%s  Support for ACLs has been disabled by setting %s to false.",
-        reason, DFSConfigKeys.DFS_NAMENODE_ACLS_ENABLED_KEY));
+        "The ACL operation has been rejected.  "
+        + "Support for ACLs has been disabled by setting %s to false.",
+        DFSConfigKeys.DFS_NAMENODE_ACLS_ENABLED_KEY));
     }
   }
 }

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

@@ -294,9 +294,6 @@ public class FSEditLogLoader {
     switch (op.opCode) {
     case OP_ADD: {
       AddCloseOp addCloseOp = (AddCloseOp)op;
-      if (addCloseOp.aclEntries != null) {
-        fsNamesys.getAclConfigFlag().checkForEditLog();
-      }
       final String path =
           renameReservedPathsOnUpgrade(addCloseOp.path, logVersion);
       if (FSNamesystem.LOG.isDebugEnabled()) {
@@ -485,9 +482,6 @@ public class FSEditLogLoader {
     }
     case OP_MKDIR: {
       MkdirOp mkdirOp = (MkdirOp)op;
-      if (mkdirOp.aclEntries != null) {
-        fsNamesys.getAclConfigFlag().checkForEditLog();
-      }
       inodeId = getAndUpdateLastInodeId(mkdirOp.inodeId, logVersion,
           lastInodeId);
       fsDir.unprotectedMkdir(inodeId,
@@ -749,7 +743,6 @@ public class FSEditLogLoader {
       break;
     }
     case OP_SET_ACL: {
-      fsNamesys.getAclConfigFlag().checkForEditLog();
       SetAclOp setAclOp = (SetAclOp) op;
       fsDir.unprotectedSetAcl(setAclOp.src, setAclOp.aclEntries);
       break;

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

@@ -7393,10 +7393,6 @@ public class FSNamesystem implements Namesystem, FSClusterStats,
     return results;
   }
 
-  AclConfigFlag getAclConfigFlag() {
-    return aclConfigFlag;
-  }
-
   void modifyAclEntries(String src, List<AclEntry> aclSpec) throws IOException {
     aclConfigFlag.checkForApiCall();
     HdfsFileStatus resultingStat = null;

hadoop-hdfs-project/hadoop-hdfs/src/main/resources/hdfs-default.xml

@@ -351,11 +351,7 @@
   <description>
     Set to true to enable support for HDFS ACLs (Access Control Lists).  By
     default, ACLs are disabled.  When ACLs are disabled, the NameNode rejects
-    all attempts to set an ACL.  An fsimage containing an ACL will cause the
-    NameNode to abort during startup, and ACLs present in the edit log will
-    cause the NameNode to abort.  To transition from ACLs enabled to ACLs
-    disabled, restart the NameNode with ACLs enabled, remove all ACLs, save a
-    new checkpoint, and then restart the NameNode with ACLs disabled.
+    all RPCs related to setting or getting ACLs.
   </description>
 </property>
 

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestAclConfigFlag.java

@@ -23,8 +23,6 @@ import static org.apache.hadoop.fs.permission.AclEntryType.*;
 import static org.apache.hadoop.fs.permission.FsAction.*;
 import static org.junit.Assert.*;
 
-import java.io.IOException;
-
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hdfs.DFSConfigKeys;
@@ -34,7 +32,6 @@ import org.apache.hadoop.hdfs.protocol.AclException;
 import org.apache.hadoop.hdfs.server.namenode.NameNode;
 import org.apache.hadoop.hdfs.server.namenode.NameNodeAdapter;
 import org.apache.hadoop.io.IOUtils;
-import org.apache.hadoop.test.GenericTestUtils;
 import org.junit.After;
 import org.junit.Rule;
 import org.junit.Test;
@@ -44,9 +41,8 @@ import com.google.common.collect.Lists;
 
 /**
  * Tests that the configuration flag that controls support for ACLs is off by
- * default and causes all attempted operations related to ACLs to fail.  This
- * includes the API calls, ACLs found while loading fsimage and ACLs found while
- * applying edit log ops.
+ * default and causes all attempted operations related to ACLs to fail.  The
+ * NameNode can still load ACLs from fsimage or edits.
  */
 public class TestAclConfigFlag {
   private static final Path PATH = new Path("/path");
@@ -125,20 +121,23 @@ public class TestAclConfigFlag {
     fs.setAcl(PATH, Lists.newArrayList(
       aclEntry(DEFAULT, USER, "foo", READ_WRITE)));
 
-    // Attempt restart with ACLs disabled.
-    try {
-      restart(false, false);
-      fail("expected IOException");
-    } catch (IOException e) {
-      GenericTestUtils.assertExceptionContains(
-        DFSConfigKeys.DFS_NAMENODE_ACLS_ENABLED_KEY, e);
-    }
+    // Restart with ACLs disabled.  Expect successful restart.
+    restart(false, false);
+  }
 
-    // Recover by restarting with ACLs enabled, deleting the ACL, saving a new
-    // checkpoint, and then restarting with ACLs disabled.
-    restart(false, true);
-    fs.removeAcl(PATH);
-    restart(true, false);
+  @Test
+  public void testFsImage() throws Exception {
+    // With ACLs enabled, set an ACL.
+    initCluster(true, true);
+    fs.mkdirs(PATH);
+    fs.setAcl(PATH, Lists.newArrayList(
+      aclEntry(DEFAULT, USER, "foo", READ_WRITE)));
+
+    // Save a new checkpoint and restart with ACLs still enabled.
+    restart(true, true);
+
+    // Restart with ACLs disabled.  Expect successful restart.
+    restart(false, false);
   }
 
   /**