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 Fri, 11 May 2018 16:39:19 GMT
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=GROUP,
> > >> name=foo*, pattern=null), it applies only to the consumer group named
> > >> "foo*".  If I create a ResourceFilter(resourceType=GROUP, 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/common/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 <andy@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.molter@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 <cmccabe@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 <
> > >>>>>>> stephane@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, <yuzhihong@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
<
> > >> andy@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 <andy@confluent.io>
> > >> wrote:
> > >>>>>>>>>>
> > >>>>>>>>>>> Hi Piyush,
> > >>>>>>>>>>>
> > >>>>>>>>>>> Thanks for raising this KIP - it's
very much appreciated.
> > >>>>>>>>>>>
> > >>>>>>>>>>> I've not had chance to digest it yet,
but...
> > >>>>>>>>>>>
> > >>>>>>>>>>> 1. you might want to add details of
how the internals of
> > >> the
> > >>>>>>>>>>> `getMatchingAcls` is implemented. We'd
want to make sure
> > >> the
> > >>>>>>>> complexity
> > >>>>>>>>>> of
> > >>>>>>>>>>> the operation isn't adversely affected.
> > >>>>>>>>>>> 2. You might want to be more explicit
that the length of
> > >> a
> > >>>>> prefix
> > >>>>>>>> does
> > >>>>>>>>>> not
> > >>>>>>>>>>> play a part in the `authorize` call,
e.g. given ACLs
> > >> {DENY,
> > >>>>>> some.*},
> > >>>>>>>>>> {ALLOW,
> > >>>>>>>>>>> some.prefix.*}, the longer, i.e. more
specific, allow ACL
> > >>>> does
> > >>>>>> _not_
> > >>>>>>>>>>> override the more general deny ACL.
> > >>>>>>>>>>>
> > >>>>>>>>>>> Thanks,
> > >>>>>>>>>>>
> > >>>>>>>>>>> Andy
> > >>>>>>>>>>>
> > >>>>>>>>>>> On 1 May 2018 at 16:59, Ron Dagostino
<rndgstn@gmail.com
> > >>>
> > >>>>> wrote:
> > >>>>>>>>>>>
> > >>>>>>>>>>>> Hi Piyush.  I appreciated your
talk at Kafka Summit and
> > >>>>>> appreciate
> > >>>>>>>> the
> > >>>>>>>>>> KIP
> > >>>>>>>>>>>> -- thanks.
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> Could you explain these mismatching
references?  Near
> > >> the
> > >>>> top
> > >>>>>> of the
> > >>>>>>>>> KIP
> > >>>>>>>>>>>> you refer to these proposed new
method signatures:
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> def getMatchingAcls(resource: Resource):
Set[Acl]
> > >>>>>>>>>>>> def getMatchingAcls(principal:
KafkaPrincipal):
> > >>>> Map[Resource,
> > >>>>>>>>> Set[Acl]]
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> But near the bottom of the KIP
you refer to different
> > >> method
> > >>>>>>>>>>>> signatures that don't seem to match
the above ones:
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> getMatchingAcls(topicRegex)
> > >>>>>>>>>>>> getMatchingAcls(principalRegex)
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> Ron
> > >>>>>>>>>>>>
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> On Tue, May 1, 2018 at 11:48 AM,
Ted Yu <
> > >>>> yuzhihong@gmail.com>
> > >>>>>>>> wrote:
> > >>>>>>>>>>>>
> > >>>>>>>>>>>>> The KIP was well written. Minor
comment on formatting:
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> https://github.com/apache/kafka/blob/trunk/core/src/
> > >>>>>>>>>>>>> main/scala/kafka/admin/AclCommand.scala
> > >>>>>>>>>>>>> to
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> Leave space between the URL
and 'to'
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> Can you describe changes for
the AdminClient ?
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> Thanks
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> On Tue, May 1, 2018 at 8:12
AM, Piyush Vijay <
> > >>>>>>>>> piyushvijay1@gmail.com>
> > >>>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>> Hi all,
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> I just opened a KIP to
add support for wildcard
> > >> suffixed
> > >>>>>> ACLs.
> > >>>>>>>>> This
> > >>>>>>>>>> is
> > >>>>>>>>>>>>> one
> > >>>>>>>>>>>>>> of the feature I talked
about in my Kafka summit
> > >> talk
> > >>>> and
> > >>>>> we
> > >>>>>>>>>> promised
> > >>>>>>>>>>>> to
> > >>>>>>>>>>>>>> upstream it :)
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> The details are here -
> > >>>>>>>>>>>>>> https://cwiki.apache.org/
> > >> confluence/display/KAFKA/KIP-
> > >>>>>>>>>>>>>> 290%3A+Support+for+wildcard+suffixed+ACLs
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> There is an open question
about the way to add the
> > >>>> support
> > >>>>>> in
> > >>>>>>>> the
> > >>>>>>>>>>>>>> AdminClient, which I can
discuss here in more detail
> > >>>> once
> > >>>>>>>> everyone
> > >>>>>>>>>> has
> > >>>>>>>>>>>>>> taken a first look at the
KIP.
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> Looking forward to discuss
the change.
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> Best,
> > >>>>>>>>>>>>>> Piyush Vijay
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>
> > >>>>>>>>>>>
> > >>>>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>
> > >>>>>>
> > >>>>>
> > >>>>>
> > >>>>> --
> > >>>>> Charly Molter
> > >>>>>
> > >>>>
> > >>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message