Jun, 10. Since entity_type "users" is new, shouldn't the JSON for these entities have version 1? I have moved "user_principal" out of the config in the samples and added to the <user, client> entries as well. But actually, do we need to store the non-encoded principal at all? The node name is URL-encoded user principal, so it is fairly readable if you are looking in ZK and *kafka_configs.sh* will show the non-encoded principal by decoding the name from the path (since it needs to do encoding anyway because the names specified on the command line will be non-encoded principals, it can do decoding too). Perhaps that is sufficient?
11. I liked the second approach since it looks neat and future-proof. Have updated the KIP. 12. Yes, that is correct. Many thanks, Rajini On Thu, Jun 16, 2016 at 11:36 PM, Jun Rao <j...@confluent.io> wrote: > Rajini, > > Thanks for the update. A few more questions/comments. > > 10. For the quota value stored in ZK, since we are adding an optional > user_principal field in the json, we should bump the version from 1 to 2. > Also, user_principal is not really part of the config values. So, perhaps > we should represent it as the following? > { > "version":2, > "config": { > "producer_byte_rate":"1024", > "consumer_byte_rate":"2048" > }, > "user_principal" : "user1" > } > > Also, we should store user_principal in the following json too, right? > // Zookeeper persistence path /users/<encoded-user2>/clients/clientA > { > "version":1, > "config": { > "producer_byte_rate":"10", > "consumer_byte_rate":"30" > } > } > > 11. For the change notification path, would it be better to change it to > something like the following and bump up version to 2? > // Change notification for quota of <user2, clientA> > { > "version":2, > [ > { "entity_type": "users", > "entity_name": "user2" > }, > { "entity_type": "clients", > "entity_name": "clientA" > } > ] > } > > Alternatively, we could change it to > // Change notification for quota of <user2, clientA> > { > "version":2, > "entity_path": "users/user2" > } > > { > "version":2, > "entity_path": "users/user2/clients/clientA" > } > > 12. Just to clarify on the meaning of remainder quota. If you have quotas > like the following, > <user1, client1> = 5 > <user1, client2> = 10 > <user1> = 12 > it means that all connections with user1 whose client-id is neither client1 > nor client2 will be sharing a quota of 12, right? In other words, the quota > of <user1> doesn't include the quota for <user1, client1> and <user1, > client2>. > > Thanks, > > Jun > > > On Thu, Jun 16, 2016 at 5:03 AM, Rajini Sivaram < > rajinisiva...@googlemail.com> wrote: > > > Jun, > > > > Actually, with quotas stored in different nodes in ZK, it is better to > > store remainder quota rather than total quota under /users/<user> so that > > quota calculations are not dependent on the order of notifications. I > have > > updated the KIP to reflect that. So the quotas in ZK now always reflect > the > > quota applied to a group of client connections and use the same format as > > client-id quotas. But it is not hierarchical, making the configuration > > simpler. > > > > On Thu, Jun 16, 2016 at 11:49 AM, Rajini Sivaram < > > rajinisiva...@googlemail.com> wrote: > > > > > Jun, > > > > > > Thank you for the review. I have updated the KIP: > > > > > > > > > 1. Added an overview section. Slightly reworded since it is better > to > > > treat user and client-id as different levels rather than the same > > level. > > > 2. Yes, it is neater to store quota for each entity in a different > > > path in Zookeeper. I put clients under users rather than the other > way > > > round since that reflects the hierarchy and also keeps a user's > quotas > > > together under a single sub-tree. I had initially used a single node > > to > > > keep quotas and sub-quotas of a user together so that updates are > > atomic > > > since changes to sub-quotas also affect remainder quotas for other > > clients. > > > But I imagine, updates to configs are rare and it is not a big > issue. > > > 3. I haven't modified the JSON for configuration change > notifications. > > > The entity_name can now be a subpath that has both user and client. > > Have > > > added an example to the KIP. The downside of keeping clients under > > users in > > > ZK in 2) is that the change notification for sub-quota has > entity_type > > > "users". I could extend the JSON to include client separately, but > > since > > > changes to a client sub-quota does impact other clients of the user > > as well > > > due to change in remainder quota, it may be ok as it is. Do let me > > know if > > > it looks confusing in the example. > > > 4. Agree, updated. > > > > > > > > > On Wed, Jun 15, 2016 at 10:27 PM, Jun Rao <j...@confluent.io> wrote: > > > > > >> Hi, Rajini, > > >> > > >> Thanks for the updated wiki. Overall, I like the new approach. It > covers > > >> the common use cases now, is extensible, and is backward compatible. A > > few > > >> comments below. > > >> > > >> 1. It would be useful to describe a bit at the high level, how the new > > >> approach works. I think this can be summarized as follows. Quotas can > be > > >> set at user, client-id or <user, client-id> levels. For a given client > > >> connection, the most specific quota matching the connection will be > > >> applied. For example, if both a <user, client-id> and a <user> quota > > match > > >> a connection, the <user, client-id> quota will be used. If more than 1 > > >> quota at the same level (e.g., a quota on a user and another quota on > a > > >> client-id) match the connection, the user level quota will be used, > > i.e., > > >> user quota takes precedence over client-id quota. > > >> > > >> 2. For the ZK data structure, would it be better to store <user, > > >> client-id> > > >> quota as the following. Then the format of the value in each path is > the > > >> same. The wiki also mentions that we want to include the original user > > >> name > > >> in the ZK value. Could you describe that in an example? > > >> // Zookeeper persistence path /clients/clientA/users/<encoded-user2> > > >> { > > >> "version":1, > > >> "config": { > > >> "producer_byte_rate":"10", > > >> "consumer_byte_rate":"20" > > >> } > > >> } > > >> > > >> 3. Could you document the format change of the ZK value in > > >> /config/changes/config_change_xxx, if any? > > >> > > >> 4. For the config command, could we specify the sub-quota like the > > >> following, instead of in the config value? This seems more intuitive. > > >> > > >> bin/kafka-configs --zookeeper localhost:2181 --alter --add-config > > >> 'producer_byte_rate=1024,consumer_byte_rate=2048' --entity-name > > >> clientA --entity-type clients --entity-name user2 --entity-type users > > >> > > >> Thanks, > > >> > > >> Jun > > >> > > >> On Wed, Jun 15, 2016 at 10:35 AM, Rajini Sivaram < > > >> rajinisiva...@googlemail.com> wrote: > > >> > > >> > Harsha, > > >> > > > >> > The sample configuration under > > >> > > > >> > > > >> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-55%3A+Secure+Quotas+for+Authenticated+Users#KIP-55:SecureQuotasforAuthenticatedUsers-QuotaConfiguration > > >> > shows > > >> > the Zookeeper data for different scenarios. > > >> > > > >> > - *user1* (/users/user1 in ZK) has only user-level quotas > > >> > - *user2* (/users/user2 in ZK) defines user-level quotas and > > >> sub-quotas > > >> > for clients *clientA* and *clientB*. Other client-ids of *user2* > > >> share > > >> > the remaining quota of *user2*. > > >> > > > >> > > > >> > On Wed, Jun 15, 2016 at 5:30 PM, Harsha <ka...@harsha.io> wrote: > > >> > > > >> > > Rajini, > > >> > > How does sub-quotas works in case of authenticated > users. > > >> > > Where are we maintaining the relation between users and > > >> their > > >> > > client Ids. Can you add an example of zk data under > > /users. > > >> > > Thanks, > > >> > > Harsha > > >> > > > > >> > > On Mon, Jun 13, 2016, at 05:01 AM, Rajini Sivaram wrote: > > >> > > > I have updated KIP-55 to reflect the changes from the > discussions > > in > > >> > the > > >> > > > voting thread ( > > >> > > > https://www.mail-archive.com/dev@kafka.apache.org/msg51610.html > ). > > >> > > > > > >> > > > Jun/Gwen, > > >> > > > > > >> > > > Existing client-id quotas will be used as default client-id > quotas > > >> for > > >> > > > users when no user quotas are configured - i.e., default user > > quota > > >> is > > >> > > > unlimited and no user-specific quota override is specified. This > > >> > enables > > >> > > > user rate limits to be configured for ANONYMOUS if required in a > > >> > cluster > > >> > > > that has both PLAINTEXT and SSL/SASL. By default, without any > user > > >> rate > > >> > > > limits set, rate limits for client-ids will apply, retaining the > > >> > current > > >> > > > client-id quota configuration for single-user clusters. > > >> > > > > > >> > > > Zookeeper will have two paths /clients with client-id quotas > that > > >> apply > > >> > > > only when user quota is unlimited similar to now. And /users > which > > >> > > > persists > > >> > > > user quotas for any user including ANONYMOUS. > > >> > > > > > >> > > > Comments and feedback are appreciated. > > >> > > > > > >> > > > Regards, > > >> > > > > > >> > > > Rajini > > >> > > > > > >> > > > > > >> > > > On Wed, Jun 8, 2016 at 9:00 PM, Rajini Sivaram > > >> > > > <rajinisiva...@googlemail.com > > >> > > > > wrote: > > >> > > > > > >> > > > > Jun, > > >> > > > > > > >> > > > > Oops, sorry, I hadn't realized that the last note was on the > > >> discuss > > >> > > > > thread. Thank you for pointing it out. I have sent another > note > > >> for > > >> > > voting. > > >> > > > > > > >> > > > > > > >> > > > > On Wed, Jun 8, 2016 at 4:30 PM, Jun Rao <j...@confluent.io> > > wrote: > > >> > > > > > > >> > > > >> Rajini, > > >> > > > >> > > >> > > > >> Perhaps it will be clearer if you start the voting in a new > > >> thread > > >> > > (with > > >> > > > >> VOTE in the subject). > > >> > > > >> > > >> > > > >> Thanks, > > >> > > > >> > > >> > > > >> Jun > > >> > > > >> > > >> > > > >> On Tue, Jun 7, 2016 at 1:55 PM, Rajini Sivaram < > > >> > > > >> rajinisiva...@googlemail.com > > >> > > > >> > wrote: > > >> > > > >> > > >> > > > >> > I would like to initiate the vote for KIP-55. > > >> > > > >> > > > >> > > > >> > The KIP details are here: KIP-55: Secure quotas for > > >> authenticated > > >> > > users > > >> > > > >> > < > > >> > > > >> > > > >> > > > >> > > >> > > > > >> > > > >> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-55%3A+Secure+Quotas+for+Authenticated+Users > > >> > > > >> > > > > >> > > > >> > . > > >> > > > >> > > > >> > > > >> > The JIRA KAFKA-3492 < > > >> > > https://issues.apache.org/jira/browse/KAFKA-3492 > > >> > > > >> > >has > > >> > > > >> > a draft PR here: https://github.com/apache/kafka/pull/1256 > . > > >> > > > >> > > > >> > > > >> > Thank you... > > >> > > > >> > > > >> > > > >> > > > >> > > > >> > Regards, > > >> > > > >> > > > >> > > > >> > Rajini > > >> > > > >> > > > >> > > > >> > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > -- > > >> > > > > Regards, > > >> > > > > > > >> > > > > Rajini > > >> > > > > > > >> > > > > > >> > > > > > >> > > > > > >> > > > -- > > >> > > > Regards, > > >> > > > > > >> > > > Rajini > > >> > > > > >> > > > >> > > > >> > > > >> > -- > > >> > Regards, > > >> > > > >> > Rajini > > >> > > > >> > > > > > > > > > > > > -- > > > Regards, > > > > > > Rajini > > > > > > > > > > > -- > > Regards, > > > > Rajini > > > -- Regards, Rajini