Uploaded image for project: 'Hadoop HDFS'
  1. Hadoop HDFS
  2. HDFS-10488

Update WebHDFS documentation regarding CREATE and MKDIR default permissions

    Details

      Description

      WebHDFS methods for creating file/directories were always creating it with 755 permissions as default for both files and directories.

      The configured fs.permissions.umask-mode is intentionally ignored.

      This jira is to update the Documentation properly, explaining umask is not applied when using WebHDFS related methods.

      HDFS-6434 has also modified the default permissions for files, which is now 644. This will also be updated on the current documentation.

      1. HDFS-10488.002.patch
        11 kB
        Wellington Chevreuil
      2. HDFS-10488.003.patch
        11 kB
        Wellington Chevreuil
      3. HDFS-10488.005.patch
        2 kB
        Wellington Chevreuil
      4. HDFS-10488.006.patch
        2 kB
        Wellington Chevreuil
      5. HDFS-10488.patch
        9 kB
        Wellington Chevreuil

        Issue Links

          Activity

          Hide
          wchevreuil Wellington Chevreuil added a comment -

          Attaching initial patch proposal to make these WebHDFS methods consistent with dfs CLI. Had also added test cases for checking permissions when calling CREATE and MDKIR methods.

          Show
          wchevreuil Wellington Chevreuil added a comment - Attaching initial patch proposal to make these WebHDFS methods consistent with dfs CLI. Had also added test cases for checking permissions when calling CREATE and MDKIR methods.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 22s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 2 new or modified test files.
          0 mvndep 0m 15s Maven dependency ordering for branch
          +1 mvninstall 6m 5s trunk passed
          +1 compile 1m 21s trunk passed
          +1 checkstyle 0m 30s trunk passed
          +1 mvnsite 1m 23s trunk passed
          +1 mvneclipse 0m 23s trunk passed
          +1 findbugs 2m 58s trunk passed
          +1 javadoc 1m 24s trunk passed
          0 mvndep 0m 7s Maven dependency ordering for patch
          +1 mvninstall 1m 14s the patch passed
          +1 compile 1m 18s the patch passed
          +1 javac 1m 18s the patch passed
          -1 checkstyle 0m 28s hadoop-hdfs-project: The patch generated 23 new + 125 unchanged - 0 fixed = 148 total (was 125)
          +1 mvnsite 1m 19s the patch passed
          +1 mvneclipse 0m 17s the patch passed
          +1 whitespace 0m 1s The patch has no whitespace issues.
          +1 findbugs 3m 11s the patch passed
          +1 javadoc 1m 20s the patch passed
          +1 unit 0m 51s hadoop-hdfs-client in the patch passed.
          -1 unit 67m 38s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          94m 3s



          Reason Tests
          Failed junit tests hadoop.hdfs.TestRenameWhileOpen
            hadoop.hdfs.web.TestWebHdfsFileSystemContract
            hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12808212/HDFS-10488.patch
          JIRA Issue HDFS-10488
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux e34a3157025f 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 106234d
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15656/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15656/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15656/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15656/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15656/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 22s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 2 new or modified test files. 0 mvndep 0m 15s Maven dependency ordering for branch +1 mvninstall 6m 5s trunk passed +1 compile 1m 21s trunk passed +1 checkstyle 0m 30s trunk passed +1 mvnsite 1m 23s trunk passed +1 mvneclipse 0m 23s trunk passed +1 findbugs 2m 58s trunk passed +1 javadoc 1m 24s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 1m 14s the patch passed +1 compile 1m 18s the patch passed +1 javac 1m 18s the patch passed -1 checkstyle 0m 28s hadoop-hdfs-project: The patch generated 23 new + 125 unchanged - 0 fixed = 148 total (was 125) +1 mvnsite 1m 19s the patch passed +1 mvneclipse 0m 17s the patch passed +1 whitespace 0m 1s The patch has no whitespace issues. +1 findbugs 3m 11s the patch passed +1 javadoc 1m 20s the patch passed +1 unit 0m 51s hadoop-hdfs-client in the patch passed. -1 unit 67m 38s hadoop-hdfs in the patch failed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 94m 3s Reason Tests Failed junit tests hadoop.hdfs.TestRenameWhileOpen   hadoop.hdfs.web.TestWebHdfsFileSystemContract   hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12808212/HDFS-10488.patch JIRA Issue HDFS-10488 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux e34a3157025f 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 106234d Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15656/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/15656/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15656/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15656/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15656/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          wchevreuil Wellington Chevreuil added a comment -

          New patch version with checkstyle fixes and changes for TestWebHdfsFileSystemContract test to now expect 777 permission as default (same as dfs CLI).

          Show
          wchevreuil Wellington Chevreuil added a comment - New patch version with checkstyle fixes and changes for TestWebHdfsFileSystemContract test to now expect 777 permission as default (same as dfs CLI).
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 21s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 6s Maven dependency ordering for branch
          +1 mvninstall 5m 56s trunk passed
          +1 compile 1m 25s trunk passed
          +1 checkstyle 0m 32s trunk passed
          +1 mvnsite 1m 33s trunk passed
          +1 mvneclipse 0m 26s trunk passed
          +1 findbugs 3m 26s trunk passed
          +1 javadoc 1m 50s trunk passed
          0 mvndep 0m 7s Maven dependency ordering for patch
          +1 mvninstall 1m 26s the patch passed
          +1 compile 1m 18s the patch passed
          +1 javac 1m 18s the patch passed
          -1 checkstyle 0m 29s hadoop-hdfs-project: The patch generated 3 new + 161 unchanged - 1 fixed = 164 total (was 162)
          +1 mvnsite 1m 17s the patch passed
          +1 mvneclipse 0m 18s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 10s the patch passed
          +1 javadoc 1m 20s the patch passed
          +1 unit 0m 50s hadoop-hdfs-client in the patch passed.
          -1 unit 71m 38s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          99m 13s



          Reason Tests
          Failed junit tests hadoop.hdfs.server.datanode.TestLargeBlockReport
            hadoop.hdfs.server.blockmanagement.TestBlockTokenWithDFSStriped
            hadoop.fs.viewfs.TestViewFileSystemAtHdfsRoot



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12808220/HDFS-10488.002.patch
          JIRA Issue HDFS-10488
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 12e2a27e107e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 106234d
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15657/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15657/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15657/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15657/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15657/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 21s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 6s Maven dependency ordering for branch +1 mvninstall 5m 56s trunk passed +1 compile 1m 25s trunk passed +1 checkstyle 0m 32s trunk passed +1 mvnsite 1m 33s trunk passed +1 mvneclipse 0m 26s trunk passed +1 findbugs 3m 26s trunk passed +1 javadoc 1m 50s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 1m 26s the patch passed +1 compile 1m 18s the patch passed +1 javac 1m 18s the patch passed -1 checkstyle 0m 29s hadoop-hdfs-project: The patch generated 3 new + 161 unchanged - 1 fixed = 164 total (was 162) +1 mvnsite 1m 17s the patch passed +1 mvneclipse 0m 18s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 10s the patch passed +1 javadoc 1m 20s the patch passed +1 unit 0m 50s hadoop-hdfs-client in the patch passed. -1 unit 71m 38s hadoop-hdfs in the patch failed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 99m 13s Reason Tests Failed junit tests hadoop.hdfs.server.datanode.TestLargeBlockReport   hadoop.hdfs.server.blockmanagement.TestBlockTokenWithDFSStriped   hadoop.fs.viewfs.TestViewFileSystemAtHdfsRoot Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12808220/HDFS-10488.002.patch JIRA Issue HDFS-10488 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 12e2a27e107e 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 106234d Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HDFS-Build/15657/artifact/patchprocess/diff-checkstyle-hadoop-hdfs-project.txt unit https://builds.apache.org/job/PreCommit-HDFS-Build/15657/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15657/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15657/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15657/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          wchevreuil Wellington Chevreuil added a comment -

          Addressing last checkstyle issues. I had looked into the failed unit tests, but it does not seem to be related with any of the changes from last patch.

          Show
          wchevreuil Wellington Chevreuil added a comment - Addressing last checkstyle issues. I had looked into the failed unit tests, but it does not seem to be related with any of the changes from last patch.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 23s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 3 new or modified test files.
          0 mvndep 0m 8s Maven dependency ordering for branch
          +1 mvninstall 6m 37s trunk passed
          +1 compile 1m 19s trunk passed
          +1 checkstyle 0m 30s trunk passed
          +1 mvnsite 1m 22s trunk passed
          +1 mvneclipse 0m 21s trunk passed
          +1 findbugs 3m 2s trunk passed
          +1 javadoc 1m 23s trunk passed
          0 mvndep 0m 7s Maven dependency ordering for patch
          +1 mvninstall 1m 16s the patch passed
          +1 compile 1m 17s the patch passed
          +1 javac 1m 17s the patch passed
          +1 checkstyle 0m 29s hadoop-hdfs-project: The patch generated 0 new + 161 unchanged - 1 fixed = 161 total (was 162)
          +1 mvnsite 1m 16s the patch passed
          +1 mvneclipse 0m 18s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 39s the patch passed
          +1 javadoc 1m 27s the patch passed
          +1 unit 0m 57s hadoop-hdfs-client in the patch passed.
          -1 unit 77m 23s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 24s The patch does not generate ASF License warnings.
          105m 11s



          Reason Tests
          Failed junit tests hadoop.hdfs.web.TestWebHdfsTimeouts
            hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12808388/HDFS-10488.003.patch
          JIRA Issue HDFS-10488
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 289ed45f7f75 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 35f255b
          Default Java 1.8.0_91
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HDFS-Build/15660/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15660/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15660/testReport/
          modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project
          Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15660/console
          Powered by Apache Yetus 0.3.0 http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 23s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 3 new or modified test files. 0 mvndep 0m 8s Maven dependency ordering for branch +1 mvninstall 6m 37s trunk passed +1 compile 1m 19s trunk passed +1 checkstyle 0m 30s trunk passed +1 mvnsite 1m 22s trunk passed +1 mvneclipse 0m 21s trunk passed +1 findbugs 3m 2s trunk passed +1 javadoc 1m 23s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 1m 16s the patch passed +1 compile 1m 17s the patch passed +1 javac 1m 17s the patch passed +1 checkstyle 0m 29s hadoop-hdfs-project: The patch generated 0 new + 161 unchanged - 1 fixed = 161 total (was 162) +1 mvnsite 1m 16s the patch passed +1 mvneclipse 0m 18s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 39s the patch passed +1 javadoc 1m 27s the patch passed +1 unit 0m 57s hadoop-hdfs-client in the patch passed. -1 unit 77m 23s hadoop-hdfs in the patch failed. +1 asflicense 0m 24s The patch does not generate ASF License warnings. 105m 11s Reason Tests Failed junit tests hadoop.hdfs.web.TestWebHdfsTimeouts   hadoop.hdfs.server.namenode.snapshot.TestOpenFilesWithSnapshot Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12808388/HDFS-10488.003.patch JIRA Issue HDFS-10488 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 289ed45f7f75 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 35f255b Default Java 1.8.0_91 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HDFS-Build/15660/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt unit test logs https://builds.apache.org/job/PreCommit-HDFS-Build/15660/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HDFS-Build/15660/testReport/ modules C: hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: hadoop-hdfs-project Console output https://builds.apache.org/job/PreCommit-HDFS-Build/15660/console Powered by Apache Yetus 0.3.0 http://yetus.apache.org This message was automatically generated.
          Hide
          jzhuge John Zhuge added a comment -

          Wellington Chevreuil Shouldn't you take the ownership of this jira?

          Show
          jzhuge John Zhuge added a comment - Wellington Chevreuil Shouldn't you take the ownership of this jira?
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          Hi Wellington Chevreuil thanks for the great finding and the patch.

          The fix itself looks good to me in general and I just have 1 question about the fix and some quick comments regarding the test:

          Regarding the fix:

          I am a little confused by the following line: seems you always call getDirFsPermission() regardless whether it's a dir or file?

          np.setPermission(fullpath, permission.getDirFsPermission());
          
          1. The convention of HDFS unit testing is to start a MiniDFSCluster using @Before annotation, and tear down the cluster using @After. You can take a look at other HDFS test files for reference.
          2. Would you mind to add additional tests to verify it works as expected if the mask fs.permissions.umask-mode is set differently? This will be helpful in detecting other webhfs permission bugs if they exist.
          3. You have fixed the WebHDFS CREATESYMLINK operation. Could you also add tests to verify this operation as well?
          Show
          jojochuang Wei-Chiu Chuang added a comment - Hi Wellington Chevreuil thanks for the great finding and the patch. The fix itself looks good to me in general and I just have 1 question about the fix and some quick comments regarding the test: Regarding the fix: I am a little confused by the following line: seems you always call getDirFsPermission() regardless whether it's a dir or file? np.setPermission(fullpath, permission.getDirFsPermission()); The convention of HDFS unit testing is to start a MiniDFSCluster using @Before annotation, and tear down the cluster using @After . You can take a look at other HDFS test files for reference. Would you mind to add additional tests to verify it works as expected if the mask fs.permissions.umask-mode is set differently? This will be helpful in detecting other webhfs permission bugs if they exist. You have fixed the WebHDFS CREATESYMLINK operation. Could you also add tests to verify this operation as well?
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          I believe he's not on the contributor list which is why he can't assign this to himself.

          Show
          jojochuang Wei-Chiu Chuang added a comment - I believe he's not on the contributor list which is why he can't assign this to himself.
          Hide
          cnauroth Chris Nauroth added a comment -

          -1 for the proposed change.

          The umask (fs.permissions.umask-mode) is a concept that is applied at the client side by individual applications, usually via their usage of the FileSystem subclasses that implement a particular file system client. The umask is not applied by the API/protocol layer such as WebHDFS or NameNode RPC. As such, the behavior of the shell, which applies umask, is not always going to look consistent with the behavior of a direct curl WebHDFS call, which does not apply the umask.

          Using the shell to access WebHDFS gives consistent results, because the logic of the WebHdfsFileSystem class used by the shell will apply the umask.

          If this patch were committed, then it would become basically impossible to create files and directories with absolute permissions through WebHDFS. For example, suppose fs.permissions.umask-mode is set to 022, but an individual application has a desire to create a file with 775 permissions. This wouldn't work as expected, because server-side enforcement of the umask would restrict permissions on the resulting file to 755. The only way to work around this would be to reconfigure fs.permissions.umask-mode and restart the NameNode, which isn't operationally desirable. Worse than that, this would likely have the long-term effect of reducing fs.permissions.umask-mode to lowest common denominator, perhaps even 000, to accommodate all possible permissions at file creation time, thus weakening the benefit of umask as applied by client applications like the shell.

          As a final point against this change, please note that it could be considered backwards-incompatible. In my example above trying to create a file with 775 permissions, but the server-side umask forcing it to 755, it means that subsequent write actions by users in the same group will be unauthorized. This may break certain workflows.

          The area where there is a possibility for change is documentation to help raise user awareness of this. That could potentially go into the HDFS Permissions Guide page or the WebHDFS REST API page, or perhaps some combination of both. I would be happy to help review and +1 documentation changes.

          Wellington Chevreuil, despite my -1, thank you for writing up your experience with this and posting a patch. If you'd like to proceed with a documentation patch, please let me know, and I'll assign the issue to you.

          Show
          cnauroth Chris Nauroth added a comment - -1 for the proposed change. The umask ( fs.permissions.umask-mode ) is a concept that is applied at the client side by individual applications, usually via their usage of the FileSystem subclasses that implement a particular file system client. The umask is not applied by the API/protocol layer such as WebHDFS or NameNode RPC. As such, the behavior of the shell, which applies umask, is not always going to look consistent with the behavior of a direct curl WebHDFS call, which does not apply the umask. Using the shell to access WebHDFS gives consistent results, because the logic of the WebHdfsFileSystem class used by the shell will apply the umask. If this patch were committed, then it would become basically impossible to create files and directories with absolute permissions through WebHDFS. For example, suppose fs.permissions.umask-mode is set to 022, but an individual application has a desire to create a file with 775 permissions. This wouldn't work as expected, because server-side enforcement of the umask would restrict permissions on the resulting file to 755. The only way to work around this would be to reconfigure fs.permissions.umask-mode and restart the NameNode, which isn't operationally desirable. Worse than that, this would likely have the long-term effect of reducing fs.permissions.umask-mode to lowest common denominator, perhaps even 000, to accommodate all possible permissions at file creation time, thus weakening the benefit of umask as applied by client applications like the shell. As a final point against this change, please note that it could be considered backwards-incompatible. In my example above trying to create a file with 775 permissions, but the server-side umask forcing it to 755, it means that subsequent write actions by users in the same group will be unauthorized. This may break certain workflows. The area where there is a possibility for change is documentation to help raise user awareness of this. That could potentially go into the HDFS Permissions Guide page or the WebHDFS REST API page, or perhaps some combination of both. I would be happy to help review and +1 documentation changes. Wellington Chevreuil , despite my -1, thank you for writing up your experience with this and posting a patch. If you'd like to proceed with a documentation patch, please let me know, and I'll assign the issue to you.
          Hide
          wchevreuil Wellington Chevreuil added a comment -

          Thanks for the suggestions, Wei-Chiu Chuang and Chris Nauroth for the detailed explanation. I was going to upload another patch with pointers made by Wei-Chiu Chuang, then I saw Chris Nauroth comments.

          So, Chris Nauroth, summarizing, fs.permissions.umask-mode should not be applied for WebHDFS created directories/files. I think the explained intentions behind this is not clear on the documentation, so yes, I would like to work on a documentation patch for this.

          Additional points:

          1) While working on this, I had found out the default permission (if no permission is specified while calling the method) for both directories and files created by WebHDFS currently is 755. However, defining "execution" permissions for HDFS files don't have any value. Should this be changed to give different default permissions for files and directories?

          2) Still on the default values, setting 755 as default can lead to confusion about umask being used. Since default umask is 022, users can conclude that the umask is being applied when they see newly created directories got 755. Should this be changed to more permissive permissions such as 777?

          3) When working on tests for WebHDFS CREATESYMLINK as suggested by Wei-Chiu Chuang, I realized this method is no longer supported. Should we simply remove from WebHDFS, or only document this is not supported anymore and leave it giving the current error?

          Show
          wchevreuil Wellington Chevreuil added a comment - Thanks for the suggestions, Wei-Chiu Chuang and Chris Nauroth for the detailed explanation. I was going to upload another patch with pointers made by Wei-Chiu Chuang , then I saw Chris Nauroth comments. So, Chris Nauroth , summarizing, fs.permissions.umask-mode should not be applied for WebHDFS created directories/files. I think the explained intentions behind this is not clear on the documentation, so yes, I would like to work on a documentation patch for this. Additional points: 1) While working on this, I had found out the default permission (if no permission is specified while calling the method) for both directories and files created by WebHDFS currently is 755 . However, defining "execution" permissions for HDFS files don't have any value. Should this be changed to give different default permissions for files and directories? 2) Still on the default values, setting 755 as default can lead to confusion about umask being used. Since default umask is 022 , users can conclude that the umask is being applied when they see newly created directories got 755 . Should this be changed to more permissive permissions such as 777 ? 3) When working on tests for WebHDFS CREATESYMLINK as suggested by Wei-Chiu Chuang , I realized this method is no longer supported. Should we simply remove from WebHDFS, or only document this is not supported anymore and leave it giving the current error?
          Hide
          cnauroth Chris Nauroth added a comment -

          So, Chris Nauroth, summarizing, fs.permissions.umask-mode should not be applied for WebHDFS created directories/files.

          I think a slight refinement of this is to say that it should not be applied by the WebHDFS server side (the NameNode). It may be applied by the WebHDFS client side. For example, the WebHdfsFileSystem class that ships in Hadoop does apply fs.permissions.umask-mode from the client side before calling the WebHDFS server side.

          While working on this, I had found out the default permission (if no permission is specified while calling the method) for both directories and files created by WebHDFS currently is 755. However, defining "execution" permissions for HDFS files don't have any value. Should this be changed to give different default permissions for files and directories?

          This part is admittedly odd, and there is a long-standing open JIRA requesting a change to 644 as the default for files. That is HDFS-6434. This change is potentially backwards-incompatible, such as if someone has an existing workflow that round-trips a file through HDFS and expects it to be executable after getting it back out, though that's likely a remote edge case. If you'd like to proceed with HDFS-6434, then I'd suggest targeting trunk/Hadoop 3.x, where we currently can make backwards-incompatible changes.

          Still on the default values, setting 755 as default can lead to confusion about umask being used. Since default umask is 022, users can conclude that the umask is being applied when they see newly created directories got 755. Should this be changed to more permissive permissions such as 777?

          I do think 777 makes sense from one perspective, but there is also a trade-off with providing behavior that is secure by default. In HDFS-2427, the project made the choice to go with 755, favoring secure default behavior (755) over the possibly more intuitive behavior (777).

          When working on tests for WebHDFS CREATESYMLINK as suggested by Wei-Chiu Chuang, I realized this method is no longer supported. Should we simply remove from WebHDFS, or only document this is not supported anymore and leave it giving the current error?

          HDFS symlinks are currently in a state where the code is partially completed but dormant due to unresolved problems with backwards-compatibility and security. We might get past those hurdles someday, so I suggest leaving that code as is. We still run tests against the symlink code paths. This works by having the tests call the private FileSystem#enableSymlinks method to toggle on the dormant symlink code.

          Show
          cnauroth Chris Nauroth added a comment - So, Chris Nauroth, summarizing, fs.permissions.umask-mode should not be applied for WebHDFS created directories/files. I think a slight refinement of this is to say that it should not be applied by the WebHDFS server side (the NameNode). It may be applied by the WebHDFS client side. For example, the WebHdfsFileSystem class that ships in Hadoop does apply fs.permissions.umask-mode from the client side before calling the WebHDFS server side. While working on this, I had found out the default permission (if no permission is specified while calling the method) for both directories and files created by WebHDFS currently is 755. However, defining "execution" permissions for HDFS files don't have any value. Should this be changed to give different default permissions for files and directories? This part is admittedly odd, and there is a long-standing open JIRA requesting a change to 644 as the default for files. That is HDFS-6434 . This change is potentially backwards-incompatible, such as if someone has an existing workflow that round-trips a file through HDFS and expects it to be executable after getting it back out, though that's likely a remote edge case. If you'd like to proceed with HDFS-6434 , then I'd suggest targeting trunk/Hadoop 3.x, where we currently can make backwards-incompatible changes. Still on the default values, setting 755 as default can lead to confusion about umask being used. Since default umask is 022, users can conclude that the umask is being applied when they see newly created directories got 755. Should this be changed to more permissive permissions such as 777? I do think 777 makes sense from one perspective, but there is also a trade-off with providing behavior that is secure by default. In HDFS-2427 , the project made the choice to go with 755, favoring secure default behavior (755) over the possibly more intuitive behavior (777). When working on tests for WebHDFS CREATESYMLINK as suggested by Wei-Chiu Chuang, I realized this method is no longer supported. Should we simply remove from WebHDFS, or only document this is not supported anymore and leave it giving the current error? HDFS symlinks are currently in a state where the code is partially completed but dormant due to unresolved problems with backwards-compatibility and security. We might get past those hurdles someday, so I suggest leaving that code as is. We still run tests against the symlink code paths. This works by having the tests call the private FileSystem#enableSymlinks method to toggle on the dormant symlink code.
          Hide
          wchevreuil Wellington Chevreuil added a comment -

          Attaching a patch with the documentation changes.

          Additional comments:

          It may be applied by the WebHDFS client side. For example, the WebHdfsFileSystem class that ships in Hadoop does apply fs.permissions.umask-mode from the client side before calling the WebHDFS server side.

          I guess that for client applications accessing WebHDFS REST directly, such as curl, there will be no option to apply fs.permissions.umask-mode, since it would be accessing WebHDFS server on Namenode directly, which simply ignores the umask. Since related REST methods already allows for specifying the desired permission as a parameter, don't think it would be useful to also allow for specifying the umask on the methods.

          Show
          wchevreuil Wellington Chevreuil added a comment - Attaching a patch with the documentation changes. Additional comments: It may be applied by the WebHDFS client side. For example, the WebHdfsFileSystem class that ships in Hadoop does apply fs.permissions.umask-mode from the client side before calling the WebHDFS server side. I guess that for client applications accessing WebHDFS REST directly, such as curl, there will be no option to apply fs.permissions.umask-mode , since it would be accessing WebHDFS server on Namenode directly, which simply ignores the umask. Since related REST methods already allows for specifying the desired permission as a parameter, don't think it would be useful to also allow for specifying the umask on the methods.
          Hide
          wchevreuil Wellington Chevreuil added a comment -

          Anyone has any additional comment on the last patch?

          Show
          wchevreuil Wellington Chevreuil added a comment - Anyone has any additional comment on the last patch?
          Hide
          andrew.wang Andrew Wang added a comment -

          The current 005 patch looks okay for branch-2, but since we just committed HDFS-6434, does this patch need to be updated for trunk? It mentions a file having default 755 permissions, but HDFS-6434 makes the default 644. The other note is that there's a "Permission" section in this doc that mentions 755 as the default for both files and dirs, this should also be updated.

          Would also appreciate if you could update the JIRA summary to reflect the contents of the patch. Thanks Wellington.

          Show
          andrew.wang Andrew Wang added a comment - The current 005 patch looks okay for branch-2, but since we just committed HDFS-6434 , does this patch need to be updated for trunk? It mentions a file having default 755 permissions, but HDFS-6434 makes the default 644. The other note is that there's a "Permission" section in this doc that mentions 755 as the default for both files and dirs, this should also be updated. Would also appreciate if you could update the JIRA summary to reflect the contents of the patch. Thanks Wellington.
          Hide
          wchevreuil Wellington Chevreuil added a comment -

          Hi Andrew Wang. Thanks for the updates.

          Attaching new patch addressing your previous comments. I had also updated the original jira summary as you had suggested.

          Show
          wchevreuil Wellington Chevreuil added a comment - Hi Andrew Wang . Thanks for the updates. Attaching new patch addressing your previous comments. I had also updated the original jira summary as you had suggested.
          Hide
          andrew.wang Andrew Wang added a comment -

          Got it, LGTM. I'll hold off on committing 006 to trunk and 005 to the branch-2s in case Chris and others want to squeeze in a review.

          Show
          andrew.wang Andrew Wang added a comment - Got it, LGTM. I'll hold off on committing 006 to trunk and 005 to the branch-2s in case Chris and others want to squeeze in a review.
          Hide
          andrew.wang Andrew Wang added a comment -

          About to commit the respective patches, thanks all.

          Show
          andrew.wang Andrew Wang added a comment - About to commit the respective patches, thanks all.
          Hide
          andrew.wang Andrew Wang added a comment -

          Committed, thanks Wellington for the patch, and everyone for the reviews!

          Show
          andrew.wang Andrew Wang added a comment - Committed, thanks Wellington for the patch, and everyone for the reviews!
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #10059 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10059/)
          HDFS-10488. Update WebHDFS documentation regarding CREATE and MKDIR (wang: rev b8f93cd2750cde31d3e80c1aecd426e0cea89ef2)

          • hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/WebHDFS.md
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #10059 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10059/ ) HDFS-10488 . Update WebHDFS documentation regarding CREATE and MKDIR (wang: rev b8f93cd2750cde31d3e80c1aecd426e0cea89ef2) hadoop-hdfs-project/hadoop-hdfs/src/site/markdown/WebHDFS.md
          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          This never made it to branch-2.7.3. I just merged it in.

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - This never made it to branch-2.7.3. I just merged it in.
          Hide
          andrew.wang Andrew Wang added a comment -

          Thanks Vinod, sorry for the miss.

          Show
          andrew.wang Andrew Wang added a comment - Thanks Vinod, sorry for the miss.
          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          Closing the JIRA as part of 2.7.3 release.

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - Closing the JIRA as part of 2.7.3 release.

            People

            • Assignee:
              wchevreuil Wellington Chevreuil
              Reporter:
              wchevreuil Wellington Chevreuil
            • Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development