HBASE-20639 Implement permission checking through AccessController instead of RSGroupAdminEndpoint

Signed-off-by: tedyu <yuzhihong@gmail.com>

hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminEndpoint.java

@@ -205,7 +205,6 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver {
         if (master.getMasterCoprocessorHost() != null) {
           master.getMasterCoprocessorHost().preMoveServers(hostPorts, request.getTargetGroup());
         }
-        checkPermission("moveServers");
         groupAdminServer.moveServers(hostPorts, request.getTargetGroup());
         if (master.getMasterCoprocessorHost() != null) {
           master.getMasterCoprocessorHost().postMoveServers(hostPorts, request.getTargetGroup());
@@ -230,7 +229,6 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver {
         if (master.getMasterCoprocessorHost() != null) {
           master.getMasterCoprocessorHost().preMoveTables(tables, request.getTargetGroup());
         }
-        checkPermission("moveTables");
         groupAdminServer.moveTables(tables, request.getTargetGroup());
         if (master.getMasterCoprocessorHost() != null) {
           master.getMasterCoprocessorHost().postMoveTables(tables, request.getTargetGroup());
@@ -250,7 +248,6 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver {
         if (master.getMasterCoprocessorHost() != null) {
           master.getMasterCoprocessorHost().preAddRSGroup(request.getRSGroupName());
         }
-        checkPermission("addRSGroup");
         groupAdminServer.addRSGroup(request.getRSGroupName());
         if (master.getMasterCoprocessorHost() != null) {
           master.getMasterCoprocessorHost().postAddRSGroup(request.getRSGroupName());
@@ -271,7 +268,6 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver {
         if (master.getMasterCoprocessorHost() != null) {
           master.getMasterCoprocessorHost().preRemoveRSGroup(request.getRSGroupName());
         }
-        checkPermission("removeRSGroup");
         groupAdminServer.removeRSGroup(request.getRSGroupName());
         if (master.getMasterCoprocessorHost() != null) {
           master.getMasterCoprocessorHost().postRemoveRSGroup(request.getRSGroupName());
@@ -292,7 +288,6 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver {
         if (master.getMasterCoprocessorHost() != null) {
           master.getMasterCoprocessorHost().preBalanceRSGroup(request.getRSGroupName());
         }
-        checkPermission("balanceRSGroup");
         boolean balancerRan = groupAdminServer.balanceRSGroup(request.getRSGroupName());
         builder.setBalanceRan(balancerRan);
         if (master.getMasterCoprocessorHost() != null) {
@@ -361,7 +356,6 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver {
           master.getMasterCoprocessorHost().preMoveServersAndTables(hostPorts, tables,
               request.getTargetGroup());
         }
-        checkPermission("moveServersAndTables");
         groupAdminServer.moveServersAndTables(hostPorts, tables, request.getTargetGroup());
         if (master.getMasterCoprocessorHost() != null) {
           master.getMasterCoprocessorHost().postMoveServersAndTables(hostPorts, tables,
@@ -389,7 +383,6 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver {
         if (master.getMasterCoprocessorHost() != null) {
           master.getMasterCoprocessorHost().preRemoveServers(servers);
         }
-        checkPermission("removeServers");
         groupAdminServer.removeServers(servers);
         if (master.getMasterCoprocessorHost() != null) {
           master.getMasterCoprocessorHost().postRemoveServers(servers);

hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsWithACL.java

@@ -23,6 +23,7 @@ import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.Coprocessor;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
 import org.apache.hadoop.hbase.HConstants;
@@ -32,9 +33,14 @@ import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
 import org.apache.hadoop.hbase.client.Connection;
 import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
 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.ObserverContextImpl;
+import org.apache.hadoop.hbase.master.MasterCoprocessorHost;
 import org.apache.hadoop.hbase.security.User;
 import org.apache.hadoop.hbase.security.access.AccessControlClient;
 import org.apache.hadoop.hbase.security.access.AccessControlLists;
+import org.apache.hadoop.hbase.security.access.AccessController;
 import org.apache.hadoop.hbase.security.access.Permission;
 import org.apache.hadoop.hbase.security.access.SecureTestUtil;
 import org.apache.hadoop.hbase.security.access.TableAuthManager;
@@ -94,6 +100,9 @@ public class TestRSGroupsWithACL extends SecureTestUtil{
   private static byte[] TEST_FAMILY = Bytes.toBytes("f1");
 
   private static RSGroupAdminEndpoint rsGroupAdminEndpoint;
+  private static AccessController accessController;
+  private static MasterCoprocessorEnvironment CP_ENV;
+  private static ObserverContext<MasterCoprocessorEnvironment> CTX;
 
   @BeforeClass
   public static void setupBeforeClass() throws Exception {
@@ -109,8 +118,15 @@ public class TestRSGroupsWithACL extends SecureTestUtil{
     configureRSGroupAdminEndpoint(conf);
 
     TEST_UTIL.startMiniCluster();
-    rsGroupAdminEndpoint = (RSGroupAdminEndpoint) TEST_UTIL.getMiniHBaseCluster().getMaster().
+    MasterCoprocessorHost masterCpHost =
-        getMasterCoprocessorHost().findCoprocessor(RSGroupAdminEndpoint.class.getName());
+        TEST_UTIL.getMiniHBaseCluster().getMaster().getMasterCoprocessorHost();
+    rsGroupAdminEndpoint =
+        (RSGroupAdminEndpoint) masterCpHost.findCoprocessor(RSGroupAdminEndpoint.class.getName());
+    accessController =
+        (AccessController) masterCpHost.findCoprocessor(AccessController.class.getName());
+    CP_ENV =
+        masterCpHost.createEnvironment(accessController, Coprocessor.PRIORITY_HIGHEST, 1, conf);
+    CTX = ObserverContextImpl.createAndPrepare(CP_ENV);
     // Wait for the ACL table to become available
     TEST_UTIL.waitUntilAllRegionsAssigned(AccessControlLists.ACL_TABLE_NAME);
 
@@ -223,9 +239,7 @@ public class TestRSGroupsWithACL extends SecureTestUtil{
       return null;
     };
 
-    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN);
+    validateAdminPermissions(action);
-    verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO,
-        USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE);
   }
 
   @Test
@@ -235,69 +249,57 @@ public class TestRSGroupsWithACL extends SecureTestUtil{
       return null;
     };
 
-    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN);
+    validateAdminPermissions(action);
-    verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO,
-        USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE);
   }
 
   @Test
   public void testMoveServers() throws Exception {
     AccessTestAction action = () -> {
-      rsGroupAdminEndpoint.checkPermission("moveServers");
+      accessController.preMoveServers(CTX, null, null);
       return null;
     };
 
-    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN);
+    validateAdminPermissions(action);
-    verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO,
-        USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE);
   }
 
   @Test
   public void testMoveTables() throws Exception {
     AccessTestAction action = () -> {
-      rsGroupAdminEndpoint.checkPermission("moveTables");
+      accessController.preMoveTables(CTX, null, null);
       return null;
     };
 
-    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN);
+    validateAdminPermissions(action);
-    verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO,
-        USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE);
   }
 
   @Test
   public void testAddRSGroup() throws Exception {
     AccessTestAction action = () -> {
-      rsGroupAdminEndpoint.checkPermission("addRSGroup");
+      accessController.preAddRSGroup(CTX, null);
       return null;
     };
 
-    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN);
+    validateAdminPermissions(action);
-    verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO,
-        USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE);
   }
 
   @Test
   public void testRemoveRSGroup() throws Exception {
     AccessTestAction action = () -> {
-      rsGroupAdminEndpoint.checkPermission("removeRSGroup");
+      accessController.preRemoveRSGroup(CTX, null);
       return null;
     };
 
-    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN);
+    validateAdminPermissions(action);
-    verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO,
-        USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE);
   }
 
   @Test
   public void testBalanceRSGroup() throws Exception {
     AccessTestAction action = () -> {
-      rsGroupAdminEndpoint.checkPermission("balanceRSGroup");
+      accessController.preBalanceRSGroup(CTX, null);
       return null;
     };
 
-    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN);
+    validateAdminPermissions(action);
-    verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO,
-        USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE);
   }
 
   @Test
@@ -307,9 +309,7 @@ public class TestRSGroupsWithACL extends SecureTestUtil{
       return null;
     };
 
-    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN);
+    validateAdminPermissions(action);
-    verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO,
-        USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE);
   }
 
   @Test
