You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ap...@apache.org on 2018/03/14 13:37:33 UTC
hbase git commit: HBASE-20185 Fix ACL check for
MasterRpcServices#execProcedure
Repository: hbase
Updated Branches:
refs/heads/master 565085383 -> 84ee32c72
HBASE-20185 Fix ACL check for MasterRpcServices#execProcedure
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/84ee32c7
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/84ee32c7
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/84ee32c7
Branch: refs/heads/master
Commit: 84ee32c72341c2ebbaa77fe26bcb1ed57f3d8352
Parents: 5650853
Author: Apekshit Sharma <ap...@apache.org>
Authored: Tue Mar 13 17:13:56 2018 +0530
Committer: Apekshit Sharma <ap...@apache.org>
Committed: Wed Mar 14 19:02:19 2018 +0530
----------------------------------------------------------------------
.../master/LogRollMasterProcedureManager.java | 10 ++++++-
.../hadoop/hbase/master/MasterRpcServices.java | 4 ++-
.../hbase/master/snapshot/SnapshotManager.java | 8 +++++
.../hbase/procedure/MasterProcedureManager.java | 31 ++++++++++++--------
.../flush/MasterFlushTableProcedureManager.java | 10 +++++++
.../hbase/regionserver/RSRpcServices.java | 2 +-
.../hbase/security/access/AccessController.java | 3 ++
.../procedure/SimpleMasterProcedureManager.java | 6 ++++
8 files changed, 58 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/84ee32c7/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/master/LogRollMasterProcedureManager.java
----------------------------------------------------------------------
diff --git a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/master/LogRollMasterProcedureManager.java b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/master/LogRollMasterProcedureManager.java
index 486b991..d53b284 100644
--- a/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/master/LogRollMasterProcedureManager.java
+++ b/hbase-backup/src/main/java/org/apache/hadoop/hbase/backup/master/LogRollMasterProcedureManager.java
@@ -38,6 +38,8 @@ import org.apache.hadoop.hbase.procedure.Procedure;
import org.apache.hadoop.hbase.procedure.ProcedureCoordinator;
import org.apache.hadoop.hbase.procedure.ProcedureCoordinatorRpcs;
import org.apache.hadoop.hbase.procedure.RegionServerProcedureManager;
+import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.security.access.AccessChecker;
import org.apache.yetus.audience.InterfaceAudience;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -47,7 +49,7 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.ProcedureDe
/**
* Master procedure manager for coordinated cluster-wide WAL roll operation, which is run during
- * backup operation, see {@link MasterProcedureManager} and and {@link RegionServerProcedureManager}
+ * backup operation, see {@link MasterProcedureManager} and {@link RegionServerProcedureManager}
*/
@InterfaceAudience.Private
public class LogRollMasterProcedureManager extends MasterProcedureManager {
@@ -157,6 +159,12 @@ public class LogRollMasterProcedureManager extends MasterProcedureManager {
monitor.rethrowException();
}
+ @Override
+ public void checkPermissions(ProcedureDescription desc, AccessChecker accessChecker, User user)
+ throws IOException {
+ // TODO: what permissions checks are needed here?
+ }
+
private boolean isBackupEnabled() {
return BackupManager.isBackupEnabled(master.getConfiguration());
}
http://git-wip-us.apache.org/repos/asf/hbase/blob/84ee32c7/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
index def4640..b63b448 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
@@ -57,6 +57,7 @@ import org.apache.hadoop.hbase.io.hfile.HFile;
import org.apache.hadoop.hbase.ipc.CoprocessorRpcUtils;
import org.apache.hadoop.hbase.ipc.PriorityFunction;
import org.apache.hadoop.hbase.ipc.QosPriority;
+import org.apache.hadoop.hbase.ipc.RpcServer;
import org.apache.hadoop.hbase.ipc.RpcServer.BlockingServiceAndInterface;
import org.apache.hadoop.hbase.ipc.RpcServerFactory;
import org.apache.hadoop.hbase.ipc.RpcServerInterface;
@@ -833,8 +834,8 @@ public class MasterRpcServices extends RSRpcServices
@Override
public ExecProcedureResponse execProcedure(RpcController controller,
ExecProcedureRequest request) throws ServiceException {
- rpcPreCheck("execProcedure");
try {
+ master.checkInitialized();
ProcedureDescription desc = request.getProcedure();
MasterProcedureManager mpm = master.getMasterProcedureManagerHost().getProcedureManager(
desc.getSignature());
@@ -843,6 +844,7 @@ public class MasterRpcServices extends RSRpcServices
+ desc.getSignature()));
}
LOG.info(master.getClientIdAuditPrefix() + " procedure request for: " + desc.getSignature());
+ mpm.checkPermissions(desc, accessChecker, RpcServer.getRequestUser().orElse(null));
mpm.execProcedure(desc);
// send back the max amount of time the client should wait for the procedure
// to complete
http://git-wip-us.apache.org/repos/asf/hbase/blob/84ee32c7/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java
index 3870601..1abd605 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/snapshot/SnapshotManager.java
@@ -63,6 +63,7 @@ import org.apache.hadoop.hbase.procedure.ZKProcedureCoordinator;
import org.apache.hadoop.hbase.procedure2.ProcedureExecutor;
import org.apache.hadoop.hbase.security.AccessDeniedException;
import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.security.access.AccessChecker;
import org.apache.hadoop.hbase.snapshot.ClientSnapshotDescriptionUtils;
import org.apache.hadoop.hbase.snapshot.HBaseSnapshotException;
import org.apache.hadoop.hbase.snapshot.RestoreSnapshotException;
@@ -1150,6 +1151,13 @@ public class SnapshotManager extends MasterProcedureManager implements Stoppable
}
@Override
+ public void checkPermissions(ProcedureDescription desc, AccessChecker accessChecker, User user)
+ throws IOException {
+ // Done by AccessController as part of preSnapshot coprocessor hook (legacy code path).
+ // In future, when we AC is removed for good, that check should be moved here.
+ }
+
+ @Override
public boolean isProcedureDone(ProcedureDescription desc) throws IOException {
return isSnapshotDone(toSnapshotDescription(desc));
}
http://git-wip-us.apache.org/repos/asf/hbase/blob/84ee32c7/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/MasterProcedureManager.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/MasterProcedureManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/MasterProcedureManager.java
index 89e55d1..b705157 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/MasterProcedureManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/MasterProcedureManager.java
@@ -19,6 +19,8 @@ package org.apache.hadoop.hbase.procedure;
import java.io.IOException;
+import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.security.access.AccessChecker;
import org.apache.yetus.audience.InterfaceAudience;
import org.apache.yetus.audience.InterfaceStability;
import org.apache.hadoop.hbase.Stoppable;
@@ -34,28 +36,26 @@ import org.apache.zookeeper.KeeperException;
*
* To implement a custom globally barriered procedure, user needs to extend two classes:
* {@link MasterProcedureManager} and {@link RegionServerProcedureManager}. Implementation of
-* {@link MasterProcedureManager} is loaded into {@link org.apache.hadoop.hbase.master.HMaster}
+* {@link MasterProcedureManager} is loaded into {@link org.apache.hadoop.hbase.master.HMaster}
* process via configuration parameter 'hbase.procedure.master.classes', while implementation of
-* {@link RegionServerProcedureManager} is loaded into
+* {@link RegionServerProcedureManager} is loaded into
* {@link org.apache.hadoop.hbase.regionserver.HRegionServer} process via
* configuration parameter 'hbase.procedure.regionserver.classes'.
*
-* An example of globally barriered procedure implementation is
+* An example of globally barriered procedure implementation is
* {@link org.apache.hadoop.hbase.master.snapshot.SnapshotManager} and
* {@link org.apache.hadoop.hbase.regionserver.snapshot.RegionServerSnapshotManager}.
*
* A globally barriered procedure is identified by its signature (usually it is the name of the
* procedure znode). During the initialization phase, the initialize methods are called by both
-* {@link org.apache.hadoop.hbase.master.HMaster}
-* and {@link org.apache.hadoop.hbase.regionserver.HRegionServer} which create the procedure znode
-* and register the listeners. A procedure can be triggered by its signature and an instant name
-* (encapsulated in a {@link ProcedureDescription} object). When the servers are shutdown,
+* {@link org.apache.hadoop.hbase.master.HMaster}
+* and {@link org.apache.hadoop.hbase.regionserver.HRegionServer} which create the procedure znode
+* and register the listeners. A procedure can be triggered by its signature and an instant name
+* (encapsulated in a {@link ProcedureDescription} object). When the servers are shutdown,
* the stop methods on both classes are called to clean up the data associated with the procedure.
*/
@InterfaceAudience.Private
-@InterfaceStability.Evolving
-public abstract class MasterProcedureManager extends ProcedureManager implements
- Stoppable {
+public abstract class MasterProcedureManager extends ProcedureManager implements Stoppable {
/**
* Initialize a globally barriered procedure for master.
*
@@ -73,9 +73,7 @@ public abstract class MasterProcedureManager extends ProcedureManager implements
* @param desc Procedure description
* @throws IOException
*/
- public void execProcedure(ProcedureDescription desc) throws IOException {
-
- }
+ public void execProcedure(ProcedureDescription desc) throws IOException {}
/**
* Execute a distributed procedure on cluster with return data.
@@ -90,6 +88,13 @@ public abstract class MasterProcedureManager extends ProcedureManager implements
}
/**
+ * Check for required permissions before executing the procedure.
+ * @throws IOException if permissions requirements are not met.
+ */
+ public abstract void checkPermissions(ProcedureDescription desc, AccessChecker accessChecker,
+ User user) throws IOException;
+
+ /**
* Check if the procedure is finished successfully
*
* @param desc Procedure description
http://git-wip-us.apache.org/repos/asf/hbase/blob/84ee32c7/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/flush/MasterFlushTableProcedureManager.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/flush/MasterFlushTableProcedureManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/flush/MasterFlushTableProcedureManager.java
index bced854..fee3dde 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/flush/MasterFlushTableProcedureManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/procedure/flush/MasterFlushTableProcedureManager.java
@@ -41,6 +41,9 @@ import org.apache.hadoop.hbase.procedure.Procedure;
import org.apache.hadoop.hbase.procedure.ProcedureCoordinator;
import org.apache.hadoop.hbase.procedure.ProcedureCoordinatorRpcs;
import org.apache.hadoop.hbase.procedure.ZKProcedureCoordinator;
+import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.security.access.AccessChecker;
+import org.apache.hadoop.hbase.security.access.Permission;
import org.apache.hadoop.hbase.util.Pair;
import org.apache.hadoop.hbase.zookeeper.MetaTableLocator;
import org.apache.yetus.audience.InterfaceAudience;
@@ -182,6 +185,13 @@ public class MasterFlushTableProcedureManager extends MasterProcedureManager {
}
@Override
+ public void checkPermissions(ProcedureDescription desc, AccessChecker accessChecker, User user)
+ throws IOException {
+ // Done by AccessController as part of preTableFlush coprocessor hook (legacy code path).
+ // In future, when we AC is removed for good, that check should be moved here.
+ }
+
+ @Override
public synchronized boolean isProcedureDone(ProcedureDescription desc) throws IOException {
// Procedure instance name is the table name.
TableName tableName = TableName.valueOf(desc.getInstance());
http://git-wip-us.apache.org/repos/asf/hbase/blob/84ee32c7/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
----------------------------------------------------------------------
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 c4fda68..3efea4e 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
@@ -331,7 +331,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
// Initialized in start() since AccessChecker needs ZKWatcher which is created by HRegionServer
// after RSRpcServices constructor and before start() is called.
// Initialized only if authorization is enabled, else remains null.
- private AccessChecker accessChecker;
+ protected AccessChecker accessChecker;
/**
* Services launched in RSRpcServices. By default they are on but you can use the below
http://git-wip-us.apache.org/repos/asf/hbase/blob/84ee32c7/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
----------------------------------------------------------------------
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 5fef8ab..171469a 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
@@ -1136,6 +1136,7 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor,
public void preSnapshot(final ObserverContext<MasterCoprocessorEnvironment> ctx,
final SnapshotDescription snapshot, final TableDescriptor hTableDescriptor)
throws IOException {
+ // Move this ACL check to SnapshotManager#checkPermissions as part of AC deprecation.
requirePermission(ctx, "snapshot " + snapshot.getName(),
hTableDescriptor.getTableName(), null, null, Permission.Action.ADMIN);
}
@@ -1266,6 +1267,8 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor,
@Override
public void preTableFlush(final ObserverContext<MasterCoprocessorEnvironment> ctx,
final TableName tableName) throws IOException {
+ // Move this ACL check to MasterFlushTableProcedureManager#checkPermissions as part of AC
+ // deprecation.
requirePermission(ctx, "flushTable", tableName,
null, null, Action.ADMIN, Action.CREATE);
}
http://git-wip-us.apache.org/repos/asf/hbase/blob/84ee32c7/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/SimpleMasterProcedureManager.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/SimpleMasterProcedureManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/SimpleMasterProcedureManager.java
index 579fcd3..e1d46ef 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/SimpleMasterProcedureManager.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/procedure/SimpleMasterProcedureManager.java
@@ -29,6 +29,8 @@ import org.apache.hadoop.hbase.errorhandling.ForeignExceptionDispatcher;
import org.apache.hadoop.hbase.master.MasterServices;
import org.apache.hadoop.hbase.master.MetricsMaster;
import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos.ProcedureDescription;
+import org.apache.hadoop.hbase.security.User;
+import org.apache.hadoop.hbase.security.access.AccessChecker;
import org.apache.zookeeper.KeeperException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -114,6 +116,10 @@ public class SimpleMasterProcedureManager extends MasterProcedureManager {
}
@Override
+ public void checkPermissions(ProcedureDescription desc, AccessChecker accessChecker, User user)
+ throws IOException {}
+
+ @Override
public boolean isProcedureDone(ProcedureDescription desc) throws IOException {
return done;
}