[ 
https://issues.apache.org/jira/browse/HIVE-27129?focusedWorklogId=850359&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-850359
 ]

ASF GitHub Bot logged work on HIVE-27129:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 10/Mar/23 17:50
            Start Date: 10/Mar/23 17:50
    Worklog Time Spent: 10m 
      Work Description: vihangk-db commented on code in PR #4104:
URL: https://github.com/apache/hive/pull/4104#discussion_r1132675659


##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -621,31 +611,84 @@ private <T extends TTransport> T 
configureThriftMaxMessageSize(T transport) {
     return transport;
   }
 
+  private Map<String, String> getAdditionalHeaders() {
+    Map<String, String> headers = new HashMap<>();
+    String keyValuePairs = MetastoreConf.getVar(conf, 
ConfVars.METASTORE_CLIENT_ADDITIONAL_HEADERS);
+    try {
+      List<String> headerKeyValues = 
Splitter.on(',').trimResults().splitToList(keyValuePairs);
+      for (String header : headerKeyValues) {
+        String[] parts = header.split("=");
+        headers.put(parts[0].trim(), parts[1].trim());
+        LOG.warn(parts[0].trim() + "=" + parts[1].trim());
+      }
+    } catch (Exception ex) {
+      LOG.warn("Could not parse the headers provided in " + 
ConfVars.METASTORE_CLIENT_ADDITIONAL_HEADERS, ex);
+    }
+    return headers;
+  }
+
   /*
   Creates a THttpClient if HTTP mode is enabled. If Client auth mode is set to 
JWT,
   then the method fetches JWT from environment variable: HMS_JWT and sets in 
auth
   header in http request
    */
