Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

Hello Ismael,

That is a fair suggestion. As of now, I do not see how Authorizer will be
used by clients and as such I am happy to adopt your suggested change.
Given that this KIP already went through multiple rounds of discussions and
votes, do we want to do another round of vote for this change, provided
there are no major concerns expressed? If the change is small enough, which
I think it is, I can just update the KIP and the associated PR. Let me know.
​

On Fri, Jun 24, 2016 at 12:34 AM, Ismael Juma  wrote:

> I'm really late to this thread, but I want to propose a small change: what
> if we changed the proposed package for the authorizer from
> `org.apache.kafka.common.security.auth` to
> `org.apache.kafka.server.security.auth` (while still living in the
> `clients` jar)?
>
> The advantages are:
> 1. It would be obvious that this is only used by the server.
> 2. We can set the checkstyle rules so that code in the `clients` and
> `common` packages don't call into the `server` package.
> 3. If we ever decide to move these server pluggable classes to their own
> module, we can do it without breaking source/binary compatibility, it would
> only require users to update their build configurations to depend on the
> new module (and in a transition period, we can make the `clients` module
> depend on this new module so that no change is required at all).
>
> The only downside I can see is that it's weird to have a `server` package
> in a `clients` jar, but that is just making explicit what is happening
> either way (we are adding a server-only interface to the `clients` jar).
>
> Thoughts?
>
> Ismael
>
> On Fri, Apr 22, 2016 at 10:56 PM, Ashish Singh 
> wrote:
>
> > Hey Guys,
> >
> > If there are no objections or major modifications to suggest I would like
> > to start a vote thread on this. I will wait till EOD today before
> starting
> > a vote thread.
> >
> > On Thu, Apr 21, 2016 at 4:36 PM, Gwen Shapira  wrote:
> >
> > > I would like to suggest taking the discussion of "how to break Kafka
> > > down into modules" outside the scope of KIP-50 and outside the scope
> > > of the next release.
> > >
> > > I understand that the current location of the authorizer API is not
> > > ideal, but I want to point out that the scope was already expanded
> > > from a new method to a complete rewrite of the authorizer. Is the
> > > current location really bad enough to expand the scope into larger
> > > refactoring of Kafka?
> > >
> > > Gwen
> > >
> > > On Wed, Apr 20, 2016 at 10:43 PM, Ismael Juma 
> wrote:
> > > > Hi Jay,
> > > >
> > > > Thanks for summarising the reasoning for the current approach. On the
> > > topic
> > > > of additional jars, the obvious example that came up recently is
> > sharing
> > > > JSON serializers between connect and streams. Given the desire not to
> > > add a
> > > > Jackson dependency to clients, it seems like adding a
> > > kafka-serializer-json
> > > > (or something like that) may be needed. This is similar to the
> > > > kafka-log4j-appender jar that we have today.
> > > >
> > > > When you look at it this way, then the situation is not as clear-cut
> as
> > > > initially described. Perhaps a way to explain this is that we only
> add
> > > > additional modules when they introduce a new dependency.
> > > >
> > > > Finally, it seems a bit weird to add something to `common` that is,
> in
> > > > fact, not common. Would it not make sense to have a separate package
> > for
> > > > pluggable core/server classes (because they are pluggable we want
> them
> > to
> > > > be in Java and not to be associated with a particular Scala version)?
> > > >
> > > > Ismael
> > > >
> > > > On Wed, Apr 20, 2016 at 4:52 PM, Jay Kreps  wrote:
> > > >
> > > >> Yeah our take when we came up with this approach was pretty much
> what
> > > Gwen
> > > >> is saying:
> > > >> 1. In practice you either need the server or client to do anything
> and
> > > the
> > > >> server depends on the client so bundling common and client doesn't
> > hurt.
> > > >> 2. Our experience with more granular jars (not in Kafka) was that
> > > although
> > > >> it feels "cleaner" the complexity comes quickly for a few reasons.
> > > First it
> > > >> gets hard to detangle the more granular packages (e.g. somebody
> needs
> > to
> > > >> use something in Utils in the authorizer package and then you no
> > longer
> > > >> have a dag). Second people end up mixing and matching in ways you
> > didn't
> > > >> anticipate which causes crazy heisenbugs (e.g. they depend on two
> > > different
> > > >> versions of the client via transitive dependencies and somehow end
> up
> > > with
> > > >> client version x and common version y due to duplicate entries on
> the
> > > class
> > > >> path).
> > > >>
> > > >> I'm not really arguing that this approach is superior, I'm just
> saying
> > > this
> > > >> is the current approach and that is the reason we 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ismael Juma]

I'm really late to this thread, but I want to propose a small change: what
if we changed the proposed package for the authorizer from
`org.apache.kafka.common.security.auth` to
`org.apache.kafka.server.security.auth` (while still living in the
`clients` jar)?

The advantages are:
1. It would be obvious that this is only used by the server.
2. We can set the checkstyle rules so that code in the `clients` and
`common` packages don't call into the `server` package.
3. If we ever decide to move these server pluggable classes to their own
module, we can do it without breaking source/binary compatibility, it would
only require users to update their build configurations to depend on the
new module (and in a transition period, we can make the `clients` module
depend on this new module so that no change is required at all).

The only downside I can see is that it's weird to have a `server` package
in a `clients` jar, but that is just making explicit what is happening
either way (we are adding a server-only interface to the `clients` jar).

Thoughts?

Ismael

On Fri, Apr 22, 2016 at 10:56 PM, Ashish Singh  wrote:

> Hey Guys,
>
> If there are no objections or major modifications to suggest I would like
> to start a vote thread on this. I will wait till EOD today before starting
> a vote thread.
>
> On Thu, Apr 21, 2016 at 4:36 PM, Gwen Shapira  wrote:
>
> > I would like to suggest taking the discussion of "how to break Kafka
> > down into modules" outside the scope of KIP-50 and outside the scope
> > of the next release.
> >
> > I understand that the current location of the authorizer API is not
> > ideal, but I want to point out that the scope was already expanded
> > from a new method to a complete rewrite of the authorizer. Is the
> > current location really bad enough to expand the scope into larger
> > refactoring of Kafka?
> >
> > Gwen
> >
> > On Wed, Apr 20, 2016 at 10:43 PM, Ismael Juma  wrote:
> > > Hi Jay,
> > >
> > > Thanks for summarising the reasoning for the current approach. On the
> > topic
> > > of additional jars, the obvious example that came up recently is
> sharing
> > > JSON serializers between connect and streams. Given the desire not to
> > add a
> > > Jackson dependency to clients, it seems like adding a
> > kafka-serializer-json
> > > (or something like that) may be needed. This is similar to the
> > > kafka-log4j-appender jar that we have today.
> > >
> > > When you look at it this way, then the situation is not as clear-cut as
> > > initially described. Perhaps a way to explain this is that we only add
> > > additional modules when they introduce a new dependency.
> > >
> > > Finally, it seems a bit weird to add something to `common` that is, in
> > > fact, not common. Would it not make sense to have a separate package
> for
> > > pluggable core/server classes (because they are pluggable we want them
> to
> > > be in Java and not to be associated with a particular Scala version)?
> > >
> > > Ismael
> > >
> > > On Wed, Apr 20, 2016 at 4:52 PM, Jay Kreps  wrote:
> > >
> > >> Yeah our take when we came up with this approach was pretty much what
> > Gwen
> > >> is saying:
> > >> 1. In practice you either need the server or client to do anything and
> > the
> > >> server depends on the client so bundling common and client doesn't
> hurt.
> > >> 2. Our experience with more granular jars (not in Kafka) was that
> > although
> > >> it feels "cleaner" the complexity comes quickly for a few reasons.
> > First it
> > >> gets hard to detangle the more granular packages (e.g. somebody needs
> to
> > >> use something in Utils in the authorizer package and then you no
> longer
> > >> have a dag). Second people end up mixing and matching in ways you
> didn't
> > >> anticipate which causes crazy heisenbugs (e.g. they depend on two
> > different
> > >> versions of the client via transitive dependencies and somehow end up
> > with
> > >> client version x and common version y due to duplicate entries on the
> > class
> > >> path).
> > >>
> > >> I'm not really arguing that this approach is superior, I'm just saying
> > this
> > >> is the current approach and that is the reason we went with it.
> > >>
> > >> So I could see splitting common and client and you could even further
> > split
> > >> the producer and consumer and multiple sub-jars in common, and if this
> > was
> > >> the approach I think a separate authorizer jar would make sense. But
> in
> > the
> > >> current approach I think the authorizer stuff would be most consistent
> > as a
> > >> public package in common. It is true that this means you build against
> > more
> > >> stuff then needed but I'm not sure this has any negative implications
> in
> > >> practice.
> > >>
> > >> -Jay
>

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

Hey Guys,

If there are no objections or major modifications to suggest I would like
to start a vote thread on this. I will wait till EOD today before starting
a vote thread.

On Thu, Apr 21, 2016 at 4:36 PM, Gwen Shapira  wrote:

> I would like to suggest taking the discussion of "how to break Kafka
> down into modules" outside the scope of KIP-50 and outside the scope
> of the next release.
>
> I understand that the current location of the authorizer API is not
> ideal, but I want to point out that the scope was already expanded
> from a new method to a complete rewrite of the authorizer. Is the
> current location really bad enough to expand the scope into larger
> refactoring of Kafka?
>
> Gwen
>
> On Wed, Apr 20, 2016 at 10:43 PM, Ismael Juma  wrote:
> > Hi Jay,
> >
> > Thanks for summarising the reasoning for the current approach. On the
> topic
> > of additional jars, the obvious example that came up recently is sharing
> > JSON serializers between connect and streams. Given the desire not to
> add a
> > Jackson dependency to clients, it seems like adding a
> kafka-serializer-json
> > (or something like that) may be needed. This is similar to the
> > kafka-log4j-appender jar that we have today.
> >
> > When you look at it this way, then the situation is not as clear-cut as
> > initially described. Perhaps a way to explain this is that we only add
> > additional modules when they introduce a new dependency.
> >
> > Finally, it seems a bit weird to add something to `common` that is, in
> > fact, not common. Would it not make sense to have a separate package for
> > pluggable core/server classes (because they are pluggable we want them to
> > be in Java and not to be associated with a particular Scala version)?
> >
> > Ismael
> >
> > On Wed, Apr 20, 2016 at 4:52 PM, Jay Kreps  wrote:
> >
> >> Yeah our take when we came up with this approach was pretty much what
> Gwen
> >> is saying:
> >> 1. In practice you either need the server or client to do anything and
> the
> >> server depends on the client so bundling common and client doesn't hurt.
> >> 2. Our experience with more granular jars (not in Kafka) was that
> although
> >> it feels "cleaner" the complexity comes quickly for a few reasons.
> First it
> >> gets hard to detangle the more granular packages (e.g. somebody needs to
> >> use something in Utils in the authorizer package and then you no longer
> >> have a dag). Second people end up mixing and matching in ways you didn't
> >> anticipate which causes crazy heisenbugs (e.g. they depend on two
> different
> >> versions of the client via transitive dependencies and somehow end up
> with
> >> client version x and common version y due to duplicate entries on the
> class
> >> path).
> >>
> >> I'm not really arguing that this approach is superior, I'm just saying
> this
> >> is the current approach and that is the reason we went with it.
> >>
> >> So I could see splitting common and client and you could even further
> split
> >> the producer and consumer and multiple sub-jars in common, and if this
> was
> >> the approach I think a separate authorizer jar would make sense. But in
> the
> >> current approach I think the authorizer stuff would be most consistent
> as a
> >> public package in common. It is true that this means you build against
> more
> >> stuff then needed but I'm not sure this has any negative implications in
> >> practice.
> >>
> >> -Jay
> >>
> >> On Wed, Apr 20, 2016 at 4:17 PM, Gwen Shapira 
> wrote:
> >>
> >> > But its just a compile-time dependency, right?
> >> > Since the third-party-authorizer-implementation will be installed on a
> >> > broker where the common classes will exist anyway.
> >> >
> >> >
> >> > On Wed, Apr 20, 2016 at 3:13 PM, Ashish Singh 
> >> wrote:
> >> > > Jay,
> >> > >
> >> > > Thanks for the info. I think having common in clients jar makes
> sense,
> >> as
> >> > > their is no direct usage of it. i.e., without depending on or using
> >> > > clients. Authorizer is a bit different, as third party
> implementations
> >> do
> >> > > not really need anything from clients or server, all they need is
> >> > > Authorizer interface and related classes. If we move authorizer into
> >> > > common, then third party implementations will have to depend on
> >> clients.
> >> > > Though third party implementations depending on clients is not a big
> >> > > problem, right now they depend on core, I think it is cleaner to
> have
> >> > > dependency on minimal modules. Would you agree?
> >> > >
> >> > > On Wed, Apr 20, 2016 at 2:27 PM, Jay Kreps 
> wrote:
> >> > >
> >> > >> I think it's great that we're moving the interface to java and
> fixing
> >> > some
> >> > >> of the naming foibles.
> >> > >>
> >> > >> This isn't explicit in the KIP which just refers to the java
> package
> >> > name
> >> > >> (I think), but it looks like you are proposing adding a new
> 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Gwen Shapira]

I would like to suggest taking the discussion of "how to break Kafka
down into modules" outside the scope of KIP-50 and outside the scope
of the next release.

I understand that the current location of the authorizer API is not
ideal, but I want to point out that the scope was already expanded
from a new method to a complete rewrite of the authorizer. Is the
current location really bad enough to expand the scope into larger
refactoring of Kafka?

Gwen

On Wed, Apr 20, 2016 at 10:43 PM, Ismael Juma  wrote:
> Hi Jay,
>
> Thanks for summarising the reasoning for the current approach. On the topic
> of additional jars, the obvious example that came up recently is sharing
> JSON serializers between connect and streams. Given the desire not to add a
> Jackson dependency to clients, it seems like adding a kafka-serializer-json
> (or something like that) may be needed. This is similar to the
> kafka-log4j-appender jar that we have today.
>
> When you look at it this way, then the situation is not as clear-cut as
> initially described. Perhaps a way to explain this is that we only add
> additional modules when they introduce a new dependency.
>
> Finally, it seems a bit weird to add something to `common` that is, in
> fact, not common. Would it not make sense to have a separate package for
> pluggable core/server classes (because they are pluggable we want them to
> be in Java and not to be associated with a particular Scala version)?
>
> Ismael
>
> On Wed, Apr 20, 2016 at 4:52 PM, Jay Kreps  wrote:
>
>> Yeah our take when we came up with this approach was pretty much what Gwen
>> is saying:
>> 1. In practice you either need the server or client to do anything and the
>> server depends on the client so bundling common and client doesn't hurt.
>> 2. Our experience with more granular jars (not in Kafka) was that although
>> it feels "cleaner" the complexity comes quickly for a few reasons. First it
>> gets hard to detangle the more granular packages (e.g. somebody needs to
>> use something in Utils in the authorizer package and then you no longer
>> have a dag). Second people end up mixing and matching in ways you didn't
>> anticipate which causes crazy heisenbugs (e.g. they depend on two different
>> versions of the client via transitive dependencies and somehow end up with
>> client version x and common version y due to duplicate entries on the class
>> path).
>>
>> I'm not really arguing that this approach is superior, I'm just saying this
>> is the current approach and that is the reason we went with it.
>>
>> So I could see splitting common and client and you could even further split
>> the producer and consumer and multiple sub-jars in common, and if this was
>> the approach I think a separate authorizer jar would make sense. But in the
>> current approach I think the authorizer stuff would be most consistent as a
>> public package in common. It is true that this means you build against more
>> stuff then needed but I'm not sure this has any negative implications in
>> practice.
>>
>> -Jay
>>
>> On Wed, Apr 20, 2016 at 4:17 PM, Gwen Shapira  wrote:
>>
>> > But its just a compile-time dependency, right?
>> > Since the third-party-authorizer-implementation will be installed on a
>> > broker where the common classes will exist anyway.
>> >
>> >
>> > On Wed, Apr 20, 2016 at 3:13 PM, Ashish Singh 
>> wrote:
>> > > Jay,
>> > >
>> > > Thanks for the info. I think having common in clients jar makes sense,
>> as
>> > > their is no direct usage of it. i.e., without depending on or using
>> > > clients. Authorizer is a bit different, as third party implementations
>> do
>> > > not really need anything from clients or server, all they need is
>> > > Authorizer interface and related classes. If we move authorizer into
>> > > common, then third party implementations will have to depend on
>> clients.
>> > > Though third party implementations depending on clients is not a big
>> > > problem, right now they depend on core, I think it is cleaner to have
>> > > dependency on minimal modules. Would you agree?
>> > >
>> > > On Wed, Apr 20, 2016 at 2:27 PM, Jay Kreps  wrote:
>> > >
>> > >> I think it's great that we're moving the interface to java and fixing
>> > some
>> > >> of the naming foibles.
>> > >>
>> > >> This isn't explicit in the KIP which just refers to the java package
>> > name
>> > >> (I think), but it looks like you are proposing adding a new authorizer
>> > jar
>> > >> for this new package and adding it as a dependency for the client jar.
>> > This
>> > >> is a bit inconsistent with how we decided to package stuff when we
>> > started
>> > >> with the new clients so it'd be good to work that out.
>> > >>
>> > >> To date the categorization has been:
>> > >> 1. Anything which is just in the clients is in org.apache.clients
>> under
>> > >> clients/
>> > >> 2. Anything which is in the server is kafka.* which is under 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ismael Juma]

Hi Jay,

Thanks for summarising the reasoning for the current approach. On the topic
of additional jars, the obvious example that came up recently is sharing
JSON serializers between connect and streams. Given the desire not to add a
Jackson dependency to clients, it seems like adding a kafka-serializer-json
(or something like that) may be needed. This is similar to the
kafka-log4j-appender jar that we have today.

When you look at it this way, then the situation is not as clear-cut as
initially described. Perhaps a way to explain this is that we only add
additional modules when they introduce a new dependency.

Finally, it seems a bit weird to add something to `common` that is, in
fact, not common. Would it not make sense to have a separate package for
pluggable core/server classes (because they are pluggable we want them to
be in Java and not to be associated with a particular Scala version)?

Ismael

On Wed, Apr 20, 2016 at 4:52 PM, Jay Kreps  wrote:

> Yeah our take when we came up with this approach was pretty much what Gwen
> is saying:
> 1. In practice you either need the server or client to do anything and the
> server depends on the client so bundling common and client doesn't hurt.
> 2. Our experience with more granular jars (not in Kafka) was that although
> it feels "cleaner" the complexity comes quickly for a few reasons. First it
> gets hard to detangle the more granular packages (e.g. somebody needs to
> use something in Utils in the authorizer package and then you no longer
> have a dag). Second people end up mixing and matching in ways you didn't
> anticipate which causes crazy heisenbugs (e.g. they depend on two different
> versions of the client via transitive dependencies and somehow end up with
> client version x and common version y due to duplicate entries on the class
> path).
>
> I'm not really arguing that this approach is superior, I'm just saying this
> is the current approach and that is the reason we went with it.
>
> So I could see splitting common and client and you could even further split
> the producer and consumer and multiple sub-jars in common, and if this was
> the approach I think a separate authorizer jar would make sense. But in the
> current approach I think the authorizer stuff would be most consistent as a
> public package in common. It is true that this means you build against more
> stuff then needed but I'm not sure this has any negative implications in
> practice.
>
> -Jay
>
> On Wed, Apr 20, 2016 at 4:17 PM, Gwen Shapira  wrote:
>
> > But its just a compile-time dependency, right?
> > Since the third-party-authorizer-implementation will be installed on a
> > broker where the common classes will exist anyway.
> >
> >
> > On Wed, Apr 20, 2016 at 3:13 PM, Ashish Singh 
> wrote:
> > > Jay,
> > >
> > > Thanks for the info. I think having common in clients jar makes sense,
> as
> > > their is no direct usage of it. i.e., without depending on or using
> > > clients. Authorizer is a bit different, as third party implementations
> do
> > > not really need anything from clients or server, all they need is
> > > Authorizer interface and related classes. If we move authorizer into
> > > common, then third party implementations will have to depend on
> clients.
> > > Though third party implementations depending on clients is not a big
> > > problem, right now they depend on core, I think it is cleaner to have
> > > dependency on minimal modules. Would you agree?
> > >
> > > On Wed, Apr 20, 2016 at 2:27 PM, Jay Kreps  wrote:
> > >
> > >> I think it's great that we're moving the interface to java and fixing
> > some
> > >> of the naming foibles.
> > >>
> > >> This isn't explicit in the KIP which just refers to the java package
> > name
> > >> (I think), but it looks like you are proposing adding a new authorizer
> > jar
> > >> for this new package and adding it as a dependency for the client jar.
> > This
> > >> is a bit inconsistent with how we decided to package stuff when we
> > started
> > >> with the new clients so it'd be good to work that out.
> > >>
> > >> To date the categorization has been:
> > >> 1. Anything which is just in the clients is in org.apache.clients
> under
> > >> clients/
> > >> 2. Anything which is in the server is kafka.* which is under core/
> > >> 3. Anything which is needed in both places (as it sounds like some
> enums
> > >> for authorization are?) is in common which is under clients/
> > >>
> > >> org.apache.clients and org.apache.common are both pure java and
> > dependency
> > >> free other than the compression libraries and slf4j and are packaged
> > into
> > >> the kafka-clients.java, the server has it's own jar which has richer
> > >> dependencies and depends on the client jar.
> > >>
> > >> There are other ways this could have been done--e.g. common could have
> > been
> > >> its own library or even split into multiple sub-libraries--but the
> > decision
> 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Gwen Shapira]

Thanks!

On Wed, Apr 20, 2016 at 8:00 PM, Ashish Singh  wrote:
> OK, in that case we can move the authorizer interface and related classes
> to existing org.apache.kafka.common.security.auth. I have updated KIP to
> reflect this.
>
>
> On Wed, Apr 20, 2016 at 4:52 PM, Jay Kreps  wrote:
>
>> Yeah our take when we came up with this approach was pretty much what Gwen
>> is saying:
>> 1. In practice you either need the server or client to do anything and the
>> server depends on the client so bundling common and client doesn't hurt.
>> 2. Our experience with more granular jars (not in Kafka) was that although
>> it feels "cleaner" the complexity comes quickly for a few reasons. First it
>> gets hard to detangle the more granular packages (e.g. somebody needs to
>> use something in Utils in the authorizer package and then you no longer
>> have a dag). Second people end up mixing and matching in ways you didn't
>> anticipate which causes crazy heisenbugs (e.g. they depend on two different
>> versions of the client via transitive dependencies and somehow end up with
>> client version x and common version y due to duplicate entries on the class
>> path).
>>
>> I'm not really arguing that this approach is superior, I'm just saying this
>> is the current approach and that is the reason we went with it.
>>
>> So I could see splitting common and client and you could even further split
>> the producer and consumer and multiple sub-jars in common, and if this was
>> the approach I think a separate authorizer jar would make sense. But in the
>> current approach I think the authorizer stuff would be most consistent as a
>> public package in common. It is true that this means you build against more
>> stuff then needed but I'm not sure this has any negative implications in
>> practice.
>>
>> -Jay
>>
>> On Wed, Apr 20, 2016 at 4:17 PM, Gwen Shapira  wrote:
>>
>> > But its just a compile-time dependency, right?
>> > Since the third-party-authorizer-implementation will be installed on a
>> > broker where the common classes will exist anyway.
>> >
>> >
>> > On Wed, Apr 20, 2016 at 3:13 PM, Ashish Singh 
>> wrote:
>> > > Jay,
>> > >
>> > > Thanks for the info. I think having common in clients jar makes sense,
>> as
>> > > their is no direct usage of it. i.e., without depending on or using
>> > > clients. Authorizer is a bit different, as third party implementations
>> do
>> > > not really need anything from clients or server, all they need is
>> > > Authorizer interface and related classes. If we move authorizer into
>> > > common, then third party implementations will have to depend on
>> clients.
>> > > Though third party implementations depending on clients is not a big
>> > > problem, right now they depend on core, I think it is cleaner to have
>> > > dependency on minimal modules. Would you agree?
>> > >
>> > > On Wed, Apr 20, 2016 at 2:27 PM, Jay Kreps  wrote:
>> > >
>> > >> I think it's great that we're moving the interface to java and fixing
>> > some
>> > >> of the naming foibles.
>> > >>
>> > >> This isn't explicit in the KIP which just refers to the java package
>> > name
>> > >> (I think), but it looks like you are proposing adding a new authorizer
>> > jar
>> > >> for this new package and adding it as a dependency for the client jar.
>> > This
>> > >> is a bit inconsistent with how we decided to package stuff when we
>> > started
>> > >> with the new clients so it'd be good to work that out.
>> > >>
>> > >> To date the categorization has been:
>> > >> 1. Anything which is just in the clients is in org.apache.clients
>> under
>> > >> clients/
>> > >> 2. Anything which is in the server is kafka.* which is under core/
>> > >> 3. Anything which is needed in both places (as it sounds like some
>> enums
>> > >> for authorization are?) is in common which is under clients/
>> > >>
>> > >> org.apache.clients and org.apache.common are both pure java and
>> > dependency
>> > >> free other than the compression libraries and slf4j and are packaged
>> > into
>> > >> the kafka-clients.java, the server has it's own jar which has richer
>> > >> dependencies and depends on the client jar.
>> > >>
>> > >> There are other ways this could have been done--e.g. common could have
>> > been
>> > >> its own library or even split into multiple sub-libraries--but the
>> > decision
>> > >> at that time was just to keep it simple and hard to mess up. Based on
>> > the
>> > >> experience with the scala clients our plan was to be ultra-hostile to
>> > any
>> > >> added client dependencies.
>> > >>
>> > >> So I think if we're continuing this model we would put the shared
>> > >> authorizer code somewhere under
>> > >> clients/src/main/java/org/apache/kafka/common as with the other shared
>> > >> authorizer. If we're moving away from this model we should probably
>> > rethink
>> > >> things and be consistent with this, at the very least splitting up

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

