diff options
author | Bjørn Christian Seime <bjorn.christian@seime.no> | 2018-08-15 10:04:36 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-08-15 10:04:36 +0200 |
commit | 325cf9f1ec7ce5adc3d68832d48eb926cea678f4 (patch) | |
tree | 073c442aec9e663533d8582cb902b3ef68036ee6 /node-admin | |
parent | 9a402fd71acf3af060302600ab436ed256069fcf (diff) | |
parent | ede5e3f05093202bf9cbcb525cd49c6be6060b28 (diff) |
Merge pull request #6578 from vespa-engine/bjorncs/si-ssl-socket-factory
Bjorncs/si ssl socket factory
Diffstat (limited to 'node-admin')
4 files changed, 47 insertions, 115 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java index aea44e728ad..be82e615f7a 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java @@ -4,13 +4,13 @@ package com.yahoo.vespa.hosted.node.admin.configserver; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.yahoo.config.provision.HostName; -import com.yahoo.vespa.athenz.api.AthenzService; -import com.yahoo.vespa.athenz.identity.ServiceIdentityProvider; +import com.yahoo.vespa.athenz.identity.ServiceIdentitySslSocketFactory; import com.yahoo.vespa.athenz.identity.SiaIdentityProvider; import com.yahoo.vespa.athenz.tls.AthenzIdentityVerifier; import com.yahoo.vespa.hosted.node.admin.component.ConfigServerInfo; import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger; import org.apache.http.HttpHeaders; +import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpDelete; import org.apache.http.client.methods.HttpGet; @@ -19,11 +19,17 @@ import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpPut; import org.apache.http.client.methods.HttpRequestBase; import org.apache.http.client.methods.HttpUriRequest; +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.entity.StringEntity; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.SSLContext; import java.io.IOException; import java.io.UnsupportedEncodingException; import java.net.URI; @@ -33,6 +39,7 @@ import java.util.Collections; import java.util.List; import java.util.Optional; +import static com.yahoo.vespa.hosted.node.admin.task.util.file.IOExceptionUtil.uncheck; import static java.util.Collections.singleton; /** @@ -40,6 +47,7 @@ import static java.util.Collections.singleton; * content-type application/json * * @author dybdahl + * @author bjorncs */ public class ConfigServerApiImpl implements ConfigServerApi { private static final PrefixLogger NODE_ADMIN_LOGGER = PrefixLogger.getNodeAdminLogger(ConfigServerApiImpl.class); @@ -48,15 +56,7 @@ public class ConfigServerApiImpl implements ConfigServerApi { private final List<URI> configServers; - private Runnable runOnClose = () -> {}; - - /** - * The 'client' may be periodically re-created through calls to setSSLConnectionSocketFactory. - * - * The 'client' reference must be volatile because it is set and read in different threads, and visibility - * of changes is only guaranteed for volatile variables. - */ - private volatile SelfCloseableHttpClient client; + private final CloseableHttpClient client; public static ConfigServerApiImpl create(ConfigServerInfo info, SiaIdentityProvider provider) { return new ConfigServerApiImpl( @@ -77,29 +77,20 @@ public class ConfigServerApiImpl implements ConfigServerApi { private ConfigServerApiImpl(Collection<URI> configServers, HostnameVerifier verifier, SiaIdentityProvider identityProvider) { - this(configServers, createClient(identityProvider.getIdentitySslContext(), verifier)); - - // Register callback for updates to the SSLContext - ServiceIdentityProvider.Listener listener = (SSLContext sslContext, AthenzService identity) -> { - this.client = createClient(sslContext, verifier); - }; - identityProvider.addIdentityListener(listener); - this.runOnClose = () -> identityProvider.removeIdentityListener(listener); + this(configServers, createClient(new SSLConnectionSocketFactory(new ServiceIdentitySslSocketFactory(identityProvider), verifier))); } - private ConfigServerApiImpl(Collection<URI> configServers, SelfCloseableHttpClient client) { + private ConfigServerApiImpl(Collection<URI> configServers, CloseableHttpClient client) { this.configServers = randomizeConfigServerUris(configServers); this.client = client; } - public static ConfigServerApiImpl createForTestingWithSocketFactory( - List<URI> configServerHosts, - SSLConnectionSocketFactory socketFactory) { - return new ConfigServerApiImpl(configServerHosts, new SelfCloseableHttpClient(socketFactory)); + public static ConfigServerApiImpl createForTesting(List<URI> configServerHosts) { + return new ConfigServerApiImpl(configServerHosts, createClient(SSLConnectionSocketFactory.getSocketFactory())); } static ConfigServerApiImpl createForTestingWithClient(List<URI> configServerHosts, - SelfCloseableHttpClient client) { + CloseableHttpClient client) { return new ConfigServerApiImpl(configServerHosts, client); } @@ -183,18 +174,38 @@ public class ConfigServerApiImpl implements ConfigServerApi { @Override public void close() { - runOnClose.run(); - client.close(); + uncheck(client::close); } private void setContentTypeToApplicationJson(HttpRequestBase request) { request.setHeader(HttpHeaders.CONTENT_TYPE, "application/json"); } - private static SelfCloseableHttpClient createClient( - SSLContext sslContext, HostnameVerifier configServerVerifier) { - return new SelfCloseableHttpClient( - new SSLConnectionSocketFactory(sslContext, configServerVerifier)); + private static CloseableHttpClient createClient(SSLConnectionSocketFactory socketFactory) { + Registry<ConnectionSocketFactory> socketFactoryRegistry = RegistryBuilder.<ConnectionSocketFactory>create() + .register("http", PlainConnectionSocketFactory.getSocketFactory()) + .register("https", socketFactory) + .build(); + + PoolingHttpClientConnectionManager cm = new PoolingHttpClientConnectionManager(socketFactoryRegistry); + cm.setMaxTotal(200); // Increase max total connections to 200, which should be enough + + // Have experienced hang in socket read, which may have been because of + // system defaults, therefore set explicit timeouts. Set arbitrarily to + // 15s > 10s used by Orchestrator lock timeout. + int timeoutMs = 15_000; + RequestConfig requestBuilder = RequestConfig.custom() + .setConnectTimeout(timeoutMs) // establishment of connection + .setConnectionRequestTimeout(timeoutMs) // connection from connection manager + .setSocketTimeout(timeoutMs) // waiting for data + .build(); + + return HttpClientBuilder.create() + .setDefaultRequestConfig(requestBuilder) + .disableAutomaticRetries() + .setUserAgent("node-admin") + .setConnectionManager(cm) + .build(); } // Shuffle config server URIs to balance load diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SelfCloseableHttpClient.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SelfCloseableHttpClient.java deleted file mode 100644 index 38c111bc3ba..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SelfCloseableHttpClient.java +++ /dev/null @@ -1,78 +0,0 @@ -// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.node.admin.configserver; - -import com.yahoo.log.LogLevel; -import org.apache.http.client.config.RequestConfig; -import org.apache.http.client.methods.CloseableHttpResponse; -import org.apache.http.client.methods.HttpUriRequest; -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.CloseableHttpClient; -import org.apache.http.impl.client.HttpClientBuilder; -import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; - -import java.io.IOException; -import java.util.logging.Logger; - -/** - * @author freva - */ -class SelfCloseableHttpClient implements AutoCloseable { - - private static final Logger log = Logger.getLogger(SelfCloseableHttpClient.class.getName()); - - private final CloseableHttpClient httpClient; - - SelfCloseableHttpClient() { - this(SSLConnectionSocketFactory.getSocketFactory()); - } - - SelfCloseableHttpClient(SSLConnectionSocketFactory sslConnectionSocketFactory) { - Registry<ConnectionSocketFactory> socketFactoryRegistry = RegistryBuilder.<ConnectionSocketFactory>create() - .register("http", PlainConnectionSocketFactory.getSocketFactory()) - .register("https", sslConnectionSocketFactory) - .build(); - - PoolingHttpClientConnectionManager cm = new PoolingHttpClientConnectionManager(socketFactoryRegistry); - cm.setMaxTotal(200); // Increase max total connections to 200, which should be enough - - // Have experienced hang in socket read, which may have been because of - // system defaults, therefore set explicit timeouts. Set arbitrarily to - // 15s > 10s used by Orchestrator lock timeout. - int timeoutMs = 15_000; - RequestConfig requestBuilder = RequestConfig.custom() - .setConnectTimeout(timeoutMs) // establishment of connection - .setConnectionRequestTimeout(timeoutMs) // connection from connection manager - .setSocketTimeout(timeoutMs) // waiting for data - .build(); - - this.httpClient = HttpClientBuilder.create() - .setDefaultRequestConfig(requestBuilder) - .disableAutomaticRetries() - .setUserAgent("node-admin") - .setConnectionManager(cm).build(); - } - - public CloseableHttpResponse execute(HttpUriRequest request) throws IOException { - return httpClient.execute(request); - } - - @Override - @SuppressWarnings("deprecation") // finalize() is deprecated from Java 9 - public void finalize() throws Throwable { - close(); - super.finalize(); - } - - @Override - public void close() { - try { - httpClient.close(); - } catch (Exception e) { - log.log(LogLevel.WARNING, "Ignoring exception thrown when closing http client", e); - } - } -} diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java index a4230e4dc8d..da7b1e6a00c 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java @@ -7,6 +7,7 @@ import org.apache.http.HttpVersion; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; import org.apache.http.entity.BasicHttpEntity; +import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.message.BasicStatusLine; import org.junit.Before; import org.junit.Test; @@ -54,7 +55,7 @@ public class ConfigServerApiImplTest { @Before public void initExecutor() throws IOException { - SelfCloseableHttpClient httpMock = mock(SelfCloseableHttpClient.class); + CloseableHttpClient httpMock = mock(CloseableHttpClient.class); when(httpMock.execute(any())).thenAnswer(invocationOnMock -> { HttpGet get = (HttpGet) invocationOnMock.getArguments()[0]; mockLog.append(get.getMethod()).append(" ").append(get.getURI()).append(" "); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java index 045b1116740..85b62660687 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepositoryTest.java @@ -10,7 +10,6 @@ import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi; import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApiImpl; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.testutils.ContainerConfig; -import org.apache.http.conn.ssl.SSLConnectionSocketFactory; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -67,9 +66,8 @@ public class RealNodeRepositoryTest { try { final int port = findRandomOpenPort(); container = JDisc.fromServicesXml(ContainerConfig.servicesXmlV2(port), Networking.enable); - ConfigServerApi configServerApi = ConfigServerApiImpl.createForTestingWithSocketFactory( - Collections.singletonList(URI.create("http://127.0.0.1:" + port)), - SSLConnectionSocketFactory.getSocketFactory()); + ConfigServerApi configServerApi = ConfigServerApiImpl.createForTesting( + Collections.singletonList(URI.create("http://127.0.0.1:" + port))); waitForJdiscContainerToServe(configServerApi); return; } catch (RuntimeException e) { |