Hive
  1. Hive
  2. HIVE-2797

Make the IP address of a Thrift client available to HMSHandler.

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.9.0
    • Component/s: Metastore
    • Labels:
      None

      Description

      Currently, in unsecured mode, metastore Thrift calls are, from the HMSHandler's point of view, anonymous. If we expose the IP address of the Thrift client to the HMSHandler from the Processor, this will help to give some context, in particular for audit logging, of where the call is coming from.

        Issue Links

          Activity

          Hide
          Phabricator added a comment -

          kevinwilfong requested code review of "HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.".
          Reviewers: JIRA

          https://issues.apache.org/jira/browse/HIVE-2797

          Created a new subclass of ThriftHiveMetastore.Processor which passes the IP address to the HMSHandler. Unsecure connections will use this processor. The IP address is stored in a thread local variable, so that multiple connections will each maintain there own IP address.

          Currently, in unsecured mode, metastore Thrift calls are, from the HMSHandler's point of view, anonymous. If we expose the IP address of the Thrift client to the HMSHandler from the Processor, this will help to give some context, in particular for audit logging, of where the call is coming from.

          TEST PLAN
          EMPTY

          REVISION DETAIL
          https://reviews.facebook.net/D1701

          AFFECTED FILES
          metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java
          metastore/src/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java

          MANAGE HERALD DIFFERENTIAL RULES
          https://reviews.facebook.net/herald/view/differential/

          WHY DID I GET THIS EMAIL?
          https://reviews.facebook.net/herald/transcript/3633/

          Tip: use the X-Herald-Rules header to filter Herald messages in your client.

          Show
          Phabricator added a comment - kevinwilfong requested code review of " HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.". Reviewers: JIRA https://issues.apache.org/jira/browse/HIVE-2797 Created a new subclass of ThriftHiveMetastore.Processor which passes the IP address to the HMSHandler. Unsecure connections will use this processor. The IP address is stored in a thread local variable, so that multiple connections will each maintain there own IP address. Currently, in unsecured mode, metastore Thrift calls are, from the HMSHandler's point of view, anonymous. If we expose the IP address of the Thrift client to the HMSHandler from the Processor, this will help to give some context, in particular for audit logging, of where the call is coming from. TEST PLAN EMPTY REVISION DETAIL https://reviews.facebook.net/D1701 AFFECTED FILES metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java metastore/src/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java MANAGE HERALD DIFFERENTIAL RULES https://reviews.facebook.net/herald/view/differential/ WHY DID I GET THIS EMAIL? https://reviews.facebook.net/herald/transcript/3633/ Tip: use the X-Herald-Rules header to filter Herald messages in your client.
          Hide
          Phabricator added a comment -

          ashutoshc has commented on the revision "HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.".

          Looks good. Mind, adding a test for it?

          REVISION DETAIL
          https://reviews.facebook.net/D1701

          Show
          Phabricator added a comment - ashutoshc has commented on the revision " HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.". Looks good. Mind, adding a test for it? REVISION DETAIL https://reviews.facebook.net/D1701
          Hide
          Phabricator added a comment -

          kevinwilfong updated the revision "HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.".
          Reviewers: JIRA, njain

          Added a test case to check that the IP address is set for remote metastores.

          REVISION DETAIL
          https://reviews.facebook.net/D1701

          AFFECTED FILES
          metastore/src/test/org/apache/hadoop/hive/metastore/IpAddressListener.java
          metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStoreIpAddress.java
          metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java
          metastore/src/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java

          Show
          Phabricator added a comment - kevinwilfong updated the revision " HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.". Reviewers: JIRA, njain Added a test case to check that the IP address is set for remote metastores. REVISION DETAIL https://reviews.facebook.net/D1701 AFFECTED FILES metastore/src/test/org/apache/hadoop/hive/metastore/IpAddressListener.java metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStoreIpAddress.java metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java metastore/src/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
          Hide
          Phabricator added a comment -

          kevinwilfong updated the revision "HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.".
          Reviewers: JIRA, njain

          REVISION DETAIL
          https://reviews.facebook.net/D1701

          AFFECTED FILES
          metastore/src/test/org/apache/hadoop/hive/metastore/IpAddressListener.java
          metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStoreIpAddress.java
          metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java
          metastore/src/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java

          Show
          Phabricator added a comment - kevinwilfong updated the revision " HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.". Reviewers: JIRA, njain REVISION DETAIL https://reviews.facebook.net/D1701 AFFECTED FILES metastore/src/test/org/apache/hadoop/hive/metastore/IpAddressListener.java metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStoreIpAddress.java metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java metastore/src/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
          Hide
          Phabricator added a comment -

          kevinwilfong updated the revision "HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.".
          Reviewers: JIRA, njain

          Ran svn up and resolved conflicts.

          REVISION DETAIL
          https://reviews.facebook.net/D1701

          AFFECTED FILES
          metastore/src/test/org/apache/hadoop/hive/metastore/IpAddressListener.java
          metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStoreIpAddress.java
          metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java
          metastore/src/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java

          Show
          Phabricator added a comment - kevinwilfong updated the revision " HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.". Reviewers: JIRA, njain Ran svn up and resolved conflicts. REVISION DETAIL https://reviews.facebook.net/D1701 AFFECTED FILES metastore/src/test/org/apache/hadoop/hive/metastore/IpAddressListener.java metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStoreIpAddress.java metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java metastore/src/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
          Hide
          Phabricator added a comment -

          ashutoshc has accepted the revision "HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.".

          Thanks for adding test-case, Kevin.
          +1 will commit if tests pass.

          REVISION DETAIL
          https://reviews.facebook.net/D1701

          BRANCH
          svn

          Show
          Phabricator added a comment - ashutoshc has accepted the revision " HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.". Thanks for adding test-case, Kevin. +1 will commit if tests pass. REVISION DETAIL https://reviews.facebook.net/D1701 BRANCH svn
          Hide
          Phabricator added a comment -

          ashutoshc has requested changes to the revision "HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.".

          I saw multiple failures in metastore tests.

          REVISION DETAIL
          https://reviews.facebook.net/D1701

          BRANCH
          svn

          Show
          Phabricator added a comment - ashutoshc has requested changes to the revision " HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.". I saw multiple failures in metastore tests. REVISION DETAIL https://reviews.facebook.net/D1701 BRANCH svn
          Hide
          Phabricator added a comment -

          kevinwilfong updated the revision "HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.".
          Reviewers: JIRA, njain, ashutoshc

          Really sorry about that Ashutosh. The TUGIContainingTranspport does not extend TSocket, which caused the errors you saw. I added a getSocket method to the class which returns the Socket object if the underlying TTransport is an instance of TSocket, otherwise null. TUGIBasedProcessor's implementation of setIpAddress now uses this method and handles the case of null.

          REVISION DETAIL
          https://reviews.facebook.net/D1701

          AFFECTED FILES
          shims/src/common/java/org/apache/hadoop/hive/thrift/TUGIContainingTransport.java
          metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteUGIHiveMetaStoreIpAddress.java
          metastore/src/test/org/apache/hadoop/hive/metastore/IpAddressListener.java
          metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStoreIpAddress.java
          metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java
          metastore/src/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java

          Show
          Phabricator added a comment - kevinwilfong updated the revision " HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.". Reviewers: JIRA, njain, ashutoshc Really sorry about that Ashutosh. The TUGIContainingTranspport does not extend TSocket, which caused the errors you saw. I added a getSocket method to the class which returns the Socket object if the underlying TTransport is an instance of TSocket, otherwise null. TUGIBasedProcessor's implementation of setIpAddress now uses this method and handles the case of null. REVISION DETAIL https://reviews.facebook.net/D1701 AFFECTED FILES shims/src/common/java/org/apache/hadoop/hive/thrift/TUGIContainingTransport.java metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteUGIHiveMetaStoreIpAddress.java metastore/src/test/org/apache/hadoop/hive/metastore/IpAddressListener.java metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStoreIpAddress.java metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java metastore/src/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
          Hide
          Phabricator added a comment -

          kevinwilfong has commented on the revision "HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.".

          I added a test case which verifies that the IP address is accessible when the setugi config variable is true.

          In addition I ran the entire test suite, TestHiveServerSessions timed out, but that seems to be an unrelated issue as it does the same on a fresh checkout.

          REVISION DETAIL
          https://reviews.facebook.net/D1701

          Show
          Phabricator added a comment - kevinwilfong has commented on the revision " HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.". I added a test case which verifies that the IP address is accessible when the setugi config variable is true. In addition I ran the entire test suite, TestHiveServerSessions timed out, but that seems to be an unrelated issue as it does the same on a fresh checkout. REVISION DETAIL https://reviews.facebook.net/D1701
          Hide
          Phabricator added a comment -

          ashutoshc has accepted the revision "HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.".

          No worries, Kevin. Thanks, for making changes.
          +1 Feel free to commit if tests pass.

          REVISION DETAIL
          https://reviews.facebook.net/D1701

          BRANCH
          svn

          Show
          Phabricator added a comment - ashutoshc has accepted the revision " HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.". No worries, Kevin. Thanks, for making changes. +1 Feel free to commit if tests pass. REVISION DETAIL https://reviews.facebook.net/D1701 BRANCH svn
          Hide
          Ashutosh Chauhan added a comment -

          Patch doesn't apply cleanly. Needs a rebase.

          Show
          Ashutosh Chauhan added a comment - Patch doesn't apply cleanly. Needs a rebase.
          Hide
          Phabricator added a comment -

          kevinwilfong updated the revision "HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.".
          Reviewers: JIRA, njain, ashutoshc

          Ran updated my checkout.

          REVISION DETAIL
          https://reviews.facebook.net/D1701

          AFFECTED FILES
          shims/src/common/java/org/apache/hadoop/hive/thrift/TUGIContainingTransport.java
          metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteUGIHiveMetaStoreIpAddress.java
          metastore/src/test/org/apache/hadoop/hive/metastore/IpAddressListener.java
          metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStoreIpAddress.java
          metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java
          metastore/src/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java
          metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java

          Show
          Phabricator added a comment - kevinwilfong updated the revision " HIVE-2797 [jira] Make the IP address of a Thrift client available to HMSHandler.". Reviewers: JIRA, njain, ashutoshc Ran updated my checkout. REVISION DETAIL https://reviews.facebook.net/D1701 AFFECTED FILES shims/src/common/java/org/apache/hadoop/hive/thrift/TUGIContainingTransport.java metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteUGIHiveMetaStoreIpAddress.java metastore/src/test/org/apache/hadoop/hive/metastore/IpAddressListener.java metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStoreIpAddress.java metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java metastore/src/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
          Hide
          Ashutosh Chauhan added a comment -

          Latest patch fails to apply with following in shims/src/common/java/org/apache/hadoop/hive/thrift/TUGIContainingTransport.java.rej

          ***************
          *** 71,74 ****
                  return transMap.get(trans);
                }
              }
          - }
          --- 85,88 ----
                  return transMap.get(trans);
                }
              }
          + }
          
          Show
          Ashutosh Chauhan added a comment - Latest patch fails to apply with following in shims/src/common/java/org/apache/hadoop/hive/thrift/TUGIContainingTransport.java.rej *************** *** 71,74 **** return transMap.get(trans); } } - } --- 85,88 ---- return transMap.get(trans); } } + }
          Hide
          Kevin Wilfong added a comment -

          Looks like something my editor must have done automatically, something about adding or removing a new line at the end of files. Anyway, I removed those lines from the patch manually (I hadn't intended to change that line anyway). HIVE-2797.7.patch contains this, and I tried applying it to a fresh checkout and did not get that error.

          Show
          Kevin Wilfong added a comment - Looks like something my editor must have done automatically, something about adding or removing a new line at the end of files. Anyway, I removed those lines from the patch manually (I hadn't intended to change that line anyway). HIVE-2797 .7.patch contains this, and I tried applying it to a fresh checkout and did not get that error.
          Hide
          Ashutosh Chauhan added a comment -

          Thanks, Kevin for updating the patch. Committed to trunk.

          Show
          Ashutosh Chauhan added a comment - Thanks, Kevin for updating the patch. Committed to trunk.
          Hide
          Hudson added a comment -

          Integrated in Hive-trunk-h0.21 #1332 (See https://builds.apache.org/job/Hive-trunk-h0.21/1332/)
          HIVE-2797: Make the IP address of a Thrift client available to HMSHandler. (Kevin Wilfong via Ashutosh Chauhan) (Revision 1305041)

          Result = FAILURE
          hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1305041
          Files :

          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java
          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java
          • /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/IpAddressListener.java
          • /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStoreIpAddress.java
          • /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteUGIHiveMetaStoreIpAddress.java
          • /hive/trunk/shims/src/common/java/org/apache/hadoop/hive/thrift/TUGIContainingTransport.java
          Show
          Hudson added a comment - Integrated in Hive-trunk-h0.21 #1332 (See https://builds.apache.org/job/Hive-trunk-h0.21/1332/ ) HIVE-2797 : Make the IP address of a Thrift client available to HMSHandler. (Kevin Wilfong via Ashutosh Chauhan) (Revision 1305041) Result = FAILURE hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1305041 Files : /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/IpAddressListener.java /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStoreIpAddress.java /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteUGIHiveMetaStoreIpAddress.java /hive/trunk/shims/src/common/java/org/apache/hadoop/hive/thrift/TUGIContainingTransport.java
          Hide
          Ashutosh Chauhan added a comment -

          This issue is closed now. It was released with the fix in 0.9.0. If there is a problem, please open a new jira and link this one with that.

          Show
          Ashutosh Chauhan added a comment - This issue is closed now. It was released with the fix in 0.9.0. If there is a problem, please open a new jira and link this one with that.
          Hide
          Hudson added a comment -

          Integrated in Hive-trunk-hadoop2 #54 (See https://builds.apache.org/job/Hive-trunk-hadoop2/54/)
          HIVE-2797: Make the IP address of a Thrift client available to HMSHandler. (Kevin Wilfong via Ashutosh Chauhan) (Revision 1305041)

          Result = ABORTED
          hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1305041
          Files :

          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java
          • /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java
          • /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/IpAddressListener.java
          • /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStoreIpAddress.java
          • /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteUGIHiveMetaStoreIpAddress.java
          • /hive/trunk/shims/src/common/java/org/apache/hadoop/hive/thrift/TUGIContainingTransport.java
          Show
          Hudson added a comment - Integrated in Hive-trunk-hadoop2 #54 (See https://builds.apache.org/job/Hive-trunk-hadoop2/54/ ) HIVE-2797 : Make the IP address of a Thrift client available to HMSHandler. (Kevin Wilfong via Ashutosh Chauhan) (Revision 1305041) Result = ABORTED hashutosh : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1305041 Files : /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java /hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/TUGIBasedProcessor.java /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/IpAddressListener.java /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStoreIpAddress.java /hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestRemoteUGIHiveMetaStoreIpAddress.java /hive/trunk/shims/src/common/java/org/apache/hadoop/hive/thrift/TUGIContainingTransport.java

            People

            • Assignee:
              Kevin Wilfong
              Reporter:
              Kevin Wilfong
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development