@@ -319,18 +319,30 @@ public class TestRSGroupsWithACL extends SecureTestUtil{
       return null;
     };
 
-    verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN);
+    validateAdminPermissions(action);
-    verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO,
-        USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE);
   }
 
   @Test
   public void testMoveServersAndTables() throws Exception {
     AccessTestAction action = () -> {
-      rsGroupAdminEndpoint.checkPermission("moveServersAndTables");
+      accessController.preMoveServersAndTables(CTX, null, null, null);
       return null;
     };
 
+    validateAdminPermissions(action);
+  }
+
+  @Test
+  public void testRemoveServers() throws Exception {
+    AccessTestAction action = () -> {
+      accessController.preRemoveServers(CTX, null);
+      return null;
+    };
+
+    validateAdminPermissions(action);
+  }
+
+  private void validateAdminPermissions(AccessTestAction action) throws Exception {
     verifyAllowed(action, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN);
     verifyDenied(action, USER_CREATE, USER_OWNER, USER_RW, USER_RO,
         USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE, USER_GROUP_CREATE);

hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java

@@ -95,6 +95,7 @@ import org.apache.hadoop.hbase.filter.FilterList;
 import org.apache.hadoop.hbase.io.hfile.HFile;
 import org.apache.hadoop.hbase.ipc.CoprocessorRpcUtils;
 import org.apache.hadoop.hbase.ipc.RpcServer;
+import org.apache.hadoop.hbase.net.Address;
 import org.apache.hadoop.hbase.protobuf.ProtobufUtil;
 import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos;
 import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.AccessControlService;
@@ -1307,6 +1308,48 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor,
     requirePermission(ctx, "recommissionRegionServers", Action.ADMIN);
   }
 
