Hi Satish, Thanks for the review.
1. Currently we are just printing toSring() of Acl class. I prefer to have similar output with or without "--principal" option. 2. Yes, we can accept multiple principals. Updated the KIP. @all If there are no more comments, I will start vote on this KIP early next week. Thanks, On Fri, Aug 24, 2018 at 11:29 AM Satish Duggana <satish.dugg...@gmail.com> wrote: > Hi Mani, > Just a minor comment on the output of the command as given in KIP-357, you > may want to remove "User:User1 has " as it is redundant for each ACL. > It may be good to accept multiple principals option to avoid running this > script multiple times with each principal to achieve the same. > > > >> sh kafka-acls.sh > <https://cwiki.apache.org/confluence/display/KAFKA/kafka-acls.sh> > --authorizer-properties zookeeper.connect=localhost: > < > https://cwiki.apache.org/confluence/display/KAFKA/zookeeper.connect=localhost > :> > 2181 --list --principal User:User1 > ACLs for principal `User:User1` > Current ACLs for resource `Group:PREFIXED:TEST_GROUP`: > User:User1 has Allow permission for operations: Read from hosts: * > > Current ACLs for resource `Topic:PREFIXED:TEST_TOPIC`: > User:User1 has Allow permission for operations: Read from hosts: * > User:User1 has Allow permission for operations: Create from hosts: * > User:User1 has Allow permission for operations: Write from hosts: * > User:User1 has Allow permission for operations: Describe from hosts: * > > Thanks, > Satish. > > > On Fri, Aug 24, 2018 at 1:13 AM, Harsha <ka...@harsha.io> wrote: > > > +1 (binding) > > > > Thanks, > > Harsha > > > > On Wed, Aug 22, 2018, at 9:15 AM, Manikumar wrote: > > > Hi Viktor, > > > We already have a method in Authorizer interface to get acls for a > given > > > principal. > > > We will use this method to fetch acls and filter the results for > > requested > > > Resources. > > > Authorizer { > > > def getAcls(principal: KafkaPrincipal): Map[Resource, Set[Acl]] > > > } > > > Currently AdminClient API doesn't have a API to fetch acls for a given > > > principal. > > > So while using AclCommand with AdminClient API (KIP-332), we just > filter > > > the results returned > > > from describeAcls API. We can add new AdminClient API/new > > > DescribeAclsRequest if required in future. > > > > > > Updated the KIP. Thanks for the review. > > > > > > Thanks, > > > > > > On Wed, Aug 22, 2018 at 5:53 PM Viktor Somogyi-Vass < > > viktorsomo...@gmail.com> > > > wrote: > > > > > > > Hi Manikumar, > > > > > > > > Implementation-wise is it just a filter over the returned ACL listing > > or do > > > > you plan to add new methods to the Authorizer as well? > > > > > > > > Thanks, > > > > Viktor > > > > > > > > On Fri, Aug 17, 2018 at 9:18 PM Priyank Shah <ps...@hortonworks.com> > > > > wrote: > > > > > > > > > +1(non-binding) > > > > > > > > > > Thanks. > > > > > Priyank > > > > > > > > > > On 8/16/18, 6:01 AM, "Manikumar" <manikumar.re...@gmail.com> > wrote: > > > > > > > > > > Hi all, > > > > > > > > > > I have created a minor KIP to add support to list ACLs per > > principal > > > > > using > > > > > AclCommand (kafka-acls.sh) > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > 357%3A++Add+support+to+list+ACLs+per+principal > > > > > > > > > > Please take a look. > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > > > > > > >
