Repository: incubator-sentry Updated Branches: refs/heads/master d88c157d8 -> 02041c56d
SENTRY-559: Allow prefix paths to be associated with authorizable objects (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/02041c56 Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/02041c56 Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/02041c56 Branch: refs/heads/master Commit: 02041c56dc1d2ef3a47cf6a2966c0850d3947236 Parents: d88c157 Author: Arun Suresh <[email protected]> Authored: Tue Dec 2 14:40:07 2014 -0800 Committer: Arun Suresh <[email protected]> Committed: Tue Dec 2 14:40:07 2014 -0800 ---------------------------------------------------------------------- .../java/org/apache/sentry/hdfs/HMSPaths.java | 12 +++++--- .../org/apache/sentry/hdfs/HMSPathsDumper.java | 5 ++++ .../sentry/hdfs/TestHMSPathsFullDump.java | 5 +++- .../sentry/hdfs/TestUpdateableAuthzPaths.java | 7 +++++ .../sentry/hdfs/SentryAuthorizationInfo.java | 26 ++++++++++++++++- .../hdfs/SentryAuthorizationProvider.java | 18 ++++++++++-- .../sentry/hdfs/SentryAuthorizationInfoX.java | 3 +- .../tests/e2e/hdfs/TestHDFSIntegration.java | 30 ++++++++++++++++++++ 8 files changed, 95 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/02041c56/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java index 0e9fc2c..d52e361 100644 --- a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java +++ b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java @@ -122,7 +122,7 @@ public class HMSPaths implements AuthzPaths { children = new HashMap<String, Entry>(); } - private void setAuthzObj(String authzObj) { + void setAuthzObj(String authzObj) { this.authzObj = authzObj; } @@ -157,6 +157,10 @@ public class HMSPaths implements AuthzPaths { child = new Entry(entryParent, lastPathElement, type, authzObj); entryParent.getChildren().put(lastPathElement, child); } else if (type == EntryType.AUTHZ_OBJECT && + child.getType() == EntryType.PREFIX) { + // Support for default db in hive (which is usually a prefix dir) + child.setAuthzObj(authzObj); + } else if (type == EntryType.AUTHZ_OBJECT && child.getType() == EntryType.DIR) { // if the entry already existed as dir, we change it to be a authz obj child.setAuthzObj(authzObj); @@ -277,17 +281,17 @@ public class HMSPaths implements AuthzPaths { boolean isPartialMatchOk, Entry lastAuthObj) { Entry found = null; if (index == pathElements.length) { - if (isPartialMatchOk && (getType() == EntryType.AUTHZ_OBJECT)) { + if (isPartialMatchOk && (getAuthzObj() != null)) { found = this; } } else { Entry child = getChildren().get(pathElements[index]); if (child != null) { if (index == pathElements.length - 1) { - found = (child.getType() == EntryType.AUTHZ_OBJECT) ? child : lastAuthObj; + found = (child.getAuthzObj() != null) ? child : lastAuthObj; } else { found = child.find(pathElements, index + 1, isPartialMatchOk, - (child.getType() == EntryType.AUTHZ_OBJECT) ? child : lastAuthObj); + (child.getAuthzObj() != null) ? child : lastAuthObj); } } else { if (isPartialMatchOk) { http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/02041c56/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java index 1537c1e..8f7bb0f 100644 --- a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java +++ b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPathsDumper.java @@ -105,6 +105,11 @@ public class HMSPathsDumper implements AuthzPathsDumper<HMSPaths> { // already exists.. else it is not part of the prefix if (child == null) continue; isChildPrefix = child.getType() == EntryType.PREFIX; + // Handle case when prefix entry has an authzObject + // For Eg (default table mapped to /user/hive/warehouse) + if (isChildPrefix) { + child.setAuthzObj(tChild.getAuthzObj()); + } } if (child == null) { child = new Entry(parent, tChild.getPathElement(), http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/02041c56/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPathsFullDump.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPathsFullDump.java b/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPathsFullDump.java index 2dfe73c..f74a75d 100644 --- a/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPathsFullDump.java +++ b/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestHMSPathsFullDump.java @@ -37,6 +37,7 @@ public class TestHMSPathsFullDump { @Test public void testDumpAndInitialize() { HMSPaths hmsPaths = new HMSPaths(new String[] {"/user/hive/warehouse", "/user/hive/w2"}); + hmsPaths._addAuthzObject("default", Lists.newArrayList("/user/hive/warehouse")); hmsPaths._addAuthzObject("db1", Lists.newArrayList("/user/hive/warehouse/db1")); hmsPaths._addAuthzObject("db1.tbl11", Lists.newArrayList("/user/hive/warehouse/db1/tbl11")); hmsPaths._addPathsToAuthzObject("db1.tbl11", Lists.newArrayList( @@ -44,11 +45,12 @@ public class TestHMSPathsFullDump { "/user/hive/warehouse/db1/tbl11/part112", "/user/hive/warehouse/db1/tbl11/p1=1/p2=x")); - // Not in prefix paths + // Not in Deserialized objects prefix paths hmsPaths._addAuthzObject("db2", Lists.newArrayList("/user/hive/w2/db2")); hmsPaths._addAuthzObject("db2.tbl21", Lists.newArrayList("/user/hive/w2/db2/tbl21")); hmsPaths._addPathsToAuthzObject("db2.tbl21", Lists.newArrayList("/user/hive/w2/db2/tbl21/p1=1/p2=x")); + Assert.assertEquals("default", hmsPaths.findAuthzObject(new String[]{"user", "hive", "warehouse"}, false)); Assert.assertEquals("db1", hmsPaths.findAuthzObject(new String[]{"user", "hive", "warehouse", "db1"}, false)); Assert.assertEquals("db1.tbl11", hmsPaths.findAuthzObject(new String[]{"user", "hive", "warehouse", "db1", "tbl11"}, false)); Assert.assertEquals("db1.tbl11", hmsPaths.findAuthzObject(new String[]{"user", "hive", "warehouse", "db1", "tbl11", "part111"}, false)); @@ -62,6 +64,7 @@ public class TestHMSPathsFullDump { TPathsDump pathsDump = serDe.createPathsDump(); HMSPaths hmsPaths2 = new HMSPaths(new String[] {"/user/hive/warehouse"}).getPathsDump().initializeFromDump(pathsDump); + Assert.assertEquals("default", hmsPaths2.findAuthzObject(new String[]{"user", "hive", "warehouse"}, false)); Assert.assertEquals("db1", hmsPaths2.findAuthzObject(new String[]{"user", "hive", "warehouse", "db1"}, false)); Assert.assertEquals("db1.tbl11", hmsPaths2.findAuthzObject(new String[]{"user", "hive", "warehouse", "db1", "tbl11"}, false)); Assert.assertEquals("db1.tbl11", hmsPaths2.findAuthzObject(new String[]{"user", "hive", "warehouse", "db1", "tbl11", "part111"}, false)); http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/02041c56/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestUpdateableAuthzPaths.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestUpdateableAuthzPaths.java b/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestUpdateableAuthzPaths.java index 80b765a..51a939d 100644 --- a/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestUpdateableAuthzPaths.java +++ b/sentry-hdfs/sentry-hdfs-common/src/test/java/org/apache/sentry/hdfs/TestUpdateableAuthzPaths.java @@ -138,6 +138,13 @@ public class TestUpdateableAuthzPaths { assertEquals("db1.tbl11", authzPaths.findAuthzObjectExactMatch(new String[]{"db1", "tbl11", "part112"})); } + @Test + public void testDefaultDbPath() { + HMSPaths hmsPaths = new HMSPaths(new String[] {"/user/hive/warehouse"}); + hmsPaths._addAuthzObject("default", Lists.newArrayList("/user/hive/warehouse")); + assertEquals("default", hmsPaths.findAuthzObject(new String[]{"user", "hive", "warehouse"})); + } + private HMSPaths createBaseHMSPaths(int dbNum, int tblNum) { String db = "db" + dbNum; String tbl = "tbl" + dbNum + "" + tblNum; http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/02041c56/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java index c0889f2..08807ca 100644 --- a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java +++ b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationInfo.java @@ -27,6 +27,7 @@ import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.permission.AclEntry; import org.apache.hadoop.util.StringUtils; import org.apache.sentry.hdfs.SentryHDFSServiceClient.SentryAuthzUpdate; @@ -35,6 +36,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; public class SentryAuthorizationInfo implements Runnable { private static Logger LOG = @@ -56,8 +58,13 @@ public class SentryAuthorizationInfo implements Runnable { // concrete implementation of a ReadWriteLock. private final ReadWriteLock lock = new ReentrantReadWriteLock(); + private String[][] pathPrefixes; + + // For use only for testing !! @VisibleForTesting - SentryAuthorizationInfo() {} + SentryAuthorizationInfo(String[] pathPrefixes) { + setPrefixPaths(pathPrefixes); + } public SentryAuthorizationInfo(Configuration conf) throws Exception { String[] pathPrefixes = conf.getTrimmedStrings( @@ -80,6 +87,7 @@ public class SentryAuthorizationInfo implements Runnable { LOG.debug("Sentry authorization will enforced in the following HDFS " + "locations: [{}]", StringUtils.arrayToString(pathPrefixes)); + setPrefixPaths(pathPrefixes); LOG.debug("Refresh interval [{}]ms, retry wait [{}], stale threshold " + "[{}]ms", new Object[] {refreshIntervalMillisec, retryWaitMillisec, staleThresholdMillisec}); @@ -92,6 +100,22 @@ public class SentryAuthorizationInfo implements Runnable { } } + private void setPrefixPaths(String[] pathPrefixes) { + this.pathPrefixes = new String[pathPrefixes.length][]; + for (int i = 0; i < this.pathPrefixes.length; i++) { + Preconditions.checkArgument( + pathPrefixes[i].startsWith("" + Path.SEPARATOR_CHAR), + "Path prefix [" + pathPrefixes[i] + "]" + + "does not starting with [" + Path.SEPARATOR_CHAR + "]"); + this.pathPrefixes[i] = + pathPrefixes[i].substring(1).split("" + Path.SEPARATOR_CHAR); + } + } + + String[][] getPathPrefixes() { + return pathPrefixes; + } + UpdateableAuthzPaths getAuthzPaths() { return authzPaths; } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/02041c56/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java index 1d209f7..cd96cc4 100644 --- a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java +++ b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java @@ -226,7 +226,7 @@ public class SentryAuthorizationProvider String group; String[] pathElements = getPathElements(node); if (!authzInfo.isManaged(pathElements)) { - group = defaultAuthzProvider.getGroup(node, snapshotId); + group = getDefaultProviderGroup(node, snapshotId); } else { if (!authzInfo.isStale()) { if (authzInfo.doesBelongToAuthzObject(pathElements)) { @@ -255,14 +255,26 @@ public class SentryAuthorizationProvider if (!authzInfo.isManaged(pathElements)) { permission = defaultAuthzProvider.getFsPermission(node, snapshotId); } else { + FsPermission returnPerm = this.permission; + // Handle case when prefix directory is itself associated with an + // authorizable object (default db directory in hive) + // An executable permission needs to be set on the the prefix directory + // in this case.. else, subdirectories (which map to other dbs) will + // not be travesible. + for (String [] prefixPath : authzInfo.getPathPrefixes()) { + if (Arrays.equals(prefixPath, pathElements)) { + returnPerm = FsPermission.createImmutable((short)(returnPerm.toShort() | 0x01)); + break; + } + } if (!authzInfo.isStale()) { if (authzInfo.doesBelongToAuthzObject(pathElements)) { - permission = this.permission; + permission = returnPerm; } else { permission = defaultAuthzProvider.getFsPermission(node, snapshotId); } } else { - permission = this.permission; + permission = returnPerm; } } return permission; http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/02041c56/sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/SentryAuthorizationInfoX.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/SentryAuthorizationInfoX.java b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/SentryAuthorizationInfoX.java index 7a1539b..4cebed2 100644 --- a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/SentryAuthorizationInfoX.java +++ b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/SentryAuthorizationInfoX.java @@ -20,7 +20,6 @@ package org.apache.sentry.hdfs; import java.util.Arrays; import java.util.List; -import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.permission.AclEntry; import org.apache.hadoop.fs.permission.AclEntryScope; import org.apache.hadoop.fs.permission.AclEntryType; @@ -29,7 +28,7 @@ import org.apache.hadoop.fs.permission.FsAction; public class SentryAuthorizationInfoX extends SentryAuthorizationInfo { public SentryAuthorizationInfoX() { - super(); + super(new String[]{"/user/authz"}); } @Override http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/02041c56/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 30c72eb..c640db5 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 @@ -487,15 +487,34 @@ public class TestHDFSIntegration { stmt.execute("create role p1_admin"); stmt.execute("grant role p1_admin to group hbase"); + // Verify default db is inaccessible initially + verifyOnAllSubDirs("/user/hive/warehouse", null, "hbase", false); + verifyOnAllSubDirs("/user/hive/warehouse/p1", null, "hbase", false); loadData(stmt); verifyHDFSandMR(stmt); + // Verify default db is STILL inaccessible after grants but tables are fine + verifyOnPath("/user/hive/warehouse", null, "hbase", false); + verifyOnAllSubDirs("/user/hive/warehouse/p1", FsAction.READ_EXECUTE, "hbase", true); + stmt.execute("revoke select on table p1 from role p1_admin"); verifyOnAllSubDirs("/user/hive/warehouse/p1", null, "hbase", false); + // Verify default db grants work + stmt.execute("grant select on database default to role p1_admin"); + verifyOnPath("/user/hive/warehouse", FsAction.READ_EXECUTE, "hbase", true); + + // Verify default db grants are propagated to the tables + verifyOnAllSubDirs("/user/hive/warehouse/p1", FsAction.READ_EXECUTE, "hbase", true); + + // Verify default db revokes work + stmt.execute("revoke select on database default from role p1_admin"); + verifyOnPath("/user/hive/warehouse", null, "hbase", false); + verifyOnAllSubDirs("/user/hive/warehouse/p1", null, "hbase", false); + stmt.execute("grant all on table p1 to role p1_admin"); verifyOnAllSubDirs("/user/hive/warehouse/p1", FsAction.ALL, "hbase", true); @@ -561,11 +580,22 @@ public class TestHDFSIntegration { stmt.execute("create table db1.tbl2 (s string)"); verifyOnAllSubDirs("/user/hive/warehouse/db1.db/tbl2", null, "hbase", false); + // Verify default db grants do not affect other dbs + stmt.execute("grant all on database default to role p1_admin"); + verifyOnPath("/user/hive/warehouse", FsAction.ALL, "hbase", true); + verifyOnAllSubDirs("/user/hive/warehouse/db1.db", null, "hbase", false); + // Verify db privileges are propagated to tables stmt.execute("grant select on database db1 to role p1_admin"); verifyOnAllSubDirs("/user/hive/warehouse/db1.db/tbl1", FsAction.READ_EXECUTE, "hbase", true); verifyOnAllSubDirs("/user/hive/warehouse/db1.db/tbl2", FsAction.READ_EXECUTE, "hbase", true); + // Verify default db revokes do not affect other dbs + stmt.execute("revoke all on database default from role p1_admin"); + verifyOnPath("/user/hive/warehouse", null, "hbase", false); + verifyOnAllSubDirs("/user/hive/warehouse/db1.db/tbl1", FsAction.READ_EXECUTE, "hbase", true); + verifyOnAllSubDirs("/user/hive/warehouse/db1.db/tbl2", FsAction.READ_EXECUTE, "hbase", true); + stmt.execute("use db1"); stmt.execute("grant all on table tbl1 to role p1_admin");
