Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15681 )
Change subject: KUDU-3078 Add Ranger tests to master_authz-itest ...................................................................... Patch Set 13: Code-Review+1 (3 comments) http://gerrit.cloudera.org:8080/#/c/15681/13/src/kudu/integration-tests/master_authz-itest.cc File src/kudu/integration-tests/master_authz-itest.cc: http://gerrit.cloudera.org:8080/#/c/15681/13/src/kudu/integration-tests/master_authz-itest.cc@515 PS13, Line 515: policy.tables.emplace_back("*"); Mentioned in previous comments, I think this can be removed. http://gerrit.cloudera.org:8080/#/c/15681/13/src/kudu/integration-tests/master_authz-itest.cc@554 PS13, Line 554: policy_new_table.tables.emplace_back("*"); This can be removed as well. http://gerrit.cloudera.org:8080/#/c/15681/13/src/kudu/ranger/ranger_client.cc File src/kudu/ranger/ranger_client.cc: http://gerrit.cloudera.org:8080/#/c/15681/13/src/kudu/ranger/ranger_client.cc@573 PS13, Line 573: if (allowed_actions.empty()) { : LOG(WARNING) << Substitute("User $0 is not authorized to perform actions $1 on table $2", : user_name, JoinMapped(*actions, ActionPB_Name, ", "), table_name); : return Status::NotAuthorized(kUnauthorizedAction); : } Should we remove this and let ranger_authz_provider handle it as well? -- To view, visit http://gerrit.cloudera.org:8080/15681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I25dc67516cd61f0624914989f8db4c4f94d7e3bf Gerrit-Change-Number: 15681 Gerrit-PatchSet: 13 Gerrit-Owner: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 09 Apr 2020 05:52:14 +0000 Gerrit-HasComments: Yes
