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 <mickael.mai...@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 <tbent...@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 <mickael.mai...@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 > > > > > >
