From 79c50f3863b2da0dceaa50481864bc8401d4595c Mon Sep 17 00:00:00 2001 From: Michael Stack Date: Mon, 23 Oct 2017 15:31:47 -0700 Subject: [PATCH] HBASE-19048 Cleanup MasterObserver hooks which takes IA private params --- .../hadoop/hbase/rsgroup/RSGroupAdminServer.java | 18 -- .../hadoop/hbase/coprocessor/MasterObserver.java | 132 --------------- .../org/apache/hadoop/hbase/master/HMaster.java | 40 +---- .../hadoop/hbase/master/MasterCoprocessorHost.java | 172 ------------------- .../hadoop/hbase/master/locking/LockManager.java | 14 -- .../hbase/security/access/AccessController.java | 97 ----------- .../hbase/coprocessor/TestMasterObserver.java | 185 --------------------- .../security/access/TestAccessController.java | 178 -------------------- 8 files changed, 5 insertions(+), 831 deletions(-) diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java index 3c82d76b9e..eb047841cd 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java @@ -423,13 +423,7 @@ public class RSGroupAdminServer implements RSGroupAdmin { @Override public void addRSGroup(String name) throws IOException { - if (master.getMasterCoprocessorHost() != null) { - master.getMasterCoprocessorHost().preAddRSGroup(name); - } rsGroupInfoManager.addRSGroup(new RSGroupInfo(name)); - if (master.getMasterCoprocessorHost() != null) { - master.getMasterCoprocessorHost().postAddRSGroup(name); - } } @Override @@ -437,9 +431,6 @@ public class RSGroupAdminServer implements RSGroupAdmin { // Hold a lock on the manager instance while moving servers to prevent // another writer changing our state while we are working. synchronized (rsGroupInfoManager) { - if (master.getMasterCoprocessorHost() != null) { - master.getMasterCoprocessorHost().preRemoveRSGroup(name); - } RSGroupInfo rsGroupInfo = rsGroupInfoManager.getRSGroup(name); if (rsGroupInfo == null) { throw new ConstraintException("RSGroup " + name + " does not exist"); @@ -464,9 +455,6 @@ public class RSGroupAdminServer implements RSGroupAdmin { } } rsGroupInfoManager.removeRSGroup(name); - if (master.getMasterCoprocessorHost() != null) { - master.getMasterCoprocessorHost().postRemoveRSGroup(name); - } } } @@ -479,9 +467,6 @@ public class RSGroupAdminServer implements RSGroupAdmin { synchronized (balancer) { // If balance not true, don't run balancer. if (!((HMaster) master).isBalancerOn()) return false; - if (master.getMasterCoprocessorHost() != null) { - master.getMasterCoprocessorHost().preBalanceRSGroup(groupName); - } if (getRSGroupInfo(groupName) == null) { throw new ConstraintException("RSGroup does not exist: "+groupName); } @@ -523,9 +508,6 @@ public class RSGroupAdminServer implements RSGroupAdmin { LOG.info("RSGroup balance " + groupName + " completed after " + (System.currentTimeMillis()-startTime) + " seconds"); } - if (master.getMasterCoprocessorHost() != null) { - master.getMasterCoprocessorHost().postBalanceRSGroup(groupName, balancerRan); - } return balancerRan; } } 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 397ec8ad4c..58b05ad3d3 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 @@ -354,58 +354,6 @@ public interface MasterObserver { final TableName tableName) throws IOException {} /** - * Called before a abortProcedure request has been processed. - * @param ctx the environment to interact with the framework and master - * @param procEnv procedure executor - * @param procId the Id of the procedure - */ - default void preAbortProcedure( - ObserverContext ctx, - final ProcedureExecutor procEnv, - final long procId) throws IOException {} - - /** - * Called after a abortProcedure request has been processed. - * @param ctx the environment to interact with the framework and master - */ - default void postAbortProcedure(ObserverContext ctx) - throws IOException {} - - /** - * Called before a getProcedures request has been processed. - * @param ctx the environment to interact with the framework and master - */ - default void preGetProcedures(ObserverContext ctx) - throws IOException {} - - /** - * Called after a getProcedures request has been processed. - * @param ctx the environment to interact with the framework and master - * @param procList the list of procedures about to be returned - */ - default void postGetProcedures( - ObserverContext ctx, - List> procList) throws IOException {} - - /** - * Called before a getLocks request has been processed. - * @param ctx the environment to interact with the framework and master - * @throws IOException if something went wrong - */ - default void preGetLocks(ObserverContext ctx) - throws IOException {} - - /** - * Called after a getLocks request has been processed. - * @param ctx the environment to interact with the framework and master - * @param lockedResources the list of locks about to be returned - * @throws IOException if something went wrong - */ - default void postGetLocks( - ObserverContext ctx, - List lockedResources) throws IOException {} - - /** * Called prior to moving a given region from one region server to another. * @param ctx the environment to interact with the framework and master * @param region the RegionInfo @@ -1101,54 +1049,6 @@ public interface MasterObserver { Set tables, String targetGroup) throws IOException {} /** - * Called before a new region server group is added - * @param ctx the environment to interact with the framework and master - * @param name group name - */ - default void preAddRSGroup(final ObserverContext ctx, - String name) throws IOException {} - - /** - * Called after a new region server group is added - * @param ctx the environment to interact with the framework and master - * @param name group name - */ - default void postAddRSGroup(final ObserverContext ctx, - String name) throws IOException {} - - /** - * Called before a region server group is removed - * @param ctx the environment to interact with the framework and master - * @param name group name - */ - default void preRemoveRSGroup(final ObserverContext ctx, - String name) throws IOException {} - - /** - * Called after a region server group is removed - * @param ctx the environment to interact with the framework and master - * @param name group name - */ - default void postRemoveRSGroup(final ObserverContext ctx, - String name) throws IOException {} - - /** - * Called before a region server group is removed - * @param ctx the environment to interact with the framework and master - * @param groupName group name - */ - default void preBalanceRSGroup(final ObserverContext ctx, - String groupName) throws IOException {} - - /** - * Called after a region server group is removed - * @param ctx the environment to interact with the framework and master - * @param groupName group name - */ - default void postBalanceRSGroup(final ObserverContext ctx, - String groupName, boolean balancerRan) throws IOException {} - - /** * Called before add a replication peer * @param ctx the environment to interact with the framework and master * @param peerId a short name that identifies the peer @@ -1265,38 +1165,6 @@ public interface MasterObserver { String regex) throws IOException {} /** - * Called before new LockProcedure is queued. - * @param ctx the environment to interact with the framework and master - */ - default void preRequestLock(ObserverContext ctx, String namespace, - TableName tableName, RegionInfo[] regionInfos, LockType type, - String description) throws IOException {} - - /** - * Called after new LockProcedure is queued. - * @param ctx the environment to interact with the framework and master - */ - default void postRequestLock(ObserverContext ctx, String namespace, - TableName tableName, RegionInfo[] regionInfos, LockType type, - String description) throws IOException {} - - /** - * Called before heartbeat to a lock. - * @param ctx the environment to interact with the framework and master - * @param keepAlive if lock should be kept alive; lock will be released if set to false. - */ - default void preLockHeartbeat(ObserverContext ctx, - LockProcedure proc, boolean keepAlive) throws IOException {} - - /** - * Called after heartbeat to a lock. - * @param ctx the environment to interact with the framework and master - * @param keepAlive if lock was kept alive; lock was released if set to false. - */ - default void postLockHeartbeat(ObserverContext ctx, - LockProcedure proc, boolean keepAlive) throws IOException {} - - /** * Called before list dead region servers. */ default void preListDeadServers(ObserverContext ctx) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index 8f2ae6b209..0158bddc3f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -3026,49 +3026,19 @@ public class HMaster extends HRegionServer implements MasterServices { @Override public boolean abortProcedure(final long procId, final boolean mayInterruptIfRunning) throws IOException { - if (cpHost != null) { - cpHost.preAbortProcedure(this.procedureExecutor, procId); - } - - final boolean result = this.procedureExecutor.abort(procId, mayInterruptIfRunning); - - if (cpHost != null) { - cpHost.postAbortProcedure(); - } - - return result; + return this.procedureExecutor.abort(procId, mayInterruptIfRunning); } @Override public List> getProcedures() throws IOException { - if (cpHost != null) { - cpHost.preGetProcedures(); - } - - final List> procList = this.procedureExecutor.getProcedures(); - - if (cpHost != null) { - cpHost.postGetProcedures(procList); - } - - return procList; + return this.procedureExecutor.getProcedures(); } @Override public List getLocks() throws IOException { - if (cpHost != null) { - cpHost.preGetLocks(); - } - - MasterProcedureScheduler procedureScheduler = procedureExecutor.getEnvironment().getProcedureScheduler(); - - final List lockedResources = procedureScheduler.getLocks(); - - if (cpHost != null) { - cpHost.postGetLocks(lockedResources); - } - - return lockedResources; + MasterProcedureScheduler procedureScheduler = + procedureExecutor.getEnvironment().getProcedureScheduler(); + return procedureScheduler.getLocks(); } /** 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 f3f34bf9af..6becaf2bc8 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 @@ -49,14 +49,8 @@ import org.apache.hadoop.hbase.coprocessor.MasterCoprocessorEnvironment; import org.apache.hadoop.hbase.coprocessor.MasterObserver; import org.apache.hadoop.hbase.coprocessor.MetricsCoprocessor; import org.apache.hadoop.hbase.coprocessor.ObserverContext; -import org.apache.hadoop.hbase.master.locking.LockProcedure; -import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; import org.apache.hadoop.hbase.metrics.MetricRegistry; import org.apache.hadoop.hbase.net.Address; -import org.apache.hadoop.hbase.procedure2.LockType; -import org.apache.hadoop.hbase.procedure2.LockedResource; -import org.apache.hadoop.hbase.procedure2.Procedure; -import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; import org.apache.hadoop.hbase.quotas.GlobalQuotaSettings; import org.apache.hadoop.hbase.replication.ReplicationPeerConfig; import org.apache.hadoop.hbase.security.User; @@ -530,62 +524,6 @@ public class MasterCoprocessorHost }); } - public boolean preAbortProcedure( - final ProcedureExecutor procEnv, - final long procId) throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { - @Override - public void call(MasterObserver observer) throws IOException { - observer.preAbortProcedure(this, procEnv, procId); - } - }); - } - - public void postAbortProcedure() throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { - @Override - public void call(MasterObserver observer) throws IOException { - observer.postAbortProcedure(this); - } - }); - } - - public boolean preGetProcedures() throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { - @Override - public void call(MasterObserver observer) throws IOException { - observer.preGetProcedures(this); - } - }); - } - - public void postGetProcedures(final List> procInfoList) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { - @Override - public void call(MasterObserver observer) throws IOException { - observer.postGetProcedures(this, procInfoList); - } - }); - } - - public boolean preGetLocks() throws IOException { - return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { - @Override - public void call(MasterObserver observer) throws IOException { - observer.preGetLocks(this); - } - }); - } - - public void postGetLocks(final List lockedResources) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { - @Override - public void call(MasterObserver observer) throws IOException { - observer.postGetLocks(this, lockedResources); - } - }); - } - public boolean preMove(final RegionInfo region, final ServerName srcServer, final ServerName destServer) throws IOException { return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @@ -1310,78 +1248,6 @@ public class MasterCoprocessorHost }); } - public void preAddRSGroup(final String name) - throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { - @Override - public void call(MasterObserver observer) throws IOException { - if(((MasterEnvironment)getEnvironment()).supportGroupCPs) { - observer.preAddRSGroup(this, name); - } - } - }); - } - - public void postAddRSGroup(final String name) - throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { - @Override - public void call(MasterObserver observer) throws IOException { - if (((MasterEnvironment)getEnvironment()).supportGroupCPs) { - observer.postAddRSGroup(this, name); - } - } - }); - } - - public void preRemoveRSGroup(final String name) - throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { - @Override - public void call(MasterObserver observer) throws IOException { - if(((MasterEnvironment)getEnvironment()).supportGroupCPs) { - observer.preRemoveRSGroup(this, name); - } - } - }); - } - - public void postRemoveRSGroup(final String name) - throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { - @Override - public void call(MasterObserver observer) throws IOException { - if(((MasterEnvironment)getEnvironment()).supportGroupCPs) { - observer.postRemoveRSGroup(this, name); - } - } - }); - } - - public void preBalanceRSGroup(final String name) - throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { - @Override - public void call(MasterObserver observer) throws IOException { - if(((MasterEnvironment)getEnvironment()).supportGroupCPs) { - observer.preBalanceRSGroup(this, name); - } - } - }); - } - - public void postBalanceRSGroup(final String name, final boolean balanceRan) - throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { - @Override - public void call(MasterObserver observer) throws IOException { - if(((MasterEnvironment)getEnvironment()).supportGroupCPs) { - observer.postBalanceRSGroup(this, name, balanceRan); - } - } - }); - } - public void preAddReplicationPeer(final String peerId, final ReplicationPeerConfig peerConfig) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @@ -1512,44 +1378,6 @@ public class MasterCoprocessorHost }); } - public void preRequestLock(String namespace, TableName tableName, RegionInfo[] regionInfos, - LockType type, String description) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { - @Override - public void call(MasterObserver observer) throws IOException { - observer.preRequestLock(this, namespace, tableName, regionInfos, type, description); - } - }); - } - - public void postRequestLock(String namespace, TableName tableName, RegionInfo[] regionInfos, - LockType type, String description) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { - @Override - public void call(MasterObserver observer) throws IOException { - observer.postRequestLock(this, namespace, tableName, regionInfos, type, description); - } - }); - } - - public void preLockHeartbeat(LockProcedure proc, boolean keepAlive) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { - @Override - public void call(MasterObserver observer) throws IOException { - observer.preLockHeartbeat(this, proc, keepAlive); - } - }); - } - - public void postLockHeartbeat(LockProcedure proc, boolean keepAlive) throws IOException { - execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { - @Override - public void call(MasterObserver observer) throws IOException { - observer.postLockHeartbeat(this, proc, keepAlive); - } - }); - } - public void preListDeadServers() throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() { @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/locking/LockManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/locking/LockManager.java index 883d6596ca..6f6370a99e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/locking/LockManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/locking/LockManager.java @@ -208,22 +208,18 @@ public final class LockManager { public long requestNamespaceLock(final String namespace, final LockType type, final String description, final NonceKey nonceKey) throws IllegalArgumentException, IOException { - master.getMasterCoprocessorHost().preRequestLock(namespace, null, null, type, description); final LockProcedure proc = new LockProcedure(master.getConfiguration(), namespace, type, description, null); submitProcedure(proc, nonceKey); - master.getMasterCoprocessorHost().postRequestLock(namespace, null, null, type, description); return proc.getProcId(); } public long requestTableLock(final TableName tableName, final LockType type, final String description, final NonceKey nonceKey) throws IllegalArgumentException, IOException { - master.getMasterCoprocessorHost().preRequestLock(null, tableName, null, type, description); final LockProcedure proc = new LockProcedure(master.getConfiguration(), tableName, type, description, null); submitProcedure(proc, nonceKey); - master.getMasterCoprocessorHost().postRequestLock(null, tableName, null, type, description); return proc.getProcId(); } @@ -233,13 +229,9 @@ public final class LockManager { public long requestRegionsLock(final RegionInfo[] regionInfos, final String description, final NonceKey nonceKey) throws IllegalArgumentException, IOException { - master.getMasterCoprocessorHost().preRequestLock(null, null, regionInfos, - LockType.EXCLUSIVE, description); final LockProcedure proc = new LockProcedure(master.getConfiguration(), regionInfos, LockType.EXCLUSIVE, description, null); submitProcedure(proc, nonceKey); - master.getMasterCoprocessorHost().postRequestLock(null, null, regionInfos, - LockType.EXCLUSIVE, description); return proc.getProcId(); } @@ -251,16 +243,10 @@ public final class LockManager { final LockProcedure proc = master.getMasterProcedureExecutor() .getProcedure(LockProcedure.class, procId); if (proc == null) return false; - - master.getMasterCoprocessorHost().preLockHeartbeat(proc, keepAlive); - proc.updateHeartBeat(); if (!keepAlive) { proc.unlock(master.getMasterProcedureExecutor().getEnvironment()); } - - master.getMasterCoprocessorHost().postLockHeartbeat(proc, keepAlive); - return proc.isLocked(); } } 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 be027c563b..a0aa73fbd4 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 @@ -100,11 +100,8 @@ 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.master.locking.LockProcedure; -import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; import org.apache.hadoop.hbase.net.Address; import org.apache.hadoop.hbase.procedure2.LockType; -import org.apache.hadoop.hbase.procedure2.Procedure; -import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; import org.apache.hadoop.hbase.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos; import org.apache.hadoop.hbase.protobuf.generated.AccessControlProtos.AccessControlService; @@ -1217,65 +1214,6 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, } @Override - public void preAbortProcedure( - ObserverContext ctx, - final ProcedureExecutor procEnv, - final long procId) throws IOException { - if (!procEnv.isProcedureOwner(procId, getActiveUser(ctx))) { - // If the user is not the procedure owner, then we should further probe whether - // he can abort the procedure. - requirePermission(getActiveUser(ctx), "abortProcedure", Action.ADMIN); - } - } - - @Override - public void postAbortProcedure(ObserverContext ctx) - throws IOException { - // There is nothing to do at this time after the procedure abort request was sent. - } - - @Override - public void preGetProcedures(ObserverContext ctx) - throws IOException { - // We are delegating the authorization check to postGetProcedures as we don't have - // any concrete set of procedures to work with - } - - @Override - public void postGetProcedures( - ObserverContext ctx, - List> procList) throws IOException { - if (procList.isEmpty()) { - return; - } - - // Retains only those which passes authorization checks, as the checks weren't done as part - // of preGetProcedures. - Iterator> itr = procList.iterator(); - User user = getActiveUser(ctx); - while (itr.hasNext()) { - Procedure proc = itr.next(); - try { - String owner = proc.getOwner(); - if (owner == null || !owner.equals(user.getShortName())) { - // If the user is not the procedure owner, then we should further probe whether - // he can see the procedure. - requirePermission(user, "getProcedures", Action.ADMIN); - } - } catch (AccessDeniedException e) { - itr.remove(); - } - } - } - - @Override - public void preGetLocks(ObserverContext ctx) - throws IOException { - User user = getActiveUser(ctx); - requirePermission(user, "getLocks", Action.ADMIN); - } - - @Override public void preMove(ObserverContext c, RegionInfo region, ServerName srcServer, ServerName destServer) throws IOException { requirePermission(getActiveUser(c), "move", region.getTable(), null, null, Action.ADMIN); @@ -2723,24 +2661,6 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, } @Override - public void preAddRSGroup(ObserverContext ctx, - String name) throws IOException { - requirePermission(getActiveUser(ctx), "addRSGroup", Action.ADMIN); - } - - @Override - public void preRemoveRSGroup(ObserverContext ctx, - String name) throws IOException { - requirePermission(getActiveUser(ctx), "removeRSGroup", Action.ADMIN); - } - - @Override - public void preBalanceRSGroup(ObserverContext ctx, - String groupName) throws IOException { - requirePermission(getActiveUser(ctx), "balanceRSGroup", Action.ADMIN); - } - - @Override public void preAddReplicationPeer(final ObserverContext ctx, String peerId, ReplicationPeerConfig peerConfig) throws IOException { requirePermission(getActiveUser(ctx), "addReplicationPeer", Action.ADMIN); @@ -2783,23 +2703,6 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, requirePermission(getActiveUser(ctx), "listReplicationPeers", Action.ADMIN); } - @Override - public void preRequestLock(ObserverContext ctx, String namespace, - TableName tableName, RegionInfo[] regionInfos, LockType type, String description) - throws IOException { - // There are operations in the CREATE and ADMIN domain which may require lock, READ - // or WRITE. So for any lock request, we check for these two perms irrespective of lock type. - String reason = String.format("Lock %s, description=%s", type, description); - checkLockPermissions(getActiveUser(ctx), namespace, tableName, regionInfos, reason); - } - - @Override - public void preLockHeartbeat(ObserverContext ctx, - LockProcedure proc, boolean keepAlive) throws IOException { - String reason = "Heartbeat for lock " + proc.getProcId(); - checkLockPermissions(getActiveUser(ctx), null, proc.getTableName(), null, reason); - } - private void checkLockPermissions(User user, String namespace, TableName tableName, RegionInfo[] regionInfos, String reason) throws IOException { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java index a1614db981..2735da3084 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java @@ -563,68 +563,6 @@ public class TestMasterObserver { } @Override - public void preAbortProcedure( - ObserverContext ctx, - final ProcedureExecutor procEnv, - final long procId) throws IOException { - preAbortProcedureCalled = true; - } - - @Override - public void postAbortProcedure( - ObserverContext ctx) throws IOException { - postAbortProcedureCalled = true; - } - - public boolean wasAbortProcedureCalled() { - return preAbortProcedureCalled && postAbortProcedureCalled; - } - - public boolean wasPreAbortProcedureCalledOnly() { - return preAbortProcedureCalled && !postAbortProcedureCalled; - } - - @Override - public void preGetProcedures( - ObserverContext ctx) throws IOException { - preGetProceduresCalled = true; - } - - @Override - public void postGetProcedures( - ObserverContext ctx, - List> procInfoList) throws IOException { - postGetProceduresCalled = true; - } - - public boolean wasGetProceduresCalled() { - return preGetProceduresCalled && postGetProceduresCalled; - } - - public boolean wasPreGetProceduresCalledOnly() { - return preGetProceduresCalled && !postGetProceduresCalled; - } - - @Override - public void preGetLocks(ObserverContext ctx) throws IOException { - preGetLocksCalled = true; - } - - @Override - public void postGetLocks(ObserverContext ctx, List lockedResources) - throws IOException { - postGetLocksCalled = true; - } - - public boolean wasGetLocksCalled() { - return preGetLocksCalled && postGetLocksCalled; - } - - public boolean wasPreGetLocksCalledOnly() { - return preGetLocksCalled && !postGetLocksCalled; - } - - @Override public void preMove(ObserverContext env, RegionInfo region, ServerName srcServer, ServerName destServer) throws IOException { @@ -1189,67 +1127,6 @@ public class TestMasterObserver { } @Override - public void preAddRSGroup(ObserverContext ctx, - String name) throws IOException { - } - - @Override - public void postAddRSGroup(ObserverContext ctx, - String name) throws IOException { - } - - @Override - public void preRemoveRSGroup(ObserverContext ctx, - String name) throws IOException { - } - - @Override - public void postRemoveRSGroup(ObserverContext ctx, - String name) throws IOException { - } - - @Override - public void preBalanceRSGroup(ObserverContext ctx, - String groupName) throws IOException { - } - - @Override - public void postBalanceRSGroup(ObserverContext ctx, - String groupName, boolean balancerRan) throws IOException { - } - - @Override - public void preRequestLock(ObserverContext ctx, String namespace, - TableName tableName, RegionInfo[] regionInfos, LockType type, - String description) throws IOException { - preRequestLockCalled = true; - } - - @Override - public void postRequestLock(ObserverContext ctx, String namespace, - TableName tableName, RegionInfo[] regionInfos, LockType type, - String description) throws IOException { - postRequestLockCalled = true; - } - - @Override - public void preLockHeartbeat(ObserverContext ctx, - LockProcedure proc, boolean keepAlive) throws IOException { - preLockHeartbeatCalled = true; - } - - @Override - public void postLockHeartbeat(ObserverContext ctx, - LockProcedure proc, boolean keepAlive) throws IOException { - postLockHeartbeatCalled = true; - } - - public boolean preAndPostForQueueLockAndHeartbeatLockCalled() { - return preRequestLockCalled && postRequestLockCalled && preLockHeartbeatCalled && - postLockHeartbeatCalled; - } - - @Override public void preSplitRegion( final ObserverContext c, final TableName tableName, @@ -1783,51 +1660,6 @@ public class TestMasterObserver { cp.wasGetTableNamesCalled()); } - @Test (timeout=180000) - public void testAbortProcedureOperation() throws Exception { - MiniHBaseCluster cluster = UTIL.getHBaseCluster(); - - HMaster master = cluster.getMaster(); - MasterCoprocessorHost host = master.getMasterCoprocessorHost(); - CPMasterObserver cp = host.findCoprocessor(CPMasterObserver.class); - cp.resetStates(); - - master.abortProcedure(1, true); - assertTrue( - "Coprocessor should be called on abort procedure request", - cp.wasAbortProcedureCalled()); - } - - @Test (timeout=180000) - public void testGetProceduresOperation() throws Exception { - MiniHBaseCluster cluster = UTIL.getHBaseCluster(); - - HMaster master = cluster.getMaster(); - MasterCoprocessorHost host = master.getMasterCoprocessorHost(); - CPMasterObserver cp = host.findCoprocessor(CPMasterObserver.class); - cp.resetStates(); - - master.getProcedures(); - assertTrue( - "Coprocessor should be called on get procedures request", - cp.wasGetProceduresCalled()); - } - - @Test (timeout=180000) - public void testGetLocksOperation() throws Exception { - MiniHBaseCluster cluster = UTIL.getHBaseCluster(); - - HMaster master = cluster.getMaster(); - MasterCoprocessorHost host = master.getMasterCoprocessorHost(); - CPMasterObserver cp = host.findCoprocessor(CPMasterObserver.class); - cp.resetStates(); - - master.getLocks(); - assertTrue( - "Coprocessor should be called on get locks request", - cp.wasGetLocksCalled()); - } - private void deleteTable(Admin admin, TableName tableName) throws Exception { // NOTE: We need a latch because admin is not sync, // so the postOp coprocessor method may be called after the admin operation returned. @@ -1836,21 +1668,4 @@ public class TestMasterObserver { tableDeletionLatch.await(); tableDeletionLatch = new CountDownLatch(1); } - - @Test - public void testQueueLockAndLockHeartbeatOperations() throws Exception { - HMaster master = UTIL.getMiniHBaseCluster().getMaster(); - CPMasterObserver cp = master.getMasterCoprocessorHost().findCoprocessor(CPMasterObserver.class); - cp.resetStates(); - - final TableName tableName = TableName.valueOf("testLockedTable"); - long procId = master.getLockManager().remoteLocks().requestTableLock(tableName, - LockType.EXCLUSIVE, "desc", null); - master.getLockManager().remoteLocks().lockHeartbeat(procId, false); - - assertTrue(cp.preAndPostForQueueLockAndHeartbeatLockCalled()); - - ProcedureTestingUtility.waitNoProcedureRunning(master.getMasterProcedureExecutor()); - ProcedureTestingUtility.assertProcNotFailed(master.getMasterProcedureExecutor(), procId); - } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java index 276487abbb..be6d16932a 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestAccessController.java @@ -550,70 +550,6 @@ public class TestAccessController extends SecureTestUtil { } } - @Test - public void testAbortProcedure() throws Exception { - final TableName tableName = TableName.valueOf(name.getMethodName()); - final ProcedureExecutor procExec = - TEST_UTIL.getHBaseCluster().getMaster().getMasterProcedureExecutor(); - Procedure proc = new TestTableDDLProcedure(procExec.getEnvironment(), tableName); - proc.setOwner(USER_OWNER); - final long procId = procExec.submitProcedure(proc); - - AccessTestAction abortProcedureAction = new AccessTestAction() { - @Override - public Object run() throws Exception { - ACCESS_CONTROLLER - .preAbortProcedure(ObserverContextImpl.createAndPrepare(CP_ENV), procExec, procId); - return null; - } - }; - - verifyAllowed(abortProcedureAction, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); - verifyAllowed(abortProcedureAction, USER_OWNER); - verifyDenied( - abortProcedureAction, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE); - } - - @Test - public void testGetProcedures() throws Exception { - final TableName tableName = TableName.valueOf(name.getMethodName()); - final ProcedureExecutor procExec = - TEST_UTIL.getHBaseCluster().getMaster().getMasterProcedureExecutor(); - Procedure proc = new TestTableDDLProcedure(procExec.getEnvironment(), tableName); - proc.setOwner(USER_OWNER); - procExec.submitProcedure(proc); - final List> procList = procExec.getProcedures(); - - AccessTestAction getProceduresAction = new AccessTestAction() { - @Override - public Object run() throws Exception { - ACCESS_CONTROLLER - .postGetProcedures(ObserverContextImpl.createAndPrepare(CP_ENV), procList); - return null; - } - }; - - verifyAllowed(getProceduresAction, SUPERUSER, USER_ADMIN, USER_GROUP_ADMIN); - verifyAllowed(getProceduresAction, USER_OWNER); - verifyIfNull( - getProceduresAction, USER_RW, USER_RO, USER_NONE, USER_GROUP_READ, USER_GROUP_WRITE); - } - - @Test (timeout=180000) - public void testGetLocks() throws Exception { - AccessTestAction action = new AccessTestAction() { - @Override - public Object run() throws Exception { - ACCESS_CONTROLLER.preGetLocks(ObserverContextImpl.createAndPrepare(CP_ENV)); - return null; - } - }; - - 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); - } - @Test (timeout=180000) public void testMove() throws Exception { List regions; @@ -2885,51 +2821,6 @@ public class TestAccessController extends SecureTestUtil { } @Test - public void testAddGroup() throws Exception { - AccessTestAction action1 = new AccessTestAction() { - @Override - public Object run() throws Exception { - ACCESS_CONTROLLER.preAddRSGroup(ObserverContextImpl.createAndPrepare(CP_ENV), - null); - return null; - } - }; - - verifyAllowed(action1, SUPERUSER, USER_ADMIN); - verifyDenied(action1, USER_CREATE, USER_RW, USER_RO, USER_NONE, USER_OWNER); - } - - @Test - public void testRemoveGroup() throws Exception { - AccessTestAction action1 = new AccessTestAction() { - @Override - public Object run() throws Exception { - ACCESS_CONTROLLER.preRemoveRSGroup(ObserverContextImpl.createAndPrepare(CP_ENV), - null); - return null; - } - }; - - verifyAllowed(action1, SUPERUSER, USER_ADMIN); - verifyDenied(action1, USER_CREATE, USER_RW, USER_RO, USER_NONE, USER_OWNER); - } - - @Test - public void testBalanceGroup() throws Exception { - AccessTestAction action1 = new AccessTestAction() { - @Override - public Object run() throws Exception { - ACCESS_CONTROLLER.preBalanceRSGroup(ObserverContextImpl.createAndPrepare(CP_ENV), - null); - return null; - } - }; - - verifyAllowed(action1, SUPERUSER, USER_ADMIN); - verifyDenied(action1, USER_CREATE, USER_RW, USER_RO, USER_NONE, USER_OWNER); - } - - @Test public void testAddReplicationPeer() throws Exception { AccessTestAction action = new AccessTestAction() { @Override @@ -3035,75 +2926,6 @@ public class TestAccessController extends SecureTestUtil { } @Test - public void testRemoteLocks() throws Exception { - String namespace = "preQueueNs"; - final TableName tableName = TableName.valueOf(namespace, name.getMethodName()); - HRegionInfo[] regionInfos = new HRegionInfo[] {new HRegionInfo(tableName)}; - - // Setup Users - // User will be granted ADMIN and CREATE on namespace. Should be denied before grant. - User namespaceUser = User.createUserForTesting(conf, "qLNSUser", new String[0]); - // User will be granted ADMIN and CREATE on table. Should be denied before grant. - User tableACUser = User.createUserForTesting(conf, "qLTableACUser", new String[0]); - // User will be granted READ, WRITE, EXECUTE on table. Should be denied. - User tableRWXUser = User.createUserForTesting(conf, "qLTableRWXUser", new String[0]); - grantOnTable(TEST_UTIL, tableRWXUser.getShortName(), tableName, null, null, - Action.READ, Action.WRITE, Action.EXEC); - // User with global READ, WRITE, EXECUTE should be denied lock access. - User globalRWXUser = User.createUserForTesting(conf, "qLGlobalRWXUser", new String[0]); - grantGlobal(TEST_UTIL, globalRWXUser.getShortName(), Action.READ, Action.WRITE, Action.EXEC); - - AccessTestAction namespaceLockAction = new AccessTestAction() { - @Override public Object run() throws Exception { - ACCESS_CONTROLLER.preRequestLock(ObserverContextImpl.createAndPrepare(CP_ENV), namespace, - null, null, LockType.EXCLUSIVE, null); - return null; - } - }; - verifyAllowed(namespaceLockAction, SUPERUSER, USER_ADMIN); - verifyDenied(namespaceLockAction, globalRWXUser, tableACUser, namespaceUser, tableRWXUser); - grantOnNamespace(TEST_UTIL, namespaceUser.getShortName(), namespace, Action.ADMIN); - verifyAllowed(namespaceLockAction, namespaceUser); - - AccessTestAction tableLockAction = new AccessTestAction() { - @Override public Object run() throws Exception { - ACCESS_CONTROLLER.preRequestLock(ObserverContextImpl.createAndPrepare(CP_ENV), - null, tableName, null, LockType.EXCLUSIVE, null); - return null; - } - }; - verifyAllowed(tableLockAction, SUPERUSER, USER_ADMIN, namespaceUser); - verifyDenied(tableLockAction, globalRWXUser, tableACUser, tableRWXUser); - grantOnTable(TEST_UTIL, tableACUser.getShortName(), tableName, null, null, - Action.ADMIN, Action.CREATE); - verifyAllowed(tableLockAction, tableACUser); - - AccessTestAction regionsLockAction = new AccessTestAction() { - @Override public Object run() throws Exception { - ACCESS_CONTROLLER.preRequestLock(ObserverContextImpl.createAndPrepare(CP_ENV), - null, null, regionInfos, LockType.EXCLUSIVE, null); - return null; - } - }; - verifyAllowed(regionsLockAction, SUPERUSER, USER_ADMIN, namespaceUser, tableACUser); - verifyDenied(regionsLockAction, globalRWXUser, tableRWXUser); - - // Test heartbeats - // Create a lock procedure and try sending heartbeat to it. It doesn't matter how the lock - // was created, we just need namespace from the lock's tablename. - LockProcedure proc = new LockProcedure(conf, tableName, LockType.EXCLUSIVE, "test", null); - AccessTestAction regionLockHeartbeatAction = new AccessTestAction() { - @Override public Object run() throws Exception { - ACCESS_CONTROLLER.preLockHeartbeat(ObserverContextImpl.createAndPrepare(CP_ENV), - proc, false); - return null; - } - }; - verifyAllowed(regionLockHeartbeatAction, SUPERUSER, USER_ADMIN, namespaceUser, tableACUser); - verifyDenied(regionLockHeartbeatAction, globalRWXUser, tableRWXUser); - } - - @Test public void testAccessControlRevokeOnlyFewPermission() throws Throwable { TableName tname = TableName.valueOf("revoke"); try { -- 2.11.0 (Apple Git-81)