Hadoop Common
  1. Hadoop Common
  2. HADOOP-6862

Add api to add user/group to AccessControlList

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.22.0
    • Component/s: security
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Add api addUser(String user) and addGroup(String group) to add user/group to AccessControlList

      1. patch-6862.txt
        2 kB
        Amareshwari Sriramadasu
      2. patch-6862-1.txt
        4 kB
        Amareshwari Sriramadasu
      3. patch-6862-2.txt
        7 kB
        Amareshwari Sriramadasu

        Issue Links

          Activity

          Hide
          Amareshwari Sriramadasu added a comment -

          I just committed this.

          Show
          Amareshwari Sriramadasu added a comment - I just committed this.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #354 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/354/)
          HADOOP-6862. Adds api to add/remove user and group to AccessControlList. Contributed by Amareshwari Sriramadasu

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #354 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/354/ ) HADOOP-6862 . Adds api to add/remove user and group to AccessControlList. Contributed by Amareshwari Sriramadasu
          Hide
          Amareshwari Sriramadasu added a comment -

          test-patch result:

               [exec]
               [exec] -1 overall.
               [exec]
               [exec]     +1 @author.  The patch does not contain any @author tags.
               [exec]
               [exec]     +1 tests included.  The patch appears to include 3 new or modified tests.
               [exec]
               [exec]     -1 javadoc.  The javadoc tool appears to have generated 1 warning messages.
               [exec]
               [exec]     +1 javac.  The applied patch does not increase the total number of javac compiler warnings.
               [exec]
               [exec]     +1 findbugs.  The patch does not introduce any new Findbugs warnings.
               [exec]
               [exec]     +1 release audit.  The applied patch does not increase the total number of release audit warnings.
               [exec]
          

          bq -1 javadoc.
          Is due to HADOOP-6830. Verified there no new javadoc warnings introduced in the patch.

          All unit tests passed on my local machine.

          Show
          Amareshwari Sriramadasu added a comment - test-patch result: [exec] [exec] -1 overall. [exec] [exec] +1 @author. The patch does not contain any @author tags. [exec] [exec] +1 tests included. The patch appears to include 3 new or modified tests. [exec] [exec] -1 javadoc. The javadoc tool appears to have generated 1 warning messages. [exec] [exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings. [exec] [exec] +1 findbugs. The patch does not introduce any new Findbugs warnings. [exec] [exec] +1 release audit. The applied patch does not increase the total number of release audit warnings. [exec] bq -1 javadoc. Is due to HADOOP-6830 . Verified there no new javadoc warnings introduced in the patch. All unit tests passed on my local machine.
          Hide
          Vinod Kumar Vavilapalli added a comment -

          Patch looks good. +1.

          Show
          Vinod Kumar Vavilapalli added a comment - Patch looks good. +1.
          Hide
          Amareshwari Sriramadasu added a comment -

          Patch with suggested changes

          Show
          Amareshwari Sriramadasu added a comment - Patch with suggested changes
          Hide
          Vinod Kumar Vavilapalli added a comment -

          I will submit a patch which throws an exception saying "wild card not allowed" if a wild card character is passed.

          +1. As an aside, all the new methods should return immediately if allAllowed is already true, no point in maintaining any more user/group names.

          Show
          Vinod Kumar Vavilapalli added a comment - I will submit a patch which throws an exception saying "wild card not allowed" if a wild card character is passed. +1. As an aside, all the new methods should return immediately if allAllowed is already true, no point in maintaining any more user/group names.
          Hide
          Amareshwari Sriramadasu added a comment -

          I think adding/removing a wild card character through this api should not be allowed, because adding a '' (the only allowed wild card character in ACL) to an already created ACL does not make sense. Also, It makes sense to accept it only in the constructor as it is now, which defines the behavior to be simple. For example, adding it twice by addUser('") and removing it by removeUser("*") once would result in allowing all the users.

          I will submit a patch which throws an exception saying "wild card not allowed" if a wild card character is passed.

          Thoughts?

          Show
          Amareshwari Sriramadasu added a comment - I think adding/removing a wild card character through this api should not be allowed, because adding a ' ' (the only allowed wild card character in ACL) to an already created ACL does not make sense. Also, It makes sense to accept it only in the constructor as it is now, which defines the behavior to be simple. For example, adding it twice by addUser(' ") and removing it by removeUser("*") once would result in allowing all the users. I will submit a patch which throws an exception saying "wild card not allowed" if a wild card character is passed. Thoughts?
          Hide
          Vinod Kumar Vavilapalli added a comment -

          The nice little patch looks fine except for the point that adding or removing the wild card character either as part of users list of the groups doesn't change the ACL at all.

          You can also separate the newly added cases into a new test

          Show
          Vinod Kumar Vavilapalli added a comment - The nice little patch looks fine except for the point that adding or removing the wild card character either as part of users list of the groups doesn't change the ACL at all. You can also separate the newly added cases into a new test
          Hide
          Amareshwari Sriramadasu added a comment -

          -1 javadoc.

          Is because of HADOOP-6830. No new javadoc warnings are introduced in the patch.

          Show
          Amareshwari Sriramadasu added a comment - -1 javadoc. Is because of HADOOP-6830 . No new javadoc warnings are introduced in the patch.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12449552/patch-6862-1.txt
          against trunk revision 964134.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          -1 javadoc. The javadoc tool appears to have generated 1 warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/614/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/614/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/614/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/614/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12449552/patch-6862-1.txt against trunk revision 964134. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. -1 javadoc. The javadoc tool appears to have generated 1 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/614/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/614/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/614/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/614/console This message is automatically generated.
          Hide
          Amareshwari Sriramadasu added a comment -

          Adds removeUser and removeGroup api also.

          Show
          Amareshwari Sriramadasu added a comment - Adds removeUser and removeGroup api also.
          Hide
          Ravi Gummadi added a comment -

          How about api for removing user/group ?

          Show
          Ravi Gummadi added a comment - How about api for removing user/group ?
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12449539/patch-6862.txt
          against trunk revision 964134.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 3 new or modified tests.

          -1 javadoc. The javadoc tool appears to have generated 1 warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          +1 core tests. The patch passed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/613/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/613/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/613/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/613/console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12449539/patch-6862.txt against trunk revision 964134. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 3 new or modified tests. -1 javadoc. The javadoc tool appears to have generated 1 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/613/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/613/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/613/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/613/console This message is automatically generated.
          Hide
          Amareshwari Sriramadasu added a comment -

          Patch adds addUser(String user) and addGroup(String group) to AccessControlList

          Show
          Amareshwari Sriramadasu added a comment - Patch adds addUser(String user) and addGroup(String group) to AccessControlList

            People

            • Assignee:
              Amareshwari Sriramadasu
              Reporter:
              Amareshwari Sriramadasu
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development