You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/10/02 17:33:25 UTC

[5/8] impala git commit: IMPALA-7520: Fix NullPointerException in SentryProxy

IMPALA-7520: Fix NullPointerException in SentryProxy

Prior to this patch, the code in SentryProxy could throw a
NullPointerException when trying to retrieve a set of privileges for a
given role name. I was able to manually reproduce the issue by doing
the following steps:

1. Get all Sentry role privileges: [a, b] --> in SentryProxy
2. Add a sleep statement before getting all Sentry roles --> in SentryProxy
3. Add a new Sentry role: [c] --> Externally via Sentry CLI
4. Get all Sentry roles: [a, b, c] --> in SentryProxy
   Role c was added in step 3.
5. Get Sentry role privileges for role c: NPE --> in SentryProxy

The fix is to add a null guard when retrieving Sentry privileges for a
given role name and let the new role get updated in the next Sentry
refresh.

Testing:
- Ran all FE tests
- Ran all authorization E2E tests
- Manually tested it by temporarily modifying the SentryProxy code and
  did not see the NullPointerException

Change-Id: I36af840056a4d037fb5c7b1d9a167c0eb8526a11
Reviewed-on: http://gerrit.cloudera.org:8080/11552
Reviewed-by: Fredy Wijaya <fw...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/de39b033
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/de39b033
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/de39b033

Branch: refs/heads/master
Commit: de39b0331e1e000162a93bfb90888e2dfbadcd13
Parents: a381483
Author: Fredy Wijaya <fw...@cloudera.com>
Authored: Mon Oct 1 08:38:00 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Tue Oct 2 00:30:18 2018 +0000

----------------------------------------------------------------------
 fe/src/main/java/org/apache/impala/util/SentryProxy.java | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/de39b033/fe/src/main/java/org/apache/impala/util/SentryProxy.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/util/SentryProxy.java b/fe/src/main/java/org/apache/impala/util/SentryProxy.java
index b47d0ce..f74e033 100644
--- a/fe/src/main/java/org/apache/impala/util/SentryProxy.java
+++ b/fe/src/main/java/org/apache/impala/util/SentryProxy.java
@@ -209,7 +209,7 @@ public class SentryProxy {
      */
     private Set<String> refreshUserPrivileges() throws ImpalaException {
       // Assume all users should be removed. Then query the Policy Service and remove
-      // roles from this set that actually exist.
+      // users from this set that actually exist.
       Set<String> usersToRemove = catalog_.getAuthPolicy().getAllUserNames();
       Map<String, Set<TSentryPrivilege>> allUsersPrivileges =
           sentryPolicyService_.listAllUsersPrivileges(processUser_);
@@ -242,8 +242,11 @@ public class SentryProxy {
       // deleted from this set and we are left with the set of privileges that need
       // to be removed.
       Set<String> privilegesToRemove = principal.getPrivilegeNames();
+      Set<TSentryPrivilege> sentryPrivileges = allPrincipalPrivileges.get(
+          principal.getName());
+      if (sentryPrivileges == null) return;
       // Check all the privileges that are part of this principal.
-      for (TSentryPrivilege sentryPriv: allPrincipalPrivileges.get(principal.getName())) {
+      for (TSentryPrivilege sentryPriv: sentryPrivileges) {
         TPrivilege thriftPriv =
             SentryPolicyService.sentryPrivilegeToTPrivilege(sentryPriv, principal);
         String privilegeName = PrincipalPrivilege.buildPrivilegeName(thriftPriv);