From 8d19bbd34781a1b442f9dd382b3db57be2f77c64 Mon Sep 17 00:00:00 2001 From: Nihal Jain Date: Mon, 28 May 2018 20:01:39 +0530 Subject: [PATCH] HBASE-20653 Add missing observer hooks for region server group to MasterObserver Signed-off-by: tedyu --- .../hbase/rsgroup/RSGroupAdminEndpoint.java | 49 ++++++----- .../hadoop/hbase/rsgroup/TestRSGroups.java | 86 +++++++++++++++++++ .../hbase/rsgroup/TestRSGroupsWithACL.java | 8 +- .../hbase/coprocessor/MasterObserver.java | 62 +++++++++++++ .../hbase/master/MasterCoprocessorHost.java | 72 ++++++++++++++++ .../security/access/AccessController.java | 24 ++++++ 6 files changed, 272 insertions(+), 29 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 8546a4067ad..51d978b565c 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 @@ -49,7 +49,6 @@ import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment; import org.apache.hadoop.hbase.coprocessor.MasterObserver; import org.apache.hadoop.hbase.coprocessor.ObserverContext; import org.apache.hadoop.hbase.ipc.CoprocessorRpcUtils; -import org.apache.hadoop.hbase.ipc.RpcServer; import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.net.Address; import org.apache.hadoop.hbase.protobuf.ProtobufUtil; @@ -78,10 +77,8 @@ import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.RemoveRSGro import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.RemoveRSGroupResponse; import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.RemoveServersRequest; import org.apache.hadoop.hbase.protobuf.generated.RSGroupAdminProtos.RemoveServersResponse; -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; @@ -161,11 +158,16 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { LOG.info(master.getClientIdAuditPrefix() + " initiates rsgroup info retrieval, group=" + groupName); try { - checkPermission("getRSGroupInfo"); + if (master.getMasterCoprocessorHost() != null) { + master.getMasterCoprocessorHost().preGetRSGroupInfo(groupName); + } RSGroupInfo rsGroupInfo = groupAdminServer.getRSGroupInfo(groupName); if (rsGroupInfo != null) { builder.setRSGroupInfo(RSGroupProtobufUtil.toProtoGroupInfo(rsGroupInfo)); } + if (master.getMasterCoprocessorHost() != null) { + master.getMasterCoprocessorHost().postGetRSGroupInfo(groupName); + } } catch (IOException e) { CoprocessorRpcUtils.setControllerException(controller, e); } @@ -180,11 +182,16 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { LOG.info(master.getClientIdAuditPrefix() + " initiates rsgroup info retrieval, table=" + tableName); try { - checkPermission("getRSGroupInfoOfTable"); + if (master.getMasterCoprocessorHost() != null) { + master.getMasterCoprocessorHost().preGetRSGroupInfoOfTable(tableName); + } RSGroupInfo RSGroupInfo = groupAdminServer.getRSGroupInfoOfTable(tableName); if (RSGroupInfo != null) { builder.setRSGroupInfo(RSGroupProtobufUtil.toProtoGroupInfo(RSGroupInfo)); } + if (master.getMasterCoprocessorHost() != null) { + master.getMasterCoprocessorHost().postGetRSGroupInfoOfTable(tableName); + } } catch (IOException e) { CoprocessorRpcUtils.setControllerException(controller, e); } @@ -307,10 +314,15 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { ListRSGroupInfosResponse.Builder builder = ListRSGroupInfosResponse.newBuilder(); LOG.info(master.getClientIdAuditPrefix() + " list rsgroup"); try { - checkPermission("listRSGroup"); + if (master.getMasterCoprocessorHost() != null) { + master.getMasterCoprocessorHost().preListRSGroupInfos(); + } for (RSGroupInfo RSGroupInfo : groupAdminServer.listRSGroups()) { builder.addRSGroupInfo(RSGroupProtobufUtil.toProtoGroupInfo(RSGroupInfo)); } + if (master.getMasterCoprocessorHost() != null) { + master.getMasterCoprocessorHost().postListRSGroupInfos(); + } } catch (IOException e) { CoprocessorRpcUtils.setControllerException(controller, e); } @@ -326,11 +338,16 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { LOG.info(master.getClientIdAuditPrefix() + " initiates rsgroup info retrieval, server=" + hp); try { - checkPermission("getRSGroupInfoOfServer"); + if (master.getMasterCoprocessorHost() != null) { + master.getMasterCoprocessorHost().preGetRSGroupInfoOfServer(hp); + } RSGroupInfo info = groupAdminServer.getRSGroupOfServer(hp); if (info != null) { builder.setRSGroupInfo(RSGroupProtobufUtil.toProtoGroupInfo(info)); } + if (master.getMasterCoprocessorHost() != null) { + master.getMasterCoprocessorHost().postGetRSGroupInfoOfServer(hp); + } } catch (IOException e) { CoprocessorRpcUtils.setControllerException(controller, e); } @@ -510,22 +527,4 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { collect(Collectors.toSet()); groupAdminServer.removeServers(clearedServer); } - - public void checkPermission(String request) throws IOException { - accessChecker.requirePermission(getActiveUser(), request, Action.ADMIN); - } - - /** - * Returns the active user to which authorization checks should be applied. - * If we are in the context of an RPC call, the remote user is used, - * otherwise the currently logged in user is used. - */ - private User getActiveUser() throws IOException { - // for non-rpc handling, fallback to system user - Optional optionalUser = RpcServer.getRequestUser(); - if (optionalUser.isPresent()) { - return optionalUser.get(); - } - return userProvider.getCurrent(); - } } diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroups.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroups.java index 3e74f819cd3..3bd76be76b1 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroups.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroups.java @@ -292,6 +292,14 @@ public class TestRSGroups extends TestRSGroupsBase { boolean postRemoveServersCalled = false; boolean preMoveServersAndTables = false; boolean postMoveServersAndTables = false; + boolean preGetRSGroupInfoCalled = false; + boolean postGetRSGroupInfoCalled = false; + boolean preGetRSGroupInfoOfTableCalled = false; + boolean postGetRSGroupInfoOfTableCalled = false; + boolean preListRSGroupInfosCalled = false; + boolean postListRSGroupInfosCalled = false; + boolean preGetRSGroupInfoOfServerCalled = false; + boolean postGetRSGroupInfoOfServerCalled = false; @Override public Optional getMasterObserver() { @@ -370,7 +378,85 @@ public class TestRSGroups extends TestRSGroupsBase { String groupName, boolean balancerRan) throws IOException { postBalanceRSGroupCalled = true; } + + @Override + public void preGetRSGroupInfo(final ObserverContext ctx, + final String groupName) throws IOException { + preGetRSGroupInfoCalled = true; + } + + @Override + public void postGetRSGroupInfo(final ObserverContext ctx, + final String groupName) throws IOException { + postGetRSGroupInfoCalled = true; + } + + @Override + public void preGetRSGroupInfoOfTable(final ObserverContext ctx, + final TableName tableName) throws IOException { + preGetRSGroupInfoOfTableCalled = true; + } + + @Override + public void postGetRSGroupInfoOfTable(final ObserverContext ctx, + final TableName tableName) throws IOException { + postGetRSGroupInfoOfTableCalled = true; + } + + @Override + public void preListRSGroupInfos(final ObserverContext ctx) + throws IOException { + preListRSGroupInfosCalled = true; + } + + @Override + public void postListRSGroupInfos(final ObserverContext ctx) + throws IOException { + postListRSGroupInfosCalled = true; + } + + @Override + public void preGetRSGroupInfoOfServer(final ObserverContext ctx, + final Address server) throws IOException { + preGetRSGroupInfoOfServerCalled = true; + } + + @Override + public void postGetRSGroupInfoOfServer(final ObserverContext ctx, + final Address server) throws IOException { + postGetRSGroupInfoOfServerCalled = true; + } } + + @Test + public void testGetRSGroupInfoCPHookCalled() throws Exception { + rsGroupAdmin.getRSGroupInfo(RSGroupInfo.DEFAULT_GROUP); + assertTrue(observer.preGetRSGroupInfoCalled); + assertTrue(observer.postGetRSGroupInfoCalled); + } + + @Test + public void testGetRSGroupInfoOfTableCPHookCalled() throws Exception { + rsGroupAdmin.getRSGroupInfoOfTable(TableName.META_TABLE_NAME); + assertTrue(observer.preGetRSGroupInfoOfTableCalled); + assertTrue(observer.postGetRSGroupInfoOfTableCalled); + } + + @Test + public void testListRSGroupInfosCPHookCalled() throws Exception { + rsGroupAdmin.listRSGroups(); + assertTrue(observer.preListRSGroupInfosCalled); + assertTrue(observer.postListRSGroupInfosCalled); + } + + @Test + public void testGetRSGroupInfoOfServerCPHookCalled() throws Exception { + ServerName masterServerName = ((MiniHBaseCluster) cluster).getMaster().getServerName(); + rsGroupAdmin.getRSGroupOfServer(masterServerName.getAddress()); + assertTrue(observer.preGetRSGroupInfoOfServerCalled); + assertTrue(observer.postGetRSGroupInfoOfServerCalled); + } + @Test public void testMoveServersAndTables() throws Exception { super.testMoveServersAndTables(); diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java index 83d76a415eb..a4ee9dfc942 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java @@ -235,7 +235,7 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ @Test public void testGetRSGroupInfo() throws Exception { AccessTestAction action = () -> { - rsGroupAdminEndpoint.checkPermission("getRSGroupInfo"); + accessController.preGetRSGroupInfo(CTX, null); return null; }; @@ -245,7 +245,7 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ @Test public void testGetRSGroupInfoOfTable() throws Exception { AccessTestAction action = () -> { - rsGroupAdminEndpoint.checkPermission("getRSGroupInfoOfTable"); + accessController.preGetRSGroupInfoOfTable(CTX, null); return null; }; @@ -305,7 +305,7 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ @Test public void testListRSGroup() throws Exception { AccessTestAction action = () -> { - rsGroupAdminEndpoint.checkPermission("listRSGroup"); + accessController.preListRSGroupInfos(CTX); return null; }; @@ -315,7 +315,7 @@ public class TestRSGroupsWithACL extends SecureTestUtil{ @Test public void testGetRSGroupInfoOfServer() throws Exception { AccessTestAction action = () -> { - rsGroupAdminEndpoint.checkPermission("getRSGroupInfoOfServer"); + accessController.preGetRSGroupInfoOfServer(CTX, null); return null; }; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java index f60a04d02a0..92209f8ebf7 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java @@ -1212,6 +1212,68 @@ public interface MasterObserver { final ObserverContext ctx, Set
servers) throws IOException {} + /** + * Called before getting region server group info of the passed groupName. + * @param ctx the environment to interact with the framework and master + * @param groupName name of the group to get RSGroupInfo for + */ + default void preGetRSGroupInfo(final ObserverContext ctx, + final String groupName) throws IOException {} + + /** + * Called after getting region server group info of the passed groupName. + * @param ctx the environment to interact with the framework and master + * @param groupName name of the group to get RSGroupInfo for + */ + default void postGetRSGroupInfo(final ObserverContext ctx, + final String groupName) throws IOException {} + + /** + * Called before getting region server group info of the passed tableName. + * @param ctx the environment to interact with the framework and master + * @param tableName name of the table to get RSGroupInfo for + */ + default void preGetRSGroupInfoOfTable(final ObserverContext ctx, + final TableName tableName) throws IOException {} + + /** + * Called after getting region server group info of the passed tableName. + * @param ctx the environment to interact with the framework and master + * @param tableName name of the table to get RSGroupInfo for + */ + default void postGetRSGroupInfoOfTable(final ObserverContext ctx, + final TableName tableName) throws IOException {} + + /** + * Called before listing region server group information. + * @param ctx the environment to interact with the framework and master + */ + default void preListRSGroupInfos(final ObserverContext ctx) + throws IOException {} + + /** + * Called after listing region server group information. + * @param ctx the environment to interact with the framework and master + */ + default void postListRSGroupInfos(final ObserverContext ctx) + throws IOException {} + + /** + * Called before getting region server group info of the passed server. + * @param ctx the environment to interact with the framework and master + * @param server server to get RSGroupInfo for + */ + default void preGetRSGroupInfoOfServer(final ObserverContext ctx, + final Address server) throws IOException {} + + /** + * Called after getting region server group info of the passed server. + * @param ctx the environment to interact with the framework and master + * @param server server to get RSGroupInfo for + */ + default void postGetRSGroupInfoOfServer(final ObserverContext ctx, + final Address server) throws IOException {} + /** * Called before add a replication peer * @param ctx the environment to interact with the framework and master 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 072ae8ae660..da4cb2686f1 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 @@ -1404,6 +1404,78 @@ public class MasterCoprocessorHost }); } + public void preGetRSGroupInfo(final String groupName) throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + @Override + public void call(MasterObserver observer) throws IOException { + observer.preGetRSGroupInfo(this, groupName); + } + }); + } + + public void postGetRSGroupInfo(final String groupName) throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + @Override + public void call(MasterObserver observer) throws IOException { + observer.postGetRSGroupInfo(this, groupName); + } + }); + } + + public void preGetRSGroupInfoOfTable(final TableName tableName) throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + @Override + public void call(MasterObserver observer) throws IOException { + observer.preGetRSGroupInfoOfTable(this, tableName); + } + }); + } + + public void postGetRSGroupInfoOfTable(final TableName tableName) throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + @Override + public void call(MasterObserver observer) throws IOException { + observer.postGetRSGroupInfoOfTable(this, tableName); + } + }); + } + + public void preListRSGroupInfos() throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + @Override + public void call(MasterObserver observer) throws IOException { + observer.preListRSGroupInfos(this); + } + }); + } + + public void postListRSGroupInfos() throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + @Override + public void call(MasterObserver observer) throws IOException { + observer.postListRSGroupInfos(this); + } + }); + } + + public void preGetRSGroupInfoOfServer(final Address server) throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + @Override + public void call(MasterObserver observer) throws IOException { + observer.preGetRSGroupInfoOfServer(this, server); + } + }); + } + + public void postGetRSGroupInfoOfServer(final Address server) throws IOException { + execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { + @Override + public void call(MasterObserver observer) throws IOException { + observer.postGetRSGroupInfoOfServer(this, server); + } + }); + } + public void preAddReplicationPeer(final String peerId, final ReplicationPeerConfig peerConfig) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { 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 7e824e20b30..1ac98f58fa1 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 @@ -1306,6 +1306,18 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, requirePermission(ctx, "recommissionRegionServers", Action.ADMIN); } + @Override + public void preGetRSGroupInfo(final ObserverContext ctx, + String groupName) throws IOException { + requirePermission(ctx, "getRSGroupInfo", Action.ADMIN); + } + + @Override + public void preGetRSGroupInfoOfTable(final ObserverContext ctx, + TableName tableName) throws IOException { + requirePermission(ctx, "getRSGroupInfoOfTable", Action.ADMIN); + } + @Override public void preMoveServersAndTables(final ObserverContext ctx, Set
servers, Set tables, String targetGroup) throws IOException { @@ -1342,6 +1354,18 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, requirePermission(ctx, "balanceRSGroup", Action.ADMIN); } + @Override + public void preListRSGroupInfos(final ObserverContext ctx) + throws IOException { + requirePermission(ctx, "listRSGroup", Action.ADMIN); + } + + @Override + public void preGetRSGroupInfoOfServer(final ObserverContext ctx, + Address server) throws IOException { + requirePermission(ctx, "getRSGroupInfoOfServer", Action.ADMIN); + } + @Override public void preRemoveServers(final ObserverContext ctx, Set
servers) throws IOException {