Todd Lipcon has posted comments on this change. Change subject: [TLS cert management] added protobuf interface ......................................................................
Patch Set 1: (10 comments) http://gerrit.cloudera.org:8080/#/c/5673/1/src/kudu/security/security.proto File src/kudu/security/security.proto: Line 52: DER = 0; should have an UNKNOWN = 999 first PS1, Line 50: // X509 formats. : enum DataFormat { : DER = 0; : PEM = 1; : } per comment elsewhere, dunno if it's really advantageous to support both... though looking at the Java libraries it seems like it only supports PKCS7 and DER, not PEM. PS1, Line 63: X509CertSigningRequestPB I think to clarify that this isn't a normal 'RequestPB' (i.e. an RPC request) we should call this X509CsrPB Line 112: optional Error error = 1; If you're using the 'extend ErrorStatusPB' thing above, you don't need to include this in your PB response. PS1, Line 114: prior/current/next This makes it sound like there will always be three valid ones. From the client perspective, do we care, or is it just 'all valid CA certs'? Line 134: enum Code { instead of separate codes for each method, might be better to just have a PB and set of codes for all CA-related errors Line 171: optional Error error = 1; see above PS1, Line 174: correspoding typo PS1, Line 184: authentity typo PS1, Line 187: Get prior, current and next root CA certificates. At every moment, : // only one pair of (private key, CA cert) is active: see above -- To view, visit http://gerrit.cloudera.org:8080/5673 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9ff31e56be42bfa8d0f9b908ba2ccd2734407f55 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
