Re: [DISCUSS] KIP-888: Batch describe ACLs and describe client quotas

[Mickael Maison <mi...@gmail.com>]

Hi Tom,

Thanks for the feedback, you raised a few interesting points.

Regarding the response size issue: With some APIs, if the same filter
is in the request multiple times, the response will only include it
once. To do so you need to include the filter in the response. For
example, this works with describeConsumerGroups because the filter,
the group id, is small. For ACLs and Quotas, I thought having the
exact same filters multiple times in the requests should be rare so I
privileged returning one entry for each filter in the request, and
avoid including the filters back in the response.

One option to limit potential abuse is to restrict the accepted
filters. For example brokers could reject requests containing
duplicate filters. Going further we could restrict the types of
filters allowed and only allow requests with multiple filters to have
multiple exact match filters, or if using fuzzy matches only allow it
on different types. I don't think this would limit the usability of
these APIs too much.

Regarding using ids for each ACL: Yes StandardAuthorizer identifies
unique ACLs with Uuids but custom Authorizer implementations don't
necessarily do that. So while this could help reduce response size
this would require significant changes to the Authorizer interface.

Thanks,
Mickael




On Fri, Dec 2, 2022 at 6:55 PM Tom Bentley <tb...@redhat.com> wrote:
>
> Hi Mickael,
>
> Thanks for the KIP. I can understand the motivation, but to be honest I
> have a doubt about this.
>
> Taking the ACLs first, by allowing multiple filters with the current
> proposal isn't there the chance that the same ACL will match multiple
> filters, and be returned multiple times in the response. In fact, in the
> worst case couldn't the client (by intent or accident) just request all
> ACLs be included in the response an arbitrary number of times? This could
> result in some pretty large responses. One way to avoid inflating the
> response like this would be for the broker to deduplicate the ACLs in the
> response by assigning an id for each, serialising each once and then using
> the id to enumerate the matches for each pattern. It's worth noting that
> the AccessControlEntryRecord used for KRaft clusters already has a Uuid. So
> for the KRaft case it might be possible to use this, rather than the broker
> having to deduplicate when handing the request.
>
> Another wrinkle is that if the broker calls
> Authorizer.acls(AclBindingFilter) once for each pattern there's no
> guarantee that the results are consistent (they could be modified between
> calls). It could be made consistent with appropriate locking, but since in
> practice this would be a very rare occurrence maybe we could just document
> that as a limitation.
>
> Thanks again,
>
> Tom
>
> On Fri, 18 Nov 2022 at 18:00, Mickael Maison <mi...@gmail.com>
> wrote:
>
> > Hi,
> >
> > I have opened KIP-888 to allow describing ACLs and Quotas in batches:
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-888%3A+Batch+describe+ACLs+and+describe+client+quotas
> >
> > Let me know if you have any feedback or suggestions.
> >
> > Thanks,
> > Mickael
> >
> >

Re: [DISCUSS] KIP-888: Batch describe ACLs and describe client quotas

[Federico Valeri <fe...@gmail.com>]

Hi Michael, thanks for the KIP.

In addition to the deduplication logic, I wonder if we should also
consider introducing a pagination mechanism. Talking about ACLs, there
could be many rules for each user and we may have lots of users on big
clusters.



On Tue, Jan 31, 2023 at 3:58 PM Mickael Maison <mi...@gmail.com> wrote:
>
> Hi Tom,
>
> Thanks for the feedback, you raised a few interesting points.
>
> Regarding the response size issue: With some APIs, if the same filter
> is in the request multiple times, the response will only include it
> once. To do so you need to include the filter in the response. For
> example, this works with describeConsumerGroups because the filter,
> the group id, is small. For ACLs and Quotas, I thought having the
> exact same filters multiple times in the requests should be rare so I
> privileged returning one entry for each filter in the request, and
> avoid including the filters back in the response.
>
> One option to limit potential abuse is to restrict the accepted
> filters. For example brokers could reject requests containing
> duplicate filters. Going further we could restrict the types of
> filters allowed and only allow requests with multiple filters to have
> multiple exact match filters, or if using fuzzy matches only allow it
> on different types. I don't think this would limit the usability of
> these APIs too much.
>
> Regarding using ids for each ACL: Yes StandardAuthorizer identifies
> unique ACLs with Uuids but custom Authorizer implementations don't
> necessarily do that. So while this could help reduce response size
> this would require significant changes to the Authorizer interface.
>
> Thanks,
> Mickael
>
>
>
>
> On Fri, Dec 2, 2022 at 6:55 PM Tom Bentley <tb...@redhat.com> wrote:
> >
> > Hi Mickael,
> >
> > Thanks for the KIP. I can understand the motivation, but to be honest I
> > have a doubt about this.
> >
> > Taking the ACLs first, by allowing multiple filters with the current
> > proposal isn't there the chance that the same ACL will match multiple
> > filters, and be returned multiple times in the response. In fact, in the
> > worst case couldn't the client (by intent or accident) just request all
> > ACLs be included in the response an arbitrary number of times? This could
> > result in some pretty large responses. One way to avoid inflating the
> > response like this would be for the broker to deduplicate the ACLs in the
> > response by assigning an id for each, serialising each once and then using
> > the id to enumerate the matches for each pattern. It's worth noting that
> > the AccessControlEntryRecord used for KRaft clusters already has a Uuid. So
> > for the KRaft case it might be possible to use this, rather than the broker
> > having to deduplicate when handing the request.
> >
> > Another wrinkle is that if the broker calls
> > Authorizer.acls(AclBindingFilter) once for each pattern there's no
> > guarantee that the results are consistent (they could be modified between
> > calls). It could be made consistent with appropriate locking, but since in
> > practice this would be a very rare occurrence maybe we could just document
> > that as a limitation.
> >
> > Thanks again,
> >
> > Tom
> >
> > On Fri, 18 Nov 2022 at 18:00, Mickael Maison <mi...@gmail.com>
> > wrote:
> >
> > > Hi,
> > >
> > > I have opened KIP-888 to allow describing ACLs and Quotas in batches:
> > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-888%3A+Batch+describe+ACLs+and+describe+client+quotas
> > >
> > > Let me know if you have any feedback or suggestions.
> > >
> > > Thanks,
> > > Mickael
> > >
> > >