Repository: sentry Updated Branches: refs/heads/master 4e2009104 -> d817d4d30
SENTRY-2168: Altering table will not update sentry permissions when HDFS sync is disabled (Kalyan Kumar kalvagadda, reviewed-by Na Li) Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/d817d4d3 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/d817d4d3 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/d817d4d3 Branch: refs/heads/master Commit: d817d4d301dcacb21d2d135e5ffa6ed9aaddbf47 Parents: 4e20091 Author: Kalyan Kumar Kalvagadda <[email protected]> Authored: Fri Mar 30 08:44:15 2018 -0500 Committer: Kalyan Kumar Kalvagadda <[email protected]> Committed: Fri Mar 30 08:45:47 2018 -0500 ---------------------------------------------------------------------- .../persistent/NotificationProcessor.java | 8 +-- .../db/service/persistent/TestHMSFollower.java | 4 +- .../TestHmsNotificationProcessingBase.java | 3 +- ...msNotificationProcessingWithOutHdfsSync.java | 55 +++++++++++++++++++- 4 files changed, 62 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/d817d4d3/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java index e5ad3b5..90bc1cc 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/NotificationProcessor.java @@ -366,10 +366,6 @@ final class NotificationProcessor { */ private boolean processAlterTable(NotificationEvent event) throws Exception { - if (!hdfsSyncEnabled) { - return false; - } - SentryJSONAlterTableMessage alterTableMessage = deserializer.getAlterTableMessage(event.getMessage()); String oldDbName = alterTableMessage.getDB(); @@ -419,6 +415,10 @@ final class NotificationProcessor { return false; } } + + if (!hdfsSyncEnabled) { + return false; + } String oldAuthzObj = oldDbName + "." + oldTableName; String newAuthzObj = newDbName + "." + newTableName; renameAuthzPath(oldAuthzObj, newAuthzObj, oldLocation, newLocation, event); http://git-wip-us.apache.org/repos/asf/sentry/blob/d817d4d3/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java index 61e3f06..4b38635 100644 --- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java +++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/persistent/TestHMSFollower.java @@ -1255,7 +1255,7 @@ public class TestHMSFollower { } @Test - public void testNoHdfsSyncAlterTableNotPersisted() throws Exception { + public void testNoHdfsSyncAlterTableIsPersisted() throws Exception { String dbName = "db1"; String tableName = "table1"; String newDbName = "db1"; @@ -1290,7 +1290,7 @@ public class TestHMSFollower { newAuthorizable.setDb(newDbName); newAuthorizable.setTable(newTableName); - verify(sentryStore, times(0)).renamePrivilege(authorizable, newAuthorizable, + verify(sentryStore, times(1)).renamePrivilege(authorizable, newAuthorizable, NotificationProcessor.getPermUpdatableOnRename(authorizable, newAuthorizable)); } } http://git-wip-us.apache.org/repos/asf/sentry/blob/d817d4d3/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java index ed80e88..ac822d4 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingBase.java @@ -33,7 +33,8 @@ public class TestHmsNotificationProcessingBase extends TestHDFSIntegrationBase{ protected final static int SHOW_GRANT_DB_POSITION = 1; protected static final String DB1 = "db_1", DB2 = "db_2", - tableName1 = "tb_1"; + tableName1 = "tb_1", + tableName2 = "tb_2"; // verify all the test privileges are dropped as we drop the objects protected void verifyPrivilegesDropped(Statement statement) http://git-wip-us.apache.org/repos/asf/sentry/blob/d817d4d3/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutHdfsSync.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutHdfsSync.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutHdfsSync.java index 9634ea1..9535dee 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutHdfsSync.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestHmsNotificationProcessingWithOutHdfsSync.java @@ -98,4 +98,57 @@ public class TestHmsNotificationProcessingWithOutHdfsSync extends TestHmsNotific //Make sure that the privileges added for that object are removed. verifyPrivilegesCount(statement, 0); } - } + + @Test + public void testHmsNotificationProcessingOnAlter() throws Throwable { + dbNames = new String[]{DB1}; + roles = new String[]{"admin_role", "read_db1", "select_tbl1"}; + admin = "hive"; + + Connection connection = hiveServer2.createConnection(admin, admin); + Statement statement = connection.createStatement(); + statement.execute("create role admin_role"); + statement.execute("grant role admin_role to group hive"); + statement.execute("grant all on server server1 to role admin_role"); + + // Add privileges for an objects that do not exist yet + statement.execute("create role read_db1"); + statement.execute("create role select_tbl1"); + statement.execute("grant role read_db1 to group hbase"); + statement.execute("grant role select_tbl1 to group hbase"); + + //add "select" sentry permission for the object's + statement.execute("grant select on database " + DB1 + " to role read_db1"); + String str = "grant select on table " + DB1 + "." + tableName1 + + " TO ROLE select_tbl1"; + statement.execute(str); + + //Add object + statement.execute("CREATE DATABASE " + DB1); + statement.execute("use " + DB1); + statement.execute("create table " + DB1 + "." + tableName1 + + " (under_col int comment 'the under column', value string)"); + + Thread.sleep(WAIT_FOR_NOTIFICATION_PROCESSING); + //Make sure that the privileges for that object are removed. + verifyPrivilegesCount(statement, 0); + + //add "select" sentry permission for the object's + statement.execute("GRANT select ON DATABASE " + DB1 + " TO ROLE read_db1"); + statement.execute("USE " + DB1); + statement.execute("GRANT SELECT ON TABLE " + tableName1 + + " TO ROLE select_tbl1"); + + // Make sure that an ACL is added for that + verifyOnAllSubDirs("/user/hive/warehouse/db_1.db", FsAction.READ_EXECUTE, "hbase", false); + verifyOnAllSubDirs("/user/hive/warehouse/db_1.db/tb_1", FsAction.READ_EXECUTE, "hbase", false); + + //alter the object + String temp = "alter table " + DB1 + "." + tableName1 + " rename to " + DB1 + "." + tableName2; + statement.execute(temp); + + Thread.sleep(WAIT_FOR_NOTIFICATION_PROCESSING); + // Make sure that an ACL is updated got the new table name + verifyOnAllSubDirs("/user/hive/warehouse/db_1.db/" + tableName2, FsAction.READ_EXECUTE, "hbase", false); + } +}
