Merge pull request #10296 from vespa-engine/bjorncs/nginx-health-check-proxy

Add health check proxy support to jdisc connectors

7 files changed, 231 insertions, 11 deletions

diff --git a/jdisc_http_service/abi-spec.json b/jdisc_http_service/abi-spec.json
index 6e04ba741e6..1615cf7e686 100644
--- a/jdisc_http_service/abi-spec.json
+++ b/jdisc_http_service/abi-spec.json
@@ -40,6 +40,7 @@
       "public com.yahoo.jdisc.http.ConnectorConfig$Builder throttling(com.yahoo.jdisc.http.ConnectorConfig$Throttling$Builder)",
       "public com.yahoo.jdisc.http.ConnectorConfig$Builder ssl(com.yahoo.jdisc.http.ConnectorConfig$Ssl$Builder)",
       "public com.yahoo.jdisc.http.ConnectorConfig$Builder tlsClientAuthEnforcer(com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder)",
+      "public com.yahoo.jdisc.http.ConnectorConfig$Builder healthCheckProxy(com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy$Builder)",
       "public final boolean dispatchGetConfig(com.yahoo.config.ConfigInstance$Producer)",
       "public final java.lang.String getDefMd5()",
       "public final java.lang.String getDefName()",
@@ -49,9 +50,41 @@
     "fields": [
       "public com.yahoo.jdisc.http.ConnectorConfig$Throttling$Builder throttling",
       "public com.yahoo.jdisc.http.ConnectorConfig$Ssl$Builder ssl",
-      "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder tlsClientAuthEnforcer"
+      "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer$Builder tlsClientAuthEnforcer",
+      "public com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy$Builder healthCheckProxy"
     ]
   },
