From 05f57f4c03c68e3184d6fcfaaa4353a0981125fa Mon Sep 17 00:00:00 2001 From: Apekshit Sharma Date: Thu, 10 May 2018 20:34:14 -0700 Subject: [PATCH] HBASE-20652 Remove internal uses of some deprecated MasterObserver hooks Remove internal uses of these hooks: preModifyNamespace postModifyNamespace preModifyTable postModifyTable preModifyTableAction postCompletedModifyTableAction Signed-off-by: tedyu --- .../hbase/rsgroup/RSGroupAdminEndpoint.java | 4 ++-- .../hbase/coprocessor/MasterObserver.java | 1 - .../security/access/AccessController.java | 22 +++++++++---------- .../CoprocessorWhitelistMasterObserver.java | 5 +++-- .../visibility/VisibilityController.java | 4 ++-- .../hbase/coprocessor/TestMasterObserver.java | 16 +++++++++----- .../TestMasterObserverPostCalls.java | 6 +++-- .../security/access/TestAccessController.java | 4 +++- .../access/TestNamespaceCommands.java | 1 + .../access/TestWithDisabledAuthorization.java | 5 ++++- 10 files changed, 39 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 759f3e0b683..8546a4067ad 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 @@ -490,8 +490,8 @@ public class RSGroupAdminEndpoint implements MasterCoprocessor, MasterObserver { @Override public void preModifyNamespace(ObserverContext ctx, - NamespaceDescriptor ns) throws IOException { - preCreateNamespace(ctx, ns); + NamespaceDescriptor currentNsDesc, NamespaceDescriptor newNsDesc) throws IOException { + preCreateNamespace(ctx, newNsDesc); } @Override 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 a37f21a8569..f60a04d02a0 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 @@ -229,7 +229,6 @@ public interface MasterObserver { throws IOException { preModifyTable(ctx, tableName, newDescriptor); } - /** * Called after the modifyTable operation has been requested. Called as part * of modify table RPC call. 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 8182b4282db..7e824e20b30 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 @@ -969,24 +969,24 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, @Override public void preModifyTable(ObserverContext c, TableName tableName, - TableDescriptor htd) throws IOException { + TableDescriptor currentDesc, TableDescriptor newDesc) throws IOException { // TODO: potentially check if this is a add/modify/delete column operation requirePermission(c, "modifyTable", tableName, null, null, Action.ADMIN, Action.CREATE); } @Override - public void postModifyTable(ObserverContext c, - TableName tableName, final TableDescriptor htd) throws IOException { + public void postModifyTable(ObserverContext c, TableName tableName, + TableDescriptor oldDesc, TableDescriptor currentDesc) throws IOException { final Configuration conf = c.getEnvironment().getConfiguration(); // default the table owner to current user, if not specified. - final String owner = (htd.getOwnerString() != null) ? htd.getOwnerString() : + final String owner = (currentDesc.getOwnerString() != null) ? currentDesc.getOwnerString() : getActiveUser(c).getShortName(); User.runAsLoginUser(new PrivilegedExceptionAction() { @Override public Void run() throws Exception { UserPermission userperm = new UserPermission(Bytes.toBytes(owner), - htd.getTableName(), null, Action.values()); + currentDesc.getTableName(), null, Action.values()); try (Table table = c.getEnvironment().getConnection(). getTable(AccessControlLists.ACL_TABLE_NAME)) { AccessControlLists.addUserPermission(conf, userperm, table); @@ -1233,18 +1233,16 @@ public class AccessController implements MasterCoprocessor, RegionCoprocessor, @Override public void preModifyNamespace(ObserverContext ctx, - NamespaceDescriptor ns) throws IOException { + NamespaceDescriptor currentNsDesc, NamespaceDescriptor newNsDesc) throws IOException { // We require only global permission so that // a user with NS admin cannot altering namespace configurations. i.e. namespace quota - requireGlobalPermission(ctx, "modifyNamespace", - Action.ADMIN, ns.getName()); + requireGlobalPermission(ctx, "modifyNamespace", Action.ADMIN, newNsDesc.getName()); } @Override - public void preGetNamespaceDescriptor(ObserverContext ctx, String namespace) - throws IOException { - requireNamespacePermission(ctx, "getNamespaceDescriptor", - namespace, Action.ADMIN); + public void preGetNamespaceDescriptor(ObserverContext ctx, + String namespace) throws IOException { + requireNamespacePermission(ctx, "getNamespaceDescriptor", namespace, Action.ADMIN); } @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/CoprocessorWhitelistMasterObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/CoprocessorWhitelistMasterObserver.java index 44f736b2e97..719fe33bc43 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/CoprocessorWhitelistMasterObserver.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/CoprocessorWhitelistMasterObserver.java @@ -55,8 +55,9 @@ public class CoprocessorWhitelistMasterObserver implements MasterCoprocessor, Ma @Override public void preModifyTable(ObserverContext ctx, - TableName tableName, TableDescriptor htd) throws IOException { - verifyCoprocessors(ctx, htd); + TableName tableName, TableDescriptor currentDesc, TableDescriptor newDesc) + throws IOException { + verifyCoprocessors(ctx, newDesc); } @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java index 6e00f40195f..4f00e7daa36 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityController.java @@ -226,8 +226,8 @@ public class VisibilityController implements MasterCoprocessor, RegionCoprocesso } @Override - public void preModifyTable(ObserverContext ctx, - TableName tableName, TableDescriptor htd) throws IOException { + public void preModifyTable(ObserverContext ctx, TableName tableName, + TableDescriptor currentDescriptor, TableDescriptor newDescriptor) throws IOException { if (!authorizationEnabled) { return; } 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 579b6d34e33..a606e27c403 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 @@ -364,13 +364,15 @@ public class TestMasterObserver { @Override public void preModifyTable(ObserverContext env, - TableName tableName, TableDescriptor htd) throws IOException { + TableName tableName, final TableDescriptor currentDescriptor, + final TableDescriptor newDescriptor) throws IOException { preModifyTableCalled = true; } @Override public void postModifyTable(ObserverContext env, - TableName tableName, TableDescriptor htd) throws IOException { + TableName tableName, final TableDescriptor oldDescriptor, + final TableDescriptor currentDescriptor) throws IOException { postModifyTableCalled = true; } @@ -424,13 +426,13 @@ public class TestMasterObserver { @Override public void preModifyNamespace(ObserverContext env, - NamespaceDescriptor ns) throws IOException { + NamespaceDescriptor currentNsDesc, NamespaceDescriptor newNsDesc) throws IOException { preModifyNamespaceCalled = true; } @Override public void postModifyNamespace(ObserverContext env, - NamespaceDescriptor ns) throws IOException { + NamespaceDescriptor oldNsDesc, NamespaceDescriptor currentNsDesc) throws IOException { postModifyNamespaceCalled = true; } @@ -917,7 +919,8 @@ public class TestMasterObserver { public void preModifyTableAction( final ObserverContext env, final TableName tableName, - final TableDescriptor htd) throws IOException { + final TableDescriptor currentDescriptor, + final TableDescriptor newDescriptor) throws IOException { preModifyTableActionCalled = true; } @@ -925,7 +928,8 @@ public class TestMasterObserver { public void postCompletedModifyTableAction( final ObserverContext env, final TableName tableName, - final TableDescriptor htd) throws IOException { + final TableDescriptor oldDescriptor, + final TableDescriptor currentDescriptor) throws IOException { postCompletedModifyTableActionCalled = true; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterObserverPostCalls.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterObserverPostCalls.java index e6357103a86..089a2a468a2 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterObserverPostCalls.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestMasterObserverPostCalls.java @@ -106,7 +106,8 @@ public class TestMasterObserverPostCalls { @Override public void postModifyNamespace( - ObserverContext ctx, NamespaceDescriptor desc) { + ObserverContext ctx, NamespaceDescriptor oldNsDesc, + NamespaceDescriptor currentNsDesc) { postHookCalls.incrementAndGet(); } @@ -125,7 +126,8 @@ public class TestMasterObserverPostCalls { @Override public void postModifyTable( - ObserverContext ctx, TableName tn, TableDescriptor td) { + ObserverContext ctx, TableName tn, + TableDescriptor oldDescriptor, TableDescriptor currentDescriptor) { postHookCalls.incrementAndGet(); } 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 2e9be309110..870fa19b37e 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 @@ -406,7 +406,9 @@ public class TestAccessController extends SecureTestUtil { htd.addFamily(new HColumnDescriptor(TEST_FAMILY)); htd.addFamily(new HColumnDescriptor("fam_" + User.getCurrent().getShortName())); ACCESS_CONTROLLER.preModifyTable(ObserverContextImpl.createAndPrepare(CP_ENV), - TEST_TABLE, htd); + TEST_TABLE, + null, // not needed by AccessController + htd); return null; } }; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java index 328024c58fa..66e37bcbce2 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestNamespaceCommands.java @@ -250,6 +250,7 @@ public class TestNamespaceCommands extends SecureTestUtil { @Override public Object run() throws Exception { ACCESS_CONTROLLER.preModifyNamespace(ObserverContextImpl.createAndPrepare(CP_ENV), + null, // not needed by AccessController NamespaceDescriptor.create(TEST_NAMESPACE).addConfiguration("abc", "156").build()); return null; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java index 57d9e4bf860..110afcd1375 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestWithDisabledAuthorization.java @@ -493,7 +493,9 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { htd.addFamily(new HColumnDescriptor(TEST_FAMILY)); htd.addFamily(new HColumnDescriptor(TEST_FAMILY2)); ACCESS_CONTROLLER.preModifyTable(ObserverContextImpl.createAndPrepare(CP_ENV), - TEST_TABLE.getTableName(), htd); + TEST_TABLE.getTableName(), + null, // not needed by AccessController + htd); return null; } }, SUPERUSER, USER_ADMIN, USER_RW, USER_RO, USER_OWNER, USER_CREATE, USER_QUAL, USER_NONE); @@ -700,6 +702,7 @@ public class TestWithDisabledAuthorization extends SecureTestUtil { public Object run() throws Exception { NamespaceDescriptor ns = NamespaceDescriptor.create("test").build(); ACCESS_CONTROLLER.preModifyNamespace(ObserverContextImpl.createAndPrepare(CP_ENV), + null, // not needed by AccessController ns); return null; }