Details

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

      Description

      Currently we use a custom login module in UnixUserGroupInformation for acquiring user-credentials (via config or exec'ing 'whoami'). We should switch to using standard JAAS components such as LoginContext and possibly implement a custom UnixLoginContext for our current requirements. In future we can use this for Kerberos etc.

      1. UserGroupInformation.java
        15 kB
        Owen O'Malley
      2. UserGroupInformation.java
        16 kB
        Jakob Homan
      3. h-6299.patch
        99 kB
        Owen O'Malley
      4. HADOOP-6299-2.patch
        105 kB
        Jakob Homan
      5. 6299-MR-early.patch
        93 kB
        Devaraj Das
      6. h-6299.patch
        116 kB
        Owen O'Malley
      7. h-6299.patch
        120 kB
        Owen O'Malley
      8. h-6299.patch
        120 kB
        Owen O'Malley
      9. h-6299.patch
        121 kB
        Owen O'Malley
      10. HADOOP-6299-Y20.patch
        362 kB
        Jakob Homan

        Issue Links

          Activity

          Hide
          Owen O'Malley added a comment -

          This is a prototype of what I have in mind. In particular:

          1. Reimplement UserGroupInformation (UGI) to be based entirely on JAAS.
          2. UGI will have a single field that is the JAAS Subject that stores all of the information.
          3. UGI will support both Unix and Kerberos authentication. Unix is the equivalent of what we have now. Kerberos will assume that the user has a TGT in the ticket cache.
          4. Servers will be able to login in using a Kerberos keytab and principal name so that they run as the user.
          5. There will be a method to create a remote user based solely on the user name.
          6. It will use the Hadoop configuration to determine whether Kerberos or simple authentication is used. The JAAS configuration is done programatically instead of needing a separate configuration file in $JAVA_HOME.
          7. Move User class into UserGroupInformation.
          8. Remove Group class.
          9. Remove UnixUserGroupInformation class.

          Show
          Owen O'Malley added a comment - This is a prototype of what I have in mind. In particular: 1. Reimplement UserGroupInformation (UGI) to be based entirely on JAAS. 2. UGI will have a single field that is the JAAS Subject that stores all of the information. 3. UGI will support both Unix and Kerberos authentication. Unix is the equivalent of what we have now. Kerberos will assume that the user has a TGT in the ticket cache. 4. Servers will be able to login in using a Kerberos keytab and principal name so that they run as the user. 5. There will be a method to create a remote user based solely on the user name. 6. It will use the Hadoop configuration to determine whether Kerberos or simple authentication is used. The JAAS configuration is done programatically instead of needing a separate configuration file in $JAVA_HOME. 7. Move User class into UserGroupInformation. 8. Remove Group class. 9. Remove UnixUserGroupInformation class.
          Hide
          Jakob Homan added a comment -

          Attaching updated example that works with Windows/cygwin authorization. It turns out that Java only includes the login module for the current operating system, so we need to explictily specify either the UnixLoginModule or the NTLoginModule. After this change, the code works for either operating system.

          Also changed the the kerberos login module authentication to optional. It may be that the user doesn't have Kerberos running, but should still be able to be authenticated.

          Show
          Jakob Homan added a comment - Attaching updated example that works with Windows/cygwin authorization. It turns out that Java only includes the login module for the current operating system, so we need to explictily specify either the UnixLoginModule or the NTLoginModule. After this change, the code works for either operating system. Also changed the the kerberos login module authentication to optional. It may be that the user doesn't have Kerberos running, but should still be able to be authenticated.
          Hide
          Kan Zhang added a comment -

          There needs to be methods for saving and retrieving tokens of various kinds.

          Show
          Kan Zhang added a comment - There needs to be methods for saving and retrieving tokens of various kinds.
          Hide
          Owen O'Malley added a comment -

          Here's a preliminary patch for common.

          This patch:

          1. Works on Linux and Mac. It should work on Windows, except for the numeric group ids.
          2. The default UserGroupInformation.getUserName() is the full qualified name if Kerberos is turned on
          3. The Kerberos login module is optional, even if Kerberos is turned on.
          4. There is a doAs method on UGI to work as another user.
          5. We don't export the Subject from UGI any more
          6. Group is removed and User is now private.
          7. You can't set/get a UGI from a configuration.
          8. Service level authorization is radically changed to use the UGI instead of the Subject.
          9. Strengthened the SLA unit test
          10. Moved SLA tests from WritableRpcEngine to the framework
          11. Passes unit tests

          It still needs:

          1. A method to add/get tokens from the subject.
          2. A method to ask whether security is turned on.
          3. A method to set the configuration.
          4. Fix for windows to use the numeric group id method.

          and of course, we need to fix HDFS and MapReduce. smile

          Show
          Owen O'Malley added a comment - Here's a preliminary patch for common. This patch: Works on Linux and Mac. It should work on Windows, except for the numeric group ids. The default UserGroupInformation.getUserName() is the full qualified name if Kerberos is turned on The Kerberos login module is optional, even if Kerberos is turned on. There is a doAs method on UGI to work as another user. We don't export the Subject from UGI any more Group is removed and User is now private. You can't set/get a UGI from a configuration. Service level authorization is radically changed to use the UGI instead of the Subject. Strengthened the SLA unit test Moved SLA tests from WritableRpcEngine to the framework Passes unit tests It still needs: A method to add/get tokens from the subject. A method to ask whether security is turned on. A method to set the configuration. Fix for windows to use the numeric group id method. and of course, we need to fix HDFS and MapReduce. smile
          Hide
          Jakob Homan added a comment -

          Owen's latest patch doesn't apply after changes from HADOOP-4656. Updated file to apply clean. Working on remaining items.

          Show
          Jakob Homan added a comment - Owen's latest patch doesn't apply after changes from HADOOP-4656 . Updated file to apply clean. Working on remaining items.
          Hide
          Devaraj Das added a comment -

          This patch fixes most of MR to do UGI stuff in the new model. This patch makes MR compile on top of common built with the last patch Jakob uploaded. I didn't attempt to run the tests yet. One reason is that I don't have the new hdfs test class MiniHDFSCluster/DFSAdmin (and maybe others), and that's used in the MR tests.

          The other thing is the SecurityUtil's group mapping service is unavailable (and a test depends on that). I believe Owen/Jakob are putting that in in the next update to the patch for common.

          This may be a good time to get rid of the JobContext.USER_NAME and its usage in the framework. The patch doesn't address that.

          And yes, moving MR to the new UGI model was messier than i initially expected.

          Show
          Devaraj Das added a comment - This patch fixes most of MR to do UGI stuff in the new model. This patch makes MR compile on top of common built with the last patch Jakob uploaded. I didn't attempt to run the tests yet. One reason is that I don't have the new hdfs test class MiniHDFSCluster/DFSAdmin (and maybe others), and that's used in the MR tests. The other thing is the SecurityUtil's group mapping service is unavailable (and a test depends on that). I believe Owen/Jakob are putting that in in the next update to the patch for common. This may be a good time to get rid of the JobContext.USER_NAME and its usage in the framework. The patch doesn't address that. And yes, moving MR to the new UGI model was messier than i initially expected.
          Hide
          Owen O'Malley added a comment -

          This patch updates Common to the new UGI.

          Details:
          1. Removes the UnixUserGroupInformation class.
          2. The UserGroupInformation becomes a thin shell over the Subject.
          3. The Subject is no longer exposed to clients.
          4. It adds a doAs method for working as another user.
          5. Simplifies the Service Level Authorization to check directly rather than going through permissions.
          6. UGI loads Kerberos tickets into the subject.
          7. methods to load user credentials from keytab files.

          Show
          Owen O'Malley added a comment - This patch updates Common to the new UGI. Details: 1. Removes the UnixUserGroupInformation class. 2. The UserGroupInformation becomes a thin shell over the Subject. 3. The Subject is no longer exposed to clients. 4. It adds a doAs method for working as another user. 5. Simplifies the Service Level Authorization to check directly rather than going through permissions. 6. UGI loads Kerberos tickets into the subject. 7. methods to load user credentials from keytab files.
          Hide
          Devaraj Das added a comment -

          The patch looks good to me. Some javadoc could be added in the UserGroupInformation class but other than that, I think it is ready. The patch has also been used by both HDFS and MR from the point of view of using the APIs defined in the patch.

          Show
          Devaraj Das added a comment - The patch looks good to me. Some javadoc could be added in the UserGroupInformation class but other than that, I think it is ready. The patch has also been used by both HDFS and MR from the point of view of using the APIs defined 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/12430808/h-6299.patch
          against trunk revision 901540.

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

          +1 tests included. The patch appears to include 20 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 appears to introduce 1 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/284/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/284/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/284/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/284/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/12430808/h-6299.patch against trunk revision 901540. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 20 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 appears to introduce 1 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/284/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/284/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/284/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/284/console This message is automatically generated.
          Hide
          Owen O'Malley added a comment -

          This patch fixes:
          1. UGI.toString now just returns the user name.
          2. Fixes the javadoc warnings
          3. Adds more javadoc into UGI

          Show
          Owen O'Malley added a comment - This patch fixes: 1. UGI.toString now just returns the user name. 2. Fixes the javadoc warnings 3. Adds more javadoc into UGI
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12431123/h-6299.patch
          against trunk revision 901924.

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

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

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

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

          -1 findbugs. The patch appears to introduce 1 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/285/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/285/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/285/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/285/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/12431123/h-6299.patch against trunk revision 901924. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 20 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 1 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/285/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/285/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/285/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/285/console This message is automatically generated.
          Hide
          Owen O'Malley added a comment -

          This patch fixes the findbugs warning about circular static dependencies. I pulled the static block that sets the HadoopConfiguration as the JAAS configuration into initialize.

          Show
          Owen O'Malley added a comment - This patch fixes the findbugs warning about circular static dependencies. I pulled the static block that sets the HadoopConfiguration as the JAAS configuration into initialize.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12431322/h-6299.patch
          against trunk revision 902745.

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

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

          +1 javadoc. The javadoc tool did not generate any 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/289/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/289/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/289/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/289/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/12431322/h-6299.patch against trunk revision 902745. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 20 new or modified tests. +1 javadoc. The javadoc tool did not generate any 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/289/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/289/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/289/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/289/console This message is automatically generated.
          Hide
          Owen O'Malley added a comment -

          This adjusts the setConfiguration to not replace the TestingGroups object if we have been
          using the fake group service for testing.

          Show
          Owen O'Malley added a comment - This adjusts the setConfiguration to not replace the TestingGroups object if we have been using the fake group service for testing.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12431411/h-6299.patch
          against trunk revision 903015.

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

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

          +1 javadoc. The javadoc tool did not generate any 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/291/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/291/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/291/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/291/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/12431411/h-6299.patch against trunk revision 903015. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 20 new or modified tests. +1 javadoc. The javadoc tool did not generate any 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/291/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/291/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/291/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/291/console This message is automatically generated.
          Hide
          Devaraj Das added a comment -

          +1 on the patch.

          Show
          Devaraj Das added a comment - +1 on the patch.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #150 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/150/)
          . Reimplement the UserGroupInformation to use the OS
          specific and Kerberos JAAS login. (omalley)

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #150 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/150/ ) . Reimplement the UserGroupInformation to use the OS specific and Kerberos JAAS login. (omalley)
          Hide
          Owen O'Malley added a comment -

          I just committed this.

          Show
          Owen O'Malley added a comment - I just committed this.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk #234 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/234/)
          . Reimplement the UserGroupInformation to use the OS
          specific and Kerberos JAAS login. (omalley)

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk #234 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/234/ ) . Reimplement the UserGroupInformation to use the OS specific and Kerberos JAAS login. (omalley)
          Hide
          Tom White added a comment -

          Sorry to come to this so late, but I notice that this change removes some public classes (e.g. User, UnixUserGroupInformation) - should we not deprecate them first before removing them?

          Show
          Tom White added a comment - Sorry to come to this so late, but I notice that this change removes some public classes (e.g. User, UnixUserGroupInformation) - should we not deprecate them first before removing them?
          Hide
          Doug Cutting added a comment -

          I share Tom's concern. Should we re-open this issue or file a new one?

          Show
          Doug Cutting added a comment - I share Tom's concern. Should we re-open this issue or file a new one?
          Hide
          Owen O'Malley added a comment -

          They were never intended to be public interfaces. They were intended to be limited to HDFS and MapReduce.

          User still exists, it was just made non-public, which is hard to mark with deprecation. If the community thinks it is important to keep a deprecated UnixUGI, please open a new jira.

          Show
          Owen O'Malley added a comment - They were never intended to be public interfaces. They were intended to be limited to HDFS and MapReduce. User still exists, it was just made non-public, which is hard to mark with deprecation. If the community thinks it is important to keep a deprecated UnixUGI, please open a new jira.
          Hide
          Doug Cutting added a comment -

          > They were never intended to be public interfaces. They were intended to be limited to HDFS and MapReduce.

          Hmm. UnixUserGroupInformation.java was added as a public class in December of 2007, in HADOOP-2299. At that point in time Java visibility was used to define back-compatibility constraints.

          HADOOP-5073 proposed changing back-compatibility constraints, but its proposed constraints were never subjected to a vote. Sanjay states that any API not listed in his final comment there (added after the commit) are unstable. I'd be much more comfortable with that policy if a patch had been approved by consensus that actually added the annotations to all unstable public classes. Without that, our users have no ability to see what's stable and what's not. This policy been subject to any vote, only the code that permits us to declare scopes has.

          Show
          Doug Cutting added a comment - > They were never intended to be public interfaces. They were intended to be limited to HDFS and MapReduce. Hmm. UnixUserGroupInformation.java was added as a public class in December of 2007, in HADOOP-2299 . At that point in time Java visibility was used to define back-compatibility constraints. HADOOP-5073 proposed changing back-compatibility constraints, but its proposed constraints were never subjected to a vote. Sanjay states that any API not listed in his final comment there (added after the commit) are unstable. I'd be much more comfortable with that policy if a patch had been approved by consensus that actually added the annotations to all unstable public classes. Without that, our users have no ability to see what's stable and what's not. This policy been subject to any vote, only the code that permits us to declare scopes has.
          Hide
          Owen O'Malley added a comment -

          Java "public" does not imply public API. As a concrete instance of that, all of the RPC protocol interfaces are Java public and none of them are considered public APIs. Another instance is the NameNode and DataNode classes.

          I agree that prior to having the Audience annotations, it wasn't clear who the intended audience is. We are making progress in marking APIs, but it is not complete.

          Show
          Owen O'Malley added a comment - Java "public" does not imply public API. As a concrete instance of that, all of the RPC protocol interfaces are Java public and none of them are considered public APIs. Another instance is the NameNode and DataNode classes. I agree that prior to having the Audience annotations, it wasn't clear who the intended audience is. We are making progress in marking APIs, but it is not complete.
          Hide
          Chris Douglas added a comment -

          I'd be much more comfortable with that policy if a patch had been approved by consensus that actually added the annotations to all unstable public classes. Without that, our users have no ability to see what's stable and what's not. This policy been subject to any vote, only the code that permits us to declare scopes has.

          HADOOP-5073 had nearly a year of discussion before it was committed; that clause was in the first draft. Still, if you feel strongly about it, start a discussion and call a vote. In Hadoop, a whitelist of "this is public" makes a lot more sense to me than a blacklist of "this is private," but I can see why some users would disagree.

          UnixUserGroupInformation.java was added as a public class in December of 2007, in HADOOP-2299. At that point in time Java visibility was used to define back-compatibility constraints.

          Really? The scope of our back-compatibility constraints was not defined, which I thought was the motivation of HADOOP-5073. Asserting that this undocumented policy should apply to this issue until HADOOP-5073 gets even more discussion isn't a gap easily bridged for me. Is functionality lost when UnixUserGroupInformation is removed or is it redundant in the new code? Is the issue only that a public class was removed without first deprecating it?

          Show
          Chris Douglas added a comment - I'd be much more comfortable with that policy if a patch had been approved by consensus that actually added the annotations to all unstable public classes. Without that, our users have no ability to see what's stable and what's not. This policy been subject to any vote, only the code that permits us to declare scopes has. HADOOP-5073 had nearly a year of discussion before it was committed; that clause was in the first draft. Still, if you feel strongly about it, start a discussion and call a vote. In Hadoop, a whitelist of "this is public" makes a lot more sense to me than a blacklist of "this is private," but I can see why some users would disagree. UnixUserGroupInformation.java was added as a public class in December of 2007, in HADOOP-2299 . At that point in time Java visibility was used to define back-compatibility constraints. Really? The scope of our back-compatibility constraints was not defined, which I thought was the motivation of HADOOP-5073 . Asserting that this undocumented policy should apply to this issue until HADOOP-5073 gets even more discussion isn't a gap easily bridged for me. Is functionality lost when UnixUserGroupInformation is removed or is it redundant in the new code? Is the issue only that a public class was removed without first deprecating it?
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk #221 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk/221/)

          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #221 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk/221/ )
          Hide
          Jakob Homan added a comment -

          Patch for Yahoo!'s distribution. Tests and test-patch are fine (except known issue HADOOP-6530).

          Show
          Jakob Homan added a comment - Patch for Yahoo!'s distribution. Tests and test-patch are fine (except known issue HADOOP-6530 ).
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Mapreduce-trunk-Commit #225 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/225/)

          Show
          Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #225 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Mapreduce-trunk-Commit/225/ )

            People

            • Assignee:
              Owen O'Malley
              Reporter:
              Arun C Murthy
            • Votes:
              0 Vote for this issue
              Watchers:
              15 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development