Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/15833 )
Change subject: IMPALA-9708: Remove Sentry support ...................................................................... Patch Set 5: Code-Review+1 (1 comment) Thanks for doing the cleanup! Is this the part-1 and there will be other patches for this JIRA? I think at least we can remove the catalog cache for users/principals/privileges since they are only used for Sentry. It's ok to do this in a separate patch since maybe a lot of codes can be removed as well. Some works (IMPALA-9002, IMPALA-9195, IMPALA-9242, IMPALA-9222, etc.) in the SHOW TABLES/DATABASES code path are done for Sentry, they may can be removed/reverted as well. http://gerrit.cloudera.org:8080/#/c/15833/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/15833/5/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2336 PS5, Line 2336: /** : * Adds a new role with the given name and grant groups to the AuthorizationPolicy. : * If a role with the same name already exists it will be overwritten. : */ : public Role addRole(String roleName, Set<String> grantGroups) { : Principal role = addPrincipal(roleName, grantGroups, TPrincipalType.ROLE); : Preconditions.checkState(role instanceof Role); : return (Role) role; : } : : /** : * Adds a new user with the given name to the AuthorizationPolicy. : * If a user with the same name already exists it will be overwritten. : */ : public User addUser(String userName) { : Principal user = addPrincipal(userName, new HashSet<>(), : TPrincipalType.USER); : Preconditions.checkState(user instanceof User); : return (User) user; : } : : /** : * Add a user to the catalog if it doesn't exist. This is necessary so privileges : * can be added for a user. example: owner privileges. : */ : public User addUserIfNotExists(String owner, Reference<Boolean> existingUser) { : versionLock_.writeLock().lock(); : try { : User user = getAuthPolicy().getUser(owner); : existingUser.setRef(Boolean.TRUE); : if (user == null) { : user = addUser(owner); : existingUser.setRef(Boolean.FALSE); : } : return user; : } finally { : versionLock_.writeLock().unlock(); : } : } : : private Principal addPrincipal(String principalName, Set<String> grantGroups, : TPrincipalType type) { : versionLock_.writeLock().lock(); : try { : Principal principal = Principal.newInstance(principalName, type, grantGroups); : principal.setCatalogVersion(incrementAndGetCatalogVersion()); : authPolicy_.addPrincipal(principal); : return principal; : } finally { : versionLock_.writeLock().unlock(); : } : } : : /** : * Removes the role with the given name from the AuthorizationPolicy. Returns the : * removed role with an incremented catalog version, or null if no role with this name : * exists. : */ : public Role removeRole(String roleName) { : Principal role = removePrincipal(roleName, TPrincipalType.ROLE); : if (role == null) return null; : Preconditions.checkState(role instanceof Role); : return (Role) role; : } : : /** : * Removes the user with the given name from the AuthorizationPolicy. Returns the : * removed user with an incremented catalog version, or null if no user with this name : * exists. : */ : public User removeUser(String userName) { : Principal user = removePrincipal(userName, TPrincipalType.USER); : if (user == null) return null; : Preconditions.checkState(user instanceof User); : return (User) user; : } : : private Principal removePrincipal(String principalName, TPrincipalType type) { : versionLock_.writeLock().lock(); : try { : Principal principal = authPolicy_.removePrincipal(principalName, type); : // TODO(todd): does this end up leaking the privileges associated : // with this principal into the CatalogObjectVersionSet on the catalogd? : if (principal == null) return null; : for (PrincipalPrivilege priv: principal.getPrivileges()) { : priv.setCatalogVersion(incrementAndGetCatalogVersion()); : deleteLog_.addRemovedObject(priv.toTCatalogObject()); : } : principal.setCatalogVersion(incrementAndGetCatalogVersion()); : deleteLog_.addRemovedObject(principal.toTCatalogObject()); : return principal; : } finally { : versionLock_.writeLock().unlock(); : } : } : : /** : * Adds a grant group to the given role name and returns the modified Role with : * an updated catalog version. If the role does not exist a CatalogException is thrown. : */ : public Role addRoleGrantGroup(String roleName, String groupName) : throws CatalogException { : versionLock_.writeLock().lock(); : try { : Role role = authPolicy_.addRoleGrantGroup(roleName, groupName); : Preconditions.checkNotNull(role); : role.setCatalogVersion(incrementAndGetCatalogVersion()); : return role; : } finally { : versionLock_.writeLock().unlock(); : } : } : : /** : * Removes a grant group from the given role name and returns the modified Role with : * an updated catalog version. If the role does not exist a CatalogException is thrown. : */ : public Role removeRoleGrantGroup(String roleName, String groupName) : throws CatalogException { : versionLock_.writeLock().lock(); : try { : Role role = authPolicy_.removeRoleGrantGroup(roleName, groupName); : Preconditions.checkNotNull(role); : role.setCatalogVersion(incrementAndGetCatalogVersion()); : return role; : } finally { : versionLock_.writeLock().unlock(); : } : } : : /** : * Adds a privilege to the given role name. Returns the new PrincipalPrivilege and : * increments the catalog version. If the parent role does not exist a CatalogException : * is thrown. : */ : public PrincipalPrivilege addRolePrivilege(String roleName, TPrivilege thriftPriv) : throws CatalogException { : Preconditions.checkArgument(thriftPriv.getPrincipal_type() == TPrincipalType.ROLE); : return addPrincipalPrivilege(roleName, thriftPriv, TPrincipalType.ROLE); : } : : /** : * Adds a privilege to the given user name. Returns the new PrincipalPrivilege and : * increments the catalog version. If the user does not exist a CatalogException is : * thrown. : */ : public PrincipalPrivilege addUserPrivilege(String userName, TPrivilege thriftPriv) : throws CatalogException { : Preconditions.checkArgument(thriftPriv.getPrincipal_type() == TPrincipalType.USER); : return addPrincipalPrivilege(userName, thriftPriv, TPrincipalType.USER); : } : : private PrincipalPrivilege addPrincipalPrivilege(String principalName, : TPrivilege thriftPriv, TPrincipalType type) throws CatalogException { : versionLock_.writeLock().lock(); : try { : Principal principal = authPolicy_.getPrincipal(principalName, type); : if (principal == null) { : throw new CatalogException(String.format("%s does not exist: %s", : Principal.toString(type), principalName)); : } : PrincipalPrivilege priv = PrincipalPrivilege.fromThrift(thriftPriv); : priv.setCatalogVersion(incrementAndGetCatalogVersion()); : authPolicy_.addPrivilege(priv); : return priv; : } finally { : versionLock_.writeLock().unlock(); : } : } : : /** : * Removes a PrincipalPrivilege from the given role name and privilege name. Returns : * the removed PrincipalPrivilege with an incremented catalog version or null if no : * matching privilege was found. Throws a CatalogException if no role exists with this : * name. : */ : public PrincipalPrivilege removeRolePrivilege(String roleName, String privilegeName) : throws CatalogException { : return removePrincipalPrivilege(roleName, privilegeName, TPrincipalType.ROLE); : } : : /** : * Removes a PrincipalPrivilege from the given user name and privilege name. Returns : * the removed PrincipalPrivilege with an incremented catalog version or null if no : * matching privilege was found. Throws a CatalogException if no user exists with this : * name. : */ : public PrincipalPrivilege removeUserPrivilege(String userName, String privilegeName) : throws CatalogException { : return removePrincipalPrivilege(userName, privilegeName, TPrincipalType.USER); : } : : private PrincipalPrivilege removePrincipalPrivilege(String principalName, : String privilegeName, TPrincipalType type) throws CatalogException { : versionLock_.writeLock().lock(); : try { : Principal principal = authPolicy_.getPrincipal(principalName, type); : if (principal == null) { : throw new CatalogException(String.format("%s does not exist: %s", : Principal.toString(type), principalName)); : } : PrincipalPrivilege principalPrivilege = principal.removePrivilege(privilegeName); : if (principalPrivilege == null) return null; : principalPrivilege.setCatalogVersion(incrementAndGetCatalogVersion()); : deleteLog_.addRemovedObject(principalPrivilege.toTCatalogObject()); : return principalPrivilege; : } finally { : versionLock_.writeLock().unlock(); : } : } : : /** : * Gets a PrincipalPrivilege from the given principal name. Returns the privilege : * if it exists, or null if no privilege matching the privilege spec exist. : * Throws a CatalogException if the principal does not exist. : */ : public PrincipalPrivilege getPrincipalPrivilege(String principalName, : TPrivilege privSpec) throws CatalogException { : String privilegeName = PrincipalPrivilege.buildPrivilegeName(privSpec); : versionLock_.readLock().lock(); : try { : Principal principal = authPolicy_.getPrincipal(principalName, : privSpec.getPrincipal_type()); : if (principal == null) { : throw new CatalogException(Principal.toString(privSpec.getPrincipal_type()) + : " does not exist: " + principalName); : } : return principal.getPrivilege(privilegeName); : } finally { : versionLock_.readLock().unlock(); : } : } These can be removed as well. They are only used for Sentry. For Ranger, the authz stuffs (user/privileges/tags/policies) are cached by the ranger plugin. We don't need to maintain users/principals/privileges in our catalog cache (both catalogd side and impalad side). -- To view, visit http://gerrit.cloudera.org:8080/15833 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8e99c15936d6d250cf258e3a1dcba11d3eb4661e Gerrit-Change-Number: 15833 Gerrit-PatchSet: 5 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Tue, 19 May 2020 02:01:17 +0000 Gerrit-HasComments: Yes
