This is an automated email from the ASF dual-hosted git repository.
amishra pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/sentry.git
The following commit(s) were added to refs/heads/master by this push:
new 6842d1d SENTRY-2146: Add better error handling to
ResourceAuthorizationProvider and improve logging in related classes (Arjun
Mishra reviewed by Kalyan Kumar Kalvagadda)
6842d1d is described below
commit 6842d1da7d57922dd81c1bd0e2469237666e7f35
Author: amishra <[email protected]>
AuthorDate: Sun Feb 10 09:00:04 2019 -0600
SENTRY-2146: Add better error handling to ResourceAuthorizationProvider and
improve logging in related classes (Arjun Mishra reviewed by Kalyan Kumar
Kalvagadda)
---
.../binding/hive/authz/HiveAuthzBinding.java | 18 ++++++------
.../sentry/binding/hive/HiveAuthzBindingHook.java | 5 ++++
.../hive/authz/HiveAuthzBindingHookBase.java | 14 ++++++++-
.../apache/sentry/core/common/utils/KeyValue.java | 4 +--
.../sentry/policy/common/CommonPrivilege.java | 6 ++++
.../common/ResourceAuthorizationProvider.java | 33 ++++++++++++----------
6 files changed, 52 insertions(+), 28 deletions(-)
diff --git
a/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
b/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
index 0397f87..5c7f84f 100644
---
a/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
+++
b/sentry-binding/sentry-binding-hive-common/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java
@@ -323,16 +323,9 @@ public class HiveAuthzBinding implements AutoCloseable {
// Check read entities
Map<AuthorizableType, EnumSet<DBModelAction>> requiredInputPrivileges =
stmtAuthPrivileges.getInputPrivileges();
- if(isDebug) {
- LOG.debug("requiredInputPrivileges = " + requiredInputPrivileges);
- LOG.debug("inputHierarchyList = " + inputHierarchyList);
- }
- Map<AuthorizableType, EnumSet<DBModelAction>> requiredOutputPrivileges =
- stmtAuthPrivileges.getOutputPrivileges();
- if(isDebug) {
- LOG.debug("requiredOuputPrivileges = " + requiredOutputPrivileges);
- LOG.debug("outputHierarchyList = " + outputHierarchyList);
- }
+ LOG.debug("requiredInputPrivileges = " + requiredInputPrivileges);
+ LOG.debug("inputHierarchyList = " + inputHierarchyList);
+
boolean found = false;
for (Map.Entry<AuthorizableType, EnumSet<DBModelAction>> entry :
requiredInputPrivileges.entrySet()) {
@@ -360,6 +353,11 @@ public class HiveAuthzBinding implements AutoCloseable {
found = false;
}
+ Map<AuthorizableType, EnumSet<DBModelAction>> requiredOutputPrivileges =
+ stmtAuthPrivileges.getOutputPrivileges();
+ LOG.debug("requiredOuputPrivileges = " + requiredOutputPrivileges);
+ LOG.debug("outputHierarchyList = " + outputHierarchyList);
+
for (Map.Entry<AuthorizableType, EnumSet<DBModelAction>> entry :
requiredOutputPrivileges.entrySet()) {
AuthorizableType key = entry.getKey();
for (List<DBModelAuthorizable> outputHierarchy : outputHierarchyList) {
diff --git
a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
index a4002b7..e87d0f6 100644
---
a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
+++
b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java
@@ -299,6 +299,9 @@ public class HiveAuthzBindingHook extends
HiveAuthzBindingHookBase {
currDB = getCanonicalDb();
break;
}
+
+ LOG.debug("preAnalyze: For Operation={}; " + this,
ast.getToken().getType());
+
return ast;
}
@@ -322,6 +325,8 @@ public class HiveAuthzBindingHook extends
HiveAuthzBindingHookBase {
Subject subject = getCurrentSubject(context);
+ LOG.debug("postAnalyze: HiveOperation={}, HiveAuthzPrivileges={},
subject={}", stmtOperation, stmtAuthObject, subject);
+
try {
if (stmtAuthObject == null) {
// We don't handle authorizing this statement
diff --git
a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java
b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java
index 46eb456..ed278c8 100644
---
a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java
+++
b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBindingHookBase.java
@@ -345,7 +345,6 @@ public abstract class HiveAuthzBindingHookBase extends
AbstractSemanticAnalyzerH
Set<List<DBModelAuthorizable>> outputHierarchy = new
HashSet<List<DBModelAuthorizable>>();
if(LOG.isDebugEnabled()) {
- LOG.debug("stmtAuthObject.getOperationScope() = " +
stmtAuthObject.getOperationScope());
LOG.debug("context.getInputs() = " + context.getInputs());
LOG.debug("context.getOutputs() = " + context.getOutputs());
}
@@ -928,4 +927,17 @@ public abstract class HiveAuthzBindingHookBase extends
AbstractSemanticAnalyzerH
return true;
}
+
+ @Override
+ public String toString() {
+ StringBuilder strb = new StringBuilder();
+ strb.append("currDB=").append(currDB).append(", currTab=").append(currTab)
+ .append(", udfURIs=").append(udfURIs).append(",
serdeURI=").append(serdeURI)
+ .append(", partitionURI=").append(partitionURI).append(",
indexURI=").append(indexURI)
+ .append(", currOutDB=").append(currOutDB).append(",
currOutTab=").append(currOutTab)
+ .append(", serdeWhiteList=").append(serdeWhiteList).append(",
serdeURIPrivilegesEnabled=").append(serdeURIPrivilegesEnabled)
+ .append(", isDescTableBasic=").append(isDescTableBasic);
+
+ return strb.toString();
+ }
}
diff --git
a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/KeyValue.java
b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/KeyValue.java
index 4e944e5..b6a1faa 100644
---
a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/KeyValue.java
+++
b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/KeyValue.java
@@ -34,9 +34,9 @@ public class KeyValue {
key = kvList.get(0);
value = kvList.get(1);
if (key.isEmpty()) {
- throw new IllegalArgumentException("Key cannot be empty");
+ throw new IllegalArgumentException("kvList=[" + kvList + "] for
keyValue[" + keyValue + "], Key cannot be empty");
} else if (value.isEmpty()) {
- throw new IllegalArgumentException("Value cannot be empty");
+ throw new IllegalArgumentException("kvList=[" + kvList + "] for
keyValue[" + keyValue + "], Value cannot be empty");
}
}
diff --git
a/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
b/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
index 61d442a..5b261e3 100644
---
a/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
+++
b/sentry-policy/sentry-policy-common/src/main/java/org/apache/sentry/policy/common/CommonPrivilege.java
@@ -30,18 +30,24 @@ import org.apache.sentry.core.common.utils.SentryConstants;
import java.util.ArrayList;
import java.util.List;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
// The class is used to compare the privilege
public class CommonPrivilege implements Privilege {
private ImmutableList<KeyValue> parts;
private boolean grantOption = false;
+ private static final Logger LOGGER =
LoggerFactory.getLogger(CommonPrivilege.class);
public CommonPrivilege(String privilegeStr) {
privilegeStr = Strings.nullToEmpty(privilegeStr).trim();
if (privilegeStr.isEmpty()) {
throw new IllegalArgumentException("Privilege string cannot be null or
empty.");
}
+
+ LOGGER.debug("Create Privilege instance for privilegeStr={}",
privilegeStr);
+
List<KeyValue> parts = Lists.newArrayList();
for (String authorizable :
SentryConstants.AUTHORIZABLE_SPLITTER.trimResults().split(
privilegeStr)) {
diff --git
a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
index 1e1aa63..222b77a 100644
---
a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
+++
b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ResourceAuthorizationProvider.java
@@ -24,7 +24,6 @@ import static
org.apache.sentry.core.common.utils.SentryConstants.PRIVILEGE_NAME
import java.util.ArrayList;
import java.util.Collections;
-import java.util.HashSet;
import java.util.List;
import java.util.Set;
@@ -125,29 +124,33 @@ public abstract class ResourceAuthorizationProvider
implements AuthorizationProv
LOGGER.debug("Groups not found for " + subject);
}
Set<String> users = Sets.newHashSet(subject.getName());
- Set<String> hierarchy = new HashSet<String>();
- for (Authorizable authorizable : authorizables) {
- hierarchy.add(KV_JOINER.join(authorizable.getTypeName(),
authorizable.getName()));
- }
List<String> requestPrivileges = buildPermissions(authorizables, actions,
requireGrantOption);
+ LOGGER.debug("requestPrivileges={}", requestPrivileges);
+ LOGGER.debug("PolicyEngine={}, PrivilegeFactory={}",
policy.getClass().getName(), policy.getPrivilegeFactory().getClass().getName());
+ LOGGER.debug("Get privileges for groups={}, users={}, roleSet={}", groups,
users, roleSet);
+
Iterable<Privilege> privileges = getPrivileges(groups, users, roleSet,
authorizables.toArray(new Authorizable[0]));
lastFailedPrivileges.get().clear();
for (String requestPrivilege : requestPrivileges) {
- Privilege priv = privilegeFactory.createPrivilege(requestPrivilege);
- for (Privilege permission : privileges) {
- /*
- * Does the permission granted in the policy file imply the requested
action?
- */
- boolean result = permission.implies(priv, model);
- if (LOGGER.isDebugEnabled()) {
+ try {
+ Privilege priv = privilegeFactory.createPrivilege(requestPrivilege);
+ for (Privilege permission : privileges) {
+ /*
+ * Does the permission granted in the policy file imply the
requested action?
+ */
+ boolean result = permission.implies(priv, model);
LOGGER.debug("ProviderPrivilege {}, RequestPrivilege {}, RoleSet {},
Result {}",
new Object[]{ permission, requestPrivilege, roleSet, result});
+ if (result) {
+ return true;
+ }
+
}
- if (result) {
- return true;
- }
+ } catch(Exception e) {
+ LOGGER.error("doHasAccess: Exception", e);
+ throw e;
}
}
