Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,8 @@ public enum Property {
+ " The resources that are used by default can be seen in"
+ " `accumulo/server/monitor/src/main/resources/templates/default.ftl`.",
"2.0.0"),
MONITOR_SUPPORT_HTTP2("monitor.http2.support", "false", PropertyType.BOOLEAN,
"If true will configure the Monitor web server to support http2 connections.", "3.1.0"),
Comment on lines +871 to +872
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this needs to be configurable. All modern browsers support it. If it improves our monitor's performance in any way, it should just be used automatically when we use TLS.

For the technical reasons listed on https://stackoverflow.com/a/46789195/196405 , I suggest not bothering to support it when TLS is off.

// per table properties
TABLE_PREFIX("table.", null, PropertyType.PREFIX,
"Properties in this category affect tablet server treatment of tablets,"
Expand Down
3 changes: 2 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@
<version.curator>5.5.0</version.curator>
<version.errorprone>2.24.1</version.errorprone>
<version.hadoop>3.4.0</version.hadoop>
<version.jetty>11.0.24</version.jetty>
<version.log4j>2.24.0</version.log4j>
<version.opentelemetry>1.34.1</version.opentelemetry>
<version.powermock>2.0.9</version.powermock>
Expand Down Expand Up @@ -206,7 +207,7 @@
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-bom</artifactId>
<version>11.0.19</version>
<version>${version.jetty}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
Expand Down
8 changes: 8 additions & 0 deletions server/monitor/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@
<groupId>org.apache.zookeeper</groupId>
<artifactId>zookeeper</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-alpn-server</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-server</artifactId>
Expand All @@ -120,6 +124,10 @@
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-util</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.jetty.http2</groupId>
<artifactId>http2-server</artifactId>
</dependency>
<dependency>
<groupId>org.glassfish.hk2</groupId>
<artifactId>hk2-api</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,12 @@

import org.apache.accumulo.core.conf.AccumuloConfiguration;
import org.apache.accumulo.core.conf.Property;
import org.eclipse.jetty.server.AbstractConnectionFactory;
import org.eclipse.jetty.alpn.server.ALPNServerConnectionFactory;
import org.eclipse.jetty.http2.server.HTTP2CServerConnectionFactory;
import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory;
import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.SecureRequestCustomizer;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.SslConnectionFactory;
Expand All @@ -41,30 +45,35 @@ public class EmbeddedWebServer {
Property.MONITOR_SSL_TRUSTSTORE, Property.MONITOR_SSL_TRUSTSTOREPASS);

private final Server server;
private final ServerConnector connector;
private final ServletContextHandler handler;
private final boolean secure;

public EmbeddedWebServer(Monitor monitor, int port) {
private int actualHttpPort;
private ServerConnector connector;

public EmbeddedWebServer(final Monitor monitor, int httpPort) {
server = new Server();
final AccumuloConfiguration conf = monitor.getContext().getConfiguration();
secure = requireForSecure.stream().map(conf::get).allMatch(s -> s != null && !s.isEmpty());

connector = new ServerConnector(server, getConnectionFactories(conf, secure));
connector.setHost(monitor.getHostname());
connector.setPort(port);
addConnectors(monitor, conf, httpPort, secure);

handler =
new ServletContextHandler(ServletContextHandler.SESSIONS | ServletContextHandler.SECURITY);
handler.getSessionHandler().getSessionCookieConfig().setHttpOnly(true);
handler.setContextPath("/");
}

private static AbstractConnectionFactory[] getConnectionFactories(AccumuloConfiguration conf,
private void addConnectors(Monitor monitor, AccumuloConfiguration conf, int httpPort,
boolean secure) {
HttpConnectionFactory httpFactory = new HttpConnectionFactory();

boolean configureHttp2 = conf.getBoolean(Property.MONITOR_SUPPORT_HTTP2);

final HttpConfiguration httpConfig = new HttpConfiguration();
httpConfig.setSendServerVersion(false);
Comment on lines +72 to +73
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unrelated, and only hides the Jetty version. Is that something we want to keep independently of this PR? What's the intention here?


if (secure) {
LOG.debug("Configuring Jetty to use TLS");
LOG.debug("Configuring Jetty with TLS");
final SslContextFactory.Server sslContextFactory = new SslContextFactory.Server();
// If the key password is the same as the keystore password, we don't
// have to explicitly set it. Thus, if the user doesn't provide a key
Expand Down Expand Up @@ -95,21 +104,47 @@ private static AbstractConnectionFactory[] getConnectionFactories(AccumuloConfig
sslContextFactory.setIncludeProtocols(includeProtocols.split(","));
}

SslConnectionFactory sslFactory =
new SslConnectionFactory(sslContextFactory, httpFactory.getProtocol());
return new AbstractConnectionFactory[] {sslFactory, httpFactory};
if (!configureHttp2) {
HttpConnectionFactory http11 = new HttpConnectionFactory(httpConfig);
SslConnectionFactory tls =
new SslConnectionFactory(sslContextFactory, http11.getProtocol());
connector = new ServerConnector(server, tls, http11);
} else {
LOG.debug("Enabling http2 support");
httpConfig.addCustomizer(new SecureRequestCustomizer());
HttpConnectionFactory http11 = new HttpConnectionFactory(httpConfig);
ALPNServerConnectionFactory alpn = new ALPNServerConnectionFactory();
alpn.setDefaultProtocol(http11.getProtocol());
HTTP2ServerConnectionFactory http2 = new HTTP2ServerConnectionFactory(httpConfig);
SslConnectionFactory tls = new SslConnectionFactory(sslContextFactory, alpn.getProtocol());
connector = new ServerConnector(server, tls, alpn, http2, http11);
}
connector.setHost(monitor.getHostname());
connector.setPort(httpPort);
server.addConnector(connector);
} else {
LOG.debug("Not configuring Jetty to use TLS");
return new AbstractConnectionFactory[] {httpFactory};
LOG.debug("Configuring Jetty without TLS");
if (!configureHttp2) {
HttpConnectionFactory http11 = new HttpConnectionFactory(httpConfig);
connector = new ServerConnector(server, http11);
} else {
LOG.debug("Enabling http2 support");
HttpConnectionFactory http11 = new HttpConnectionFactory(httpConfig);
HTTP2CServerConnectionFactory http2 = new HTTP2CServerConnectionFactory(httpConfig);
connector = new ServerConnector(server, http11, http2);
}
connector.setHost(monitor.getHostname());
connector.setPort(httpPort);
server.addConnector(connector);
}
}

public void addServlet(ServletHolder restServlet, String where) {
handler.addServlet(restServlet, where);
}

public int getPort() {
return connector.getLocalPort();
public int getHttpPort() {
return this.actualHttpPort;
}

public boolean isSecure() {
Expand All @@ -118,9 +153,9 @@ public boolean isSecure() {

public void start() {
try {
server.addConnector(connector);
server.setHandler(handler);
server.start();
this.actualHttpPort = connector.getLocalPort();
} catch (Exception e) {
stop();
throw new RuntimeException(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ public boolean add(Pair<Long,T> obj) {
+ "'accumulo compaction-coordinator'.";

private EmbeddedWebServer server;
private int livePort = 0;

private ServiceLock monitorLock;

Expand Down Expand Up @@ -465,7 +464,6 @@ public void run() {
server.addServlet(getRestServlet(), "/rest/*");
server.addServlet(getViewServlet(), "/*");
server.start();
livePort = port;
break;
} catch (Exception ex) {
log.error("Unable to start embedded web server", ex);
Expand All @@ -475,7 +473,7 @@ public void run() {
throw new RuntimeException(
"Unable to start embedded web server on ports: " + Arrays.toString(ports));
} else {
log.debug("Monitor started on port {}", livePort);
log.debug("Monitor started on port {}", server.getHttpPort());
}

String advertiseHost = getHostname();
Expand All @@ -486,7 +484,8 @@ public void run() {
log.error("Unable to get hostname", e);
}
}
HostAndPort monitorHostAndPort = HostAndPort.fromParts(advertiseHost, livePort);

HostAndPort monitorHostAndPort = HostAndPort.fromParts(advertiseHost, server.getHttpPort());
log.debug("Using {} to advertise monitor location in ZooKeeper", monitorHostAndPort);

try {
Expand All @@ -502,9 +501,10 @@ public void run() {
metricsInfo.init();

try {
URL url = new URL(server.isSecure() ? "https" : "http", advertiseHost, server.getPort(), "/");
final String path = context.getZooKeeperRoot() + Constants.ZMONITOR_HTTP_ADDR;
final ZooReaderWriter zoo = context.getZooReaderWriter();
URL url =
new URL(server.isSecure() ? "https" : "http", advertiseHost, server.getHttpPort(), "/");
final String path = context.getZooKeeperRoot() + Constants.ZMONITOR_HTTP_ADDR;
// Delete before we try to re-create in case the previous session hasn't yet expired
zoo.delete(path);
zoo.putEphemeralData(path, url.toString().getBytes(UTF_8));
Expand Down Expand Up @@ -1045,8 +1045,4 @@ public RecentLogs recentLogs() {
public Optional<HostAndPort> getCoordinatorHost() {
return coordinatorHost;
}

public int getLivePort() {
return livePort;
}
}
20 changes: 20 additions & 0 deletions test/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,26 @@
<groupId>org.easymock</groupId>
<artifactId>easymock</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-client</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-io</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-util</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.jetty.http2</groupId>
<artifactId>http2-client</artifactId>
</dependency>
<dependency>
<groupId>org.eclipse.jetty.http2</groupId>
<artifactId>http2-http-client-transport</artifactId>
</dependency>
<dependency>
<groupId>org.jline</groupId>
<artifactId>jline</artifactId>
Expand Down
Loading