Zoltan Chovan has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18469 )

Change subject: rpc: plumb JWTs into the RPC layer
......................................................................


Patch Set 9:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/18469/8/src/kudu/rpc/client_negotiation.h
File src/kudu/rpc/client_negotiation.h:

http://gerrit.cloudera.org:8080/#/c/18469/8/src/kudu/rpc/client_negotiation.h@189
PS8, Line 189:
> nit: would be nice to add a short blurb to be in-line with the rest of the
Done


http://gerrit.cloudera.org:8080/#/c/18469/8/src/kudu/rpc/client_negotiation.h@191
PS8, Line 191:                
> nit: the indent is off
Done


http://gerrit.cloudera.org:8080/#/c/18469/2/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:

http://gerrit.cloudera.org:8080/#/c/18469/2/src/kudu/rpc/server_negotiation.cc@56
PS2, Line 56: #include "kudu/util/jwt.h"
> error: 'kudu/util/jwt.h' file not found [clang-diagnostic-error]
Ack


http://gerrit.cloudera.org:8080/#/c/18469/2/src/kudu/rpc/server_negotiation.cc@701
PS2, Line 701:
> Actually, this widens DOS attack vector -- crashing a server like this is a
Done


http://gerrit.cloudera.org:8080/#/c/18469/8/src/kudu/util/jwt-util.h
File src/kudu/util/jwt-util.h:

http://gerrit.cloudera.org:8080/#/c/18469/8/src/kudu/util/jwt-util.h@97
PS8, Line 97:       jwks_uri_(std::move(jwks_uri)),
            :       is_local_file_(is_local_file) {
> nit: the indent is off
Done


http://gerrit.cloudera.org:8080/#/c/18469/2/src/kudu/util/jwt-util.h
File src/kudu/util/jwt-util.h:

http://gerrit.cloudera.org:8080/#/c/18469/2/src/kudu/util/jwt-util.h@109
PS2, Line 109: };
> warning: namespace 'kudu' ends with a comment that refers to a wrong namesp
Done


http://gerrit.cloudera.org:8080/#/c/18469/8/src/kudu/util/jwt-util.cc
File src/kudu/util/jwt-util.cc:

http://gerrit.cloudera.org:8080/#/c/18469/8/src/kudu/util/jwt-util.cc@925
PS8, Line 925:
             :
> Why not just
Done


http://gerrit.cloudera.org:8080/#/c/18469/8/src/kudu/util/jwt.h
File src/kudu/util/jwt.h:

http://gerrit.cloudera.org:8080/#/c/18469/8/src/kudu/util/jwt.h@30
PS8, Line 30:
> nit: this indent should be 2 spaces according the style guideline
Done



--
To view, visit http://gerrit.cloudera.org:8080/18469
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I252f1e597d9df4408379c3b695f266dbd7f48dcc
Gerrit-Change-Number: 18469
Gerrit-PatchSet: 9
Gerrit-Owner: Andrew Wong <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Reviewer: Zoltan Chovan <[email protected]>
Gerrit-Comment-Date: Fri, 11 Nov 2022 14:08:36 +0000
Gerrit-HasComments: Yes

Reply via email to