kafka-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andy Coates <a...@confluent.io>
Subject Re: [DISCUSS] KIP-290: Support for wildcard suffixed ACLs
Date Sun, 20 May 2018 13:53:47 GMT
Awesome last minute effort Piyush.

Really appreciate your time and input,

Andy

Sent from my iPhone

> On 19 May 2018, at 03:43, Piyush Vijay <piyushvijay1@gmail.com> wrote:
> 
> Updated the KIP.
> 
> 1. New enum field 'ResourceNameType' in Resource and ResourceFilter classes.
> 2. modify getAcls() and rely on ResourceNameType' field in Resource to
> return either exact matches or all matches based on wildcard-suffix.
> 3. CLI changes to identify if resource name is literal or wildcard-suffix
> 4. Escaping doesn't work and isn't required if we're keeping a separate
> path on ZK (kafka-wildcard-acl) to store wildcard-suffix ACLs.
> 5. New API keys for Create / Delete / Describe Acls request with a new
> field in schemas for 'ResourceNameType'.
> 
> Looks ready to me for the vote, will start voting thread now. Thanks
> everyone for the valuable feedback.
> 
> 
> 
> 
> Piyush Vijay
> 
> 
> Piyush Vijay
> 
>> On Fri, May 18, 2018 at 6:07 PM, Andy Coates <andy@confluent.io> wrote:
>> 
>> Hi Piyush,
>> 
>> We're fast approaching the KIP deadline. Are you actively working on this?
>> If you're not I can take over.
>> 
>> Thanks,
>> 
>> Andy
>> 
>>> On 18 May 2018 at 14:25, Andy Coates <andy@confluent.io> wrote:
>>> 
>>> OK I've read it now.
>>> 
>>> 1. I see you have an example:
>>>> For example: If I want to fetch all ACLs that match ’topicA*’, it’s not
>>> possible without introducing new API AND maintaining backwards
>>> compatibility.
>>> getAcls takes a Resource, right, which would be either a full resource
>>> name or 'ALL', i.e. '*', right?  The point of the call is to get all ACLs
>>> relating to a specific resource, not a partial resource like 'topicA*'.
>>> Currently, I'm guessing / half-remembering that if you ask it for ACLs
>> for
>>> topic 'foo' it doesn't include global 'ALL' ACLs in the list - that would
>>> be a different call.  With the introduction of partial wildcards I think
>>> the _most_ backwards compatible change would be to have
>>> getAcls("topic:foo") to return all the ACLs, including that affect this
>>> topic. This could include any '*'/ALL Acls, (which would be a small
>>> backwards compatible change), or exclude them as it current does.
>>> Excluding any matching partial wildcard acl, e.g. 'f*' would break
>>> compatibility IMHO.
>>> 
>>> 2. Example command lines, showing how to add ACLs to specific resources
>>> that *end* with an asterisk char and adding wildcard-suffixed ACLs, would
>>> really help clarify the KIP. e.g.
>>> 
>>> bin/kafka-acls.sh --authorizer-properties zookeeper.connect=localhost:
>> 2181
>>> --add --allow-principal User:Bob --allow-principal User:Alice
>> --allow-host
>>> 198.51.100.0 --allow-host 198.51.100.1 --operation Read --group my-app-*
>>> 
>>> With the above command I can't see how the code can know if the user
>> means
>>> a literal group called 'my-app-*', or a wildcard suffix for any group
>>> starting with 'my-app-'. Escaping isn't enough as the escape char can
>> clash
>>> too, e.g. escaping a literal to 'my-app-\*' can still clash with someone
>>> wanting a wildcard sufiix matching any group starting with 'my-app-\'.
>>> 
>>> So there needs to be a syntax change here, I think.  Maybe some new
>>> command line switch to either explicitly enable or disable
>>> 'wildcard-suffix' support?  Probably defaulting to wildcard-suffix being
>>> on, (better experience going forward), though off is more backwards
>>> compatible.
>>> 
>>> 
>>> 3. Again, examples of how to store ACLs for specific resources that
>> *end* with
>>> an asterisk and wildcard-suffix ACLs, with any escaping would really
>> help.
>>> 
>>> 
>>> 
>>>> On 18 May 2018 at 13:55, Andy Coates <andy@confluent.io> wrote:
>>>> 
>>>> Hey Piyush,
>>>> 
>>>> Thanks for getting this in! :D
>>>> 
>>>> About to read now. But just quickly...
>>>> 
>>>> 1. I'll read up on the need for getMatchingAcls - but just playing
>> devils
>>>> advocate for a moment - if a current caller of getAcls() expects it to
>>>> return the full set of ACLs for a given resource, would post this change
>>>> only returning a sub set and requiring them to return getMatchingAcls to
>>>> get the full set not itself be a break in compatibility? I'm thinking
>> about
>>>> any tooling / UI / etc people may have built on top of this.  If Im
>> missing
>>>> the point, then maybe a concrete example, (if you've not already added
>> one
>>>> to the doc), may help.
>>>> 
>>>> 2. Something must change on the command line, surely? e.g. as command
>>>> line user how would the command differ if I wanted to add an ACL onto a
>>>> group called 'foo*' as opposed to a all groups starting with 'foo'?
>>>> 
>>>> 3. Thinking this through, I actually bracktracked - I don't think it
>> will
>>>> work due to path collisions, even with escaping - as the escaped version
>>>> can still collide.
>>>> 
>>>> Off to read the doc now.
>>>> 
>>>>> On 18 May 2018 at 13:33, Piyush Vijay <piyushvijay1@gmail.com> wrote:
>>>>> 
>>>>> Ready to review. Let me know if something looks missing or not clear.
>>>>> 
>>>>> Thanks
>>>>> 
>>>>> 
>>>>> Piyush Vijay
>>>>> 
>>>>> On Fri, May 18, 2018 at 12:54 PM, Piyush Vijay <piyushvijay1@gmail.com
>>> 
>>>>> wrote:
>>>>> 
>>>>>> Andy,
>>>>>> 
>>>>>> 1. Updated the KIP about need for getMatchingAcls(). Basically, it's
>>>>>> required to add an inspection method without breaking compatibility.
>>>>>> getAcls() only looks at a single location.
>>>>>> 
>>>>>> 2. Nothing will change from user's perspective. they will add /
>> delete/
>>>>>> list ACLs as earlier.
>>>>>> 
>>>>>> 3. Good point about moving everything to a v2 path. We might still
>>>>> have to
>>>>>> support snowflakes but I will this better.
>>>>>> 
>>>>>> I'm giving it a final read. I'll update here once I think it's ready.
>>>>>> 
>>>>>> Thanks
>>>>>> 
>>>>>> 
>>>>>> Piyush Vijay
>>>>>> 
>>>>>> On Fri, May 18, 2018 at 12:18 PM, Piyush Vijay <
>> piyushvijay1@gmail.com
>>>>>> 
>>>>>> wrote:
>>>>>> 
>>>>>>> On it, Andy. It should be out in next 30 mins.
>>>>>>> 
>>>>>>> On Fri, May 18, 2018 at 12:17 PM Andy Coates <andy@confluent.io>
>>>>> wrote:
>>>>>>> 
>>>>>>>> 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 <piyushvijay1@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 <andy@confluent.io>
>>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>>> Hey Piyush - my bad. Sorry.
>>>>>>>>>> 
>>>>>>>>>> On 17 May 2018 at 13:23, Piyush Vijay <piyushvijay1@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 <
>>>>> andy@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 <cmccabe@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 <
>>>>>>>> cmccabe@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 <
>>>>>>>>>>>>> rajinisivaram@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 <
>>>>>>>>>>>> cmccabe@apache.org
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> On Tue, May 15, 2018 at 7:18 PM, Rajini
>>>>> Sivaram <
>>>>>>>>>>>>>>>>> rajinisivaram@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+build
>>>>>>>> er+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 <
>>>>>>>>>>>>>>>>> piyushvijay1@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 <
>>>>>>>>>>>>>>> cmccabe@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 <
>>>>>>>>>>>>>>> andy@confluent.io
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>> +1
>>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>>> On 11 May 2018 at 17:14, Colin
>> McCabe
>>>>> <
>>>>>>>>>>>>> cmccabe@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
>>>>>>>> <
>>>>>>>>>>>>>>> cmccabe@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
>>>>>>>>>> <
>>>>>>>>>>>>>>>>>>>>>> piyushvijay1@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 <
>>>>>>>>>>>>>>>>>>>>>>>> cmccabe@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=GR
>>>>> OUP,
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 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
>>>>>>>>>>>>>>>>>>>> 
>>>>> 
>>>> ...
>>> 
>>> [Message clipped]
>> 

Mime
View raw message