Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16837 )
Change subject: IMPALA-10211 (Part 1): Add support for role-related statements ...................................................................... Patch Set 2: (2 comments) Left some comments and will address them in the next patch. http://gerrit.cloudera.org:8080/#/c/16837/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java: http://gerrit.cloudera.org:8080/#/c/16837/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@144 PS2, Line 144: // To avoid confusion, we do not use the error message from Ranger directly since When a Ranger administrator is revoke an existing role from a non-existing group, Ranger will return an error message indicating the group does not exist in Ranger. Thus, by handling the exception this way, we mask the error message due to other types of error. I will push a revised patch soon. http://gerrit.cloudera.org:8080/#/c/16837/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java: http://gerrit.cloudera.org:8080/#/c/16837/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@163 PS2, Line 163: /** : * For now there is no dedicated REST API that allows Impala to determine whether : * 'user' is a Ranger administrator. In this regard, we call : * RangerBasePlugin#getAllRoles(), whose server side method, : * RoleREST#getAllRoleNames(), will call RoleREST#ensureAdminAccess() on 'user' to make : * sure 'user' is a Ranger administrator. An Exception will be thrown if it is not the : * case. : */ : private void validateRangerAdmin(String user) throws Exception { : plugin_.get().getAllRoles(user, null); : } This method is not used. So I will remove it in the next patch. -- To view, visit http://gerrit.cloudera.org:8080/16837 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860 Gerrit-Change-Number: 16837 Gerrit-PatchSet: 2 Gerrit-Owner: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Wed, 09 Dec 2020 22:06:00 +0000 Gerrit-HasComments: Yes
