Alexey Serbin has posted comments on this change. Change subject: rpc: add basic service and method-level authorization ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/4897/1/src/kudu/rpc/protoc-gen-krpc.cc File src/kudu/rpc/protoc-gen-krpc.cc: PS1, Line 23: #include <boost/optional.hpp> : #include <ctype.h> : #include <glog/logging.h> : #include <google/protobuf/compiler/code_generator.h> : #include <google/protobuf/compiler/plugin.h> : #include <google/protobuf/descriptor.h> : #include <google/protobuf/descriptor.pb.h> : #include <google/protobuf/io/printer.h> : #include <google/protobuf/io/zero_copy_stream.h> : #include <google/protobuf/stubs/common.h> : #include <iostream> : #include <map> : #include <memory> : #include <sstream> : #include <string> While you are at it, could you please re-organize those in accordance with the styling guide? https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes http://gerrit.cloudera.org:8080/#/c/4897/1/src/kudu/rpc/rpc_header.proto File src/kudu/rpc/rpc_header.proto: PS1, Line 262: // An option to set the authorization method for this particular : // RPC method. Does it make sense to add a comment that if the authz method is not specified, then access is allowed at this level by default? http://gerrit.cloudera.org:8080/#/c/4897/1/src/kudu/rpc/service_if.cc File src/kudu/rpc/service_if.cc: PS1, Line 131: } It's just a tiny stylistic nit, but do you think it would be better to promote a proper pattern for handling authz method results? I think the following would be more idiomatic: if (!method_info->authz_method(ctx->request_pb(), resp, ctx)) { return; } method_info->func(ctx->request_pb(), resp, ctx); -- To view, visit http://gerrit.cloudera.org:8080/4897 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia9206f5f89391d8bccfa30952d2b252900ab6566 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-HasComments: Yes
