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

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,

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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 > >

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

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

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

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.

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

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

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

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

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

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

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

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

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

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

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

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

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

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?

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

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

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

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

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

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

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

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

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

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

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

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

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

"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

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

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

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

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

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

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

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

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

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

> > 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 >

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

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,

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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 >

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

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

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

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

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

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

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

+ 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

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

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

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

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

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

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.