Csaba Ringhofer 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 4: (5 comments) Great work, especially the workarounds for the Ranger bugs! I ran through most of the code, I plan to do another pass soon. http://gerrit.cloudera.org:8080/#/c/16837/4/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/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@105 PS4, Line 105: issue Is there a Ranger ticket for this? http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@133 PS4, Line 133: // actually revoke the role from the group. This should be considered a bug of Is there a Ranger ticket for this? http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@143 PS4, Line 143: plugin_.get().revokeRole(request, null); : plugin_.get().grantRole(request, null); Is it possible to run this with more than one threads at the same time? Two parallel grants to the same group could run like this: revoke role // revoking role if the group already had it revoke role // no effect grant role // granting role grant role // revoking role Even if this is possible, I don't think that it is a very serious issue, but it would be good to know whether we have to think about parallelism. http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@167 PS4, Line 167: dropping Are we logging "dropping" intentionally? http://gerrit.cloudera.org:8080/#/c/16837/4/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test File testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test: http://gerrit.cloudera.org:8080/#/c/16837/4/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test@1277 PS4, Line 1277: # Clean up the granted privileges and test roles. Will it cause problems if we fail do drop these? My understanding is that executing test files stop at the first failed test. -- 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: 4 Gerrit-Owner: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Comment-Date: Thu, 10 Dec 2020 18:43:20 +0000 Gerrit-HasComments: Yes
