walterddr commented on a change in pull request #8329: URL: https://github.com/apache/pinot/pull/8329#discussion_r829216994
########## File path: pinot-common/src/main/java/org/apache/pinot/common/utils/TlsUtils.java ########## @@ -255,6 +257,14 @@ public static URL makeKeyStoreUrl(String storePath) return inputUri.toURL(); } + public static SSLContext getSslContext() { + return _sslContext != null ? _sslContext : SSLContexts.createDefault(); Review comment: i dont know whether this needs to be synchronized as it is only generated once. the only way to modify it is via setSslContext, which should also be statically accessed when the secure environment constructs at startup time). ########## File path: pinot-common/src/main/java/org/apache/pinot/common/utils/TlsUtils.java ########## @@ -255,6 +257,14 @@ public static URL makeKeyStoreUrl(String storePath) return inputUri.toURL(); } + public static SSLContext getSslContext() { + return _sslContext != null ? _sslContext : SSLContexts.createDefault(); Review comment: i dont know whether this needs to be synchronized as it is only generated once. the only way to modify it is via setSslContext, which should also be statically accessed when the secure environment constructs at startup time). CC the original author @apucher ^ ########## File path: pinot-common/src/main/java/org/apache/pinot/common/utils/TlsUtils.java ########## @@ -255,6 +257,14 @@ public static URL makeKeyStoreUrl(String storePath) return inputUri.toURL(); } + public static SSLContext getSslContext() { + return _sslContext != null ? _sslContext : SSLContexts.createDefault(); Review comment: yeah I was thinking about using lazy static initialization similar to https://stackoverflow.com/a/7420714 since it is static thread safe. However the current way how TLSUtils install the TLSSpec is non-static. so it might not be possible to acquire the TLSSpecs at jvm start up time. ########## File path: pinot-common/src/main/java/org/apache/pinot/common/utils/TlsUtils.java ########## @@ -255,6 +257,14 @@ public static URL makeKeyStoreUrl(String storePath) return inputUri.toURL(); } + public static SSLContext getSslContext() { + return _sslContext != null ? _sslContext : SSLContexts.createDefault(); Review comment: yeah. this is what I meant previously (less the `OVERRIDE` part which brilliantly solve the tls keystore init issue). thanks @richardstartin ! ########## File path: pinot-common/src/main/java/org/apache/pinot/common/utils/http/HttpClient.java ########## @@ -0,0 +1,364 @@ +/** + * 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.pinot.common.utils.http; + +import java.io.IOException; +import java.net.URI; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import javax.net.ssl.SSLContext; +import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.http.Header; +import org.apache.http.HttpEntity; +import org.apache.http.HttpHeaders; +import org.apache.http.HttpVersion; +import org.apache.http.NameValuePair; +import org.apache.http.StatusLine; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.client.methods.HttpPut; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.client.methods.RequestBuilder; +import org.apache.http.conn.ssl.NoopHostnameVerifier; +import org.apache.http.conn.ssl.SSLConnectionSocketFactory; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; +import org.apache.http.entity.mime.MultipartEntityBuilder; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.message.BasicHeader; +import org.apache.http.util.EntityUtils; +import org.apache.pinot.common.exception.HttpErrorStatusException; +import org.apache.pinot.common.utils.SimpleHttpErrorInfo; +import org.apache.pinot.common.utils.SimpleHttpResponse; +import org.apache.pinot.common.utils.TlsUtils; +import org.apache.pinot.spi.utils.CommonConstants; +import org.apache.pinot.spi.utils.JsonUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + + +/** + * The {@code HTTPClient} wraps around a {@link CloseableHttpClient} to provide a reusable client for making + * HTTP requests. + */ +public class HttpClient implements AutoCloseable { + private static final Logger LOGGER = LoggerFactory.getLogger(HttpClient.class); + + public static final int DEFAULT_SOCKET_TIMEOUT_MS = 600 * 1000; // 10 minutes + public static final int GET_REQUEST_SOCKET_TIMEOUT_MS = 5 * 1000; // 5 seconds + public static final int DELETE_REQUEST_SOCKET_TIMEOUT_MS = 10 * 1000; // 10 seconds + public static final String AUTH_HTTP_HEADER = "Authorization"; + public static final String JSON_CONTENT_TYPE = "application/json"; + + private final CloseableHttpClient _httpClient; + + public HttpClient() { + this(null); + } + + public HttpClient(@Nullable SSLContext sslContext) { + SSLContext context = sslContext != null ? sslContext : TlsUtils.getSslContext(); + // Set NoopHostnameVerifier to skip validating hostname when uploading/downloading segments. + SSLConnectionSocketFactory csf = new SSLConnectionSocketFactory(context, NoopHostnameVerifier.INSTANCE); + _httpClient = HttpClients.custom().setSSLSocketFactory(csf).build(); + } + + // -------------------------------------------------------------------------- + // Generic HTTP Request APIs + // -------------------------------------------------------------------------- + + /** + * Deprecated due to lack of auth header support. May break for deployments with auth enabled + * + * @see #sendGetRequest(URI, String) + */ + public SimpleHttpResponse sendGetRequest(URI uri) + throws IOException { + RequestBuilder requestBuilder = RequestBuilder.get(uri).setVersion(HttpVersion.HTTP_1_1); + setTimeout(requestBuilder, GET_REQUEST_SOCKET_TIMEOUT_MS); + return sendRequest(requestBuilder.build()); + } + + public SimpleHttpResponse sendGetRequest(URI uri, @Nullable String authToken) + throws IOException { + RequestBuilder requestBuilder = RequestBuilder.get(uri).setVersion(HttpVersion.HTTP_1_1); + if (StringUtils.isNotBlank(authToken)) { + requestBuilder.addHeader(AUTH_HTTP_HEADER, authToken); + } + setTimeout(requestBuilder, GET_REQUEST_SOCKET_TIMEOUT_MS); + return sendRequest(requestBuilder.build()); + } + + /** + * Deprecated due to lack of auth header support. May break for deployments with auth enabled + * + * @see #sendDeleteRequest(URI, String) + */ + public SimpleHttpResponse sendDeleteRequest(URI uri) + throws IOException { + RequestBuilder requestBuilder = RequestBuilder.delete(uri).setVersion(HttpVersion.HTTP_1_1); + setTimeout(requestBuilder, DELETE_REQUEST_SOCKET_TIMEOUT_MS); + return sendRequest(requestBuilder.build()); + } + + public SimpleHttpResponse sendDeleteRequest(URI uri, @Nullable String authToken) + throws IOException { + RequestBuilder requestBuilder = RequestBuilder.delete(uri).setVersion(HttpVersion.HTTP_1_1); + if (StringUtils.isNotBlank(authToken)) { + requestBuilder.addHeader(AUTH_HTTP_HEADER, authToken); + } + setTimeout(requestBuilder, DELETE_REQUEST_SOCKET_TIMEOUT_MS); + return sendRequest(requestBuilder.build()); + } + + /** + * Deprecated due to lack of auth header support. May break for deployments with auth enabled + * + * @see #sendPostRequest(URI, HttpEntity, Map, String) + */ + public SimpleHttpResponse sendPostRequest(URI uri, HttpEntity payload, Map<String, String> headers) + throws IOException { + return sendPostRequest(uri, payload, headers, null); + } + + public SimpleHttpResponse sendPostRequest(URI uri, HttpEntity payload, Map<String, String> headers, + @Nullable String authToken) + throws IOException { + RequestBuilder requestBuilder = RequestBuilder.post(uri).setVersion(HttpVersion.HTTP_1_1); + if (payload != null) { + requestBuilder.setEntity(payload); + } + if (StringUtils.isNotBlank(authToken)) { + requestBuilder.addHeader(AUTH_HTTP_HEADER, authToken); + } + if (MapUtils.isNotEmpty(headers)) { + for (Map.Entry<String, String> header : headers.entrySet()) { + requestBuilder.addHeader(header.getKey(), header.getValue()); + } + } + setTimeout(requestBuilder, DEFAULT_SOCKET_TIMEOUT_MS); + return sendRequest(requestBuilder.build()); + } + + /** + * Deprecated due to lack of auth header support. May break for deployments with auth enabled + * + * @see #sendPutRequest(URI, HttpEntity, Map, String) + */ + public SimpleHttpResponse sendPutRequest(URI uri, HttpEntity payload, Map<String, String> headers) + throws IOException { + return sendPutRequest(uri, payload, headers, null); + } + + public SimpleHttpResponse sendPutRequest(URI uri, HttpEntity payload, Map<String, String> headers, + @Nullable String authToken) + throws IOException { + RequestBuilder requestBuilder = RequestBuilder.put(uri).setVersion(HttpVersion.HTTP_1_1); + if (payload != null) { + requestBuilder.setEntity(payload); + } + if (StringUtils.isNotBlank(authToken)) { + requestBuilder.addHeader(AUTH_HTTP_HEADER, authToken); + } + setTimeout(requestBuilder, DELETE_REQUEST_SOCKET_TIMEOUT_MS); + return sendRequest(requestBuilder.build()); + } + + // -------------------------------------------------------------------------- + // JSON post/put utility APIs + // -------------------------------------------------------------------------- + + public SimpleHttpResponse postJsonRequest(URI uri, @Nullable String jsonRequestBody) Review comment: yeah weirdly we have some post method with empty body :-/ ########## File path: pinot-common/src/main/java/org/apache/pinot/common/utils/http/HttpClient.java ########## @@ -0,0 +1,364 @@ +/** + * 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.pinot.common.utils.http; + +import java.io.IOException; +import java.net.URI; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import javax.net.ssl.SSLContext; +import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.http.Header; +import org.apache.http.HttpEntity; +import org.apache.http.HttpHeaders; +import org.apache.http.HttpVersion; +import org.apache.http.NameValuePair; +import org.apache.http.StatusLine; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.client.methods.HttpPut; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.client.methods.RequestBuilder; +import org.apache.http.conn.ssl.NoopHostnameVerifier; +import org.apache.http.conn.ssl.SSLConnectionSocketFactory; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; +import org.apache.http.entity.mime.MultipartEntityBuilder; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.message.BasicHeader; +import org.apache.http.util.EntityUtils; +import org.apache.pinot.common.exception.HttpErrorStatusException; +import org.apache.pinot.common.utils.SimpleHttpErrorInfo; +import org.apache.pinot.common.utils.SimpleHttpResponse; +import org.apache.pinot.common.utils.TlsUtils; +import org.apache.pinot.spi.utils.CommonConstants; +import org.apache.pinot.spi.utils.JsonUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + + +/** + * The {@code HTTPClient} wraps around a {@link CloseableHttpClient} to provide a reusable client for making + * HTTP requests. + */ +public class HttpClient implements AutoCloseable { + private static final Logger LOGGER = LoggerFactory.getLogger(HttpClient.class); + + public static final int DEFAULT_SOCKET_TIMEOUT_MS = 600 * 1000; // 10 minutes + public static final int GET_REQUEST_SOCKET_TIMEOUT_MS = 5 * 1000; // 5 seconds + public static final int DELETE_REQUEST_SOCKET_TIMEOUT_MS = 10 * 1000; // 10 seconds + public static final String AUTH_HTTP_HEADER = "Authorization"; + public static final String JSON_CONTENT_TYPE = "application/json"; + + private final CloseableHttpClient _httpClient; + + public HttpClient() { + this(null); + } + + public HttpClient(@Nullable SSLContext sslContext) { + SSLContext context = sslContext != null ? sslContext : TlsUtils.getSslContext(); + // Set NoopHostnameVerifier to skip validating hostname when uploading/downloading segments. + SSLConnectionSocketFactory csf = new SSLConnectionSocketFactory(context, NoopHostnameVerifier.INSTANCE); + _httpClient = HttpClients.custom().setSSLSocketFactory(csf).build(); + } + + // -------------------------------------------------------------------------- + // Generic HTTP Request APIs + // -------------------------------------------------------------------------- + + /** + * Deprecated due to lack of auth header support. May break for deployments with auth enabled + * + * @see #sendGetRequest(URI, String) + */ + public SimpleHttpResponse sendGetRequest(URI uri) + throws IOException { + RequestBuilder requestBuilder = RequestBuilder.get(uri).setVersion(HttpVersion.HTTP_1_1); + setTimeout(requestBuilder, GET_REQUEST_SOCKET_TIMEOUT_MS); + return sendRequest(requestBuilder.build()); + } + + public SimpleHttpResponse sendGetRequest(URI uri, @Nullable String authToken) + throws IOException { + RequestBuilder requestBuilder = RequestBuilder.get(uri).setVersion(HttpVersion.HTTP_1_1); + if (StringUtils.isNotBlank(authToken)) { + requestBuilder.addHeader(AUTH_HTTP_HEADER, authToken); + } + setTimeout(requestBuilder, GET_REQUEST_SOCKET_TIMEOUT_MS); + return sendRequest(requestBuilder.build()); + } + + /** + * Deprecated due to lack of auth header support. May break for deployments with auth enabled + * + * @see #sendDeleteRequest(URI, String) + */ + public SimpleHttpResponse sendDeleteRequest(URI uri) + throws IOException { + RequestBuilder requestBuilder = RequestBuilder.delete(uri).setVersion(HttpVersion.HTTP_1_1); + setTimeout(requestBuilder, DELETE_REQUEST_SOCKET_TIMEOUT_MS); + return sendRequest(requestBuilder.build()); + } + + public SimpleHttpResponse sendDeleteRequest(URI uri, @Nullable String authToken) + throws IOException { + RequestBuilder requestBuilder = RequestBuilder.delete(uri).setVersion(HttpVersion.HTTP_1_1); + if (StringUtils.isNotBlank(authToken)) { + requestBuilder.addHeader(AUTH_HTTP_HEADER, authToken); + } + setTimeout(requestBuilder, DELETE_REQUEST_SOCKET_TIMEOUT_MS); + return sendRequest(requestBuilder.build()); + } + + /** + * Deprecated due to lack of auth header support. May break for deployments with auth enabled + * + * @see #sendPostRequest(URI, HttpEntity, Map, String) + */ + public SimpleHttpResponse sendPostRequest(URI uri, HttpEntity payload, Map<String, String> headers) + throws IOException { + return sendPostRequest(uri, payload, headers, null); + } + + public SimpleHttpResponse sendPostRequest(URI uri, HttpEntity payload, Map<String, String> headers, + @Nullable String authToken) + throws IOException { + RequestBuilder requestBuilder = RequestBuilder.post(uri).setVersion(HttpVersion.HTTP_1_1); + if (payload != null) { + requestBuilder.setEntity(payload); + } + if (StringUtils.isNotBlank(authToken)) { + requestBuilder.addHeader(AUTH_HTTP_HEADER, authToken); + } + if (MapUtils.isNotEmpty(headers)) { + for (Map.Entry<String, String> header : headers.entrySet()) { + requestBuilder.addHeader(header.getKey(), header.getValue()); + } + } + setTimeout(requestBuilder, DEFAULT_SOCKET_TIMEOUT_MS); + return sendRequest(requestBuilder.build()); + } + + /** + * Deprecated due to lack of auth header support. May break for deployments with auth enabled + * + * @see #sendPutRequest(URI, HttpEntity, Map, String) + */ + public SimpleHttpResponse sendPutRequest(URI uri, HttpEntity payload, Map<String, String> headers) + throws IOException { + return sendPutRequest(uri, payload, headers, null); + } + + public SimpleHttpResponse sendPutRequest(URI uri, HttpEntity payload, Map<String, String> headers, + @Nullable String authToken) + throws IOException { + RequestBuilder requestBuilder = RequestBuilder.put(uri).setVersion(HttpVersion.HTTP_1_1); + if (payload != null) { + requestBuilder.setEntity(payload); + } + if (StringUtils.isNotBlank(authToken)) { + requestBuilder.addHeader(AUTH_HTTP_HEADER, authToken); + } + setTimeout(requestBuilder, DELETE_REQUEST_SOCKET_TIMEOUT_MS); + return sendRequest(requestBuilder.build()); + } + + // -------------------------------------------------------------------------- + // JSON post/put utility APIs + // -------------------------------------------------------------------------- + + public SimpleHttpResponse postJsonRequest(URI uri, @Nullable String jsonRequestBody) + throws IOException { + return postJsonRequest(uri, jsonRequestBody, null); + } + + public SimpleHttpResponse postJsonRequest(URI uri, @Nullable String requestBody, + @Nullable Map<String, String> headers) + throws IOException { + if (MapUtils.isEmpty(headers)) { + headers = new HashMap<>(); + } + headers.put(HttpHeaders.CONTENT_TYPE, JSON_CONTENT_TYPE); + HttpEntity entity = requestBody == null ? null : new StringEntity(requestBody, ContentType.APPLICATION_JSON); + return sendPostRequest(uri, entity, headers, null); + } + + public SimpleHttpResponse putJsonRequest(URI uri, @Nullable String jsonRequestBody) + throws IOException { + return putJsonRequest(uri, jsonRequestBody, null); + } + + public SimpleHttpResponse putJsonRequest(URI uri, @Nullable String requestBody, + @Nullable Map<String, String> headers) + throws IOException { + if (MapUtils.isEmpty(headers)) { + headers = new HashMap<>(); + } + headers.put(HttpHeaders.CONTENT_TYPE, JSON_CONTENT_TYPE); + HttpEntity entity = requestBody == null ? null : new StringEntity(requestBody, ContentType.APPLICATION_JSON); + return sendPutRequest(uri, entity, headers, null); + } + + // -------------------------------------------------------------------------- + // Lower-level request/execute APIs. + // -------------------------------------------------------------------------- + + public SimpleHttpResponse sendRequest(HttpUriRequest request) + throws IOException { + try (CloseableHttpResponse response = _httpClient.execute(request)) { + if (response.containsHeader(CommonConstants.Controller.HOST_HTTP_HEADER)) { + String controllerHost = response.getFirstHeader(CommonConstants.Controller.HOST_HTTP_HEADER).getValue(); + String controllerVersion = response.getFirstHeader(CommonConstants.Controller.VERSION_HTTP_HEADER).getValue(); + LOGGER.info(String + .format("Sending request: %s to controller: %s, version: %s", request.getURI(), controllerHost, + controllerVersion)); + } + int statusCode = response.getStatusLine().getStatusCode(); + if (statusCode >= 300) { + return new SimpleHttpResponse(statusCode, getErrorMessage(request, response)); + } + return new SimpleHttpResponse(statusCode, EntityUtils.toString(response.getEntity())); + } + } + + public CloseableHttpResponse execute(HttpUriRequest request) + throws IOException { + return _httpClient.execute(request); + } + + // -------------------------------------------------------------------------- + // Multi-part post/put APIs. + // -------------------------------------------------------------------------- + + public SimpleHttpResponse sendMultipartPostRequest(String url, String body) + throws IOException { + HttpPost post = new HttpPost(url); + // our handlers ignore key...so we can put anything here + MultipartEntityBuilder builder = MultipartEntityBuilder.create(); + builder.addTextBody("body", body); + post.setEntity(builder.build()); Review comment: for now i dont see it be useful. so i would prefer to add it when it is used. (also tried to use sendPostRequest it had some weird test errors) ########## File path: pinot-common/src/main/java/org/apache/pinot/common/utils/http/HttpClient.java ########## @@ -0,0 +1,364 @@ +/** + * 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.pinot.common.utils.http; + +import java.io.IOException; +import java.net.URI; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import javax.net.ssl.SSLContext; +import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.http.Header; +import org.apache.http.HttpEntity; +import org.apache.http.HttpHeaders; +import org.apache.http.HttpVersion; +import org.apache.http.NameValuePair; +import org.apache.http.StatusLine; +import org.apache.http.client.config.RequestConfig; +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpPost; +import org.apache.http.client.methods.HttpPut; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.client.methods.RequestBuilder; +import org.apache.http.conn.ssl.NoopHostnameVerifier; +import org.apache.http.conn.ssl.SSLConnectionSocketFactory; +import org.apache.http.entity.ContentType; +import org.apache.http.entity.StringEntity; +import org.apache.http.entity.mime.MultipartEntityBuilder; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClients; +import org.apache.http.message.BasicHeader; +import org.apache.http.util.EntityUtils; +import org.apache.pinot.common.exception.HttpErrorStatusException; +import org.apache.pinot.common.utils.SimpleHttpErrorInfo; +import org.apache.pinot.common.utils.SimpleHttpResponse; +import org.apache.pinot.common.utils.TlsUtils; +import org.apache.pinot.spi.utils.CommonConstants; +import org.apache.pinot.spi.utils.JsonUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + + +/** + * The {@code HTTPClient} wraps around a {@link CloseableHttpClient} to provide a reusable client for making + * HTTP requests. + */ +public class HttpClient implements AutoCloseable { + private static final Logger LOGGER = LoggerFactory.getLogger(HttpClient.class); + + public static final int DEFAULT_SOCKET_TIMEOUT_MS = 600 * 1000; // 10 minutes + public static final int GET_REQUEST_SOCKET_TIMEOUT_MS = 5 * 1000; // 5 seconds + public static final int DELETE_REQUEST_SOCKET_TIMEOUT_MS = 10 * 1000; // 10 seconds + public static final String AUTH_HTTP_HEADER = "Authorization"; + public static final String JSON_CONTENT_TYPE = "application/json"; + + private final CloseableHttpClient _httpClient; + + public HttpClient() { + this(null); + } + + public HttpClient(@Nullable SSLContext sslContext) { + SSLContext context = sslContext != null ? sslContext : TlsUtils.getSslContext(); + // Set NoopHostnameVerifier to skip validating hostname when uploading/downloading segments. + SSLConnectionSocketFactory csf = new SSLConnectionSocketFactory(context, NoopHostnameVerifier.INSTANCE); + _httpClient = HttpClients.custom().setSSLSocketFactory(csf).build(); + } + + // -------------------------------------------------------------------------- + // Generic HTTP Request APIs + // -------------------------------------------------------------------------- + + /** + * Deprecated due to lack of auth header support. May break for deployments with auth enabled + * + * @see #sendGetRequest(URI, String) + */ + public SimpleHttpResponse sendGetRequest(URI uri) + throws IOException { + RequestBuilder requestBuilder = RequestBuilder.get(uri).setVersion(HttpVersion.HTTP_1_1); + setTimeout(requestBuilder, GET_REQUEST_SOCKET_TIMEOUT_MS); + return sendRequest(requestBuilder.build()); + } + + public SimpleHttpResponse sendGetRequest(URI uri, @Nullable String authToken) Review comment: this would be a major refactoring. should we do this in a separate PR? ########## File path: pinot-common/src/main/java/org/apache/pinot/common/utils/TlsUtils.java ########## @@ -232,8 +235,7 @@ public static void installDefaultSSLSocketFactory(String keyStoreType, String ke Protocol.registerProtocol("https", new Protocol(CommonConstants.HTTPS_PROTOCOL, new PinotProtocolSocketFactory(sc.getSocketFactory()), 443)); - // FileUploadDownloadClient - FileUploadDownloadClient.installDefaultSSLContext(sc); + _sslContext = sc; Review comment: yup. i guess it aligns with other comments will address it accordingly ########## File path: pinot-common/src/main/java/org/apache/pinot/common/utils/TlsUtils.java ########## @@ -60,6 +61,8 @@ private static final String TRUSTSTORE_PASSWORD = "truststore.password"; private static final String SSL_PROVIDER = "ssl.provider"; + private static SSLContext _sslContext = null; Review comment: seems it only apply to private static final variables -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org