You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "junlinzeng-db (via GitHub)" <gi...@apache.org> on 2023/03/09 05:08:57 UTC
[GitHub] [hive] junlinzeng-db opened a new pull request, #4104: [HIVE-27129] Add enhanced support for Hive Metastore Client http support
junlinzeng-db opened a new pull request, #4104:
URL: https://github.com/apache/hive/pull/4104
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
3. Ensure you have added or run the appropriate tests for your PR:
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX: Your PR title ...'.
5. Be sure to keep the PR description updated to reflect all changes.
6. Please write your PR title to summarize what this PR proposes.
7. If possible, provide a concise example to reproduce the issue for a faster review.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
If no, write 'No'.
-->
This PR targets to improve the usability of the http hive metastore client, by adding support for custom headers and also, supporting default trust store for https.
1. Add a new hive conf to allow users to inject custom headers
2. Do not force the headers to be http when the useSSL is turned off, so that if the users are able to use https if they want, w/o a custom truststore.
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
ut for the http headers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4104: [HIVE-27129] Add enhanced support for Hive Metastore Client http support
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4104:
URL: https://github.com/apache/hive/pull/4104#issuecomment-1461786578
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4104)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4104&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4104&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4104&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=CODE_SMELL) [4 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4104&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4104&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] vihangk1 commented on a diff in pull request #4104: [HIVE-27129] Add enhanced support for Hive Metastore Client http support
Posted by "vihangk1 (via GitHub)" <gi...@apache.org>.
vihangk1 commented on code in PR #4104:
URL: https://github.com/apache/hive/pull/4104#discussion_r1130492724
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -35,25 +35,15 @@
import java.net.UnknownHostException;
import java.nio.ByteBuffer;
import java.security.PrivilegedExceptionAction;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
Review Comment:
We generally don't use wild-card imports. Can you please revert this part and explicitly add the necessary imports (You may have to change your IDE settings so that it doesn't do this for automatically).
##########
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());
Review Comment:
Is this log needed? If not, let's remove it or change it to debug.
##########
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:
Please annotate this with `@VisibleForTesting`
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -659,42 +702,16 @@ public void process(HttpRequest httpRequest, HttpContext httpContext)
}
final String httpUser = user;
httpClientBuilder.addInterceptorFirst(new HttpRequestInterceptor() {
- @Override
- public void process(HttpRequest httpRequest, HttpContext httpContext)
+ @Override public void process(HttpRequest httpRequest, HttpContext httpContext)
Review Comment:
same here.
##########
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 {
Review Comment:
Please reformat this file once you have your IDE setup with the formatter above to fix code-style issues.
##########
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:
This method can be in a `BeforeClass` I guess. Also, you probably need a `AfterClass` to shutdown the metastore that you started just to make sure that you clean up after the test completes.
##########
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 {
HttpClientBuilder httpClientBuilder = HttpClientBuilder.create();
String authType = MetastoreConf.getVar(conf, ConfVars.METASTORE_CLIENT_AUTH_MODE);
+ Map<String, String> additionalHeaders = getAdditionalHeaders();
if (authType.equalsIgnoreCase("jwt")) {
// fetch JWT token from environment and set it in Auth Header in HTTP request
String jwtToken = System.getenv("HMS_JWT");
if (jwtToken == null || jwtToken.isEmpty()) {
LOG.debug("No jwt token set in environment variable: HMS_JWT");
- throw new MetaException("For auth mode JWT, valid signed jwt token must be provided in the "
- + "environment variable HMS_JWT");
+ throw new MetaException(
Review Comment:
nit, unintentional new line?
##########
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 {
HttpClientBuilder httpClientBuilder = HttpClientBuilder.create();
String authType = MetastoreConf.getVar(conf, ConfVars.METASTORE_CLIENT_AUTH_MODE);
+ Map<String, String> additionalHeaders = getAdditionalHeaders();
if (authType.equalsIgnoreCase("jwt")) {
// fetch JWT token from environment and set it in Auth Header in HTTP request
String jwtToken = System.getenv("HMS_JWT");
if (jwtToken == null || jwtToken.isEmpty()) {
LOG.debug("No jwt token set in environment variable: HMS_JWT");
- throw new MetaException("For auth mode JWT, valid signed jwt token must be provided in the "
- + "environment variable HMS_JWT");
+ throw new MetaException(
+ "For auth mode JWT, valid signed jwt token must be provided in the " + "environment variable HMS_JWT");
}
httpClientBuilder.addInterceptorFirst(new HttpRequestInterceptor() {
- @Override
- public void process(HttpRequest httpRequest, HttpContext httpContext)
+ @Override public void process(HttpRequest httpRequest, HttpContext httpContext)
Review Comment:
You may want set up your IDE to change the it's default formatting style. There are instructions in https://cwiki.apache.org/confluence/display/Hive/HowToContribute#HowToContribute-CodingConventions
Generally the `@Override` is in a separate line above `public void process` as per the convention.
##########
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.*;
Review Comment:
avoid wildcard imports.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] junlinzeng-db commented on a diff in pull request #4104: [HIVE-27129] Add enhanced support for Hive Metastore Client http support
Posted by "junlinzeng-db (via GitHub)" <gi...@apache.org>.
junlinzeng-db commented on code in PR #4104:
URL: https://github.com/apache/hive/pull/4104#discussion_r1131853638
##########
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:
I think most of the Hive test do not do tear down. I was mostly copying them. I would prefer to not change it if you are ok
https://github.com/apache/hive/blob/cd53e0a4d9b99f825beb7a7f5c9acc1b3e822edd/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStoreIpAddress.java#L42
https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStore.java#L52
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4104: HIVE-27129: Add enhanced support for Hive Metastore Client http support
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4104:
URL: https://github.com/apache/hive/pull/4104#issuecomment-1464245830
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4104)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4104&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4104&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4104&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4104&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4104&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4104: [HIVE-27129] Add enhanced support for Hive Metastore Client http support
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4104:
URL: https://github.com/apache/hive/pull/4104#issuecomment-1463500119
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4104)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4104&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4104&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4104&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=CODE_SMELL) [2 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4104&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4104&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4104&metric=duplicated_lines_density&view=list) No Duplication information
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] vihangk1 commented on pull request #4104: HIVE-27129: Add enhanced support for Hive Metastore Client http support
Posted by "vihangk1 (via GitHub)" <gi...@apache.org>.
vihangk1 commented on PR #4104:
URL: https://github.com/apache/hive/pull/4104#issuecomment-1464180280
@junlinzeng-db Jfyi, I fixed the description and the summary title of the PR as the convention.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] junlinzeng-db commented on a diff in pull request #4104: [HIVE-27129] Add enhanced support for Hive Metastore Client http support
Posted by "junlinzeng-db (via GitHub)" <gi...@apache.org>.
junlinzeng-db commented on code in PR #4104:
URL: https://github.com/apache/hive/pull/4104#discussion_r1131834462
##########
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:
do I still need this to be protocted?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] junlinzeng-db commented on a diff in pull request #4104: [HIVE-27129] Add enhanced support for Hive Metastore Client http support
Posted by "junlinzeng-db (via GitHub)" <gi...@apache.org>.
junlinzeng-db commented on code in PR #4104:
URL: https://github.com/apache/hive/pull/4104#discussion_r1131853638
##########
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:
I was mostly copying https://github.com/apache/hive/blob/cd53e0a4d9b99f825beb7a7f5c9acc1b3e822edd/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestRemoteHiveMetaStoreIpAddress.java#L42, but sure I can make it happen
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] junlinzeng-db commented on a diff in pull request #4104: [HIVE-27129] Add enhanced support for Hive Metastore Client http support
Posted by "junlinzeng-db (via GitHub)" <gi...@apache.org>.
junlinzeng-db commented on code in PR #4104:
URL: https://github.com/apache/hive/pull/4104#discussion_r1131832954
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -35,25 +35,15 @@
import java.net.UnknownHostException;
import java.nio.ByteBuffer;
import java.security.PrivilegedExceptionAction;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
Review Comment:
yeah it was my IDE. fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] vihangk-db commented on a diff in pull request #4104: [HIVE-27129] Add enhanced support for Hive Metastore Client http support
Posted by "vihangk-db (via GitHub)" <gi...@apache.org>.
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] vihangk1 merged pull request #4104: HIVE-27129: Add enhanced support for Hive Metastore Client http support
Posted by "vihangk1 (via GitHub)" <gi...@apache.org>.
vihangk1 merged PR #4104:
URL: https://github.com/apache/hive/pull/4104
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org