ConfigServerHttpRequestExecutor -> ConfigServerApiImpl, also removed scheduler that refreshes SSLConnectionSocketFactory

5 files changed, 50 insertions, 101 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
new file mode 100644
index 00000000000..2de84bf10e6
--- /dev/null
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApi.java
@@ -0,0 +1,27 @@
+// 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 java.util.Optional;
+
+/**
+ * Interface to execute basic HTTP request against config server(s)
+ *
+ * @author freva
+ */
+public interface ConfigServerApi {
+
+    <T> T get(String path, Class<T> wantedReturnType);
+
+    <T> T post(String path, Object bodyJsonPojo, Class<T> wantedReturnType);
+
+    <T> T put(String path, Optional<Object> bodyJsonPojo, Class<T> wantedReturnType);
+
+    <T> T patch(String path, Object bodyJsonPojo, Class<T> wantedReturnType);
+
+    <T> T delete(String path, Class<T> wantedReturnType);
+
+    /**
+     * Close the underlying HTTP client and any threads this class might have started.
+     */
+    void stop();
+}
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java
index 13bfc949533..c10340525da 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutor.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImpl.java
@@ -1,12 +1,9 @@
 // Copyright 2017 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.util;
+package com.yahoo.vespa.hosted.node.admin.configserver;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.yahoo.concurrent.ThreadFactoryFactory;
-import com.yahoo.vespa.athenz.api.AthenzIdentity;
-import com.yahoo.vespa.athenz.tls.AthenzIdentityVerifier;
-import com.yahoo.vespa.athenz.tls.AthenzSslContextBuilder;
+import com.yahoo.vespa.hosted.node.admin.util.PrefixLogger;
 import org.apache.http.HttpHeaders;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpDelete;
@@ -18,28 +15,15 @@ import org.apache.http.client.methods.HttpRequestBase;
 import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
 import org.apache.http.entity.StringEntity;
-import org.bouncycastle.jce.provider.BouncyCastleProvider;
 
-import javax.net.ssl.HostnameVerifier;
-import javax.net.ssl.SSLContext;
-import java.io.FileInputStream;
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.net.URI;
-import java.nio.file.Path;
-import java.security.GeneralSecurityException;
-import java.security.KeyStore;
-import java.security.Security;
-import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
-import java.util.function.Supplier;
 
 /**
  * Retries request on config server a few times before giving up. Assumes that all requests should be sent with
@@ -47,13 +31,11 @@ import java.util.function.Supplier;
  *
  * @author dybdahl
  */
