From 2870afc8679f38e1282b3e66e33b93db7f7382ed 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 | 60 +++++++++++++++++----- .../hadoop/hbase/regionserver/HRegionServer.java | 5 +- .../org/apache/hadoop/hbase/TestInfoServers.java | 3 +- 3 files changed, 52 insertions(+), 16 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..c4a4af9 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; @@ -208,7 +210,6 @@ import com.google.protobuf.Service; public class HMaster extends HRegionServer implements MasterServices { private static final Log LOG = LogFactory.getLog(HMaster.class.getName()); - /** * Protection against zombie master. Started once Master accepts active responsibility and * starts taking over responsibilities. Allows a finite time window before giving up ownership. @@ -391,14 +392,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 '" + + MASTER_HOSTNAME_KEY + "' 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 '" + MASTER_HOSTNAME_KEY + "' 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 +529,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 +548,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 = shouldUseThisHostnameInstead() ? useThisHostnameInstead : 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/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java index 58830b8..07807fd 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java @@ -438,9 +438,10 @@ public class HRegionServer extends HasThread implements // key to the config parameter of server hostname // the specification of server hostname is optional. The hostname should be resolvable from // both master and region server + @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG) final static String RS_HOSTNAME_KEY = "hbase.regionserver.hostname"; - - final static String MASTER_HOSTNAME_KEY = "hbase.master.hostname"; + @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG) + protected final static String MASTER_HOSTNAME_KEY = "hbase.master.hostname"; /** * This servers startcode. 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