OK, in that case we can move the authorizer interface and related classes
to existing org.apache.kafka.common.security.auth. I have updated KIP to
reflect this.
​

On Wed, Apr 20, 2016 at 4:52 PM, Jay Kreps  wrote:

> Yeah our take when we came up with this approach was pretty much what Gwen
> is saying:
> 1. In practice you either need the server or client to do anything and the
> server depends on the client so bundling common and client doesn't hurt.
> 2. Our experience with more granular jars (not in Kafka) was that although
> it feels "cleaner" the complexity comes quickly for a few reasons. First it
> gets hard to detangle the more granular packages (e.g. somebody needs to
> use something in Utils in the authorizer package and then you no longer
> have a dag). Second people end up mixing and matching in ways you didn't
> anticipate which causes crazy heisenbugs (e.g. they depend on two different
> versions of the client via transitive dependencies and somehow end up with
> client version x and common version y due to duplicate entries on the class
> path).
>
> I'm not really arguing that this approach is superior, I'm just saying this
> is the current approach and that is the reason we went with it.
>
> So I could see splitting common and client and you could even further split
> the producer and consumer and multiple sub-jars in common, and if this was
> the approach I think a separate authorizer jar would make sense. But in the
> current approach I think the authorizer stuff would be most consistent as a
> public package in common. It is true that this means you build against more
> stuff then needed but I'm not sure this has any negative implications in
> practice.
>
> -Jay
>
> On Wed, Apr 20, 2016 at 4:17 PM, Gwen Shapira  wrote:
>
> > But its just a compile-time dependency, right?
> > Since the third-party-authorizer-implementation will be installed on a
> > broker where the common classes will exist anyway.
> >
> >
> > On Wed, Apr 20, 2016 at 3:13 PM, Ashish Singh 
> wrote:
> > > Jay,
> > >
> > > Thanks for the info. I think having common in clients jar makes sense,
> as
> > > their is no direct usage of it. i.e., without depending on or using
> > > clients. Authorizer is a bit different, as third party implementations
> do
> > > not really need anything from clients or server, all they need is
> > > Authorizer interface and related classes. If we move authorizer into
> > > common, then third party implementations will have to depend on
> clients.
> > > Though third party implementations depending on clients is not a big
> > > problem, right now they depend on core, I think it is cleaner to have
> > > dependency on minimal modules. Would you agree?
> > >
> > > On Wed, Apr 20, 2016 at 2:27 PM, Jay Kreps  wrote:
> > >
> > >> I think it's great that we're moving the interface to java and fixing
> > some
> > >> of the naming foibles.
> > >>
> > >> This isn't explicit in the KIP which just refers to the java package
> > name
> > >> (I think), but it looks like you are proposing adding a new authorizer
> > jar
> > >> for this new package and adding it as a dependency for the client jar.
> > This
> > >> is a bit inconsistent with how we decided to package stuff when we
> > started
> > >> with the new clients so it'd be good to work that out.
> > >>
> > >> To date the categorization has been:
> > >> 1. Anything which is just in the clients is in org.apache.clients
> under
> > >> clients/
> > >> 2. Anything which is in the server is kafka.* which is under core/
> > >> 3. Anything which is needed in both places (as it sounds like some
> enums
> > >> for authorization are?) is in common which is under clients/
> > >>
> > >> org.apache.clients and org.apache.common are both pure java and
> > dependency
> > >> free other than the compression libraries and slf4j and are packaged
> > into
> > >> the kafka-clients.java, the server has it's own jar which has richer
> > >> dependencies and depends on the client jar.
> > >>
> > >> There are other ways this could have been done--e.g. common could have
> > been
> > >> its own library or even split into multiple sub-libraries--but the
> > decision
> > >> at that time was just to keep it simple and hard to mess up. Based on
> > the
> > >> experience with the scala clients our plan was to be ultra-hostile to
> > any
> > >> added client dependencies.
> > >>
> > >> So I think if we're continuing this model we would put the shared
> > >> authorizer code somewhere under
> > >> clients/src/main/java/org/apache/kafka/common as with the other shared
> > >> authorizer. If we're moving away from this model we should probably
> > rethink
> > >> things and be consistent with this, at the very least splitting up
> > common
> > >> and clients.
> > >>
> > >> -Jay
> > >>
> > >> On Wed, Apr 20, 2016 at 1:04 PM, Ashish Singh 
> > wrote:
> > >>
> > >> > Jun/ Jay/ Gwen/ Harsha/ Ismael,
> 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Jay Kreps]

Yeah our take when we came up with this approach was pretty much what Gwen
is saying:
1. In practice you either need the server or client to do anything and the
server depends on the client so bundling common and client doesn't hurt.
2. Our experience with more granular jars (not in Kafka) was that although
it feels "cleaner" the complexity comes quickly for a few reasons. First it
gets hard to detangle the more granular packages (e.g. somebody needs to
use something in Utils in the authorizer package and then you no longer
have a dag). Second people end up mixing and matching in ways you didn't
anticipate which causes crazy heisenbugs (e.g. they depend on two different
versions of the client via transitive dependencies and somehow end up with
client version x and common version y due to duplicate entries on the class
path).

I'm not really arguing that this approach is superior, I'm just saying this
is the current approach and that is the reason we went with it.

So I could see splitting common and client and you could even further split
the producer and consumer and multiple sub-jars in common, and if this was
the approach I think a separate authorizer jar would make sense. But in the
current approach I think the authorizer stuff would be most consistent as a
public package in common. It is true that this means you build against more
stuff then needed but I'm not sure this has any negative implications in
practice.

-Jay

On Wed, Apr 20, 2016 at 4:17 PM, Gwen Shapira  wrote:

> But its just a compile-time dependency, right?
> Since the third-party-authorizer-implementation will be installed on a
> broker where the common classes will exist anyway.
>
>
> On Wed, Apr 20, 2016 at 3:13 PM, Ashish Singh  wrote:
> > Jay,
> >
> > Thanks for the info. I think having common in clients jar makes sense, as
> > their is no direct usage of it. i.e., without depending on or using
> > clients. Authorizer is a bit different, as third party implementations do
> > not really need anything from clients or server, all they need is
> > Authorizer interface and related classes. If we move authorizer into
> > common, then third party implementations will have to depend on clients.
> > Though third party implementations depending on clients is not a big
> > problem, right now they depend on core, I think it is cleaner to have
> > dependency on minimal modules. Would you agree?
> >
> > On Wed, Apr 20, 2016 at 2:27 PM, Jay Kreps  wrote:
> >
> >> I think it's great that we're moving the interface to java and fixing
> some
> >> of the naming foibles.
> >>
> >> This isn't explicit in the KIP which just refers to the java package
> name
> >> (I think), but it looks like you are proposing adding a new authorizer
> jar
> >> for this new package and adding it as a dependency for the client jar.
> This
> >> is a bit inconsistent with how we decided to package stuff when we
> started
> >> with the new clients so it'd be good to work that out.
> >>
> >> To date the categorization has been:
> >> 1. Anything which is just in the clients is in org.apache.clients under
> >> clients/
> >> 2. Anything which is in the server is kafka.* which is under core/
> >> 3. Anything which is needed in both places (as it sounds like some enums
> >> for authorization are?) is in common which is under clients/
> >>
> >> org.apache.clients and org.apache.common are both pure java and
> dependency
> >> free other than the compression libraries and slf4j and are packaged
> into
> >> the kafka-clients.java, the server has it's own jar which has richer
> >> dependencies and depends on the client jar.
> >>
> >> There are other ways this could have been done--e.g. common could have
> been
> >> its own library or even split into multiple sub-libraries--but the
> decision
> >> at that time was just to keep it simple and hard to mess up. Based on
> the
> >> experience with the scala clients our plan was to be ultra-hostile to
> any
> >> added client dependencies.
> >>
> >> So I think if we're continuing this model we would put the shared
> >> authorizer code somewhere under
> >> clients/src/main/java/org/apache/kafka/common as with the other shared
> >> authorizer. If we're moving away from this model we should probably
> rethink
> >> things and be consistent with this, at the very least splitting up
> common
> >> and clients.
> >>
> >> -Jay
> >>
> >> On Wed, Apr 20, 2016 at 1:04 PM, Ashish Singh 
> wrote:
> >>
> >> > Jun/ Jay/ Gwen/ Harsha/ Ismael,
> >> >
> >> > As you guys have provided feedback on this earlier, could you review
> the
> >> > KIP again? I have updated the PR <
> >> https://github.com/apache/kafka/pull/861>
> >> > as
> >> > well.
> >> >
> >> > On Wed, Apr 20, 2016 at 1:01 PM, Ashish Singh 
> >> wrote:
> >> >
> >> > > Hi Grant,
> >> > >
> >> > > On Tue, Apr 19, 2016 at 8:13 AM, Grant Henke 
> >> > wrote:
> >> > >
> >> > >> Hi 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Gwen Shapira]

But its just a compile-time dependency, right?
Since the third-party-authorizer-implementation will be installed on a
broker where the common classes will exist anyway.


On Wed, Apr 20, 2016 at 3:13 PM, Ashish Singh  wrote:
> Jay,
>
> Thanks for the info. I think having common in clients jar makes sense, as
> their is no direct usage of it. i.e., without depending on or using
> clients. Authorizer is a bit different, as third party implementations do
> not really need anything from clients or server, all they need is
> Authorizer interface and related classes. If we move authorizer into
> common, then third party implementations will have to depend on clients.
> Though third party implementations depending on clients is not a big
> problem, right now they depend on core, I think it is cleaner to have
> dependency on minimal modules. Would you agree?
>
> On Wed, Apr 20, 2016 at 2:27 PM, Jay Kreps  wrote:
>
>> I think it's great that we're moving the interface to java and fixing some
>> of the naming foibles.
>>
>> This isn't explicit in the KIP which just refers to the java package name
>> (I think), but it looks like you are proposing adding a new authorizer jar
>> for this new package and adding it as a dependency for the client jar. This
>> is a bit inconsistent with how we decided to package stuff when we started
>> with the new clients so it'd be good to work that out.
>>
>> To date the categorization has been:
>> 1. Anything which is just in the clients is in org.apache.clients under
>> clients/
>> 2. Anything which is in the server is kafka.* which is under core/
>> 3. Anything which is needed in both places (as it sounds like some enums
>> for authorization are?) is in common which is under clients/
>>
>> org.apache.clients and org.apache.common are both pure java and dependency
>> free other than the compression libraries and slf4j and are packaged into
>> the kafka-clients.java, the server has it's own jar which has richer
>> dependencies and depends on the client jar.
>>
>> There are other ways this could have been done--e.g. common could have been
>> its own library or even split into multiple sub-libraries--but the decision
>> at that time was just to keep it simple and hard to mess up. Based on the
>> experience with the scala clients our plan was to be ultra-hostile to any
>> added client dependencies.
>>
>> So I think if we're continuing this model we would put the shared
>> authorizer code somewhere under
>> clients/src/main/java/org/apache/kafka/common as with the other shared
>> authorizer. If we're moving away from this model we should probably rethink
>> things and be consistent with this, at the very least splitting up common
>> and clients.
>>
>> -Jay
>>
>> On Wed, Apr 20, 2016 at 1:04 PM, Ashish Singh  wrote:
>>
>> > Jun/ Jay/ Gwen/ Harsha/ Ismael,
>> >
>> > As you guys have provided feedback on this earlier, could you review the
>> > KIP again? I have updated the PR <
>> https://github.com/apache/kafka/pull/861>
>> > as
>> > well.
>> >
>> > On Wed, Apr 20, 2016 at 1:01 PM, Ashish Singh 
>> wrote:
>> >
>> > > Hi Grant,
>> > >
>> > > On Tue, Apr 19, 2016 at 8:13 AM, Grant Henke 
>> > wrote:
>> > >
>> > >> Hi Ashish,
>> > >>
>> > >> Thanks for the updates. I have a few questions below:
>> > >>
>> > >> > Move following interfaces to new package,
>> org.apche.kafka.authorizer.
>> > >> >
>> > >> >1. Authorizer
>> > >> >2. Acl
>> > >> >3. Operation
>> > >> >4. PermissionType
>> > >> >5. Resource
>> > >> >6. ResourceType
>> > >> >7. KafkaPrincipal
>> > >> >8. Session
>> > >> >
>> > >> >
>> > >> This means the client would be required to depend on the authorizer
>> > >> package
>> > >> as a part of KIP-4. Another option is to have the client objects in
>> > >> common.
>> > >> Have we ruled out leaving the interface in the core module?
>> > >>
>> > >  With this entities that use Authorizer will depend only on Authorizer
>> > > package. Third party implementations can have only the authorizer pkg
>> as
>> > > dependency. core and client modules will also have to depend on the
>> > > authorizer with this approach. Do you see any issue with it?
>> > >
>> > >>
>> > >> Authorizer interface will be updated to remove getter naming
>> convention.
>> > >>
>> > >>
>> > >> Now that this is Java do we still want to change to the Scala naming
>> > >> convention?
>> > >>
>> > > Even in clients module I do not see getter naming convention being
>> > > followed, it is better to be consistent I guess.
>> > >
>> > >>
>> > >>
>> > >> Since we are completely rewriting the interface, can we add some (at
>> > least
>> > >> one to start with) standard exceptions that each method is recommended
>> > to
>> > >> use/throw? This will help the server in KIP-4 provide meaningful error
>> > >> codes. KAFKA-3507 
>> is
>> > >> tracking it 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

Jay,

Thanks for the info. I think having common in clients jar makes sense, as
their is no direct usage of it. i.e., without depending on or using
clients. Authorizer is a bit different, as third party implementations do
not really need anything from clients or server, all they need is
Authorizer interface and related classes. If we move authorizer into
common, then third party implementations will have to depend on clients.
Though third party implementations depending on clients is not a big
problem, right now they depend on core, I think it is cleaner to have
dependency on minimal modules. Would you agree?

On Wed, Apr 20, 2016 at 2:27 PM, Jay Kreps  wrote:

> I think it's great that we're moving the interface to java and fixing some
> of the naming foibles.
>
> This isn't explicit in the KIP which just refers to the java package name
> (I think), but it looks like you are proposing adding a new authorizer jar
> for this new package and adding it as a dependency for the client jar. This
> is a bit inconsistent with how we decided to package stuff when we started
> with the new clients so it'd be good to work that out.
>
> To date the categorization has been:
> 1. Anything which is just in the clients is in org.apache.clients under
> clients/
> 2. Anything which is in the server is kafka.* which is under core/
> 3. Anything which is needed in both places (as it sounds like some enums
> for authorization are?) is in common which is under clients/
>
> org.apache.clients and org.apache.common are both pure java and dependency
> free other than the compression libraries and slf4j and are packaged into
> the kafka-clients.java, the server has it's own jar which has richer
> dependencies and depends on the client jar.
>
> There are other ways this could have been done--e.g. common could have been
> its own library or even split into multiple sub-libraries--but the decision
> at that time was just to keep it simple and hard to mess up. Based on the
> experience with the scala clients our plan was to be ultra-hostile to any
> added client dependencies.
>
> So I think if we're continuing this model we would put the shared
> authorizer code somewhere under
> clients/src/main/java/org/apache/kafka/common as with the other shared
> authorizer. If we're moving away from this model we should probably rethink
> things and be consistent with this, at the very least splitting up common
> and clients.
>
> -Jay
>
> On Wed, Apr 20, 2016 at 1:04 PM, Ashish Singh  wrote:
>
> > Jun/ Jay/ Gwen/ Harsha/ Ismael,
> >
> > As you guys have provided feedback on this earlier, could you review the
> > KIP again? I have updated the PR <
> https://github.com/apache/kafka/pull/861>
> > as
> > well.
> >
> > On Wed, Apr 20, 2016 at 1:01 PM, Ashish Singh 
> wrote:
> >
> > > Hi Grant,
> > >
> > > On Tue, Apr 19, 2016 at 8:13 AM, Grant Henke 
> > wrote:
> > >
> > >> Hi Ashish,
> > >>
> > >> Thanks for the updates. I have a few questions below:
> > >>
> > >> > Move following interfaces to new package,
> org.apche.kafka.authorizer.
> > >> >
> > >> >1. Authorizer
> > >> >2. Acl
> > >> >3. Operation
> > >> >4. PermissionType
> > >> >5. Resource
> > >> >6. ResourceType
> > >> >7. KafkaPrincipal
> > >> >8. Session
> > >> >
> > >> >
> > >> This means the client would be required to depend on the authorizer
> > >> package
> > >> as a part of KIP-4. Another option is to have the client objects in
> > >> common.
> > >> Have we ruled out leaving the interface in the core module?
> > >>
> > >  With this entities that use Authorizer will depend only on Authorizer
> > > package. Third party implementations can have only the authorizer pkg
> as
> > > dependency. core and client modules will also have to depend on the
> > > authorizer with this approach. Do you see any issue with it?
> > >
> > >>
> > >> Authorizer interface will be updated to remove getter naming
> convention.
> > >>
> > >>
> > >> Now that this is Java do we still want to change to the Scala naming
> > >> convention?
> > >>
> > > Even in clients module I do not see getter naming convention being
> > > followed, it is better to be consistent I guess.
> > >
> > >>
> > >>
> > >> Since we are completely rewriting the interface, can we add some (at
> > least
> > >> one to start with) standard exceptions that each method is recommended
> > to
> > >> use/throw? This will help the server in KIP-4 provide meaningful error
> > >> codes. KAFKA-3507 
> is
> > >> tracking it right now.
> > >>
> > > That should be good to have. Will include that. Thanks.
> > >
> > >>
> > >> Thanks,
> > >> Grant
> > >>
> > >>
> > >>
> > >> On Tue, Apr 19, 2016 at 9:48 AM, Ashish Singh 
> > >> wrote:
> > >>
> > >> > I have updated KIP-50
> > >> > <
> > >> >
> > >>
> >
> 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Jay Kreps]

I think it's great that we're moving the interface to java and fixing some
of the naming foibles.

This isn't explicit in the KIP which just refers to the java package name
(I think), but it looks like you are proposing adding a new authorizer jar
for this new package and adding it as a dependency for the client jar. This
is a bit inconsistent with how we decided to package stuff when we started
with the new clients so it'd be good to work that out.

To date the categorization has been:
1. Anything which is just in the clients is in org.apache.clients under
clients/
2. Anything which is in the server is kafka.* which is under core/
3. Anything which is needed in both places (as it sounds like some enums
for authorization are?) is in common which is under clients/

org.apache.clients and org.apache.common are both pure java and dependency
free other than the compression libraries and slf4j and are packaged into
the kafka-clients.java, the server has it's own jar which has richer
dependencies and depends on the client jar.

There are other ways this could have been done--e.g. common could have been
its own library or even split into multiple sub-libraries--but the decision
at that time was just to keep it simple and hard to mess up. Based on the
experience with the scala clients our plan was to be ultra-hostile to any
added client dependencies.

So I think if we're continuing this model we would put the shared
authorizer code somewhere under
clients/src/main/java/org/apache/kafka/common as with the other shared
authorizer. If we're moving away from this model we should probably rethink
things and be consistent with this, at the very least splitting up common
and clients.

-Jay

On Wed, Apr 20, 2016 at 1:04 PM, Ashish Singh  wrote:

> Jun/ Jay/ Gwen/ Harsha/ Ismael,
>
> As you guys have provided feedback on this earlier, could you review the
> KIP again? I have updated the PR 
> as
> well.
>
> On Wed, Apr 20, 2016 at 1:01 PM, Ashish Singh  wrote:
>
> > Hi Grant,
> >
> > On Tue, Apr 19, 2016 at 8:13 AM, Grant Henke 
> wrote:
> >
> >> Hi Ashish,
> >>
> >> Thanks for the updates. I have a few questions below:
> >>
> >> > Move following interfaces to new package, org.apche.kafka.authorizer.
> >> >
> >> >1. Authorizer
> >> >2. Acl
> >> >3. Operation
> >> >4. PermissionType
> >> >5. Resource
> >> >6. ResourceType
> >> >7. KafkaPrincipal
> >> >8. Session
> >> >
> >> >
> >> This means the client would be required to depend on the authorizer
> >> package
> >> as a part of KIP-4. Another option is to have the client objects in
> >> common.
> >> Have we ruled out leaving the interface in the core module?
> >>
> >  With this entities that use Authorizer will depend only on Authorizer
> > package. Third party implementations can have only the authorizer pkg as
> > dependency. core and client modules will also have to depend on the
> > authorizer with this approach. Do you see any issue with it?
> >
> >>
> >> Authorizer interface will be updated to remove getter naming convention.
> >>
> >>
> >> Now that this is Java do we still want to change to the Scala naming
> >> convention?
> >>
> > Even in clients module I do not see getter naming convention being
> > followed, it is better to be consistent I guess.
> >
> >>
> >>
> >> Since we are completely rewriting the interface, can we add some (at
> least
> >> one to start with) standard exceptions that each method is recommended
> to
> >> use/throw? This will help the server in KIP-4 provide meaningful error
> >> codes. KAFKA-3507  is
> >> tracking it right now.
> >>
> > That should be good to have. Will include that. Thanks.
> >
> >>
> >> Thanks,
> >> Grant
> >>
> >>
> >>
> >> On Tue, Apr 19, 2016 at 9:48 AM, Ashish Singh 
> >> wrote:
> >>
> >> > I have updated KIP-50
> >> > <
> >> >
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-50+-+Move+Authorizer+to+a+separate+package
> >> > >
> >> > and PR  as per recent
> >> > discussions. Please take a look.
> >> >
> >> > @Harsha / Don, it would be nice if you guys can review the KIP and PR
> as
> >> > well.
> >> > ​
> >> >
> >> > On Mon, Apr 11, 2016 at 7:36 PM, Ashish Singh 
> >> wrote:
> >> >
> >> > > Yes, Jun. I would like to try get option 2 in, if possible in 0.10.
> I
> >> am
> >> > > not asking for delaying 0.10 for it, but some reviews and early
> >> feedback
> >> > > would be great. At this point this is what I have in mind.
> >> > >
> >> > > 1. Move authorizer and related entities to its own package. Note
> that
> >> I
> >> > am
> >> > > proposing to drop scala interface completely. Ranger team is fine
> >> with it
> >> > > and I will update Sentry.
> >> > > 2. The only new public method that will be added to authorizer
> >> interface
> >> > > 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

Jun/ Jay/ Gwen/ Harsha/ Ismael,

As you guys have provided feedback on this earlier, could you review the
KIP again? I have updated the PR  as
well.

On Wed, Apr 20, 2016 at 1:01 PM, Ashish Singh  wrote:

