Todd Lipcon has posted comments on this change.

Change subject: [security]  security-flags
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/6052/2//COMMIT_MSG
Commit Message:

PS2, Line 20: A follow up commit will hook this flag
            :     into the RPC system to ensure that authentication is enforced 
as
            :     necessary.
keep in mind that clients don't have the ability to configure flags, so we'll 
need to make it a KuduClientBuilder parameter in those cases. (also to allow a 
single client to talk to multiple clusters).

(same below)


PS2, Line 42: 1) It's not strictly a server flag, it also applies to
            :     the kudu CLI tool.
how does it apply to the CLI tool? Given that we don't currently support 
client-cert authentication for Kudu (BYOPKI), I'm surprised that this is 
respected by the CLI.

Actually, I think that we should probably remove (or mark experimental/hidden) 
all of these flags until we officially support and test the BYOPKI option). 
Their main purpose today is to enable Impala, and if we removed them and 
provided APIs, Impala can define them in their own server code and pass them 
into the Messenger at startup (like we do with the IPKI keys/certs)


PS2, Line 43:  2) It's really long, and the length doesn't add
            :     useful description or specificity. 3) The short form (cert 
instead
            :     of certificate) is common in database CLI configs [1], [2], 
[3].
> I mentioned this last night in #kudu-security on Slack, but I'll repeat it 
yea, I agree that 'rpc_key' isn't very descriptive. At the least I'd expect 
'rpc_tls_key' or 'rpc_ssl_key', since who knows what other kinds of keys might 
exist.

Again, though, may be a moot point since we aren't allowing end users to 
configure these things.


Line 58: --webserver_certificate_file
this and the below two were chosen for compatibility with Impala. Obviously 
it's not a strict requirement that we be configured identically, but I think 
it's easier for operators to deal with if we are.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa53348b8969e83d9f794e1e0553bdec12252d9a
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Dan Burkert <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to