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