> Hi Grant,
>
> On Tue, Apr 19, 2016 at 8:13 AM, Grant Henke  wrote:
>
>> Hi Ashish,
>>
>> Thanks for the updates. I have a few questions below:
>>
>> > Move following interfaces to new package, org.apche.kafka.authorizer.
>> >
>> >1. Authorizer
>> >2. Acl
>> >3. Operation
>> >4. PermissionType
>> >5. Resource
>> >6. ResourceType
>> >7. KafkaPrincipal
>> >8. Session
>> >
>> >
>> This means the client would be required to depend on the authorizer
>> package
>> as a part of KIP-4. Another option is to have the client objects in
>> common.
>> Have we ruled out leaving the interface in the core module?
>>
>  With this entities that use Authorizer will depend only on Authorizer
> package. Third party implementations can have only the authorizer pkg as
> dependency. core and client modules will also have to depend on the
> authorizer with this approach. Do you see any issue with it?
>
>>
>> Authorizer interface will be updated to remove getter naming convention.
>>
>>
>> Now that this is Java do we still want to change to the Scala naming
>> convention?
>>
> Even in clients module I do not see getter naming convention being
> followed, it is better to be consistent I guess.
>
>>
>>
>> Since we are completely rewriting the interface, can we add some (at least
>> one to start with) standard exceptions that each method is recommended to
>> use/throw? This will help the server in KIP-4 provide meaningful error
>> codes. KAFKA-3507  is
>> tracking it right now.
>>
> That should be good to have. Will include that. Thanks.
>
>>
>> Thanks,
>> Grant
>>
>>
>>
>> On Tue, Apr 19, 2016 at 9:48 AM, Ashish Singh 
>> wrote:
>>
>> > I have updated KIP-50
>> > <
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-50+-+Move+Authorizer+to+a+separate+package
>> > >
>> > and PR  as per recent
>> > discussions. Please take a look.
>> >
>> > @Harsha / Don, it would be nice if you guys can review the KIP and PR as
>> > well.
>> > ​
>> >
>> > On Mon, Apr 11, 2016 at 7:36 PM, Ashish Singh 
>> wrote:
>> >
>> > > Yes, Jun. I would like to try get option 2 in, if possible in 0.10. I
>> am
>> > > not asking for delaying 0.10 for it, but some reviews and early
>> feedback
>> > > would be great. At this point this is what I have in mind.
>> > >
>> > > 1. Move authorizer and related entities to its own package. Note that
>> I
>> > am
>> > > proposing to drop scala interface completely. Ranger team is fine
>> with it
>> > > and I will update Sentry.
>> > > 2. The only new public method that will be added to authorizer
>> interface
>> > > is description().
>> > > 3. Update SimpleAclAuthorizer to use the new interface and classes.
>> > >
>> > > On Mon, Apr 11, 2016 at 6:38 PM, Jun Rao  wrote:
>> > >
>> > >> Ashish,
>> > >>
>> > >> So, you want to take a shot at option 2 for 0.10.0? That's fine with
>> me
>> > >> too. I am just not sure if we have enough time to think through the
>> > >> changes.
>> > >>
>> > >> Thanks,
>> > >>
>> > >> Jun
>> > >>
>> > >> On Mon, Apr 11, 2016 at 6:05 PM, Ashish Singh 
>> > >> wrote:
>> > >>
>> > >> > Hello Jun,
>> > >> >
>> > >> > The 3rd option will require Apache Sentry to go GA with current
>> > >> authorizer
>> > >> > interface, and at this point it seems that the interface won't last
>> > >> long.
>> > >> > Within a few months, Sentry will have to make a breaking change. I
>> do
>> > >> > understand that Kafka should not have to delay its release due to
>> one
>> > of
>> > >> > the authorizer implementations. However, can we assist Sentry
>> users to
>> > >> > avoid that breaking upgrade? I think it is worth a shot. If the
>> > changes
>> > >> are
>> > >> > not done by 0.10 code freeze, then sure lets punt it to next
>> release.
>> > >> Does
>> > >> > this seem reasonable to you?
>> > >> >
>> > >> > On Sun, Apr 10, 2016 at 11:42 AM, Jun Rao 
>> wrote:
>> > >> >
>> > >> > > Ashish,
>> > >> > >
>> > >> > > A 3rd option is to in 0.10.0, just sanity check the principal
>> type
>> > in
>> > >> the
>> > >> > > implementation of addAcls/removeAcls of Authorizer, but don't
>> change
>> > >> the
>> > >> > > Authorizer api to add the getDescription() method. This fixes the
>> > >> > immediate
>> > >> > > issue that an acl rule with the wrong principal type is silently
>> > >> ignored.
>> > >> > > Knowing valid user types is nice, but not critical (we can
>> include
>> > the
>> > >> > > supported user type in the UnsupportedPrincipalTypeException
>> thrown
>> > >> from
>> > >> > > addAcls/removeAcls). This 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

Hi Grant,

On Tue, Apr 19, 2016 at 8:13 AM, Grant Henke  wrote:

> Hi Ashish,
>
> Thanks for the updates. I have a few questions below:
>
> > Move following interfaces to new package, org.apche.kafka.authorizer.
> >
> >1. Authorizer
> >2. Acl
> >3. Operation
> >4. PermissionType
> >5. Resource
> >6. ResourceType
> >7. KafkaPrincipal
> >8. Session
> >
> >
> This means the client would be required to depend on the authorizer package
> as a part of KIP-4. Another option is to have the client objects in common.
> Have we ruled out leaving the interface in the core module?
>
 With this entities that use Authorizer will depend only on Authorizer
package. Third party implementations can have only the authorizer pkg as
dependency. core and client modules will also have to depend on the
authorizer with this approach. Do you see any issue with it?

>
> Authorizer interface will be updated to remove getter naming convention.
>
>
> Now that this is Java do we still want to change to the Scala naming
> convention?
>
Even in clients module I do not see getter naming convention being
followed, it is better to be consistent I guess.

>
>
> Since we are completely rewriting the interface, can we add some (at least
> one to start with) standard exceptions that each method is recommended to
> use/throw? This will help the server in KIP-4 provide meaningful error
> codes. KAFKA-3507  is
> tracking it right now.
>
That should be good to have. Will include that. Thanks.

>
> Thanks,
> Grant
>
>
>
> On Tue, Apr 19, 2016 at 9:48 AM, Ashish Singh  wrote:
>
> > I have updated KIP-50
> > <
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-50+-+Move+Authorizer+to+a+separate+package
> > >
> > and PR  as per recent
> > discussions. Please take a look.
> >
> > @Harsha / Don, it would be nice if you guys can review the KIP and PR as
> > well.
> > ​
> >
> > On Mon, Apr 11, 2016 at 7:36 PM, Ashish Singh 
> wrote:
> >
> > > Yes, Jun. I would like to try get option 2 in, if possible in 0.10. I
> am
> > > not asking for delaying 0.10 for it, but some reviews and early
> feedback
> > > would be great. At this point this is what I have in mind.
> > >
> > > 1. Move authorizer and related entities to its own package. Note that I
> > am
> > > proposing to drop scala interface completely. Ranger team is fine with
> it
> > > and I will update Sentry.
> > > 2. The only new public method that will be added to authorizer
> interface
> > > is description().
> > > 3. Update SimpleAclAuthorizer to use the new interface and classes.
> > >
> > > On Mon, Apr 11, 2016 at 6:38 PM, Jun Rao  wrote:
> > >
> > >> Ashish,
> > >>
> > >> So, you want to take a shot at option 2 for 0.10.0? That's fine with
> me
> > >> too. I am just not sure if we have enough time to think through the
> > >> changes.
> > >>
> > >> Thanks,
> > >>
> > >> Jun
> > >>
> > >> On Mon, Apr 11, 2016 at 6:05 PM, Ashish Singh 
> > >> wrote:
> > >>
> > >> > Hello Jun,
> > >> >
> > >> > The 3rd option will require Apache Sentry to go GA with current
> > >> authorizer
> > >> > interface, and at this point it seems that the interface won't last
> > >> long.
> > >> > Within a few months, Sentry will have to make a breaking change. I
> do
> > >> > understand that Kafka should not have to delay its release due to
> one
> > of
> > >> > the authorizer implementations. However, can we assist Sentry users
> to
> > >> > avoid that breaking upgrade? I think it is worth a shot. If the
> > changes
> > >> are
> > >> > not done by 0.10 code freeze, then sure lets punt it to next
> release.
> > >> Does
> > >> > this seem reasonable to you?
> > >> >
> > >> > On Sun, Apr 10, 2016 at 11:42 AM, Jun Rao  wrote:
> > >> >
> > >> > > Ashish,
> > >> > >
> > >> > > A 3rd option is to in 0.10.0, just sanity check the principal type
> > in
> > >> the
> > >> > > implementation of addAcls/removeAcls of Authorizer, but don't
> change
> > >> the
> > >> > > Authorizer api to add the getDescription() method. This fixes the
> > >> > immediate
> > >> > > issue that an acl rule with the wrong principal type is silently
> > >> ignored.
> > >> > > Knowing valid user types is nice, but not critical (we can include
> > the
> > >> > > supported user type in the UnsupportedPrincipalTypeException
> thrown
> > >> from
> > >> > > addAcls/removeAcls). This will give us more time to clean up the
> > >> > Authorizer
> > >> > > api post 0.10.0.
> > >> > >
> > >> > > Thanks
> > >> > >
> > >> > > Jun
> > >> > >
> > >> > > On Fri, Apr 8, 2016 at 9:04 AM, Ashish Singh  >
> > >> > wrote:
> > >> > >
> > >> > > > Thanks for the input Don. One of the possible paths for Option 2
> > is
> > >> to
> > >> > > > completely drop Scala interface, would that be Ok with you
> folks?
> > >> 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Grant Henke]

Hi Ashish,

Thanks for the updates. I have a few questions below:

> Move following interfaces to new package, org.apche.kafka.authorizer.
>
>1. Authorizer
>2. Acl
>3. Operation
>4. PermissionType
>5. Resource
>6. ResourceType
>7. KafkaPrincipal
>8. Session
>
>
This means the client would be required to depend on the authorizer package
as a part of KIP-4. Another option is to have the client objects in common.
Have we ruled out leaving the interface in the core module?

Authorizer interface will be updated to remove getter naming convention.


Now that this is Java do we still want to change to the Scala naming
convention?


Since we are completely rewriting the interface, can we add some (at least
one to start with) standard exceptions that each method is recommended to
use/throw? This will help the server in KIP-4 provide meaningful error
codes. KAFKA-3507  is
tracking it right now.

Thanks,
Grant



On Tue, Apr 19, 2016 at 9:48 AM, Ashish Singh  wrote:

> I have updated KIP-50
> <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-50+-+Move+Authorizer+to+a+separate+package
> >
> and PR  as per recent
> discussions. Please take a look.
>
> @Harsha / Don, it would be nice if you guys can review the KIP and PR as
> well.
> ​
>
> On Mon, Apr 11, 2016 at 7:36 PM, Ashish Singh  wrote:
>
> > Yes, Jun. I would like to try get option 2 in, if possible in 0.10. I am
> > not asking for delaying 0.10 for it, but some reviews and early feedback
> > would be great. At this point this is what I have in mind.
> >
> > 1. Move authorizer and related entities to its own package. Note that I
> am
> > proposing to drop scala interface completely. Ranger team is fine with it
> > and I will update Sentry.
> > 2. The only new public method that will be added to authorizer interface
> > is description().
> > 3. Update SimpleAclAuthorizer to use the new interface and classes.
> >
> > On Mon, Apr 11, 2016 at 6:38 PM, Jun Rao  wrote:
> >
> >> Ashish,
> >>
> >> So, you want to take a shot at option 2 for 0.10.0? That's fine with me
> >> too. I am just not sure if we have enough time to think through the
> >> changes.
> >>
> >> Thanks,
> >>
> >> Jun
> >>
> >> On Mon, Apr 11, 2016 at 6:05 PM, Ashish Singh 
> >> wrote:
> >>
> >> > Hello Jun,
> >> >
> >> > The 3rd option will require Apache Sentry to go GA with current
> >> authorizer
> >> > interface, and at this point it seems that the interface won't last
> >> long.
> >> > Within a few months, Sentry will have to make a breaking change. I do
> >> > understand that Kafka should not have to delay its release due to one
> of
> >> > the authorizer implementations. However, can we assist Sentry users to
> >> > avoid that breaking upgrade? I think it is worth a shot. If the
> changes
> >> are
> >> > not done by 0.10 code freeze, then sure lets punt it to next release.
> >> Does
> >> > this seem reasonable to you?
> >> >
> >> > On Sun, Apr 10, 2016 at 11:42 AM, Jun Rao  wrote:
> >> >
> >> > > Ashish,
> >> > >
> >> > > A 3rd option is to in 0.10.0, just sanity check the principal type
> in
> >> the
> >> > > implementation of addAcls/removeAcls of Authorizer, but don't change
> >> the
> >> > > Authorizer api to add the getDescription() method. This fixes the
> >> > immediate
> >> > > issue that an acl rule with the wrong principal type is silently
> >> ignored.
> >> > > Knowing valid user types is nice, but not critical (we can include
> the
> >> > > supported user type in the UnsupportedPrincipalTypeException thrown
> >> from
> >> > > addAcls/removeAcls). This will give us more time to clean up the
> >> > Authorizer
> >> > > api post 0.10.0.
> >> > >
> >> > > Thanks
> >> > >
> >> > > Jun
> >> > >
> >> > > On Fri, Apr 8, 2016 at 9:04 AM, Ashish Singh 
> >> > wrote:
> >> > >
> >> > > > Thanks for the input Don. One of the possible paths for Option 2
> is
> >> to
> >> > > > completely drop Scala interface, would that be Ok with you folks?
> >> > > >
> >> > > > On Thursday, April 7, 2016, Don Bosco Durai 
> >> wrote:
> >> > > >
> >> > > > > Ranger team would prefer option #2. Right now, we have to access
> >> some
> >> > > of
> >> > > > > the nested constants using constructs like Group$.MODULE$, which
> >> is
> >> > not
> >> > > > > intuitive in Java.
> >> > > > >
> >> > > > > Thanks
> >> > > > >
> >> > > > > Bosco
> >> > > > >
> >> > > > >
> >> > > > >
> >> > > > >
> >> > > > > On 4/7/16, 4:30 PM, "Ashish Singh"  >> > > >
> >> > > > > wrote:
> >> > > > >
> >> > > > > >Harsha/ Don,
> >> > > > > >
> >> > > > > >Are you guys OK with option 2? I am not aware of all the
> existing
> >> > > > > >authorizer implementations, however ranger has one for sure.
> >> Getting
> >> > > > > direct
> >> > > > 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

I have updated KIP-50

and PR  as per recent
discussions. Please take a look.

@Harsha / Don, it would be nice if you guys can review the KIP and PR as
well.
​

On Mon, Apr 11, 2016 at 7:36 PM, Ashish Singh  wrote:

> Yes, Jun. I would like to try get option 2 in, if possible in 0.10. I am
> not asking for delaying 0.10 for it, but some reviews and early feedback
> would be great. At this point this is what I have in mind.
>
> 1. Move authorizer and related entities to its own package. Note that I am
> proposing to drop scala interface completely. Ranger team is fine with it
> and I will update Sentry.
> 2. The only new public method that will be added to authorizer interface
> is description().
> 3. Update SimpleAclAuthorizer to use the new interface and classes.
>
> On Mon, Apr 11, 2016 at 6:38 PM, Jun Rao  wrote:
>
>> Ashish,
>>
>> So, you want to take a shot at option 2 for 0.10.0? That's fine with me
>> too. I am just not sure if we have enough time to think through the
>> changes.
>>
>> Thanks,
>>
>> Jun
>>
>> On Mon, Apr 11, 2016 at 6:05 PM, Ashish Singh 
>> wrote:
>>
>> > Hello Jun,
>> >
>> > The 3rd option will require Apache Sentry to go GA with current
>> authorizer
>> > interface, and at this point it seems that the interface won't last
>> long.
>> > Within a few months, Sentry will have to make a breaking change. I do
>> > understand that Kafka should not have to delay its release due to one of
>> > the authorizer implementations. However, can we assist Sentry users to
>> > avoid that breaking upgrade? I think it is worth a shot. If the changes
>> are
>> > not done by 0.10 code freeze, then sure lets punt it to next release.
>> Does
>> > this seem reasonable to you?
>> >
>> > On Sun, Apr 10, 2016 at 11:42 AM, Jun Rao  wrote:
>> >
>> > > Ashish,
>> > >
>> > > A 3rd option is to in 0.10.0, just sanity check the principal type in
>> the
>> > > implementation of addAcls/removeAcls of Authorizer, but don't change
>> the
>> > > Authorizer api to add the getDescription() method. This fixes the
>> > immediate
>> > > issue that an acl rule with the wrong principal type is silently
>> ignored.
>> > > Knowing valid user types is nice, but not critical (we can include the
>> > > supported user type in the UnsupportedPrincipalTypeException thrown
>> from
>> > > addAcls/removeAcls). This will give us more time to clean up the
>> > Authorizer
>> > > api post 0.10.0.
>> > >
>> > > Thanks
>> > >
>> > > Jun
>> > >
>> > > On Fri, Apr 8, 2016 at 9:04 AM, Ashish Singh 
>> > wrote:
>> > >
>> > > > Thanks for the input Don. One of the possible paths for Option 2 is
>> to
>> > > > completely drop Scala interface, would that be Ok with you folks?
>> > > >
>> > > > On Thursday, April 7, 2016, Don Bosco Durai 
>> wrote:
>> > > >
>> > > > > Ranger team would prefer option #2. Right now, we have to access
>> some
>> > > of
>> > > > > the nested constants using constructs like Group$.MODULE$, which
>> is
>> > not
>> > > > > intuitive in Java.
>> > > > >
>> > > > > Thanks
>> > > > >
>> > > > > Bosco
>> > > > >
>> > > > >
>> > > > >
>> > > > >
>> > > > > On 4/7/16, 4:30 PM, "Ashish Singh" > > > >
>> > > > > wrote:
>> > > > >
>> > > > > >Harsha/ Don,
>> > > > > >
>> > > > > >Are you guys OK with option 2? I am not aware of all the existing
>> > > > > >authorizer implementations, however ranger has one for sure.
>> Getting
>> > > > > direct
>> > > > > >feedback from you guys will be really valuable.
>> > > > > >
>> > > > > >On Thu, Apr 7, 2016 at 3:52 PM, Ismael Juma > > > > > > wrote:
>> > > > > >
>> > > > > >> Hi Don,
>> > > > > >>
>> > > > > >> This is true in Java 7, but Java 8 introduces default methods
>> and
>> > > this
>> > > > > >> workaround is no longer required. During the Interceptor KIP
>> > > > > discussion, it
>> > > > > >> was decided that it was fine to stick to interfaces given that
>> we
>> > > are
>> > > > > >> likely to move to Java 8 in the nearish future (probably no
>> later
>> > > than
>> > > > > the
>> > > > > >> Java 9 release).
>> > > > > >>
>> > > > > >> Ismael
>> > > > > >>
>> > > > > >> On Thu, Apr 7, 2016 at 11:36 PM, Don Bosco Durai <
>> > bo...@apache.org
>> > > > > > wrote:
>> > > > > >>
>> > > > > >> > Hi Ashish
>> > > > > >> >
>> > > > > >> > If we are going by option #2, then I can suggest we give an
>> > > abstract
>> > > > > >> > implementation of the Interface and recommend anyone
>> > implementing
>> > > > > their
>> > > > > >> own
>> > > > > >> > plugin to extend from the abstract class, rather than
>> implement
>> > > the
>> > > > > >> > interface?
>> > > > > >> >
>> > > > > >> > The advantage is, in the future if we add add any new
>> methods 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

Yes, Jun. I would like to try get option 2 in, if possible in 0.10. I am
not asking for delaying 0.10 for it, but some reviews and early feedback
would be great. At this point this is what I have in mind.

1. Move authorizer and related entities to its own package. Note that I am
proposing to drop scala interface completely. Ranger team is fine with it
and I will update Sentry.
2. The only new public method that will be added to authorizer interface is
description().
3. Update SimpleAclAuthorizer to use the new interface and classes.

On Mon, Apr 11, 2016 at 6:38 PM, Jun Rao  wrote:

> Ashish,
>
> So, you want to take a shot at option 2 for 0.10.0? That's fine with me
> too. I am just not sure if we have enough time to think through the
> changes.
>
> Thanks,
>
> Jun
>
> On Mon, Apr 11, 2016 at 6:05 PM, Ashish Singh  wrote:
>
> > Hello Jun,
> >
> > The 3rd option will require Apache Sentry to go GA with current
> authorizer
> > interface, and at this point it seems that the interface won't last long.
> > Within a few months, Sentry will have to make a breaking change. I do
> > understand that Kafka should not have to delay its release due to one of
> > the authorizer implementations. However, can we assist Sentry users to
> > avoid that breaking upgrade? I think it is worth a shot. If the changes
> are
> > not done by 0.10 code freeze, then sure lets punt it to next release.
> Does
> > this seem reasonable to you?
> >
> > On Sun, Apr 10, 2016 at 11:42 AM, Jun Rao  wrote:
> >
> > > Ashish,
> > >
> > > A 3rd option is to in 0.10.0, just sanity check the principal type in
> the
> > > implementation of addAcls/removeAcls of Authorizer, but don't change
> the
> > > Authorizer api to add the getDescription() method. This fixes the
> > immediate
> > > issue that an acl rule with the wrong principal type is silently
> ignored.
> > > Knowing valid user types is nice, but not critical (we can include the
> > > supported user type in the UnsupportedPrincipalTypeException thrown
> from
> > > addAcls/removeAcls). This will give us more time to clean up the
> > Authorizer
> > > api post 0.10.0.
> > >
> > > Thanks
> > >
> > > Jun
> > >
> > > On Fri, Apr 8, 2016 at 9:04 AM, Ashish Singh 
> > wrote:
> > >
> > > > Thanks for the input Don. One of the possible paths for Option 2 is
> to
> > > > completely drop Scala interface, would that be Ok with you folks?
> > > >
> > > > On Thursday, April 7, 2016, Don Bosco Durai 
> wrote:
> > > >
> > > > > Ranger team would prefer option #2. Right now, we have to access
> some
> > > of
> > > > > the nested constants using constructs like Group$.MODULE$, which is
> > not
> > > > > intuitive in Java.
> > > > >
> > > > > Thanks
> > > > >
> > > > > Bosco
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > On 4/7/16, 4:30 PM, "Ashish Singh"  > > >
> > > > > wrote:
> > > > >
> > > > > >Harsha/ Don,
> > > > > >
> > > > > >Are you guys OK with option 2? I am not aware of all the existing
> > > > > >authorizer implementations, however ranger has one for sure.
> Getting
> > > > > direct
> > > > > >feedback from you guys will be really valuable.
> > > > > >
> > > > > >On Thu, Apr 7, 2016 at 3:52 PM, Ismael Juma  > > > > > wrote:
> > > > > >
> > > > > >> Hi Don,
> > > > > >>
> > > > > >> This is true in Java 7, but Java 8 introduces default methods
> and
> > > this
> > > > > >> workaround is no longer required. During the Interceptor KIP
> > > > > discussion, it
> > > > > >> was decided that it was fine to stick to interfaces given that
> we
> > > are
> > > > > >> likely to move to Java 8 in the nearish future (probably no
> later
> > > than
> > > > > the
> > > > > >> Java 9 release).
> > > > > >>
> > > > > >> Ismael
> > > > > >>
> > > > > >> On Thu, Apr 7, 2016 at 11:36 PM, Don Bosco Durai <
> > bo...@apache.org
> > > > > > wrote:
> > > > > >>
> > > > > >> > Hi Ashish
> > > > > >> >
> > > > > >> > If we are going by option #2, then I can suggest we give an
> > > abstract
> > > > > >> > implementation of the Interface and recommend anyone
> > implementing
> > > > > their
> > > > > >> own
> > > > > >> > plugin to extend from the abstract class, rather than
> implement
> > > the
> > > > > >> > interface?
> > > > > >> >
> > > > > >> > The advantage is, in the future if we add add any new methods
> in
> > > the
> > > > > >> > Interface (e.g. Similar to getDescription()), then we can
> give a
> > > > dummy
> > > > > >> > implementation of the new method and this won’t break the
> > > > compilation
> > > > > of
> > > > > >> > any external implementation. Else over the time it will be
> > > > challenging
> > > > > >> for
> > > > > >> > anyone customizing the implementation to keep track of changes
> > to
> > > > the
> > > > > >> > Interface.
> > > > > >> >
> > > > > >> > Thanks
> > > > > >> >
> > > > > >> > Bosco
> > > > > >> 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Jun Rao]

Ashish,

So, you want to take a shot at option 2 for 0.10.0? That's fine with me
too. I am just not sure if we have enough time to think through the changes.

Thanks,

Jun

On Mon, Apr 11, 2016 at 6:05 PM, Ashish Singh  wrote:

> Hello Jun,
>
> The 3rd option will require Apache Sentry to go GA with current authorizer
> interface, and at this point it seems that the interface won't last long.
> Within a few months, Sentry will have to make a breaking change. I do
> understand that Kafka should not have to delay its release due to one of
> the authorizer implementations. However, can we assist Sentry users to
> avoid that breaking upgrade? I think it is worth a shot. If the changes are
> not done by 0.10 code freeze, then sure lets punt it to next release. Does
> this seem reasonable to you?
>
> On Sun, Apr 10, 2016 at 11:42 AM, Jun Rao  wrote:
>
> > Ashish,
> >
> > A 3rd option is to in 0.10.0, just sanity check the principal type in the
> > implementation of addAcls/removeAcls of Authorizer, but don't change the
> > Authorizer api to add the getDescription() method. This fixes the
> immediate
> > issue that an acl rule with the wrong principal type is silently ignored.
> > Knowing valid user types is nice, but not critical (we can include the
> > supported user type in the UnsupportedPrincipalTypeException thrown from
> > addAcls/removeAcls). This will give us more time to clean up the
> Authorizer
> > api post 0.10.0.
> >
> > Thanks
> >
> > Jun
> >
> > On Fri, Apr 8, 2016 at 9:04 AM, Ashish Singh 
> wrote:
> >
> > > Thanks for the input Don. One of the possible paths for Option 2 is to
> > > completely drop Scala interface, would that be Ok with you folks?
> > >
> > > On Thursday, April 7, 2016, Don Bosco Durai  wrote:
> > >
> > > > Ranger team would prefer option #2. Right now, we have to access some
> > of
> > > > the nested constants using constructs like Group$.MODULE$, which is
> not
> > > > intuitive in Java.
> > > >
> > > > Thanks
> > > >
> > > > Bosco
> > > >
> > > >
> > > >
> > > >
> > > > On 4/7/16, 4:30 PM, "Ashish Singh"  > >
> > > > wrote:
> > > >
> > > > >Harsha/ Don,
> > > > >
> > > > >Are you guys OK with option 2? I am not aware of all the existing
> > > > >authorizer implementations, however ranger has one for sure. Getting
> > > > direct
> > > > >feedback from you guys will be really valuable.
> > > > >
> > > > >On Thu, Apr 7, 2016 at 3:52 PM, Ismael Juma  > > > > wrote:
> > > > >
> > > > >> Hi Don,
> > > > >>
> > > > >> This is true in Java 7, but Java 8 introduces default methods and
> > this
> > > > >> workaround is no longer required. During the Interceptor KIP
> > > > discussion, it
> > > > >> was decided that it was fine to stick to interfaces given that we
> > are
> > > > >> likely to move to Java 8 in the nearish future (probably no later
> > than
> > > > the
> > > > >> Java 9 release).
> > > > >>
> > > > >> Ismael
> > > > >>
> > > > >> On Thu, Apr 7, 2016 at 11:36 PM, Don Bosco Durai <
> bo...@apache.org
> > > > > wrote:
> > > > >>
> > > > >> > Hi Ashish
> > > > >> >
> > > > >> > If we are going by option #2, then I can suggest we give an
> > abstract
> > > > >> > implementation of the Interface and recommend anyone
> implementing
> > > > their
> > > > >> own
> > > > >> > plugin to extend from the abstract class, rather than implement
> > the
> > > > >> > interface?
> > > > >> >
> > > > >> > The advantage is, in the future if we add add any new methods in
> > the
> > > > >> > Interface (e.g. Similar to getDescription()), then we can give a
> > > dummy
> > > > >> > implementation of the new method and this won’t break the
> > > compilation
> > > > of
> > > > >> > any external implementation. Else over the time it will be
> > > challenging
> > > > >> for
> > > > >> > anyone customizing the implementation to keep track of changes
> to
> > > the
> > > > >> > Interface.
> > > > >> >
> > > > >> > Thanks
> > > > >> >
> > > > >> > Bosco
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> >
> > > > >> > On 4/7/16, 11:21 AM, "Ashish Singh"  > > > > wrote:
> > > > >> >
> > > > >> > >Hello Harsha,
> > > > >> > >
> > > > >> > >On Thu, Apr 7, 2016 at 11:03 AM, Harsha  > > > > wrote:
> > > > >> > >
> > > > >> > >"My only ask is to have this in 0.10. As Jay pointed out, right
> > now
> > > > >> > >> there
> > > > >> > >> are not many implementations out there, we might want to fix
> it
> > > > ASAP."
> > > > >> > >>
> > > > >> > >> Probably there aren't many implementations but there are lot
> of
> > > > users
> > > > >> > >> using these implementations in production clusters.
> > > > >> > >> Isn't this going to break the rolling upgrade?
> > > > >> > >
> > > > >> > >It will and it is a concern, in my previous mail I have
> mentioned
> > > > this
> > > > >> as
> > > > 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

Hello Jun,

The 3rd option will require Apache Sentry to go GA with current authorizer
interface, and at this point it seems that the interface won't last long.
Within a few months, Sentry will have to make a breaking change. I do
understand that Kafka should not have to delay its release due to one of
the authorizer implementations. However, can we assist Sentry users to
avoid that breaking upgrade? I think it is worth a shot. If the changes are
not done by 0.10 code freeze, then sure lets punt it to next release. Does
this seem reasonable to you?

On Sun, Apr 10, 2016 at 11:42 AM, Jun Rao  wrote:

> Ashish,
>
> A 3rd option is to in 0.10.0, just sanity check the principal type in the
> implementation of addAcls/removeAcls of Authorizer, but don't change the
> Authorizer api to add the getDescription() method. This fixes the immediate
> issue that an acl rule with the wrong principal type is silently ignored.
> Knowing valid user types is nice, but not critical (we can include the
> supported user type in the UnsupportedPrincipalTypeException thrown from
> addAcls/removeAcls). This will give us more time to clean up the Authorizer
> api post 0.10.0.
>
> Thanks
>
> Jun
>
> On Fri, Apr 8, 2016 at 9:04 AM, Ashish Singh  wrote:
>
> > Thanks for the input Don. One of the possible paths for Option 2 is to
> > completely drop Scala interface, would that be Ok with you folks?
> >
> > On Thursday, April 7, 2016, Don Bosco Durai  wrote:
> >
> > > Ranger team would prefer option #2. Right now, we have to access some
> of
> > > the nested constants using constructs like Group$.MODULE$, which is not
> > > intuitive in Java.
> > >
> > > Thanks
> > >
> > > Bosco
> > >
> > >
> > >
> > >
> > > On 4/7/16, 4:30 PM, "Ashish Singh"  >
> > > wrote:
> > >
> > > >Harsha/ Don,
> > > >
> > > >Are you guys OK with option 2? I am not aware of all the existing
> > > >authorizer implementations, however ranger has one for sure. Getting
> > > direct
> > > >feedback from you guys will be really valuable.
> > > >
> > > >On Thu, Apr 7, 2016 at 3:52 PM, Ismael Juma  > > > wrote:
> > > >
> > > >> Hi Don,
> > > >>
> > > >> This is true in Java 7, but Java 8 introduces default methods and
> this
> > > >> workaround is no longer required. During the Interceptor KIP
> > > discussion, it
> > > >> was decided that it was fine to stick to interfaces given that we
> are
> > > >> likely to move to Java 8 in the nearish future (probably no later
> than
> > > the
> > > >> Java 9 release).
> > > >>
> > > >> Ismael
> > > >>
> > > >> On Thu, Apr 7, 2016 at 11:36 PM, Don Bosco Durai  > > > wrote:
> > > >>
> > > >> > Hi Ashish
> > > >> >
> > > >> > If we are going by option #2, then I can suggest we give an
> abstract
> > > >> > implementation of the Interface and recommend anyone implementing
> > > their
> > > >> own
> > > >> > plugin to extend from the abstract class, rather than implement
> the
> > > >> > interface?
> > > >> >
> > > >> > The advantage is, in the future if we add add any new methods in
> the
> > > >> > Interface (e.g. Similar to getDescription()), then we can give a
> > dummy
> > > >> > implementation of the new method and this won’t break the
> > compilation
> > > of
> > > >> > any external implementation. Else over the time it will be
> > challenging
> > > >> for
> > > >> > anyone customizing the implementation to keep track of changes to
> > the
> > > >> > Interface.
> > > >> >
> > > >> > Thanks
> > > >> >
> > > >> > Bosco
> > > >> >
> > > >> >
> > > >> >
> > > >> >
> > > >> > On 4/7/16, 11:21 AM, "Ashish Singh"  > > > wrote:
> > > >> >
> > > >> > >Hello Harsha,
> > > >> > >
> > > >> > >On Thu, Apr 7, 2016 at 11:03 AM, Harsha  > > > wrote:
> > > >> > >
> > > >> > >"My only ask is to have this in 0.10. As Jay pointed out, right
> now
> > > >> > >> there
> > > >> > >> are not many implementations out there, we might want to fix it
> > > ASAP."
> > > >> > >>
> > > >> > >> Probably there aren't many implementations but there are lot of
> > > users
> > > >> > >> using these implementations in production clusters.
> > > >> > >> Isn't this going to break the rolling upgrade?
> > > >> > >
> > > >> > >It will and it is a concern, in my previous mail I have mentioned
> > > this
> > > >> as
> > > >> > >an issue if we choose to go this route. However, if we actually
> > > decide
> > > >> to
> > > >> > >do this, I would say it is better to do it sooner than later, as
> > > fewer
> > > >> > >implementations will be affected. Below is excerpt from my
> previous
> > > >> mail.
> > > >> > >
> > > >> > >Increase scope of KIP-50 to move authorizer and related classes
> to
> > a
> > > >> > >separate package. The new package will have java interface. This
> > will
> > > >> > allow
> > > >> > >implementations to not depend on kafka 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Jun Rao]

Ashish,

A 3rd option is to in 0.10.0, just sanity check the principal type in the
implementation of addAcls/removeAcls of Authorizer, but don't change the
Authorizer api to add the getDescription() method. This fixes the immediate
issue that an acl rule with the wrong principal type is silently ignored.
Knowing valid user types is nice, but not critical (we can include the
supported user type in the UnsupportedPrincipalTypeException thrown from
addAcls/removeAcls). This will give us more time to clean up the Authorizer
api post 0.10.0.

Thanks

Jun

On Fri, Apr 8, 2016 at 9:04 AM, Ashish Singh  wrote:

> Thanks for the input Don. One of the possible paths for Option 2 is to
> completely drop Scala interface, would that be Ok with you folks?
>
> On Thursday, April 7, 2016, Don Bosco Durai  wrote:
>
> > Ranger team would prefer option #2. Right now, we have to access some of
> > the nested constants using constructs like Group$.MODULE$, which is not
> > intuitive in Java.
> >
> > Thanks
> >
> > Bosco
> >
> >
> >
> >
> > On 4/7/16, 4:30 PM, "Ashish Singh" >
> > wrote:
> >
> > >Harsha/ Don,
> > >
> > >Are you guys OK with option 2? I am not aware of all the existing
> > >authorizer implementations, however ranger has one for sure. Getting
> > direct
> > >feedback from you guys will be really valuable.
> > >
> > >On Thu, Apr 7, 2016 at 3:52 PM, Ismael Juma  > > wrote:
> > >
> > >> Hi Don,
> > >>
> > >> This is true in Java 7, but Java 8 introduces default methods and this
> > >> workaround is no longer required. During the Interceptor KIP
> > discussion, it
> > >> was decided that it was fine to stick to interfaces given that we are
> > >> likely to move to Java 8 in the nearish future (probably no later than
> > the
> > >> Java 9 release).
> > >>
> > >> Ismael
> > >>
> > >> On Thu, Apr 7, 2016 at 11:36 PM, Don Bosco Durai  > > wrote:
> > >>
> > >> > Hi Ashish
> > >> >
> > >> > If we are going by option #2, then I can suggest we give an abstract
> > >> > implementation of the Interface and recommend anyone implementing
> > their
> > >> own
> > >> > plugin to extend from the abstract class, rather than implement the
> > >> > interface?
> > >> >
> > >> > The advantage is, in the future if we add add any new methods in the
> > >> > Interface (e.g. Similar to getDescription()), then we can give a
> dummy
> > >> > implementation of the new method and this won’t break the
> compilation
> > of
> > >> > any external implementation. Else over the time it will be
> challenging
> > >> for
> > >> > anyone customizing the implementation to keep track of changes to
> the
> > >> > Interface.
> > >> >
> > >> > Thanks
> > >> >
> > >> > Bosco
> > >> >
> > >> >
> > >> >
> > >> >
> > >> > On 4/7/16, 11:21 AM, "Ashish Singh"  > > wrote:
> > >> >
> > >> > >Hello Harsha,
> > >> > >
> > >> > >On Thu, Apr 7, 2016 at 11:03 AM, Harsha  > > wrote:
> > >> > >
> > >> > >"My only ask is to have this in 0.10. As Jay pointed out, right now
> > >> > >> there
> > >> > >> are not many implementations out there, we might want to fix it
> > ASAP."
> > >> > >>
> > >> > >> Probably there aren't many implementations but there are lot of
> > users
> > >> > >> using these implementations in production clusters.
> > >> > >> Isn't this going to break the rolling upgrade?
> > >> > >
> > >> > >It will and it is a concern, in my previous mail I have mentioned
> > this
> > >> as
> > >> > >an issue if we choose to go this route. However, if we actually
> > decide
> > >> to
> > >> > >do this, I would say it is better to do it sooner than later, as
> > fewer
> > >> > >implementations will be affected. Below is excerpt from my previous
> > >> mail.
> > >> > >
> > >> > >Increase scope of KIP-50 to move authorizer and related classes to
> a
> > >> > >separate package. The new package will have java interface. This
> will
> > >> > allow
> > >> > >implementations to not depend on kafka core and just on authorizer
> > >> > package,
> > >> > >make authorization interface follow kafka’s coding standards and
> will
> > >> > allow
> > >> > >java implementations to be cleaner. We can either completely drop
> > scala
> > >> > >interface, which might be a pain for existing implementations, or
> we
> > can
> > >> > >have scala interface wrap java interface. Later allows a cleaner
> > >> > >deprecation path for existing scala authorizer interface, however
> it
> > may
> > >> > or
> > >> > >may not be feasible as Kafka server will have to somehow decide
> which
> > >> > >interface it should be looking for while loading authorizer
> > >> > implementation,
> > >> > >this can probably be solved with a config or some reflection. If we
> > >> choose
> > >> > >to go this route, I can dig deeper.
> > >> > >
> > >> > >If we go with option 2 and commit on getting this in ASAP,
> > 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Don Bosco Durai]

Yes, if we have Java interface, then we won’t need Scala interface.

Thanks

Bosco





On 4/8/16, 9:04 AM, "Ashish Singh"  wrote:

>Thanks for the input Don. One of the possible paths for Option 2 is to
>completely drop Scala interface, would that be Ok with you folks?
>
>On Thursday, April 7, 2016, Don Bosco Durai  wrote:
>
>> Ranger team would prefer option #2. Right now, we have to access some of
>> the nested constants using constructs like Group$.MODULE$, which is not
>> intuitive in Java.
>>
>> Thanks
>>
>> Bosco
>>
>>
>>
>>
>> On 4/7/16, 4:30 PM, "Ashish Singh" >
>> wrote:
>>
>> >Harsha/ Don,
>> >
>> >Are you guys OK with option 2? I am not aware of all the existing
>> >authorizer implementations, however ranger has one for sure. Getting
>> direct
>> >feedback from you guys will be really valuable.
>> >
>> >On Thu, Apr 7, 2016 at 3:52 PM, Ismael Juma > > wrote:
>> >
>> >> Hi Don,
>> >>
>> >> This is true in Java 7, but Java 8 introduces default methods and this
>> >> workaround is no longer required. During the Interceptor KIP
>> discussion, it
>> >> was decided that it was fine to stick to interfaces given that we are
>> >> likely to move to Java 8 in the nearish future (probably no later than
>> the
>> >> Java 9 release).
>> >>
>> >> Ismael
>> >>
>> >> On Thu, Apr 7, 2016 at 11:36 PM, Don Bosco Durai > > wrote:
>> >>
>> >> > Hi Ashish
>> >> >
>> >> > If we are going by option #2, then I can suggest we give an abstract
>> >> > implementation of the Interface and recommend anyone implementing
>> their
>> >> own
>> >> > plugin to extend from the abstract class, rather than implement the
>> >> > interface?
>> >> >
>> >> > The advantage is, in the future if we add add any new methods in the
>> >> > Interface (e.g. Similar to getDescription()), then we can give a dummy
>> >> > implementation of the new method and this won’t break the compilation
>> of
>> >> > any external implementation. Else over the time it will be challenging
>> >> for
>> >> > anyone customizing the implementation to keep track of changes to the
>> >> > Interface.
>> >> >
>> >> > Thanks
>> >> >
>> >> > Bosco
>> >> >
>> >> >
>> >> >
>> >> >
>> >> > On 4/7/16, 11:21 AM, "Ashish Singh" > > wrote:
>> >> >
>> >> > >Hello Harsha,
>> >> > >
>> >> > >On Thu, Apr 7, 2016 at 11:03 AM, Harsha > > wrote:
>> >> > >
>> >> > >"My only ask is to have this in 0.10. As Jay pointed out, right now
>> >> > >> there
>> >> > >> are not many implementations out there, we might want to fix it
>> ASAP."
>> >> > >>
>> >> > >> Probably there aren't many implementations but there are lot of
>> users
>> >> > >> using these implementations in production clusters.
>> >> > >> Isn't this going to break the rolling upgrade?
>> >> > >
>> >> > >It will and it is a concern, in my previous mail I have mentioned
>> this
>> >> as
>> >> > >an issue if we choose to go this route. However, if we actually
>> decide
>> >> to
>> >> > >do this, I would say it is better to do it sooner than later, as
>> fewer
>> >> > >implementations will be affected. Below is excerpt from my previous
>> >> mail.
>> >> > >
>> >> > >Increase scope of KIP-50 to move authorizer and related classes to a
>> >> > >separate package. The new package will have java interface. This will
>> >> > allow
>> >> > >implementations to not depend on kafka core and just on authorizer
>> >> > package,
>> >> > >make authorization interface follow kafka’s coding standards and will
>> >> > allow
>> >> > >java implementations to be cleaner. We can either completely drop
>> scala
>> >> > >interface, which might be a pain for existing implementations, or we
>> can
>> >> > >have scala interface wrap java interface. Later allows a cleaner
>> >> > >deprecation path for existing scala authorizer interface, however it
>> may
>> >> > or
>> >> > >may not be feasible as Kafka server will have to somehow decide which
>> >> > >interface it should be looking for while loading authorizer
>> >> > implementation,
>> >> > >this can probably be solved with a config or some reflection. If we
>> >> choose
>> >> > >to go this route, I can dig deeper.
>> >> > >
>> >> > >If we go with option 2 and commit on getting this in ASAP,
>> preferably in
>> >> > >0.10, there will be fewer implementations that will be affected.
>> >> > >
>> >> > >and also moving to Java ,
>> >> > >> a authorizer implementation going to run inside a KafkaBroker and I
>> >> > >> don't see why this is necessary to move to clients package.
>> >> > >> Are we planning on introducing common module to have it
>> independent of
>> >> > >> broker and client code?
>> >> > >>
>> >> > >Yes, I think that would take away the requirement of depending on
>> Kafka
>> >> > >core from authorizer implementations. Do you suggest otherwise?
>> >> > >
>> >> > >
>> >> > >> -Harsha
>> >> > 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

Thanks for the input Don. One of the possible paths for Option 2 is to
completely drop Scala interface, would that be Ok with you folks?

On Thursday, April 7, 2016, Don Bosco Durai  wrote:

> Ranger team would prefer option #2. Right now, we have to access some of
> the nested constants using constructs like Group$.MODULE$, which is not
> intuitive in Java.
>
> Thanks
>
> Bosco
>
>
>
>
> On 4/7/16, 4:30 PM, "Ashish Singh" >
> wrote:
>
> >Harsha/ Don,
> >
> >Are you guys OK with option 2? I am not aware of all the existing
> >authorizer implementations, however ranger has one for sure. Getting
> direct
> >feedback from you guys will be really valuable.
> >
> >On Thu, Apr 7, 2016 at 3:52 PM, Ismael Juma  > wrote:
> >
> >> Hi Don,
> >>
> >> This is true in Java 7, but Java 8 introduces default methods and this
> >> workaround is no longer required. During the Interceptor KIP
> discussion, it
> >> was decided that it was fine to stick to interfaces given that we are
> >> likely to move to Java 8 in the nearish future (probably no later than
> the
> >> Java 9 release).
> >>
> >> Ismael
> >>
> >> On Thu, Apr 7, 2016 at 11:36 PM, Don Bosco Durai  > wrote:
> >>
> >> > Hi Ashish
> >> >
> >> > If we are going by option #2, then I can suggest we give an abstract
> >> > implementation of the Interface and recommend anyone implementing
> their
> >> own
> >> > plugin to extend from the abstract class, rather than implement the
> >> > interface?
> >> >
> >> > The advantage is, in the future if we add add any new methods in the
> >> > Interface (e.g. Similar to getDescription()), then we can give a dummy
> >> > implementation of the new method and this won’t break the compilation
> of
> >> > any external implementation. Else over the time it will be challenging
> >> for
> >> > anyone customizing the implementation to keep track of changes to the
> >> > Interface.
> >> >
> >> > Thanks
> >> >
> >> > Bosco
> >> >
> >> >
> >> >
> >> >
> >> > On 4/7/16, 11:21 AM, "Ashish Singh"  > wrote:
> >> >
> >> > >Hello Harsha,
> >> > >
> >> > >On Thu, Apr 7, 2016 at 11:03 AM, Harsha  > wrote:
> >> > >
> >> > >"My only ask is to have this in 0.10. As Jay pointed out, right now
> >> > >> there
> >> > >> are not many implementations out there, we might want to fix it
> ASAP."
> >> > >>
> >> > >> Probably there aren't many implementations but there are lot of
> users
> >> > >> using these implementations in production clusters.
> >> > >> Isn't this going to break the rolling upgrade?
> >> > >
> >> > >It will and it is a concern, in my previous mail I have mentioned
> this
> >> as
> >> > >an issue if we choose to go this route. However, if we actually
> decide
> >> to
> >> > >do this, I would say it is better to do it sooner than later, as
> fewer
> >> > >implementations will be affected. Below is excerpt from my previous
> >> mail.
> >> > >
> >> > >Increase scope of KIP-50 to move authorizer and related classes to a
> >> > >separate package. The new package will have java interface. This will
> >> > allow
> >> > >implementations to not depend on kafka core and just on authorizer
> >> > package,
> >> > >make authorization interface follow kafka’s coding standards and will
> >> > allow
> >> > >java implementations to be cleaner. We can either completely drop
> scala
> >> > >interface, which might be a pain for existing implementations, or we
> can
> >> > >have scala interface wrap java interface. Later allows a cleaner
> >> > >deprecation path for existing scala authorizer interface, however it
> may
> >> > or
> >> > >may not be feasible as Kafka server will have to somehow decide which
> >> > >interface it should be looking for while loading authorizer
> >> > implementation,
> >> > >this can probably be solved with a config or some reflection. If we
> >> choose
> >> > >to go this route, I can dig deeper.
> >> > >
> >> > >If we go with option 2 and commit on getting this in ASAP,
> preferably in
> >> > >0.10, there will be fewer implementations that will be affected.
> >> > >
> >> > >and also moving to Java ,
> >> > >> a authorizer implementation going to run inside a KafkaBroker and I
> >> > >> don't see why this is necessary to move to clients package.
> >> > >> Are we planning on introducing common module to have it
> independent of
> >> > >> broker and client code?
> >> > >>
> >> > >Yes, I think that would take away the requirement of depending on
> Kafka
> >> > >core from authorizer implementations. Do you suggest otherwise?
> >> > >
> >> > >
> >> > >> -Harsha
> >> > >>
> >> > >> On Thu, Apr 7, 2016, at 10:52 AM, Ashish Singh wrote:
> >> > >> > We might want to take a call here. Following are the options.
> >> > >> >
> >> > >> >1. Let KIP-50 be the way it is, i.e., just add
> getDescription()
> >> to
> >> > >> >existing scala authorizer 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Don Bosco Durai]

Ranger team would prefer option #2. Right now, we have to access some of the 
nested constants using constructs like Group$.MODULE$, which is not intuitive 
in Java.

Thanks

Bosco




On 4/7/16, 4:30 PM, "Ashish Singh"  wrote:

>Harsha/ Don,
>
>Are you guys OK with option 2? I am not aware of all the existing
>authorizer implementations, however ranger has one for sure. Getting direct
>feedback from you guys will be really valuable.
>
>On Thu, Apr 7, 2016 at 3:52 PM, Ismael Juma  wrote:
>
>> Hi Don,
>>
>> This is true in Java 7, but Java 8 introduces default methods and this
>> workaround is no longer required. During the Interceptor KIP discussion, it
>> was decided that it was fine to stick to interfaces given that we are
>> likely to move to Java 8 in the nearish future (probably no later than the
>> Java 9 release).
>>
>> Ismael
>>
>> On Thu, Apr 7, 2016 at 11:36 PM, Don Bosco Durai  wrote:
>>
>> > Hi Ashish
>> >
>> > If we are going by option #2, then I can suggest we give an abstract
>> > implementation of the Interface and recommend anyone implementing their
>> own
>> > plugin to extend from the abstract class, rather than implement the
>> > interface?
>> >
>> > The advantage is, in the future if we add add any new methods in the
>> > Interface (e.g. Similar to getDescription()), then we can give a dummy
>> > implementation of the new method and this won’t break the compilation of
>> > any external implementation. Else over the time it will be challenging
>> for
>> > anyone customizing the implementation to keep track of changes to the
>> > Interface.
>> >
>> > Thanks
>> >
>> > Bosco
>> >
>> >
>> >
>> >
>> > On 4/7/16, 11:21 AM, "Ashish Singh"  wrote:
>> >
>> > >Hello Harsha,
>> > >
>> > >On Thu, Apr 7, 2016 at 11:03 AM, Harsha  wrote:
>> > >
>> > >"My only ask is to have this in 0.10. As Jay pointed out, right now
>> > >> there
>> > >> are not many implementations out there, we might want to fix it ASAP."
>> > >>
>> > >> Probably there aren't many implementations but there are lot of users
>> > >> using these implementations in production clusters.
>> > >> Isn't this going to break the rolling upgrade?
>> > >
>> > >It will and it is a concern, in my previous mail I have mentioned this
>> as
>> > >an issue if we choose to go this route. However, if we actually decide
>> to
>> > >do this, I would say it is better to do it sooner than later, as fewer
>> > >implementations will be affected. Below is excerpt from my previous
>> mail.
>> > >
>> > >Increase scope of KIP-50 to move authorizer and related classes to a
>> > >separate package. The new package will have java interface. This will
>> > allow
>> > >implementations to not depend on kafka core and just on authorizer
>> > package,
>> > >make authorization interface follow kafka’s coding standards and will
>> > allow
>> > >java implementations to be cleaner. We can either completely drop scala
>> > >interface, which might be a pain for existing implementations, or we can
>> > >have scala interface wrap java interface. Later allows a cleaner
>> > >deprecation path for existing scala authorizer interface, however it may
>> > or
>> > >may not be feasible as Kafka server will have to somehow decide which
>> > >interface it should be looking for while loading authorizer
>> > implementation,
>> > >this can probably be solved with a config or some reflection. If we
>> choose
>> > >to go this route, I can dig deeper.
>> > >
>> > >If we go with option 2 and commit on getting this in ASAP, preferably in
>> > >0.10, there will be fewer implementations that will be affected.
>> > >
>> > >and also moving to Java ,
>> > >> a authorizer implementation going to run inside a KafkaBroker and I
>> > >> don't see why this is necessary to move to clients package.
>> > >> Are we planning on introducing common module to have it independent of
>> > >> broker and client code?
>> > >>
>> > >Yes, I think that would take away the requirement of depending on Kafka
>> > >core from authorizer implementations. Do you suggest otherwise?
>> > >
>> > >
>> > >> -Harsha
>> > >>
>> > >> On Thu, Apr 7, 2016, at 10:52 AM, Ashish Singh wrote:
>> > >> > We might want to take a call here. Following are the options.
>> > >> >
>> > >> >1. Let KIP-50 be the way it is, i.e., just add getDescription()
>> to
>> > >> >existing scala authorizer interface. It will break binary
>> > >> >compatibility
>> > >> >(only when using CLI and/or AdminCommand from >= 0.10 against
>> > >> >authorizer
>> > >> >implementations based on 0.9.). If we go this route, it is a
>> > simpler
>> > >> >change
>> > >> >and existing implementations won’t have to change anything on
>> their
>> > >> >end.
>> > >> >2. Increase scope of KIP-50 to move authorizer and related
>> classes
>> > to
>> > >> >a
>> > >> >separate package. The new package will have java interface. This
>> > will
>> 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Don Bosco Durai]

