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 >
