Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15074 )
Change subject: [java] KUDU-2972: add Kudu Ranger plugin ...................................................................... Patch Set 4: (7 comments) http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/RangerProtocolHandler.java File java/kudu-ranger/src/main/java/org/apache/kudu/ranger/RangerProtocolHandler.java: http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/RangerProtocolHandler.java@33 PS4, Line 33: request to Ranger and get authorization decision nit: "sends requests to Ranger and gets authorization decisions" http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/RangerProtocolHandler.java@56 PS4, Line 56: getUser Do we expect this to change from request to request in a single ListPB? http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java File java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java: http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java@37 PS4, Line 37: // The following properties need to match the Kudu service def in Ranger. nit: maybe link to https://github.com/apache/ranger/blob/master/agents-common/src/main/resources/service-defs/ranger-servicedef-kudu.json http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java@48 PS4, Line 48: plugin = new RangerBasePlugin(SERVICE_TYPE, APP_ID); > How do we configure the plugin? For example, how does it know where Ranger +1 Is there some ranger-site.xml we need to pass as an argument? http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java@61 PS4, Line 61: * @param action action to be authorized on the resource : * @param user user performs the action : * @param db the database name the action is to be performed on : * @param table the table name the action is to be performed on : * @param col the column name the action is to be performed on nit: could you also document if/how these can be empty or null? For instance, if I just want to authorize for SELECT ON TABLE("default.foo"), what should the 'col' argument be? http://gerrit.cloudera.org:8080/#/c/15074/4/java/kudu-ranger/src/main/java/org/apache/kudu/ranger/authorization/RangerKuduAuthorizer.java@103 PS4, Line 103: createRequest Could this and getUserGroups be static? http://gerrit.cloudera.org:8080/#/c/15074/4/src/kudu/ranger/ranger.proto File src/kudu/ranger/ranger.proto: http://gerrit.cloudera.org:8080/#/c/15074/4/src/kudu/ranger/ranger.proto@62 PS4, Line 62: : // Describes a list of authorization descison responded from the Ranger : // service for a request. nit: note that the order is determined by the order of RangerRequestListPB? -- To view, visit http://gerrit.cloudera.org:8080/15074 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0c995ac1a48ebf57667231cd3a82d3794f6ddf8d Gerrit-Change-Number: 15074 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 17 Feb 2020 04:15:29 +0000 Gerrit-HasComments: Yes