Hi Ashish

If we are going by option #2, then I can suggest we give an abstract 
implementation of the Interface and recommend anyone implementing their own 
plugin to extend from the abstract class, rather than implement the interface?

The advantage is, in the future if we add add any new methods in the Interface 
(e.g. Similar to getDescription()), then we can give a dummy implementation of 
the new method and this won’t break the compilation of any external 
implementation. Else over the time it will be challenging for anyone 
customizing the implementation to keep track of changes to the Interface.

Thanks

Bosco




On 4/7/16, 11:21 AM, "Ashish Singh"  wrote:

>Hello Harsha,
>
>On Thu, Apr 7, 2016 at 11:03 AM, Harsha  wrote:
>
>"My only ask is to have this in 0.10. As Jay pointed out, right now
>> there
>> are not many implementations out there, we might want to fix it ASAP."
>>
>> Probably there aren't many implementations but there are lot of users
>> using these implementations in production clusters.
>> Isn't this going to break the rolling upgrade?
>
>It will and it is a concern, in my previous mail I have mentioned this as
>an issue if we choose to go this route. However, if we actually decide to
>do this, I would say it is better to do it sooner than later, as fewer
>implementations will be affected. Below is excerpt from my previous mail.
>
>Increase scope of KIP-50 to move authorizer and related classes to a
>separate package. The new package will have java interface. This will allow
>implementations to not depend on kafka core and just on authorizer package,
>make authorization interface follow kafka’s coding standards and will allow
>java implementations to be cleaner. We can either completely drop scala
>interface, which might be a pain for existing implementations, or we can
>have scala interface wrap java interface. Later allows a cleaner
>deprecation path for existing scala authorizer interface, however it may or
>may not be feasible as Kafka server will have to somehow decide which
>interface it should be looking for while loading authorizer implementation,
>this can probably be solved with a config or some reflection. If we choose
>to go this route, I can dig deeper.
>
>If we go with option 2 and commit on getting this in ASAP, preferably in
>0.10, there will be fewer implementations that will be affected.
>
>and also moving to Java ,
>> a authorizer implementation going to run inside a KafkaBroker and I
>> don't see why this is necessary to move to clients package.
>> Are we planning on introducing common module to have it independent of
>> broker and client code?
>>
>Yes, I think that would take away the requirement of depending on Kafka
>core from authorizer implementations. Do you suggest otherwise?
>
>
>> -Harsha
>>
>> On Thu, Apr 7, 2016, at 10:52 AM, Ashish Singh wrote:
>> > We might want to take a call here. Following are the options.
>> >
>> >1. Let KIP-50 be the way it is, i.e., just add getDescription() to
>> >existing scala authorizer interface. It will break binary
>> >compatibility
>> >(only when using CLI and/or AdminCommand from >= 0.10 against
>> >authorizer
>> >implementations based on 0.9.). If we go this route, it is a simpler
>> >change
>> >and existing implementations won’t have to change anything on their
>> >end.
>> >2. Increase scope of KIP-50 to move authorizer and related classes to
>> >a
>> >separate package. The new package will have java interface. This will
>> >allow
>> >implementations to not depend on kafka core and just on authorizer
>> >package,
>> >make authorization interface follow kafka’s coding standards and will
>> >allow
>> >java implementations to be cleaner. We can either completely drop
>> >scala
>> >interface, which might be a pain for existing implementations, or we
>> >can
>> >have scala interface wrap java interface. Later allows a cleaner
>> >deprecation path for existing scala authorizer interface, however it
>> >may or
>> >may not be feasible as Kafka server will have to somehow decide which
>> >interface it should be looking for while loading authorizer
>> >implementation,
>> >this can probably be solved with a config or some reflection. If we
>> >choose
>> >to go this route, I can dig deeper.
>> >
>> > If we decide to go with option 1, I think it would be fair to say that
>> > scala authorizer interface will be around for some time, as there will be
>> > more implementations relying on it. If we go with option 2 and commit on
>> > getting this in ASAP, preferably in 0.10, there will be fewer
>> > implementations that will be affected.
>> >
>> > *Another thing to notice is that scala authorizer interface is not
>> > annotated as unstable.*
>> > ​
>> >
>> > On Wed, Apr 6, 2016 at 9:41 AM, Ashish Singh 
>> wrote:
>> >
>> > > I see value in minimizing 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Harsha]

Gwen, Ashish,
  Rolling upgrade should be fine in this case. Users can
  bring down single broker at a time and upgrade their
  kafka binaries along with new authorizer
  implementation . This looks fine to me. 

Thanks,
Harsha

On Thu, Apr 7, 2016, at 03:15 PM, Ashish Singh wrote:
> Harsha,
> 
> No, ZK data format for acls or SimplaAclAthorizer implementation is not
> proposed to be changed.
> 
> Gwen,
> 
> The concern I was pointing at is that if we choose to go with Option 2,
> i.e., move interface to a separate package, authorizer implementation
> will
> have to be updated along with broker upgrade and authorizer
> implementations
> need to make changes. I am not sure how this affects rolling upgrade.
> Harsha, do you mind sharing your concerns on rolling upgrade?
> 
> On Thu, Apr 7, 2016 at 12:52 PM, Harsha  wrote:
> 
> > Ashish,
> >   Thanks for the details. We are not changing any of the zookeeper
> >   data format for acls right?
> >
> > Thanks,
> > Harsha
> >
> > On Thu, Apr 7, 2016, at 11:25 AM, Gwen Shapira wrote:
> > > Can you guys go into details on what will happen during a rolling upgrade
> > > exactly?
> > >
> > > Gwen
> > >
> > > On Thu, Apr 7, 2016 at 11:21 AM, Ashish Singh 
> > > wrote:
> > >
> > > > Hello Harsha,
> > > >
> > > > On Thu, Apr 7, 2016 at 11:03 AM, Harsha  wrote:
> > > >
> > > > "My only ask is to have this in 0.10. As Jay pointed out, right now
> > > > > there
> > > > > are not many implementations out there, we might want to fix it
> > ASAP."
> > > > >
> > > > > Probably there aren't many implementations but there are lot of users
> > > > > using these implementations in production clusters.
> > > > > Isn't this going to break the rolling upgrade?
> > > >
> > > > It will and it is a concern, in my previous mail I have mentioned this
> > as
> > > > an issue if we choose to go this route. However, if we actually decide
> > to
> > > > do this, I would say it is better to do it sooner than later, as fewer
> > > > implementations will be affected. Below is excerpt from my previous
> > mail.
> > > >
> > > > Increase scope of KIP-50 to move authorizer and related classes to a
> > > > separate package. The new package will have java interface. This will
> > allow
> > > > implementations to not depend on kafka core and just on authorizer
> > package,
> > > > make authorization interface follow kafka’s coding standards and will
> > allow
> > > > java implementations to be cleaner. We can either completely drop scala
> > > > interface, which might be a pain for existing implementations, or we
> > can
> > > > have scala interface wrap java interface. Later allows a cleaner
> > > > deprecation path for existing scala authorizer interface, however it
> > may or
> > > > may not be feasible as Kafka server will have to somehow decide which
> > > > interface it should be looking for while loading authorizer
> > implementation,
> > > > this can probably be solved with a config or some reflection. If we
> > choose
> > > > to go this route, I can dig deeper.
> > > >
> > > > If we go with option 2 and commit on getting this in ASAP, preferably
> > in
> > > > 0.10, there will be fewer implementations that will be affected.
> > > >
> > > > and also moving to Java ,
> > > > > a authorizer implementation going to run inside a KafkaBroker and I
> > > > > don't see why this is necessary to move to clients package.
> > > > > Are we planning on introducing common module to have it independent
> > of
> > > > > broker and client code?
> > > > >
> > > > Yes, I think that would take away the requirement of depending on Kafka
> > > > core from authorizer implementations. Do you suggest otherwise?
> > > >
> > > >
> > > > > -Harsha
> > > > >
> > > > > On Thu, Apr 7, 2016, at 10:52 AM, Ashish Singh wrote:
> > > > > > We might want to take a call here. Following are the options.
> > > > > >
> > > > > >1. Let KIP-50 be the way it is, i.e., just add getDescription()
> > to
> > > > > >existing scala authorizer interface. It will break binary
> > > > > >compatibility
> > > > > >(only when using CLI and/or AdminCommand from >= 0.10 against
> > > > > >authorizer
> > > > > >implementations based on 0.9.). If we go this route, it is a
> > simpler
> > > > > >change
> > > > > >and existing implementations won’t have to change anything on
> > their
> > > > > >end.
> > > > > >2. Increase scope of KIP-50 to move authorizer and related
> > classes
> > > > to
> > > > > >a
> > > > > >separate package. The new package will have java interface. This
> > > > will
> > > > > >allow
> > > > > >implementations to not depend on kafka core and just on
> > authorizer
> > > > > >package,
> > > > > >make authorization interface follow kafka’s coding standards and
> > > > will
> > > > > >allow
> > > > > >java implementations to be cleaner. We 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

Harsha,

No, ZK data format for acls or SimplaAclAthorizer implementation is not
proposed to be changed.

Gwen,

The concern I was pointing at is that if we choose to go with Option 2,
i.e., move interface to a separate package, authorizer implementation will
have to be updated along with broker upgrade and authorizer implementations
need to make changes. I am not sure how this affects rolling upgrade.
Harsha, do you mind sharing your concerns on rolling upgrade?

On Thu, Apr 7, 2016 at 12:52 PM, Harsha  wrote:

> Ashish,
>   Thanks for the details. We are not changing any of the zookeeper
>   data format for acls right?
>
> Thanks,
> Harsha
>
> On Thu, Apr 7, 2016, at 11:25 AM, Gwen Shapira wrote:
> > Can you guys go into details on what will happen during a rolling upgrade
> > exactly?
> >
> > Gwen
> >
> > On Thu, Apr 7, 2016 at 11:21 AM, Ashish Singh 
> > wrote:
> >
> > > Hello Harsha,
> > >
> > > On Thu, Apr 7, 2016 at 11:03 AM, Harsha  wrote:
> > >
> > > "My only ask is to have this in 0.10. As Jay pointed out, right now
> > > > there
> > > > are not many implementations out there, we might want to fix it
> ASAP."
> > > >
> > > > Probably there aren't many implementations but there are lot of users
> > > > using these implementations in production clusters.
> > > > Isn't this going to break the rolling upgrade?
> > >
> > > It will and it is a concern, in my previous mail I have mentioned this
> as
> > > an issue if we choose to go this route. However, if we actually decide
> to
> > > do this, I would say it is better to do it sooner than later, as fewer
> > > implementations will be affected. Below is excerpt from my previous
> mail.
> > >
> > > Increase scope of KIP-50 to move authorizer and related classes to a
> > > separate package. The new package will have java interface. This will
> allow
> > > implementations to not depend on kafka core and just on authorizer
> package,
> > > make authorization interface follow kafka’s coding standards and will
> allow
> > > java implementations to be cleaner. We can either completely drop scala
> > > interface, which might be a pain for existing implementations, or we
> can
> > > have scala interface wrap java interface. Later allows a cleaner
> > > deprecation path for existing scala authorizer interface, however it
> may or
> > > may not be feasible as Kafka server will have to somehow decide which
> > > interface it should be looking for while loading authorizer
> implementation,
> > > this can probably be solved with a config or some reflection. If we
> choose
> > > to go this route, I can dig deeper.
> > >
> > > If we go with option 2 and commit on getting this in ASAP, preferably
> in
> > > 0.10, there will be fewer implementations that will be affected.
> > >
> > > and also moving to Java ,
> > > > a authorizer implementation going to run inside a KafkaBroker and I
> > > > don't see why this is necessary to move to clients package.
> > > > Are we planning on introducing common module to have it independent
> of
> > > > broker and client code?
> > > >
> > > Yes, I think that would take away the requirement of depending on Kafka
> > > core from authorizer implementations. Do you suggest otherwise?
> > >
> > >
> > > > -Harsha
> > > >
> > > > On Thu, Apr 7, 2016, at 10:52 AM, Ashish Singh wrote:
> > > > > We might want to take a call here. Following are the options.
> > > > >
> > > > >1. Let KIP-50 be the way it is, i.e., just add getDescription()
> to
> > > > >existing scala authorizer interface. It will break binary
> > > > >compatibility
> > > > >(only when using CLI and/or AdminCommand from >= 0.10 against
> > > > >authorizer
> > > > >implementations based on 0.9.). If we go this route, it is a
> simpler
> > > > >change
> > > > >and existing implementations won’t have to change anything on
> their
> > > > >end.
> > > > >2. Increase scope of KIP-50 to move authorizer and related
> classes
> > > to
> > > > >a
> > > > >separate package. The new package will have java interface. This
> > > will
> > > > >allow
> > > > >implementations to not depend on kafka core and just on
> authorizer
> > > > >package,
> > > > >make authorization interface follow kafka’s coding standards and
> > > will
> > > > >allow
> > > > >java implementations to be cleaner. We can either completely
> drop
> > > > >scala
> > > > >interface, which might be a pain for existing implementations,
> or we
> > > > >can
> > > > >have scala interface wrap java interface. Later allows a cleaner
> > > > >deprecation path for existing scala authorizer interface,
> however it
> > > > >may or
> > > > >may not be feasible as Kafka server will have to somehow decide
> > > which
> > > > >interface it should be looking for while loading authorizer
> > > > >implementation,
> > > > >this can probably be solved with a config or some 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Harsha]

Ashish,
  Thanks for the details. We are not changing any of the zookeeper
  data format for acls right?

Thanks,
Harsha

On Thu, Apr 7, 2016, at 11:25 AM, Gwen Shapira wrote:
> Can you guys go into details on what will happen during a rolling upgrade
> exactly?
> 
> Gwen
> 
> On Thu, Apr 7, 2016 at 11:21 AM, Ashish Singh 
> wrote:
> 
> > Hello Harsha,
> >
> > On Thu, Apr 7, 2016 at 11:03 AM, Harsha  wrote:
> >
> > "My only ask is to have this in 0.10. As Jay pointed out, right now
> > > there
> > > are not many implementations out there, we might want to fix it ASAP."
> > >
> > > Probably there aren't many implementations but there are lot of users
> > > using these implementations in production clusters.
> > > Isn't this going to break the rolling upgrade?
> >
> > It will and it is a concern, in my previous mail I have mentioned this as
> > an issue if we choose to go this route. However, if we actually decide to
> > do this, I would say it is better to do it sooner than later, as fewer
> > implementations will be affected. Below is excerpt from my previous mail.
> >
> > Increase scope of KIP-50 to move authorizer and related classes to a
> > separate package. The new package will have java interface. This will allow
> > implementations to not depend on kafka core and just on authorizer package,
> > make authorization interface follow kafka’s coding standards and will allow
> > java implementations to be cleaner. We can either completely drop scala
> > interface, which might be a pain for existing implementations, or we can
> > have scala interface wrap java interface. Later allows a cleaner
> > deprecation path for existing scala authorizer interface, however it may or
> > may not be feasible as Kafka server will have to somehow decide which
> > interface it should be looking for while loading authorizer implementation,
> > this can probably be solved with a config or some reflection. If we choose
> > to go this route, I can dig deeper.
> >
> > If we go with option 2 and commit on getting this in ASAP, preferably in
> > 0.10, there will be fewer implementations that will be affected.
> >
> > and also moving to Java ,
> > > a authorizer implementation going to run inside a KafkaBroker and I
> > > don't see why this is necessary to move to clients package.
> > > Are we planning on introducing common module to have it independent of
> > > broker and client code?
> > >
> > Yes, I think that would take away the requirement of depending on Kafka
> > core from authorizer implementations. Do you suggest otherwise?
> >
> >
> > > -Harsha
> > >
> > > On Thu, Apr 7, 2016, at 10:52 AM, Ashish Singh wrote:
> > > > We might want to take a call here. Following are the options.
> > > >
> > > >1. Let KIP-50 be the way it is, i.e., just add getDescription() to
> > > >existing scala authorizer interface. It will break binary
> > > >compatibility
> > > >(only when using CLI and/or AdminCommand from >= 0.10 against
> > > >authorizer
> > > >implementations based on 0.9.). If we go this route, it is a simpler
> > > >change
> > > >and existing implementations won’t have to change anything on their
> > > >end.
> > > >2. Increase scope of KIP-50 to move authorizer and related classes
> > to
> > > >a
> > > >separate package. The new package will have java interface. This
> > will
> > > >allow
> > > >implementations to not depend on kafka core and just on authorizer
> > > >package,
> > > >make authorization interface follow kafka’s coding standards and
> > will
> > > >allow
> > > >java implementations to be cleaner. We can either completely drop
> > > >scala
> > > >interface, which might be a pain for existing implementations, or we
> > > >can
> > > >have scala interface wrap java interface. Later allows a cleaner
> > > >deprecation path for existing scala authorizer interface, however it
> > > >may or
> > > >may not be feasible as Kafka server will have to somehow decide
> > which
> > > >interface it should be looking for while loading authorizer
> > > >implementation,
> > > >this can probably be solved with a config or some reflection. If we
> > > >choose
> > > >to go this route, I can dig deeper.
> > > >
> > > > If we decide to go with option 1, I think it would be fair to say that
> > > > scala authorizer interface will be around for some time, as there will
> > be
> > > > more implementations relying on it. If we go with option 2 and commit
> > on
> > > > getting this in ASAP, preferably in 0.10, there will be fewer
> > > > implementations that will be affected.
> > > >
> > > > *Another thing to notice is that scala authorizer interface is not
> > > > annotated as unstable.*
> > > > ​
> > > >
> > > > On Wed, Apr 6, 2016 at 9:41 AM, Ashish Singh 
> > > wrote:
> > > >
> > > > > I see value in minimizing breaking changes and I do not oppose the
> 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Gwen Shapira]

Can you guys go into details on what will happen during a rolling upgrade
exactly?

Gwen

On Thu, Apr 7, 2016 at 11:21 AM, Ashish Singh  wrote:

> Hello Harsha,
>
> On Thu, Apr 7, 2016 at 11:03 AM, Harsha  wrote:
>
> "My only ask is to have this in 0.10. As Jay pointed out, right now
> > there
> > are not many implementations out there, we might want to fix it ASAP."
> >
> > Probably there aren't many implementations but there are lot of users
> > using these implementations in production clusters.
> > Isn't this going to break the rolling upgrade?
>
> It will and it is a concern, in my previous mail I have mentioned this as
> an issue if we choose to go this route. However, if we actually decide to
> do this, I would say it is better to do it sooner than later, as fewer
> implementations will be affected. Below is excerpt from my previous mail.
>
> Increase scope of KIP-50 to move authorizer and related classes to a
> separate package. The new package will have java interface. This will allow
> implementations to not depend on kafka core and just on authorizer package,
> make authorization interface follow kafka’s coding standards and will allow
> java implementations to be cleaner. We can either completely drop scala
> interface, which might be a pain for existing implementations, or we can
> have scala interface wrap java interface. Later allows a cleaner
> deprecation path for existing scala authorizer interface, however it may or
> may not be feasible as Kafka server will have to somehow decide which
> interface it should be looking for while loading authorizer implementation,
> this can probably be solved with a config or some reflection. If we choose
> to go this route, I can dig deeper.
>
> If we go with option 2 and commit on getting this in ASAP, preferably in
> 0.10, there will be fewer implementations that will be affected.
>
> and also moving to Java ,
> > a authorizer implementation going to run inside a KafkaBroker and I
> > don't see why this is necessary to move to clients package.
> > Are we planning on introducing common module to have it independent of
> > broker and client code?
> >
> Yes, I think that would take away the requirement of depending on Kafka
> core from authorizer implementations. Do you suggest otherwise?
>
>
> > -Harsha
> >
> > On Thu, Apr 7, 2016, at 10:52 AM, Ashish Singh wrote:
> > > We might want to take a call here. Following are the options.
> > >
> > >1. Let KIP-50 be the way it is, i.e., just add getDescription() to
> > >existing scala authorizer interface. It will break binary
> > >compatibility
> > >(only when using CLI and/or AdminCommand from >= 0.10 against
> > >authorizer
> > >implementations based on 0.9.). If we go this route, it is a simpler
> > >change
> > >and existing implementations won’t have to change anything on their
> > >end.
> > >2. Increase scope of KIP-50 to move authorizer and related classes
> to
> > >a
> > >separate package. The new package will have java interface. This
> will
> > >allow
> > >implementations to not depend on kafka core and just on authorizer
> > >package,
> > >make authorization interface follow kafka’s coding standards and
> will
> > >allow
> > >java implementations to be cleaner. We can either completely drop
> > >scala
> > >interface, which might be a pain for existing implementations, or we
> > >can
> > >have scala interface wrap java interface. Later allows a cleaner
> > >deprecation path for existing scala authorizer interface, however it
> > >may or
> > >may not be feasible as Kafka server will have to somehow decide
> which
> > >interface it should be looking for while loading authorizer
> > >implementation,
> > >this can probably be solved with a config or some reflection. If we
> > >choose
> > >to go this route, I can dig deeper.
> > >
> > > If we decide to go with option 1, I think it would be fair to say that
> > > scala authorizer interface will be around for some time, as there will
> be
> > > more implementations relying on it. If we go with option 2 and commit
> on
> > > getting this in ASAP, preferably in 0.10, there will be fewer
> > > implementations that will be affected.
> > >
> > > *Another thing to notice is that scala authorizer interface is not
> > > annotated as unstable.*
> > > ​
> > >
> > > On Wed, Apr 6, 2016 at 9:41 AM, Ashish Singh 
> > wrote:
> > >
> > > > I see value in minimizing breaking changes and I do not oppose the
> > idea of
> > > > increasing scope of KIP-50 to move auth interface to java.
> > > >
> > > > As authorizer implementations do not really need to depend on Kafka
> > core,
> > > > I would suggest that we keep authorizer interface and its components
> > in a
> > > > separate package. I share the concern that right now using Resource,
> > > > Operation, etc, in java implementations is messy. I had 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

Hello Harsha,

On Thu, Apr 7, 2016 at 11:03 AM, Harsha  wrote:

"My only ask is to have this in 0.10. As Jay pointed out, right now
> there
> are not many implementations out there, we might want to fix it ASAP."
>
> Probably there aren't many implementations but there are lot of users
> using these implementations in production clusters.
> Isn't this going to break the rolling upgrade?

It will and it is a concern, in my previous mail I have mentioned this as
an issue if we choose to go this route. However, if we actually decide to
do this, I would say it is better to do it sooner than later, as fewer
implementations will be affected. Below is excerpt from my previous mail.

Increase scope of KIP-50 to move authorizer and related classes to a
separate package. The new package will have java interface. This will allow
implementations to not depend on kafka core and just on authorizer package,
make authorization interface follow kafka’s coding standards and will allow
java implementations to be cleaner. We can either completely drop scala
interface, which might be a pain for existing implementations, or we can
have scala interface wrap java interface. Later allows a cleaner
deprecation path for existing scala authorizer interface, however it may or
may not be feasible as Kafka server will have to somehow decide which
interface it should be looking for while loading authorizer implementation,
this can probably be solved with a config or some reflection. If we choose
to go this route, I can dig deeper.

If we go with option 2 and commit on getting this in ASAP, preferably in
0.10, there will be fewer implementations that will be affected.

and also moving to Java ,
> a authorizer implementation going to run inside a KafkaBroker and I
> don't see why this is necessary to move to clients package.
> Are we planning on introducing common module to have it independent of
> broker and client code?
>
Yes, I think that would take away the requirement of depending on Kafka
core from authorizer implementations. Do you suggest otherwise?


