From f1a80aadedfc8198ca02456e6638b28996c535ac Mon Sep 17 00:00:00 2001 From: Pankaj Kumar Date: Sat, 8 Oct 2016 12:48:27 +0800 Subject: [PATCH] HBASE-16663, JMX ConnectorServer stopped when unauthorized user try to stop HM/RS/cluster. --- .../hadoop/hbase/master/MasterCoprocessorHost.java | 58 +++++- .../regionserver/RegionServerCoprocessorHost.java | 51 ++++- .../hadoop/hbase/TestJMXConnectorServer.java | 206 +++++++++++++++++++++ 3 files changed, 309 insertions(+), 6 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 f585bb7..4135570 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 @@ -37,9 +37,6 @@ import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.coprocessor.*; import org.apache.hadoop.hbase.protobuf.generated.HBaseProtos.SnapshotDescription; -import java.io.IOException; -import java.util.List; - /** * Provides the coprocessor framework and environment for master oriented * operations. {@link HMaster} interacts with the loaded coprocessors @@ -738,7 +735,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 { @@ -753,7 +752,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 { @@ -948,4 +949,51 @@ 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; + } } 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 dfe9818..cebae51 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 @@ -77,7 +77,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 { @@ -290,6 +292,53 @@ public class RegionServerCoprocessorHost extends } /** + * 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 0000000..345bf7f --- /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.HBaseAdmin; +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 HBaseAdmin 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.getHBaseAdmin(); + + // 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.getHBaseAdmin(); + + 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.getHBaseAdmin(); + + 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 -- 2.7.2.windows.1