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

[Ismael Juma <is...@juma.me.uk>]

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 <as...@cloudera.com> 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 <gw...@confluent.io> 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 <is...@juma.me.uk> 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 <ja...@confluent.io> 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 <as...@cloudera.com>]

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 <is...@juma.me.uk> 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 <as...@cloudera.com>
> 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 <gw...@confluent.io> 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 <is...@juma.me.uk>
> 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 <ja...@confluent.io> 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
> >
>



-- 

Regards,
Ashish