Dan Burkert has posted comments on this change. Change subject: webserver: improve SSL certificate handling ......................................................................
Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/5015/2/src/kudu/security/test/test_certs.h File src/kudu/security/test/test_certs.h: Line 28: Status CreateTestSSLCerts(const std::string& dir, Could you use instead or replace usage of CreateSSLServerCert/CreateSSLPrivateKey in rpc-test-base.h with this? http://gerrit.cloudera.org:8080/#/c/5015/2/src/kudu/server/webserver.cc File src/kudu/server/webserver.cc: Line 158: Status s = Subprocess::Call(argv, "" /* stdin */, &key_password, &stderr); Adar and I traced through the other day and found that we never shelled out in production Kudu code - is it OK to do so here? I assume it's not too bad since it's close to startup? Line 166: } else { Is this else attached to the correct if? It would make more sense to me on the inner if. PS2, Line 197: SimpleItoa consider using std::to_string http://gerrit.cloudera.org:8080/#/c/5015/2/src/kudu/util/curl_util.cc File src/kudu/util/curl_util.cc: Line 72: RETURN_NOT_OK(TranslateError(curl_easy_setopt(curl_, CURLOPT_SSL_VERIFYPEER, 0))); This might be simpler without the if block, and setting the value to veify_peer_ instead of 0. -- To view, visit http://gerrit.cloudera.org:8080/5015 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4b508cebbe6f31556e6d5a5fba5e5e9fb44cf1b9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
