Gwen/Jay, Have you had a chance to look at the updated KIP? It will be good to get your feedback as well before restarting vote on the updated KIP.
If there are no objections, I will start the vote tomorrow. On Fri, Jun 17, 2016 at 6:59 PM, Rajini Sivaram < rajinisiva...@googlemail.com> wrote: > Thank you, Jun. I have removed user_principal from the KIP. > > On Fri, Jun 17, 2016 at 6:00 PM, Jun Rao <j...@confluent.io> wrote: > >> Rajini, >> >> 10. Yes, then we can probably leave out the user_principal field and keep >> the version to be 1. >> >> Other than that, the KIP looks good to me. >> >> Thanks, >> >> Jun >> >> On Fri, Jun 17, 2016 at 3:29 AM, Rajini Sivaram < >> rajinisiva...@googlemail.com> wrote: >> >> > 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 >> > >> > > > > -- > Regards, > > Rajini > -- Regards, Rajini