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