Sailesh Mukil has posted comments on this change. Change subject: KUDU-1929: [rpc] Allow using encrypted private keys for TLS ......................................................................
Patch Set 7: (7 comments) Thanks for the review! http://gerrit.cloudera.org:8080/#/c/6635/6/src/kudu/rpc/messenger.cc File src/kudu/rpc/messenger.cc: PS6, Line 98: the RPC server's private key " : "file specif > I think it should just be 'the RPC server's private key file' - it doesn't Done Line 303: [&](){ > Could this closure be '[&](){' in order to take the flags by reference? Good idea. Done. http://gerrit.cloudera.org:8080/#/c/6635/6/src/kudu/rpc/rpc-test.cc File src/kudu/rpc/rpc-test.cc: Line 155: // Test making successful RPC calls while using a TLS certificate with a password protected > Update comment Done Line 162: string passwd; > Might be worth adding an attempt at passing an incorrect password, just to I wanted to add that but unfortunately StartTestServer() doesn't return a Status, so providing a wrong password would fail an assert inside DoStartTestServer() as it always expects to succeed and has ASSERT_OK() checks for the same. So basically there's no way to test that starting the server doesn't work without breaking the test unless I add a function like DoStartTestServerExpectFail() or something. What do you think? Line 183: } > extra newline Done http://gerrit.cloudera.org:8080/#/c/6635/6/src/kudu/security/tls_context.cc File src/kudu/security/tls_context.cc: Line 423: const PasswordCallback& password_cb) { > Please add Done PS6, Line 430: "failed to load private key file"); : RETURN_NOT_OK(UseCertificateAndKey(c, k)); : is_extern > nit: might this be just Done -- To view, visit http://gerrit.cloudera.org:8080/6635 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifd6369581fa426ceab11e4a10441658c7da47e81 Gerrit-PatchSet: 7 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
