Todd Lipcon has posted comments on this change. Change subject: [security] tailored TokenSigner for system catalog ......................................................................
Patch Set 11: (6 comments) http://gerrit.cloudera.org:8080/#/c/5930/10/src/kudu/security/token_signer.cc File src/kudu/security/token_signer.cc: Line 62: int64_t key_pre_exp_rotation_seconds, > In essence we need to make sure that the public part of the new key appears hm, just saw Dan's comment after writing basically the same comment. I still think we could simplify and just use rotation = propagation interval. Otherwise there are a lot of constraints to think about (eg rotation interval must be greater than propagation interval) http://gerrit.cloudera.org:8080/#/c/5930/10/src/kudu/security/token_signer.h File src/kudu/security/token_signer.h: Line 188: // > Yep, that's a point to discuss. From one side, in the case of the master I yea, I think the RPC layer would be treating it read-only (just to verify tokens) http://gerrit.cloudera.org:8080/#/c/5930/11/src/kudu/security/token_signer.h File src/kudu/security/token_signer.h: PS11, Line 50: assigned nit: assigned a PS11, Line 103: // propagation interval: 1 day it's a bit odd that propagation = rotation interval in this diagram. Although come to think of it, I'm not entirely sure what the propagation interval is for -- could we get rid of it and just always set propagation interval = rotation interval? I think that was the original design described in this comment, and I don't see any reason to need to customize it, but maybe I'm missing something PS11, Line 118: perios typo PS11, Line 223: indended typo: intended -- To view, visit http://gerrit.cloudera.org:8080/5930 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie2417e2ccba6a1114db366b2f642f95362bf479c Gerrit-PatchSet: 11 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <[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
