HDFS-4659 Support setting execution bit for regular files (Brandon Li via sanjay)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1477994 13f79535-47bb-0310-9956-ffa450edef68

Author: Sanjay Radia

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileContextPermissionBase.java

@@ -202,8 +202,6 @@ void doFilePermissionCheck(FsPermission expectedPerm, FsPermission actualPerm) {
 
 
   /*
-   * Some filesystem like HDFS ignore the "x" bit if the permission.
-   * Others like localFs does not.
    * Override the method below if the file system being tested masks our
    * certain bits for file masks.
    */

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

@@ -22,6 +22,8 @@ Trunk (Unreleased)
     Azure environments. (See breakdown of tasks below for subtasks and
     contributors)
 
+    HDFS-4659 Support setting execution bit for regular files (Brandon Li via sanjay)
+
   IMPROVEMENTS
 
     HDFS-4665. Move TestNetworkTopologyWithNodeGroup to common.

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

@@ -44,9 +44,6 @@ public static INodeFile valueOf(INode inode, String path
     return (INodeFile)inode;
   }
 
-  static final FsPermission UMASK = FsPermission.createImmutable((short)0111);
-
-
   /** Format: [16 bits for replication][48 bits for PreferredBlockSize] */
   private static class HeaderFormat {
     /** Number of bits for Block size */
@@ -101,16 +98,6 @@ public final boolean isFile() {
     return true;
   }
 
-  /**
-   * Set the {@link FsPermission} of this {@link INodeFile}.
-   * Since this is a file,
-   * the {@link FsAction#EXECUTE} action, if any, is ignored.
-   */
-  @Override
-  void setPermission(FsPermission permission) {
-    super.setPermission(permission.applyUMask(UMASK));
-  }
-
   /** @return the replication factor of the file. */
   @Override
   public short getBlockReplication() {

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

@@ -57,8 +57,8 @@ public static INodeFileUnderConstruction valueOf(INode inode, String path
                              String clientName,
                              String clientMachine,
                              DatanodeDescriptor clientNode) {
-    super(id, permissions.applyUMask(UMASK), BlockInfo.EMPTY_ARRAY,
-        replication, modTime, modTime, preferredBlockSize);
+    super(id, permissions, BlockInfo.EMPTY_ARRAY, replication, modTime,
+        modTime, preferredBlockSize);
     this.clientName = clientName;
     this.clientMachine = clientMachine;
     this.clientNode = clientNode;

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/TestFcHdfsPermission.java

@@ -67,15 +67,4 @@ public void setUp() throws Exception {
   public void tearDown() throws Exception {
     super.tearDown();
   }
-  
-  /*
-   * HDFS ignore the "x" bit if the permission.
-   * 
-   */
-  static final FsPermission FILE_MASK_IGNORE_X_BIT = 
-    new FsPermission((short) ~0666);
-  @Override
-  FsPermission getFileMask() {
-    return FILE_MASK_IGNORE_X_BIT;
-  }
 }

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/permission/TestStickyBit.java

@@ -141,16 +141,7 @@ private void confirmSettingAndGetting(FileSystem hdfs, Path baseDir)
     hdfs.setPermission(p1, new FsPermission(withSB));
     assertTrue(hdfs.getFileStatus(p1).getPermission().getStickyBit());
 
-    // However, while you can set the sticky bit on files, it has no effect,
-    // following the linux/unix model:
-    //
-    // [user@host test]$ ls -alh
-    // -rw-r--r-- 1 user users 0 Dec 31 01:46 aFile
-    // [user@host test]$ chmod +t aFile
-    // [user@host test]$ ls -alh
-    // -rw-r--r-- 1 user users 0 Dec 31 01:46 aFile
-
-    // Write a file to the fs, try to set its sticky bit, expect to be ignored
+    // Write a file to the fs, try to set its sticky bit
     Path f = new Path(baseDir, "somefile");
     writeFile(hdfs, f);
     assertFalse(hdfs.getFileStatus(f).getPermission().getStickyBit());
@@ -159,7 +150,7 @@ private void confirmSettingAndGetting(FileSystem hdfs, Path baseDir)
 
     hdfs.setPermission(f, new FsPermission(withSB));
 
-    assertFalse(hdfs.getFileStatus(f).getPermission().getStickyBit());
+    assertTrue(hdfs.getFileStatus(f).getPermission().getStickyBit());
   }
 
   @Test

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java

@@ -63,7 +63,6 @@ public class TestDFSPermission {
 
   final private static short MAX_PERMISSION = 511;
   final private static short DEFAULT_UMASK = 022;
-  final private static short FILE_MASK = 0666;
   final private static FsPermission DEFAULT_PERMISSION = 
     FsPermission.createImmutable((short) 0777);
   final static private int NUM_TEST_PERMISSIONS = 
@@ -171,7 +170,7 @@ private void testPermissionSetting(OpType op) throws Exception {
     uMask = (short) 0111;
     initFileSystem(uMask);
     fs.setPermission(FILE_DIR_PATH, new FsPermission(uMask));
-    short expectedPermission = (short) ((op == OpType.CREATE) ? 0 : 0111);
+    short expectedPermission = (short) 0111;
     checkPermission(FILE_DIR_PATH, expectedPermission, true);
     closeFileSystem();
 
@@ -234,7 +233,7 @@ private void createAndCheckPermission(OpType op, Path name, short umask,
 
     // get the expected permission
     short expectedPermission = (op == OpType.CREATE) ? (short) (~umask
-        & permissionNum & FILE_MASK) : (short) (~umask & permissionNum);
+        & permissionNum) : (short) (~umask & permissionNum);
 
     // check if permission is correctly set
     checkPermission(name, expectedPermission, delete);

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/security/TestPermission.java

@@ -190,14 +190,28 @@ public void testFilePermission() throws Exception {
       catch(java.io.FileNotFoundException e) {
         LOG.info("GOOD: got " + e);
       }
+      
+      // make sure nn can take user specified permission (with default fs
+      // permission umask applied)
+      FSDataOutputStream out = nnfs.create(CHILD_FILE1, new FsPermission(
+          (short) 0777), true, 1024, (short) 1, 1024, null);
+      FileStatus status = nnfs.getFileStatus(CHILD_FILE1);
+      // FS_PERMISSIONS_UMASK_DEFAULT is 0022
+      assertTrue(status.getPermission().toString().equals("rwxr-xr-x"));
+      nnfs.delete(CHILD_FILE1, false);
+      
       // following dir/file creations are legal
       nnfs.mkdirs(CHILD_DIR1);
-      FSDataOutputStream out = nnfs.create(CHILD_FILE1);
+      out = nnfs.create(CHILD_FILE1);
+      status = nnfs.getFileStatus(CHILD_FILE1);
+      assertTrue(status.getPermission().toString().equals("rw-r--r--"));
       byte data[] = new byte[FILE_LEN];
       RAN.nextBytes(data);
       out.write(data);
       out.close();
       nnfs.setPermission(CHILD_FILE1, new FsPermission("700"));
+      status = nnfs.getFileStatus(CHILD_FILE1);
+      assertTrue(status.getPermission().toString().equals("rwx------"));
 
       // following read is legal
       byte dataIn[] = new byte[FILE_LEN];
@@ -208,6 +222,15 @@ public void testFilePermission() throws Exception {
         assertEquals(data[i], dataIn[i]);
       }
 
+      // test execution bit support for files
+      nnfs.setPermission(CHILD_FILE1, new FsPermission("755"));
+      status = nnfs.getFileStatus(CHILD_FILE1);
+      assertTrue(status.getPermission().toString().equals("rwxr-xr-x"));
+      nnfs.setPermission(CHILD_FILE1, new FsPermission("744"));
+      status = nnfs.getFileStatus(CHILD_FILE1);
+      assertTrue(status.getPermission().toString().equals("rwxr--r--"));
+      nnfs.setPermission(CHILD_FILE1, new FsPermission("700"));
+      
       ////////////////////////////////////////////////////////////////
       // test illegal file/dir creation
       UserGroupInformation userGroupInfo =