Colin, Thanks for your insightful comments. I came to the same conclusion.
I do have 2 Jira tickets to simplify some of this. 1) KAFKA-17316 <https://issues.apache.org/jira/browse/KAFKA-17316> - Makes developing a new Authorizer by creating a new implementation of the StandardAuthorizerData easier. Basically adds interfaces, abstract classes and lots of tests. 2) KAFKA-17423 <https://issues.apache.org/jira/browse/KAFKA-17423> - Builds on KAFKA-17316 by creating a Trie implementation (without the new wildcards from KIP-1042). This provides an order of magnitude faster processing of ACL requests. My plan is to get these through the process, so I have a better understanding of the Kafka development process, and then rework KIP-1042 to create a new PatternType with wildcard processing. I would appreciate your feedback on the above tickets if you have the time. Claude On Fri, Aug 23, 2024 at 9:43 PM Colin McCabe <cmcc...@apache.org> wrote: > On Sat, Jul 27, 2024, at 04:20, Claude Warren, Jr wrote: > > I have updated the KIP with results from the Trie implementation and they > > are dramatic to say the least. For most searches they are at least an > > order of magnitude faster and use less memory. The wildcard search is > not > > a regular expression but rather a file type wild card (*=1 or more > > character, ?=1 character). > > > > Hi Claude, > > One issue here is that your change is incompatible. For example, if I have > a consumer group named '?', and an ALLOW ACL for '?', this change alters > the meaning of that ACL. Previously it just ALLOWed '?' Now it allows any > single-character group. This obviously could be a major security issue for > people doing upgrades. > > (This wouldn't be an issue if we restricted consumer group names to a > sensible set of characters, like we did with topics. But that ship has > sailed...) > > If you want to add new behavior in a backwards-compatible fashion, your > best bet probably is to create a new PatternType. Unfortunately this makes > the complexity issue worse, but I don't see a way around it. Perhaps we can > deprecate LITERAL and PREFIXED at some future date, if this wildcard thing > makes them unecessary. > > Another issue I see, somewhat related, is that Authorizers are pluggable, > and probably won't be updated all at once to support this new type. That > should be fine, but we should document the error that is returned when > someone tries to create your new-style ACLs with an authorizer that does > not support them. > > > Code is available on my personal branch [1]. It is not fully documented > > and does not meet the checkstyle requirements yet. I also modified the > jmh > > tests to run the Trie tests and limit the test to the single case > mentioned > > in the KIP documentation. > > > > If there are no issues with this code, I will complete the documentation > > and fix the checkstyle and then open a pull request. > > It's fine to open a pull request. You can link it on the KIP page. As > always, we don't want to commit it until the KIP is accepted. > > best, > Colin > > > > > Claude > > > > [1] > https://github.com/Claudenw/kafka/pull/new/KIP-1042_Trie_Implementation > > > > > > On Wed, Jul 3, 2024 at 2:21 PM Claude Warren, Jr <claude.war...@aiven.io > > > > wrote: > > > >> I think that if we put in a trie based system we should be able to halve > >> the normal searhc times and still be able to locate wild card matches > very > >> quickly. Users should be warned that "head wildcard" matches are slow > and > >> to use them sparingly. I am going to see if I can work out how to do > >> wildcard matches within the trie. > >> > >> But in all cases can show that the trie is faster than the current > >> implementation. > >> > >> Claude > >> > >> > >> > >> On Wed, Jun 19, 2024 at 7:53 PM Muralidhar Basani > >> <muralidhar.bas...@aiven.io.invalid> wrote: > >> > >>> There are some test results mentioned in the Test Plan section of the > Kip, > >>> but we need to do more testing with various patterns and permission > types. > >>> As mentioned in the discuss thread, the trie implementation could > >>> potentially surpass the current speed of ACL match. > >>> > >>> However, we can only accurately assess the results after updating the > >>> actual classes and analysing them with AuthorizerBenchmark. > >>> > >>> Thanks, > >>> > >>> Murali > >>> > >>> On Mon, 17 Jun 2024 at 20:39, Colin McCabe <cmcc...@apache.org> wrote: > >>> > >>> > My concern is that the extra complexity may actually slow us down. In > >>> > general people already complain about the speed of ACL matches, and > >>> adding > >>> > another "degree of freedom" seems likely to make things worse. > >>> > > >>> > It would be useful to understand how much faster or slower the code > is > >>> > with the propsed changes, versus without them. > >>> > > >>> > best, > >>> > Colin > >>> > > >>> > > >>> > On Mon, Jun 17, 2024, at 01:26, Muralidhar Basani wrote: > >>> > > Hi all, > >>> > > > >>> > > I would like to call a vote on KIP-1042 which extends creation of > acls > >>> > with > >>> > > MATCH pattern type. > >>> > > > >>> > > KIP - > >>> > > > >>> > > >>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1042%3A+Support+for+wildcard+when+creating+new+acls > >>> > > > >>> > > Discussion thread - > >>> > > https://lists.apache.org/thread/xx3lcg60kp4v34x0j9p6xobby8l4cfq2 > >>> > > > >>> > > Thanks, > >>> > > Murali > >>> > > >>> > >> >
