Repository: incubator-sentry Updated Branches: refs/heads/master f0aebaa94 -> 17a4c97f7
SENTRY-944: Setting HDFS rules on Sentry managed hdfs paths should not affect original hdfs rules ( Hao Hao, Reviewed by: Sravya Tirukkovalur) Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/17a4c97f Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/17a4c97f Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/17a4c97f Branch: refs/heads/master Commit: 17a4c97f760fe3795a17505409b6cec67767d0a0 Parents: f0aebaa Author: Sravya Tirukkovalur <[email protected]> Authored: Mon Nov 9 14:07:02 2015 -0800 Committer: Sravya Tirukkovalur <[email protected]> Committed: Mon Nov 9 14:07:02 2015 -0800 ---------------------------------------------------------------------- .../hdfs/SentryAuthorizationProvider.java | 54 +++++++++++++++++--- .../hdfs/TestSentryAuthorizationProvider.java | 44 ++++++++++++++++ 2 files changed, 92 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/17a4c97f/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 419ab68..4d03ba3 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 @@ -195,7 +195,18 @@ public class SentryAuthorizationProvider @Override public void setUser(INodeAuthorizationInfo node, String user) { - defaultAuthzProvider.setUser(node, user); + String[] pathElements = getPathElements(node); + + // For the non sentry managed paths, set the user based on + // the requests. Otherwise should be a no op. + if (!authzInfo.isManaged(pathElements) + || !authzInfo.doesBelongToAuthzObject(pathElements)) { + defaultAuthzProvider.setUser(node, user); + } else { + if (LOG.isErrorEnabled()) { + LOG.error("### setUser is a no op for the sentry managed path.\n"); + } + } } @Override @@ -214,7 +225,18 @@ public class SentryAuthorizationProvider @Override public void setGroup(INodeAuthorizationInfo node, String group) { - defaultAuthzProvider.setGroup(node, group); + String[] pathElements = getPathElements(node); + + // For the non sentry managed paths, set the group based on + // the requests. Otherwise should be a no op. + if (!authzInfo.isManaged(pathElements) + || !authzInfo.doesBelongToAuthzObject(pathElements)) { + defaultAuthzProvider.setGroup(node, group); + } else { + if (LOG.isErrorEnabled()) { + LOG.error("### setGroup is a no op for the sentry managed path.\n"); + } + } } @Override @@ -232,9 +254,19 @@ public class SentryAuthorizationProvider } @Override - public void setPermission(INodeAuthorizationInfo node, - FsPermission permission) { - defaultAuthzProvider.setPermission(node, permission); + public void setPermission(INodeAuthorizationInfo node, FsPermission permission) { + String[] pathElements = getPathElements(node); + + // For the non sentry managed paths, set the permission based on + // the requests. Otherwise should be a no op. + if (!authzInfo.isManaged(pathElements) + || !authzInfo.doesBelongToAuthzObject(pathElements)) { + defaultAuthzProvider.setPermission(node, permission); + } else { + if (LOG.isErrorEnabled()) { + LOG.error("### setPermission is a no op for the sentry managed path.\n"); + } + } } @Override @@ -375,8 +407,18 @@ public class SentryAuthorizationProvider @Override public void removeAclFeature(INodeAuthorizationInfo node) { AclFeature aclFeature = node.getAclFeature(CURRENT_STATE_ID); - if (aclFeature.getClass() != SentryAclFeature.class) { + String[] pathElements = getPathElements(node); + + // For non sentry managed paths, remove the ACLs based on + // the requests. Otherwise should be a no op. + if (aclFeature.getClass() != SentryAclFeature.class + && !authzInfo.isManaged(pathElements)) { defaultAuthzProvider.removeAclFeature(node); + } else { + if (LOG.isErrorEnabled()) { + LOG.error("### removeAclFeature is a no op for " + + "the path under prefix.\n"); + } } } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/17a4c97f/sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryAuthorizationProvider.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryAuthorizationProvider.java b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryAuthorizationProvider.java index fd5146f..5da0dc2 100644 --- a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryAuthorizationProvider.java +++ b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/test/java/org/apache/sentry/hdfs/TestSentryAuthorizationProvider.java @@ -169,6 +169,50 @@ public class TestSentryAuthorizationProvider { Assert.assertEquals(new FsPermission((short) 0755), status.getPermission()); Assert.assertTrue(fs.getAclStatus(path).getEntries().isEmpty()); + // setPermission sets the permission for dir outside of prefix. + // setUser/setGroup sets the user/group for dir outside of prefix. + Path pathOutside = new Path("/user/xxx"); + + fs.setPermission(pathOutside, new FsPermission((short) 0000)); + Assert.assertEquals(new FsPermission((short) 0000), fs.getFileStatus(pathOutside).getPermission()); + fs.setOwner(pathOutside, sysUser, "supergroup"); + Assert.assertEquals(sysUser, fs.getFileStatus(pathOutside).getOwner()); + Assert.assertEquals("supergroup", fs.getFileStatus(pathOutside).getGroup()); + + // removeAcl removes the ACL entries for dir outside of prefix. + List<AclEntry> aclsOutside = new ArrayList<AclEntry>(baseAclList); + List<AclEntry> acl = new ArrayList<AclEntry>(); + acl.add(new AclEntry.Builder().setName("supergroup").setType(AclEntryType.GROUP).setScope(AclEntryScope.ACCESS). + setPermission(FsAction.READ_EXECUTE).build()); + aclsOutside.addAll(acl); + fs.setAcl(pathOutside, aclsOutside); + fs.removeAclEntries(pathOutside, acl); + Assert.assertFalse(fs.getAclStatus(pathOutside).getEntries().containsAll(acl)); + + // setPermission sets the permission for dir inside of prefix but not a hive obj. + // setUser/setGroup sets the user/group for dir inside of prefix but not a hive obj. + Path pathInside = new Path("/user/authz/xxx"); + + fs.setPermission(pathInside, new FsPermission((short) 0000)); + Assert.assertEquals(new FsPermission((short) 0000), fs.getFileStatus(pathInside).getPermission()); + fs.setOwner(pathInside, sysUser, "supergroup"); + Assert.assertEquals(sysUser, fs.getFileStatus(pathInside).getOwner()); + Assert.assertEquals("supergroup", fs.getFileStatus(pathInside).getGroup()); + + // removeAcl is a no op for dir inside of prefix. + Assert.assertTrue(fs.getAclStatus(pathInside).getEntries().isEmpty()); + fs.removeAclEntries(pathInside, acl); + Assert.assertTrue(fs.getAclStatus(pathInside).getEntries().isEmpty()); + + // setPermission/setUser/setGroup is a no op for dir inside of prefix, and is a hive obj. + Path pathInsideAndHive = new Path("/user/authz/obj"); + + fs.setPermission(pathInsideAndHive, new FsPermission((short) 0000)); + Assert.assertEquals(new FsPermission((short) 0771), fs.getFileStatus(pathInsideAndHive).getPermission()); + fs.setOwner(pathInsideAndHive, sysUser, "supergroup"); + Assert.assertEquals("hive", fs.getFileStatus(pathInsideAndHive).getOwner()); + Assert.assertEquals("hive", fs.getFileStatus(pathInsideAndHive).getGroup()); + return null; } });
