Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15206 )
Change subject: [WIP] Ranger client ...................................................................... Patch Set 3: (3 comments) Only looked at the protobuf definition and ranger client as it is a wip. http://gerrit.cloudera.org:8080/#/c/15206/3/src/kudu/ranger/ranger.proto File src/kudu/ranger/ranger.proto: http://gerrit.cloudera.org:8080/#/c/15206/3/src/kudu/ranger/ranger.proto@24 PS3, Line 24: SCAN To match with Ranger server side definition, we should use SELECT instead of SCAN here. http://gerrit.cloudera.org:8080/#/c/15206/3/src/kudu/ranger/ranger_action.cc File src/kudu/ranger/ranger_action.cc: http://gerrit.cloudera.org:8080/#/c/15206/3/src/kudu/ranger/ranger_action.cc@45 PS3, Line 45: case Action::METADATA: : return ActionPB::METADATA; Throw exception for unknown actions? http://gerrit.cloudera.org:8080/#/c/15206/3/src/kudu/ranger/ranger_client.h File src/kudu/ranger/ranger_client.h: http://gerrit.cloudera.org:8080/#/c/15206/3/src/kudu/ranger/ranger_client.h@44 PS3, Line 44: // Authorizes listing tables. If there is at least one table that user is : // authorized to access metadata of, it returns OK and sets 'table_names' to : // the tables the user is authorized to list. Otherwise it returns : // NotAuthorized and it doesn't modify 'table_names'. : Status AuthorizeList(const std::string& user_name, : std::unordered_set<std::string>* table_names) WARN_UNUSED_RESULT; : : // Authorizes an action on the table. Returns OK if 'user_name' is authorized : // to perform 'action' on 'table_name', NotAuthorized otherwise. : Status AuthorizeAction(const std::string& user_name, const Action& action, : const std::string& table_name) WARN_UNUSED_RESULT; : : // Authorizes a scan on a table. Returns OK if 'user_name' is authorized to : // scan the whole table or at least one of the specified columns, : // NotAuthorized otherwise. If the user isn't authorized to scan the whole : // table, 'column_names' is changed to contain only the columns the user is : // authorized to scan. : Status AuthorizeScan(const std::string& user_name, const std::string& table_name, : std::unordered_set<std::string>* column_names) WARN_UNUSED_RESULT; These three methods kind of overlap with each other, they are all for authorizing some action upon some resources(either table level or column level) for a user. Maybe define a structure that hold all possible information for a request(e.g action, db name, table name, column name), and just use one method which takes a list of such structure (e.g RangerRequest) and returns a list of boolean which match the order of the request. In this case, the protobuf definition for Ranger can be simpler and information like default_database won't need to be present in the wire. -- To view, visit http://gerrit.cloudera.org:8080/15206 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2e1ec19ed3aeb4d82ad38fe1fb655f57021c1a4 Gerrit-Change-Number: 15206 Gerrit-PatchSet: 3 Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 13 Feb 2020 06:07:58 +0000 Gerrit-HasComments: Yes
