Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-4685 Implementation of ACLs in HDFS
  3. HDFS-5615

NameNode: implement handling of ACLs in combination with sticky bit.

    Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: HDFS ACLs (HDFS-4685)
    • Fix Version/s: HDFS ACLs (HDFS-4685)
    • Component/s: namenode
    • Labels:
      None

      Description

      The sticky bit must work in combination with ACLs, similar to how the sticky bit already works with permissions.

      1. HDFS-5615.1.patch
        19 kB
        Chris Nauroth
      2. HDFS-5615.2.patch
        18 kB
        Chris Nauroth

        Issue Links

          Activity

          Haohui Mai made changes -
          Status In Progress [ 3 ] Resolved [ 5 ]
          Hadoop Flags Reviewed [ 10343 ]
          Fix Version/s HDFS ACLs (HDFS-4685) [ 12325671 ]
          Resolution Fixed [ 1 ]
          Hide
          Haohui Mai added a comment -

          I've committed this patch. Thanks Chris for the contribution.

          Show
          Haohui Mai added a comment - I've committed this patch. Thanks Chris for the contribution.
          Hide
          Haohui Mai added a comment -

          +1. I'll commit shortly.

          Show
          Haohui Mai added a comment - +1. I'll commit shortly.
          Chris Nauroth made changes -
          Attachment HDFS-5615.2.patch [ 12624072 ]
          Hide
          Chris Nauroth added a comment -

          Thanks, Haohui. I'm uploading version 2 with the refactoring that you suggested.

          Show
          Chris Nauroth added a comment - Thanks, Haohui. I'm uploading version 2 with the refactoring that you suggested.
          Hide
          Haohui Mai added a comment -

          Looks good to me. Nit: You might be able to write a helper method to simplify both testAclMovingFiles() and testMovingFiles(). For example:

          private void testMovingFileHelper(boolean useAcl) {
            // construct files
            if (useAcl) {
              applyAcl(...);
            }
            // test
          }
          
          public void testMovingFile() {
            testMovingFileHelper(false);
          }
          
          public void testAclMovingFile() {
            testMovingFileHelper(true);
          }
          

          Alternatively, combining them also works.

          Show
          Haohui Mai added a comment - Looks good to me. Nit: You might be able to write a helper method to simplify both testAclMovingFiles() and testMovingFiles() . For example: private void testMovingFileHelper( boolean useAcl) { // construct files if (useAcl) { applyAcl(...); } // test } public void testMovingFile() { testMovingFileHelper( false ); } public void testAclMovingFile() { testMovingFileHelper( true ); } Alternatively, combining them also works.
          Chris Nauroth made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Chris Nauroth made changes -
          Attachment HDFS-5615.1.patch [ 12624037 ]
          Hide
          Chris Nauroth added a comment -

          I'm attaching a patch that adds new tests to TestStickyBit. I also did some cleanup work while I was in this file.

          As I expected, this is only tests. The existing code in the HDFS-4685 branch already worked as expected, because sticky bit handling is decoupled from permission bits vs. ACLs.

          Summary:

          1. For each existing test, add a new variation of the test that applies an ACL.
          2. Refactor test cluster initialization and shutdown to BeforeClass and AfterClass methods. This gives a modest improvement in running time for the test suite.
          3. Refactor the various assertions to achieve some code sharing between the permission variant and the ACL variant of each test.
          Show
          Chris Nauroth added a comment - I'm attaching a patch that adds new tests to TestStickyBit . I also did some cleanup work while I was in this file. As I expected, this is only tests. The existing code in the HDFS-4685 branch already worked as expected, because sticky bit handling is decoupled from permission bits vs. ACLs. Summary: For each existing test, add a new variation of the test that applies an ACL. Refactor test cluster initialization and shutdown to BeforeClass and AfterClass methods. This gives a modest improvement in running time for the test suite. Refactor the various assertions to achieve some code sharing between the permission variant and the ACL variant of each test.
          Chris Nauroth made changes -
          Assignee Chris Nauroth [ cnauroth ]
          Hide
          Chris Nauroth added a comment -

          I expect that no code changes are required to support ACLs in combination with symlinks. Sticky bit is already handled as a separate concern in FSPermissionChecker. The patch in HDFS-5612 implements a new code path to enforce permissions for ACLs, but this has no impact on the code path for sticky bit handling. I'm still going to leave this issue open so that we can add tests to TestStickyBit to cover it.

          Show
          Chris Nauroth added a comment - I expect that no code changes are required to support ACLs in combination with symlinks. Sticky bit is already handled as a separate concern in FSPermissionChecker . The patch in HDFS-5612 implements a new code path to enforce permissions for ACLs, but this has no impact on the code path for sticky bit handling. I'm still going to leave this issue open so that we can add tests to TestStickyBit to cover it.
          Chris Nauroth made changes -
          Link This issue relates to HDFS-5612 [ HDFS-5612 ]
          Chris Nauroth made changes -
          Field Original Value New Value
          Affects Version/s HDFS ACLs (HDFS-4685) [ 12325671 ]
          Target Version/s HDFS ACLs (HDFS-4685) [ 12325671 ]
          Chris Nauroth created issue -

            People

            • Assignee:
              Chris Nauroth
              Reporter:
              Chris Nauroth
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development