From 6a4991ae2f512b77bc1d1f09d18a3be1bb788525 Mon Sep 17 00:00:00 2001 From: Sean Busbey Date: Wed, 15 Feb 2017 16:48:58 -0500 Subject: [PATCH] HBASE-15328 sanity check the redirect used to send master info requests to the embedded regionserver. --- .../org/apache/hadoop/hbase/master/HMaster.java | 72 ++++++++++++++++++---- .../org/apache/hadoop/hbase/TestInfoServers.java | 3 +- 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index 4aff21a..641d7eb 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -86,6 +86,7 @@ import org.apache.hadoop.hbase.executor.ExecutorType; import org.apache.hadoop.hbase.favored.FavoredNodesManager; import org.apache.hadoop.hbase.favored.FavoredNodesPromoter; import org.apache.hadoop.hbase.fs.HFileSystem; +import org.apache.hadoop.hbase.http.InfoServer; import org.apache.hadoop.hbase.ipc.CoprocessorRpcUtils; import org.apache.hadoop.hbase.ipc.RpcServer; import org.apache.hadoop.hbase.ipc.ServerNotRunningYetException; @@ -180,6 +181,7 @@ import org.apache.zookeeper.KeeperException; import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; +import org.eclipse.jetty.servlet.ServletHolder; import org.eclipse.jetty.webapp.WebAppContext; import com.google.common.annotations.VisibleForTesting; @@ -207,6 +209,19 @@ import com.google.protobuf.Service; @SuppressWarnings("deprecation") public class HMaster extends HRegionServer implements MasterServices { private static final Log LOG = LogFactory.getLog(HMaster.class.getName()); + /** + * You probably don't need to set this configuration value. You might need to if you have an esoteric network + * deployment. + * + * If set, config will be used to fill in the hostname component of all HTTP redirects sent on the old Master + * InfoServer port. If unset, we will use the hostname for the incoming request if it can be verified to + * resolv on this node to one of the local network interfaces (and return an HTTP 400 if it can't). Note that + * if set we will use the configured hostname without any additional validation (we do not even check if it + * is a valid hostname). It should be a hostname that redirected clients can use to reach the InfoServer of + * the embedded RegionServer hosted by the Master process. + */ + @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG) + private static final String REGION_SERVER_INFO_REDIRECT_HOST = "hbase.regionserver.info.redirect.hostname"; /** @@ -391,14 +406,42 @@ public class HMaster extends HRegionServer implements MasterServices { private Server masterJettyServer; public static class RedirectServlet extends HttpServlet { - private static final long serialVersionUID = 2894774810058302472L; - private static int regionServerInfoPort; + private static final long serialVersionUID = 2894774810058302473L; + private final int regionServerInfoPort; + private final String regionServerHostname; + + /** + * @param infoServer that we're trying to send all requests to + * @param hostname may be null. if given, will be used for redirects instead of host from client. + */ + public RedirectServlet(InfoServer infoServer, String hostname) { + regionServerInfoPort = infoServer.getPort(); + regionServerHostname = hostname; + } @Override public void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { + String redirectHost = regionServerHostname; + if(redirectHost == null) { + redirectHost = request.getServerName(); + if(!Addressing.isLocalAddress(InetAddress.getByName(redirectHost))) { + LOG.warn("Couldn't resolve '" + redirectHost + "' as an address local to this node and '" + + REGION_SERVER_INFO_REDIRECT_HOST + "' is not set; client will get a HTTP 400 response. If " + + "your HBase deployment relies on client accessible names that the region server process " + + "can't resolve locally, then you should set the previously mentioned configuration variable " + + "to an appropriate hostname."); + // no sending client provided input back to the client, so the goal host is just in the logs. + response.sendError(400, "Request was to a host that I can't resolve for any of the network interfaces on " + + "this node. If this is due to an intermediary such as an HTTP load balancer or other proxy, your HBase " + + "administrator can set '" + REGION_SERVER_INFO_REDIRECT_HOST + "' to point to the correct hostname."); + return; + } + } + // TODO this scheme should come from looking at the scheme registered in the infoserver's http server for the + // host and port we're using, but it's buried way too deep to do that ATM. String redirectUrl = request.getScheme() + "://" - + request.getServerName() + ":" + regionServerInfoPort + + redirectHost + ":" + regionServerInfoPort + request.getRequestURI(); response.sendRedirect(redirectUrl); } @@ -500,13 +543,16 @@ public class HMaster extends HRegionServer implements MasterServices { if (!conf.getBoolean("hbase.master.infoserver.redirect", true)) { return -1; } - int infoPort = conf.getInt("hbase.master.info.port.orig", + final int infoPort = conf.getInt("hbase.master.info.port.orig", HConstants.DEFAULT_MASTER_INFOPORT); // -1 is for disabling info server, so no redirecting if (infoPort < 0 || infoServer == null) { return -1; } - String addr = conf.get("hbase.master.info.bindAddress", "0.0.0.0"); + if(infoPort == infoServer.getPort()) { + return infoPort; + } + final String addr = conf.get("hbase.master.info.bindAddress", "0.0.0.0"); if (!Addressing.isLocalAddress(InetAddress.getByName(addr))) { String msg = "Failed to start redirecting jetty server. Address " + addr @@ -516,19 +562,21 @@ public class HMaster extends HRegionServer implements MasterServices { throw new IOException(msg); } - RedirectServlet.regionServerInfoPort = infoServer.getPort(); - if(RedirectServlet.regionServerInfoPort == infoPort) { - return infoPort; - } + // TODO I'm pretty sure we could just add another binding to the InfoServer run by + // the RegionServer and have it run the RedirectServlet instead of standing up + // a second entire stack here. masterJettyServer = new Server(); - ServerConnector connector = new ServerConnector(masterJettyServer); + final ServerConnector connector = new ServerConnector(masterJettyServer); connector.setHost(addr); connector.setPort(infoPort); masterJettyServer.addConnector(connector); masterJettyServer.setStopAtShutdown(true); - WebAppContext context = new WebAppContext(null, "/", null, null, null, null, WebAppContext.NO_SESSIONS); - context.addServlet(RedirectServlet.class, "/*"); + final String redirectHostname = conf.get(REGION_SERVER_INFO_REDIRECT_HOST, null); + + final RedirectServlet redirect = new RedirectServlet(infoServer, redirectHostname); + final WebAppContext context = new WebAppContext(null, "/", null, null, null, null, WebAppContext.NO_SESSIONS); + context.addServlet(new ServletHolder(redirect), "/*"); context.setServer(masterJettyServer); try { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestInfoServers.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestInfoServers.java index 2bafc33..f55ecf8 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestInfoServers.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestInfoServers.java @@ -71,7 +71,8 @@ public class TestInfoServers { } /** - * @throws Exception + * Ensure when we go to top level index pages that we get redirected to an info-server specific status + * page. */ @Test public void testInfoServersRedirect() throws Exception { -- 2.7.2