-public class ConfigServerHttpRequestExecutor implements AutoCloseable {
-    private static final PrefixLogger NODE_ADMIN_LOGGER = PrefixLogger.getNodeAdminLogger(ConfigServerHttpRequestExecutor.class);
-    private static final Duration CLIENT_REFRESH_INTERVAL = Duration.ofHours(1);
+public class ConfigServerApiImpl implements ConfigServerApi {
+    private static final PrefixLogger NODE_ADMIN_LOGGER = PrefixLogger.getNodeAdminLogger(ConfigServerApiImpl.class);
 
     private final ObjectMapper mapper = new ObjectMapper();
-    private final ScheduledExecutorService clientRefresherScheduler =
-            Executors.newScheduledThreadPool(1, ThreadFactoryFactory.getDaemonThreadFactory("http-client-refresher"));
+
     private final List<URI> configServerHosts;
 
     /**
@@ -66,30 +48,20 @@ public class ConfigServerHttpRequestExecutor implements AutoCloseable {
      */
     private volatile SelfCloseableHttpClient client;
 
-    public static ConfigServerHttpRequestExecutor create(
-            Collection<URI> configServerUris,
-            Optional<KeyStoreOptions> keyStoreOptions,
-            Optional<KeyStoreOptions> trustStoreOptions,
-            Optional<AthenzIdentity> athenzIdentity) {
-        Security.addProvider(new BouncyCastleProvider());
-
-        Supplier<SelfCloseableHttpClient> clientSupplier = () -> createHttpClient(keyStoreOptions, trustStoreOptions, athenzIdentity);
-        ConfigServerHttpRequestExecutor requestExecutor = new ConfigServerHttpRequestExecutor(
-                randomizeConfigServerUris(configServerUris), clientSupplier.get());
+    public ConfigServerApiImpl(Collection<URI> configServerUris) {
+        this(configServerUris, SSLConnectionSocketFactory.getSocketFactory());
+    }
 
-        if (keyStoreOptions.isPresent() || trustStoreOptions.isPresent()) {
-            requestExecutor.clientRefresherScheduler.scheduleAtFixedRate(() -> requestExecutor.client = clientSupplier.get(),
-                    CLIENT_REFRESH_INTERVAL.toMillis(), CLIENT_REFRESH_INTERVAL.toMillis(), TimeUnit.MILLISECONDS);
-        }
-        return requestExecutor;
+    ConfigServerApiImpl(Collection<URI> configServerUris, SSLConnectionSocketFactory sslConnectionSocketFactory) {
+        this(randomizeConfigServerUris(configServerUris), new SelfCloseableHttpClient(sslConnectionSocketFactory));
     }
 
-    ConfigServerHttpRequestExecutor(List<URI> configServerHosts, SelfCloseableHttpClient client) {
+    ConfigServerApiImpl(List<URI> configServerHosts, SelfCloseableHttpClient client) {
         this.configServerHosts = configServerHosts;
         this.client = client;
     }
 
-    public interface CreateRequest {
+    interface CreateRequest {
         HttpUriRequest createRequest(URI configServerUri) throws JsonProcessingException, UnsupportedEncodingException;
     }
 
@@ -181,6 +153,10 @@ public class ConfigServerHttpRequestExecutor implements AutoCloseable {
         request.setHeader(HttpHeaders.CONTENT_TYPE, "application/json");
     }
 
+    public void setSSLConnectionSocketFactory(SSLConnectionSocketFactory sslSocketFactory) {
+        this.client = new SelfCloseableHttpClient(sslSocketFactory);
+    }
+
     // Shuffle config server URIs to balance load
     private static List<URI> randomizeConfigServerUris(Collection<URI> configServerUris) {
         List<URI> shuffledConfigServerHosts = new ArrayList<>(configServerUris);
@@ -188,62 +164,8 @@ public class ConfigServerHttpRequestExecutor implements AutoCloseable {
         return shuffledConfigServerHosts;
     }
 
-    private static SelfCloseableHttpClient createHttpClient(Optional<KeyStoreOptions> keyStoreOptions,
-                                                            Optional<KeyStoreOptions> trustStoreOptions,
-                                                            Optional<AthenzIdentity> athenzIdentity) {
-        NODE_ADMIN_LOGGER.info("Creating new HTTP client");
-        try {
-            SSLContext sslContext = makeSslContext(keyStoreOptions, trustStoreOptions);
-            HostnameVerifier hostnameVerifier = makeHostnameVerifier(athenzIdentity);
-            SSLConnectionSocketFactory sslSocketFactory = new SSLConnectionSocketFactory(sslContext, hostnameVerifier);
-            return new SelfCloseableHttpClient(sslSocketFactory);
-        } catch (Exception e) {
-            NODE_ADMIN_LOGGER.error("Failed to create HTTP client with custom SSL Context, proceeding with default", e);
-            return new SelfCloseableHttpClient();
-        }
-    }
-
-    private static SSLContext makeSslContext(Optional<KeyStoreOptions> keyStoreOptions, Optional<KeyStoreOptions> trustStoreOptions) {
-        AthenzSslContextBuilder sslContextBuilder = new AthenzSslContextBuilder();
-        trustStoreOptions.ifPresent(options -> sslContextBuilder.withTrustStore(options.path.toFile(), options.type));
-        keyStoreOptions.ifPresent(options -> {
-            try {
-                KeyStore keyStore = loadKeyStoreFromFileWithProvider(options.path, options.password, options.type, "BC");
-                sslContextBuilder.withKeyStore(keyStore, options.password);
-            } catch (Exception e) {
-                throw new RuntimeException("Failed to read key store", e);
-            }
-        });
-
-        return sslContextBuilder.build();
-    }
-
-    private static HostnameVerifier makeHostnameVerifier(Optional<AthenzIdentity> athenzIdentity) {
-        return athenzIdentity
-                .map(identity -> (HostnameVerifier) new AthenzIdentityVerifier(Collections.singleton(identity)))
-                .orElse(SSLConnectionSocketFactory.getDefaultHostnameVerifier());
-    }
-
     @Override
-    public void close() {
-        clientRefresherScheduler.shutdown();
-        do {
-            try {
-                clientRefresherScheduler.awaitTermination(Long.MAX_VALUE, TimeUnit.NANOSECONDS);
-            } catch (InterruptedException e1) {
-                NODE_ADMIN_LOGGER.info("Interrupted while waiting for clientRefresherScheduler to shutdown");
-            }
-        } while (!clientRefresherScheduler.isTerminated());
-
+    public void stop() {
         client.close();
     }
-
-    private static KeyStore loadKeyStoreFromFileWithProvider(Path path, char[] password, String keyStoreType, String provider)
-            throws IOException, GeneralSecurityException {
-        KeyStore keyStore = KeyStore.getInstance(keyStoreType, provider);
-        try (FileInputStream in = new FileInputStream(path.toFile())) {
-            keyStore.load(in, password);
-        }
-        return keyStore;
-    }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/HttpException.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java
index 55d3ecc4e60..0a2ae1bd426 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/HttpException.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/HttpException.java
@@ -1,5 +1,5 @@
 // Copyright 2017 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.util;
+package com.yahoo.vespa.hosted.node.admin.configserver;
 
 import javax.ws.rs.core.Response;
 import java.util.Optional;
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/SelfCloseableHttpClient.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SelfCloseableHttpClient.java
index 8e516729aff..cead7816387 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/util/SelfCloseableHttpClient.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/SelfCloseableHttpClient.java
@@ -1,5 +1,5 @@
 // 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.util;
+package com.yahoo.vespa.hosted.node.admin.configserver;
 
 import com.yahoo.log.LogLevel;
 import org.apache.http.client.config.RequestConfig;
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutorTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java
index 175d3a9a051..f39a64d2dee 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/util/ConfigServerHttpRequestExecutorTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/ConfigServerApiImplTest.java
@@ -1,5 +1,5 @@
 // Copyright 2017 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.util;
+package com.yahoo.vespa.hosted.node.admin.configserver;
 
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import com.fasterxml.jackson.annotation.JsonProperty;
@@ -34,7 +34,7 @@ import static org.mockito.Mockito.when;
  *
  * @author dybis
  */
-public class ConfigServerHttpRequestExecutorTest {
+public class ConfigServerApiImplTest {
 
     @JsonIgnoreProperties(ignoreUnknown = true)
     public static class TestPojo {
@@ -49,7 +49,7 @@ public class ConfigServerHttpRequestExecutorTest {
     private final List<URI> configServers = Arrays.asList(URI.create(uri1), URI.create(uri2));
     private final StringBuilder mockLog = new StringBuilder();
 
-    private ConfigServerHttpRequestExecutor executor;
+    private ConfigServerApiImpl executor;
     private int mockReturnCode = 200;
 
     @Before
@@ -72,7 +72,7 @@ public class ConfigServerHttpRequestExecutorTest {
 
             return response;
         });
-        executor = new ConfigServerHttpRequestExecutor(configServers, httpMock);
+        executor = new ConfigServerApiImpl(configServers, httpMock);
     }
 
     @Test