sentry-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From amis...@apache.org
Subject [sentry] branch master updated: SENTRY-2146: Add better error handling to ResourceAuthorizationProvider and improve logging in related classes (Arjun Mishra reviewed by Kalyan Kumar Kalvagadda)
Date Sun, 10 Feb 2019 15:11:25 GMT
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 <amishra@cloudera.com>
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;
       }
     }
 


Mime
View raw message