> -Harsha
>
> On Thu, Apr 7, 2016, at 10:52 AM, Ashish Singh wrote:
> > We might want to take a call here. Following are the options.
> >
> >1. Let KIP-50 be the way it is, i.e., just add getDescription() to
> >existing scala authorizer interface. It will break binary
> >compatibility
> >(only when using CLI and/or AdminCommand from >= 0.10 against
> >authorizer
> >implementations based on 0.9.). If we go this route, it is a simpler
> >change
> >and existing implementations won’t have to change anything on their
> >end.
> >2. Increase scope of KIP-50 to move authorizer and related classes to
> >a
> >separate package. The new package will have java interface. This will
> >allow
> >implementations to not depend on kafka core and just on authorizer
> >package,
> >make authorization interface follow kafka’s coding standards and will
> >allow
> >java implementations to be cleaner. We can either completely drop
> >scala
> >interface, which might be a pain for existing implementations, or we
> >can
> >have scala interface wrap java interface. Later allows a cleaner
> >deprecation path for existing scala authorizer interface, however it
> >may or
> >may not be feasible as Kafka server will have to somehow decide which
> >interface it should be looking for while loading authorizer
> >implementation,
> >this can probably be solved with a config or some reflection. If we
> >choose
> >to go this route, I can dig deeper.
> >
> > If we decide to go with option 1, I think it would be fair to say that
> > scala authorizer interface will be around for some time, as there will be
> > more implementations relying on it. If we go with option 2 and commit on
> > getting this in ASAP, preferably in 0.10, there will be fewer
> > implementations that will be affected.
> >
> > *Another thing to notice is that scala authorizer interface is not
> > annotated as unstable.*
> > ​
> >
> > On Wed, Apr 6, 2016 at 9:41 AM, Ashish Singh 
> wrote:
> >
> > > I see value in minimizing breaking changes and I do not oppose the
> idea of
> > > increasing scope of KIP-50 to move auth interface to java.
> > >
> > > As authorizer implementations do not really need to depend on Kafka
> core,
> > > I would suggest that we keep authorizer interface and its components
> in a
> > > separate package. I share the concern that right now using Resource,
> > > Operation, etc, in java implementations is messy. I had to deal with
> lot of
> > > it while writing Apache Sentry plugin.
> > >
> > > My only ask is to have this in 0.10. As Jay pointed out, right now
> there
> > > are not many implementations out there, we might want to fix it ASAP.
> I can
> > > only speak of Sentry integration and I think 0.10 will be the best for
> such
> > > a change, as I should be able to adopt the 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Harsha]

"My only ask is to have this in 0.10. As Jay pointed out, right now
there
are not many implementations out there, we might want to fix it ASAP."

Probably there aren't many implementations but there are lot of users
using these implementations in production clusters. 
Isn't this going to break the rolling upgrade? and also moving to Java ,
a authorizer implementation going to run inside a KafkaBroker and I
don't see why this is necessary to move to clients package. 
Are we planning on introducing common module to have it independent of
broker and client code?

-Harsha

On Thu, Apr 7, 2016, at 10:52 AM, Ashish Singh wrote:
> We might want to take a call here. Following are the options.
> 
>1. Let KIP-50 be the way it is, i.e., just add getDescription() to
>existing scala authorizer interface. It will break binary
>compatibility
>(only when using CLI and/or AdminCommand from >= 0.10 against
>authorizer
>implementations based on 0.9.). If we go this route, it is a simpler
>change
>and existing implementations won’t have to change anything on their
>end.
>2. Increase scope of KIP-50 to move authorizer and related classes to
>a
>separate package. The new package will have java interface. This will
>allow
>implementations to not depend on kafka core and just on authorizer
>package,
>make authorization interface follow kafka’s coding standards and will
>allow
>java implementations to be cleaner. We can either completely drop
>scala
>interface, which might be a pain for existing implementations, or we
>can
>have scala interface wrap java interface. Later allows a cleaner
>deprecation path for existing scala authorizer interface, however it
>may or
>may not be feasible as Kafka server will have to somehow decide which
>interface it should be looking for while loading authorizer
>implementation,
>this can probably be solved with a config or some reflection. If we
>choose
>to go this route, I can dig deeper.
> 
> If we decide to go with option 1, I think it would be fair to say that
> scala authorizer interface will be around for some time, as there will be
> more implementations relying on it. If we go with option 2 and commit on
> getting this in ASAP, preferably in 0.10, there will be fewer
> implementations that will be affected.
> 
> *Another thing to notice is that scala authorizer interface is not
> annotated as unstable.*
> ​
> 
> On Wed, Apr 6, 2016 at 9:41 AM, Ashish Singh  wrote:
> 
> > I see value in minimizing breaking changes and I do not oppose the idea of
> > increasing scope of KIP-50 to move auth interface to java.
> >
> > As authorizer implementations do not really need to depend on Kafka core,
> > I would suggest that we keep authorizer interface and its components in a
> > separate package. I share the concern that right now using Resource,
> > Operation, etc, in java implementations is messy. I had to deal with lot of
> > it while writing Apache Sentry plugin.
> >
> > My only ask is to have this in 0.10. As Jay pointed out, right now there
> > are not many implementations out there, we might want to fix it ASAP. I can
> > only speak of Sentry integration and I think 0.10 will be the best for such
> > a change, as I should be able to adopt the changes in Sentry integration
> > before a lot of users start using it.
> >
> > On Wed, Apr 6, 2016 at 9:25 AM, Ismael Juma  wrote:
> >
> >> It is small, but breaks binary compatibility.
> >>
> >> Ismael
> >>
> >> On Wed, Apr 6, 2016 at 5:20 PM, Grant Henke  wrote:
> >>
> >> > KIP-50 as defined is very small. I don't see any harm in putting it in
> >> as
> >> > is and then tackling the follow up work.
> >> >
> >> >
> >> > On Wed, Apr 6, 2016 at 11:16 AM, Ismael Juma  wrote:
> >> >
> >> > > Thanks Grant. I wonder if KIP-50 should just be done as part of this
> >> > work.
> >> > >
> >> > > Ismael
> >> > >
> >> > > On Wed, Apr 6, 2016 at 5:12 PM, Grant Henke 
> >> wrote:
> >> > >
> >> > > > My work with KIP-4 found that many of the Scala classes used in the
> >> > > > Authorizer interface are needed in the Clients package when adding
> >> the
> >> > > > various ACL requests and responses. I also found that we don't have
> >> > > > standard Exceptions defined for the authorizer interface. This means
> >> > that
> >> > > > when I add the Authorizer calls to the broker and wire protocols all
> >> > > > exceptions will be reported as an "Unknown Error" back to the user
> >> via
> >> > > the
> >> > > > wire protocol.
> >> > > >
> >> > > > I have written more about it on the KIP-4 wiki and created jiras to
> >> > track
> >> > > > those issues (See below). I think we should wrap up this KIP as is
> >> and
> >> > > > tackle the Java/Exception changes as a part of those jiras/kips.
> >> > > >
> >> > > >- KIP-4 "Follow Up Changes"
> >> > > ><
> 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

We might want to take a call here. Following are the options.

   1. Let KIP-50 be the way it is, i.e., just add getDescription() to
   existing scala authorizer interface. It will break binary compatibility
   (only when using CLI and/or AdminCommand from >= 0.10 against authorizer
   implementations based on 0.9.). If we go this route, it is a simpler change
   and existing implementations won’t have to change anything on their end.
   2. Increase scope of KIP-50 to move authorizer and related classes to a
   separate package. The new package will have java interface. This will allow
   implementations to not depend on kafka core and just on authorizer package,
   make authorization interface follow kafka’s coding standards and will allow
   java implementations to be cleaner. We can either completely drop scala
   interface, which might be a pain for existing implementations, or we can
   have scala interface wrap java interface. Later allows a cleaner
   deprecation path for existing scala authorizer interface, however it may or
   may not be feasible as Kafka server will have to somehow decide which
   interface it should be looking for while loading authorizer implementation,
   this can probably be solved with a config or some reflection. If we choose
   to go this route, I can dig deeper.

If we decide to go with option 1, I think it would be fair to say that
scala authorizer interface will be around for some time, as there will be
more implementations relying on it. If we go with option 2 and commit on
getting this in ASAP, preferably in 0.10, there will be fewer
implementations that will be affected.

*Another thing to notice is that scala authorizer interface is not
annotated as unstable.*
​

On Wed, Apr 6, 2016 at 9:41 AM, Ashish Singh  wrote:

> I see value in minimizing breaking changes and I do not oppose the idea of
> increasing scope of KIP-50 to move auth interface to java.
>
> As authorizer implementations do not really need to depend on Kafka core,
> I would suggest that we keep authorizer interface and its components in a
> separate package. I share the concern that right now using Resource,
> Operation, etc, in java implementations is messy. I had to deal with lot of
> it while writing Apache Sentry plugin.
>
> My only ask is to have this in 0.10. As Jay pointed out, right now there
> are not many implementations out there, we might want to fix it ASAP. I can
> only speak of Sentry integration and I think 0.10 will be the best for such
> a change, as I should be able to adopt the changes in Sentry integration
> before a lot of users start using it.
>
> On Wed, Apr 6, 2016 at 9:25 AM, Ismael Juma  wrote:
>
>> It is small, but breaks binary compatibility.
>>
>> Ismael
>>
>> On Wed, Apr 6, 2016 at 5:20 PM, Grant Henke  wrote:
>>
>> > KIP-50 as defined is very small. I don't see any harm in putting it in
>> as
>> > is and then tackling the follow up work.
>> >
>> >
>> > On Wed, Apr 6, 2016 at 11:16 AM, Ismael Juma  wrote:
>> >
>> > > Thanks Grant. I wonder if KIP-50 should just be done as part of this
>> > work.
>> > >
>> > > Ismael
>> > >
>> > > On Wed, Apr 6, 2016 at 5:12 PM, Grant Henke 
>> wrote:
>> > >
>> > > > My work with KIP-4 found that many of the Scala classes used in the
>> > > > Authorizer interface are needed in the Clients package when adding
>> the
>> > > > various ACL requests and responses. I also found that we don't have
>> > > > standard Exceptions defined for the authorizer interface. This means
>> > that
>> > > > when I add the Authorizer calls to the broker and wire protocols all
>> > > > exceptions will be reported as an "Unknown Error" back to the user
>> via
>> > > the
>> > > > wire protocol.
>> > > >
>> > > > I have written more about it on the KIP-4 wiki and created jiras to
>> > track
>> > > > those issues (See below). I think we should wrap up this KIP as is
>> and
>> > > > tackle the Java/Exception changes as a part of those jiras/kips.
>> > > >
>> > > >- KIP-4 "Follow Up Changes"
>> > > ><
>> > > >
>> > >
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-4+-+Command+line+and+centralized+administrative+operations#KIP-4-Commandlineandcentralizedadministrativeoperations-FollowUpChangesfollow-up-changes
>> > > > >
>> > > >- KAFKA-3509 :
>> > > > Provide
>> > > >an Authorizer interface using the Java client enumerator classes
>> > > >- KAFKA-3507 :
>> > > Define
>> > > >standard exceptions for the Authorizer interface
>> > > >
>> > > > Thank you,
>> > > > Grant
>> > > >
>> > > > On Wed, Apr 6, 2016 at 10:58 AM, Jay Kreps 
>> wrote:
>> > > >
>> > > > > Hey Ismael,
>> > > > >
>> > > > > Yeah I think this is a minor cleanliness thing. Since this is kind
>> > of a
>> > > > > power user interface I don't feel 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

I see value in minimizing breaking changes and I do not oppose the idea of
increasing scope of KIP-50 to move auth interface to java.

As authorizer implementations do not really need to depend on Kafka core, I
would suggest that we keep authorizer interface and its components in a
separate package. I share the concern that right now using Resource,
Operation, etc, in java implementations is messy. I had to deal with lot of
it while writing Apache Sentry plugin.

My only ask is to have this in 0.10. As Jay pointed out, right now there
are not many implementations out there, we might want to fix it ASAP. I can
only speak of Sentry integration and I think 0.10 will be the best for such
a change, as I should be able to adopt the changes in Sentry integration
before a lot of users start using it.

On Wed, Apr 6, 2016 at 9:25 AM, Ismael Juma  wrote:

> It is small, but breaks binary compatibility.
>
> Ismael
>
> On Wed, Apr 6, 2016 at 5:20 PM, Grant Henke  wrote:
>
> > KIP-50 as defined is very small. I don't see any harm in putting it in as
> > is and then tackling the follow up work.
> >
> >
> > On Wed, Apr 6, 2016 at 11:16 AM, Ismael Juma  wrote:
> >
> > > Thanks Grant. I wonder if KIP-50 should just be done as part of this
> > work.
> > >
> > > Ismael
> > >
> > > On Wed, Apr 6, 2016 at 5:12 PM, Grant Henke 
> wrote:
> > >
> > > > My work with KIP-4 found that many of the Scala classes used in the
> > > > Authorizer interface are needed in the Clients package when adding
> the
> > > > various ACL requests and responses. I also found that we don't have
> > > > standard Exceptions defined for the authorizer interface. This means
> > that
> > > > when I add the Authorizer calls to the broker and wire protocols all
> > > > exceptions will be reported as an "Unknown Error" back to the user
> via
> > > the
> > > > wire protocol.
> > > >
> > > > I have written more about it on the KIP-4 wiki and created jiras to
> > track
> > > > those issues (See below). I think we should wrap up this KIP as is
> and
> > > > tackle the Java/Exception changes as a part of those jiras/kips.
> > > >
> > > >- KIP-4 "Follow Up Changes"
> > > ><
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-4+-+Command+line+and+centralized+administrative+operations#KIP-4-Commandlineandcentralizedadministrativeoperations-FollowUpChangesfollow-up-changes
> > > > >
> > > >- KAFKA-3509 :
> > > > Provide
> > > >an Authorizer interface using the Java client enumerator classes
> > > >- KAFKA-3507 :
> > > Define
> > > >standard exceptions for the Authorizer interface
> > > >
> > > > Thank you,
> > > > Grant
> > > >
> > > > On Wed, Apr 6, 2016 at 10:58 AM, Jay Kreps  wrote:
> > > >
> > > > > Hey Ismael,
> > > > >
> > > > > Yeah I think this is a minor cleanliness thing. Since this is kind
> > of a
> > > > > power user interface I don't feel strongly either way.
> > > > >
> > > > > My motivation with Scala is just that we've tried to move to having
> > the
> > > > > public interfaces be Java, and as a group we definitely struggled a
> > lot
> > > > > with understanding and maintaining Scala compatibility in the older
> > > > > clients.
> > > > >
> > > > > -Jay
> > > > >
> > > > > On Tue, Apr 5, 2016 at 11:46 PM, Ismael Juma 
> > > wrote:
> > > > >
> > > > > > Hi Jay,
> > > > > >
> > > > > > On Wed, Apr 6, 2016 at 3:48 AM, Jay Kreps 
> > wrote:
> > > > > >
> > > > > > > Given that we're breaking compatibility anyway should we:
> > > > > > >
> > > > > >
> > > > > > We are not breaking source compatibility since the new method
> has a
> > > > > default
> > > > > > implementation. I take it that you mean binary compatibility?
> > > > > >
> > > > > >
> > > > > > > 1. Remove the get prefix on this method and the existing one
> > which
> > > > > > violate
> > > > > > > our own code style guidelines (Oops! Kind of sad we went
> through
> > > the
> > > > > > whole
> > > > > > > KIP process and no one even flagged this)
> > > > > > >
> > > > > >
> > > > > > I did flag this during the discussion and Ashish said he would
> > change
> > > > it
> > > > > if
> > > > > > other people felt that it should be changed.
> > > > > >
> > > > > >
> > > > > > > 2. Move the interface out of scala to be a normal Java
> interface
> > > > > > >
> > > > > > > This breaks source compatibility but probably what we should
> have
> > > > done
> > > > > > > originally I suspect. Probably there are few enough
> > implementations
> > > > of
> > > > > > this
> > > > > > > that it is better to just rip the bandaid off.
> > > > > > >
> > > > > >
> > > > > > Can you please explain the motivation? It did come up in previous
> > > > > > discussions that some things like Operation and ResourceType
> should
> > > be
> > > 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Grant Henke]

I understand. My thinking is that since this is a major release, if we are
going to break in anyway we should do it now. Otherwise we will need to
wait until 0.11. The work in adding standard exceptions and
an alternative Java interface should be able to be done in a compatible
manner (deprecating the old stuff for later removal).

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ismael Juma]

It is small, but breaks binary compatibility.

Ismael

On Wed, Apr 6, 2016 at 5:20 PM, Grant Henke  wrote:

> KIP-50 as defined is very small. I don't see any harm in putting it in as
> is and then tackling the follow up work.
>
>
> On Wed, Apr 6, 2016 at 11:16 AM, Ismael Juma  wrote:
>
> > Thanks Grant. I wonder if KIP-50 should just be done as part of this
> work.
> >
> > Ismael
> >
> > On Wed, Apr 6, 2016 at 5:12 PM, Grant Henke  wrote:
> >
> > > My work with KIP-4 found that many of the Scala classes used in the
> > > Authorizer interface are needed in the Clients package when adding the
> > > various ACL requests and responses. I also found that we don't have
> > > standard Exceptions defined for the authorizer interface. This means
> that
> > > when I add the Authorizer calls to the broker and wire protocols all
> > > exceptions will be reported as an "Unknown Error" back to the user via
> > the
> > > wire protocol.
> > >
> > > I have written more about it on the KIP-4 wiki and created jiras to
> track
> > > those issues (See below). I think we should wrap up this KIP as is and
> > > tackle the Java/Exception changes as a part of those jiras/kips.
> > >
> > >- KIP-4 "Follow Up Changes"
> > ><
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-4+-+Command+line+and+centralized+administrative+operations#KIP-4-Commandlineandcentralizedadministrativeoperations-FollowUpChangesfollow-up-changes
> > > >
> > >- KAFKA-3509 :
> > > Provide
> > >an Authorizer interface using the Java client enumerator classes
> > >- KAFKA-3507 :
> > Define
> > >standard exceptions for the Authorizer interface
> > >
> > > Thank you,
> > > Grant
> > >
> > > On Wed, Apr 6, 2016 at 10:58 AM, Jay Kreps  wrote:
> > >
> > > > Hey Ismael,
> > > >
> > > > Yeah I think this is a minor cleanliness thing. Since this is kind
> of a
> > > > power user interface I don't feel strongly either way.
> > > >
> > > > My motivation with Scala is just that we've tried to move to having
> the
> > > > public interfaces be Java, and as a group we definitely struggled a
> lot
> > > > with understanding and maintaining Scala compatibility in the older
> > > > clients.
> > > >
> > > > -Jay
> > > >
> > > > On Tue, Apr 5, 2016 at 11:46 PM, Ismael Juma 
> > wrote:
> > > >
> > > > > Hi Jay,
> > > > >
> > > > > On Wed, Apr 6, 2016 at 3:48 AM, Jay Kreps 
> wrote:
> > > > >
> > > > > > Given that we're breaking compatibility anyway should we:
> > > > > >
> > > > >
> > > > > We are not breaking source compatibility since the new method has a
> > > > default
> > > > > implementation. I take it that you mean binary compatibility?
> > > > >
> > > > >
> > > > > > 1. Remove the get prefix on this method and the existing one
> which
> > > > > violate
> > > > > > our own code style guidelines (Oops! Kind of sad we went through
> > the
> > > > > whole
> > > > > > KIP process and no one even flagged this)
> > > > > >
> > > > >
> > > > > I did flag this during the discussion and Ashish said he would
> change
> > > it
> > > > if
> > > > > other people felt that it should be changed.
> > > > >
> > > > >
> > > > > > 2. Move the interface out of scala to be a normal Java interface
> > > > > >
> > > > > > This breaks source compatibility but probably what we should have
> > > done
> > > > > > originally I suspect. Probably there are few enough
> implementations
> > > of
> > > > > this
> > > > > > that it is better to just rip the bandaid off.
> > > > > >
> > > > >
> > > > > Can you please explain the motivation? It did come up in previous
> > > > > discussions that some things like Operation and ResourceType should
> > be
> > > in
> > > > > the clients library, but not Authorizer itself. Are we saying that
> > any
> > > > > pluggable interface should be in Java so that users can implement
> it
> > > > > without including the Scala library?
> > > > >
> > > > > Grant, you originally suggested that some things would have to be
> in
> > > the
> > > > > Java side as well. Can you please elaborate on this?
> > > > >
> > > > > Ismael
> > > > >
> > > >
> > >
> > >
> > >
> > > --
> > > Grant Henke
> > > Software Engineer | Cloudera
> > > gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke
> > >
> >
>
>
>
> --
> Grant Henke
> Software Engineer | Cloudera
> gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke
>

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Grant Henke]

>
> The follow-up question is where these classes should live. We have the
> common package in clients for common code. However, if something is
> exclusively used by the broker, we could add it under core/src/main/java
> instead. There are pros and cons for each approach, so I was wondering if
> some thought has gone into this already.


Just adding my 2 cents here. Since the implemented Authorizer needs to
exist on the brokers classpath, I think the interface should be a part of
the core package regardless of any Java vs Scala choice. Its components,
that need to be used in the clients for requests/responses should be in the
clients/common package though. Some of this has been done in my preliminary
ACLs request/response pull request:
https://github.com/apache/kafka/pull/1005

On Wed, Apr 6, 2016 at 11:20 AM, Grant Henke  wrote:

> KIP-50 as defined is very small. I don't see any harm in putting it in as
> is and then tackling the follow up work.
>
>
> On Wed, Apr 6, 2016 at 11:16 AM, Ismael Juma  wrote:
>
>> Thanks Grant. I wonder if KIP-50 should just be done as part of this work.
>>
>> Ismael
>>
>> On Wed, Apr 6, 2016 at 5:12 PM, Grant Henke  wrote:
>>
>> > My work with KIP-4 found that many of the Scala classes used in the
>> > Authorizer interface are needed in the Clients package when adding the
>> > various ACL requests and responses. I also found that we don't have
>> > standard Exceptions defined for the authorizer interface. This means
>> that
>> > when I add the Authorizer calls to the broker and wire protocols all
>> > exceptions will be reported as an "Unknown Error" back to the user via
>> the
>> > wire protocol.
>> >
>> > I have written more about it on the KIP-4 wiki and created jiras to
>> track
>> > those issues (See below). I think we should wrap up this KIP as is and
>> > tackle the Java/Exception changes as a part of those jiras/kips.
>> >
>> >- KIP-4 "Follow Up Changes"
>> ><
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-4+-+Command+line+and+centralized+administrative+operations#KIP-4-Commandlineandcentralizedadministrativeoperations-FollowUpChangesfollow-up-changes
>> > >
>> >- KAFKA-3509 :
>> > Provide
>> >an Authorizer interface using the Java client enumerator classes
>> >- KAFKA-3507 :
>> Define
>> >standard exceptions for the Authorizer interface
>> >
>> > Thank you,
>> > Grant
>> >
>> > On Wed, Apr 6, 2016 at 10:58 AM, Jay Kreps  wrote:
>> >
>> > > Hey Ismael,
>> > >
>> > > Yeah I think this is a minor cleanliness thing. Since this is kind of
>> a
>> > > power user interface I don't feel strongly either way.
>> > >
>> > > My motivation with Scala is just that we've tried to move to having
>> the
>> > > public interfaces be Java, and as a group we definitely struggled a
>> lot
>> > > with understanding and maintaining Scala compatibility in the older
>> > > clients.
>> > >
>> > > -Jay
>> > >
>> > > On Tue, Apr 5, 2016 at 11:46 PM, Ismael Juma 
>> wrote:
>> > >
>> > > > Hi Jay,
>> > > >
>> > > > On Wed, Apr 6, 2016 at 3:48 AM, Jay Kreps  wrote:
>> > > >
>> > > > > Given that we're breaking compatibility anyway should we:
>> > > > >
>> > > >
>> > > > We are not breaking source compatibility since the new method has a
>> > > default
>> > > > implementation. I take it that you mean binary compatibility?
>> > > >
>> > > >
>> > > > > 1. Remove the get prefix on this method and the existing one which
>> > > > violate
>> > > > > our own code style guidelines (Oops! Kind of sad we went through
>> the
>> > > > whole
>> > > > > KIP process and no one even flagged this)
>> > > > >
>> > > >
>> > > > I did flag this during the discussion and Ashish said he would
>> change
>> > it
>> > > if
>> > > > other people felt that it should be changed.
>> > > >
>> > > >
>> > > > > 2. Move the interface out of scala to be a normal Java interface
>> > > > >
>> > > > > This breaks source compatibility but probably what we should have
>> > done
>> > > > > originally I suspect. Probably there are few enough
>> implementations
>> > of
>> > > > this
>> > > > > that it is better to just rip the bandaid off.
>> > > > >
>> > > >
>> > > > Can you please explain the motivation? It did come up in previous
>> > > > discussions that some things like Operation and ResourceType should
>> be
>> > in
>> > > > the clients library, but not Authorizer itself. Are we saying that
>> any
>> > > > pluggable interface should be in Java so that users can implement it
>> > > > without including the Scala library?
>> > > >
>> > > > Grant, you originally suggested that some things would have to be in
>> > the
>> > > > Java side as well. Can you please elaborate on this?
>> > > >
>> > > > Ismael
>> > > >
>> > >
>> >
>> >
>> >
>> > --
>> > Grant Henke
>> > Software Engineer | 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Grant Henke]

KIP-50 as defined is very small. I don't see any harm in putting it in as
is and then tackling the follow up work.


On Wed, Apr 6, 2016 at 11:16 AM, Ismael Juma  wrote:

