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 <fwij...@cloudera.com> Tested-by: Impala Public Jenkins <impala-public-jenk...@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 <fwij...@cloudera.com> Authored: Mon Oct 1 08:38:00 2018 -0700 Committer: Impala Public Jenkins <impala-public-jenk...@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);