Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11797 )
Change subject: [sentry] Integrate AuthzProvider into CatalogManager ...................................................................... Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/11797/7/src/kudu/master/catalog_manager.cc@1713 PS7, Line 1713: optional<const string&> user = rpc ? : make_optional<const string&>(rpc->remote_user().username()) : none; > Yeah, it is only for tests. However, I don't see an easy way of defining du One option might be to pass around the optional 'user' instead of 'rpc', since AFAICT that's all we use it for. It still pulls some test-only code in here, but that way at least this clutter is only in FindLockAndAuthorizeTable. I know there are calls that already have 'rpc' as a part of their signature, so changing those might be unwarranted, but for the new ones (e.g. GetTableLocations, IsCreateTableDone, ListTables, GetTabletLocations, GetTableSchema), seems like we don't need the rest of the contexts? IIUC, functions of master_service.cc would not have null RpcContexts and _should_ have remote users. wdyt? -- To view, visit http://gerrit.cloudera.org:8080/11797 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iab4aa027ae6eb4520db48ce348db552c9feec2a8 Gerrit-Change-Number: 11797 Gerrit-PatchSet: 10 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 07 Mar 2019 22:29:24 +0000 Gerrit-HasComments: Yes
