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
> >> > >
> >> >
> >>
> >
> >
>

Reply via email to