Repository: sentry Updated Branches: refs/heads/master f46214908 -> 60f95b3d7
SENTRY-2143: Table renames should synchronize with Sentry (Na Li, reviewed by Sergio Pena, Kalyan Kumar Kalvagadda, Alexander Kolbasov) Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/60f95b3d Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/60f95b3d Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/60f95b3d Branch: refs/heads/master Commit: 60f95b3d74ef524fe17c42e671e70f5461640681 Parents: f462149 Author: lina.li <[email protected]> Authored: Sun Jun 3 20:57:06 2018 -0500 Committer: lina.li <[email protected]> Committed: Sun Jun 3 20:57:06 2018 -0500 ---------------------------------------------------------------------- ...rySyncHMSNotificationsPostEventListener.java | 33 +++++++++- .../TestDbPrivilegeCleanupOnDrop.java | 68 ++++++++++++++++++-- .../tests/e2e/hdfs/TestHDFSIntegrationBase.java | 6 +- 3 files changed, 95 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/60f95b3d/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java ---------------------------------------------------------------------- diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java index 7b2d8be..ccb60ff 100644 --- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java +++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentrySyncHMSNotificationsPostEventListener.java @@ -20,6 +20,7 @@ package org.apache.sentry.binding.metastore; import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hadoop.hive.metastore.api.Table; import org.apache.hadoop.hive.metastore.MetaStoreEventListener; import org.apache.hadoop.hive.metastore.MetaStoreEventListenerConstants; import org.apache.hadoop.hive.metastore.api.MetaException; @@ -46,7 +47,9 @@ import java.util.concurrent.atomic.AtomicLong; * whenever a DDL event happens on the Hive metastore. */ public class SentrySyncHMSNotificationsPostEventListener extends MetaStoreEventListener { - private static final Logger LOGGER = LoggerFactory.getLogger(SentrySyncHMSNotificationsPostEventListener.class); + + private static final Logger LOGGER = LoggerFactory + .getLogger(SentrySyncHMSNotificationsPostEventListener.class); private final HiveAuthzConf authzConf; @@ -78,7 +81,7 @@ public class SentrySyncHMSNotificationsPostEventListener extends MetaStoreEventL throw new RuntimeException(error); } - authzConf = HiveAuthzConf.getAuthzConf((HiveConf)config); + authzConf = HiveAuthzConf.getAuthzConf((HiveConf) config); } @Override @@ -93,7 +96,31 @@ public class SentrySyncHMSNotificationsPostEventListener extends MetaStoreEventL @Override public void onAlterTable(AlterTableEvent tableEvent) throws MetaException { - // no-op + + if (tableEvent == null) { + return; + } + + Table oldTable = tableEvent.getOldTable(); + Table newTable = tableEvent.getNewTable(); + + if (oldTable == null) { + return; + } + + if (newTable == null) { + return; + } + + String oldDbName = oldTable.getDbName(); + String newDbName = newTable.getDbName(); + String oldTableName = oldTable.getTableName(); + String newTableName = newTable.getTableName(); + + if (!newDbName.equalsIgnoreCase(oldDbName) || !newTableName.equalsIgnoreCase(oldTableName)) { + // make sure sentry gets the table rename event + syncNotificationEvents(tableEvent, "onAlterTable"); + } } @Override http://git-wip-us.apache.org/repos/asf/sentry/blob/60f95b3d/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java index cbfdb94..7102539 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbPrivilegeCleanupOnDrop.java @@ -30,6 +30,8 @@ import java.util.List; import org.apache.sentry.tests.e2e.hdfs.TestHDFSIntegrationBase; import org.apache.sentry.tests.e2e.hive.StaticUserGroup; import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Ignore; import org.junit.Test; import com.google.common.collect.Lists; @@ -60,6 +62,16 @@ public class TestDbPrivilegeCleanupOnDrop extends TestHDFSIntegrationBase { private Connection connection; private Statement statement; + @BeforeClass + public static void setup() throws Exception { + // TODO: once HIVE-18783 is fixed, set high interval + // set high interval to test if table rename is synced with fetching notification from HMS + // HMSFOLLOWER_INTERVAL_MILLS = 2000; + WAIT_FOR_NOTIFICATION_PROCESSING = HMSFOLLOWER_INTERVAL_MILLS * 3; + + TestHDFSIntegrationBase.setup(); + } + @Before public void initialize() throws Exception{ super.setUpTempDir(); @@ -113,7 +125,6 @@ public class TestDbPrivilegeCleanupOnDrop extends TestHDFSIntegrationBase { setupDbObjects(statement); // create test DBs and Tables setupPrivileges(statement); // setup privileges for USER1 dropDbObjects(statement); // drop objects - Thread.sleep(WAIT_FOR_NOTIFICATION_PROCESSING); verifyPrivilegesDropped(statement); // verify privileges are removed statement.close(); @@ -152,7 +163,6 @@ public class TestDbPrivilegeCleanupOnDrop extends TestHDFSIntegrationBase { dropDbObjects(statement); // drop DB and tables setupDbObjects(statement); // recreate same DBs and tables - Thread.sleep(WAIT_FOR_NOTIFICATION_PROCESSING); verifyPrivilegesDropped(statement); // verify the stale privileges removed } @@ -171,7 +181,6 @@ public class TestDbPrivilegeCleanupOnDrop extends TestHDFSIntegrationBase { setupRoles(statement); // create required roles setupDbObjects(statement); // create test DBs and Tables setupPrivileges(statement); // setup privileges for USER1 - Thread.sleep(WAIT_FOR_NOTIFICATION_PROCESSING); // verify privileges on the created tables statement.execute("USE " + DB2); @@ -181,9 +190,10 @@ public class TestDbPrivilegeCleanupOnDrop extends TestHDFSIntegrationBase { verifyTablePrivilegeExist(statement, Lists.newArrayList("all_tbl2"), tableName2); - renameTables(statement); // alter tables to rename + // alter tables to rename + renameTables(statement); + // verify privileges removed for old tables - Thread.sleep(WAIT_FOR_NOTIFICATION_PROCESSING); verifyTablePrivilegesDropped(statement); // verify privileges created for new tables @@ -255,12 +265,58 @@ public class TestDbPrivilegeCleanupOnDrop extends TestHDFSIntegrationBase { } /** + * add single privilege, rename table within the same DB and verify privilege moved to + * new table name right away + * + * @throws Exception + */ + @Test + public void testRenameTablesWithinDBSinglePrivilege() throws Exception { + dbNames = new String[]{DB1, DB2}; + roles = new String[]{"admin_role", "read_db1", "all_db1", "select_tbl1", + "insert_tbl1", "all_tbl1", "all_tbl2", "all_prod"}; + + // create required roles + setupRoles(statement); + + // create test DBs and Tables + statement.execute("CREATE DATABASE " + DB1); + statement.execute("CREATE DATABASE " + DB2); + statement.execute("create table " + DB2 + "." + tableName1 + + " (under_col int comment 'the under column', value string)"); + + // setup privileges for USER1 + statement.execute("GRANT ALL ON DATABASE " + DB1 + " TO ROLE all_db1"); + statement.execute("GRANT SELECT ON DATABASE " + DB1 + + " TO ROLE read_db1"); + statement.execute("GRANT ALL ON DATABASE " + DB2 + " TO ROLE all_prod"); + statement.execute("USE " + DB2); + + // grant priviledges + statement.execute("GRANT SELECT ON TABLE " + tableName1 + + " TO ROLE select_tbl1"); + + // rename table + statement.execute("ALTER TABLE " + tableName1 + " RENAME TO " + + tableName1 + renameTag); + + // verify privileges created for new table + verifyTablePrivilegeExist(statement, + Lists.newArrayList("select_tbl1"), + tableName1 + renameTag); + + statement.close(); + connection.close(); + } + + /** * After we drop/rename table, we will drop/rename all privileges(ALL,SELECT,INSERT,ALTER,DROP...) * from this role * * @throws Exception */ @Test + @Ignore("once HIVE-18783 is fixed in hive version, enable this test") public void testDropAndRenameWithMultiAction() throws Exception { dbNames = new String[]{DB1, DB2}; roles = new String[]{"admin_role", "user_role"}; @@ -286,11 +342,9 @@ public class TestDbPrivilegeCleanupOnDrop extends TestHDFSIntegrationBase { statement = connection.createStatement(); statement.execute("USE " + DB1); statement.execute("ALTER TABLE t1 RENAME TO t2"); - Thread.sleep(WAIT_FOR_NOTIFICATION_PROCESSING); // After rename table t1 to t2, user_role should have permission to drop t2 statement.execute("drop table t2"); - Thread.sleep(WAIT_FOR_NOTIFICATION_PROCESSING); ResultSet resultSet = statement.executeQuery("SHOW GRANT ROLE user_role"); // user_role will revoke all privilege from table t2, only remain DROP/CREATE on db_1 assertRemainingRows(resultSet, 2); http://git-wip-us.apache.org/repos/asf/sentry/blob/60f95b3d/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java index 04c679f..100219b 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegrationBase.java @@ -172,8 +172,8 @@ public abstract class TestHDFSIntegrationBase { ServerConfig.SENTRY_HMSFOLLOWER_INIT_DELAY_MILLS_DEFAULT + ServerConfig.SENTRY_HMSFOLLOWER_INTERVAL_MILLS_DEFAULT * 2 + CACHE_REFRESH * 2; - protected static final long HMSFOLLOWER_INTERVAL_MILLS = 50; - protected static final long WAIT_FOR_NOTIFICATION_PROCESSING = HMSFOLLOWER_INTERVAL_MILLS * 3; + protected static long HMSFOLLOWER_INTERVAL_MILLS = 50; + protected static long WAIT_FOR_NOTIFICATION_PROCESSING = HMSFOLLOWER_INTERVAL_MILLS * 3; // Time to wait before running next tests. The unit is milliseconds. // Deleting HDFS may finish, but HDFS may not be ready for creating the same file again. @@ -624,6 +624,8 @@ public abstract class TestHDFSIntegrationBase { hiveConf.set("hive.security.authorization.task.factory", "org.apache.sentry.binding.hive.SentryHiveAuthorizationTaskFactoryImpl"); hiveConf.set("hive.server2.session.hook", "org.apache.sentry.binding.hive.HiveAuthzBindingSessionHook"); hiveConf.set("sentry.metastore.service.users", "hive");// queries made by hive user (beeline) skip meta store check + // make sure metastore calls sentry post event listener + hiveConf.set("hive.metastore.event.listeners", "org.apache.sentry.binding.metastore.SentrySyncHMSNotificationsPostEventListener"); HiveAuthzConf authzConf = new HiveAuthzConf(Resources.getResource("sentry-site.xml")); authzConf.addResource(hiveConf);
