w.r.t. naming, we can keep wildcard and drop 'prefixed' (or 'suffixed') since the use of regex would always start with non-wildcard portion.
Cheers On Tue, May 1, 2018 at 12:13 PM, Andy Coates <a...@confluent.io> wrote: > Hi Piyush, > > Can you also document in the Compatibility section what would happen should > the cluster be upgraded, wildcard-suffixed ACLs are added, and then the > cluster is rolled back to the previous version. On downgrade the partial > wildcard ACLs will be treated as literals and hence never match anything. > This is fine for ALLOW ACLs, but some might consider this a security hole > if a DENY ACL is ignored, so this needs to be documented, both in the KIP > and the final docs. > > For some reason I find the term 'prefixed wildcard ACLs' easier to grok > than 'wildcard suffixed ACLs'. Probably because in the former the > 'wildcard' term comes after the positional adjective, which matches the > position of the wildcard char in the resource name, i.e. "some*". It's > most likely a person thing, but I thought I'd mention it as naming is > important when it comes to making this initiative for users. > > On 1 May 2018 at 19:57, Andy Coates <a...@confluent.io> wrote: > > > Hi Piyush, > > > > Thanks for raising this KIP - it's very much appreciated. > > > > I've not had chance to digest it yet, but... > > > > 1. you might want to add details of how the internals of the > > `getMatchingAcls` is implemented. We'd want to make sure the complexity > of > > the operation isn't adversely affected. > > 2. You might want to be more explicit that the length of a prefix does > not > > play a part in the `authorize` call, e.g. given ACLs {DENY, some.*}, > {ALLOW, > > some.prefix.*}, the longer, i.e. more specific, allow ACL does _not_ > > override the more general deny ACL. > > > > Thanks, > > > > Andy > > > > On 1 May 2018 at 16:59, Ron Dagostino <rndg...@gmail.com> wrote: > > > >> Hi Piyush. I appreciated your talk at Kafka Summit and appreciate the > KIP > >> -- thanks. > >> > >> Could you explain these mismatching references? Near the top of the KIP > >> you refer to these proposed new method signatures: > >> > >> def getMatchingAcls(resource: Resource): Set[Acl] > >> def getMatchingAcls(principal: KafkaPrincipal): Map[Resource, Set[Acl]] > >> > >> But near the bottom of the KIP you refer to different method > >> signatures that don't seem to match the above ones: > >> > >> getMatchingAcls(topicRegex) > >> getMatchingAcls(principalRegex) > >> > >> Ron > >> > >> > >> On Tue, May 1, 2018 at 11:48 AM, Ted Yu <yuzhih...@gmail.com> wrote: > >> > >> > The KIP was well written. Minor comment on formatting: > >> > > >> > https://github.com/apache/kafka/blob/trunk/core/src/ > >> > main/scala/kafka/admin/AclCommand.scala > >> > to > >> > > >> > Leave space between the URL and 'to' > >> > > >> > Can you describe changes for the AdminClient ? > >> > > >> > Thanks > >> > > >> > On Tue, May 1, 2018 at 8:12 AM, Piyush Vijay <piyushvij...@gmail.com> > >> > wrote: > >> > > >> > > Hi all, > >> > > > >> > > I just opened a KIP to add support for wildcard suffixed ACLs. This > is > >> > one > >> > > of the feature I talked about in my Kafka summit talk and we > promised > >> to > >> > > upstream it :) > >> > > > >> > > The details are here - > >> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > >> > > 290%3A+Support+for+wildcard+suffixed+ACLs > >> > > > >> > > There is an open question about the way to add the support in the > >> > > AdminClient, which I can discuss here in more detail once everyone > has > >> > > taken a first look at the KIP. > >> > > > >> > > Looking forward to discuss the change. > >> > > > >> > > Best, > >> > > Piyush Vijay > >> > > > >> > > >> > > > > >