Hey Piyush, How are you getting on updating the KIP? Can we offer any support? We're starting to fly really close to the required 72 hours cut off for KIPs. This doesn't leave much room for resolving any issues any committers find. Also, we now require at least three committers to review this KIP today _and_ find no issues if we're to get this KIP accepted.
Thanks, Andy On 18 May 2018 at 01:21, Piyush Vijay <piyushvij...@gmail.com> wrote: > Hi Andy, > > I still have some minor changes left to the KIP. I'll make them in the > morning. I'm sorry I got caught up in some other things today. But that > would still give us 72 hours before the deadline :) > > Thanks > > > Piyush Vijay > > On Thu, May 17, 2018 at 1:27 PM, Andy Coates <a...@confluent.io> wrote: > > > Hey Piyush - my bad. Sorry. > > > > On 17 May 2018 at 13:23, Piyush Vijay <piyushvij...@gmail.com> wrote: > > > > > It's still not complete. I'll drop a message here when I'm done with > the > > > updates. > > > > > > Thanks > > > > > > > > > Piyush Vijay > > > > > > On Thu, May 17, 2018 at 12:04 PM, Andy Coates <a...@confluent.io> > wrote: > > > > > > > Thanks for the update to the KIP Piyush! > > > > > > > > Reading it through again, I've a couple of questions: > > > > > > > > 1. Why is there a need for a new 'getMatchingAcls' method, over the > > > > existing getAcls method? They both take a Resource instance and > return > > a > > > > set of Acls. What is the difference in their behaviour? > > > > 2. It's not clear to me from the KIP alone what will change, from a > > users > > > > perspective, on how they add / list / delete ACLs. I'm assuming this > > > won't > > > > change. > > > > 3. Writing ACLs to a new location to get around the issues of > embedded > > > > wildcards in existing group ACLs makes sense to me - but just a > > thought, > > > > will we be writing all new ACLs under this new path, or just those > that > > > are > > > > partial wildcards? I'm assuming its the latter, but it could just be > > > 'all' > > > > right? As we could escape illegal chars. So we could just make this > > new > > > > path 'v2' rather wildcard. > > > > > > > > Andy > > > > > > > > On 17 May 2018 at 09:32, Colin McCabe <cmcc...@apache.org> wrote: > > > > > > > > > On Thu, May 17, 2018, at 09:28, Piyush Vijay wrote: > > > > > > I was planning to do that. > > > > > > > > > > > > Another unrelated detail is the presence of the support for ‘*’ > ACL > > > > > > currently. Looks like we’ll have to keep supporting this as a > > special > > > > > case, > > > > > > even though using a different location for wildcard-suffix ACLs > on > > > Zk. > > > > > > > > > > +1. > > > > > > > > > > Thanks, Piyush. > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, May 17, 2018 at 9:15 AM Colin McCabe <cmcc...@apache.org > > > > > > wrote: > > > > > > > > > > > > > Thanks, Piyush. +1 for starting the vote soon. > > > > > > > > > > > > > > Can you please also add a discussion about escaping? For > > example, > > > > > earlier > > > > > > > we discussed using backslashes to escape special characters. > So > > > that > > > > > users > > > > > > > can create an ACL referring to a literal "foo*" group by > creating > > > an > > > > > ACL > > > > > > > for "foo\*" Similarly, you can get a literal backslash with > > "\\". > > > > > This is > > > > > > > the standard UNIX escaping mechanism. > > > > > > > > > > > > > > Also, for the section that says "Changes to AdminClient (needs > > > > > > > discussion)", we need a new method that will allow users to > > escape > > > > > consumer > > > > > > > group names and other names. So you can feed this method your > > > > "foo\*" > > > > > > > consumer group name, and it will give you "foo\\\*", which is > > what > > > > you > > > > > > > would need to use to create an ACL for this consumer group in > > > > > AdminClient. > > > > > > > I think that's the only change we need to admin client > > > > > > > > > > > > > > regards, > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > On Thu, May 17, 2018, at 08:55, Piyush Vijay wrote: > > > > > > > > Hi Rajini/Colin, > > > > > > > > > > > > > > > > I will remove the wildcard principals from the scope for now, > > > > > updating > > > > > > > KIP > > > > > > > > right now and will open it for vote. > > > > > > > > > > > > > > > > Thanks > > > > > > > > > > > > > > > > > > > > > > > > Piyush Vijay > > > > > > > > > > > > > > > > On Thu, May 17, 2018 at 6:59 AM, Rajini Sivaram < > > > > > rajinisiva...@gmail.com > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hi Piyush, > > > > > > > > > > > > > > > > > > I have added a PR (https://github.com/apache/ > kafka/pull/5030 > > ) > > > > with > > > > > > > tests > > > > > > > > > to > > > > > > > > > show how group principals can be used for authorization > with > > > > custom > > > > > > > > > principal builders. One of the tests uses SASL. It is not > > quite > > > > the > > > > > > > same as > > > > > > > > > a full-fledged user groups, but since it works with all > > > security > > > > > > > protocols, > > > > > > > > > it could be an alternative to wildcarded principals. > > > > > > > > > > > > > > > > > > Let us know if we can help in any way to get this KIP > updated > > > and > > > > > > > ready for > > > > > > > > > voting to include in 2.0.0. > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > > > Rajini > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, May 16, 2018 at 10:21 PM, Colin McCabe < > > > > cmcc...@apache.org > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > On Tue, May 15, 2018 at 7:18 PM, Rajini Sivaram < > > > > > > > > > rajinisiva...@gmail.com > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > Hi Piyush, > > > > > > > > > > > > > > > > > > > > > > > > It is possible to configure PrincipalBuilder for > SASL ( > > > > > > > > > > > > https://cwiki.apache.org/ > confluence/display/KAFKA/KIP- > > > > > > > > > > > > 189%3A+Improve+principal+builder+interface+and+add+ > > > > > > > > > support+for+SASL). > > > > > > > > > > If > > > > > > > > > > > > that satisfies your requirements, perhaps we can move > > > > > wildcarded > > > > > > > > > > principals > > > > > > > > > > > > out of this KIP and focus on wildcarded resources? > > > > > > > > > > > > > > > > > > > > +1. > > > > > > > > > > > > > > > > > > > > We also need to determine which characters will be > reserved > > > for > > > > > the > > > > > > > > > > future. I think previously we thought about @, #, $, %, > ^, > > > &, > > > > *. > > > > > > > > > > > > > > > > > > > > > > On Tue, May 15, 2018 at 7:15 PM, Piyush Vijay < > > > > > > > > > piyushvij...@gmail.com> > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > >> Hi Colin, > > > > > > > > > > > >> > > > > > > > > > > > >> Escaping at this level is making sense to me but let > > me > > > > > think > > > > > > > more > > > > > > > > > > and get > > > > > > > > > > > >> back to you. > > > > > > > > > > > > > > > > > > > > Thanks, Piyush. What questions do you think are still > open > > > > > regarding > > > > > > > > > > escape characters? > > > > > > > > > > As Rajini mentioned, we have to get this in soon in order > > to > > > > make > > > > > > > the KIP > > > > > > > > > > freeze. > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > >> But should we not just get rid of one of AclBinding > or > > > > > > > > > > AclBindingFilter > > > > > > > > > > > >> then? Is there a reason to keep both given that > > > > > > > AclBindingFilter and > > > > > > > > > > > >> AclBinding look exact copy of each other after this > > > > change? > > > > > This > > > > > > > > > will > > > > > > > > > > be a > > > > > > > > > > > >> one-time breaking change in APIs marked as > "Evolving", > > > but > > > > > makes > > > > > > > > > > sense in > > > > > > > > > > > >> the long term? Am I missing something here? > > > > > > > > > > > > > > > > > > > > AclBinding represents an ACL. AclBindingFilter is a > filter > > > > which > > > > > > > can be > > > > > > > > > > used to locate AclBinding objects. Similarly with > Resource > > > and > > > > > > > > > > ResourceFilter. There is no reason to combine them > because > > > > they > > > > > > > > > represent > > > > > > > > > > different things. Although they contain many of the same > > > > fields, > > > > > > > they > > > > > > > > > are > > > > > > > > > > not exact copies. Many fields can be null in > > > > AclBindingFilter-- > > > > > > > fields > > > > > > > > > can > > > > > > > > > > never be null in AclBinding. > > > > > > > > > > > > > > > > > > > > For example, you can have an AclBindingFilter that > matches > > > > every > > > > > > > > > > AclBinding. There is more discussion of this on the > > original > > > > KIP > > > > > > > that > > > > > > > > > > added ACL support to AdminClient. > > > > > > > > > > > > > > > > > > > > best, > > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > >> Piyush Vijay > > > > > > > > > > > >> > > > > > > > > > > > >> On Tue, May 15, 2018 at 9:01 AM, Colin McCabe < > > > > > > > cmcc...@apache.org> > > > > > > > > > > wrote: > > > > > > > > > > > >> > > > > > > > > > > > >> > Hi Piyush, > > > > > > > > > > > >> > > > > > > > > > > > > >> > I think AclBinding should operate the same way as > > > > > > > > > AclBindingFilter. > > > > > > > > > > > >> > > > > > > > > > > > > >> > So you should be able to do something like this: > > > > > > > > > > > >> > > AclBindingFilter filter = new > AclBindingFiler(new > > > > > > > > > > > >> > ResourceFilter(ResourceType.GROUP, "foo*")) > > > > > > > > > > > >> > > AclBinding binding = new AclBinding(new > > > > > > > > > > Resource(ResourceType.GROUP, > > > > > > > > > > > >> > "foo*")) > > > > > > > > > > > >> > > assertTrue(filter.matches(binding)); > > > > > > > > > > > >> > > > > > > > > > > > > >> > Thinking about this more, it's starting to feel > > really > > > > > messy > > > > > > > to > > > > > > > > > > create > > > > > > > > > > > >> new > > > > > > > > > > > >> > "pattern" constructors for Resource and > > > > ResourceFilter. I > > > > > > > don't > > > > > > > > > > think > > > > > > > > > > > >> > people will be able to figure this out. Maybe we > > > should > > > > > just > > > > > > > > > have a > > > > > > > > > > > >> > limited compatibility break here, where it is now > > > > > required to > > > > > > > > > escape > > > > > > > > > > > >> weird > > > > > > > > > > > >> > consumer group names when creating ACLs for them. > > > > > > > > > > > >> > > > > > > > > > > > > >> > To future-proof this, we should reserve a bunch of > > > > > characters > > > > > > > at > > > > > > > > > > once, > > > > > > > > > > > >> > like *, @, $, %, ^, &, +, [, ], etc. If these > > > > characters > > > > > > > appear > > > > > > > > > in > > > > > > > > > > a > > > > > > > > > > > >> > resource name, it should be an error, unless they > > are > > > > > escaped > > > > > > > > > with a > > > > > > > > > > > >> > backslash. That way, we can use them in the > future. > > > We > > > > > > > should > > > > > > > > > > create a > > > > > > > > > > > >> > Resource.escapeName function which adds the > correct > > > > escape > > > > > > > > > > characters to > > > > > > > > > > > >> > resource names (so it would translate foo* into > > foo\*, > > > > > foo+bar > > > > > > > > > into > > > > > > > > > > > >> > foo\+bar, etc. etc. > > > > > > > > > > > >> > > > > > > > > > > > > >> > best, > > > > > > > > > > > >> > Colin > > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > > >> > On Mon, May 14, 2018, at 17:08, Piyush Vijay > wrote: > > > > > > > > > > > >> > > Colin, > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > createAcls take a AclBinding, however, instead > of > > > > > > > > > > AclBindingFilter. > > > > > > > > > > > >> What > > > > > > > > > > > >> > > are your thoughts here? > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > public abstract DescribeAclsResult > > > > > > > describeAcls(AclBindingFilter > > > > > > > > > > > >> > > filter, DescribeAclsOptions options); > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > public abstract CreateAclsResult > > > > createAcls(Collection< > > > > > > > > > > AclBinding> > > > > > > > > > > > >> > > acls, CreateAclsOptions options); > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > public abstract DeleteAclsResult > > > > > > > > > > > >> > > deleteAcls(Collection<AclBindingFilter> > filters, > > > > > > > > > > DeleteAclsOptions > > > > > > > > > > > >> > > options); > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > Thanks > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > Piyush Vijay > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > On Mon, May 14, 2018 at 9:26 AM, Andy Coates < > > > > > > > a...@confluent.io > > > > > > > > > > > > > > > > > > > > > >> wrote: > > > > > > > > > > > >> > > > > > > > > > > > > > >> > > > +1 > > > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > On 11 May 2018 at 17:14, Colin McCabe < > > > > > cmcc...@apache.org > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > >> > > > > > > > > > > > > > > >> > > > > Hi Andy, > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > I see what you mean. I guess my thought > here > > is > > > > > that > > > > > > > if the > > > > > > > > > > > >> fields > > > > > > > > > > > >> > are > > > > > > > > > > > >> > > > > private, we can change it later if we need > to. > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > I definitely agree that we should use the > > scheme > > > > you > > > > > > > > > describe > > > > > > > > > > for > > > > > > > > > > > >> > sending > > > > > > > > > > > >> > > > > ACLs over the wire (just the string + > version > > > > > number) > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > cheers, > > > > > > > > > > > >> > > > > Colin > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > On Fri, May 11, 2018, at 09:39, Andy Coates > > > wrote: > > > > > > > > > > > >> > > > > > i think I'm agreeing with you. I was > merely > > > > > suggesting > > > > > > > > > that > > > > > > > > > > > >> having > > > > > > > > > > > >> > an > > > > > > > > > > > >> > > > > > additional field that controls how the > > current > > > > > field > > > > > > > is > > > > > > > > > > > >> > interpreted is > > > > > > > > > > > >> > > > > more > > > > > > > > > > > >> > > > > > flexible / extensible in the future than > > > using a > > > > > > > 'union' > > > > > > > > > > style > > > > > > > > > > > >> > > > approach, > > > > > > > > > > > >> > > > > > where only one of several possible fields > > > should > > > > > be > > > > > > > > > > populated. > > > > > > > > > > > >> But > > > > > > > > > > > >> > > > it's a > > > > > > > > > > > >> > > > > > minor thing. > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > On 10 May 2018 at 09:29, Colin McCabe < > > > > > > > cmcc...@apache.org > > > > > > > > > > > > > > > > > > > > > >> wrote: > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > Hi Andy, > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > The issue that I was trying to solve > here > > is > > > > the > > > > > > > Java > > > > > > > > > API. > > > > > > > > > > > >> Right > > > > > > > > > > > >> > > > now, > > > > > > > > > > > >> > > > > > > someone can write "new > > > > > ResourceFilter(ResourceType. > > > > > > > > > > > >> > TRANSACTIONAL_ID, > > > > > > > > > > > >> > > > > > > "foo*") and have a ResourceFilter that > > > applies > > > > > to a > > > > > > > > > > > >> > Transactional ID > > > > > > > > > > > >> > > > > named > > > > > > > > > > > >> > > > > > > "foo*". This has to continue to work, > or > > > else > > > > > we > > > > > > > have > > > > > > > > > > broken > > > > > > > > > > > >> > > > > compatibility. > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > I was proposing that there would be > > > something > > > > > like > > > > > > > a new > > > > > > > > > > > >> function > > > > > > > > > > > >> > > > like > > > > > > > > > > > >> > > > > > > ResourceFilter.fromPattern( > > > > > > > > > ResourceType.TRANSACTIONAL_ID, > > > > > > > > > > > >> > "foo*") > > > > > > > > > > > >> > > > > which > > > > > > > > > > > >> > > > > > > would create a ResourceFilter that > applied > > > to > > > > > > > > > > transactional > > > > > > > > > > > >> IDs > > > > > > > > > > > >> > > > > starting > > > > > > > > > > > >> > > > > > > with "foo", rather than transactional > IDs > > > > named > > > > > > > "foo*" > > > > > > > > > > > >> > specifically. > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > I don't think it's important whether the > > > Java > > > > > class > > > > > > > has > > > > > > > > > an > > > > > > > > > > > >> > integer, > > > > > > > > > > > >> > > > an > > > > > > > > > > > >> > > > > > > enum, or two string fields. The > important > > > > > thing is > > > > > > > that > > > > > > > > > > > >> there's > > > > > > > > > > > >> > a > > > > > > > > > > > >> > > > new > > > > > > > > > > > >> > > > > > > static function, or new constructor > > > overload, > > > > > etc. > > > > > > > that > > > > > > > > > > works > > > > > > > > > > > >> for > > > > > > > > > > > >> > > > > patterns > > > > > > > > > > > >> > > > > > > rather than literal strings. > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > On Thu, May 10, 2018, at 03:30, Andy > > Coates > > > > > wrote: > > > > > > > > > > > >> > > > > > > > Rather than having name and pattern > > fields > > > > on > > > > > the > > > > > > > > > > > >> > ResourceFilter, > > > > > > > > > > > >> > > > > where > > > > > > > > > > > >> > > > > > > > it’s only valid for one to be set, and > > we > > > > > want to > > > > > > > > > > restrict > > > > > > > > > > > >> the > > > > > > > > > > > >> > > > > character > > > > > > > > > > > >> > > > > > > > set in case future enhancements need > > them, > > > > we > > > > > > > could > > > > > > > > > > instead > > > > > > > > > > > >> > add a > > > > > > > > > > > >> > > > new > > > > > > > > > > > >> > > > > > > > integer ‘nameType’ field, and use > > > constants > > > > to > > > > > > > > > indicate > > > > > > > > > > how > > > > > > > > > > > >> the > > > > > > > > > > > >> > > > name > > > > > > > > > > > >> > > > > > > > field should be interpreted, e.g. 0 = > > > > > literal, 1 = > > > > > > > > > > wildcard. > > > > > > > > > > > >> > This > > > > > > > > > > > >> > > > > would > > > > > > > > > > > >> > > > > > > > be extendable, e.g we can later add 2 > = > > > > > regex, or > > > > > > > what > > > > > > > > > > ever, > > > > > > > > > > > >> > and > > > > > > > > > > > >> > > > > > > > wouldn’t require any escaping. > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > This is very user-unfriendly, though. > > Users > > > > > don't > > > > > > > want > > > > > > > > > to > > > > > > > > > > > >> have > > > > > > > > > > > >> > to > > > > > > > > > > > >> > > > > > > explicitly supply a version number when > > > using > > > > > the > > > > > > > API, > > > > > > > > > > which > > > > > > > > > > > >> is > > > > > > > > > > > >> > what > > > > > > > > > > > >> > > > > this > > > > > > > > > > > >> > > > > > > would force them to do. I don't think > > users > > > > are > > > > > > > going > > > > > > > > > to > > > > > > > > > > > >> want to > > > > > > > > > > > >> > > > > memorize > > > > > > > > > > > >> > > > > > > that version 4 supprted "+", whereas > > > version 3 > > > > > only > > > > > > > > > > supported > > > > > > > > > > > >> > > > "[0-9]", > > > > > > > > > > > >> > > > > or > > > > > > > > > > > >> > > > > > > whatever. > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > Just as an example, do you remember > which > > > > > versions > > > > > > > of > > > > > > > > > > > >> > FetchRequest > > > > > > > > > > > >> > > > > added > > > > > > > > > > > >> > > > > > > which features? I don't. I always have > > to > > > > > look at > > > > > > > the > > > > > > > > > > code > > > > > > > > > > > >> to > > > > > > > > > > > >> > > > > remember. > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > Also, escaping is still required any > time > > > you > > > > > > > overload a > > > > > > > > > > > >> > character to > > > > > > > > > > > >> > > > > mean > > > > > > > > > > > >> > > > > > > two things. Escaping is required in the > > > > current > > > > > > > > > proposal > > > > > > > > > > to > > > > > > > > > > > >> be > > > > > > > > > > > >> > able > > > > > > > > > > > >> > > > to > > > > > > > > > > > >> > > > > > > create a pattern that matches only > "foo*". > > > > You > > > > > > > have to > > > > > > > > > > type > > > > > > > > > > > >> > "foo\*" > > > > > > > > > > > >> > > > > It > > > > > > > > > > > >> > > > > > > would be required if we forced users to > > > > specify > > > > > a > > > > > > > > > > version, as > > > > > > > > > > > >> > well. > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > best, > > > > > > > > > > > >> > > > > > > Colin > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > Sent from my iPhone > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > >> > > > > > > > > On 7 May 2018, at 05:16, Piyush > Vijay > > < > > > > > > > > > > > >> > piyushvij...@gmail.com> > > > > > > > > > > > >> > > > > wrote: > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > Makes sense. I'll update the KIP. > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > Does anyone have any other comments? > > :) > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > Thanks > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > > Piyush Vijay > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > >> > > > > > > > >> On Thu, May 3, 2018 at 11:55 AM, > > Colin > > > > > McCabe < > > > > > > > > > > > >> > > > cmcc...@apache.org > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > wrote: > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >> Yeah, I guess that's a good point. > > It > > > > > probably > > > > > > > > > makes > > > > > > > > > > > >> sense > > > > > > > > > > > >> > to > > > > > > > > > > > >> > > > > > > support the > > > > > > > > > > > >> > > > > > > > >> prefix scheme for consumer groups > and > > > > > > > transactional > > > > > > > > > > IDs > > > > > > > > > > > >> as > > > > > > > > > > > >> > well > > > > > > > > > > > >> > > > as > > > > > > > > > > > >> > > > > > > topics. > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >> I agree that the current situation > > > where > > > > > > > anything > > > > > > > > > > goes in > > > > > > > > > > > >> > > > consumer > > > > > > > > > > > >> > > > > > > group > > > > > > > > > > > >> > > > > > > > >> names and transactional ID names is > > not > > > > > > > ideal. I > > > > > > > > > > wish we > > > > > > > > > > > >> > could > > > > > > > > > > > >> > > > > > > rewind the > > > > > > > > > > > >> > > > > > > > >> clock and impose restrictions on > the > > > > names. > > > > > > > > > > However, it > > > > > > > > > > > >> > doesn't > > > > > > > > > > > >> > > > > seem > > > > > > > > > > > >> > > > > > > > >> practical at the moment. Adding > new > > > > > > > restrictions > > > > > > > > > > would > > > > > > > > > > > >> > break a > > > > > > > > > > > >> > > > > lot of > > > > > > > > > > > >> > > > > > > > >> existing users after an upgrade. > It > > > > would > > > > > be a > > > > > > > > > > really > > > > > > > > > > > >> bad > > > > > > > > > > > >> > > > upgrade > > > > > > > > > > > >> > > > > > > > >> experience. > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >> However, I think we can support > this > > > in a > > > > > > > > > compatible > > > > > > > > > > way. > > > > > > > > > > > >> > From > > > > > > > > > > > >> > > > > the > > > > > > > > > > > >> > > > > > > > >> perspective of AdminClient, we just > > > have > > > > to > > > > > > > add a > > > > > > > > > new > > > > > > > > > > > >> field > > > > > > > > > > > >> > to > > > > > > > > > > > >> > > > > > > > >> ResourceFilter. Currently, it has > > two > > > > > fields, > > > > > > > > > > > >> resourceType > > > > > > > > > > > >> > and > > > > > > > > > > > >> > > > > name: > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >>> /** > > > > > > > > > > > >> > > > > > > > >>> * A filter which matches Resource > > > > objects. > > > > > > > > > > > >> > > > > > > > >>> * > > > > > > > > > > > >> > > > > > > > >>> * The API for this class is still > > > > evolving > > > > > > > and we > > > > > > > > > > may > > > > > > > > > > > >> break > > > > > > > > > > > >> > > > > > > > >> compatibility in minor releases, if > > > > > necessary. > > > > > > > > > > > >> > > > > > > > >>> */ > > > > > > > > > > > >> > > > > > > > >>> @InterfaceStability.Evolving > > > > > > > > > > > >> > > > > > > > >>> public class ResourceFilter { > > > > > > > > > > > >> > > > > > > > >>> private final ResourceType > > > > > resourceType; > > > > > > > > > > > >> > > > > > > > >>> private final String name; > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >> We can add a third field, pattern. > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >> So the API will basically be, if I > > > > create a > > > > > > > > > > > >> > > > > > > ResourceFilter(resourceType=GROUP, > > > > > > > > > > > >> > > > > > > > >> name=foo*, pattern=null), it > applies > > > only > > > > > to > > > > > > > the > > > > > > > > > > consumer > > > > > > > > > > > >> > group > > > > > > > > > > > >> > > > > named > > > > > > > > > > > >> > > > > > > > >> "foo*". If I create a > > > > > > > > > ResourceFilter(resourceType=GR > > > > > > > > > > > >> OUP, > > > > > > > > > > > >> > > > > name=null, > > > > > > > > > > > >> > > > > > > > >> pattern=foo*), it applies to any > > > consumer > > > > > group > > > > > > > > > > starting > > > > > > > > > > > >> in > > > > > > > > > > > >> > > > "foo". > > > > > > > > > > > >> > > > > > > name > > > > > > > > > > > >> > > > > > > > >> and pattern cannot be both set at > the > > > > same > > > > > > > time. > > > > > > > > > > This > > > > > > > > > > > >> > preserves > > > > > > > > > > > >> > > > > > > > >> compatibility at the AdminClient > > level. > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >> It's possible that we will want to > > add > > > > more > > > > > > > types > > > > > > > > > of > > > > > > > > > > > >> > pattern in > > > > > > > > > > > >> > > > > the > > > > > > > > > > > >> > > > > > > > >> future. So we should reserve > > "special > > > > > > > characters" > > > > > > > > > > such > > > > > > > > > > > >> as > > > > > > > > > > > >> > +, /, > > > > > > > > > > > >> > > > > &, > > > > > > > > > > > >> > > > > > > %, #, > > > > > > > > > > > >> > > > > > > > >> $, etc. These characters should be > > > > > treated as > > > > > > > > > > special > > > > > > > > > > > >> > unless > > > > > > > > > > > >> > > > > they are > > > > > > > > > > > >> > > > > > > > >> prefixed with a backslash to escape > > > them. > > > > > This > > > > > > > > > will > > > > > > > > > > > >> allow > > > > > > > > > > > >> > us to > > > > > > > > > > > >> > > > > add > > > > > > > > > > > >> > > > > > > > >> support for using these characters > in > > > the > > > > > > > future > > > > > > > > > > without > > > > > > > > > > > >> > > > breaking > > > > > > > > > > > >> > > > > > > > >> compatibility. > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >> At the protocol level, we need a > new > > > API > > > > > > > version > > > > > > > > > for > > > > > > > > > > > >> > > > > > > CreateAclsRequest / > > > > > > > > > > > >> > > > > > > > >> DeleteAclsRequest. The new API > > version > > > > > will > > > > > > > send > > > > > > > > > all > > > > > > > > > > > >> > special > > > > > > > > > > > >> > > > > > > characters > > > > > > > > > > > >> > > > > > > > >> over the wire escaped rather than > > > > directly. > > > > > > > (So > > > > > > > > > > there > > > > > > > > > > > >> is no > > > > > > > > > > > >> > > > need > > > > > > > > > > > >> > > > > for > > > > > > > > > > > >> > > > > > > the > > > > > > > > > > > >> > > > > > > > >> equivalent of both "name" and > > > "pattern"-- > > > > > we > > > > > > > > > > translate > > > > > > > > > > > >> name > > > > > > > > > > > >> > > > into a > > > > > > > > > > > >> > > > > > > validly > > > > > > > > > > > >> > > > > > > > >> escaped pattern that matches only > one > > > > > thing, by > > > > > > > > > > adding > > > > > > > > > > > >> > escape > > > > > > > > > > > >> > > > > > > characters as > > > > > > > > > > > >> > > > > > > > >> appropriate.) The broker will > > validate > > > > the > > > > > > > new API > > > > > > > > > > > >> version > > > > > > > > > > > >> > and > > > > > > > > > > > >> > > > > reject > > > > > > > > > > > >> > > > > > > > >> malformed of unsupported patterns. > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >> At the ZK level, we can introduce a > > > > > protocol > > > > > > > > > version > > > > > > > > > > to > > > > > > > > > > > >> the > > > > > > > > > > > >> > data > > > > > > > > > > > >> > > > > in > > > > > > > > > > > >> > > > > > > ZK-- > > > > > > > > > > > >> > > > > > > > >> or store it under a different root. > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >> best, > > > > > > > > > > > >> > > > > > > > >> Colin > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > >> > > > > > > > >>> On Wed, May 2, 2018, at 18:09, > > Piyush > > > > > Vijay > > > > > > > wrote: > > > > > > > > > > > >> > > > > > > > >>> Thank you everyone for the > interest > > > and, > > > > > > > prompt > > > > > > > > > and > > > > > > > > > > > >> > valuable > > > > > > > > > > > >> > > > > > > feedback. I > > > > > > > > > > > >> > > > > > > > >>> really appreciate the quick > > > turnaround. > > > > > I’ve > > > > > > > tried > > > > > > > > > > to > > > > > > > > > > > >> > organize > > > > > > > > > > > >> > > > > the > > > > > > > > > > > >> > > > > > > > >> comments > > > > > > > > > > > >> > > > > > > > >>> into common headings. See my > replies > > > > > below: > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> *Case of ‘*’ might already be > > present > > > in > > > > > > > consumer > > > > > > > > > > groups > > > > > > > > > > > >> > and > > > > > > > > > > > >> > > > > > > > >> transactional > > > > > > > > > > > >> > > > > > > > >>> ids* > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> - We definitely need wildcard > ACLs > > > > > support > > > > > > > for > > > > > > > > > > > >> resources > > > > > > > > > > > >> > like > > > > > > > > > > > >> > > > > > > consumer > > > > > > > > > > > >> > > > > > > > >>> groups and transactional ids for > > the > > > > > reason > > > > > > > Andy > > > > > > > > > > > >> > mentioned. A > > > > > > > > > > > >> > > > > big > > > > > > > > > > > >> > > > > > > win > > > > > > > > > > > >> > > > > > > > >> of > > > > > > > > > > > >> > > > > > > > >>> this feature is that service > > > providers > > > > > don’t > > > > > > > > > have > > > > > > > > > > to > > > > > > > > > > > >> > track > > > > > > > > > > > >> > > > and > > > > > > > > > > > >> > > > > keep > > > > > > > > > > > >> > > > > > > > >>> up-to-date all the consumer > groups > > > > their > > > > > > > > > > customers are > > > > > > > > > > > >> > using. > > > > > > > > > > > >> > > > > > > > >>> - I agree with Andy’s thoughts > on > > > the > > > > > two > > > > > > > > > possible > > > > > > > > > > > >> ways. > > > > > > > > > > > >> > > > > > > > >>> - My vote would be to do the > > > breaking > > > > > change > > > > > > > > > > because > > > > > > > > > > > >> we > > > > > > > > > > > >> > > > should > > > > > > > > > > > >> > > > > > > > >> restrict > > > > > > > > > > > >> > > > > > > > >>> the format of consumer groups > and > > > > > > > transactional > > > > > > > > > > ids > > > > > > > > > > > >> > sooner > > > > > > > > > > > >> > > > than > > > > > > > > > > > >> > > > > > > later. > > > > > > > > > > > >> > > > > > > > >>> - Consumer groups and > > > transactional > > > > > ids > > > > > > > are > > > > > > > > > > basic > > > > > > > > > > > >> > Kafka > > > > > > > > > > > >> > > > > > > concepts. > > > > > > > > > > > >> > > > > > > > >>> There is a lot of value in > > > having a > > > > > > > defined > > > > > > > > > > naming > > > > > > > > > > > >> > > > > convention on > > > > > > > > > > > >> > > > > > > > >> these > > > > > > > > > > > >> > > > > > > > >>> concepts. > > > > > > > > > > > >> > > > > > > > >>> - This will help us not run > > into > > > > more > > > > > > > issues > > > > > > > > > > down > > > > > > > > > > > >> the > > > > > > > > > > > >> > > > line. > > > > > > > > > > > >> > > > > > > > >>> - I’m not sure if people > > actually > > > > use > > > > > > > ‘*’ in > > > > > > > > > > their > > > > > > > > > > > >> > > > consumer > > > > > > > > > > > >> > > > > > > group > > > > > > > > > > > >> > > > > > > > >>> names anyway. > > > > > > > > > > > >> > > > > > > > >>> - Escaping ‘*’ isn’t trivial > > > > because > > > > > ‘\’ > > > > > > > is > > > > > > > > > an > > > > > > > > > > > >> allowed > > > > > > > > > > > >> > > > > character > > > > > > > > > > > >> > > > > > > > >> too. > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> *Why introduce two new APIs?* > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> - It’s possible to make this > > change > > > > > without > > > > > > > > > > > >> introducing > > > > > > > > > > > >> > new > > > > > > > > > > > >> > > > > APIs > > > > > > > > > > > >> > > > > > > but > > > > > > > > > > > >> > > > > > > > >> new > > > > > > > > > > > >> > > > > > > > >>> APIs are required for > inspection. > > > > > > > > > > > >> > > > > > > > >>> - For example: If I want to > fetch > > > all > > > > > ACLs > > > > > > > that > > > > > > > > > > match > > > > > > > > > > > >> > > > > ’topicA*’, > > > > > > > > > > > >> > > > > > > it’s > > > > > > > > > > > >> > > > > > > > >>> not possible without introducing > > new > > > > > APIs > > > > > > > AND > > > > > > > > > > > >> maintaining > > > > > > > > > > > >> > > > > backwards > > > > > > > > > > > >> > > > > > > > >>> compatibility. > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> *Matching multiple hosts* > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> - Rajini is right that wildcard > > ACLs > > > > > aren’t > > > > > > > the > > > > > > > > > > > >> correct > > > > > > > > > > > >> > fit > > > > > > > > > > > >> > > > for > > > > > > > > > > > >> > > > > > > > >>> specifying range of hosts. > > > > > > > > > > > >> > > > > > > > >>> - We will rely on KIP-252 for > the > > > > proper > > > > > > > > > > > >> functionality ( > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> https://cwiki.apache.org/ > > > > > > > > > > confluence/display/KAFKA/KIP- > > > > > > > > > > > >> > > > > > > > >> 252+-+Extend+ACLs+to+allow+ > > > > > > > > > > filtering+based+on+ip+ranges+ > > > > > > > > > > > >> > > > > and+subnets > > > > > > > > > > > >> > > > > > > > >>> ) > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> *Implementation of matching > > algorithm > > > > and > > > > > > > > > > performance > > > > > > > > > > > >> > concerns* > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> - Updated the KIP with an > > > > > implementation. > > > > > > > > > > > >> > > > > > > > >>> - Andy, you’re right. The length > > > > doesn’t > > > > > > > play a > > > > > > > > > > part. > > > > > > > > > > > >> The > > > > > > > > > > > >> > > > > request > > > > > > > > > > > >> > > > > > > will > > > > > > > > > > > >> > > > > > > > >>> be authorized *iff* there is at > > > least > > > > > one > > > > > > > > > matching > > > > > > > > > > > >> ALLOW > > > > > > > > > > > >> > and > > > > > > > > > > > >> > > > no > > > > > > > > > > > >> > > > > > > > >> matching > > > > > > > > > > > >> > > > > > > > >>> DENY irrespective of the prefix > > > > length. > > > > > > > Included > > > > > > > > > > this > > > > > > > > > > > >> > detail > > > > > > > > > > > >> > > > > in the > > > > > > > > > > > >> > > > > > > > >> KIP. > > > > > > > > > > > >> > > > > > > > >>> - Since everything is stored in > > > > memory, > > > > > the > > > > > > > > > > > >> performance > > > > > > > > > > > >> > hit > > > > > > > > > > > >> > > > > isn’t > > > > > > > > > > > >> > > > > > > > >> really > > > > > > > > > > > >> > > > > > > > >>> significantly worse than the > > current > > > > > > > behavior. > > > > > > > > > > > >> > > > > > > > >>> - Stephane’s performance > > improvement > > > > > > > suggestion > > > > > > > > > > is a > > > > > > > > > > > >> > great > > > > > > > > > > > >> > > > > idea but > > > > > > > > > > > >> > > > > > > > >>> orthogonal. > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> *Why extend this for principal?* > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> - Thanks for the amazing points > > > > Rajini. > > > > > > > > > > > >> > > > > > > > >>> - There is a use case where ALL > > > > > principals > > > > > > > from > > > > > > > > > > an org > > > > > > > > > > > >> > might > > > > > > > > > > > >> > > > > want > > > > > > > > > > > >> > > > > > > to > > > > > > > > > > > >> > > > > > > > >>> access fix set of topics. > > > > > > > > > > > >> > > > > > > > >>> - User group functionality is > > > > currently > > > > > > > missing. > > > > > > > > > > > >> > > > > > > > >>> - IIRC SASL doesn’t use custom > > > > principal > > > > > > > > > builder. > > > > > > > > > > > >> > > > > > > > >>> - However, prefixing is not the > > > right > > > > > > > choice in > > > > > > > > > > all > > > > > > > > > > > >> > cases. > > > > > > > > > > > >> > > > > Agreed. > > > > > > > > > > > >> > > > > > > > >>> - Thoughts? > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> *Changes to AdminClient to support > > > > > wildcard > > > > > > > ACLs* > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> - Thanks Colin for the > > > implementation. > > > > > It’s > > > > > > > good > > > > > > > > > > to > > > > > > > > > > > >> have > > > > > > > > > > > >> > you > > > > > > > > > > > >> > > > > and > > > > > > > > > > > >> > > > > > > > >>> others > > > > > > > > > > > >> > > > > > > > >>> here for the expert opinions. > > > > > > > > > > > >> > > > > > > > >>> - The current implementation > uses > > > two > > > > > > > classes: > > > > > > > > > > > >> > AclBinding and > > > > > > > > > > > >> > > > > > > > >>> AclBindingFilter. ( > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> https://github.com/apache/ > > > > > > > > > > kafka/blob/trunk/clients/src/ > > > > > > > > > > > >> > > > > > > > >> main/java/org/apache/kafka/ > > > > > > > > > > common/acl/AclBinding.java > > > > > > > > > > > >> > > > > > > > >>> and > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> https://github.com/apache/ > > > > > > > > > > kafka/blob/trunk/clients/src/ > > > > > > > > > > > >> > > > > > > > >> main/java/org/apache/kafka/com > > > > > > > > > > > >> mon/acl/AclBindingFilter.java > > > > > > > > > > > >> > > > > > > > >>> ) > > > > > > > > > > > >> > > > > > > > >>> - AclBinding is definition of an > > > Acl. > > > > > It’s > > > > > > > used > > > > > > > > > to > > > > > > > > > > > >> create > > > > > > > > > > > >> > > > ACLs. > > > > > > > > > > > >> > > > > > > > >>> - AclBindingFilter is used to > > fetch > > > or > > > > > > > delete > > > > > > > > > > > >> “matching’ > > > > > > > > > > > >> > > > ACLs. > > > > > > > > > > > >> > > > > > > > >>> - In the context of wildcard > > > suffixed > > > > > ACLs, > > > > > > > a > > > > > > > > > > stored > > > > > > > > > > > >> ACL > > > > > > > > > > > >> > may > > > > > > > > > > > >> > > > > have > > > > > > > > > > > >> > > > > > > ‘*’ > > > > > > > > > > > >> > > > > > > > >>> in > > > > > > > > > > > >> > > > > > > > >>> it. *It really removes the > > > distinction > > > > > > > between > > > > > > > > > > these > > > > > > > > > > > >> two > > > > > > > > > > > >> > > > > classes.* > > > > > > > > > > > >> > > > > > > > >>> - The current implementation > uses > > > > > ‘null’ to > > > > > > > > > define > > > > > > > > > > > >> > wildcard > > > > > > > > > > > >> > > > ACL > > > > > > > > > > > >> > > > > > > > >>> (‘*’). I > > > > > > > > > > > >> > > > > > > > >>> think it’s not a good pattern > and > > we > > > > > should > > > > > > > use > > > > > > > > > > ‘*’ > > > > > > > > > > > >> for > > > > > > > > > > > >> > the > > > > > > > > > > > >> > > > > > > wildcard > > > > > > > > > > > >> > > > > > > > >>> ACL ( > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> https://github.com/apache/ > > > > > > > > > > kafka/blob/trunk/clients/src/ > > > > > > > > > > > >> > > > > > > > >> main/java/org/apache/kafka/ > > > > > > > > > > common/acl/AclBindingFilter. > > > > > > > > > > > >> > java#L39 > > > > > > > > > > > >> > > > > > > > >>> and > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> https://github.com/apache/ > > > > > > > > > > kafka/blob/trunk/clients/src/ > > > > > > > > > > > >> > > > > > > > >> main/java/org/apache/kafka/ > > > > > common/resource/ > > > > > > > > > > > >> > > > > ResourceFilter.java#L37 > > > > > > > > > > > >> > > > > > > > >>> ). > > > > > > > > > > > >> > > > > > > > >>> - However, the above two changes > > are > > > > > > > breaking > > > > > > > > > > change > > > > > > > > > > > >> but > > > > > > > > > > > >> > it > > > > > > > > > > > >> > > > > should > > > > > > > > > > > >> > > > > > > be > > > > > > > > > > > >> > > > > > > > >>> acceptable because the API is > > marked > > > > > with > > > > > > > > > > > >> > > > > > > > >>> @InterfaceStability.Evolving. > > > > > > > > > > > >> > > > > > > > >>> - If everyone agrees to the > above > > > two > > > > > > > changes > > > > > > > > > > (merging > > > > > > > > > > > >> > the > > > > > > > > > > > >> > > > two > > > > > > > > > > > >> > > > > > > > >>> classes > > > > > > > > > > > >> > > > > > > > >>> and using non-null values for > > > blanket > > > > > > > access), > > > > > > > > > the > > > > > > > > > > > >> only > > > > > > > > > > > >> > other > > > > > > > > > > > >> > > > > > > change > > > > > > > > > > > >> > > > > > > > >>> is > > > > > > > > > > > >> > > > > > > > >>> using the matching algorithm > from > > > the > > > > > KIP to > > > > > > > > > match > > > > > > > > > > > >> ACLs. > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> Other comments: > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> - > It may be worth excluding > > > > delegation > > > > > > > token > > > > > > > > > > ACLs > > > > > > > > > > > >> from > > > > > > > > > > > >> > > > using > > > > > > > > > > > >> > > > > > > > >> prefixed > > > > > > > > > > > >> > > > > > > > >>> wildcards since it doesn't make > > much > > > > > sense. > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> I want to ask for clarification on > > > what > > > > > > > delegation > > > > > > > > > > token > > > > > > > > > > > >> > ACLs > > > > > > > > > > > >> > > > are > > > > > > > > > > > >> > > > > > > before > > > > > > > > > > > >> > > > > > > > >>> commenting. Wildcard suffixed ACLs > > are > > > > > > > supported > > > > > > > > > > only > > > > > > > > > > > >> for > > > > > > > > > > > >> > > > > resource > > > > > > > > > > > >> > > > > > > and > > > > > > > > > > > >> > > > > > > > >>> principal names. > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> - A quick read makes me believe > > that > > > > > I’ve > > > > > > > fixed > > > > > > > > > > the > > > > > > > > > > > >> > > > formatting > > > > > > > > > > > >> > > > > > > issues > > > > > > > > > > > >> > > > > > > > >>> reported by Ted. Let me know if > > > > > something is > > > > > > > > > still > > > > > > > > > > > >> wrong > > > > > > > > > > > >> > and > > > > > > > > > > > >> > > > I > > > > > > > > > > > >> > > > > > > would > > > > > > > > > > > >> > > > > > > > >> be > > > > > > > > > > > >> > > > > > > > >>> happy to fix it. > > > > > > > > > > > >> > > > > > > > >>> - I’ve fixed the mismatch in > > > signature > > > > > > > reported > > > > > > > > > by > > > > > > > > > > > >> Ron. > > > > > > > > > > > >> > > > > > > > >>> - Andy, I’ve updated the KIP > with > > > the > > > > > > > security > > > > > > > > > > hole > > > > > > > > > > > >> > related > > > > > > > > > > > >> > > > to > > > > > > > > > > > >> > > > > DENY > > > > > > > > > > > >> > > > > > > > >>> wildcard ACLs ‘*’ on the > downgrade > > > > path. > > > > > > > > > > > >> > > > > > > > >>> - Wrt naming, wildcard suffixed > > ACLs > > > > > sound > > > > > > > > > > reasonable > > > > > > > > > > > >> to > > > > > > > > > > > >> > me > > > > > > > > > > > >> > > > > until > > > > > > > > > > > >> > > > > > > > >>> someone raise a major objection. > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> Let me know your thoughts. Looking > > > > > forward to > > > > > > > the > > > > > > > > > > next > > > > > > > > > > > >> > > > iteration. > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> Best, > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> Piyush > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> Piyush Vijay > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>> On Wed, May 2, 2018 at 1:33 PM, > Andy > > > > > Coates < > > > > > > > > > > > >> > a...@confluent.io > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > wrote: > > > > > > > > > > > >> > > > > > > > >>> > > > > > > > > > > > >> > > > > > > > >>>>> Perhaps there is a simpler way. > > It > > > > > seems > > > > > > > like > > > > > > > > > the > > > > > > > > > > > >> > resource > > > > > > > > > > > >> > > > > that > > > > > > > > > > > >> > > > > > > > >> people > > > > > > > > > > > >> > > > > > > > >>>> really want to use prefixed ACLs > > with > > > > is > > > > > > > topic > > > > > > > > > > names. > > > > > > > > > > > >> > Because > > > > > > > > > > > >> > > > > topic > > > > > > > > > > > >> > > > > > > > >> names > > > > > > > > > > > >> > > > > > > > >>>> can't contain "*", there is no > > > > ambiguity > > > > > > > there, > > > > > > > > > > > >> either. I > > > > > > > > > > > >> > > > think > > > > > > > > > > > >> > > > > > > maybe > > > > > > > > > > > >> > > > > > > > >> we > > > > > > > > > > > >> > > > > > > > >>>> should restrict the prefix > handling > > > to > > > > > topic > > > > > > > > > > resources. > > > > > > > > > > > >> > > > > > > > >>>> > > > > > > > > > > > >> > > > > > > > >>>> The KIP should cover consumer > > groups > > > > for > > > > > > > sure, > > > > > > > > > > > >> otherwise > > > > > > > > > > > >> > we're > > > > > > > > > > > >> > > > > back > > > > > > > > > > > >> > > > > > > to > > > > > > > > > > > >> > > > > > > > >> the > > > > > > > > > > > >> > > > > > > > >>>> situation users need to know, up > > > front, > > > > > the > > > > > > > set > > > > > > > > > of > > > > > > > > > > > >> > consumer > > > > > > > > > > > >> > > > > groups > > > > > > > > > > > >> > > > > > > an > > > > > > > > > > > >> > > > > > > > >>>> KStreams topology is going to > use. > > > > > > > > > > > >> > > > > > > > >>>> > > > > > > > > > > > >> > > > > > > > >>>> I'm assuming transactional > producer > > > Ids > > > > > > > would be > > > > > > > > > > the > > > > > > > > > > > >> same. > > > > > > > > > > > >> > > > > > > > >>>> > > > > > > > > > > > >> > > > > > > > >>>> The cleanest solution would be to > > > > > restrict > > > > > > > the > > > > > > > > > > > >> characters > > > > > > > > > > > >> > in > > > > > > > > > > > >> > > > > group > > > > > > > > > > > >> > > > > > > and > > > > > > > > > > > >> > > > > > > > >>>> transaction producer ids, but > > that's > > > a > > > > > > > breaking > > > > > > > > > > change > > > > > > > > > > > >> > that > > > > > > > > > > > >> > > > > might > > > > > > > > > > > >> > > > > > > > >> affect a > > > > > > > > > > > >> > > > > > > > >>>> lot of people. > > > > > > > > > > > >> > > > > > > > >>>> > > > > > > > > > > > >> > > > > > > > >>>> Another solution might be to add > a > > > > > protocol > > > > > > > > > > version to > > > > > > > > > > > >> the > > > > > > > > > > > >> > > > `Acl` > > > > > > > > > > > >> > > > > > > type, > > > > > > > > > > > >> > > > > > > > >> (not > > > > > > > > > > > >> > > > > > > > >>>> the current `version` field used > > for > > > > > > > optimistic > > > > > > > > > > > >> > concurrency > > > > > > > > > > > >> > > > > > > control), > > > > > > > > > > > >> > > > > > > > >>>> defaulting it to version 1 if it > is > > > not > > > > > > > present, > > > > > > > > > > and > > > > > > > > > > > >> > releasing > > > > > > > > > > > >> > > > > this > > > > > > > > > > > >> > > > > > > > >> change > > > > > > > > > > > >> > > > > > > > >>>> as version 2. This would at > least > > > > allow > > > > > us > > > > > > > to > > > > > > > > > > leave > > > > > > > > > > > >> the > > > > > > > > > > > >> > > > > version 1 > > > > > > > > > > > >> > > > > > > ACLs > > > > > > > > > > > >> > > > > > > > >>>> as-is, (which makes for a cleaner > > > > storey > > > > > if a > > > > > > > > > > cluster > > > > > > > > > > > >> is > > > > > > > > > > > >> > > > > > > downgraded). > > > > > > > > > > > >> > > > > > > > >> There > > > > > > > > > > > >> > > > > > > > >>>> is then potential to escape '*' > > when > > > > > writing > > > > > > > > > > version 2 > > > > > > > > > > > >> > ACLs. > > > > > > > > > > > >> > > > > (And > > > > > > > > > > > >> > > > > > > > >> introduce > > > > > > > > > > > >> > > > > > > > >>>> code to check for supported ACL > > > > versions > > > > > > > going > > > > > > > > > > > >> forward). > > > > > > > > > > > >> > > > > Though... > > > > > > > > > > > >> > > > > > > how > > > > > > > > > > > >> > > > > > > > >> do > > > > > > > > > > > >> > > > > > > > >>>> we escape? If the consumer group > is > > > > free > > > > > > > form, > > > > > > > > > > then any > > > > > > > > > > > >> > escape > > > > > > > > > > > >> > > > > > > > >> sequence is > > > > > > > > > > > >> > > > > > > > >>>> also valid. Aren't there > metrics > > > that > > > > > use > > > > > > > the > > > > > > > > > > group > > > > > > > > > > > >> > name? > > > > > > > > > > > >> > > > If > > > > > > > > > > > >> > > > > > > there > > > > > > > > > > > >> > > > > > > > >> are, > > > > > > > > > > > >> > > > > > > > >>>> I'd of thought we'd need to > > restrict > > > > the > > > > > > > char set > > > > > > > > > > > >> anyway. > > > > > > > > > > > >> > > > > > > > >>>> > > > > > > > > > > > >> > > > > > > > >>>>> On 2 May 2018 at 18:57, charly > > > molter > > > > < > > > > > > > > > > > >> > > > charly.mol...@gmail.com > > > > > > > > > > > >> > > > > > > > > > > > > > > > > >> > > > > > > wrote: > > > > > > > > > > > >> > > > > > > > >>>>> > > > > > > > > > > > >> > > > > > > > >>>>> The fact that consumer groups > and > > > > > > > > > > > >> transactionalProducerId > > > > > > > > > > > >> > > > don't > > > > > > > > > > > >> > > > > > > have > > > > > > > > > > > >> > > > > > > > >> a > > > > > > > > > > > >> > > > > > > > >>>>> strict format is problematic (we > > > have > > > > > > > problems > > > > > > > > > > with > > > > > > > > > > > >> > people > > > > > > > > > > > >> > > > with > > > > > > > > > > > >> > > > > > > empty > > > > > > > > > > > >> > > > > > > > >>>>> spaces at the end of their > > consumer > > > > > group > > > > > > > for > > > > > > > > > > > >> example). > > > > > > > > > > > >> > > > > > > > >>>>> With 2.0 around the corner and > the > > > > > > > possibility > > > > > > > > > to > > > > > > > > > > fix > > > > > > > > > > > >> > these > > > > > > > > > > > >> > > > > errors > > > > > > > > > > > >> > > > > > > > >> from > > > > > > > > > > > >> > > > > > > > >>>> the > > > > > > > > > > > >> > > > > > > > >>>>> past should we create a KIP to > > > > restrict > > > > > > > these > > > > > > > > > > names > > > > > > > > > > > >> like > > > > > > > > > > > >> > we > > > > > > > > > > > >> > > > do > > > > > > > > > > > >> > > > > for > > > > > > > > > > > >> > > > > > > > >>>> topics? > > > > > > > > > > > >> > > > > > > > >>>>> > > > > > > > > > > > >> > > > > > > > >>>>> Thanks! > > > > > > > > > > > >> > > > > > > > >>>>> Charly > > > > > > > > > > > >> > > > > > > > >>>>> > > > > > > > > > > > >> > > > > > > > >>>>> On Wed, May 2, 2018 at 6:22 PM > > Colin > > > > > McCabe > > > > > > > < > > > > > > > > > > > >> > > > > cmcc...@apache.org> > > > > > > > > > > > >> > > > > > > > >> wrote: > > > > > > > > > > > >> > > > > > > > >>>>> > > > > > > > > > > > >> > > > > > > > >>>>>> Hi Piyush, > > > > > > > > > > > >> > > > > > > > >>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>> Thanks for the KIP! It seems > > like > > > it > > > > > will > > > > > > > be > > > > > > > > > > really > > > > > > > > > > > >> > useful. > > > > > > > > > > > >> > > > > > > > >>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>> As Rajini commented, the names > > for > > > > some > > > > > > > > > resources > > > > > > > > > > > >> (such > > > > > > > > > > > >> > as > > > > > > > > > > > >> > > > > > > consumer > > > > > > > > > > > >> > > > > > > > >>>>>> groups) can include stars. So > > your > > > > > > > consumer > > > > > > > > > > group > > > > > > > > > > > >> > might be > > > > > > > > > > > >> > > > > named > > > > > > > > > > > >> > > > > > > > >>>> "foo*". > > > > > > > > > > > >> > > > > > > > >>>>>> We need a way of explicitly > > > referring > > > > > to > > > > > > > that > > > > > > > > > > > >> consumer > > > > > > > > > > > >> > group > > > > > > > > > > > >> > > > > name, > > > > > > > > > > > >> > > > > > > > >>>> rather > > > > > > > > > > > >> > > > > > > > >>>>>> than to the foo prefix. A > simple > > > way > > > > > > > would be > > > > > > > > > to > > > > > > > > > > > >> > escape the > > > > > > > > > > > >> > > > > star > > > > > > > > > > > >> > > > > > > > >> with > > > > > > > > > > > >> > > > > > > > >>>> a > > > > > > > > > > > >> > > > > > > > >>>>>> backslash: "foo\*" During the > > > > software > > > > > > > upgrade > > > > > > > > > > > >> > process, we > > > > > > > > > > > >> > > > > also > > > > > > > > > > > >> > > > > > > > >> need > > > > > > > > > > > >> > > > > > > > >>>> to > > > > > > > > > > > >> > > > > > > > >>>>>> translate all ACLs that refer > to > > > > "foo*" > > > > > > > into > > > > > > > > > > "foo\*". > > > > > > > > > > > >> > > > > Otherwise, > > > > > > > > > > > >> > > > > > > > >> the > > > > > > > > > > > >> > > > > > > > >>>>>> upgrade could create a security > > > hole > > > > by > > > > > > > > > granting > > > > > > > > > > more > > > > > > > > > > > >> > access > > > > > > > > > > > >> > > > > than > > > > > > > > > > > >> > > > > > > > >> the > > > > > > > > > > > >> > > > > > > > >>>>>> administrator intended. > > > > > > > > > > > >> > > > > > > > >>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>> Perhaps there is a simpler way. > > It > > > > > seems > > > > > > > like > > > > > > > > > > the > > > > > > > > > > > >> > resource > > > > > > > > > > > >> > > > > that > > > > > > > > > > > >> > > > > > > > >> people > > > > > > > > > > > >> > > > > > > > >>>>>> really want to use prefixed > ACLs > > > with > > > > > is > > > > > > > topic > > > > > > > > > > names. > > > > > > > > > > > >> > > > Because > > > > > > > > > > > >> > > > > > > > >> topic > > > > > > > > > > > >> > > > > > > > >>>>> names > > > > > > > > > > > >> > > > > > > > >>>>>> can't contain "*", there is no > > > > > ambiguity > > > > > > > there, > > > > > > > > > > > >> > either. I > > > > > > > > > > > >> > > > > think > > > > > > > > > > > >> > > > > > > > >> maybe > > > > > > > > > > > >> > > > > > > > >>>> we > > > > > > > > > > > >> > > > > > > > >>>>>> should restrict the prefix > > handling > > > > to > > > > > > > topic > > > > > > > > > > > >> resources. > > > > > > > > > > > >> > > > > > > > >>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>> Are the new "getMatchingAcls" > > > methods > > > > > > > needed? > > > > > > > > > It > > > > > > > > > > > >> seems > > > > > > > > > > > >> > like > > > > > > > > > > > >> > > > > they > > > > > > > > > > > >> > > > > > > > >> break > > > > > > > > > > > >> > > > > > > > >>>>>> encapsulation. All that the > > > calling > > > > > code > > > > > > > > > really > > > > > > > > > > > >> needs > > > > > > > > > > > >> > to do > > > > > > > > > > > >> > > > > is > > > > > > > > > > > >> > > > > > > > >> call > > > > > > > > > > > >> > > > > > > > >>>>>> Authorizer#authorize(session, > > > > > operation, > > > > > > > > > > resource). > > > > > > > > > > > >> The > > > > > > > > > > > >> > > > > > > authorizer > > > > > > > > > > > >> > > > > > > > >>>> knows > > > > > > > > > > > >> > > > > > > > >>>>>> that if it has an ACL allowing > > > access > > > > > to > > > > > > > topics > > > > > > > > > > > >> starting > > > > > > > > > > > >> > > > with > > > > > > > > > > > >> > > > > > > > >> "foo" and > > > > > > > > > > > >> > > > > > > > >>>>>> someone calls > authorize(foobar), > > it > > > > > should > > > > > > > > > allow > > > > > > > > > > > >> access. > > > > > > > > > > > >> > > > > It's not > > > > > > > > > > > >> > > > > > > > >>>>>> necessary for the calling code > to > > > > know > > > > > > > exactly > > > > > > > > > > what > > > > > > > > > > > >> the > > > > > > > > > > > >> > > > rules > > > > > > > > > > > >> > > > > are > > > > > > > > > > > >> > > > > > > > >> for > > > > > > > > > > > >> > > > > > > > >>>>>> authorization. The > > > > Authorizer#getAcls, > > > > > > > APIs > > > > > > > > > are > > > > > > > > > > only > > > > > > > > > > > >> > needed > > > > > > > > > > > >> > > > > when > > > > > > > > > > > >> > > > > > > > >> the > > > > > > > > > > > >> > > > > > > > >>>>>> AdminClient wants to list ACLs. > > > > > > > > > > > >> > > > > > > > >>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>> best, > > > > > > > > > > > >> > > > > > > > >>>>>> Colin > > > > > > > > > > > >> > > > > > > > >>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>> On Wed, May 2, 2018, at 03:31, > > > > Rajini > > > > > > > Sivaram > > > > > > > > > > wrote: > > > > > > > > > > > >> > > > > > > > >>>>>>> Hi Piyush, > > > > > > > > > > > >> > > > > > > > >>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>> Thanks for the KIP for this > > widely > > > > > > > requested > > > > > > > > > > > >> feature. > > > > > > > > > > > >> > A few > > > > > > > > > > > >> > > > > > > > >>>>>>> comments/questions: > > > > > > > > > > > >> > > > > > > > >>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>> 1. Some resource names can > > contain > > > > > '*'. > > > > > > > For > > > > > > > > > > example, > > > > > > > > > > > >> > > > consumer > > > > > > > > > > > >> > > > > > > > >> groups > > > > > > > > > > > >> > > > > > > > >>>> or > > > > > > > > > > > >> > > > > > > > >>>>>>> transactional ids. I am > > wondering > > > > > whether > > > > > > > we > > > > > > > > > > need to > > > > > > > > > > > >> > > > restrict > > > > > > > > > > > >> > > > > > > > >>>>> characters > > > > > > > > > > > >> > > > > > > > >>>>>>> for these entities or provide > a > > > way > > > > to > > > > > > > > > > distinguish > > > > > > > > > > > >> > > > wildcarded > > > > > > > > > > > >> > > > > > > > >>>> resource > > > > > > > > > > > >> > > > > > > > >>>>>> from > > > > > > > > > > > >> > > > > > > > >>>>>>> a resource containing the > > wildcard > > > > > > > character. > > > > > > > > > > > >> > > > > > > > >>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>> 2. I am not sure we want to do > > > > > wildcarded > > > > > > > > > > > >> principals. > > > > > > > > > > > >> > It > > > > > > > > > > > >> > > > > feels > > > > > > > > > > > >> > > > > > > > >> like a > > > > > > > > > > > >> > > > > > > > >>>>>>> workaround in the absence of > > user > > > > > groups. > > > > > > > In > > > > > > > > > the > > > > > > > > > > > >> longer > > > > > > > > > > > >> > > > > term, I > > > > > > > > > > > >> > > > > > > > >> think > > > > > > > > > > > >> > > > > > > > >>>>>>> groups (without wildcards) > would > > > be > > > > a > > > > > > > better > > > > > > > > > > option > > > > > > > > > > > >> to > > > > > > > > > > > >> > > > > configure > > > > > > > > > > > >> > > > > > > > >> ACLs > > > > > > > > > > > >> > > > > > > > >>>>> for > > > > > > > > > > > >> > > > > > > > >>>>>>> groups of users, rather than > > > > building > > > > > user > > > > > > > > > > > >> principals > > > > > > > > > > > >> > which > > > > > > > > > > > >> > > > > have > > > > > > > > > > > >> > > > > > > > >>>> common > > > > > > > > > > > >> > > > > > > > >>>>>>> prefixes. In the shorter term, > > > > since a > > > > > > > > > > configurable > > > > > > > > > > > >> > > > > > > > >> PrincipalBuilder > > > > > > > > > > > >> > > > > > > > >>>> is > > > > > > > > > > > >> > > > > > > > >>>>>>> used to build the principal > used > > > for > > > > > > > > > > authorization, > > > > > > > > > > > >> > perhaps > > > > > > > > > > > >> > > > > > > > >> using the > > > > > > > > > > > >> > > > > > > > >>>>>>> prefix itself as the principal > > > would > > > > > work > > > > > > > > > > (unless > > > > > > > > > > > >> > different > > > > > > > > > > > >> > > > > > > > >> quotas > > > > > > > > > > > >> > > > > > > > >>>> are > > > > > > > > > > > >> > > > > > > > >>>>>>> required for the full user > > name)? > > > > User > > > > > > > > > principal > > > > > > > > > > > >> > strings > > > > > > > > > > > >> > > > take > > > > > > > > > > > >> > > > > > > > >>>> different > > > > > > > > > > > >> > > > > > > > >>>>>>> formats for different security > > > > > protocols > > > > > > > (eg. > > > > > > > > > > > >> > > > > CN=xxx,O=org,C=UK > > > > > > > > > > > >> > > > > > > > >> for > > > > > > > > > > > >> > > > > > > > >>>>> SSL) > > > > > > > > > > > >> > > > > > > > >>>>>>> and simple prefixing isn't > > > probably > > > > > the > > > > > > > right > > > > > > > > > > > >> grouping > > > > > > > > > > > >> > in > > > > > > > > > > > >> > > > > many > > > > > > > > > > > >> > > > > > > > >> cases. > > > > > > > > > > > >> > > > > > > > >>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>> 3. I am assuming we don't want > > to > > > do > > > > > > > > > wildcarded > > > > > > > > > > > >> hosts > > > > > > > > > > > >> > in > > > > > > > > > > > >> > > > > this KIP > > > > > > > > > > > >> > > > > > > > >>>> since > > > > > > > > > > > >> > > > > > > > >>>>>>> wildcard suffix doesn't really > > > work > > > > > for > > > > > > > hosts. > > > > > > > > > > > >> > > > > > > > >>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>> 4. It may be worth excluding > > > > > delegation > > > > > > > token > > > > > > > > > > ACLs > > > > > > > > > > > >> from > > > > > > > > > > > >> > > > using > > > > > > > > > > > >> > > > > > > > >>>> prefixed > > > > > > > > > > > >> > > > > > > > >>>>>>> wildcards since it doesn't > make > > > much > > > > > > > sense. > > > > > > > > > > > >> > > > > > > > >>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>> Regards, > > > > > > > > > > > >> > > > > > > > >>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>> Rajini > > > > > > > > > > > >> > > > > > > > >>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>> On Wed, May 2, 2018 at 2:05 > AM, > > > > > Stephane > > > > > > > > > Maarek > > > > > > > > > > < > > > > > > > > > > > >> > > > > > > > >>>>>>> > steph...@simplemachines.com.au> > > > > > wrote: > > > > > > > > > > > >> > > > > > > > >>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>>> Hi, thanks for this badly > > needed > > > > > feature > > > > > > > > > > > >> > > > > > > > >>>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>>> 1) Why introduce two new APIs > > in > > > > > > > authorizer > > > > > > > > > > > >> instead of > > > > > > > > > > > >> > > > > > > > >> replacing > > > > > > > > > > > >> > > > > > > > >>>> the > > > > > > > > > > > >> > > > > > > > >>>>>>>> implementation for simple ACL > > > > > authorizer > > > > > > > with > > > > > > > > > > > >> adding > > > > > > > > > > > >> > the > > > > > > > > > > > >> > > > > > > > >> wildcard > > > > > > > > > > > >> > > > > > > > >>>>>>>> capability? > > > > > > > > > > > >> > > > > > > > >>>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>>> 2) is there an impact to > > > > performance > > > > > as > > > > > > > now > > > > > > > > > > we're > > > > > > > > > > > >> > > > evaluating > > > > > > > > > > > >> > > > > > > > >> more > > > > > > > > > > > >> > > > > > > > >>>>>> rules ? A > > > > > > > > > > > >> > > > > > > > >>>>>>>> while back I had evaluated > the > > > > > concept of > > > > > > > > > > cached > > > > > > > > > > > >> Acl > > > > > > > > > > > >> > > > result > > > > > > > > > > > >> > > > > so > > > > > > > > > > > >> > > > > > > > >>>>> swallow > > > > > > > > > > > >> > > > > > > > >>>>>> the > > > > > > > > > > > >> > > > > > > > >>>>>>>> cost of computing an > > > authorisation > > > > > cost > > > > > > > once > > > > > > > > > > and > > > > > > > > > > > >> then > > > > > > > > > > > >> > > > doing > > > > > > > > > > > >> > > > > in > > > > > > > > > > > >> > > > > > > > >>>> memory > > > > > > > > > > > >> > > > > > > > >>>>>>>> lookups. CF: > > > > > https://issues.apache.org/ > > > > > > > > > > > >> > > > > jira/browse/KAFKA-5261 > > > > > > > > > > > >> > > > > > > > >>>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>>> 3) is there any need to also > > > extend > > > > > this > > > > > > > to > > > > > > > > > > > >> consumer > > > > > > > > > > > >> > group > > > > > > > > > > > >> > > > > > > > >>>> resources > > > > > > > > > > > >> > > > > > > > >>>>> ? > > > > > > > > > > > >> > > > > > > > >>>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>>> 4) create topics KIP as > > recently > > > > > moved > > > > > > > > > > permissions > > > > > > > > > > > >> > out of > > > > > > > > > > > >> > > > > > > > >> Cluster > > > > > > > > > > > >> > > > > > > > >>>>> into > > > > > > > > > > > >> > > > > > > > >>>>>>>> Topic. Will wildcard be > > supported > > > > for > > > > > > > this > > > > > > > > > > action > > > > > > > > > > > >> too? > > > > > > > > > > > >> > > > > > > > >>>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>>> Thanks a lot for this ! > > > > > > > > > > > >> > > > > > > > >>>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>>> On Wed., 2 May 2018, 1:37 am > > Ted > > > > Yu, > > > > > < > > > > > > > > > > > >> > yuzhih...@gmail.com > > > > > > > > > > > >> > > > > > > > > > > > > > > > >> > > > > > > > >> wrote: > > > > > > > > > > > >> > > > > > > > >>>>>>>> > > > > > > > > > > > >> > > > > > > > >>>>>>>>> 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, > > > > > > > > > > > > > > > > > > > > > >
