Hi Jun, Yaodong,
21. The proposed approach sounds very hacky. User principals can contain
arbitrary characters. So we can't simply split `user1/clients/clientA` into
tokens using '/' as delimiter. Internally, we sanitize names before
storing in ZK, but the names provided by the user are actual principals and
client-ids. I think we want to have entity names that explicitly specify
(type, name) as in the CLI kafka-configs.sh and allow multiple entities to
be specified together for (user, client-id). That will also enable us to
configure defaults in a consistent way.
22. Yes, that sounds reasonable.
On Fri, Apr 5, 2019 at 11:13 PM Jun Rao <j...@confluent.io> wrote:
> Hi, Yaodong,
>
> Yes, what you proposed makes sense. A couple of more comments.
>
> 21. Could you add those examples to the KIP wiki? It would also be useful
> to know how to set the ConfigEntry value for quotas at
> DefaultClientInUser, DefaultClientDefaultUser and DefaultUser level.
>
> 22. To support KIP-257, I guess we can just pass in some special string
> value in ConfigEntry value through alterConfig and let the customized
> implementation of ClientQuotaCallback parse it. It would be useful to
> document this. Does that sound reasonable, Rajini?
>
> Thanks,
>
> Jun
>
> On Fri, Apr 5, 2019 at 2:03 PM Yaodong Yang <yangyaodon...@gmail.com>
> wrote:
>
>> Hi Jun,
>>
>> The proposal we have right now is to directly set the quota through
>> existing admin client APIs. See following examples:
>>
>> Example 1: set a user quota
>>
>> AdminClient adminClient = mock(AdminClient.class);
>> Map<ConfigResource, Config> configs = new HashMap();
>> Config config = new Config(Arrays.asList(new ConfigEntry("user1",
>> "producer_byte_rate=1024")));
>> configs.put(singletonMap(ConfigResource.USER, config));
>> adminClient.alterConfigs(configs);
>>
>>
>> Example 2: set a client id quota
>>
>> AdminClient adminClient = mock(AdminClient.class);
>> Map<ConfigResource, Config> configs = new HashMap();
>> Config config = new Config(Arrays.asList(new ConfigEntry("client1",
>> "producer_byte_rate=1024")));
>> configs.put(singletonMap(ConfigResource.CLIENT, config));
>> adminClient.alterConfigs(configs);
>>
>>
>> Example 3: set a <user, client-id> quota
>>
>> AdminClient adminClient = mock(AdminClient.class);
>> Map<ConfigResource, Config> configs = new HashMap();
>> Config config = new Config(Arrays.asList(new
>> ConfigEntry("user1/clients/client2", "producer_byte_rate=1024")));
>> configs.put(singletonMap(ConfigResource.USER, config));
>> adminClient.alterConfigs(configs);
>>
>>
>> The current KIP is orthogonal to KIP 257. It only adds a new way to store
>> the quotas in ZK through AdminClient. For customizable quotas, they will
>> also be a property in User resources or Client resources.
>>
>> Quote from *CustomQuotaCallbackTest.scala::GroupedUserQuotaCallback* in
>> the
>> codebase, “Group quotas are configured in ZooKeeper as user quotas with
>> the
>> entity name "${group}_". Default group quotas are configured in ZooKeeper
>> as user quotas with the entity name "_".”
>> In this example, they are always stored as properties in the User
>> resource,
>> the property name is “$(group)_” and “_”. The client application needs to
>> encode them correctly before store them in ZK through AdminClient, while
>> the customizedCallback needs to decode them and apply during the process
>> of
>> each request.
>>
>> The advantage of the current KIP is simple and extensible for the future
>> release. The alternative is to introduce a new set of quota related APIs
>> in
>> the AdminClient, similar to the ACL related. I'm not sure which one people
>> prefer.
>>
>> Thanks!
>> Yaodong
>>
>> On Wed, Mar 13, 2019 at 6:29 PM Jun Rao <j...@confluent.io> wrote:
>>
>> > Hi, Yaodong,
>> >
>> > Thanks for the updated KIP. A few more comments below.
>> >
>> > 11. For quotas, in addition to user and client, we need to be able to
>> set a
>> > quota for <client, user> combination. Would that be a new resource type?
>> > How would the name look like for this resource?
>> >
>> git chec
>>
>> >
>> > 12. Similarly, to support customizable quota (
>> >
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-257+-+Configurable+Quota+Management
>> > ),
>> > do we need a new resource type? What would the name of the resource
>> looks
>> > like?
>> >
>> > 13. You only listed 2 new ConfigSource. Could you list all the new ones?
>> > For example,
>> >
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-248+-+Create+New+ConfigCommand+That+Uses+The+New+AdminClient
>> > listed a few others such as ClientInUser, DefaultClientInUser.
>> >
>> > 14. Could you list any new ACL that might be required? For example, what
>> > types of operations are allowed for the new Resource (User, Client,
>> etc)?
>> > What are the permissions needed for the new operations?
>> >
>> > 15. Could you give a few examples on how to use the new API?
>> >
>> > Thanks,
>> >
>> > Jun
>> >
>> > On Tue, Mar 12, 2019 at 9:56 AM Yaodong Yang <yangyaodon...@gmail.com>
>> > wrote:
>> >
>> > > ping...
>> > >
>> > > On Thu, Feb 28, 2019 at 10:23 AM Yaodong Yang <
>> yangyaodon...@gmail.com>
>> > > wrote:
>> > >
>> > >> Hello folks,
>> > >>
>> > >> Please share your comments for this KIP 😄
>> > >>
>> > >> Thanks!
>> > >> Yaodong
>> > >>
>> > >> On Tue, Feb 26, 2019 at 6:53 PM Yaodong Yang <
>> yangyaodon...@gmail.com>
>> > >> wrote:
>> > >>
>> > >>> Hello Colin,
>> > >>>
>> > >>> There is a POC PR for this KIP, and it contains most changes we are
>> > >>> proposing now.
>> > >>>
>> > >>> Best,
>> > >>> Yaodong
>> > >>>
>> > >>> On Tue, Feb 26, 2019 at 6:51 PM Yaodong Yang <
>> yangyaodon...@gmail.com>
>> > >>> wrote:
>> > >>>
>> > >>>> Hello Colin,
>> > >>>>
>> > >>>> CIL,
>> > >>>>
>> > >>>> Thanks!
>> > >>>> Yaodong
>> > >>>>
>> > >>>>
>> > >>>> On Tue, Feb 26, 2019 at 9:59 AM Colin McCabe <cmcc...@apache.org>
>> > >>>> wrote:
>> > >>>>
>> > >>>>> Hi Yaodong,
>> > >>>>>
>> > >>>>> I don't understand how the proposed API would be used. It talks
>> > about
>> > >>>>> adding a ConfigResource type for clients and users, but doesn't
>> > explain
>> > >>>>> what can be done with these.
>> > >>>>>
>> > >>>>
>> > >>>> Sorry for the confusion. I just updated the KIP, and hopefully it
>> will
>> > >>>> make it easier for you and other people. Looking forward to your
>> > feedback!
>> > >>>>
>> > >>>>
>> > >>>>> In the compatibility section (?) it says "We only add a new way to
>> > >>>>> configure the quotas" which suggests that quotas are involved
>> > somehow What
>> > >>>>> relationship does this have with KIP-257?
>> > >>>>>
>> > >>>>
>> > >>>> Let me give you more context, feel free to correct me if I'm wrong
>> 😁
>> > >>>>
>> > >>>> 1. Originally we hit an issue that we can not config client quota
>> > >>>> through AdminClient. The only way available for us is directly talk
>> > to ZK
>> > >>>> and manage quota directly.
>> > >>>>
>> > >>>> 2. As our client service may not in the same DC as ZooKeeper, there
>> > >>>> could be some cross DC communication which is less desirable.
>> > >>>>
>> > >>>> 3. We deicide to add the quota configuration feature in the
>> > >>>> AdminClient, which will perfectly solve this issue for us.
>> > >>>>
>> > >>>> 4. In addition, we realized that this change can also serve as a
>> way
>> > to
>> > >>>> config other users or clients configuration in Zookeeper. For
>> > instance, if
>> > >>>> we have a new client configuration introduced in the future and
>> they
>> > need
>> > >>>> to be in the Zookeeper as well, we can mange it through the same
>> API.
>> > >>>> Therefore, this KIP is renamed to manage users/clients
>> configurations.
>> > >>>> Quota management is one use case for this configuration management.
>> > >>>>
>> > >>>> 5. KIP-257 is also compatible with the current KIP. For instance,
>> if
>> > >>>> user want to update a quota for a metric, the client side need to
>> > parse it,
>> > >>>> and eventually pass in a user or client config to AdminClient.
>> > AdminClient
>> > >>>> will make sure such configuration changes are applied in the
>> > Zookeeper.
>> > >>>>
>> > >>>>
>> > >>>>> best,
>> > >>>>> Colin
>> > >>>>>
>> > >>>>>
>> > >>>>> On Fri, Feb 22, 2019, at 15:11, Yaodong Yang wrote:
>> > >>>>> > Hi Colin,
>> > >>>>> >
>> > >>>>> > CIL,
>> > >>>>> >
>> > >>>>> > Thanks!
>> > >>>>> > Yaodong
>> > >>>>> >
>> > >>>>> >
>> > >>>>> > On Fri, Feb 22, 2019 at 10:56 AM Colin McCabe <
>> cmcc...@apache.org>
>> > >>>>> wrote:
>> > >>>>> >
>> > >>>>> > > Hi Yaodong,
>> > >>>>> > >
>> > >>>>> > > KIP-422 says that it would be good if "applications [could]
>> > >>>>> leverage the
>> > >>>>> > > unified KafkaAdminClient to manage their user/client
>> > >>>>> configurations,
>> > >>>>> > > instead of the direct dependency on Zookeeper." But the KIP
>> > >>>>> doesn't talk
>> > >>>>> > > about any changes to KafkaAdminClient. Instead, the only
>> changes
>> > >>>>> proposed
>> > >>>>> > > are to AdminZKClient. But that is an internal class-- we
>> don't
>> > >>>>> need a KIP
>> > >>>>> > > to change it, and it's not a public API that users can use.
>> > >>>>> > >
>> > >>>>> >
>> > >>>>> > Sorry for the confusion in the KIP. Actually there is no change
>> to
>> > >>>>> > AdminZKClient needed for this KIP, we just leverage them to
>> > >>>>> configure the
>> > >>>>> > properties in the ZK. You can find the details from this PR
>> > >>>>> > https://github.com/apache/kafka/pull/6189
>> > >>>>> >
>> > >>>>> > As you can see from the PR, we need the client side and server
>> > >>>>> process
>> > >>>>> > changes, so I feel like we still need the KIP for this change.
>> > >>>>> >
>> > >>>>> >
>> > >>>>> > > I realize that the naming might be a bit confusing, but
>> > >>>>> > > kafka.zk.AdminZKClient and kafka.admin.AdminClient are
>> internal
>> > >>>>> classes.
>> > >>>>> > > As the JavaDoc says, kafka.admin.AdminClient is deprecated as
>> > >>>>> well. The
>> > >>>>> > > public class that we would be adding new methods to is
>> > >>>>> > > org.apache.kafka.clients.admin.AdminClient.
>> > >>>>> > >
>> > >>>>> >
>> > >>>>> > I agree. Thanks for pointing this out!
>> > >>>>> >
>> > >>>>> >
>> > >>>>> > > best,
>> > >>>>> > > Colin
>> > >>>>> > >
>> > >>>>> > > On Tue, Feb 19, 2019, at 15:21, Yaodong Yang wrote:
>> > >>>>> > > > Hello Jun, Viktor, Snoke and Stan,
>> > >>>>> > > >
>> > >>>>> > > > Thanks for taking time to look at this KIP-422! For some
>> > reason,
>> > >>>>> this
>> > >>>>> > > email
>> > >>>>> > > > was put in my spam folder. Sorry about that.
>> > >>>>> > > >
>> > >>>>> > > > Jun is right, the main motivation for this KIP-422 is to
>> allow
>> > >>>>> users to
>> > >>>>> > > > config user/clientId quota through AdminClient. In addition,
>> > >>>>> this KIP-422
>> > >>>>> > > > also allows users to set or update any config related to a
>> user
>> > >>>>> or
>> > >>>>> > > clientId
>> > >>>>> > > > entity if needed in the future.
>> > >>>>> > > >
>> > >>>>> > > > For the KIP-257, I agree with Jun that we should add support
>> > for
>> > >>>>> it. I
>> > >>>>> > > will
>> > >>>>> > > > look at the current implementation and update the KIP-422
>> with
>> > >>>>> new
>> > >>>>> > > change.
>> > >>>>> > > >
>> > >>>>> > > > I will ping this thread once I updated the KIP.
>> > >>>>> > > >
>> > >>>>> > > > Thanks again!
>> > >>>>> > > > Yaodong
>> > >>>>> > > >
>> > >>>>> > > > On Fri, Feb 15, 2019 at 1:28 AM Viktor Somogyi-Vass <
>> > >>>>> > > viktorsomo...@gmail.com>
>> > >>>>> > > > wrote:
>> > >>>>> > > >
>> > >>>>> > > > > Hi Guys,
>> > >>>>> > > > >
>> > >>>>> > > > > I wanted to reject that KIP, split it up and revamp it as
>> in
>> > >>>>> the
>> > >>>>> > > meantime
>> > >>>>> > > > > there were some overlapping works I just didn't get to it
>> due
>> > >>>>> to other
>> > >>>>> > > > > higher priority work.
>> > >>>>> > > > > One of the splitted KIPs would have been the quota part of
>> > >>>>> that and
>> > >>>>> > > I'd be
>> > >>>>> > > > > happy if that lived in this KIP if Yaodong thinks it's
>> worth
>> > to
>> > >>>>> > > > > incorporate. I'd be also happy to rebase that wire
>> protocol
>> > and
>> > >>>>> > > contribute
>> > >>>>> > > > > it to this KIP.
>> > >>>>> > > > >
>> > >>>>> > > > > Viktor
>> > >>>>> > > > >
>> > >>>>> > > > > On Wed, Feb 13, 2019 at 7:14 PM Jun Rao <j...@confluent.io
>> >
>> > >>>>> wrote:
>> > >>>>> > > > >
>> > >>>>> > > > > > Hi, Yaodong,
>> > >>>>> > > > > >
>> > >>>>> > > > > > Thanks for the KIP. As Stan mentioned earlier, it seems
>> > that
>> > >>>>> this is
>> > >>>>> > > > > > mostly covered by KIP-248, which was originally
>> proposed by
>> > >>>>> Victor.
>> > >>>>> > > > > >
>> > >>>>> > > > > > Hi, Victor,
>> > >>>>> > > > > >
>> > >>>>> > > > > > Do you still plan to work on KIP-248? It seems that you
>> > >>>>> already got
>> > >>>>> > > > > pretty
>> > >>>>> > > > > > far on that. If not, would you mind letting Yaodong take
>> > >>>>> over this?
>> > >>>>> > > > > >
>> > >>>>> > > > > > For both KIP-248 and KIP-422, one thing that I found
>> > missing
>> > >>>>> is the
>> > >>>>> > > > > > support for customized quota (
>> > >>>>> > > > > >
>> > >>>>> > > > >
>> > >>>>> > >
>> > >>>>>
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-257+-+Configurable+Quota+Management
>> > >>>>> > > > > ).
>> > >>>>> > > > > > With KIP-257, it's possible for one to construct a
>> > >>>>> customized quota
>> > >>>>> > > > > defined
>> > >>>>> > > > > > through a map of metric tags. It would be useful to
>> support
>> > >>>>> that in
>> > >>>>> > > the
>> > >>>>> > > > > > AdminClient API and the wire protocol.
>> > >>>>> > > > > >
>> > >>>>> > > > > > Hi, Sonke,
>> > >>>>> > > > > >
>> > >>>>> > > > > > I think the proposal is to support the user/clientId
>> level
>> > >>>>> quota
>> > >>>>> > > through
>> > >>>>> > > > > > an AdminClient api. The user can be obtained from any
>> > >>>>> existing
>> > >>>>> > > > > > authentication mechanisms.
>> > >>>>> > > > > >
>> > >>>>> > > > > > Thanks,
>> > >>>>> > > > > >
>> > >>>>> > > > > > Jun
>> > >>>>> > > > > >
>> > >>>>> > > > > > On Thu, Feb 7, 2019 at 5:59 AM Sönke Liebau
>> > >>>>> > > > > > <soenke.lie...@opencore.com.invalid> wrote:
>> > >>>>> > > > > >
>> > >>>>> > > > > >> Hi Yaodong,
>> > >>>>> > > > > >>
>> > >>>>> > > > > >> thanks for the KIP!
>> > >>>>> > > > > >>
>> > >>>>> > > > > >> If I understand your intentions correctly then this KIP
>> > >>>>> would only
>> > >>>>> > > > > >> address a fairly specific use case, namely SASL-PLAIN
>> with
>> > >>>>> users
>> > >>>>> > > > > >> defined in Zookeeper. For all other authentication
>> > >>>>> mechanisms like
>> > >>>>> > > > > >> SSL, SASL-GSSAPI or SASL-PLAIN with users defined in
>> jaas
>> > >>>>> files I
>> > >>>>> > > > > >> don't see how the AdminClient could directly create new
>> > >>>>> users.
>> > >>>>> > > > > >> Is this correct, or am I missing something?
>> > >>>>> > > > > >>
>> > >>>>> > > > > >> Best regards,
>> > >>>>> > > > > >> Sönke
>> > >>>>> > > > > >>
>> > >>>>> > > > > >> On Thu, Feb 7, 2019 at 2:47 PM Stanislav Kozlovski
>> > >>>>> > > > > >> <stanis...@confluent.io> wrote:
>> > >>>>> > > > > >> >
>> > >>>>> > > > > >> > This KIP seems to duplicate some of the functionality
>> > >>>>> proposed in
>> > >>>>> > > > > >> KIP-248
>> > >>>>> > > > > >> > <
>> > >>>>> > > > > >>
>> > >>>>> > > > >
>> > >>>>> > >
>> > >>>>>
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-248+-+Create+New+ConfigCommand+That+Uses+The+New+AdminClient
>> > >>>>> > > > > >> >.
>> > >>>>> > > > > >> > KIP-248 has been stuck in a vote thread since July
>> 2018.
>> > >>>>> > > > > >> >
>> > >>>>> > > > > >> > Viktor, do you plan on working on the KIP?
>> > >>>>> > > > > >> >
>> > >>>>> > > > > >> > On Thu, Feb 7, 2019 at 1:27 PM Stanislav Kozlovski <
>> > >>>>> > > > > >> stanis...@confluent.io>
>> > >>>>> > > > > >> > wrote:
>> > >>>>> > > > > >> >
>> > >>>>> > > > > >> > > Hey there Yaodong, thanks for the KIP!
>> > >>>>> > > > > >> > >
>> > >>>>> > > > > >> > > I'm not too familiar with the user/client
>> > >>>>> configurations we
>> > >>>>> > > > > currently
>> > >>>>> > > > > >> > > allow, is there a KIP describing the initial
>> feature?
>> > >>>>> If there
>> > >>>>> > > is,
>> > >>>>> > > > > it
>> > >>>>> > > > > >> would
>> > >>>>> > > > > >> > > be useful to include in KIP-422.
>> > >>>>> > > > > >> > >
>> > >>>>> > > > > >> > > I also didn't see any authorization in the PR,
>> have we
>> > >>>>> thought
>> > >>>>> > > about
>> > >>>>> > > > > >> > > needing to authorize the alter/describe requests
>> per
>> > the
>> > >>>>> > > > > user/client?
>> > >>>>> > > > > >> > >
>> > >>>>> > > > > >> > > Thanks,
>> > >>>>> > > > > >> > > Stanislav
>> > >>>>> > > > > >> > >
>> > >>>>> > > > > >> > > On Fri, Jan 25, 2019 at 5:47 PM Yaodong Yang <
>> > >>>>> > > > > yangyaodon...@gmail.com
>> > >>>>> > > > > >> >
>> > >>>>> > > > > >> > > wrote:
>> > >>>>> > > > > >> > >
>> > >>>>> > > > > >> > >> Hi folks,
>> > >>>>> > > > > >> > >>
>> > >>>>> > > > > >> > >> I've published KIP-422 which is about adding
>> support
>> > >>>>> for
>> > >>>>> > > > > user/client
>> > >>>>> > > > > >> > >> configurations in the Kafka Admin Client.
>> > >>>>> > > > > >> > >>
>> > >>>>> > > > > >> > >> Basically the story here is to allow
>> KafkaAdminClient
>> > >>>>> to
>> > >>>>> > > configure
>> > >>>>> > > > > >> the
>> > >>>>> > > > > >> > >> user
>> > >>>>> > > > > >> > >> or client configurations for users, instead of
>> > >>>>> requiring users
>> > >>>>> > > to
>> > >>>>> > > > > >> directly
>> > >>>>> > > > > >> > >> talk to ZK.
>> > >>>>> > > > > >> > >>
>> > >>>>> > > > > >> > >> The link for this KIP is
>> > >>>>> > > > > >> > >> following:
>> > >>>>> > > > > >> > >>
>> > >>>>> > > > > >>
>> > >>>>> > > > >
>> > >>>>> > >
>> > >>>>>
>> >
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=97555704
>> > >>>>> > > > > >> > >>
>> > >>>>> > > > > >> > >> I'd be happy to receive some feedback about the
>> KIP I
>> > >>>>> > > published.
>> > >>>>> > > > > >> > >>
>> > >>>>> > > > > >> > >> --
>> > >>>>> > > > > >> > >> Best,
>> > >>>>> > > > > >> > >> Yaodong Yang
>> > >>>>> > > > > >> > >>
>> > >>>>> > > > > >> > >
>> > >>>>> > > > > >> > >
>> > >>>>> > > > > >> > > --
>> > >>>>> > > > > >> > > Best,
>> > >>>>> > > > > >> > > Stanislav
>> > >>>>> > > > > >> > >
>> > >>>>> > > > > >> >
>> > >>>>> > > > > >> >
>> > >>>>> > > > > >> > --
>> > >>>>> > > > > >> > Best,
>> > >>>>> > > > > >> > Stanislav
>> > >>>>> > > > > >>
>> > >>>>> > > > > >>
>> > >>>>> > > > > >>
>> > >>>>> > > > > >> --
>> > >>>>> > > > > >> Sönke Liebau
>> > >>>>> > > > > >> Partner
>> > >>>>> > > > > >> Tel. +49 179 7940878
>> > >>>>> > > > > >> OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880
>> > Wedel
>> > >>>>> -
>> > >>>>> > > Germany
>> > >>>>> > > > > >>
>> > >>>>> > > > > >
>> > >>>>> > > > >
>> > >>>>> > > >
>> > >>>>> > >
>> > >>>>> >
>> > >>>>>
>> > >>>>
>> >
>>
>