> Thanks Grant. I wonder if KIP-50 should just be done as part of this work.
>
> Ismael
>
> On Wed, Apr 6, 2016 at 5:12 PM, Grant Henke  wrote:
>
> > My work with KIP-4 found that many of the Scala classes used in the
> > Authorizer interface are needed in the Clients package when adding the
> > various ACL requests and responses. I also found that we don't have
> > standard Exceptions defined for the authorizer interface. This means that
> > when I add the Authorizer calls to the broker and wire protocols all
> > exceptions will be reported as an "Unknown Error" back to the user via
> the
> > wire protocol.
> >
> > I have written more about it on the KIP-4 wiki and created jiras to track
> > those issues (See below). I think we should wrap up this KIP as is and
> > tackle the Java/Exception changes as a part of those jiras/kips.
> >
> >- KIP-4 "Follow Up Changes"
> ><
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-4+-+Command+line+and+centralized+administrative+operations#KIP-4-Commandlineandcentralizedadministrativeoperations-FollowUpChangesfollow-up-changes
> > >
> >- KAFKA-3509 :
> > Provide
> >an Authorizer interface using the Java client enumerator classes
> >- KAFKA-3507 :
> Define
> >standard exceptions for the Authorizer interface
> >
> > Thank you,
> > Grant
> >
> > On Wed, Apr 6, 2016 at 10:58 AM, Jay Kreps  wrote:
> >
> > > Hey Ismael,
> > >
> > > Yeah I think this is a minor cleanliness thing. Since this is kind of a
> > > power user interface I don't feel strongly either way.
> > >
> > > My motivation with Scala is just that we've tried to move to having the
> > > public interfaces be Java, and as a group we definitely struggled a lot
> > > with understanding and maintaining Scala compatibility in the older
> > > clients.
> > >
> > > -Jay
> > >
> > > On Tue, Apr 5, 2016 at 11:46 PM, Ismael Juma 
> wrote:
> > >
> > > > Hi Jay,
> > > >
> > > > On Wed, Apr 6, 2016 at 3:48 AM, Jay Kreps  wrote:
> > > >
> > > > > Given that we're breaking compatibility anyway should we:
> > > > >
> > > >
> > > > We are not breaking source compatibility since the new method has a
> > > default
> > > > implementation. I take it that you mean binary compatibility?
> > > >
> > > >
> > > > > 1. Remove the get prefix on this method and the existing one which
> > > > violate
> > > > > our own code style guidelines (Oops! Kind of sad we went through
> the
> > > > whole
> > > > > KIP process and no one even flagged this)
> > > > >
> > > >
> > > > I did flag this during the discussion and Ashish said he would change
> > it
> > > if
> > > > other people felt that it should be changed.
> > > >
> > > >
> > > > > 2. Move the interface out of scala to be a normal Java interface
> > > > >
> > > > > This breaks source compatibility but probably what we should have
> > done
> > > > > originally I suspect. Probably there are few enough implementations
> > of
> > > > this
> > > > > that it is better to just rip the bandaid off.
> > > > >
> > > >
> > > > Can you please explain the motivation? It did come up in previous
> > > > discussions that some things like Operation and ResourceType should
> be
> > in
> > > > the clients library, but not Authorizer itself. Are we saying that
> any
> > > > pluggable interface should be in Java so that users can implement it
> > > > without including the Scala library?
> > > >
> > > > Grant, you originally suggested that some things would have to be in
> > the
> > > > Java side as well. Can you please elaborate on this?
> > > >
> > > > Ismael
> > > >
> > >
> >
> >
> >
> > --
> > Grant Henke
> > Software Engineer | Cloudera
> > gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke
> >
>



-- 
Grant Henke
Software Engineer | Cloudera
gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ismael Juma]

Hi Jay,

Thanks for the explanation.

My take is that the main benefit in keeping pluggable broker interfaces in
Java is that implementors don't have to include the Scala library (and
don't have to choose a Scala version, for example). I think this is a
strong enough reason on its own. I am less convinced about the
compatibility argument, but it doesn't matter. :)

The follow-up question is where these classes should live. We have the
common package in clients for common code. However, if something is
exclusively used by the broker, we could add it under core/src/main/java
instead. There are pros and cons for each approach, so I was wondering if
some thought has gone into this already.

Ismael

On Wed, Apr 6, 2016 at 4:58 PM, Jay Kreps  wrote:

> Hey Ismael,
>
> Yeah I think this is a minor cleanliness thing. Since this is kind of a
> power user interface I don't feel strongly either way.
>
> My motivation with Scala is just that we've tried to move to having the
> public interfaces be Java, and as a group we definitely struggled a lot
> with understanding and maintaining Scala compatibility in the older
> clients.
>
> -Jay
>
> On Tue, Apr 5, 2016 at 11:46 PM, Ismael Juma  wrote:
>
> > Hi Jay,
> >
> > On Wed, Apr 6, 2016 at 3:48 AM, Jay Kreps  wrote:
> >
> > > Given that we're breaking compatibility anyway should we:
> > >
> >
> > We are not breaking source compatibility since the new method has a
> default
> > implementation. I take it that you mean binary compatibility?
> >
> >
> > > 1. Remove the get prefix on this method and the existing one which
> > violate
> > > our own code style guidelines (Oops! Kind of sad we went through the
> > whole
> > > KIP process and no one even flagged this)
> > >
> >
> > I did flag this during the discussion and Ashish said he would change it
> if
> > other people felt that it should be changed.
> >
> >
> > > 2. Move the interface out of scala to be a normal Java interface
> > >
> > > This breaks source compatibility but probably what we should have done
> > > originally I suspect. Probably there are few enough implementations of
> > this
> > > that it is better to just rip the bandaid off.
> > >
> >
> > Can you please explain the motivation? It did come up in previous
> > discussions that some things like Operation and ResourceType should be in
> > the clients library, but not Authorizer itself. Are we saying that any
> > pluggable interface should be in Java so that users can implement it
> > without including the Scala library?
> >
> > Grant, you originally suggested that some things would have to be in the
> > Java side as well. Can you please elaborate on this?
> >
> > Ismael
> >
>

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Grant Henke]

My work with KIP-4 found that many of the Scala classes used in the
Authorizer interface are needed in the Clients package when adding the
various ACL requests and responses. I also found that we don't have
standard Exceptions defined for the authorizer interface. This means that
when I add the Authorizer calls to the broker and wire protocols all
exceptions will be reported as an "Unknown Error" back to the user via the
wire protocol.

I have written more about it on the KIP-4 wiki and created jiras to track
those issues (See below). I think we should wrap up this KIP as is and
tackle the Java/Exception changes as a part of those jiras/kips.

   - KIP-4 "Follow Up Changes"
   

   - KAFKA-3509 : Provide
   an Authorizer interface using the Java client enumerator classes
   - KAFKA-3507 : Define
   standard exceptions for the Authorizer interface

Thank you,
Grant

On Wed, Apr 6, 2016 at 10:58 AM, Jay Kreps  wrote:

> Hey Ismael,
>
> Yeah I think this is a minor cleanliness thing. Since this is kind of a
> power user interface I don't feel strongly either way.
>
> My motivation with Scala is just that we've tried to move to having the
> public interfaces be Java, and as a group we definitely struggled a lot
> with understanding and maintaining Scala compatibility in the older
> clients.
>
> -Jay
>
> On Tue, Apr 5, 2016 at 11:46 PM, Ismael Juma  wrote:
>
> > Hi Jay,
> >
> > On Wed, Apr 6, 2016 at 3:48 AM, Jay Kreps  wrote:
> >
> > > Given that we're breaking compatibility anyway should we:
> > >
> >
> > We are not breaking source compatibility since the new method has a
> default
> > implementation. I take it that you mean binary compatibility?
> >
> >
> > > 1. Remove the get prefix on this method and the existing one which
> > violate
> > > our own code style guidelines (Oops! Kind of sad we went through the
> > whole
> > > KIP process and no one even flagged this)
> > >
> >
> > I did flag this during the discussion and Ashish said he would change it
> if
> > other people felt that it should be changed.
> >
> >
> > > 2. Move the interface out of scala to be a normal Java interface
> > >
> > > This breaks source compatibility but probably what we should have done
> > > originally I suspect. Probably there are few enough implementations of
> > this
> > > that it is better to just rip the bandaid off.
> > >
> >
> > Can you please explain the motivation? It did come up in previous
> > discussions that some things like Operation and ResourceType should be in
> > the clients library, but not Authorizer itself. Are we saying that any
> > pluggable interface should be in Java so that users can implement it
> > without including the Scala library?
> >
> > Grant, you originally suggested that some things would have to be in the
> > Java side as well. Can you please elaborate on this?
> >
> > Ismael
> >
>



-- 
Grant Henke
Software Engineer | Cloudera
gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Jay Kreps]

Hey Ismael,

Yeah I think this is a minor cleanliness thing. Since this is kind of a
power user interface I don't feel strongly either way.

My motivation with Scala is just that we've tried to move to having the
public interfaces be Java, and as a group we definitely struggled a lot
with understanding and maintaining Scala compatibility in the older clients.

-Jay

On Tue, Apr 5, 2016 at 11:46 PM, Ismael Juma  wrote:

> Hi Jay,
>
> On Wed, Apr 6, 2016 at 3:48 AM, Jay Kreps  wrote:
>
> > Given that we're breaking compatibility anyway should we:
> >
>
> We are not breaking source compatibility since the new method has a default
> implementation. I take it that you mean binary compatibility?
>
>
> > 1. Remove the get prefix on this method and the existing one which
> violate
> > our own code style guidelines (Oops! Kind of sad we went through the
> whole
> > KIP process and no one even flagged this)
> >
>
> I did flag this during the discussion and Ashish said he would change it if
> other people felt that it should be changed.
>
>
> > 2. Move the interface out of scala to be a normal Java interface
> >
> > This breaks source compatibility but probably what we should have done
> > originally I suspect. Probably there are few enough implementations of
> this
> > that it is better to just rip the bandaid off.
> >
>
> Can you please explain the motivation? It did come up in previous
> discussions that some things like Operation and ResourceType should be in
> the clients library, but not Authorizer itself. Are we saying that any
> pluggable interface should be in Java so that users can implement it
> without including the Scala library?
>
> Grant, you originally suggested that some things would have to be in the
> Java side as well. Can you please elaborate on this?
>
> Ismael
>

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ismael Juma]

Hi Jay,

On Wed, Apr 6, 2016 at 3:48 AM, Jay Kreps  wrote:

> Given that we're breaking compatibility anyway should we:
>

We are not breaking source compatibility since the new method has a default
implementation. I take it that you mean binary compatibility?


> 1. Remove the get prefix on this method and the existing one which violate
> our own code style guidelines (Oops! Kind of sad we went through the whole
> KIP process and no one even flagged this)
>

I did flag this during the discussion and Ashish said he would change it if
other people felt that it should be changed.


> 2. Move the interface out of scala to be a normal Java interface
>
> This breaks source compatibility but probably what we should have done
> originally I suspect. Probably there are few enough implementations of this
> that it is better to just rip the bandaid off.
>

Can you please explain the motivation? It did come up in previous
discussions that some things like Operation and ResourceType should be in
the clients library, but not Authorizer itself. Are we saying that any
pluggable interface should be in Java so that users can implement it
without including the Scala library?

Grant, you originally suggested that some things would have to be in the
Java side as well. Can you please elaborate on this?

Ismael

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Jay Kreps]

Given that we're breaking compatibility anyway should we:
1. Remove the get prefix on this method and the existing one which violate
our own code style guidelines (Oops! Kind of sad we went through the whole
KIP process and no one even flagged this)
2. Move the interface out of scala to be a normal Java interface

This breaks source compatibility but probably what we should have done
originally I suspect. Probably there are few enough implementations of this
that it is better to just rip the bandaid off.

-Jay

On Sat, Apr 2, 2016 at 11:18 AM, Ashish Singh  wrote:

