You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by ls...@apache.org on 2016/01/21 21:47:13 UTC
incubator-sentry git commit: SENTRY-1008: Path should be not be
updated if the create/drop table/partition event fails (Hao Hao via Lenni
Kuff)
Repository: incubator-sentry
Updated Branches:
refs/heads/master 1b6fe629c -> 2ae1befda
SENTRY-1008: Path should be not be updated if the create/drop table/partition event fails (Hao Hao via Lenni Kuff)
Change-Id: I0b83686612ea10ea7c678f70c16ca975fc7c338e
Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/2ae1befd
Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/2ae1befd
Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/2ae1befd
Branch: refs/heads/master
Commit: 2ae1befdad80a5d23f4efe25d29555d767c592d8
Parents: 1b6fe62
Author: Lenni Kuff <ls...@cloudera.com>
Authored: Thu Jan 21 12:40:52 2016 -0800
Committer: Lenni Kuff <ls...@cloudera.com>
Committed: Thu Jan 21 12:40:52 2016 -0800
----------------------------------------------------------------------
.../SentryMetastorePostEventListener.java | 84 +++++--
.../tests/e2e/hdfs/TestHDFSIntegration.java | 232 +++++++++++++++++++
2 files changed, 299 insertions(+), 17 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/2ae1befd/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java
----------------------------------------------------------------------
diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java
index a45d115..cb797af 100644
--- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java
+++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java
@@ -88,6 +88,14 @@ public class SentryMetastorePostEventListener extends MetaStoreEventListener {
@Override
public void onCreateTable (CreateTableEvent tableEvent) throws MetaException {
+
+ // don't sync paths/privileges if the operation has failed
+ if (!tableEvent.getStatus()) {
+ LOGGER.debug("Skip sync paths/privileges with Sentry server for onCreateTable event," +
+ " since the operation failed. \n");
+ return;
+ }
+
if (tableEvent.getTable().getSd().getLocation() != null) {
String authzObj = tableEvent.getTable().getDbName() + "."
+ tableEvent.getTable().getTableName();
@@ -96,21 +104,27 @@ public class SentryMetastorePostEventListener extends MetaStoreEventListener {
plugin.addPath(authzObj, path);
}
}
+
// drop the privileges on the given table, in case if anything was left
// behind during the drop
if (!syncWithPolicyStore(AuthzConfVars.AUTHZ_SYNC_CREATE_WITH_POLICY_STORE)) {
return;
}
- // don't sync privileges if the operation has failed
- if (!tableEvent.getStatus()) {
- return;
- }
+
dropSentryTablePrivilege(tableEvent.getTable().getDbName(),
tableEvent.getTable().getTableName());
}
@Override
public void onDropTable(DropTableEvent tableEvent) throws MetaException {
+
+ // don't sync paths/privileges if the operation has failed
+ if (!tableEvent.getStatus()) {
+ LOGGER.debug("Skip syncing paths/privileges with Sentry server for onDropTable event," +
+ " since the operation failed. \n");
+ return;
+ }
+
if (tableEvent.getTable().getSd().getLocation() != null) {
String authzObj = tableEvent.getTable().getDbName() + "."
+ tableEvent.getTable().getTableName();
@@ -122,10 +136,11 @@ public class SentryMetastorePostEventListener extends MetaStoreEventListener {
if (!syncWithPolicyStore(AuthzConfVars.AUTHZ_SYNC_DROP_WITH_POLICY_STORE)) {
return;
}
- // don't sync privileges if the operation has failed
+
if (!tableEvent.getStatus()) {
return;
}
+
dropSentryTablePrivilege(tableEvent.getTable().getDbName(),
tableEvent.getTable().getTableName());
}
@@ -133,6 +148,14 @@ public class SentryMetastorePostEventListener extends MetaStoreEventListener {
@Override
public void onCreateDatabase(CreateDatabaseEvent dbEvent)
throws MetaException {
+
+ // don't sync paths/privileges if the operation has failed
+ if (!dbEvent.getStatus()) {
+ LOGGER.debug("Skip syncing paths/privileges with Sentry server for onCreateDatabase event," +
+ " since the operation failed. \n");
+ return;
+ }
+
if (dbEvent.getDatabase().getLocationUri() != null) {
String authzObj = dbEvent.getDatabase().getName();
String path = dbEvent.getDatabase().getLocationUri();
@@ -140,25 +163,30 @@ public class SentryMetastorePostEventListener extends MetaStoreEventListener {
plugin.addPath(authzObj, path);
}
}
- // drop the privileges on the database, incase anything left behind during
+ // drop the privileges on the database, in case anything left behind during
// last drop db
if (!syncWithPolicyStore(AuthzConfVars.AUTHZ_SYNC_CREATE_WITH_POLICY_STORE)) {
return;
}
- // don't sync privileges if the operation has failed
- if (!dbEvent.getStatus()) {
- return;
- }
+
dropSentryDbPrivileges(dbEvent.getDatabase().getName());
}
/**
- * Drop the privileges on the database // note that child tables will be
- * dropped individually by client, so we // just need to handle the removing
- * the db privileges. The table drop // should cleanup the table privileges
+ * Drop the privileges on the database. Note that child tables will be
+ * dropped individually by client, so we just need to handle the removing
+ * the db privileges. The table drop should cleanup the table privileges.
*/
@Override
public void onDropDatabase(DropDatabaseEvent dbEvent) throws MetaException {
+
+ // don't sync paths/privileges if the operation has failed
+ if (!dbEvent.getStatus()) {
+ LOGGER.debug("Skip syncing paths/privileges with Sentry server for onDropDatabase event," +
+ " since the operation failed. \n");
+ return;
+ }
+
String authzObj = dbEvent.getDatabase().getName();
for (SentryMetastoreListenerPlugin plugin : sentryPlugins) {
List<String> tNames = dbEvent.getHandler().get_all_tables(authzObj);
@@ -167,10 +195,7 @@ public class SentryMetastorePostEventListener extends MetaStoreEventListener {
if (!syncWithPolicyStore(AuthzConfVars.AUTHZ_SYNC_DROP_WITH_POLICY_STORE)) {
return;
}
- // don't sync privileges if the operation has failed
- if (!dbEvent.getStatus()) {
- return;
- }
+
dropSentryDbPrivileges(dbEvent.getDatabase().getName());
}
@@ -180,17 +205,22 @@ public class SentryMetastorePostEventListener extends MetaStoreEventListener {
@Override
public void onAlterTable (AlterTableEvent tableEvent) throws MetaException {
String oldTableName = null, newTableName = null;
+
// don't sync privileges if the operation has failed
if (!tableEvent.getStatus()) {
+ LOGGER.debug("Skip syncing privileges with Sentry server for onAlterTable event," +
+ " since the operation failed. \n");
return;
}
if (tableEvent.getOldTable() != null) {
oldTableName = tableEvent.getOldTable().getTableName();
}
+
if (tableEvent.getNewTable() != null) {
newTableName = tableEvent.getNewTable().getTableName();
}
+
renameSentryTablePrivilege(tableEvent.getOldTable().getDbName(),
oldTableName, tableEvent.getOldTable().getSd().getLocation(),
tableEvent.getNewTable().getDbName(), newTableName,
@@ -200,10 +230,14 @@ public class SentryMetastorePostEventListener extends MetaStoreEventListener {
@Override
public void onAlterPartition(AlterPartitionEvent partitionEvent)
throws MetaException {
+
// don't sync privileges if the operation has failed
if (!partitionEvent.getStatus()) {
+ LOGGER.debug("Skip syncing privileges with Sentry server for onAlterPartition event," +
+ " since the operation failed. \n");
return;
}
+
String oldLoc = null, newLoc = null;
if (partitionEvent.getOldPartition() != null) {
oldLoc = partitionEvent.getOldPartition().getSd().getLocation();
@@ -226,6 +260,14 @@ public class SentryMetastorePostEventListener extends MetaStoreEventListener {
@Override
public void onAddPartition(AddPartitionEvent partitionEvent)
throws MetaException {
+
+ // don't sync path if the operation has failed
+ if (!partitionEvent.getStatus()) {
+ LOGGER.debug("Skip syncing path with Sentry server for onAddPartition event," +
+ " since the operation failed. \n");
+ return;
+ }
+
for (Partition part : partitionEvent.getPartitions()) {
if (part.getSd() != null && part.getSd().getLocation() != null) {
String authzObj = part.getDbName() + "." + part.getTableName();
@@ -241,6 +283,14 @@ public class SentryMetastorePostEventListener extends MetaStoreEventListener {
@Override
public void onDropPartition(DropPartitionEvent partitionEvent)
throws MetaException {
+
+ // don't sync path if the operation has failed
+ if (!partitionEvent.getStatus()) {
+ LOGGER.debug("Skip syncing path with Sentry server for onDropPartition event," +
+ " since the operation failed. \n");
+ return;
+ }
+
String authzObj = partitionEvent.getTable().getDbName() + "."
+ partitionEvent.getTable().getTableName();
String path = partitionEvent.getPartition().getSd().getLocation();
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/2ae1befd/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 5a93ba0..fc7f324 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
@@ -252,6 +252,12 @@ public class TestHDFSIntegration {
hiveConf.set("datanucleus.autoStartMechanism", "SchemaTable");
hmsPort = findPort();
LOGGER.info("\n\n HMS port : " + hmsPort + "\n\n");
+
+ // Sets hive.metastore.authorization.storage.checks to true, so that
+ // disallow the operations such as drop-partition if the user in question
+ // doesn't have permissions to delete the corresponding directory
+ // on the storage.
+ hiveConf.set("hive.metastore.authorization.storage.checks", "true");
hiveConf.set("hive.metastore.uris", "thrift://localhost:" + hmsPort);
hiveConf.set("hive.metastore.pre.event.listeners", "org.apache.sentry.binding.metastore.MetastoreAuthzBinding");
hiveConf.set("hive.metastore.event.listeners", "org.apache.sentry.binding.metastore.SentryMetastorePostEventListener");
@@ -944,6 +950,232 @@ public class TestHDFSIntegration {
}
+ /**
+ * Make sure when events such as table creation fail, the path should not be sync to NameNode plugin.
+ */
+ @Test
+ public void testTableCreationFailure() throws Throwable {
+ String dbName = "db1";
+
+ tmpHDFSDir = new Path("/tmp/external");
+ if (!miniDFS.getFileSystem().exists(tmpHDFSDir)) {
+ miniDFS.getFileSystem().mkdirs(tmpHDFSDir);
+ }
+
+ miniDFS.getFileSystem().setPermission(tmpHDFSDir, FsPermission.valueOf("drwxrwx---"));
+ Path partitionDir = new Path("/tmp/external/p1");
+ if (!miniDFS.getFileSystem().exists(partitionDir)) {
+ miniDFS.getFileSystem().mkdirs(partitionDir);
+ }
+
+ dbNames = new String[]{dbName};
+ roles = new String[]{"admin_role"};
+ admin = StaticUserGroup.ADMIN1;
+
+ Connection conn;
+ Statement stmt;
+
+ conn = hiveServer2.createConnection("hive", "hive");
+ stmt = conn.createStatement();
+ stmt.execute("create role admin_role");
+ stmt.execute("grant all on server server1 to role admin_role");
+ stmt.execute("grant all on uri 'hdfs:///tmp/external' to role admin_role");
+ stmt.execute("grant role admin_role to group " + StaticUserGroup.ADMINGROUP);
+ stmt.execute("grant role admin_role to group " + StaticUserGroup.HIVE);
+ stmt.close();
+ conn.close();
+
+ conn = hiveServer2.createConnection(StaticUserGroup.ADMIN1, StaticUserGroup.ADMIN1);
+ stmt = conn.createStatement();
+ stmt.execute("create database " + dbName);
+
+ // Expect table creation to fail because hive:hive does not have
+ // permission to write at parent directory.
+ try {
+ stmt.execute("create external table tab1(a int) location 'hdfs:///tmp/external/p1'");
+ Assert.fail("Expect table creation to fail");
+ } catch (Exception ex) {
+ LOGGER.error("Exception when creating table: " + ex.getMessage());
+ }
+
+ // When the table creation failed, the path will not be managed by sentry. And the
+ // permission of the path will not be hive:hive.
+ verifyOnAllSubDirs("/tmp/external/p1", null, StaticUserGroup.HIVE, true);
+
+ stmt.close();
+ conn.close();
+ }
+
+ /**
+ * Make sure when events such as add partition fail, the path should not be sync to NameNode plugin.
+ */
+ @Test
+ public void testAddPartitionFailure() throws Throwable {
+ String dbName = "db1";
+
+ tmpHDFSDir = new Path("/tmp/external");
+ if (!miniDFS.getFileSystem().exists(tmpHDFSDir)) {
+ miniDFS.getFileSystem().mkdirs(tmpHDFSDir);
+ }
+
+ miniDFS.getFileSystem().setPermission(tmpHDFSDir, FsPermission.valueOf("drwxrwx---"));
+ Path partitionDir = new Path("/tmp/external/p1");
+ if (!miniDFS.getFileSystem().exists(partitionDir)) {
+ miniDFS.getFileSystem().mkdirs(partitionDir);
+ }
+
+ dbNames = new String[]{dbName};
+ roles = new String[]{"admin_role"};
+ admin = StaticUserGroup.ADMIN1;
+
+ Connection conn;
+ Statement stmt;
+
+ conn = hiveServer2.createConnection("hive", "hive");
+ stmt = conn.createStatement();
+ stmt.execute("create role admin_role");
+ stmt.execute("grant all on server server1 to role admin_role");
+ stmt.execute("grant role admin_role to group " + StaticUserGroup.ADMINGROUP);
+ stmt.close();
+ conn.close();
+
+ conn = hiveServer2.createConnection(StaticUserGroup.ADMIN1, StaticUserGroup.ADMIN1);
+ stmt = conn.createStatement();
+ stmt.execute("create database " + dbName);
+ stmt.execute("create external table tab2 (s string) partitioned by (month int)");
+
+ // Expect adding partition to fail because hive:hive does not have
+ // permission to write at parent directory.
+ try {
+ stmt.execute("alter table tab2 add partition (month = 1) location '/tmp/external/p1'");
+ Assert.fail("Expect adding partition to fail");
+ } catch (Exception ex) {
+ LOGGER.error("Exception when adding partition: " + ex.getMessage());
+ }
+
+ // When the table creation failed, the path will not be managed by sentry. And the
+ // permission of the path will not be hive:hive.
+ verifyOnAllSubDirs("/tmp/external/p1", null, StaticUserGroup.HIVE, true);
+
+ stmt.close();
+ conn.close();
+ }
+
+ /**
+ * Make sure when events such as drop table fail, the path should not be sync to NameNode plugin.
+ */
+ @Test
+ public void testDropTableFailure() throws Throwable {
+ String dbName = "db1";
+ tmpHDFSDir = new Path("/tmp/external");
+ if (!miniDFS.getFileSystem().exists(tmpHDFSDir)) {
+ miniDFS.getFileSystem().mkdirs(tmpHDFSDir);
+ }
+
+ miniDFS.getFileSystem().setPermission(tmpHDFSDir, FsPermission.valueOf("drwxrwxrwx"));
+ Path partitionDir = new Path("/tmp/external/p1");
+ if (!miniDFS.getFileSystem().exists(partitionDir)) {
+ miniDFS.getFileSystem().mkdirs(partitionDir);
+ }
+ dbNames = new String[]{dbName};
+ roles = new String[]{"admin_role"};
+ admin = StaticUserGroup.ADMIN1;
+
+ Connection conn;
+ Statement stmt;
+
+ conn = hiveServer2.createConnection("hive", "hive");
+ stmt = conn.createStatement();
+ stmt.execute("create role admin_role");
+ stmt.execute("grant all on server server1 to role admin_role");
+ stmt.execute("grant role admin_role to group " + StaticUserGroup.ADMINGROUP);
+ stmt.close();
+ conn.close();
+
+ conn = hiveServer2.createConnection(StaticUserGroup.ADMIN1, StaticUserGroup.ADMIN1);
+ stmt = conn.createStatement();
+ stmt.execute("create database " + dbName);
+ stmt.execute("create external table tab1(a int) location 'hdfs:///tmp/external/p1'");
+
+ miniDFS.getFileSystem().setPermission(tmpHDFSDir, FsPermission.valueOf("drwxrwx---"));
+
+ // Expect dropping table to fail because hive:hive does not have
+ // permission to write at parent directory when
+ // hive.metastore.authorization.storage.checks property is true.
+ try {
+ stmt.execute("drop table tab1");
+ Assert.fail("Expect dropping table to fail");
+ } catch (Exception ex) {
+ LOGGER.error("Exception when creating table: " + ex.getMessage());
+ }
+
+ // When the table dropping failed, the path will still be managed by sentry. And the
+ // permission of the path still should be hive:hive.
+ verifyOnAllSubDirs("/tmp/external/p1", FsAction.ALL, StaticUserGroup.HIVE, true);
+
+ stmt.close();
+ conn.close();
+ }
+
+ /**
+ * Make sure when events such as drop table fail, the path should not be sync to NameNode plugin.
+ */
+ @Test
+ public void testDropPartitionFailure() throws Throwable {
+ String dbName = "db1";
+
+ tmpHDFSDir = new Path("/tmp/external");
+ if (!miniDFS.getFileSystem().exists(tmpHDFSDir)) {
+ miniDFS.getFileSystem().mkdirs(tmpHDFSDir);
+ }
+
+ miniDFS.getFileSystem().setPermission(tmpHDFSDir, FsPermission.valueOf("drwxrwxrwx"));
+ Path partitionDir = new Path("/tmp/external/p1");
+ if (!miniDFS.getFileSystem().exists(partitionDir)) {
+ miniDFS.getFileSystem().mkdirs(partitionDir);
+ }
+
+ dbNames = new String[]{dbName};
+ roles = new String[]{"admin_role"};
+ admin = StaticUserGroup.ADMIN1;
+
+ Connection conn;
+ Statement stmt;
+
+ conn = hiveServer2.createConnection("hive", "hive");
+ stmt = conn.createStatement();
+ stmt.execute("create role admin_role");
+ stmt.execute("grant all on server server1 to role admin_role");
+ stmt.execute("grant role admin_role to group " + StaticUserGroup.ADMINGROUP);
+ stmt.close();
+ conn.close();
+
+ conn = hiveServer2.createConnection(StaticUserGroup.ADMIN1, StaticUserGroup.ADMIN1);
+ stmt = conn.createStatement();
+ stmt.execute("create database " + dbName);
+ stmt.execute("create table tab3 (s string) partitioned by (month int)");
+ stmt.execute("alter table tab3 add partition (month = 1) location '/tmp/external/p1'");
+
+ miniDFS.getFileSystem().setPermission(tmpHDFSDir, FsPermission.valueOf("drwxrwx---"));
+
+
+ // Expect dropping partition to fail because because hive:hive does not have
+ // permission to write at parent directory.
+ try {
+ stmt.execute("ALTER TABLE tab3 DROP PARTITION (month = 1)");
+ Assert.fail("Expect dropping partition to fail");
+ } catch (Exception ex) {
+ LOGGER.error("Exception when dropping partition: " + ex.getMessage());
+ }
+
+ // When the partition dropping failed, the path for the partition will still
+ // be managed by sentry. And the permission of the path still should be hive:hive.
+ verifyOnAllSubDirs("/tmp/external/p1", FsAction.ALL, StaticUserGroup.HIVE, true);
+
+ stmt.close();
+ conn.close();
+ }
+
@Test
public void testColumnPrivileges() throws Throwable {
String dbName = "db2";