Hi Colin, I took another look and have a couple of comments:
1. DescribeAclsResponse only has an error code at the top level. In the DescribeConfigs KIP, you suggested that we should have an error code at the batch level. Doesn't that apply here too? 2. CreateAclsResponse don't include the resource type and resource name (the order of the responses has to be used to match against the request). It seems like this would make it harder to debug and it's inconsistent with other responses. Is it really worth the space savings? Ismael On Tue, May 9, 2017 at 12:15 AM, Colin McCabe <cmcc...@apache.org> wrote: > Just a quick note: earlier Magnus and I talked about what happens if you > attempt to add a duplicate ACL. It appears that SimpleAuthorizer > silently allows re-adding an ACL that already exists. There is no error > raised. However, the ACLs are deduplicated on the back end (only one > ACL will show up in describeAcls). Since modifying authorizer semantics > is out of scope for this KIP, those are the semantics we'll have for > now. We can revisit this in KIP-50. > > cheers, > Colin > > > On Mon, May 8, 2017, at 10:52, Colin McCabe wrote: > > Quick update: I renamed ListAcls to DescribeAcls, for consistency with > > DescribeTopics, DescribeGroups, DescribeConfigs, etc. > > > > cheers, > > Colin > > > > > > On Fri, May 5, 2017, at 11:54, Colin McCabe wrote: > > > Hi all, > > > > > > Thanks for all the discussion. I made some changes to the KIP. > > > > > > * After some discussion about permissions, CreateAcls/DeleteAcls now > > > require Cluster:Alter rather than Cluster:All. > > > > > > * The resource type, operation type, etc. enums now start at 0 rather > > > than at -2, for simplicity's sake > > > > > > * Added a new exception, SecurityDisabledException, which is returned > > > when we attempt to list, add, or remove ACLs, but security is disabled > > > (i.e. there is no authorizer configured to handle the request). > > > > > > * Added a note to listAclsRequest specifying that the arguments are > > > ANDed together to act as a filter. > > > > > > * Added some clarification about the compatibility rules for ACL > > > deletion when the client is newer than the broker > > > > > > cheers, > > > Colin > > > > > > > > > On Thu, May 4, 2017, at 13:46, Colin McCabe wrote: > > > > Hi Guozhang, > > > > > > > > Thanks for taking a look. > > > > > > > > On Thu, May 4, 2017, at 00:43, Guozhang Wang wrote: > > > > > Colin, > > > > > > > > > > Thanks for the KIP. A general question I have is whether we can > support > > > > > "wildcard" resource names as well? For example, if we want to > create / > > > > > delete ACLs for all topic names following a wildcard regex? I read > though > > > > > the principle wildcards description but I am not sure if it is > designed > > > > > for > > > > > this purposes. > > > > > > > > If you want to delete all ACLs that apply to topics, you could use a > > > > deletion filter with resourceType=AclResourceType.TOPIC, > > > > resourceName=null. I think that answers the question, unless I > > > > misunderstood. > > > > > > > > Note that it's important to distinguish between wildcards (the "*" > > > > value) and "any". For example, when your deletionFilter has > > > > principal="User:*" you are not deleting ACLs for all users. Just > ACLs > > > > that have the principal field set to "User:*" If you want to delete > > > > ACLs for all users, you can use principal=null. Similarly, > > > > principal=null is only valid in a filter, not in an actual ACL that > is > > > > applied to a resource. > > > > > > > > best, > > > > Colin > > > > > > > > > > > > > > Otherwise, lgtm! > > > > > > > > > > > > > > > Guozhang > > > > > > > > > > > > > > > On Fri, Apr 28, 2017 at 10:37 PM, Dongjin Lee <dong...@apache.org> > wrote: > > > > > > > > > > > +1 > > > > > > > > > > > > On 29 Apr 2017, 9:51 AM +0900, Michael Pearce < > michael.pea...@ig.com>, > > > > > > wrote: > > > > > > > +1 > > > > > > > ________________________________________ > > > > > > > From: Colin McCabe <cmcc...@apache.org > > > > > > > Sent: Saturday, April 29, 2017 1:09:25 AM > > > > > > > To: dev@kafka.apache.org > > > > > > > Subject: [VOTE] KIP-140: Add administrative RPCs for adding, > deleting, > > > > > > and listing ACLs > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > I'd like to start the voting for KIP-140: Add administrative > RPCs for > > > > > > > adding, deleting, and listing ACLs. This provides an API for > adding, > > > > > > > deleting, and listing the access control lists (ACLs) which > are used to > > > > > > > control access on Kafka topics and brokers. > > > > > > > > > > > > > > The wiki page is here: > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > > > > 140%3A+Add+administrative+RPCs+for+adding%2C+deleting% > 2C+and+listing+ACLs > > > > > > > > > > > > > > The previous [DISCUSS] thread: > > > > > > > https://www.mail-archive.com/dev@kafka.apache.org/msg70858. > html > > > > > > > > > > > > > > cheers, > > > > > > > Colin > > > > > > > The information contained in this email is strictly > confidential and for > > > > > > the use of the addressee only, unless otherwise indicated. If > you are not > > > > > > the intended recipient, please do not read, copy, use or > disclose to others > > > > > > this message or any attachment. Please also notify the sender by > replying > > > > > > to this email or by telephone (+44(020 7896 0011 > (tel:020%207896%200011)) > > > > > > and then delete the email and any copies of it. Opinions, > conclusion (etc) > > > > > > that do not relate to the official business of this company > shall be > > > > > > understood as neither given nor endorsed by it. IG is a trading > name of IG > > > > > > Markets Limited (a company registered in England and Wales, > company number > > > > > > 04008957 (tel:04008957)) and IG Index Limited (a company > registered in > > > > > > England and Wales, company number 01190902 (tel:01190902)). > Registered > > > > > > address at Cannon Bridge House, 25 Dowgate Hill, London EC4R > 2YA. Both IG > > > > > > Markets Limited (register number 195355) and IG Index Limited > (register > > > > > > number 114059) are authorised and regulated > > > > > > by the Financial Conduct Authority. > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > -- Guozhang >
