Jun, Thank you, I will start a vote.
On Tue, Jun 7, 2016 at 8:49 PM, Jun Rao <j...@confluent.io> wrote: > Rajini, > > Thanks for the updated wiki. It looks good to me. Do you want to start a > vote on this? > > Jun > > On Fri, May 27, 2016 at 11:47 AM, Rajini Sivaram < > rajinisiva...@googlemail.com> wrote: > > > Jun, > > > > Thank you for the review and the suggestions. I have updated the KIP with > > the changes you suggested. > > > > > > On Fri, May 27, 2016 at 5:24 PM, Jun Rao <j...@confluent.io> wrote: > > > > > Rajini, > > > > > > Thanks for the updated KIP. Looks good overall. Just a few minor > > comments. > > > > > > 10. For quota related metric names, currently, they already have a tag > > > "client-d". It seems that we can just replace it with a similar tag > > "user" > > > if quota.type is user. The sensor names only have the client-id value. > We > > > can generalize it to something like quota.type-type.value. Note that > only > > > the metric names affect the jmx names, not the sensors. > > > > > > 11. For the value that we store in ZK for a user level quota, would it > be > > > better to include the user name? Since the path is base64, this could > > make > > > it easier to see what the actual user is associated with the path. > > > > > > 12. For values for quota.type, perhaps we can use "client-id" and > "user"? > > > > > > Jun > > > > > > > > > > > > On Wed, May 25, 2016 at 12:29 PM, Rajini Sivaram < > > > rajinisiva...@googlemail.com> wrote: > > > > > > > Hi Aditya, > > > > > > > > Thank you for the review. When *quota.type=user*, quotas are based on > > the > > > > user principal which may be an authenticated or unauthenticated user. > > For > > > > PLAINTEXT, the principal would be "*anonymous*" by default, but can > be > > > > overridden by supplying a principal builder. Quotas can be applied > to " > > > > *anonymous*". For example, in a cluster that has both PLAINTEXT and > > SSL, > > > > you can limit the quota for PLAINTEXT by specifying a quota for > > > > "*anonymous*". > > > > With PLAINTEXT, you can limit the quota for traffic from an IP > address > > by > > > > setting principal using a custom principal builder and specifying > quota > > > for > > > > the principal. > > > > > > > > I used "*clients*" for *quota.type* since the term was already used > as > > > > ConfigType to set quotas for client-ids in kaka-configs.sh and in the > > > > Zookeeper path. But I understand that clients/users can be confusing. > > How > > > > about client-id and 'user-principal'? > > > > > > > > Regards, > > > > > > > > Rajini > > > > > > > > On Wed, May 25, 2016 at 6:16 PM, Aditya Auradkar < > > > > aaurad...@linkedin.com.invalid> wrote: > > > > > > > > > Hey Rajini - > > > > > > > > > > If the quota.type is set to 'user', what happens to unauthenticated > > > > > clients? They don't supply a principal, so are they essentially > > > > > unthrottled? > > > > > > > > > > This may be a nit, but I prefer 'quota.type' options to be > > > > > 'authenticated-user' and 'client-id' as opposed to 'client' and > > 'user'. > > > > For > > > > > a new user, the options 'client' and 'user' sound essentially the > > same. > > > > > > > > > > Aditya > > > > > > > > > > On Tue, May 24, 2016 at 5:55 AM, Rajini Sivaram < > > > > > rajinisiva...@googlemail.com> wrote: > > > > > > > > > > > Jun, > > > > > > > > > > > > I have updated the KIP based on your suggestion. Can you take a > > look? > > > > > > > > > > > > Thank you, > > > > > > > > > > > > Rajini > > > > > > > > > > > > On Tue, May 24, 2016 at 11:20 AM, Rajini Sivaram < > > > > > > rajinisiva...@googlemail.com> wrote: > > > > > > > > > > > > > Jun, > > > > > > > > > > > > > > Thank you for the review. I agree that a simple user principal > > > based > > > > > > quota > > > > > > > is sufficient to allocate broker resources fairly in a > multi-user > > > > > system. > > > > > > > Hierarchical quotas proposed in the KIP currently enables > clients > > > of > > > > a > > > > > > user > > > > > > > to be rate-limited as well. This allows a user to run multiple > > > > clients > > > > > > > which don't interfere with each other's quotas. Since there is > no > > > > clear > > > > > > > requirement to support this at the moment, I am happy to limit > > the > > > > > scope > > > > > > of > > > > > > > the KIP to a single-level user-based quota. Will update the KIP > > > > today. > > > > > > > > > > > > > > Regards, > > > > > > > > > > > > > > Rajini > > > > > > > > > > > > > > On Mon, May 23, 2016 at 5:24 PM, Jun Rao <j...@confluent.io> > > wrote: > > > > > > > > > > > > > >> Rajini, > > > > > > >> > > > > > > >> Thanks for the KIP. When we first added the quota support, the > > > > > intention > > > > > > >> was to be able to add a quota per application. Since at that > > time, > > > > we > > > > > > >> don't > > > > > > >> have security yet. We essentially simulated users with > > client-ids. > > > > Now > > > > > > >> that > > > > > > >> we do have security. It seems that we just need to have a way > to > > > set > > > > > > quota > > > > > > >> at the user level. Setting quota at the combination of users > and > > > > > > >> client-ids > > > > > > >> seems more complicated and I am not sure if there is a good > use > > > > case. > > > > > > >> > > > > > > >> Also, the new config quota.secure.enable seems a bit weird. > > Would > > > it > > > > > be > > > > > > >> better to add a new config quota.type. It defaults to clientId > > for > > > > > > >> backward > > > > > > >> compatibility. If one sets it to user, then the default broker > > > level > > > > > > quota > > > > > > >> is for users w/o a customized quota. In this setting, brokers > > will > > > > > also > > > > > > >> only take quota set at the user level (i.e., quota set at > > clientId > > > > > level > > > > > > >> will be ignored). > > > > > > >> > > > > > > >> Thanks, > > > > > > >> > > > > > > >> Jun > > > > > > >> > > > > > > >> On Tue, May 3, 2016 at 4:32 AM, Rajini Sivaram < > > > > > > >> rajinisiva...@googlemail.com > > > > > > >> > wrote: > > > > > > >> > > > > > > >> > Ewen, > > > > > > >> > > > > > > > >> > Thank you for the review. I agree that ideally we would have > > one > > > > > > >> definition > > > > > > >> > of quotas that handles all cases. But I couldn't quite fit > all > > > the > > > > > > >> > combinations that are possible today with client-id-based > > quotas > > > > > into > > > > > > >> the > > > > > > >> > new configuration. I think upgrade path is not bad since > > quotas > > > > are > > > > > > >> > per-broker. You can configure quotas based on the new > > > > configuration, > > > > > > set > > > > > > >> > quota.secure.enable=true and restart the broker. Since there > > is > > > no > > > > > > >> > requirement for both insecure client-id based quotas and > > secure > > > > > > >> user-based > > > > > > >> > quotas to co-exist in a cluster, isn't that sufficient? The > > > > > > >> implementation > > > > > > >> > does use a unified approach, so if an alternative > > configuration > > > > can > > > > > be > > > > > > >> > defined (perhaps with some acceptable limitations?) which > can > > > > > express > > > > > > >> both, > > > > > > >> > it will be easy to implement. Suggestions welcome :-) > > > > > > >> > > > > > > > >> > The cases that the new configuration cannot express, but the > > old > > > > one > > > > > > can > > > > > > >> > are: > > > > > > >> > > > > > > > >> > 1. SSL/SASL with multiple users, same client ids used by > > > > multiple > > > > > > >> users, > > > > > > >> > client-id based quotas where quotas are shared between > > > multiple > > > > > > users > > > > > > >> > 2. Default quotas for client-ids. In the new > configuration, > > > > > default > > > > > > >> > quotas are defined for users and clients with no > configured > > > > > > sub-quota > > > > > > >> > share > > > > > > >> > the user's quota. > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > > >> > On Sat, Apr 30, 2016 at 6:21 AM, Ewen Cheslack-Postava < > > > > > > >> e...@confluent.io> > > > > > > >> > wrote: > > > > > > >> > > > > > > > >> > > Rajini, > > > > > > >> > > > > > > > > >> > > I'm admittedly not very familiar with a lot of this code > or > > > > > > >> > implementation, > > > > > > >> > > so correct me if I'm making any incorrect assumptions. > > > > > > >> > > > > > > > > >> > > I've only scanned the KIP, but my main concern is the > > > rejection > > > > of > > > > > > the > > > > > > >> > > alternative -- unifying client-id and principal quotas. In > > > > > > particular, > > > > > > >> > > doesn't this make an upgrade for brokers using those > > different > > > > > > >> approaches > > > > > > >> > > difficult since you have to make a hard break between > > > client-id > > > > > and > > > > > > >> > > principal quotas? If people adopt client-id quotas to > begin > > > > with, > > > > > it > > > > > > >> > seems > > > > > > >> > > like we might not be providing a clean upgrade path. > > > > > > >> > > > > > > > > >> > > As I said, I haven't kept up to date with the details of > the > > > > > > security > > > > > > >> and > > > > > > >> > > quota features, but I'd want to make sure we didn't > suggest > > > one > > > > > path > > > > > > >> with > > > > > > >> > > 0.9, then add another that we can't provide a clean > upgrade > > > path > > > > > to. > > > > > > >> > > > > > > > > >> > > -Ewen > > > > > > >> > > > > > > > > >> > > On Fri, Apr 22, 2016 at 7:22 AM, Rajini Sivaram < > > > > > > >> > > rajinisiva...@googlemail.com> wrote: > > > > > > >> > > > > > > > > >> > > > The PR for KAFKA-3492 ( > > > > > https://github.com/apache/kafka/pull/1256) > > > > > > >> > > contains > > > > > > >> > > > the code associated with KIP-55. I will keep it updated > > > during > > > > > the > > > > > > >> > review > > > > > > >> > > > process. > > > > > > >> > > > > > > > > > >> > > > Thanks, > > > > > > >> > > > > > > > > > >> > > > Rajini > > > > > > >> > > > > > > > > > >> > > > On Mon, Apr 18, 2016 at 4:41 PM, Rajini Sivaram < > > > > > > >> > > > rajinisiva...@googlemail.com> wrote: > > > > > > >> > > > > > > > > > >> > > > > Hi All, > > > > > > >> > > > > > > > > > > >> > > > > I have just created KIP-55 to support quotas based on > > > > > > >> authenticated > > > > > > >> > > user > > > > > > >> > > > > principals. > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-55%3A+Secure+Quotas+for+Authenticated+Users > > > > > > >> > > > > > > > > > > >> > > > > Comments and feedback are appreciated. > > > > > > >> > > > > > > > > > > >> > > > > Thank you... > > > > > > >> > > > > > > > > > > >> > > > > Regards, > > > > > > >> > > > > > > > > > > >> > > > > Rajini > > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > -- > > > > > > >> > > > Regards, > > > > > > >> > > > > > > > > > >> > > > Rajini > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > -- > > > > > > >> > > Thanks, > > > > > > >> > > Ewen > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > > >> > > > > > > > >> > -- > > > > > > >> > Regards, > > > > > > >> > > > > > > > >> > Rajini > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > Regards, > > > > > > > > > > > > > > Rajini > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > Regards, > > > > > > > > > > > > Rajini > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > Regards, > > > > > > > > Rajini > > > > > > > > > > > > > > > -- > > Regards, > > > > Rajini > > > -- Regards, Rajini
