You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by sr...@apache.org on 2015/12/18 00:01:50 UTC
incubator-sentry git commit: SENTRY-988: It's better to let
SentryAuthorization setter path always fall through and update HDFS (Yongjun
Zhang, Reviewed by: Sravya Tirukkovalur)
Repository: incubator-sentry
Updated Branches:
refs/heads/master 4b33ad929 -> 06688cee6
SENTRY-988: It's better to let SentryAuthorization setter path always fall through and update HDFS (Yongjun Zhang, Reviewed by: Sravya Tirukkovalur)
Change-Id: I6f868ad1b3f72ff0e8bdc06e945b56b406dbf062
Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/06688cee
Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/06688cee
Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/06688cee
Branch: refs/heads/master
Commit: 06688cee637a86c99c6bf9c8ec22534e1d628b80
Parents: 4b33ad9
Author: Sravya Tirukkovalur <sr...@cloudera.com>
Authored: Thu Dec 17 15:01:10 2015 -0800
Committer: Sravya Tirukkovalur <sr...@cloudera.com>
Committed: Thu Dec 17 15:01:10 2015 -0800
----------------------------------------------------------------------
.../sentry/hdfs/SentryAuthorizationInfo.java | 17 +-
.../hdfs/SentryAuthorizationProvider.java | 166 ++++++++++---------
.../sentry/hdfs/SentryAuthorizationInfoX.java | 2 +-
3 files changed, 103 insertions(+), 82 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/06688cee/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 c8d56be..def34a4 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
@@ -251,7 +251,7 @@ public class SentryAuthorizationInfo implements Runnable {
return stale;
}
- public boolean isManaged(String[] pathElements) {
+ public boolean isUnderPrefix(String[] pathElements) {
lock.readLock().lock();
try {
return authzPaths.isUnderPrefix(pathElements);
@@ -260,6 +260,11 @@ public class SentryAuthorizationInfo implements Runnable {
}
}
+ @Deprecated
+ public boolean isManaged(String[] pathElements) {
+ return isUnderPrefix(pathElements);
+ }
+
public boolean doesBelongToAuthzObject(String[] pathElements) {
lock.readLock().lock();
try {
@@ -269,6 +274,16 @@ public class SentryAuthorizationInfo implements Runnable {
}
}
+ public boolean isSentryManaged(final String[] pathElements) {
+ lock.readLock().lock();
+ try {
+ return authzPaths.isUnderPrefix(pathElements) &&
+ authzPaths.findAuthzObject(pathElements) != null;
+ } finally {
+ lock.readLock().unlock();
+ }
+ }
+
@SuppressWarnings("unchecked")
public List<AclEntry> getAclEntries(String[] pathElements) {
lock.readLock().lock();
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/06688cee/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 4d03ba3..b7e94f3 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
@@ -21,7 +21,6 @@ import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
-import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -38,6 +37,7 @@ import org.apache.hadoop.fs.permission.FsPermission;
import org.apache.hadoop.hdfs.DFSConfigKeys;
import org.apache.hadoop.hdfs.server.namenode.AclFeature;
import org.apache.hadoop.hdfs.server.namenode.AuthorizationProvider;
+import org.apache.hadoop.hdfs.server.namenode.snapshot.Snapshot;
import org.apache.hadoop.security.AccessControlException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -66,6 +66,9 @@ public class SentryAuthorizationProvider
private boolean originalAuthzAsAcl;
private SentryAuthorizationInfo authzInfo;
+ private static String WARN_VISIBILITY =
+ " The result won't be visible when the path is managed by Sentry";
+
public SentryAuthorizationProvider() {
this(null);
}
@@ -193,80 +196,61 @@ public class SentryAuthorizationProvider
return paths;
}
- @Override
- public void setUser(INodeAuthorizationInfo node, String user) {
+ private boolean isSentryManaged(final String[] pathElements) {
+ return authzInfo.isSentryManaged(pathElements);
+ }
+
+ private boolean isSentryManaged(INodeAuthorizationInfo node) {
String[] pathElements = getPathElements(node);
+ return isSentryManaged(pathElements);
+ }
- // 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
+ public void setUser(INodeAuthorizationInfo node, String user) {
+ // always fall through to defaultAuthZProvider,
+ // issue warning when the path is sentry managed
+ if (isSentryManaged(node)) {
+ LOG.warn("### setUser {} (sentry managed path) to {}, update HDFS." +
+ WARN_VISIBILITY,
+ node.getFullPathName(), user);
}
+ defaultAuthzProvider.setUser(node, user);
}
@Override
public String getUser(INodeAuthorizationInfo node, int snapshotId) {
- String user;
- String[] pathElements = getPathElements(node);
- if (!authzInfo.isManaged(pathElements)) {
- user = defaultAuthzProvider.getUser(node, snapshotId);
- } else if (!authzInfo.doesBelongToAuthzObject(pathElements)) {
- user = defaultAuthzProvider.getUser(node, snapshotId);
- } else {
- user = this.user;
- }
- return user;
+ return isSentryManaged(node)?
+ this.user : defaultAuthzProvider.getUser(node, snapshotId);
}
@Override
public void setGroup(INodeAuthorizationInfo node, String 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");
- }
+ // always fall through to defaultAuthZProvider,
+ // issue warning when the path is sentry managed
+ if (isSentryManaged(node)) {
+ LOG.warn("### setGroup {} (sentry managed path) to {}, update HDFS." +
+ WARN_VISIBILITY,
+ node.getFullPathName(), group);
}
+ defaultAuthzProvider.setGroup(node, group);
}
@Override
public String getGroup(INodeAuthorizationInfo node, int snapshotId) {
- String group;
- String[] pathElements = getPathElements(node);
- if (!authzInfo.isManaged(pathElements)) {
- group = getDefaultProviderGroup(node, snapshotId);
- } else if (!authzInfo.doesBelongToAuthzObject(pathElements)) {
- group = getDefaultProviderGroup(node, snapshotId);
- } else {
- group = this.group;
- }
- return group;
+ return isSentryManaged(node)?
+ this.group : defaultAuthzProvider.getGroup(node, snapshotId);
}
@Override
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");
- }
+ // always fall through to defaultAuthZProvider,
+ // issue warning when the path is sentry managed
+ if (isSentryManaged(node)) {
+ LOG.warn("### setPermission {} (sentry managed path) to {}, update HDFS." +
+ WARN_VISIBILITY,
+ node.getFullPathName(), permission.toString());
}
+ defaultAuthzProvider.setPermission(node, permission);
}
@Override
@@ -274,12 +258,9 @@ public class SentryAuthorizationProvider
INodeAuthorizationInfo node, int snapshotId) {
FsPermission permission;
String[] pathElements = getPathElements(node);
- if (!authzInfo.isManaged(pathElements)) {
- permission = defaultAuthzProvider.getFsPermission(node, snapshotId);
- } else if (!authzInfo.doesBelongToAuthzObject(pathElements)) {
+ if (!isSentryManaged(pathElements)) {
permission = defaultAuthzProvider.getFsPermission(node, snapshotId);
- }
- else {
+ } else {
FsPermission returnPerm = this.permission;
// Handle case when prefix directory is itself associated with an
// authorizable object (default db directory in hive)
@@ -329,18 +310,18 @@ public class SentryAuthorizationProvider
AclFeature f = null;
String[] pathElements = getPathElements(node);
String p = Arrays.toString(pathElements);
- boolean isManaged = false;
+ boolean isPrefixed = false;
boolean isStale = false;
boolean hasAuthzObj = false;
Map<String, AclEntry> aclMap = null;
- if (!authzInfo.isManaged(pathElements)) {
- isManaged = false;
+ if (!authzInfo.isUnderPrefix(pathElements)) {
+ isPrefixed = false;
f = defaultAuthzProvider.getAclFeature(node, snapshotId);
} else if (!authzInfo.doesBelongToAuthzObject(pathElements)) {
- isManaged = true;
+ isPrefixed = true;
f = defaultAuthzProvider.getAclFeature(node, snapshotId);
} else {
- isManaged = true;
+ isPrefixed = true;
hasAuthzObj = true;
aclMap = new HashMap<String, AclEntry>();
if (originalAuthzAsAcl) {
@@ -363,7 +344,7 @@ public class SentryAuthorizationProvider
}
if (LOG.isDebugEnabled()) {
LOG.debug("### getAclEntry \n[" + (p == null ? "null" : p) + "] : ["
- + "isManaged=" + isManaged
+ + "isPreifxed=" + isPrefixed
+ ", isStale=" + isStale
+ ", hasAuthzObj=" + hasAuthzObj
+ ", origAuthzAsAcl=" + originalAuthzAsAcl + "]\n"
@@ -404,30 +385,55 @@ public class SentryAuthorizationProvider
return group;
}
- @Override
- public void removeAclFeature(INodeAuthorizationInfo node) {
- AclFeature aclFeature = node.getAclFeature(CURRENT_STATE_ID);
- 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)) {
+ /*
+ * Check if the given node has ACL, remove the ACL if so. Issue a warning
+ * message when the node doesn't have ACL and warn is true.
+ * TODO: We need this to maintain backward compatibility (not throw error in
+ * some cases). We may remove this when we release sentry major version.
+ */
+ private void checkAndRemoveHdfsAcl(INodeAuthorizationInfo node,
+ boolean warn) {
+ AclFeature f = defaultAuthzProvider.getAclFeature(node,
+ Snapshot.CURRENT_STATE_ID);
+ if (f != null) {
defaultAuthzProvider.removeAclFeature(node);
} else {
- if (LOG.isErrorEnabled()) {
- LOG.error("### removeAclFeature is a no op for " +
- "the path under prefix.\n");
+ if (warn) {
+ LOG.warn("### removeAclFeature is requested on {}, but it doesn't " +
+ "have any acl.", node);
}
}
}
@Override
+ public void removeAclFeature(INodeAuthorizationInfo node) {
+ // always fall through to defaultAuthZProvider,
+ // issue warning when the path is sentry managed
+ if (isSentryManaged(node)) {
+ LOG.warn("### removeAclFeature {} (sentry managed path), update HDFS." +
+ WARN_VISIBILITY,
+ node.getFullPathName());
+ // For Sentry-managed paths, client code may try to remove a
+ // non-existing ACL, ignore the request with a warning if the ACL
+ // doesn't exist
+ checkAndRemoveHdfsAcl(node, true);
+ } else {
+ defaultAuthzProvider.removeAclFeature(node);
+ }
+ }
+
+ @Override
public void addAclFeature(INodeAuthorizationInfo node, AclFeature f) {
- String[] pathElements = getPathElements(node);
- if (!authzInfo.isManaged(pathElements)) {
- defaultAuthzProvider.addAclFeature(node, f);
+ // always fall through to defaultAuthZProvider,
+ // issue warning when the path is sentry managed
+ if (isSentryManaged(node)) {
+ LOG.warn("### addAclFeature {} (sentry managed path) {}, update HDFS." +
+ WARN_VISIBILITY,
+ node.getFullPathName(), f.toString());
+ // For Sentry-managed path, remove ACL silently before adding new ACL
+ checkAndRemoveHdfsAcl(node, false);
}
+ defaultAuthzProvider.addAclFeature(node, f);
}
}
http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/06688cee/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 0ed290d..a81f7ab 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
@@ -67,7 +67,7 @@ public class SentryAuthorizationInfoX extends SentryAuthorizationInfo {
}
@Override
- public boolean isManaged(String[] pathElements) {
+ public boolean isUnderPrefix(String[] pathElements) {
return hasPrefix(MANAGED, pathElements);
}