Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13681 )
Change subject: KUDU-2870: use coarse-grained authz for Checksum ...................................................................... Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/13681/2/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/13681/2/src/kudu/tserver/tablet_service.cc@887 PS2, Line 887: bool TabletServiceImpl::AuthorizeChecksum(const google::protobuf::Message* req, : google::protobuf::Message* resp, : rpc::RpcContext* context) { : if (FLAGS_tserver_enforce_access_control) { : return server_->Authorize(context, ServerBase::SUPER_USER); : } : return AuthorizeClient(req, resp, context); : } > This looks a bit strange to me. Basically, it says that in case of fine-gr Right, those are similar semantics to what we do for AuthorizeListTablets. If you think it's that strange to reason about, I can remove --checksum_require_authz_token entirely. I opted not to because I didn't want to remove the tablet-server-side test coverage we have for it (i.e. that in tablet_server_authorization-test) that will still be valuable when a more complete solution is implemented. If we're not enforcing fine-grained authz, there isn't much point in restricting this to super-users. I'm hesitant to change existing behavior when fine-grained authz is enabled. Also not a huge fan of pushing this into VerifyAuthzTokenOrRespond since that's a generic method not specific to checksums. http://gerrit.cloudera.org:8080/#/c/13681/2/src/kudu/tserver/tserver_service.proto File src/kudu/tserver/tserver_service.proto: http://gerrit.cloudera.org:8080/#/c/13681/2/src/kudu/tserver/tserver_service.proto@54 PS2, Line 54: : rpc Checksum(ChecksumRe > I'm not sure I understand what this means. You mean the tserver that handl Ah you're right. I meant this more as commentary on the CLI implementation, not the service definition itself. -- To view, visit http://gerrit.cloudera.org:8080/13681 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9da21f41702da747a081ab037d75865748d981a8 Gerrit-Change-Number: 13681 Gerrit-PatchSet: 4 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 19 Jun 2019 23:33:56 +0000 Gerrit-HasComments: Yes
