From 512b8be7914ca66d0e4648ce0095b63bc508b474 Mon Sep 17 00:00:00 2001 From: javanna <cavannaluca@gmail.com> Date: Tue, 12 Jul 2016 11:29:00 +0200 Subject: [PATCH] RestClient: simplify ssl configuration and make http config callback functional friendly --- .../org/elasticsearch/client/RestClient.java | 38 +++++++++---- .../SSLSocketFactoryHttpConfigCallback.java | 53 +++++++++++++++++++ .../client/RestClientBuilderTests.java | 20 ++++--- .../elasticsearch/http/HttpCompressionIT.java | 5 -- .../test/rest/client/RestTestClient.java | 46 +++++----------- 5 files changed, 106 insertions(+), 56 deletions(-) create mode 100644 client/rest/src/main/java/org/elasticsearch/client/SSLSocketFactoryHttpConfigCallback.java diff --git a/client/rest/src/main/java/org/elasticsearch/client/RestClient.java b/client/rest/src/main/java/org/elasticsearch/client/RestClient.java index 804fe7821d7..0d6941c7857 100644 --- a/client/rest/src/main/java/org/elasticsearch/client/RestClient.java +++ b/client/rest/src/main/java/org/elasticsearch/client/RestClient.java @@ -395,6 +395,7 @@ public final class RestClient implements Closeable { private Header[] defaultHeaders = EMPTY_HEADERS; private FailureListener failureListener; private HttpClientConfigCallback httpClientConfigCallback; + private RequestConfigCallback requestConfigCallback; /** * Creates a new builder instance and sets the hosts that the client will send requests to. @@ -450,6 +451,15 @@ public final class RestClient implements Closeable { return this; } + /** + * Sets the {@link RequestConfigCallback} to be used to customize http client configuration + */ + public Builder setRequestConfigCallback(RequestConfigCallback requestConfigCallback) { + Objects.requireNonNull(requestConfigCallback, "requestConfigCallback must not be null"); + this.requestConfigCallback = requestConfigCallback; + return this; + } + /** * Creates a new {@link RestClient} based on the provided configuration. */ @@ -467,8 +477,8 @@ public final class RestClient implements Closeable { .setSocketTimeout(DEFAULT_SOCKET_TIMEOUT_MILLIS) .setConnectionRequestTimeout(DEFAULT_CONNECTION_REQUEST_TIMEOUT_MILLIS); - if (httpClientConfigCallback != null) { - httpClientConfigCallback.customizeDefaultRequestConfig(requestConfigBuilder); + if (requestConfigCallback != null) { + requestConfigCallback.customizeRequestConfig(requestConfigBuilder); } RequestConfig requestConfig = requestConfigBuilder.build(); @@ -487,24 +497,30 @@ public final class RestClient implements Closeable { } } + /** + * Callback used the default {@link RequestConfig} being set to the {@link CloseableHttpClient} + * @see HttpClientBuilder#setDefaultRequestConfig + */ + public interface RequestConfigCallback { + /** + * Allows to customize the {@link RequestConfig} that will be used with each request. + * It is common to customize the different timeout values through this method without losing any other useful default + * value that the {@link RestClient.Builder} internally sets. + */ + void customizeRequestConfig(RequestConfig.Builder requestConfigBuilder); + } + /** * Callback used to customize the {@link CloseableHttpClient} instance used by a {@link RestClient} instance. * Allows to customize default {@link RequestConfig} being set to the client and any parameter that * can be set through {@link HttpClientBuilder} */ public interface HttpClientConfigCallback { - /** - * Allows to customize the {@link RequestConfig} that will be used with each request. - * It is common to customize the different timeout values through this method without losing any other useful default - * value that the {@link RestClient.Builder} sets internally. - */ - void customizeDefaultRequestConfig(RequestConfig.Builder requestConfigBuilder); - /** * Allows to customize the {@link CloseableHttpClient} being created and used by the {@link RestClient}. * It is common to customzie the default {@link org.apache.http.client.CredentialsProvider} through this method, - * or the {@link org.apache.http.conn.HttpClientConnectionManager} being used without losing any other useful default - * value that the {@link RestClient.Builder} sets internally. + * without losing any other useful default value that the {@link RestClient.Builder} internally sets. + * Also useful to setup ssl through {@link SSLSocketFactoryHttpConfigCallback}. */ void customizeHttpClient(HttpClientBuilder httpClientBuilder); } diff --git a/client/rest/src/main/java/org/elasticsearch/client/SSLSocketFactoryHttpConfigCallback.java b/client/rest/src/main/java/org/elasticsearch/client/SSLSocketFactoryHttpConfigCallback.java new file mode 100644 index 00000000000..3f18f9938c1 --- /dev/null +++ b/client/rest/src/main/java/org/elasticsearch/client/SSLSocketFactoryHttpConfigCallback.java @@ -0,0 +1,53 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch 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.elasticsearch.client; + +import org.apache.http.config.Registry; +import org.apache.http.config.RegistryBuilder; +import org.apache.http.conn.socket.ConnectionSocketFactory; +import org.apache.http.conn.socket.PlainConnectionSocketFactory; +import org.apache.http.conn.ssl.SSLConnectionSocketFactory; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; + +/** + * Helps configuring the http client when needing to communicate over ssl. It effectively replaces the connection manager + * with one that has ssl properly configured thanks to the provided {@link SSLConnectionSocketFactory}. + */ +public class SSLSocketFactoryHttpConfigCallback implements RestClient.HttpClientConfigCallback { + + private final SSLConnectionSocketFactory sslSocketFactory; + + public SSLSocketFactoryHttpConfigCallback(SSLConnectionSocketFactory sslSocketFactory) { + this.sslSocketFactory = sslSocketFactory; + } + + @Override + public void customizeHttpClient(HttpClientBuilder httpClientBuilder) { + Registry<ConnectionSocketFactory> socketFactoryRegistry = RegistryBuilder.<ConnectionSocketFactory>create() + .register("http", PlainConnectionSocketFactory.getSocketFactory()) + .register("https", sslSocketFactory).build(); + PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(socketFactoryRegistry); + //default settings may be too constraining + connectionManager.setDefaultMaxPerRoute(10); + connectionManager.setMaxTotal(30); + httpClientBuilder.setConnectionManager(connectionManager); + } +} diff --git a/client/rest/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java b/client/rest/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java index 001c14e8355..f032933db24 100644 --- a/client/rest/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java +++ b/client/rest/src/test/java/org/elasticsearch/client/RestClientBuilderTests.java @@ -92,6 +92,13 @@ public class RestClientBuilderTests extends RestClientTestCase { assertEquals("httpClientConfigCallback must not be null", e.getMessage()); } + try { + RestClient.builder(new HttpHost("localhost", 9200)).setRequestConfigCallback(null); + fail("should have failed"); + } catch(NullPointerException e) { + assertEquals("requestConfigCallback must not be null", e.getMessage()); + } + int numNodes = RandomInts.randomIntBetween(getRandom(), 1, 5); HttpHost[] hosts = new HttpHost[numNodes]; for (int i = 0; i < numNodes; i++) { @@ -100,14 +107,15 @@ public class RestClientBuilderTests extends RestClientTestCase { RestClient.Builder builder = RestClient.builder(hosts); if (getRandom().nextBoolean()) { builder.setHttpClientConfigCallback(new RestClient.HttpClientConfigCallback() { - @Override - public void customizeDefaultRequestConfig(RequestConfig.Builder requestConfigBuilder) { - - } - @Override public void customizeHttpClient(HttpClientBuilder httpClientBuilder) { - + } + }); + } + if (getRandom().nextBoolean()) { + builder.setRequestConfigCallback(new RestClient.RequestConfigCallback() { + @Override + public void customizeRequestConfig(RequestConfig.Builder requestConfigBuilder) { } }); } diff --git a/qa/smoke-test-http/src/test/java/org/elasticsearch/http/HttpCompressionIT.java b/qa/smoke-test-http/src/test/java/org/elasticsearch/http/HttpCompressionIT.java index 292d453c725..6319d494e16 100644 --- a/qa/smoke-test-http/src/test/java/org/elasticsearch/http/HttpCompressionIT.java +++ b/qa/smoke-test-http/src/test/java/org/elasticsearch/http/HttpCompressionIT.java @@ -22,7 +22,6 @@ import org.apache.http.Header; import org.apache.http.HttpException; import org.apache.http.HttpHeaders; import org.apache.http.HttpResponseInterceptor; -import org.apache.http.client.config.RequestConfig; import org.apache.http.entity.StringEntity; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.message.BasicHeader; @@ -138,10 +137,6 @@ public class HttpCompressionIT extends ESIntegTestCase { this.contentEncodingHeaderExtractor = contentEncodingHeaderExtractor; } - @Override - public void customizeDefaultRequestConfig(RequestConfig.Builder requestConfigBuilder) { - } - @Override public void customizeHttpClient(HttpClientBuilder httpClientBuilder) { httpClientBuilder.addInterceptorFirst(contentEncodingHeaderExtractor); diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/client/RestTestClient.java b/test/framework/src/main/java/org/elasticsearch/test/rest/client/RestTestClient.java index 04801ade863..c97ca7cd2fd 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/client/RestTestClient.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/client/RestTestClient.java @@ -22,14 +22,8 @@ import com.carrotsearch.randomizedtesting.RandomizedTest; import org.apache.http.Header; import org.apache.http.HttpEntity; import org.apache.http.HttpHost; -import org.apache.http.client.config.RequestConfig; -import org.apache.http.config.Registry; -import org.apache.http.config.RegistryBuilder; -import org.apache.http.conn.socket.ConnectionSocketFactory; import org.apache.http.conn.ssl.SSLConnectionSocketFactory; import org.apache.http.entity.StringEntity; -import org.apache.http.impl.client.HttpClientBuilder; -import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.apache.http.message.BasicHeader; import org.apache.http.ssl.SSLContexts; import org.apache.lucene.util.IOUtils; @@ -37,6 +31,7 @@ import org.elasticsearch.Version; import org.elasticsearch.client.Response; import org.elasticsearch.client.ResponseException; import org.elasticsearch.client.RestClient; +import org.elasticsearch.client.SSLSocketFactoryHttpConfigCallback; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.logging.ESLogger; @@ -267,7 +262,15 @@ public class RestTestClient implements Closeable { } private static RestClient createRestClient(URL[] urls, Settings settings) throws IOException { - PoolingHttpClientConnectionManager connectionManager; + String protocol = settings.get(PROTOCOL, "http"); + HttpHost[] hosts = new HttpHost[urls.length]; + for (int i = 0; i < hosts.length; i++) { + URL url = urls[i]; + hosts[i] = new HttpHost(url.getHost(), url.getPort(), protocol); + } + RestClient.Builder builder = RestClient.builder(hosts).setMaxRetryTimeoutMillis(30000) + .setRequestConfigCallback(requestConfigBuilder -> requestConfigBuilder.setSocketTimeout(30000)); + String keystorePath = settings.get(TRUSTSTORE_PATH); if (keystorePath != null) { final String keystorePass = settings.get(TRUSTSTORE_PASSWORD); @@ -284,38 +287,13 @@ public class RestTestClient implements Closeable { keyStore.load(is, keystorePass.toCharArray()); } SSLContext sslcontext = SSLContexts.custom().loadTrustMaterial(keyStore, null).build(); - Registry<ConnectionSocketFactory> socketFactoryRegistry = RegistryBuilder.<ConnectionSocketFactory>create() - .register("https", new SSLConnectionSocketFactory(sslcontext)).build(); - connectionManager = new PoolingHttpClientConnectionManager(socketFactoryRegistry); + SSLConnectionSocketFactory sslConnectionSocketFactory = new SSLConnectionSocketFactory(sslcontext); + builder.setHttpClientConfigCallback(new SSLSocketFactoryHttpConfigCallback(sslConnectionSocketFactory)); } catch (KeyStoreException|NoSuchAlgorithmException|KeyManagementException|CertificateException e) { throw new RuntimeException(e); } - } else { - connectionManager = new PoolingHttpClientConnectionManager(); - } - //default settings may be too constraining - connectionManager.setDefaultMaxPerRoute(10); - connectionManager.setMaxTotal(30); - - String protocol = settings.get(PROTOCOL, "http"); - HttpHost[] hosts = new HttpHost[urls.length]; - for (int i = 0; i < hosts.length; i++) { - URL url = urls[i]; - hosts[i] = new HttpHost(url.getHost(), url.getPort(), protocol); } - RestClient.Builder builder = RestClient.builder(hosts).setMaxRetryTimeoutMillis(30000) - .setHttpClientConfigCallback(new RestClient.HttpClientConfigCallback() { - @Override - public void customizeDefaultRequestConfig(RequestConfig.Builder requestConfigBuilder) { - requestConfigBuilder.setSocketTimeout(30000); - } - - @Override - public void customizeHttpClient(HttpClientBuilder httpClientBuilder) { - httpClientBuilder.setConnectionManager(connectionManager); - } - }); try (ThreadContext threadContext = new ThreadContext(settings)) { Header[] defaultHeaders = new Header[threadContext.getHeaders().size()]; int i = 0;