SENTRY-885: DB name should be case insensitive in HDFS sync plugin ( Sravya Tirukkovalur, Reviewed by: Lenni Kuff)
Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/e8723d02 Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/e8723d02 Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/e8723d02 Branch: refs/heads/hive_plugin_v2 Commit: e8723d02e79828ae884afbf851f482de267fb1b0 Parents: 2bbeaa3 Author: Sravya Tirukkovalur <[email protected]> Authored: Fri Sep 11 13:59:13 2015 -0700 Committer: Sun Dapeng <[email protected]> Committed: Mon Nov 2 16:36:26 2015 +0800 ---------------------------------------------------------------------- .../sentry/hdfs/MetastoreCacheInitializer.java | 20 +++++++++------- .../org/apache/sentry/hdfs/MetastorePlugin.java | 22 +++++++++++------- .../org/apache/sentry/hdfs/SentryPlugin.java | 4 ++-- .../tests/e2e/hdfs/TestHDFSIntegration.java | 24 ++++++++++++++++++++ 4 files changed, 52 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/e8723d02/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java index 093d21a..f1e28e9 100644 --- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java +++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastoreCacheInitializer.java @@ -17,6 +17,7 @@ */ package org.apache.sentry.hdfs; +import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hive.metastore.IHMSHandler; @@ -127,23 +128,24 @@ class MetastoreCacheInitializer implements Closeable { } for (Table tbl : tables) { TPathChanges tblPathChange; + // Table names are case insensitive + String tableName = tbl.getTableName().toLowerCase(); synchronized (update) { - tblPathChange = update.newPathChange(tbl.getDbName() + "." + tbl - .getTableName()); + Preconditions.checkArgument(tbl.getDbName().equalsIgnoreCase(db.getName())); + tblPathChange = update.newPathChange(db.getName() + "." + tableName); } if (tbl.getSd().getLocation() != null) { List<String> tblPath = PathsUpdate.parsePath(tbl.getSd().getLocation()); tblPathChange.addToAddPaths(tblPath); List<String> tblPartNames = - hmsHandler.get_partition_names(db.getName(), tbl - .getTableName(), (short) -1); + hmsHandler.get_partition_names(db.getName(), tableName, (short) -1); for (int i = 0; i < tblPartNames.size(); i += maxPartitionsPerCall) { List<String> partsToFetch = tblPartNames.subList(i, Math.min( i + maxPartitionsPerCall, tblPartNames.size())); Callable<CallResult> partTask = - new PartitionTask(db.getName(), tbl.getTableName(), + new PartitionTask(db.getName(), tableName, partsToFetch, tblPathChange); synchronized (results) { results.add(threadPool.submit(partTask)); @@ -162,7 +164,8 @@ class MetastoreCacheInitializer implements Closeable { DbTask(PathsUpdate update, String dbName) { super(); this.update = update; - this.dbName = dbName; + //Database names are case insensitive + this.dbName = dbName.toLowerCase(); } @Override @@ -171,10 +174,11 @@ class MetastoreCacheInitializer implements Closeable { List<String> dbPath = PathsUpdate.parsePath(db.getLocationUri()); if (dbPath != null) { synchronized (update) { - update.newPathChange(db.getName()).addToAddPaths(dbPath); + Preconditions.checkArgument(dbName.equalsIgnoreCase(db.getName())); + update.newPathChange(dbName).addToAddPaths(dbPath); } } - List<String> allTblStr = hmsHandler.get_all_tables(db.getName()); + List<String> allTblStr = hmsHandler.get_all_tables(dbName); for (int i = 0; i < allTblStr.size(); i += maxTablesPerCall) { List<String> tablesToFetch = allTblStr.subList(i, Math.min( http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/e8723d02/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java index d7b5d5a..8abdc83 100644 --- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java +++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java @@ -192,10 +192,10 @@ public class MetastorePlugin extends SentryMetastoreListenerPlugin { } LOGGER.debug("#### HMS Path Update [" + "OP : addPath, " - + "authzObj : " + authzObj + ", " + + "authzObj : " + authzObj.toLowerCase() + ", " + "path : " + path + "]"); PathsUpdate update = createHMSUpdate(); - update.newPathChange(authzObj).addToAddPaths(pathTree); + update.newPathChange(authzObj.toLowerCase()).addToAddPaths(pathTree); notifySentryAndApplyLocal(update); } @@ -203,16 +203,16 @@ public class MetastorePlugin extends SentryMetastoreListenerPlugin { public void removeAllPaths(String authzObj, List<String> childObjects) { LOGGER.debug("#### HMS Path Update [" + "OP : removeAllPaths, " - + "authzObj : " + authzObj + ", " + + "authzObj : " + authzObj.toLowerCase() + ", " + "childObjs : " + (childObjects == null ? "[]" : childObjects) + "]"); PathsUpdate update = createHMSUpdate(); if (childObjects != null) { for (String childObj : childObjects) { - update.newPathChange(authzObj + "." + childObj).addToDelPaths( + update.newPathChange(authzObj.toLowerCase() + "." + childObj).addToDelPaths( Lists.newArrayList(PathsUpdate.ALL_PATHS)); } } - update.newPathChange(authzObj).addToDelPaths( + update.newPathChange(authzObj.toLowerCase()).addToDelPaths( Lists.newArrayList(PathsUpdate.ALL_PATHS)); notifySentryAndApplyLocal(update); } @@ -220,7 +220,7 @@ public class MetastorePlugin extends SentryMetastoreListenerPlugin { @Override public void removePath(String authzObj, String path) { if ("*".equals(path)) { - removeAllPaths(authzObj, null); + removeAllPaths(authzObj.toLowerCase(), null); } else { List<String> pathTree = PathsUpdate.parsePath(path); if(pathTree == null) { @@ -228,10 +228,10 @@ public class MetastorePlugin extends SentryMetastoreListenerPlugin { } LOGGER.debug("#### HMS Path Update [" + "OP : removePath, " - + "authzObj : " + authzObj + ", " + + "authzObj : " + authzObj.toLowerCase() + ", " + "path : " + path + "]"); PathsUpdate update = createHMSUpdate(); - update.newPathChange(authzObj).addToDelPaths(pathTree); + update.newPathChange(authzObj.toLowerCase()).addToDelPaths(pathTree); notifySentryAndApplyLocal(update); } } @@ -239,6 +239,12 @@ public class MetastorePlugin extends SentryMetastoreListenerPlugin { @Override public void renameAuthzObject(String oldName, String oldPath, String newName, String newPath) { + if (oldName != null) { + oldName = oldName.toLowerCase(); + } + if (newName != null) { + newName = newName.toLowerCase(); + } PathsUpdate update = createHMSUpdate(); LOGGER.debug("#### HMS Path Update [" + "OP : renameAuthzObject, " http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/e8723d02/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java index 7587a1d..93514e6 100644 --- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java +++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/SentryPlugin.java @@ -264,7 +264,7 @@ public class SentryPlugin implements SentryPolicyStorePlugin { authzObj = dbName + "." + tblName; } } - return authzObj; + return authzObj == null ? null : authzObj.toLowerCase(); } private String getAuthzObj(TSentryAuthorizable authzble) { @@ -278,6 +278,6 @@ public class SentryPlugin implements SentryPolicyStorePlugin { authzObj = dbName + "." + tblName; } } - return authzObj; + return authzObj == null ? null : authzObj.toLowerCase(); } } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/e8723d02/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java index 5e29d65..208c93b 100644 --- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java +++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hdfs/TestHDFSIntegration.java @@ -632,6 +632,30 @@ public class TestHDFSIntegration { verifyOnAllSubDirs("/user/hive/warehouse/p3", FsAction.WRITE_EXECUTE, "hbase", true); verifyOnAllSubDirs("/user/hive/warehouse/p3/month=1/day=3", FsAction.WRITE_EXECUTE, "hbase", true); + // Test DB case insensitivity + stmt.execute("create database extdb"); + stmt.execute("grant all on database ExtDb to role p1_admin"); + writeToPath("/tmp/external/ext100", 5, "foo", "bar"); + writeToPath("/tmp/external/ext101", 5, "foo", "bar"); + stmt.execute("use extdb"); + stmt.execute( + "create table ext100 (s string) location \'/tmp/external/ext100\'"); + verifyQuery(stmt, "ext100", 5); + verifyOnAllSubDirs("/tmp/external/ext100", FsAction.ALL, "hbase", true); + stmt.execute("use default"); + + stmt.execute("use EXTDB"); + stmt.execute( + "create table ext101 (s string) location \'/tmp/external/ext101\'"); + verifyQuery(stmt, "ext101", 5); + verifyOnAllSubDirs("/tmp/external/ext101", FsAction.ALL, "hbase", true); + + // Test table case insensitivity + stmt.execute("grant all on table exT100 to role tab_role"); + verifyOnAllSubDirs("/tmp/external/ext100", FsAction.ALL, "flume", true); + + stmt.execute("use default"); + //TODO: SENTRY-795: HDFS permissions do not sync when Sentry restarts in HA mode. if(!testSentryHA) { long beforeStop = System.currentTimeMillis();
