Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.15.0
    • Fix Version/s: 0.16.0
    • Component/s: fs
    • Labels:
      None

      Description

      Give a simple implementation of HADOOP-1701. Hadoop clients are assumed to be started within a Unix-like network which provides user and group management. This implementation read user information from the OS and send them to the NameNode in plaintexts through RPC (see also HADOOP-2184). NameNode trusts all information given and uses them for permission checking.

      1. ugi9.patch
        21 kB
        Hairong Kuang
      2. ugi7.patch
        20 kB
        Hairong Kuang
      3. ugi6.patch
        20 kB
        Hairong Kuang
      4. ugi5.patch
        20 kB
        Hairong Kuang
      5. ugi4.patch
        19 kB
        Hairong Kuang
      6. ugi3.patch
        20 kB
        Hairong Kuang
      7. ugi2.patch
        19 kB
        Hairong Kuang
      8. ugi1.patch
        17 kB
        Hairong Kuang
      9. ugi.patch
        18 kB
        Hairong Kuang

        Issue Links

          Activity

          Hide
          Hairong Kuang added a comment -

          Since HADOOP-1701 is still under debate, this issue intends to provide the minumum abstraction that needs to support file/directory permission in dfs. It defines a Writable interface called UserGroupInformation that contains user name, the names of the groups that it belongs to, and its umask.

          public interface UserGroupInformation extends Writable {
            /** Get username */
            public String getUserName();
            
            /*** Get the name of the groups that the user belong to */
            public String[] getGroupNames();
            
            /** Get the default group name.  */
            public String getDefaultGroupName();
          
            /** Get the user's umask. */
            public short getUMask();
          }
          

          It also provides a Unix implementation of the UGI interface. The implementation additionly provides a static method login with the following signature:

            public static UnixUserGroupInformation login(Configuration conf) throws LoginException; 

          It allows to construct a UnixUserGroupInformation object either from conf if it is contained there or from the underlying Unix system.

          Show
          Hairong Kuang added a comment - Since HADOOP-1701 is still under debate, this issue intends to provide the minumum abstraction that needs to support file/directory permission in dfs. It defines a Writable interface called UserGroupInformation that contains user name, the names of the groups that it belongs to, and its umask. public interface UserGroupInformation extends Writable { /** Get username */ public String getUserName(); /*** Get the name of the groups that the user belong to */ public String [] getGroupNames(); /** Get the default group name. */ public String getDefaultGroupName(); /** Get the user's umask. */ public short getUMask(); } It also provides a Unix implementation of the UGI interface. The implementation additionly provides a static method login with the following signature: public static UnixUserGroupInformation login(Configuration conf) throws LoginException; It allows to construct a UnixUserGroupInformation object either from conf if it is contained there or from the underlying Unix system.
          Hide
          Hairong Kuang added a comment -

          The patch implements the idea described above.

          Similliar to Owen's suggestion in HADOOP-1873, UnixUserGroupsInformation provides two static methods readFromConf and saveToConf that support (de)serializing a UnixUGI from/to conf.

          Show
          Hairong Kuang added a comment - The patch implements the idea described above. Similliar to Owen's suggestion in HADOOP-1873 , UnixUserGroupsInformation provides two static methods readFromConf and saveToConf that support (de)serializing a UnixUGI from/to conf.
          Hide
          Hairong Kuang added a comment -

          On the second thought, I feel that getUMask() should not be part of the UserGroupInformation (UGI) interface. One reason is that umask is not needed on the server side and it does not need to be transferred over the wire. In the new method, I removed getUMask from UGI, but UnixUserGroupInformation still implements this method and umask gets saved into configuration.

          Show
          Hairong Kuang added a comment - On the second thought, I feel that getUMask() should not be part of the UserGroupInformation (UGI) interface. One reason is that umask is not needed on the server side and it does not need to be transferred over the wire. In the new method, I removed getUMask from UGI, but UnixUserGroupInformation still implements this method and umask gets saved into configuration.
          Hide
          Hairong Kuang added a comment -

          This new patch enforces that the first entry in groups list is the default group.

          Show
          Hairong Kuang added a comment - This new patch enforces that the first entry in groups list is the default group.
          Hide
          Tsz Wo Nicholas Sze added a comment -

          In UnixUserGroupInformation.equals(...), it is better to do set comparison for the group array since we don't care ordering. For example,

          String [] a = ...
          String [] b = ...
          return a == b || new TreeSet<String>(Arrays.asList(a)).equals(new TreeSet<String>(Arrays.asList(b)));
          
          Show
          Tsz Wo Nicholas Sze added a comment - In UnixUserGroupInformation.equals(...), it is better to do set comparison for the group array since we don't care ordering. For example, String [] a = ... String [] b = ... return a == b || new TreeSet< String >(Arrays.asList(a)).equals( new TreeSet< String >(Arrays.asList(b)));
          Hide
          Hairong Kuang added a comment -

          This patch incorporates Tsz Wo's comments. It also makes the string form of umask to be in the octal format.

          Show
          Hairong Kuang added a comment - This patch incorporates Tsz Wo's comments. It also makes the string form of umask to be in the octal format.
          Hide
          Doug Cutting added a comment -

          Why is umask only a Unix feature, rather than in the interface?

          Also, it seems to me that perhaps umask should use the api from HADOOP-2288, exposing itself as an FsPermission, or at least linking to that in the javadoc, so that one knows how to use that value.

          Show
          Doug Cutting added a comment - Why is umask only a Unix feature, rather than in the interface? Also, it seems to me that perhaps umask should use the api from HADOOP-2288 , exposing itself as an FsPermission, or at least linking to that in the javadoc, so that one knows how to use that value.
          Hide
          Hairong Kuang added a comment -

          > Why is umask only a Unix feature, rather than in the interface?
          I took umask out of the UGI interface on the considering that the namenode does not need umask in UGI.

          > Also, it seems to me that perhaps umask should use the api from HADOOP-2288, exposing itself as an FsPermission, or at least linking to that in the javadoc, so that one knows how to use that value.

          It's a good idea to return umask as FsPermission. But a umask needs to be negated before it is applied to the user permission. Yes, at least I will do the linking.

          Show
          Hairong Kuang added a comment - > Why is umask only a Unix feature, rather than in the interface? I took umask out of the UGI interface on the considering that the namenode does not need umask in UGI. > Also, it seems to me that perhaps umask should use the api from HADOOP-2288 , exposing itself as an FsPermission, or at least linking to that in the javadoc, so that one knows how to use that value. It's a good idea to return umask as FsPermission. But a umask needs to be negated before it is applied to the user permission. Yes, at least I will do the linking.
          Hide
          Hairong Kuang added a comment -

          Ok, now the proposal for umask to remove umask from both UGI and UnixUGI. Instead FSPermission (defined in HADOOP-2288) is going to provide get/set umask methods. The approach is that rather than getting a user's umask from UNIX, umask is going to be read from hadoop configuration. If it is not set in the configuration, it is set to a default value 0022.

          This new patch reflects the proposed change.

          Show
          Hairong Kuang added a comment - Ok, now the proposal for umask to remove umask from both UGI and UnixUGI. Instead FSPermission (defined in HADOOP-2288 ) is going to provide get/set umask methods. The approach is that rather than getting a user's umask from UNIX, umask is going to be read from hadoop configuration. If it is not set in the configuration, it is set to a default value 0022. This new patch reflects the proposed change.
          Hide
          Doug Cutting added a comment -

          +1 This looks good to me.

          Show
          Doug Cutting added a comment - +1 This looks good to me.
          Hide
          Sanjay Radia added a comment -

          In writeFields, please write the kind of authentication technology (something like "STRING_UGI") to distinguish it from other authentication info such as tickets.
          readFields, it should raise an exception if the the string does not match.
          (This may be moot if we go to JAAS and GSS, as it probably has its own way of taking care of this, but at least it would be consistent for now).

          The javadocs comments for some of the public methods are not in the conventional javadoc style.

          Show
          Sanjay Radia added a comment - In writeFields, please write the kind of authentication technology (something like "STRING_UGI") to distinguish it from other authentication info such as tickets. readFields, it should raise an exception if the the string does not match. (This may be moot if we go to JAAS and GSS, as it probably has its own way of taking care of this, but at least it would be consistent for now). The javadocs comments for some of the public methods are not in the conventional javadoc style.
          Hide
          Hairong Kuang added a comment -

          This patch incorporates Sanjay's comments.

          Show
          Hairong Kuang added a comment - This patch incorporates Sanjay's comments.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12370727/ugi5.patch
          against trunk revision r600019.

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

          javadoc +1. The javadoc tool did not generate any warning messages.

          javac +1. The applied patch does not generate any new compiler warnings.

          findbugs -1. The patch appears to introduce 3 new Findbugs warnings.

          core tests -1. The patch failed core unit tests.

          contrib tests -1. The patch failed contrib unit tests.

          Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1225/testReport/
          Findbugs warnings: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1225/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1225/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1225/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/12370727/ugi5.patch against trunk revision r600019. @author +1. The patch does not contain any @author tags. javadoc +1. The javadoc tool did not generate any warning messages. javac +1. The applied patch does not generate any new compiler warnings. findbugs -1. The patch appears to introduce 3 new Findbugs warnings. core tests -1. The patch failed core unit tests. contrib tests -1. The patch failed contrib unit tests. Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1225/testReport/ Findbugs warnings: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1225/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1225/artifact/trunk/build/test/checkstyle-errors.html Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1225/console This message is automatically generated.
          Hide
          Owen O'Malley added a comment -

          This looks pretty good. You need to fix the findbugs warnings and I'd make all of the unix commands more standard (use whoami and groups) and make the invoking path strings relative paths. Making them absolute makes it much more likely to be wrong on non-linux platforms.

          Show
          Owen O'Malley added a comment - This looks pretty good. You need to fix the findbugs warnings and I'd make all of the unix commands more standard (use whoami and groups) and make the invoking path strings relative paths. Making them absolute makes it much more likely to be wrong on non-linux platforms.
          Hide
          Hairong Kuang added a comment -

          This patch fixed the findbug errors and incorporated Owen's comments. Also instead of using the command "id", it uses "whoami" and "groups" to get the current user's name & goups list.

          Show
          Hairong Kuang added a comment - This patch fixed the findbug errors and incorporated Owen's comments. Also instead of using the command "id", it uses "whoami" and "groups" to get the current user's name & goups list.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12370875/ugi6.patch
          against trunk revision r600707.

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

          javadoc +1. The javadoc tool did not generate any warning messages.

          javac +1. The applied patch does not generate any new compiler warnings.

          findbugs -1. The patch appears to introduce 2 new Findbugs warnings.

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

          contrib tests -1. The patch failed contrib unit tests.

          Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1248/testReport/
          Findbugs warnings: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1248/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1248/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1248/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/12370875/ugi6.patch against trunk revision r600707. @author +1. The patch does not contain any @author tags. javadoc +1. The javadoc tool did not generate any warning messages. javac +1. The applied patch does not generate any new compiler warnings. findbugs -1. The patch appears to introduce 2 new Findbugs warnings. core tests +1. The patch passed core unit tests. contrib tests -1. The patch failed contrib unit tests. Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1248/testReport/ Findbugs warnings: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1248/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1248/artifact/trunk/build/test/checkstyle-errors.html Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1248/console This message is automatically generated.
          Hide
          Hairong Kuang added a comment -

          The patch fixed the new findbug errors.

          Show
          Hairong Kuang added a comment - The patch fixed the new findbug errors.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12370899/ugi7.patch
          against trunk revision r600771.

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

          javadoc +1. The javadoc tool did not generate any warning messages.

          javac +1. The applied patch does not generate any new compiler warnings.

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

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

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

          Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1253/testReport/
          Findbugs warnings: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1253/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1253/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1253/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/12370899/ugi7.patch against trunk revision r600771. @author +1. The patch does not contain any @author tags. javadoc +1. The javadoc tool did not generate any warning messages. javac +1. The applied patch does not generate any new compiler warnings. findbugs +1. The patch does not introduce any new Findbugs warnings. core tests +1. The patch passed core unit tests. contrib tests +1. The patch passed contrib unit tests. Test results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1253/testReport/ Findbugs warnings: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1253/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1253/artifact/trunk/build/test/checkstyle-errors.html Console output: http://lucene.zones.apache.org:8080/hudson/job/Hadoop-Patch/1253/console This message is automatically generated.
          Hide
          Hairong Kuang added a comment -

          The previous patch conflicts with HADOOP-2288. So here is a new one.

          Show
          Hairong Kuang added a comment - The previous patch conflicts with HADOOP-2288 . So here is a new one.
          Hide
          dhruba borthakur added a comment -

          I just committed this. Thanks Hairong!

          Show
          dhruba borthakur added a comment - I just committed this. Thanks Hairong!

            People

            • Assignee:
              Hairong Kuang
              Reporter:
              Tsz Wo Nicholas Sze
            • Votes:
              1 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development