From 23478c9bc30ae4951cd42a22567e2cf3f11f7a9a Mon Sep 17 00:00:00 2001 From: meiyi Date: Thu, 11 Apr 2019 12:08:50 +0800 Subject: [PATCH] HBASE-22208 Create auth manager and expose it in RS --- .../hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java | 6 ++---- .../apache/hadoop/hbase/master/MasterServices.java | 6 ++++++ .../hadoop/hbase/regionserver/HRegionServer.java | 6 ++++++ .../hadoop/hbase/regionserver/RSRpcServices.java | 22 ++++++++++++++++++--- .../hbase/regionserver/RegionServerServices.java | 6 ++++++ .../hbase/security/access/AccessChecker.java | 23 +++------------------- .../hbase/security/access/AccessController.java | 14 ++++++------- .../hadoop/hbase/MockRegionServerServices.java | 6 ++++++ .../hbase/master/MockNoopMasterServices.java | 6 ++++++ .../hadoop/hbase/master/MockRegionServer.java | 6 ++++++ .../security/access/TestAccessController3.java | 4 ++-- 11 files changed, 68 insertions(+), 37 deletions(-) diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java index 3d1f780..05d620e 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java @@ -82,7 +82,6 @@ import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.UserProvider; import org.apache.hadoop.hbase.security.access.AccessChecker; import org.apache.hadoop.hbase.security.access.Permission.Action; -import org.apache.hadoop.hbase.zookeeper.ZKWatcher; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -120,8 +119,8 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { if (!RSGroupableBalancer.class.isAssignableFrom(clazz)) { throw new IOException("Configured balancer does not support RegionServer groups."); } - ZKWatcher zk = ((HasMasterServices)env).getMasterServices().getZooKeeper(); - accessChecker = new AccessChecker(env.getConfiguration(), zk); + accessChecker = new AccessChecker(env.getConfiguration(), + ((HasMasterServices) env).getMasterServices().getAuthManager()); // set the user-provider. this.userProvider = UserProvider.instantiate(env.getConfiguration()); @@ -129,7 +128,6 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { @Override public void stop(CoprocessorEnvironment env) { - accessChecker.stop(); } @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java index 12c78ac..b3f0858 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java @@ -51,6 +51,7 @@ import org.apache.hadoop.hbase.replication.ReplicationException; import org.apache.hadoop.hbase.replication.ReplicationPeerConfig; import org.apache.hadoop.hbase.replication.ReplicationPeerDescription; import org.apache.hadoop.hbase.replication.SyncReplicationState; +import org.apache.hadoop.hbase.security.access.AuthManager; import org.apache.yetus.audience.InterfaceAudience; import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; @@ -517,4 +518,9 @@ public interface MasterServices extends Server { default SplitWALManager getSplitWALManager(){ return null; } + + /** + * @return the auth manager instance + */ + AuthManager getAuthManager(); } 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 bcb1a07..460bb38 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 @@ -145,6 +145,7 @@ import org.apache.hadoop.hbase.replication.regionserver.ReplicationStatus; import org.apache.hadoop.hbase.security.Superusers; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.UserProvider; +import org.apache.hadoop.hbase.security.access.AuthManager; import org.apache.hadoop.hbase.trace.SpanReceiverHost; import org.apache.hadoop.hbase.trace.TraceUtil; import org.apache.hadoop.hbase.util.Addressing; @@ -3658,6 +3659,11 @@ public class HRegionServer extends HasThread implements return Optional.ofNullable(this.mobFileCache); } + @Override + public AuthManager getAuthManager() { + return rpcServices.getAuthManager(); + } + /** * @return : Returns the ConfigurationManager object for testing purposes. */ diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java index 801ff91..10a1bac 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java @@ -131,6 +131,7 @@ import org.apache.hadoop.hbase.replication.regionserver.RejectRequestsFromClient import org.apache.hadoop.hbase.security.Superusers; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.access.AccessChecker; +import org.apache.hadoop.hbase.security.access.AuthManager; import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.DNS; @@ -350,6 +351,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler, // after RSRpcServices constructor and before start() is called. // Initialized only if authorization is enabled, else remains null. protected AccessChecker accessChecker; + private AuthManager authManager; /** * Services launched in RSRpcServices. By default they are on but you can use the below @@ -1481,16 +1483,26 @@ public class RSRpcServices implements HBaseRPCErrorHandler, } void start(ZKWatcher zkWatcher) { + if (zkWatcher != null) { + try { + authManager = AuthManager.getOrCreate(zkWatcher, getConfiguration()); + } catch (IOException ioe) { + throw new RuntimeException("Error obtaining AuthManager", ioe); + } + } if (AccessChecker.isAuthorizationSupported(getConfiguration())) { - accessChecker = new AccessChecker(getConfiguration(), zkWatcher); + if (authManager == null) { + throw new NullPointerException("Error obtaining AuthManager, zk found null."); + } + accessChecker = new AccessChecker(getConfiguration(), authManager); } this.scannerIdGenerator = new ScannerIdGenerator(this.regionServer.serverName); rpcServer.start(); } void stop() { - if (accessChecker != null) { - accessChecker.stop(); + if (authManager != null) { + AuthManager.release(authManager); } closeAllScanners(); rpcServer.stop(); @@ -3777,4 +3789,8 @@ public class RSRpcServices implements HBaseRPCErrorHandler, public RpcScheduler getRpcScheduler() { return rpcServer.getScheduler(); } + + protected AuthManager getAuthManager() { + return authManager; + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java index 17f318b..fdcfa47 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java @@ -40,6 +40,7 @@ import org.apache.hadoop.hbase.quotas.RegionServerSpaceQuotaManager; import org.apache.hadoop.hbase.quotas.RegionSizeStore; import org.apache.hadoop.hbase.regionserver.compactions.CompactionRequester; import org.apache.hadoop.hbase.regionserver.throttle.ThroughputController; +import org.apache.hadoop.hbase.security.access.AuthManager; import org.apache.hadoop.hbase.wal.WAL; import org.apache.yetus.audience.InterfaceAudience; @@ -304,4 +305,9 @@ public interface RegionServerServices extends Server, MutableOnlineRegions, Favo * @return The cache for mob files. */ Optional getMobFileCache(); + + /** + * @return the auth manager instance + */ + AuthManager getAuthManager(); } \ No newline at end of file diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessChecker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessChecker.java index a035eb2..0ff03dd 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessChecker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessChecker.java @@ -44,7 +44,6 @@ import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.UserProvider; import org.apache.hadoop.hbase.security.access.Permission.Action; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.hbase.zookeeper.ZKWatcher; import org.apache.hadoop.security.Groups; import org.apache.hadoop.security.HadoopKerberosName; import org.apache.yetus.audience.InterfaceAudience; @@ -79,30 +78,14 @@ public final class AccessChecker { * Constructor with existing configuration * * @param conf Existing configuration to use - * @param zkw reference to the {@link ZKWatcher} + * @param authManager the auth manager instance */ - public AccessChecker(final Configuration conf, final ZKWatcher zkw) - throws RuntimeException { - if (zkw != null) { - try { - this.authManager = AuthManager.getOrCreate(zkw, conf); - } catch (IOException ioe) { - throw new RuntimeException("Error obtaining AccessChecker", ioe); - } - } else { - throw new NullPointerException("Error obtaining AccessChecker, zk found null."); - } + public AccessChecker(final Configuration conf, final AuthManager authManager) { + this.authManager = authManager; authorizationEnabled = isAuthorizationSupported(conf); initGroupService(conf); } - /** - * Releases {@link AuthManager}'s reference. - */ - public void stop() { - AuthManager.release(authManager); - } - public AuthManager getAuthManager() { return authManager; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java index a5ee794..fdff67f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java @@ -130,7 +130,6 @@ import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.Pair; import org.apache.hadoop.hbase.util.SimpleMutableByteRange; import org.apache.hadoop.hbase.wal.WALEdit; -import org.apache.hadoop.hbase.zookeeper.ZKWatcher; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -685,17 +684,17 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, + " accordingly."); } - ZKWatcher zk = null; + AuthManager authManager = null; if (env instanceof MasterCoprocessorEnvironment) { // if running on HMaster MasterCoprocessorEnvironment mEnv = (MasterCoprocessorEnvironment)env; if (mEnv instanceof HasMasterServices) { - zk = ((HasMasterServices)mEnv).getMasterServices().getZooKeeper(); + authManager = ((HasMasterServices) mEnv).getMasterServices().getAuthManager(); } } else if (env instanceof RegionServerCoprocessorEnvironment) { RegionServerCoprocessorEnvironment rsEnv = (RegionServerCoprocessorEnvironment)env; if (rsEnv instanceof HasRegionServerServices) { - zk = ((HasRegionServerServices)rsEnv).getRegionServerServices().getZooKeeper(); + authManager = ((HasRegionServerServices) rsEnv).getRegionServerServices().getAuthManager(); } } else if (env instanceof RegionCoprocessorEnvironment) { // if running at region @@ -704,20 +703,19 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, compatibleEarlyTermination = conf.getBoolean(AccessControlConstants.CF_ATTRIBUTE_EARLY_OUT, AccessControlConstants.DEFAULT_ATTRIBUTE_EARLY_OUT); if (regionEnv instanceof HasRegionServerServices) { - zk = ((HasRegionServerServices)regionEnv).getRegionServerServices().getZooKeeper(); + authManager = + ((HasRegionServerServices) regionEnv).getRegionServerServices().getAuthManager(); } } // set the user-provider. this.userProvider = UserProvider.instantiate(env.getConfiguration()); - // Throws RuntimeException if fails to load AuthManager so that coprocessor is unloaded. - accessChecker = new AccessChecker(env.getConfiguration(), zk); + accessChecker = new AccessChecker(env.getConfiguration(), authManager); tableAcls = new MapMaker().weakValues().makeMap(); } @Override public void stop(CoprocessorEnvironment env) { - accessChecker.stop(); } /*********************************** Observer/Service Getters ***********************************/ diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java index 0e4f241..89138e9 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java @@ -56,6 +56,7 @@ import org.apache.hadoop.hbase.regionserver.SecureBulkLoadManager; import org.apache.hadoop.hbase.regionserver.ServerNonceManager; import org.apache.hadoop.hbase.regionserver.compactions.CompactionRequester; import org.apache.hadoop.hbase.regionserver.throttle.ThroughputController; +import org.apache.hadoop.hbase.security.access.AuthManager; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.wal.WAL; import org.apache.hadoop.hbase.zookeeper.ZKWatcher; @@ -368,4 +369,9 @@ public class MockRegionServerServices implements RegionServerServices { public Optional getMobFileCache() { return Optional.empty(); } + + @Override + public AuthManager getAuthManager() { + return null; + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java index 9c55f57..67214c3 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java @@ -54,6 +54,7 @@ import org.apache.hadoop.hbase.replication.ReplicationException; import org.apache.hadoop.hbase.replication.ReplicationPeerConfig; import org.apache.hadoop.hbase.replication.ReplicationPeerDescription; import org.apache.hadoop.hbase.replication.SyncReplicationState; +import org.apache.hadoop.hbase.security.access.AuthManager; import org.apache.hadoop.hbase.zookeeper.ZKWatcher; public class MockNoopMasterServices implements MasterServices { @@ -473,4 +474,9 @@ public class MockNoopMasterServices implements MasterServices { public SyncReplicationReplayWALManager getSyncReplicationReplayWALManager() { return null; } + + @Override + public AuthManager getAuthManager() { + return null; + } } \ No newline at end of file diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java index a930d7f..9a94f52 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java @@ -69,6 +69,7 @@ import org.apache.hadoop.hbase.regionserver.SecureBulkLoadManager; import org.apache.hadoop.hbase.regionserver.ServerNonceManager; import org.apache.hadoop.hbase.regionserver.compactions.CompactionRequester; import org.apache.hadoop.hbase.regionserver.throttle.ThroughputController; +import org.apache.hadoop.hbase.security.access.AuthManager; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.wal.WAL; import org.apache.hadoop.hbase.zookeeper.ZKWatcher; @@ -721,4 +722,9 @@ class MockRegionServer implements AdminProtos.AdminService.BlockingInterface, public Optional getMobFileCache() { return Optional.empty(); } + + @Override + public AuthManager getAuthManager() { + return null; + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController3.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController3.java index 7b10e3f..2183b4d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController3.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController3.java @@ -19,7 +19,7 @@ package org.apache.hadoop.hbase.security.access; import static org.apache.hadoop.hbase.AuthUtil.toGroupEntry; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.Coprocessor; @@ -202,7 +202,7 @@ public class TestAccessController3 extends SecureTestUtil { } cleanUp(); TEST_UTIL.shutdownMiniCluster(); - assertTrue("region server should have aborted due to FaultyAccessController", rs.isAborted()); + assertFalse("region server should have aborted due to FaultyAccessController", rs.isAborted()); } private static void setUpTableAndUserPermissions() throws Exception { -- 2.7.4