From 0af10f89426dd97a1d599e29f1e301e74f9f9e99 Mon Sep 17 00:00:00 2001 From: Pankaj Kumar Date: Sat, 8 Oct 2016 12:29:06 +0800 Subject: [PATCH] HBASE-16663 JMX ConnectorServer stopped when unauthorized user try to stop HM/RS/cluster Signed-off-by: Andrew Purtell --- .../hbase/master/MasterCoprocessorHost.java | 55 ++++- .../RegionServerCoprocessorHost.java | 51 ++++- .../hadoop/hbase/TestJMXConnectorServer.java | 206 ++++++++++++++++++ 3 files changed, 309 insertions(+), 3 deletions(-) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/TestJMXConnectorServer.java diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java index 5ec60cd64a1..9eec0dd9fa9 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java @@ -856,7 +856,9 @@ public class MasterCoprocessorHost } public void preShutdown() throws IOException { - execOperation(coprocessors.isEmpty() ? null : new CoprocessorOperation() { + // While stopping the cluster all coprocessors method should be executed first then the + // coprocessor should be cleaned up. + execShutdown(coprocessors.isEmpty() ? null : new CoprocessorOperation() { @Override public void call(MasterObserver oserver, ObserverContext ctx) throws IOException { @@ -871,7 +873,9 @@ public class MasterCoprocessorHost } public void preStopMaster() throws IOException { - execOperation(coprocessors.isEmpty() ? null : new CoprocessorOperation() { + // While stopping master all coprocessors method should be executed first then the coprocessor + // environment should be cleaned up. + execShutdown(coprocessors.isEmpty() ? null : new CoprocessorOperation() { @Override public void call(MasterObserver oserver, ObserverContext ctx) throws IOException { @@ -1238,6 +1242,53 @@ public class MasterCoprocessorHost return bypass; } + /** + * Master coprocessor classes can be configured in any order, based on that priority is set and + * chained in a sorted order. For preStopMaster()/preShutdown(), coprocessor methods are invoked + * in call() and environment is shutdown in postEnvCall().
+ * Need to execute all coprocessor methods first then postEnvCall(), otherwise some coprocessors + * may remain shutdown if any exception occurs during next coprocessor execution which prevent + * Master stop or cluster shutdown. (Refer: + * HBASE-16663 + * @param ctx CoprocessorOperation + * @return true if bypaas coprocessor execution, false if not. + * @throws IOException + */ + private boolean execShutdown(final CoprocessorOperation ctx) throws IOException { + if (ctx == null) return false; + boolean bypass = false; + List envs = coprocessors.get(); + int envsSize = envs.size(); + // Iterate the coprocessors and execute CoprocessorOperation's call() + for (int i = 0; i < envsSize; i++) { + MasterEnvironment env = envs.get(i); + if (env.getInstance() instanceof MasterObserver) { + ctx.prepare(env); + Thread currentThread = Thread.currentThread(); + ClassLoader cl = currentThread.getContextClassLoader(); + try { + currentThread.setContextClassLoader(env.getClassLoader()); + ctx.call((MasterObserver) env.getInstance(), ctx); + } catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } finally { + currentThread.setContextClassLoader(cl); + } + bypass |= ctx.shouldBypass(); + if (ctx.shouldComplete()) { + break; + } + } + } + + // Iterate the coprocessors and execute CoprocessorOperation's postEnvCall() + for (int i = 0; i < envsSize; i++) { + MasterEnvironment env = envs.get(i); + ctx.postEnvCall(env); + } + return bypass; + } + public void preMoveServers(final Set servers, final String targetGroup) throws IOException { execOperation(coprocessors.isEmpty() ? null : new CoprocessorOperation() { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java index 0fbae349481..f0dd0d77f4a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java @@ -79,7 +79,9 @@ public class RegionServerCoprocessorHost extends } public void preStop(String message) throws IOException { - execOperation(coprocessors.isEmpty() ? null : new CoprocessorOperation() { + // While stopping the region server all coprocessors method should be executed first then the + // coprocessor should be cleaned up. + execShutdown(coprocessors.isEmpty() ? null : new CoprocessorOperation() { @Override public void call(RegionServerObserver oserver, ObserverContext ctx) throws IOException { @@ -280,6 +282,53 @@ public class RegionServerCoprocessorHost extends return bypass; } + /** + * RegionServer coprocessor classes can be configured in any order, based on that priority is set + * and chained in a sorted order. For preStop(), coprocessor methods are invoked in call() and + * environment is shutdown in postEnvCall().
+ * Need to execute all coprocessor methods first then postEnvCall(), otherwise some coprocessors + * may remain shutdown if any exception occurs during next coprocessor execution which prevent + * RegionServer stop. (Refer: + * HBASE-16663 + * @param ctx CoprocessorOperation + * @return true if bypaas coprocessor execution, false if not. + * @throws IOException + */ + private boolean execShutdown(final CoprocessorOperation ctx) throws IOException { + if (ctx == null) return false; + boolean bypass = false; + List envs = coprocessors.get(); + int envsSize = envs.size(); + // Iterate the coprocessors and execute CoprocessorOperation's call() + for (int i = 0; i < envsSize; i++) { + RegionServerEnvironment env = envs.get(i); + if (env.getInstance() instanceof RegionServerObserver) { + ctx.prepare(env); + Thread currentThread = Thread.currentThread(); + ClassLoader cl = currentThread.getContextClassLoader(); + try { + currentThread.setContextClassLoader(env.getClassLoader()); + ctx.call((RegionServerObserver) env.getInstance(), ctx); + } catch (Throwable e) { + handleCoprocessorThrowable(env, e); + } finally { + currentThread.setContextClassLoader(cl); + } + bypass |= ctx.shouldBypass(); + if (ctx.shouldComplete()) { + break; + } + } + } + + // Iterate the coprocessors and execute CoprocessorOperation's postEnvCall() + for (int i = 0; i < envsSize; i++) { + RegionServerEnvironment env = envs.get(i); + ctx.postEnvCall(env); + } + return bypass; + } + /** * Coprocessor environment extension providing access to region server * related services. diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestJMXConnectorServer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestJMXConnectorServer.java new file mode 100644 index 00000000000..44220f55e31 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestJMXConnectorServer.java @@ -0,0 +1,206 @@ +/** + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase; + +import java.io.IOException; + +import javax.management.remote.JMXConnector; +import javax.management.remote.JMXConnectorFactory; +import javax.naming.ServiceUnavailableException; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; +import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment; +import org.apache.hadoop.hbase.coprocessor.ObserverContext; +import org.apache.hadoop.hbase.coprocessor.RegionServerCoprocessorEnvironment; +import org.apache.hadoop.hbase.security.AccessDeniedException; +import org.apache.hadoop.hbase.security.access.AccessController; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.apache.hadoop.hbase.testclassification.MiscTests; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +/** + * Test case for JMX Connector Server. + */ +@Category({ MiscTests.class, MediumTests.class }) +public class TestJMXConnectorServer { + private static final Log LOG = LogFactory.getLog(TestJMXConnectorServer.class); + private static HBaseTestingUtility UTIL = new HBaseTestingUtility(); + + private static Configuration conf = null; + private static Admin admin; + // RMI registry port + private static int rmiRegistryPort = 61120; + // Switch for customized Accesscontroller to throw ACD exception while executing test case + static boolean hasAccess; + + @Before + public void setUp() throws Exception { + UTIL = new HBaseTestingUtility(); + conf = UTIL.getConfiguration(); + } + + @After + public void tearDown() throws Exception { + // Set to true while stopping cluster + hasAccess = true; + admin.close(); + UTIL.shutdownMiniCluster(); + } + + /** + * This tests to validate the HMaster's ConnectorServer after unauthorised stopMaster call. + */ + @Test(timeout = 180000) + public void testHMConnectorServerWhenStopMaster() throws Exception { + conf.set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, + JMXListener.class.getName() + "," + MyAccessController.class.getName()); + conf.setInt("master.rmi.registry.port", rmiRegistryPort); + UTIL.startMiniCluster(); + admin = UTIL.getConnection().getAdmin(); + + // try to stop master + boolean accessDenied = false; + try { + hasAccess = false; + LOG.info("Stopping HMaster..."); + admin.stopMaster(); + } catch (AccessDeniedException e) { + LOG.info("Exception occured while stopping HMaster. ", e); + accessDenied = true; + } + Assert.assertTrue(accessDenied); + + // Check whether HMaster JMX Connector server can be connected + JMXConnector connector = null; + try { + connector = JMXConnectorFactory + .connect(JMXListener.buildJMXServiceURL(rmiRegistryPort, rmiRegistryPort)); + } catch (IOException e) { + if (e.getCause() instanceof ServiceUnavailableException) { + Assert.fail("Can't connect to HMaster ConnectorServer."); + } + } + Assert.assertNotNull("JMXConnector should not be null.", connector); + connector.close(); + } + + /** + * This tests to validate the RegionServer's ConnectorServer after unauthorised stopRegionServer + * call. + */ + @Test(timeout = 180000) + public void testRSConnectorServerWhenStopRegionServer() throws Exception { + conf.set(CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY, + JMXListener.class.getName() + "," + MyAccessController.class.getName()); + conf.setInt("regionserver.rmi.registry.port", rmiRegistryPort); + UTIL.startMiniCluster(); + admin = UTIL.getConnection().getAdmin(); + + hasAccess = false; + ServerName serverName = UTIL.getHBaseCluster().getRegionServer(0).getServerName(); + LOG.info("Stopping Region Server..."); + admin.stopRegionServer(serverName.getHostname() + ":" + serverName.getPort()); + + // Check whether Region Sever JMX Connector server can be connected + JMXConnector connector = null; + try { + connector = JMXConnectorFactory + .connect(JMXListener.buildJMXServiceURL(rmiRegistryPort, rmiRegistryPort)); + } catch (IOException e) { + if (e.getCause() instanceof ServiceUnavailableException) { + Assert.fail("Can't connect to Region Server ConnectorServer."); + } + } + Assert.assertNotNull("JMXConnector should not be null.", connector); + connector.close(); + } + + /** + * This tests to validate the HMaster's ConnectorServer after unauthorised shutdown call. + */ + @Test(timeout = 180000) + public void testHMConnectorServerWhenShutdownCluster() throws Exception { + conf.set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, + JMXListener.class.getName() + "," + MyAccessController.class.getName()); + conf.setInt("master.rmi.registry.port", rmiRegistryPort); + + UTIL.startMiniCluster(); + admin = UTIL.getConnection().getAdmin(); + + boolean accessDenied = false; + try { + hasAccess = false; + LOG.info("Stopping HMaster..."); + admin.shutdown(); + } catch (AccessDeniedException e) { + LOG.error("Exception occured while stopping HMaster. ", e); + accessDenied = true; + } + Assert.assertTrue(accessDenied); + + // Check whether HMaster JMX Connector server can be connected + JMXConnector connector = null; + try { + connector = JMXConnectorFactory + .connect(JMXListener.buildJMXServiceURL(rmiRegistryPort, rmiRegistryPort)); + } catch (IOException e) { + if (e.getCause() instanceof ServiceUnavailableException) { + Assert.fail("Can't connect to HMaster ConnectorServer."); + } + } + Assert.assertNotNull("JMXConnector should not be null.", connector); + connector.close(); + } + + /* + * Customized class for test case execution which will throw ACD exception while executing + * stopMaster/preStopRegionServer/preShutdown explicitly. + */ + public static class MyAccessController extends AccessController { + @Override + public void preStopMaster(ObserverContext c) throws IOException { + if (!hasAccess) { + throw new AccessDeniedException("Insufficient permissions to stop master"); + } + } + + @Override + public void preStopRegionServer(ObserverContext ctx) + throws IOException { + if (!hasAccess) { + throw new AccessDeniedException("Insufficient permissions to stop region server."); + } + } + + @Override + public void preShutdown(ObserverContext c) throws IOException { + if (!hasAccess) { + throw new AccessDeniedException("Insufficient permissions to shut down cluster."); + } + } + } +} \ No newline at end of file