Re: [DISCUSS] KIP-888: Batch describe ACLs and describe client quotas
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 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 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 > > 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
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 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 > 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
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 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 > >
[DISCUSS] KIP-888: Batch describe ACLs and describe client quotas
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