diff options
author | HÃ¥kon Hallingstad <hakon@oath.com> | 2018-05-09 22:21:40 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-05-09 22:21:40 +0200 |
commit | d43e41ae60f20b2b7103df2e3ec828b0cd66c248 (patch) | |
tree | 412ed0ef223bfeee232bcbf773489227f26ef7a4 /node-admin | |
parent | 677dc8d79d31895ad4732a72c1eea32543c2f41d (diff) |
Revert "Revert "Further simplifications of config server clients""
Diffstat (limited to 'node-admin')
7 files changed, 69 insertions, 248 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java index 9f6684ba84a..9b496526804 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java @@ -1,8 +1,6 @@ // 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 org.apache.http.conn.ssl.SSLConnectionSocketFactory; - import java.util.Optional; /** @@ -22,9 +20,6 @@ public interface ConfigServerApi extends AutoCloseable { <T> T delete(String path, Class<T> wantedReturnType); - /** Set or update the socket factory */ - void setSSLConnectionSocketFactory(SSLConnectionSocketFactory sslSocketFactory); - /** Close the underlying HTTP client and any threads this class might have started. */ @Override void close(); 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 c29028c493e..24f6890857a 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,6 +4,10 @@ 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.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; @@ -18,6 +22,7 @@ import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.conn.ssl.SSLConnectionSocketFactory; import org.apache.http.entity.StringEntity; +import javax.net.ssl.SSLContext; import java.io.IOException; import java.io.UnsupportedEncodingException; import java.net.URI; @@ -27,6 +32,8 @@ import java.util.Collections; import java.util.List; import java.util.Optional; +import static java.util.Collections.singleton; + /** * Retries request on config server a few times before giving up. Assumes that all requests should be sent with * content-type application/json @@ -38,16 +45,12 @@ public class ConfigServerApiImpl implements ConfigServerApi { private final ObjectMapper mapper = new ObjectMapper(); - private final List<URI> configServerHosts; + private final List<URI> configServers; - // If this instance is associated with asynchronous updating, this field is set - // to unregister from the updater at close() - private Optional<SslConnectionSocketFactoryUpdater> socketFactoryUpdater = Optional.empty(); + private Runnable runOnClose = () -> {}; /** - * The 'client' will be periodically re-created by clientRefresherScheduler if we provide keyStoreOptions - * or trustStoreOptions. This is needed because the key/trust stores are updated outside of node-admin, - * but we want to use the most recent store. + * 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. @@ -55,30 +58,6 @@ public class ConfigServerApiImpl implements ConfigServerApi { private volatile SelfCloseableHttpClient client; /** - * Creates an api for talking to the config servers. - * - * <p>Registers with a SslConnectionSocketFactoryUpdater to keep the socket factory - * up to date as e.g. any client certificate expires (and unregisters at {@link #close()}) - */ - public static ConfigServerApiImpl create(ConfigServerInfo configServerInfo, - SslConnectionSocketFactoryUpdater updater) { - return createFor(updater, configServerInfo.getConfigServerUris()); - } - - /** - * Creates an api for talking to a single config server. - * - * <p>Registers with a SslConnectionSocketFactoryUpdater to keep the socket factory - * up to date as e.g. any client certificate expires (and unregisters at {@link #close()}) - */ - public static ConfigServerApiImpl createFor(ConfigServerInfo configServerInfo, - SslConnectionSocketFactoryUpdater updater, - HostName configServer) { - URI uri = configServerInfo.getConfigServerUri(configServer.value()); - return createFor(updater, Collections.singletonList(uri)); - } - - /** * Creates an api for talking to the config servers with a fixed socket factory. * * <p>This may be used to avoid requiring background certificate signing requests (CSR) @@ -87,7 +66,19 @@ public class ConfigServerApiImpl implements ConfigServerApi { public static ConfigServerApiImpl createWithSocketFactory( List<URI> configServerHosts, SSLConnectionSocketFactory socketFactory) { - return new ConfigServerApiImpl(configServerHosts, socketFactory); + return new ConfigServerApiImpl(configServerHosts, new SelfCloseableHttpClient(socketFactory)); + } + + public static ConfigServerApiImpl create(ConfigServerInfo configServerInfo, + SiaIdentityProvider identityProvider) { + return new ConfigServerApiImpl(configServerInfo.getConfigServerUris(), identityProvider); + } + + public static ConfigServerApiImpl createFor(ConfigServerInfo configServerInfo, + SiaIdentityProvider identityProvider, + HostName configServer) { + URI uri = configServerInfo.getConfigServerUri(configServer.value()); + return new ConfigServerApiImpl(Collections.singletonList(uri), identityProvider); } static ConfigServerApiImpl createForTestingWithClient(List<URI> configServerHosts, @@ -95,25 +86,18 @@ public class ConfigServerApiImpl implements ConfigServerApi { return new ConfigServerApiImpl(configServerHosts, client); } - private static ConfigServerApiImpl createFor(SslConnectionSocketFactoryUpdater updater, - List<URI> configServers) { - ConfigServerApiImpl api = new ConfigServerApiImpl( - configServers, - // Passing null here (only) works because startSocketFactoryUpdating will - // set the client. This avoids an unnecessary allocation of a client. - (SelfCloseableHttpClient) null); - api.startSocketFactoryUpdating(updater); - assert api.client != null; - return api; - } + private ConfigServerApiImpl(Collection<URI> configServers, SiaIdentityProvider identityProvider) { + this(configServers, createClient(identityProvider)); - private ConfigServerApiImpl(Collection<URI> configServerUris, - SSLConnectionSocketFactory sslConnectionSocketFactory) { - this(configServerUris, new SelfCloseableHttpClient(sslConnectionSocketFactory)); + // The same object MUST be passed to both addIdentityListener and removeIdentityListener, + // as two method references aren't equal. + ServiceIdentityProvider.Listener listener = this::setClient; + identityProvider.addIdentityListener(listener); + this.runOnClose = () -> identityProvider.removeIdentityListener(listener); } - private ConfigServerApiImpl(Collection<URI> configServerHosts, SelfCloseableHttpClient client) { - this.configServerHosts = randomizeConfigServerUris(configServerHosts); + private ConfigServerApiImpl(Collection<URI> configServers, SelfCloseableHttpClient client) { + this.configServers = randomizeConfigServerUris(configServers); this.client = client; } @@ -123,7 +107,7 @@ public class ConfigServerApiImpl implements ConfigServerApi { private <T> T tryAllConfigServers(CreateRequest requestFactory, Class<T> wantedReturnType) { Exception lastException = null; - for (URI configServer : configServerHosts) { + for (URI configServer : configServers) { final CloseableHttpResponse response; try { response = client.execute(requestFactory.createRequest(configServer)); @@ -163,7 +147,7 @@ public class ConfigServerApiImpl implements ConfigServerApi { } throw new RuntimeException("All requests against the config servers (" - + configServerHosts + ") failed, last as follows:", lastException); + + configServers + ") failed, last as follows:", lastException); } @Override @@ -210,13 +194,28 @@ public class ConfigServerApiImpl implements ConfigServerApi { }, wantedReturnType); } + @Override + public void close() { + runOnClose.run(); + client.close(); + } + private void setContentTypeToApplicationJson(HttpRequestBase request) { request.setHeader(HttpHeaders.CONTENT_TYPE, "application/json"); } - @Override - public void setSSLConnectionSocketFactory(SSLConnectionSocketFactory sslSocketFactory) { - this.client = new SelfCloseableHttpClient(sslSocketFactory); + private void setClient(SSLContext sslContext, AthenzService identity) { + this.client = createClient(sslContext, identity); + } + + private static SelfCloseableHttpClient createClient(SSLContext sslContext, AthenzService identity) { + AthenzIdentityVerifier identityVerifier = new AthenzIdentityVerifier(singleton(identity)); + SSLConnectionSocketFactory socketFactory = new SSLConnectionSocketFactory(sslContext, identityVerifier); + return new SelfCloseableHttpClient(socketFactory); + } + + private static SelfCloseableHttpClient createClient(SiaIdentityProvider identityProvider) { + return createClient(identityProvider.getIdentitySslContext(), identityProvider.identity()); } // Shuffle config server URIs to balance load @@ -225,20 +224,4 @@ public class ConfigServerApiImpl implements ConfigServerApi { Collections.shuffle(shuffledConfigServerHosts); return shuffledConfigServerHosts; } - - private void startSocketFactoryUpdating(SslConnectionSocketFactoryUpdater updater) { - updater.registerConfigServerApi(this); - this.socketFactoryUpdater = Optional.of(updater); - } - - private void stopSocketFactoryUpdating() { - this.socketFactoryUpdater.ifPresent(updater -> updater.unregisterConfigServerApi(this)); - this.socketFactoryUpdater = Optional.empty(); - } - - @Override - public void close() { - stopSocketFactoryUpdating(); - client.close(); - } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerClients.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerClients.java index 7c15f94852b..4a2496f4d3e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerClients.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerClients.java @@ -1,7 +1,6 @@ // 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.config.provision.HostName; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator; import com.yahoo.vespa.hosted.node.admin.configserver.state.State; @@ -18,8 +17,8 @@ public interface ConfigServerClients { /** Get handle to /orchestrator/v1/ REST API */ Orchestrator orchestrator(); - /** Get handle to the /state/v1 REST API of the specified config server */ - default State state(HostName hostname) { throw new java.lang.UnsupportedOperationException(); } + /** Get handle to the /state/v1 REST API */ + default State state() { throw new UnsupportedOperationException(); } void stop(); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/RealConfigServerClients.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/RealConfigServerClients.java index 07dbc8aef9c..6c982bfa71c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/RealConfigServerClients.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/RealConfigServerClients.java @@ -1,9 +1,6 @@ // 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.config.provision.HostName; -import com.yahoo.vespa.athenz.identity.SiaIdentityProvider; -import com.yahoo.vespa.hosted.node.admin.component.ConfigServerInfo; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeRepository; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.RealNodeRepository; import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.Orchestrator; @@ -11,38 +8,26 @@ import com.yahoo.vespa.hosted.node.admin.configserver.orchestrator.OrchestratorI import com.yahoo.vespa.hosted.node.admin.configserver.state.State; import com.yahoo.vespa.hosted.node.admin.configserver.state.StateImpl; -import java.util.concurrent.ConcurrentHashMap; - /** + * {@link ConfigServerClients} using the default implementation for the various clients, + * and backed by a {@link ConfigServerApi}. + * * @author freva */ public class RealConfigServerClients implements ConfigServerClients { - - private final SslConnectionSocketFactoryUpdater updater; - - // ConfigServerApi that talks to all config servers private final ConfigServerApi configServerApi; - private final NodeRepository nodeRepository; private final Orchestrator orchestrator; - private final ConcurrentHashMap<HostName, State> states = new ConcurrentHashMap<>(); - private final ConfigServerInfo configServerInfo; + private final State state; /** - * Create config server clients against a real (remote) config server. - * - * If a client certificate is required, one will be requested from the config server - * and kept up to date. On failure, this constructor will throw an exception and - * the caller may retry later. + * @param configServerApi the backend API to use - will be closed at {@link #stop()}. */ - public RealConfigServerClients(SiaIdentityProvider identityProvider, ConfigServerInfo info) { - this.configServerInfo = info; - updater = SslConnectionSocketFactoryUpdater.createAndRefreshKeyStoreIfNeeded(identityProvider, info.getAthenzIdentity().get()); - - configServerApi = ConfigServerApiImpl.create(info, updater); - + public RealConfigServerClients(ConfigServerApi configServerApi) { + this.configServerApi = configServerApi; nodeRepository = new RealNodeRepository(configServerApi); orchestrator = new OrchestratorImpl(configServerApi); + state = new StateImpl(configServerApi); } @Override @@ -56,21 +41,12 @@ public class RealConfigServerClients implements ConfigServerClients { } @Override - public State state(HostName hostname) { - return states.computeIfAbsent(hostname, this::createState); + public State state() { + return state; } @Override public void stop() { - updater.unregisterConfigServerApi(configServerApi); configServerApi.close(); - updater.close(); - } - - private State createState(HostName hostname) { - ConfigServerApi configServerApi = ConfigServerApiImpl.createFor( - configServerInfo, updater, hostname); - - return new StateImpl(configServerApi); } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SslConnectionSocketFactoryUpdater.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SslConnectionSocketFactoryUpdater.java deleted file mode 100644 index 1888749b998..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SslConnectionSocketFactoryUpdater.java +++ /dev/null @@ -1,104 +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.vespa.athenz.api.AthenzIdentity; -import com.yahoo.vespa.athenz.api.AthenzService; -import com.yahoo.vespa.athenz.identity.SiaIdentityProvider; -import com.yahoo.vespa.athenz.tls.AthenzIdentityVerifier; -import com.yahoo.vespa.athenz.tls.SslContextBuilder; -import org.apache.http.conn.ssl.SSLConnectionSocketFactory; - -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.SSLContext; -import java.util.HashSet; -import java.util.Set; - -import static java.util.Collections.singleton; - -/** - * Responsible for updating {@link SSLConnectionSocketFactory} on {@link ConfigServerApiImpl} asynchronously - * using SIA based certificates (through {@link SiaIdentityProvider}). - * - * @author bjorncs - * @author hakon - */ -public class SslConnectionSocketFactoryUpdater implements AutoCloseable { - - private final Object monitor = new Object(); - private final HostnameVerifier configServerHostnameVerifier; - private final SiaIdentityProvider sia; - - private final Set<ConfigServerApi> configServerApis = new HashSet<>(); - private SSLConnectionSocketFactory socketFactory; - - /** - * Creates an updater with valid initial {@link SSLConnectionSocketFactory} - * - * @throws RuntimeException if e.g. key store options have been specified, but was unable - * create a create a key store with a valid certificate - */ - public static SslConnectionSocketFactoryUpdater createAndRefreshKeyStoreIfNeeded(SiaIdentityProvider identityProvider, - AthenzIdentity configserverIdentity) { - return new SslConnectionSocketFactoryUpdater(identityProvider, createHostnameVerifier(configserverIdentity)); - } - - SslConnectionSocketFactoryUpdater(SiaIdentityProvider siaIdentityProvider, - HostnameVerifier configServerHostnameVerifier) { - this.configServerHostnameVerifier = configServerHostnameVerifier; - this.sia = siaIdentityProvider; - if (siaIdentityProvider != null) { - siaIdentityProvider.addIdentityListener(this::updateSocketFactory); - socketFactory = createSocketFactory(siaIdentityProvider.getIdentitySslContext()); - } else { - socketFactory = createDefaultSslConnectionSocketFactory(); - } - } - - private void updateSocketFactory(SSLContext sslContext, AthenzService identity) { - synchronized (monitor) { - socketFactory = createSocketFactory(sslContext); - configServerApis.forEach(api -> api.setSSLConnectionSocketFactory(socketFactory)); - } - } - - public SSLConnectionSocketFactory getCurrentSocketFactory() { - synchronized (monitor) { - return socketFactory; - } - } - - /** Register a {@link ConfigServerApi} whose {@link SSLConnectionSocketFactory} will be kept up to date */ - public void registerConfigServerApi(ConfigServerApi configServerApi) { - synchronized (monitor) { - configServerApi.setSSLConnectionSocketFactory(socketFactory); - configServerApis.add(configServerApi); - } - } - - public void unregisterConfigServerApi(ConfigServerApi configServerApi) { - synchronized (monitor) { - configServerApis.remove(configServerApi); - } - } - - @Override - public void close() { - if (sia != null) { - sia.deconstruct(); - } - } - - private SSLConnectionSocketFactory createSocketFactory(SSLContext sslContext) { - return new SSLConnectionSocketFactory(sslContext, configServerHostnameVerifier); - } - - private SSLConnectionSocketFactory createDefaultSslConnectionSocketFactory() { - SSLContext sslContext = new SslContextBuilder().build(); - return createSocketFactory(sslContext); - } - - private static HostnameVerifier createHostnameVerifier(AthenzIdentity identity) { - return new AthenzIdentityVerifier(singleton(identity)); - } - -} diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/NodeAdminProvider.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/NodeAdminProvider.java index ac1b1cbb600..4bac2cdfbf2 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/NodeAdminProvider.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/provider/NodeAdminProvider.java @@ -10,6 +10,8 @@ import com.yahoo.vespa.hosted.dockerapi.metrics.MetricReceiverWrapper; import com.yahoo.vespa.hosted.node.admin.component.ConfigServerInfo; import com.yahoo.vespa.hosted.node.admin.component.DockerAdminComponent; import com.yahoo.vespa.hosted.node.admin.config.ConfigServerConfig; +import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApi; +import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerApiImpl; import com.yahoo.vespa.hosted.node.admin.configserver.ConfigServerClients; import com.yahoo.vespa.hosted.node.admin.configserver.RealConfigServerClients; @@ -22,8 +24,9 @@ public class NodeAdminProvider implements Provider<NodeAdminStateUpdater> { Docker docker, MetricReceiverWrapper metricReceiver, ClassLocking classLocking) { - ConfigServerClients clients = - new RealConfigServerClients(identityProvider, new ConfigServerInfo(configServerConfig)); + ConfigServerInfo configServerInfo = new ConfigServerInfo(configServerConfig); + ConfigServerApi configServerApi = ConfigServerApiImpl.create(configServerInfo, identityProvider); + ConfigServerClients clients = new RealConfigServerClients(configServerApi); dockerAdmin = new DockerAdminComponent(configServerConfig, identityProvider, diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/SslConnectionSocketFactoryUpdaterTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/SslConnectionSocketFactoryUpdaterTest.java deleted file mode 100644 index 6c6f8cf40fe..00000000000 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/SslConnectionSocketFactoryUpdaterTest.java +++ /dev/null @@ -1,31 +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.vespa.athenz.identity.SiaIdentityProvider; -import com.yahoo.vespa.athenz.tls.SslContextBuilder; -import org.junit.Test; - -import static org.junit.Assert.assertNotNull; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; - -/** - * @author bjorncs - */ -public class SslConnectionSocketFactoryUpdaterTest { - - @Test - public void creates_default_ssl_connection_factory_when_no_sia_provided() { - SslConnectionSocketFactoryUpdater updater = - new SslConnectionSocketFactoryUpdater(null, (hostname, session) -> true); - assertNotNull(updater.getCurrentSocketFactory()); - } - - @Test - public void creates_ssl_connection_factory_when_sia_provided() { - SiaIdentityProvider sia = mock(SiaIdentityProvider.class); - when(sia.getIdentitySslContext()).thenReturn(new SslContextBuilder().build()); - SslConnectionSocketFactoryUpdater updater = new SslConnectionSocketFactoryUpdater(sia, (hostname, session) -> true); - assertNotNull(updater.getCurrentSocketFactory()); - } -}
\ No newline at end of file |