Hadoop Common
  1. Hadoop Common
  2. HADOOP-2683

Provide a way to specifiy login out side an RPC

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 0.16.0
    • Fix Version/s: 0.16.1
    • Component/s: None
    • Labels:
      None

      Description

      Requirements AFIK :

      It is required in some special cases (benchmarks etc) to invoke NameNode functionality without an RPC. For this users should be able to set user information that is otherwise available only an RPC.

      Patch for HADOOP-1298 includes a change to Server.java so that Server.getUserInfo() does not need to in an RPC. This probably will be replaced by patch here.

      Please include any other Jira's that depend on this.

      Proposed fix:

      • UserGroupInformation becomes an abstract class
      • public static UserGroupInformation.getUserInfo() is added. which usually just returns Server.getUserInfo();
      • public static UserGroupInformation.setUserInfo(UserGroupInformation) sets a thread local that will returned if Server.getUserInfo() returns null.
      • all invocations of Server.getUserInfo() will be replaced by UserGroupInformation.getUserInfo().
      1. 2683_20080211.patch
        9 kB
        Tsz Wo Nicholas Sze
      2. 2683_20080211b.patch
        5 kB
        Tsz Wo Nicholas Sze
      3. 2683_20080212.patch
        5 kB
        Tsz Wo Nicholas Sze
      4. 2683_20080213.patch
        11 kB
        Tsz Wo Nicholas Sze
      5. 2683_20080213b.patch
        11 kB
        Tsz Wo Nicholas Sze

        Activity

        Hide
        Doug Cutting added a comment -

        UserGroupInformation should not become dependent on Server, rather, UserGroupInformation should own the ThreadLocal, and Server should be a client of that functionality.

        Show
        Doug Cutting added a comment - UserGroupInformation should not become dependent on Server, rather, UserGroupInformation should own the ThreadLocal, and Server should be a client of that functionality.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        Here is a patch implementing the idea of Raghu and Doug

        Show
        Tsz Wo Nicholas Sze added a comment - Here is a patch implementing the idea of Raghu and Doug
        Hide
        Tsz Wo Nicholas Sze added a comment -

        Note that the running time of TestNNThroughputBenchmark drops from 14.281 sec to 4.937 sec in my machine.

        Show
        Tsz Wo Nicholas Sze added a comment - Note that the running time of TestNNThroughputBenchmark drops from 14.281 sec to 4.937 sec in my machine.
        Hide
        Raghu Angadi added a comment -

        A few comments:

        1. Isn't this a security hole to get super user privileges? All client needs to do is to send NULL for ticket, right? I think Server.getUserInfo() should only return ticket associated with the RPC.
        2. Minor: Could you remove debug in setCurrentUGI()? It is run for evey RPC.
        3. unrelated warning fixes in Server.java. It will conflict with HADOOP-2789.
        Show
        Raghu Angadi added a comment - A few comments: Isn't this a security hole to get super user privileges? All client needs to do is to send NULL for ticket, right? I think Server.getUserInfo() should only return ticket associated with the RPC. Minor: Could you remove debug in setCurrentUGI()? It is run for evey RPC. unrelated warning fixes in Server.java. It will conflict with HADOOP-2789 .
        Hide
        Raghu Angadi added a comment -

        Note that (1) above can happen even with a programming error, need not be deliberate.

        Show
        Raghu Angadi added a comment - Note that (1) above can happen even with a programming error, need not be deliberate.
        Hide
        Raghu Angadi added a comment -

        After an RPC is done, UGI is not reset. I would implement Server.java changes like this:

        • Server.java does not call UserGroupInformation.setCurrentUGI()
        • Server.getUserInfo() looks like this:
              public static UserGroupInformation getUserInfo() {
               Call call = CurCall.get();
               if (call != null) { // inside an RPC
                 return call.connection.ticket;
               }
               return UserGroupInformation.getCurrentUGI();
             } 
        • Otherwise, there is no need to have Server.getUserInfo() at all.
        Show
        Raghu Angadi added a comment - After an RPC is done, UGI is not reset. I would implement Server.java changes like this: Server.java does not call UserGroupInformation.setCurrentUGI() Server.getUserInfo() looks like this: public static UserGroupInformation getUserInfo() { Call call = CurCall.get(); if (call != null ) { // inside an RPC return call.connection.ticket; } return UserGroupInformation.getCurrentUGI(); } Otherwise, there is no need to have Server.getUserInfo() at all.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12375247/2683_20080211.patch
        against trunk revision 619744.

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

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

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

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

        release audit +1. The applied patch does not generate any new release audit warnings.

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

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

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

        Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1772/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1772/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1772/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1772/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/12375247/2683_20080211.patch against trunk revision 619744. @author +1. The patch does not contain any @author tags. tests included +1. The patch appears to include 3 new or modified tests. javadoc -1. The javadoc tool appears to have generated 1 warning messages. javac +1. The applied patch does not generate any new javac compiler warnings. release audit +1. The applied patch does not generate any new release audit warnings. findbugs +1. The patch does not introduce any new Findbugs warnings. core tests -1. The patch failed core unit tests. contrib tests +1. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1772/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1772/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1772/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1772/console This message is automatically generated.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        Thank you for your comment, Raghu. I like your idea for (1).

        2683_20080211b.patch:

        1. see above
        2. check LOG.isDebugEnabled() first before LOG.debug(...). Also RPC calls do not invoke UserGroupInformation.setCurrentUGI(...) anymore.
        3. reverted the unrelated warning fixes
        Show
        Tsz Wo Nicholas Sze added a comment - Thank you for your comment, Raghu. I like your idea for (1). 2683_20080211b.patch: see above check LOG.isDebugEnabled() first before LOG.debug(...). Also RPC calls do not invoke UserGroupInformation.setCurrentUGI(...) anymore. reverted the unrelated warning fixes
        Hide
        Raghu Angadi added a comment -

        +1.

        Show
        Raghu Angadi added a comment - +1.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12375257/2683_20080211b.patch
        against trunk revision 619744.

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

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

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

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

        release audit +1. The applied patch does not generate any new release audit warnings.

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

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

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

        Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1774/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1774/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1774/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1774/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/12375257/2683_20080211b.patch against trunk revision 619744. @author +1. The patch does not contain any @author tags. tests included +1. The patch appears to include 3 new or modified tests. javadoc +1. The javadoc tool did not generate any warning messages. javac +1. The applied patch does not generate any new javac compiler warnings. release audit +1. The applied patch does not generate any new release audit warnings. findbugs +1. The patch does not introduce any new Findbugs warnings. core tests -1. The patch failed core unit tests. contrib tests +1. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1774/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1774/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1774/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1774/console This message is automatically generated.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        Timeout on TestBalancer, which is addressed in HADOOP-2784. It seems it is not related to this issue.

        2683_20080212.patch: updated with current trunk.

        Show
        Tsz Wo Nicholas Sze added a comment - Timeout on TestBalancer, which is addressed in HADOOP-2784 . It seems it is not related to this issue. 2683_20080212.patch: updated with current trunk.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12375398/2683_20080212.patch
        against trunk revision 619744.

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

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

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

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

        release audit +1. The applied patch does not generate any new release audit 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://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1783/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1783/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1783/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1783/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/12375398/2683_20080212.patch against trunk revision 619744. @author +1. The patch does not contain any @author tags. tests included +1. The patch appears to include 3 new or modified tests. javadoc +1. The javadoc tool did not generate any warning messages. javac +1. The applied patch does not generate any new javac compiler warnings. release audit +1. The applied patch does not generate any new release audit 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://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1783/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1783/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1783/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1783/console This message is automatically generated.
        Hide
        Konstantin Shvachko added a comment - - edited

        I'd prefer if we could get rid of Server.getUserInfo().
        Current code and the proposed patch need to call Server.getUserInfo() even if there is no RPC and therefore no Server.
        Instead the Server should set UserGroupInformation.currentUGI before executing the call.
        The name-node will call UserGroupInformation.getCurrentUGI() instead Server.getUserInfo().
        If currentUGI = null the PermissionChecker should throw AccessControlException.
        I think we should not automatically assume superuser privileges when currentUGI is null.
        In case of non-RPC calls to name-node methods the caller should explicitly invoke UserGroupInformation.setCurrentUGI(..).
        It could be done once per thread and should not make calls inefficient.

        Show
        Konstantin Shvachko added a comment - - edited I'd prefer if we could get rid of Server.getUserInfo(). Current code and the proposed patch need to call Server.getUserInfo() even if there is no RPC and therefore no Server. Instead the Server should set UserGroupInformation.currentUGI before executing the call. The name-node will call UserGroupInformation.getCurrentUGI() instead Server.getUserInfo(). If currentUGI = null the PermissionChecker should throw AccessControlException. I think we should not automatically assume superuser privileges when currentUGI is null. In case of non-RPC calls to name-node methods the caller should explicitly invoke UserGroupInformation.setCurrentUGI(..). It could be done once per thread and should not make calls inefficient.
        Hide
        Raghu Angadi added a comment -

        I'd prefer if we could get rid of Server.getUserInfo().

        Current code and the proposed patch need to call Server.getUserInfo() even if there is no RPC and therefore no Server.

        +0.5 . It is required to call this only if caller expects to be in an RPC. But getting rid of it sounds just as good.

        I think we should not automatically assume super privileges currentUGI is null.

        +1.

        Show
        Raghu Angadi added a comment - I'd prefer if we could get rid of Server.getUserInfo(). Current code and the proposed patch need to call Server.getUserInfo() even if there is no RPC and therefore no Server. +0.5 . It is required to call this only if caller expects to be in an RPC. But getting rid of it sounds just as good. I think we should not automatically assume super privileges currentUGI is null. +1.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        2683_20080213.patch: no auto login, Server.getUserInfo() is deprecated.

        Thanks, Konstantin.

        Show
        Tsz Wo Nicholas Sze added a comment - 2683_20080213.patch: no auto login, Server.getUserInfo() is deprecated. Thanks, Konstantin.
        Hide
        Konstantin Shvachko added a comment -

        +1

        Show
        Konstantin Shvachko added a comment - +1
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12375531/2683_20080213.patch
        against trunk revision 619744.

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

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

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

        javac -1. The applied patch generated 604 javac compiler warnings (more than the trunk's current 603 warnings).

        release audit +1. The applied patch does not generate any new release audit 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://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1795/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1795/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1795/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1795/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/12375531/2683_20080213.patch against trunk revision 619744. @author +1. The patch does not contain any @author tags. tests included +1. The patch appears to include 9 new or modified tests. javadoc +1. The javadoc tool did not generate any warning messages. javac -1. The applied patch generated 604 javac compiler warnings (more than the trunk's current 603 warnings). release audit +1. The applied patch does not generate any new release audit 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://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1795/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1795/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1795/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1795/console This message is automatically generated.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        2683_20080213b.patch: added an annotation for a deprecated api

        Show
        Tsz Wo Nicholas Sze added a comment - 2683_20080213b.patch: added an annotation for a deprecated api
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12375548/2683_20080213b.patch
        against trunk revision 619744.

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

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

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

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

        release audit +1. The applied patch does not generate any new release audit 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://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1798/testReport/
        Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1798/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
        Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1798/artifact/trunk/build/test/checkstyle-errors.html
        Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1798/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/12375548/2683_20080213b.patch against trunk revision 619744. @author +1. The patch does not contain any @author tags. tests included +1. The patch appears to include 9 new or modified tests. javadoc +1. The javadoc tool did not generate any warning messages. javac +1. The applied patch does not generate any new javac compiler warnings. release audit +1. The applied patch does not generate any new release audit 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://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1798/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1798/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1798/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch/1798/console This message is automatically generated.
        Hide
        Konstantin Shvachko added a comment -

        I just committed this. Thank you Nicholas.

        Show
        Konstantin Shvachko added a comment - I just committed this. Thank you Nicholas.
        Hide
        Hudson added a comment -
        Show
        Hudson added a comment - Integrated in Hadoop-trunk #401 (See http://hudson.zones.apache.org/hudson/job/Hadoop-trunk/401/ )

          People

          • Assignee:
            Tsz Wo Nicholas Sze
            Reporter:
            Raghu Angadi
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development