> Hello Ismael,
>
> On Fri, Apr 1, 2016 at 4:08 PM, Ismael Juma  wrote:
>
> > Hi Ashish,
> >
> > A few comments on the proposal:
> >
> > 1. In Kafka, we don't use use getter convention generally. However, the
> > other methods in the `Authorizer` interface do follow the getter
> > convention, which is unfortunate. So, I am OK with the name you suggested
> > (getSupportedPrincipalTypes instead of supportedPrincipalTypes), but I
> > wanted to mention this in case others have a different opinion.
> >
> If multiple people feel the same, I am happy to rename the method.
>
> >
> > 2. The proposed change to the Authorizer trait (adding a method with a
> > default implementation) is source compatible, but _not_ binary
> compatible.
> > So, it won't be possible for someone to implement the Authorizer and
> > compile it once so that it works with both Kafka 0.9.0.x and Kafka
> > 0.10.0.x. Not sure how much of an issue this is, but it's worth
> mentioning
> > it in the KIP.
> >
> Right, will mention that. It is binary incompatible only when someone uses
> >= 0.10 AdminCommand with an Authorizer implementation based on 0.9.
>
> >
> > 3. If an Authorizer wanted to support any type of principal without
> > specifying them, is there a way to do that? Is it something that we want
> to
> > support? Before the KIP was proposed, there was a discussion in the PR
> > about different principal types potentially being used by the
> > authentication layer where the Authorizer is agnostic.
> >
> Authorizer is agnostic of principal types right now. As we have already
> seen that this opens up space for user errors. We want to keep kafka-acls
> be generic so that various authorizer implementation can use the same cli.
> Each implementation has freedom of supporting their own principal types.
> How do we expect users to know what principal types are supported by a
> particular implementation? Sure we can document it, we have it documented
> for SimpleAclAuthorizer, still users ran into issue. Worst thing is that
> this error happens silently, invalid acls are persisted by authorizer, as
> authorizers themselves are not aware of principal types it supports. This
> is what the KIP is trying to solve.
>
>
> > 4. There is a PR for introducing a "group" principal type (
> > https://github.com/apache/kafka/pull/483/files), would that have any
> > impact
> > on this proposal?
> >
> Override of getSupportedPrincipalTypes in  SimpleAclAuthorizer will have to
> return group as well.
>
> >
> > Ismael
> >
> > On Mon, Mar 28, 2016 at 9:28 PM, Ashish Singh 
> wrote:
> >
> > > Hello Harsha,
> > >
> > > Pinging again. This is a minor KIP and it has been lying around for
> quite
> > > some time. If providing supported principal types via a config is what
> > you
> > > suggest, I am fine with it.
> > >
> > > On Wed, Mar 9, 2016 at 12:32 PM, Ashish Singh 
> > wrote:
> > >
> > > > Hi Harsha,
> > > >
> > > > On Wed, Mar 9, 2016 at 12:04 PM, Harsha  wrote:
> > > >
> > > >> Why we need to add this additional method just for validation. This
> > will
> > > >> invalidate all the existing authorizer implementations.
> > > >
> > > > As PrincipalTypes is implementation specific, wouldn't it be required
> > for
> > > > users to know what principal types they can use in add/removeAcls?
> > > >
> > > > All the existing authorizer implementations will continue to work, as
> > the
> > > > method by default will return List(User), as User is the only
> principal
> > > > type that is supported out of the box as of now. Let me know if I
> > missed
> > > > your question here.
> > > >
> > > >
> > > >> Why can't we add
> > > >> the logic for validation and pass it as authorizer config.
> > > >>
> > > > Do you mean passing PrincipalTypes as authorizer config? If I am
> > getting
> > > > your question correctly, then we are asking users to be aware of what
> > > > PrincipalTypes an authorizer supports. That defeats the purpose of
> > > > validation, right?
> > > >
> > > >>
> > > >> -Harsha
> > > >>
> > > >> On Mon, Mar 7, 2016, at 04:33 PM, Ashish Singh wrote:
> > > >> > + Parth, Harsha
> > > >> >
> > > >> > On Mon, Mar 7, 2016 at 4:32 PM, Ashish Singh  >
> > > >> wrote:
> > > >> >
> > > >> > > Thanks Gwen.
> > > >> > >
> > > >> > > @Parth, @Harsha pinging you guys for your feedback. Based on

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

Hello Ismael,

On Fri, Apr 1, 2016 at 4:08 PM, Ismael Juma  wrote:

> Hi Ashish,
>
> A few comments on the proposal:
>
> 1. In Kafka, we don't use use getter convention generally. However, the
> other methods in the `Authorizer` interface do follow the getter
> convention, which is unfortunate. So, I am OK with the name you suggested
> (getSupportedPrincipalTypes instead of supportedPrincipalTypes), but I
> wanted to mention this in case others have a different opinion.
>
If multiple people feel the same, I am happy to rename the method.

>
> 2. The proposed change to the Authorizer trait (adding a method with a
> default implementation) is source compatible, but _not_ binary compatible.
> So, it won't be possible for someone to implement the Authorizer and
> compile it once so that it works with both Kafka 0.9.0.x and Kafka
> 0.10.0.x. Not sure how much of an issue this is, but it's worth mentioning
> it in the KIP.
>
Right, will mention that. It is binary incompatible only when someone uses
>= 0.10 AdminCommand with an Authorizer implementation based on 0.9.

>
> 3. If an Authorizer wanted to support any type of principal without
> specifying them, is there a way to do that? Is it something that we want to
> support? Before the KIP was proposed, there was a discussion in the PR
> about different principal types potentially being used by the
> authentication layer where the Authorizer is agnostic.
>
Authorizer is agnostic of principal types right now. As we have already
seen that this opens up space for user errors. We want to keep kafka-acls
be generic so that various authorizer implementation can use the same cli.
Each implementation has freedom of supporting their own principal types.
How do we expect users to know what principal types are supported by a
particular implementation? Sure we can document it, we have it documented
for SimpleAclAuthorizer, still users ran into issue. Worst thing is that
this error happens silently, invalid acls are persisted by authorizer, as
authorizers themselves are not aware of principal types it supports. This
is what the KIP is trying to solve.


> 4. There is a PR for introducing a "group" principal type (
> https://github.com/apache/kafka/pull/483/files), would that have any
> impact
> on this proposal?
>
Override of getSupportedPrincipalTypes in  SimpleAclAuthorizer will have to
return group as well.

>
> Ismael
>
> On Mon, Mar 28, 2016 at 9:28 PM, Ashish Singh  wrote:
>
> > Hello Harsha,
> >
> > Pinging again. This is a minor KIP and it has been lying around for quite
> > some time. If providing supported principal types via a config is what
> you
> > suggest, I am fine with it.
> >
> > On Wed, Mar 9, 2016 at 12:32 PM, Ashish Singh 
> wrote:
> >
> > > Hi Harsha,
> > >
> > > On Wed, Mar 9, 2016 at 12:04 PM, Harsha  wrote:
> > >
> > >> Why we need to add this additional method just for validation. This
> will
> > >> invalidate all the existing authorizer implementations.
> > >
> > > As PrincipalTypes is implementation specific, wouldn't it be required
> for
> > > users to know what principal types they can use in add/removeAcls?
> > >
> > > All the existing authorizer implementations will continue to work, as
> the
> > > method by default will return List(User), as User is the only principal
> > > type that is supported out of the box as of now. Let me know if I
> missed
> > > your question here.
> > >
> > >
> > >> Why can't we add
> > >> the logic for validation and pass it as authorizer config.
> > >>
> > > Do you mean passing PrincipalTypes as authorizer config? If I am
> getting
> > > your question correctly, then we are asking users to be aware of what
> > > PrincipalTypes an authorizer supports. That defeats the purpose of
> > > validation, right?
> > >
> > >>
> > >> -Harsha
> > >>
> > >> On Mon, Mar 7, 2016, at 04:33 PM, Ashish Singh wrote:
> > >> > + Parth, Harsha
> > >> >
> > >> > On Mon, Mar 7, 2016 at 4:32 PM, Ashish Singh 
> > >> wrote:
> > >> >
> > >> > > Thanks Gwen.
> > >> > >
> > >> > > @Parth, @Harsha pinging you guys for your feedback. Based on
> > >> discussion on
> > >> > > JIRA, we have following open questions.
> > >> > >
> > >> > >1.
> > >> > >
> > >> > >How to allow an authorizer implementation to specify supported
> > >> > >principal types?
> > >> > >
> > >> > >An alternative of providing supported Principal types via
> > >> interface is
> > >> > >via a config option. Having a config option will be helpful for
> > >> certain
> > >> > >third party implementations that uses SimpleAclAuthorizer but
> > >> support more
> > >> > >PrincipalTypes. However, it requires adds one more config.
> > >> > >
> > >> > >2.
> > >> > >
> > >> > >ACLs validation should be done by client or by authorizer?
> > >> > >
> > >> > >Once this method is added we expect the Client of the
> authorizer
> > >> to do
> > >> > >the 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ismael Juma]

Hi Ashish,

A few comments on the proposal:

1. In Kafka, we don't use use getter convention generally. However, the
other methods in the `Authorizer` interface do follow the getter
convention, which is unfortunate. So, I am OK with the name you suggested
(getSupportedPrincipalTypes instead of supportedPrincipalTypes), but I
wanted to mention this in case others have a different opinion.

2. The proposed change to the Authorizer trait (adding a method with a
default implementation) is source compatible, but _not_ binary compatible.
So, it won't be possible for someone to implement the Authorizer and
compile it once so that it works with both Kafka 0.9.0.x and Kafka
0.10.0.x. Not sure how much of an issue this is, but it's worth mentioning
it in the KIP.

3. If an Authorizer wanted to support any type of principal without
specifying them, is there a way to do that? Is it something that we want to
support? Before the KIP was proposed, there was a discussion in the PR
about different principal types potentially being used by the
authentication layer where the Authorizer is agnostic.

4. There is a PR for introducing a "group" principal type (
https://github.com/apache/kafka/pull/483/files), would that have any impact
on this proposal?

Ismael

On Mon, Mar 28, 2016 at 9:28 PM, Ashish Singh  wrote:

> Hello Harsha,
>
> Pinging again. This is a minor KIP and it has been lying around for quite
> some time. If providing supported principal types via a config is what you
> suggest, I am fine with it.
>
> On Wed, Mar 9, 2016 at 12:32 PM, Ashish Singh  wrote:
>
> > Hi Harsha,
> >
> > On Wed, Mar 9, 2016 at 12:04 PM, Harsha  wrote:
> >
> >> Why we need to add this additional method just for validation. This will
> >> invalidate all the existing authorizer implementations.
> >
> > As PrincipalTypes is implementation specific, wouldn't it be required for
> > users to know what principal types they can use in add/removeAcls?
> >
> > All the existing authorizer implementations will continue to work, as the
> > method by default will return List(User), as User is the only principal
> > type that is supported out of the box as of now. Let me know if I missed
> > your question here.
> >
> >
> >> Why can't we add
> >> the logic for validation and pass it as authorizer config.
> >>
> > Do you mean passing PrincipalTypes as authorizer config? If I am getting
> > your question correctly, then we are asking users to be aware of what
> > PrincipalTypes an authorizer supports. That defeats the purpose of
> > validation, right?
> >
> >>
> >> -Harsha
> >>
> >> On Mon, Mar 7, 2016, at 04:33 PM, Ashish Singh wrote:
> >> > + Parth, Harsha
> >> >
> >> > On Mon, Mar 7, 2016 at 4:32 PM, Ashish Singh 
> >> wrote:
> >> >
> >> > > Thanks Gwen.
> >> > >
> >> > > @Parth, @Harsha pinging you guys for your feedback. Based on
> >> discussion on
> >> > > JIRA, we have following open questions.
> >> > >
> >> > >1.
> >> > >
> >> > >How to allow an authorizer implementation to specify supported
> >> > >principal types?
> >> > >
> >> > >An alternative of providing supported Principal types via
> >> interface is
> >> > >via a config option. Having a config option will be helpful for
> >> certain
> >> > >third party implementations that uses SimpleAclAuthorizer but
> >> support more
> >> > >PrincipalTypes. However, it requires adds one more config.
> >> > >
> >> > >2.
> >> > >
> >> > >ACLs validation should be done by client or by authorizer?
> >> > >
> >> > >Once this method is added we expect the Client of the authorizer
> >> to do
> >> > >the validation on principal types and the authorizer will still
> >> not do any
> >> > >validation by it self. As an alternative we can add the
> validation
> >> at
> >> > >Authorizer level. Having validation done at client side enables
> >> clients to
> >> > >fail fast for invalid principal types, whereas implementing it at
> >> > >authorization level removes the requirement of having the
> >> validation done
> >> > >on each client implementation.
> >> > >
> >> > >
> >> > > On Mon, Mar 7, 2016 at 3:47 PM, Gwen Shapira 
> >> wrote:
> >> > >
> >> > > Ashish,
> >> > >>
> >> > >> I'm neutral on this (+0), but would be good to have feedback from
> >> > >> Harsha and Parth. If you can get their "sounds good", we can
> probably
> >> > >> get it through fairly soon and in time for 0.10.0.
> >> > >>
> >> > >> Gwen
> >> > >>
> >> > >> On Wed, Mar 2, 2016 at 9:47 AM, Ashish Singh 
> >> wrote:
> >> > >> > Here is link to the KIP,
> >> > >> >
> >> > >>
> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-50+-+Enhance+Authorizer+interface+to+be+aware+of+supported+Principal+Types
> >> > >> >
> >> > >> > On Wed, Mar 2, 2016 at 9:46 AM, Ashish Singh <
> asi...@cloudera.com>
> >> > >> wrote:
> >> > >> >
> >> > >> >> Hi Guys,
> >> > >> >>
> >> > 

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

I am going to initiate a vote thread for this.

On Mon, Mar 28, 2016 at 1:28 PM, Ashish Singh  wrote:

> Hello Harsha,
>
> Pinging again. This is a minor KIP and it has been lying around for quite
> some time. If providing supported principal types via a config is what you
> suggest, I am fine with it.
>
> On Wed, Mar 9, 2016 at 12:32 PM, Ashish Singh  wrote:
>
>> Hi Harsha,
>>
>> On Wed, Mar 9, 2016 at 12:04 PM, Harsha  wrote:
>>
>>> Why we need to add this additional method just for validation. This will
>>> invalidate all the existing authorizer implementations.
>>
>> As PrincipalTypes is implementation specific, wouldn't it be required for
>> users to know what principal types they can use in add/removeAcls?
>>
>> All the existing authorizer implementations will continue to work, as the
>> method by default will return List(User), as User is the only principal
>> type that is supported out of the box as of now. Let me know if I missed
>> your question here.
>>
>>
>>> Why can't we add
>>> the logic for validation and pass it as authorizer config.
>>>
>> Do you mean passing PrincipalTypes as authorizer config? If I am getting
>> your question correctly, then we are asking users to be aware of what
>> PrincipalTypes an authorizer supports. That defeats the purpose of
>> validation, right?
>>
>>>
>>> -Harsha
>>>
>>> On Mon, Mar 7, 2016, at 04:33 PM, Ashish Singh wrote:
>>> > + Parth, Harsha
>>> >
>>> > On Mon, Mar 7, 2016 at 4:32 PM, Ashish Singh 
>>> wrote:
>>> >
>>> > > Thanks Gwen.
>>> > >
>>> > > @Parth, @Harsha pinging you guys for your feedback. Based on
>>> discussion on
>>> > > JIRA, we have following open questions.
>>> > >
>>> > >1.
>>> > >
>>> > >How to allow an authorizer implementation to specify supported
>>> > >principal types?
>>> > >
>>> > >An alternative of providing supported Principal types via
>>> interface is
>>> > >via a config option. Having a config option will be helpful for
>>> certain
>>> > >third party implementations that uses SimpleAclAuthorizer but
>>> support more
>>> > >PrincipalTypes. However, it requires adds one more config.
>>> > >
>>> > >2.
>>> > >
>>> > >ACLs validation should be done by client or by authorizer?
>>> > >
>>> > >Once this method is added we expect the Client of the authorizer
>>> to do
>>> > >the validation on principal types and the authorizer will still
>>> not do any
>>> > >validation by it self. As an alternative we can add the
>>> validation at
>>> > >Authorizer level. Having validation done at client side enables
>>> clients to
>>> > >fail fast for invalid principal types, whereas implementing it at
>>> > >authorization level removes the requirement of having the
>>> validation done
>>> > >on each client implementation.
>>> > >
>>> > >
>>> > > On Mon, Mar 7, 2016 at 3:47 PM, Gwen Shapira 
>>> wrote:
>>> > >
>>> > > Ashish,
>>> > >>
>>> > >> I'm neutral on this (+0), but would be good to have feedback from
>>> > >> Harsha and Parth. If you can get their "sounds good", we can
>>> probably
>>> > >> get it through fairly soon and in time for 0.10.0.
>>> > >>
>>> > >> Gwen
>>> > >>
>>> > >> On Wed, Mar 2, 2016 at 9:47 AM, Ashish Singh 
>>> wrote:
>>> > >> > Here is link to the KIP,
>>> > >> >
>>> > >>
>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-50+-+Enhance+Authorizer+interface+to+be+aware+of+supported+Principal+Types
>>> > >> >
>>> > >> > On Wed, Mar 2, 2016 at 9:46 AM, Ashish Singh >> >
>>> > >> wrote:
>>> > >> >
>>> > >> >> Hi Guys,
>>> > >> >>
>>> > >> >> I would like to initiate a discuss thread on KIP-50. Kafka
>>> authorizer
>>> > >> is
>>> > >> >> agnostic of principal types it supports, so are the acls CRUD
>>> methods
>>> > >> >> in kafka.security.auth.Authorizer. The intent behind is to keep
>>> Kafka
>>> > >> >> authorization pluggable, which is really great. However, this
>>> leads to
>>> > >> Acls
>>> > >> >> CRUD methods not performing any check on validity of acls, as
>>> they are
>>> > >> not
>>> > >> >> aware of what principal types Authorizer implementation
>>> supports. This
>>> > >> >> opens up space for lots of user errors, KAFKA-3097
>>> > >> >>  for an
>>> instance.
>>> > >> >>
>>> > >> >> This KIP proposes adding a getSupportedPrincipalTypes method to
>>> > >> authorizer
>>> > >> >> and use that for acls verification during acls CRUD.
>>> > >> >>
>>> > >> >> Feedbacks and comments are welcome.
>>> > >> >>
>>> > >> >> --
>>> > >> >>
>>> > >> >> Regards,
>>> > >> >> Ashish
>>> > >> >>
>>> > >> >
>>> > >> >
>>> > >> >
>>> > >> > --
>>> > >> >
>>> > >> > Regards,
>>> > >> > Ashish
>>> > >>
>>> > > ​
>>> > > --
>>> > >
>>> > > Regards,
>>> > > Ashish
>>> > >
>>> >
>>> >
>>> >
>>> > --
>>> >
>>> > Regards,
>>> > Ashish
>>>
>>
>>
>>
>> --
>>
>> Regards,
>> Ashish
>>
>

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

Hello Harsha,

Pinging again. This is a minor KIP and it has been lying around for quite
some time. If providing supported principal types via a config is what you
suggest, I am fine with it.

On Wed, Mar 9, 2016 at 12:32 PM, Ashish Singh  wrote:

> Hi Harsha,
>
> On Wed, Mar 9, 2016 at 12:04 PM, Harsha  wrote:
>
>> Why we need to add this additional method just for validation. This will
>> invalidate all the existing authorizer implementations.
>
> As PrincipalTypes is implementation specific, wouldn't it be required for
> users to know what principal types they can use in add/removeAcls?
>
> All the existing authorizer implementations will continue to work, as the
> method by default will return List(User), as User is the only principal
> type that is supported out of the box as of now. Let me know if I missed
> your question here.
>
>
>> Why can't we add
>> the logic for validation and pass it as authorizer config.
>>
> Do you mean passing PrincipalTypes as authorizer config? If I am getting
> your question correctly, then we are asking users to be aware of what
> PrincipalTypes an authorizer supports. That defeats the purpose of
> validation, right?
>
>>
>> -Harsha
>>
>> On Mon, Mar 7, 2016, at 04:33 PM, Ashish Singh wrote:
>> > + Parth, Harsha
>> >
>> > On Mon, Mar 7, 2016 at 4:32 PM, Ashish Singh 
>> wrote:
>> >
>> > > Thanks Gwen.
>> > >
>> > > @Parth, @Harsha pinging you guys for your feedback. Based on
>> discussion on
>> > > JIRA, we have following open questions.
>> > >
>> > >1.
>> > >
>> > >How to allow an authorizer implementation to specify supported
>> > >principal types?
>> > >
>> > >An alternative of providing supported Principal types via
>> interface is
>> > >via a config option. Having a config option will be helpful for
>> certain
>> > >third party implementations that uses SimpleAclAuthorizer but
>> support more
>> > >PrincipalTypes. However, it requires adds one more config.
>> > >
>> > >2.
>> > >
>> > >ACLs validation should be done by client or by authorizer?
>> > >
>> > >Once this method is added we expect the Client of the authorizer
>> to do
>> > >the validation on principal types and the authorizer will still
>> not do any
>> > >validation by it self. As an alternative we can add the validation
>> at
>> > >Authorizer level. Having validation done at client side enables
>> clients to
>> > >fail fast for invalid principal types, whereas implementing it at
>> > >authorization level removes the requirement of having the
>> validation done
>> > >on each client implementation.
>> > >
>> > >
>> > > On Mon, Mar 7, 2016 at 3:47 PM, Gwen Shapira 
>> wrote:
>> > >
>> > > Ashish,
>> > >>
>> > >> I'm neutral on this (+0), but would be good to have feedback from
>> > >> Harsha and Parth. If you can get their "sounds good", we can probably
>> > >> get it through fairly soon and in time for 0.10.0.
>> > >>
>> > >> Gwen
>> > >>
>> > >> On Wed, Mar 2, 2016 at 9:47 AM, Ashish Singh 
>> wrote:
>> > >> > Here is link to the KIP,
>> > >> >
>> > >>
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-50+-+Enhance+Authorizer+interface+to+be+aware+of+supported+Principal+Types
>> > >> >
>> > >> > On Wed, Mar 2, 2016 at 9:46 AM, Ashish Singh 
>> > >> wrote:
>> > >> >
>> > >> >> Hi Guys,
>> > >> >>
>> > >> >> I would like to initiate a discuss thread on KIP-50. Kafka
>> authorizer
>> > >> is
>> > >> >> agnostic of principal types it supports, so are the acls CRUD
>> methods
>> > >> >> in kafka.security.auth.Authorizer. The intent behind is to keep
>> Kafka
>> > >> >> authorization pluggable, which is really great. However, this
>> leads to
>> > >> Acls
>> > >> >> CRUD methods not performing any check on validity of acls, as
>> they are
>> > >> not
>> > >> >> aware of what principal types Authorizer implementation supports.
>> This
>> > >> >> opens up space for lots of user errors, KAFKA-3097
>> > >> >>  for an
>> instance.
>> > >> >>
>> > >> >> This KIP proposes adding a getSupportedPrincipalTypes method to
>> > >> authorizer
>> > >> >> and use that for acls verification during acls CRUD.
>> > >> >>
>> > >> >> Feedbacks and comments are welcome.
>> > >> >>
>> > >> >> --
>> > >> >>
>> > >> >> Regards,
>> > >> >> Ashish
>> > >> >>
>> > >> >
>> > >> >
>> > >> >
>> > >> > --
>> > >> >
>> > >> > Regards,
>> > >> > Ashish
>> > >>
>> > > ​
>> > > --
>> > >
>> > > Regards,
>> > > Ashish
>> > >
>> >
>> >
>> >
>> > --
>> >
>> > Regards,
>> > Ashish
>>
>
>
>
> --
>
> Regards,
> Ashish
>



-- 

Regards,
Ashish

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

Hi Harsha,

On Wed, Mar 9, 2016 at 12:04 PM, Harsha  wrote:

> Why we need to add this additional method just for validation. This will
> invalidate all the existing authorizer implementations.

As PrincipalTypes is implementation specific, wouldn't it be required for
users to know what principal types they can use in add/removeAcls?

All the existing authorizer implementations will continue to work, as the
method by default will return List(User), as User is the only principal
type that is supported out of the box as of now. Let me know if I missed
your question here.


> Why can't we add
> the logic for validation and pass it as authorizer config.
>
Do you mean passing PrincipalTypes as authorizer config? If I am getting
your question correctly, then we are asking users to be aware of what
PrincipalTypes an authorizer supports. That defeats the purpose of
validation, right?

>
> -Harsha
>
> On Mon, Mar 7, 2016, at 04:33 PM, Ashish Singh wrote:
> > + Parth, Harsha
> >
> > On Mon, Mar 7, 2016 at 4:32 PM, Ashish Singh 
> wrote:
> >
> > > Thanks Gwen.
> > >
> > > @Parth, @Harsha pinging you guys for your feedback. Based on
> discussion on
> > > JIRA, we have following open questions.
> > >
> > >1.
> > >
> > >How to allow an authorizer implementation to specify supported
> > >principal types?
> > >
> > >An alternative of providing supported Principal types via interface
> is
> > >via a config option. Having a config option will be helpful for
> certain
> > >third party implementations that uses SimpleAclAuthorizer but
> support more
> > >PrincipalTypes. However, it requires adds one more config.
> > >
> > >2.
> > >
> > >ACLs validation should be done by client or by authorizer?
> > >
> > >Once this method is added we expect the Client of the authorizer to
> do
> > >the validation on principal types and the authorizer will still not
> do any
> > >validation by it self. As an alternative we can add the validation
> at
> > >Authorizer level. Having validation done at client side enables
> clients to
> > >fail fast for invalid principal types, whereas implementing it at
> > >authorization level removes the requirement of having the
> validation done
> > >on each client implementation.
> > >
> > >
> > > On Mon, Mar 7, 2016 at 3:47 PM, Gwen Shapira 
> wrote:
> > >
> > > Ashish,
> > >>
> > >> I'm neutral on this (+0), but would be good to have feedback from
> > >> Harsha and Parth. If you can get their "sounds good", we can probably
> > >> get it through fairly soon and in time for 0.10.0.
> > >>
> > >> Gwen
> > >>
> > >> On Wed, Mar 2, 2016 at 9:47 AM, Ashish Singh 
> wrote:
> > >> > Here is link to the KIP,
> > >> >
> > >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-50+-+Enhance+Authorizer+interface+to+be+aware+of+supported+Principal+Types
> > >> >
> > >> > On Wed, Mar 2, 2016 at 9:46 AM, Ashish Singh 
> > >> wrote:
> > >> >
> > >> >> Hi Guys,
> > >> >>
> > >> >> I would like to initiate a discuss thread on KIP-50. Kafka
> authorizer
> > >> is
> > >> >> agnostic of principal types it supports, so are the acls CRUD
> methods
> > >> >> in kafka.security.auth.Authorizer. The intent behind is to keep
> Kafka
> > >> >> authorization pluggable, which is really great. However, this
> leads to
> > >> Acls
> > >> >> CRUD methods not performing any check on validity of acls, as they
> are
> > >> not
> > >> >> aware of what principal types Authorizer implementation supports.
> This
> > >> >> opens up space for lots of user errors, KAFKA-3097
> > >> >>  for an
> instance.
> > >> >>
> > >> >> This KIP proposes adding a getSupportedPrincipalTypes method to
> > >> authorizer
> > >> >> and use that for acls verification during acls CRUD.
> > >> >>
> > >> >> Feedbacks and comments are welcome.
> > >> >>
> > >> >> --
> > >> >>
> > >> >> Regards,
> > >> >> Ashish
> > >> >>
> > >> >
> > >> >
> > >> >
> > >> > --
> > >> >
> > >> > Regards,
> > >> > Ashish
> > >>
> > > ​
> > > --
> > >
> > > Regards,
> > > Ashish
> > >
> >
> >
> >
> > --
> >
> > Regards,
> > Ashish
>



-- 

Regards,
Ashish

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Harsha]

Why we need to add this additional method just for validation. This will
invalidate all the existing authorizer implementations. Why can't we add
the logic for validation and pass it as authorizer config.

-Harsha 

On Mon, Mar 7, 2016, at 04:33 PM, Ashish Singh wrote:
> + Parth, Harsha
> 
> On Mon, Mar 7, 2016 at 4:32 PM, Ashish Singh  wrote:
> 
> > Thanks Gwen.
> >
> > @Parth, @Harsha pinging you guys for your feedback. Based on discussion on
> > JIRA, we have following open questions.
> >
> >1.
> >
> >How to allow an authorizer implementation to specify supported
> >principal types?
> >
> >An alternative of providing supported Principal types via interface is
> >via a config option. Having a config option will be helpful for certain
> >third party implementations that uses SimpleAclAuthorizer but support 
> > more
> >PrincipalTypes. However, it requires adds one more config.
> >
> >2.
> >
> >ACLs validation should be done by client or by authorizer?
> >
> >Once this method is added we expect the Client of the authorizer to do
> >the validation on principal types and the authorizer will still not do 
> > any
> >validation by it self. As an alternative we can add the validation at
> >Authorizer level. Having validation done at client side enables clients 
> > to
> >fail fast for invalid principal types, whereas implementing it at
> >authorization level removes the requirement of having the validation done
> >on each client implementation.
> >
> >
> > On Mon, Mar 7, 2016 at 3:47 PM, Gwen Shapira  wrote:
> >
> > Ashish,
> >>
> >> I'm neutral on this (+0), but would be good to have feedback from
> >> Harsha and Parth. If you can get their "sounds good", we can probably
> >> get it through fairly soon and in time for 0.10.0.
> >>
> >> Gwen
> >>
> >> On Wed, Mar 2, 2016 at 9:47 AM, Ashish Singh  wrote:
> >> > Here is link to the KIP,
> >> >
> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP-50+-+Enhance+Authorizer+interface+to+be+aware+of+supported+Principal+Types
> >> >
> >> > On Wed, Mar 2, 2016 at 9:46 AM, Ashish Singh 
> >> wrote:
> >> >
> >> >> Hi Guys,
> >> >>
> >> >> I would like to initiate a discuss thread on KIP-50. Kafka authorizer
> >> is
> >> >> agnostic of principal types it supports, so are the acls CRUD methods
> >> >> in kafka.security.auth.Authorizer. The intent behind is to keep Kafka
> >> >> authorization pluggable, which is really great. However, this leads to
> >> Acls
> >> >> CRUD methods not performing any check on validity of acls, as they are
> >> not
> >> >> aware of what principal types Authorizer implementation supports. This
> >> >> opens up space for lots of user errors, KAFKA-3097
> >> >>  for an instance.
> >> >>
> >> >> This KIP proposes adding a getSupportedPrincipalTypes method to
> >> authorizer
> >> >> and use that for acls verification during acls CRUD.
> >> >>
> >> >> Feedbacks and comments are welcome.
> >> >>
> >> >> --
> >> >>
> >> >> Regards,
> >> >> Ashish
> >> >>
> >> >
> >> >
> >> >
> >> > --
> >> >
> >> > Regards,
> >> > Ashish
> >>
> > ​
> > --
> >
> > Regards,
> > Ashish
> >
> 
> 
> 
> -- 
> 
> Regards,
> Ashish

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

+ Parth, Harsha

On Mon, Mar 7, 2016 at 4:32 PM, Ashish Singh  wrote:

> Thanks Gwen.
>
> @Parth, @Harsha pinging you guys for your feedback. Based on discussion on
> JIRA, we have following open questions.
>
>1.
>
>How to allow an authorizer implementation to specify supported
>principal types?
>
>An alternative of providing supported Principal types via interface is
>via a config option. Having a config option will be helpful for certain
>third party implementations that uses SimpleAclAuthorizer but support more
>PrincipalTypes. However, it requires adds one more config.
>
>2.
>
>ACLs validation should be done by client or by authorizer?
>
>Once this method is added we expect the Client of the authorizer to do
>the validation on principal types and the authorizer will still not do any
>validation by it self. As an alternative we can add the validation at
>Authorizer level. Having validation done at client side enables clients to
>fail fast for invalid principal types, whereas implementing it at
>authorization level removes the requirement of having the validation done
>on each client implementation.
>
>
> On Mon, Mar 7, 2016 at 3:47 PM, Gwen Shapira  wrote:
>
> Ashish,
>>
>> I'm neutral on this (+0), but would be good to have feedback from
>> Harsha and Parth. If you can get their "sounds good", we can probably
>> get it through fairly soon and in time for 0.10.0.
>>
>> Gwen
>>
>> On Wed, Mar 2, 2016 at 9:47 AM, Ashish Singh  wrote:
>> > Here is link to the KIP,
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-50+-+Enhance+Authorizer+interface+to+be+aware+of+supported+Principal+Types
>> >
>> > On Wed, Mar 2, 2016 at 9:46 AM, Ashish Singh 
>> wrote:
>> >
>> >> Hi Guys,
>> >>
>> >> I would like to initiate a discuss thread on KIP-50. Kafka authorizer
>> is
>> >> agnostic of principal types it supports, so are the acls CRUD methods
>> >> in kafka.security.auth.Authorizer. The intent behind is to keep Kafka
>> >> authorization pluggable, which is really great. However, this leads to
>> Acls
>> >> CRUD methods not performing any check on validity of acls, as they are
>> not
>> >> aware of what principal types Authorizer implementation supports. This
>> >> opens up space for lots of user errors, KAFKA-3097
>> >>  for an instance.
>> >>
>> >> This KIP proposes adding a getSupportedPrincipalTypes method to
>> authorizer
>> >> and use that for acls verification during acls CRUD.
>> >>
>> >> Feedbacks and comments are welcome.
>> >>
>> >> --
>> >>
>> >> Regards,
>> >> Ashish
>> >>
>> >
>> >
>> >
>> > --
>> >
>> > Regards,
>> > Ashish
>>
> ​
> --
>
> Regards,
> Ashish
>



-- 

Regards,
Ashish

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

Thanks Gwen.

@Parth, @Harsha pinging you guys for your feedback. Based on discussion on
JIRA, we have following open questions.

   1.

   How to allow an authorizer implementation to specify supported principal
   types?

   An alternative of providing supported Principal types via interface is
   via a config option. Having a config option will be helpful for certain
   third party implementations that uses SimpleAclAuthorizer but support more
   PrincipalTypes. However, it requires adds one more config.

   2.

   ACLs validation should be done by client or by authorizer?

   Once this method is added we expect the Client of the authorizer to do
   the validation on principal types and the authorizer will still not do any
   validation by it self. As an alternative we can add the validation at
   Authorizer level. Having validation done at client side enables clients to
   fail fast for invalid principal types, whereas implementing it at
   authorization level removes the requirement of having the validation done
   on each client implementation.


On Mon, Mar 7, 2016 at 3:47 PM, Gwen Shapira  wrote:

Ashish,
>
> I'm neutral on this (+0), but would be good to have feedback from
> Harsha and Parth. If you can get their "sounds good", we can probably
> get it through fairly soon and in time for 0.10.0.
>
> Gwen
>
> On Wed, Mar 2, 2016 at 9:47 AM, Ashish Singh  wrote:
> > Here is link to the KIP,
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-50+-+Enhance+Authorizer+interface+to+be+aware+of+supported+Principal+Types
> >
> > On Wed, Mar 2, 2016 at 9:46 AM, Ashish Singh 
> wrote:
> >
> >> Hi Guys,
> >>
> >> I would like to initiate a discuss thread on KIP-50. Kafka authorizer is
> >> agnostic of principal types it supports, so are the acls CRUD methods
> >> in kafka.security.auth.Authorizer. The intent behind is to keep Kafka
> >> authorization pluggable, which is really great. However, this leads to
> Acls
> >> CRUD methods not performing any check on validity of acls, as they are
> not
> >> aware of what principal types Authorizer implementation supports. This
> >> opens up space for lots of user errors, KAFKA-3097
> >>  for an instance.
> >>
> >> This KIP proposes adding a getSupportedPrincipalTypes method to
> authorizer
> >> and use that for acls verification during acls CRUD.
> >>
> >> Feedbacks and comments are welcome.
> >>
> >> --
> >>
> >> Regards,
> >> Ashish
> >>
> >
> >
> >
> > --
> >
> > Regards,
> > Ashish
>
​
-- 

Regards,
Ashish

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Gwen Shapira]

Ashish,

I'm neutral on this (+0), but would be good to have feedback from
Harsha and Parth. If you can get their "sounds good", we can probably
get it through fairly soon and in time for 0.10.0.

Gwen

On Wed, Mar 2, 2016 at 9:47 AM, Ashish Singh  wrote:
> Here is link to the KIP,
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-50+-+Enhance+Authorizer+interface+to+be+aware+of+supported+Principal+Types
>
> On Wed, Mar 2, 2016 at 9:46 AM, Ashish Singh  wrote:
>
>> Hi Guys,
>>
>> I would like to initiate a discuss thread on KIP-50. Kafka authorizer is
>> agnostic of principal types it supports, so are the acls CRUD methods
>> in kafka.security.auth.Authorizer. The intent behind is to keep Kafka
>> authorization pluggable, which is really great. However, this leads to Acls
>> CRUD methods not performing any check on validity of acls, as they are not
>> aware of what principal types Authorizer implementation supports. This
>> opens up space for lots of user errors, KAFKA-3097
>>  for an instance.
>>
>> This KIP proposes adding a getSupportedPrincipalTypes method to authorizer
>> and use that for acls verification during acls CRUD.
>>
>> Feedbacks and comments are welcome.
>>
>> --
>>
>> Regards,
>> Ashish
>>
>
>
>
> --
>
> Regards,
> Ashish

Re: [DISCUSS] KIP-50 - Enhance Authorizer interface to be aware of supported Principal Types

[Ashish Singh]

Here is link to the KIP,
https://cwiki.apache.org/confluence/display/KAFKA/KIP-50+-+Enhance+Authorizer+interface+to+be+aware+of+supported+Principal+Types

On Wed, Mar 2, 2016 at 9:46 AM, Ashish Singh  wrote:

> Hi Guys,
>
> I would like to initiate a discuss thread on KIP-50. Kafka authorizer is
> agnostic of principal types it supports, so are the acls CRUD methods
> in kafka.security.auth.Authorizer. The intent behind is to keep Kafka
> authorization pluggable, which is really great. However, this leads to Acls
> CRUD methods not performing any check on validity of acls, as they are not
> aware of what principal types Authorizer implementation supports. This
> opens up space for lots of user errors, KAFKA-3097
>  for an instance.
>
> This KIP proposes adding a getSupportedPrincipalTypes method to authorizer
> and use that for acls verification during acls CRUD.
>
> Feedbacks and comments are welcome.
>
> --
>
> Regards,
> Ashish
>



-- 

Regards,
Ashish