-  private THttpClient createHttpClient(URI store, boolean useSSL) throws 
MetaException,
-      TTransportException {
+  private THttpClient createHttpClient(URI store, boolean useSSL) throws 
MetaException, TTransportException {
     String path = MetaStoreUtils.getHttpPath(MetastoreConf.getVar(conf, 
ConfVars.THRIFT_HTTP_PATH));
-    String httpUrl = (useSSL ? "https://"; : "http://";) + store.getHost() + ":" 
+ store.getPort() + path;
+    String urlScheme;
+    if (useSSL || Objects.equals(store.getScheme(), "https")) {
+      urlScheme = "https://";;
+    } else {
+      urlScheme = "http://";;
+    }
+    String httpUrl = urlScheme + store.getHost() + ":" + store.getPort() + 
path;
+
+    HttpClientBuilder httpClientBuilder = createHttpClientBuilder();
+    THttpClient tHttpClient;
+    try {
+      if (useSSL) {
+        String trustStorePath = MetastoreConf.getVar(conf, 
ConfVars.SSL_TRUSTSTORE_PATH).trim();
+        if (trustStorePath.isEmpty()) {
+          throw new IllegalArgumentException(ConfVars.SSL_TRUSTSTORE_PATH + " 
Not configured for SSL connection");
+        }
+        String trustStorePassword = MetastoreConf.getPassword(conf, 
MetastoreConf.ConfVars.SSL_TRUSTSTORE_PASSWORD);
+        String trustStoreType = MetastoreConf.getVar(conf, 
ConfVars.SSL_TRUSTSTORE_TYPE).trim();
+        String trustStoreAlgorithm = MetastoreConf.getVar(conf, 
ConfVars.SSL_TRUSTMANAGERFACTORY_ALGORITHM).trim();
+        tHttpClient =
+            SecurityUtils.getThriftHttpsClient(httpUrl, trustStorePath, 
trustStorePassword, trustStoreAlgorithm,
+                trustStoreType, httpClientBuilder);
+      } else {
+        tHttpClient = new THttpClient(httpUrl, httpClientBuilder.build());
+      }
+    } catch (Exception e) {
+      if (e instanceof TTransportException) {
+        throw (TTransportException) e;
+      } else {
+        throw new MetaException("Failed to create http transport client to 
url: " + httpUrl + ". Error:" + e);
+      }
+    }
+    LOG.debug("Created thrift http client for URL: " + httpUrl);
+    return configureThriftMaxMessageSize(tHttpClient);
+  }
 
+  protected HttpClientBuilder createHttpClientBuilder() throws MetaException {

Review Comment:
   keeping it as protected doesn't harm. I think this should be okay.



##########
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetastoreHttpHeaders.java:
##########
@@ -0,0 +1,103 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.annotation.MetastoreCheckinTest;
+import org.apache.hadoop.hive.metastore.api.Database;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.client.builder.DatabaseBuilder;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.security.HadoopThriftAuthBridge;
+import org.apache.http.Header;
+import org.apache.http.HttpException;
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpRequestInterceptor;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.protocol.HttpContext;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.io.IOException;
+
+import static org.junit.Assert.*;
+
+@Category(MetastoreCheckinTest.class) public class 
TestHiveMetastoreHttpHeaders {
+  private static Configuration conf;
+  private static HiveMetaStoreClient msc;
+  private static int port;
+  private static final String testHeaderKey1 = "X-XXXX";
+  private static final String testHeaderVal1 = "yyyy";
+  private static final String testHeaderKey2 = "X-ZZZZ";
+  private static final String testHeaderVal2 = "aaaa";
+
+  static class TestHiveMetaStoreClient extends HiveMetaStoreClient {
+    public TestHiveMetaStoreClient(Configuration conf) throws MetaException {
+      super(conf);
+    }
+
+    @Override protected HttpClientBuilder createHttpClientBuilder() throws 
MetaException {
+      HttpClientBuilder builder = super.createHttpClientBuilder();
+      builder.addInterceptorLast(new HttpRequestInterceptor() {
+        @Override public void process(HttpRequest httpRequest, HttpContext 
httpContext)
+            throws HttpException, IOException {
+          Header header1 = httpRequest.getFirstHeader(testHeaderKey1);
+          assertEquals(testHeaderVal1, header1.getValue());
+          Header header2 = httpRequest.getFirstHeader(testHeaderKey2);
+          assertEquals(testHeaderVal2, header2.getValue());
+        }
+      });
+      return builder;
+    }
+  }
+
+  @Before public void setUp() throws Exception {
+    conf = MetastoreConf.newMetastoreConf();
+
+    MetaStoreTestUtils.setConfForStandloneMode(conf);
+    MetastoreConf.setBoolVar(conf, MetastoreConf.ConfVars.EXECUTE_SET_UGI, 
false);
+    MetastoreConf.setVar(conf, MetastoreConf.ConfVars.THRIFT_TRANSPORT_MODE, 
"http");
+    MetastoreConf.setVar(conf, 
MetastoreConf.ConfVars.METASTORE_CLIENT_THRIFT_TRANSPORT_MODE, "http");
+    port = 
MetaStoreTestUtils.startMetaStoreWithRetry(HadoopThriftAuthBridge.getBridge(), 
conf);

Review Comment:
   Ah,  I see. That's interesting. In that case I think it is okay to follow 
existing pattern.



##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java:
##########
@@ -1588,6 +1588,10 @@ public enum ConfVars {
                     " and password. Any other value is ignored right now but 
may be used later."
                 + "If JWT- Supported only in HTTP transport mode. If set, HMS 
Client will pick the value of JWT from "
                 + "environment variable HMS_JWT and set it in Authorization 
header in http request"),
+

Review Comment:
   nit, please remove the extra line here.





Issue Time Tracking
-------------------

    Worklog Id:     (was: 850359)
    Time Spent: 1.5h  (was: 1h 20m)

> Enhanced support to Hive Client http support
> --------------------------------------------
>
>                 Key: HIVE-27129
>                 URL: https://issues.apache.org/jira/browse/HIVE-27129
>             Project: Hive
>          Issue Type: Improvement
>            Reporter: Junlin Zeng
>            Assignee: Junlin Zeng
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 1.5h
>  Remaining Estimate: 0h
>
> Currently we support using http in the hive metastore connection. However, we 
> do not support custom headers and also default trust store. This ticket 
> tracks the work to improve the http journey.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to