Todd Lipcon has posted comments on this change. Change subject: master: complete hooking up tokens and IPKI ......................................................................
Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/6075/3/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: Line 790: "couldn't self-sign master cert with CA cert"); > self-sign here is a little misleading, probably best to just use 'sign'. Done PS3, Line 781: auto* tls = master_->mutable_tls_context(); : RETURN_NOT_OK_PREPEND(tls->AddTrustedCertificate(ca->ca_cert()), : "could not trust master CA cert"); : // If we haven't signed our own server cert yet, do so. : boost::optional<security::CertSignRequest> csr = : tls->GetCsrIfNecessary(); : if (csr) { : Cert cert; : RETURN_NOT_OK_PREPEND(ca->SignServerCSR(*csr, &cert), : "couldn't self-sign master cert with CA cert"); : RETURN_NOT_OK_PREPEND(tls->AdoptSignedCert(cert), : "couldn't adopt signed master cert"); > hm, interesting idea.. although this would be the first place in which foll We discussed this on slack and came to the conclusion that having the signed cert without having the TSKs is not helpful. I moved this code into LoadCertAuthorityInfo() http://gerrit.cloudera.org:8080/#/c/6075/3/src/kudu/security/tls_context.cc File src/kudu/security/tls_context.cc: Line 157: return Status::OK(); > nit: does it make sense to add a log about an attempt to add already presen this actually happens every time we reconnect to the cluster in the client, so not worth logging IMO. -- To view, visit http://gerrit.cloudera.org:8080/6075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3dfadb427491c7b406ad2d2bc1245b3a1cdb9170 Gerrit-PatchSet: 3 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-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