+  "com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy$Builder": {
+    "superClass": "java.lang.Object",
+    "interfaces": [
+      "com.yahoo.config.ConfigBuilder"
+    ],
+    "attributes": [
+      "public"
+    ],
+    "methods": [
+      "public void <init>()",
+      "public void <init>(com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy)",
+      "public com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy$Builder enable(boolean)",
+      "public com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy$Builder port(int)",
+      "public com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy build()"
+    ],
+    "fields": []
+  },
+  "com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy": {
+    "superClass": "com.yahoo.config.InnerNode",
+    "interfaces": [],
+    "attributes": [
+      "public",
+      "final"
+    ],
+    "methods": [
+      "public void <init>(com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy$Builder)",
+      "public boolean enable()",
+      "public int port()"
+    ],
+    "fields": []
+  },
   "com.yahoo.jdisc.http.ConnectorConfig$Producer": {
     "superClass": "java.lang.Object",
     "interfaces": [
@@ -244,7 +277,8 @@
       "public boolean tcpNoDelay()",
       "public com.yahoo.jdisc.http.ConnectorConfig$Throttling throttling()",
       "public com.yahoo.jdisc.http.ConnectorConfig$Ssl ssl()",
-      "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer tlsClientAuthEnforcer()"
+      "public com.yahoo.jdisc.http.ConnectorConfig$TlsClientAuthEnforcer tlsClientAuthEnforcer()",
+      "public com.yahoo.jdisc.http.ConnectorConfig$HealthCheckProxy healthCheckProxy()"
     ],
     "fields": [
       "public static final java.lang.String CONFIG_DEF_MD5",
diff --git a/jdisc_http_service/pom.xml b/jdisc_http_service/pom.xml
index 978275ad9cc..979b9418f4a 100644
--- a/jdisc_http_service/pom.xml
+++ b/jdisc_http_service/pom.xml
@@ -16,6 +16,13 @@
   <packaging>container-plugin</packaging>
   <name>${project.artifactId}</name>
   <dependencies>
+    <!-- COMPILE SCOPE -->
+    <dependency>
+      <groupId>org.apache.httpcomponents</groupId>
+      <artifactId>httpclient</artifactId>
+      <scope>compile</scope>
+    </dependency>
+
     <!-- PROVIDED SCOPE -->
     <dependency>
       <groupId>com.google.inject</groupId>
@@ -80,11 +87,6 @@
     </dependency>
     <dependency>
       <groupId>org.apache.httpcomponents</groupId>
-      <artifactId>httpclient</artifactId>
-      <scope>test</scope>
-    </dependency>
-    <dependency>
-      <groupId>org.apache.httpcomponents</groupId>
       <artifactId>httpmime</artifactId>
       <scope>test</scope>
     </dependency>
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java
index c9586530734..01d6fa02d6e 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java
@@ -53,7 +53,9 @@ public class ConnectorFactory {
 
     private List<ConnectionFactory> createConnectionFactories() {
         HttpConnectionFactory httpConnectionFactory = newHttpConnectionFactory();
-        if (connectorConfig.ssl().enabled()) {
+        if (connectorConfig.healthCheckProxy().enable()) {
+            return List.of(httpConnectionFactory);
+        } else if (connectorConfig.ssl().enabled()) {
             return List.of(newSslConnectionFactory(), httpConnectionFactory);
         } else if (TransportSecurityUtils.isTransportSecurityEnabled()) {
             SslConnectionFactory sslConnectionsFactory = newSslConnectionFactory();
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HealthCheckProxyHandler.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HealthCheckProxyHandler.java
new file mode 100644
index 00000000000..4dfdbd55fab
--- /dev/null
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HealthCheckProxyHandler.java
@@ -0,0 +1,155 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.jdisc.http.server.jetty;
+
+import com.yahoo.jdisc.Response;
+import com.yahoo.jdisc.http.ConnectorConfig;
+import org.apache.http.Header;
+import org.apache.http.HttpEntity;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.conn.ssl.NoopHostnameVerifier;
+import org.apache.http.impl.NoConnectionReuseStrategy;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.SslConnectionFactory;
+import org.eclipse.jetty.server.handler.HandlerWrapper;
+import org.eclipse.jetty.util.ssl.SslContextFactory;
+
+import javax.servlet.ServletException;
+import javax.servlet.ServletOutputStream;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+import static com.yahoo.jdisc.Response.Status.NOT_FOUND;
+
+/**
+ * A handler that proxies status.html health checks
+ *
+ * @author bjorncs
+ */
+class HealthCheckProxyHandler extends HandlerWrapper {
+
+    private static final Logger log = Logger.getLogger(HealthCheckProxyHandler.class.getName());
+
+    private static final String HEALTH_CHECK_PATH = "/status.html";
+
+    private final Map<Integer, ProxyTarget> portToProxyTargetMapping;
+
+    HealthCheckProxyHandler(List<JDiscServerConnector> connectors) {
+        this.portToProxyTargetMapping = createPortToProxyTargetMapping(connectors);
+    }
+
+    private static Map<Integer, ProxyTarget> createPortToProxyTargetMapping(List<JDiscServerConnector> connectors) {
+        var mapping = new HashMap<Integer, ProxyTarget>();
+        for (JDiscServerConnector connector : connectors) {
+            ConnectorConfig.HealthCheckProxy proxyConfig = connector.connectorConfig().healthCheckProxy();
+            if (proxyConfig.enable()) {
+                mapping.put(connector.listenPort(), createProxyTarget(proxyConfig.port(), connectors));
+            }
+        }
+        return mapping;
+    }
+
+    private static ProxyTarget createProxyTarget(int targetPort, List<JDiscServerConnector> connectors) {
+        JDiscServerConnector targetConnector = connectors.stream()
+                .filter(connector -> connector.listenPort() == targetPort)
+                .findAny()
+                .orElseThrow(() -> new IllegalArgumentException("Could not find any connector with listen port " + targetPort));
+        SslContextFactory sslContextFactory =
+                Optional.ofNullable(targetConnector.getConnectionFactory(SslConnectionFactory.class))
+                        .map(SslConnectionFactory::getSslContextFactory)
+                        .orElseThrow(() -> new IllegalArgumentException("Health check proxy can only target https port"));
+        return new ProxyTarget(targetPort, sslContextFactory);
+    }
+
+    @Override
+    public void handle(String target, Request request, HttpServletRequest servletRequest, HttpServletResponse servletResponse) throws IOException, ServletException {
+        ProxyTarget proxyTarget = portToProxyTargetMapping.get(request.getLocalPort());
+        if (proxyTarget != null) {
+            if (servletRequest.getRequestURI().equals(HEALTH_CHECK_PATH)) {
+                try (CloseableHttpResponse proxyResponse = proxyTarget.requestStatusHtml()) {
+                    servletResponse.setStatus(proxyResponse.getStatusLine().getStatusCode());
+                    servletResponse.setHeader("Vespa-Health-Check-Proxy-Target", Integer.toString(proxyTarget.port));
+                    HttpEntity entity = proxyResponse.getEntity();
+                    if (entity != null) {
+                        Header contentType = entity.getContentType();
+                        if (contentType != null) {
+                            servletResponse.addHeader("Content-Type", contentType.getValue());
+                        }
+                        try (ServletOutputStream output = servletResponse.getOutputStream()) {
+                            entity.getContent().transferTo(output);
+                        }
+                    }
+                } catch (Exception e) {
+                    String message = "Unable to proxy health check request: " + e.getMessage();
+                    log.log(Level.WARNING, e, () -> message);
+                    servletResponse.sendError(Response.Status.INTERNAL_SERVER_ERROR, message);
+                }
+            } else {
+                servletResponse.sendError(NOT_FOUND);
+            }
+        } else {
+            _handler.handle(target, request, servletRequest, servletResponse);
+        }
+    }
+
+    @Override
+    protected void doStop() throws Exception {
+        for (ProxyTarget target : portToProxyTargetMapping.values()) {
+            target.close();
+        }
+        super.doStop();
+    }
+
+    private static class ProxyTarget implements AutoCloseable {
+        final int port;
+        final SslContextFactory sslContextFactory;
+        volatile CloseableHttpClient client;
+
+        ProxyTarget(int port, SslContextFactory sslContextFactory) {
+            this.port = port;
+            this.sslContextFactory = sslContextFactory;
+        }
+
+        CloseableHttpResponse requestStatusHtml() throws IOException {
+            HttpGet request = new HttpGet("https://localhost:" + port + HEALTH_CHECK_PATH);
+            request.setHeader("Connection", "Close");
+            return client().execute(request);
+        }
+
+        // Client construction must be delayed to ensure that the SslContextFactory is started before calling getSslContext().
+        private CloseableHttpClient client() {
+            if (client == null) {
+                synchronized (this) {
+                    if (client == null) {
+                        client = HttpClientBuilder.create()
+                                .disableAutomaticRetries()
+                                .setConnectionReuseStrategy(NoConnectionReuseStrategy.INSTANCE)
+                                .setSslcontext(sslContextFactory.getSslContext())
+                                .setSSLHostnameVerifier(NoopHostnameVerifier.INSTANCE)
+                                .setUserTokenHandler(context -> null) // https://stackoverflow.com/a/42112034/1615280
+                                .build();
+                    }
+                }
+            }
+            return client;
+        }
+
+        @Override
+        public void close() throws IOException {
+            synchronized (this) {
+                if (client != null) {
+                    client.close();
+                }
+            }
+        }
+    }
+}
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java
index 556d80d3772..5fa43a15912 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JDiscServerConnector.java
@@ -31,6 +31,7 @@ class JDiscServerConnector extends ServerConnector {
     private final Metric.Context metricCtx;
     private final Map<RequestDimensions, Metric.Context> requestMetricContextCache = new ConcurrentHashMap<>();
     private final ServerConnectionStatistics statistics;
+    private final ConnectorConfig config;
     private final boolean tcpKeepAlive;
     private final boolean tcpNoDelay;
     private final ServerSocketChannel channelOpenedByActivator;
@@ -42,6 +43,7 @@ class JDiscServerConnector extends ServerConnector {
                          ServerSocketChannel channelOpenedByActivator, ConnectionFactory... factories) {
         super(server, factories);
         this.channelOpenedByActivator = channelOpenedByActivator;
+        this.config = config;
         this.tcpKeepAlive = config.tcpKeepAliveEnabled();
         this.tcpNoDelay = config.tcpNoDelay();
         this.metric = metric;
@@ -139,6 +141,14 @@ class JDiscServerConnector extends ServerConnector {
         return (JDiscServerConnector) request.getAttribute(REQUEST_ATTRIBUTE);
     }
 
+    ConnectorConfig connectorConfig() {
+        return config;
+    }
+
+    int listenPort() {
+        return listenPort;
+    }
+
     private static Map<String, Object> createConnectorDimensions(int listenPort, String connectorName) {
         Map<String, Object> props = new HashMap<>();
         props.put(JettyHttpServer.Metrics.NAME_DIMENSION, connectorName);
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
index 2b9cb426dda..7a683b74656 100644
--- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
+++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/JettyHttpServer.java
@@ -46,6 +46,7 @@ import java.net.BindException;
 import java.net.MalformedURLException;
 import java.nio.channels.ServerSocketChannel;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.EnumSet;
 import java.util.HashMap;
@@ -60,6 +61,8 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
+import static java.util.stream.Collectors.toList;
+
 /**
  * @author Simon Thoresen Hult
  * @author bjorncs
@@ -168,11 +171,15 @@ public class JettyHttpServer extends AbstractServerProvider {
         ServletHolder jdiscServlet = new ServletHolder(new JDiscHttpServlet(jDiscContext));
         FilterHolder jDiscFilterInvokerFilter = new FilterHolder(new JDiscFilterInvokerFilter(jDiscContext, filterInvoker));
 
+        List<JDiscServerConnector> connectors = Arrays.stream(server.getConnectors())
+                .map(JDiscServerConnector.class::cast)
+                .collect(toList());
+
         server.setHandler(
                 getHandlerCollection(
                         serverConfig,
                         servletPathsConfig,
-                        connectorConfigs,
+                        connectors,
                         jdiscServlet,
                         servletHolders,
                         jDiscFilterInvokerFilter));
@@ -222,7 +229,7 @@ public class JettyHttpServer extends AbstractServerProvider {
     private HandlerCollection getHandlerCollection(
             ServerConfig serverConfig,
             ServletPathsConfig servletPathsConfig,
-            List<ConnectorConfig> connectorConfigs,
+            List<JDiscServerConnector> connectors,
             ServletHolder jdiscServlet,
             ComponentRegistry<ServletHolder> servletHolders,
             FilterHolder jDiscFilterInvokerFilter) {
@@ -237,8 +244,12 @@ public class JettyHttpServer extends AbstractServerProvider {
 
         servletContextHandler.addServlet(jdiscServlet, "/*");
 
+        var proxyHandler = new HealthCheckProxyHandler(connectors);
+        proxyHandler.setHandler(servletContextHandler);
+
+        List<ConnectorConfig> connectorConfigs = connectors.stream().map(JDiscServerConnector::connectorConfig).collect(toList());
         var authEnforcer = new TlsClientAuthenticationEnforcer(connectorConfigs);
-        authEnforcer.setHandler(servletContextHandler);
+        authEnforcer.setHandler(proxyHandler);
 
         GzipHandler gzipHandler = newGzipHandler(serverConfig);
         gzipHandler.setHandler(authEnforcer);
diff --git a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def
index f02a0d7b4a3..1122b1db3a9 100644
--- a/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def
+++ b/jdisc_http_service/src/main/resources/configdefinitions/jdisc.http.connector.def
@@ -88,3 +88,9 @@ tlsClientAuthEnforcer.enable          bool   default=false
 
 # Paths where client authentication should not be enforced. To be used in combination with WANT_AUTH. Typically used for health checks.
 tlsClientAuthEnforcer.pathWhitelist[]  string
+
+# Use connector only for proxying '/status.html' health checks. Any ssl configuration will be ignored if this option is enabled.
+healthCheckProxy.enable        bool    default=false
+
+# Which port to proxy
+healthCheckProxy.port          int     default=8080