Alexey Serbin has posted comments on this change.

Change subject: WIP: generate self-signed certs on server startup, remove 
server_cert_manager
......................................................................


Patch Set 1:

(6 comments)

some nits so far.

It seems you are working on revision 2, right?

http://gerrit.cloudera.org:8080/#/c/5955/1/src/kudu/security/tls_context.cc
File src/kudu/security/tls_context.cc:

PS1, Line 210: Signed
nit: consider dropping 'Signed' since we don't have non-signed ones.


PS1, Line 230: signed 
nit: drop this


PS1, Line 241: signed 
nit: drop this


PS1, Line 243: LOG(INFO) << "resetting CSR";
nit: probably, this is just debug log, i.e. use VLOG() or something


PS1, Line 261: AddTrustedRootCertificate
Does this work for intermediate CA certs?


http://gerrit.cloudera.org:8080/#/c/5955/1/src/kudu/tserver/heartbeater.cc
File src/kudu/tserver/heartbeater.cc:

PS1, Line 426: AdoptSignedCert
naming nit while you are at it: why 'Signed'?  I think in all cases we have 
only signed certificates.  Consider dropping 'Signed', so it would be just 
'AdoptCert(cert)'


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie785cc80d1cd8275defa3987f8e2a3bbcae02622
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to