+  @Override
+  public void preMoveServersAndTables(final ObserverContext<MasterCoprocessorEnvironment> ctx,
+      Set<Address> servers, Set<TableName> tables, String targetGroup) throws IOException {
+    requirePermission(ctx, "moveServersAndTables", Action.ADMIN);
+  }
+
+  @Override
+  public void preMoveServers(final ObserverContext<MasterCoprocessorEnvironment> ctx,
+      Set<Address> servers, String targetGroup) throws IOException {
+    requirePermission(ctx, "moveServers", Action.ADMIN);
+  }
+
+  @Override
+  public void preMoveTables(final ObserverContext<MasterCoprocessorEnvironment> ctx,
+      Set<TableName> tables, String targetGroup) throws IOException {
+    requirePermission(ctx, "moveTables", Action.ADMIN);
+  }
+
+  @Override
+  public void preAddRSGroup(final ObserverContext<MasterCoprocessorEnvironment> ctx, String name)
+      throws IOException {
+    requirePermission(ctx, "addRSGroup", Action.ADMIN);
+  }
+
+  @Override
+  public void preRemoveRSGroup(final ObserverContext<MasterCoprocessorEnvironment> ctx, String name)
+      throws IOException {
+    requirePermission(ctx, "removeRSGroup", Action.ADMIN);
+  }
+
+  @Override
+  public void preBalanceRSGroup(final ObserverContext<MasterCoprocessorEnvironment> ctx,
+      String groupName) throws IOException {
+    requirePermission(ctx, "balanceRSGroup", Action.ADMIN);
+  }
+
+  @Override
+  public void preRemoveServers(final ObserverContext<MasterCoprocessorEnvironment> ctx,
+      Set<Address> servers) throws IOException {
+    requirePermission(ctx, "removeServers", Action.ADMIN);
+  }
+
   /* ---- RegionObserver implementation ---- */
 
   @Override