Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests ...................................................................... Patch Set 5: (6 comments) http://gerrit.cloudera.org:8080/#/c/14798/5/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java File fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java: http://gerrit.cloudera.org:8080/#/c/14798/5/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java@61 PS5, Line 61: return (ResourceAuthorizationProvider) ConstructorUtils.invokeConstructor( Remove extra spaces http://gerrit.cloudera.org:8080/#/c/14798/1/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java File fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java: http://gerrit.cloudera.org:8080/#/c/14798/1/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@947 PS1, Line 947: test.error(accessError("functional.*.*")); Can you explicitly test that Rager allows access to functional.* ? Same question for all similar checks below. http://gerrit.cloudera.org:8080/#/c/14798/5/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java File fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java: http://gerrit.cloudera.org:8080/#/c/14798/5/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@946 PS5, Line 946: if (authzProvider_ == AuthorizationProvider.SENTRY) { Should there be an explicit Ranger testcase in the else clause here and in all of the similar checks below? http://gerrit.cloudera.org:8080/#/c/14798/5/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@1562 PS5, Line 1562: if (authzProvider_ == AuthorizationProvider.SENTRY) { Add comment here about what this specific case is and why it is allowed by Ranger http://gerrit.cloudera.org:8080/#/c/14798/5/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@1596 PS5, Line 1596: // The following do not result in Ranger authorization errors. A better comment would be something like: Default Ranger policies allow CREATE DATABASE http://gerrit.cloudera.org:8080/#/c/14798/5/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@3243 PS5, Line 3243: AS_OWNER = false; Where was this set? Looks strange to clean it up here. -- To view, visit http://gerrit.cloudera.org:8080/14798 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2 Gerrit-Change-Number: 14798 Gerrit-PatchSet: 5 Gerrit-Owner: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Comment-Date: Tue, 10 Dec 2019 18:24:54 +0000 Gerrit-HasComments: Yes
