Zoltan Chovan has posted comments on this change. ( http://gerrit.cloudera.org:8080/24826 )
Change subject: [rpc] Add proxy user identity support ...................................................................... Patch Set 1: (5 comments) I'm not seeing any new tests added, I think it would be best to add some coverage for the new code paths http://gerrit.cloudera.org:8080/#/c/24826/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24826/1//COMMIT_MSG@18 PS1, Line 18: AuthenticationCredentialsPB gains a matching field I'm not seeing this field in this commit, src/kudu/client/client.proto is not included in this PS, however there is an effective_user field for UserInformationPB in rpc_header.proto http://gerrit.cloudera.org:8080/#/c/24826/1/src/kudu/rpc/server_negotiation.cc File src/kudu/rpc/server_negotiation.cc: http://gerrit.cloudera.org:8080/#/c/24826/1/src/kudu/rpc/server_negotiation.cc@746 PS1, Line 746: if (c_username != nullptr && std::string(c_username) != c_authuser nit: how about strcmp? it would save a cast http://gerrit.cloudera.org:8080/#/c/24826/1/src/kudu/rpc/server_negotiation.cc@746 PS1, Line 746: (c_username != nullptr is this necessary? c_username was already checked for nullptr at #712 http://gerrit.cloudera.org:8080/#/c/24826/1/src/kudu/rpc/server_negotiation.cc@751 PS1, Line 751: c_username same as above http://gerrit.cloudera.org:8080/#/c/24826/1/src/kudu/rpc/server_negotiation.cc@1170 PS1, Line 1170: ProxyPolicyCb could you add some comments explaining the reasoning why returning with an unconditional SASL_OK is fine here? As far as I understand this disables cyrus-sasl's built-in proxy check, right? -- To view, visit http://gerrit.cloudera.org:8080/24826 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3f26169f14552d2566272a32525156cd30627aa3 Gerrit-Change-Number: 24826 Gerrit-PatchSet: 1 Gerrit-Owner: mintao <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Zoltan Chovan <[email protected]> Gerrit-Comment-Date: Wed, 16 Sep 2026 10:33:41 +0000 Gerrit-HasComments: Yes
