Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12914 )
Change subject: IMPALA-8226: Add grant/revoke to/from group for Ranger ...................................................................... Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/12914/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java: http://gerrit.cloudera.org:8080/#/c/12914/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java@165 PS4, Line 165: String[] idents = {"myRole", "ROLE myRole", "GROUP myGroup", "USER myUser"}; : boolean[] isGrantVals = {true, false}; do we have tests for bad idents? http://gerrit.cloudera.org:8080/#/c/12914/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java File fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java: http://gerrit.cloudera.org:8080/#/c/12914/4/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java@3153 PS4, Line 3153: withPrincipals.add((isUser_) ? new WithRangerUser() : new WithRangerGroup()); shouldn't we be running both user and ranger? http://gerrit.cloudera.org:8080/#/c/12914/4/fe/src/test/java/org/apache/impala/analysis/ParserTest.java File fe/src/test/java/org/apache/impala/analysis/ParserTest.java: http://gerrit.cloudera.org:8080/#/c/12914/4/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@3592 PS4, Line 3592: String[] resources = {"SERVER", "SERVER foo", "DATABASE foo", "TABLE foo", : "URI 'foo'"}; : String[] badResources = {"DATABASE", "TABLE", "URI", "URI foo", "TABLE 'foo'", : "SERVER 'foo'", "DATABASE 'foo'"}; : String[] privileges = {"SELECT", "INSERT", "ALL", "REFRESH", "CREATE", "ALTER", : "DROP"}; : String[] badPrivileges = {"UPDATE", "DELETE", "UPSERT", "FAKE"}; : String[] columnPrivResource = {"SELECT (a, b) ON TABLE foo", "SELECT () on TABLE foo", : "INSERT (a, b) ON TABLE foo", "ALL (a, b) ON TABLE foo"}; : String[] badColumnPrivResource = {"SELECT (a,) ON TABLE foo", : "SELECT (*) ON TABLE foo", "SELECT (a), b ON TABLE foo", : "SELECT ((a)) ON TABLE foo", "SELECT (a, b) ON URI foo", : "SELECT ON TABLE (a, b) foo",}; : String[][] grantRevoke = {{"GRANT", "TO"}, {"REVOKE", "FROM"}}; : String[] idents = {"myRole", "GROUP myGroup", "USER user", "ROLE myRole"}; : String[] badIdents = {"GROUP", "ROLE", "GROUP group", "GROUP role", "USER role", : "FOOBAR foobar", ""}; this is a bit hard to read, maybe put each element in a new line where it makes sense? -- To view, visit http://gerrit.cloudera.org:8080/12914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96 Gerrit-Change-Number: 12914 Gerrit-PatchSet: 4 Gerrit-Owner: Austin Nobis <ano...@cloudera.com> Gerrit-Reviewer: Austin Nobis <ano...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: radford nguyen <radford.ngu...@gmail.com> Gerrit-Comment-Date: Wed, 03 Apr 2019 19:06:36 +0000 Gerrit-HasComments: Yes
