Re: [VOTE] Add REST Server to Apache Kafka

[parth brahmbhatt]

+1.

On Tue, Oct 25, 2016 at 2:58 PM, Jay Kreps  wrote:

> -1
>
> I think the REST server for Kafka that already exists is quite good and
> getting contributions. Moving this into the core project doesn't solve a
> problem that I see.
>
> -Jay
>
> On Tue, Oct 25, 2016 at 2:16 PM, Harsha Chintalapani 
> wrote:
>
> > Hi All,
> >We are proposing to have a REST Server as part of  Apache
> Kafka
> > to provide producer/consumer/admin APIs. We Strongly believe having
> > REST server functionality with Apache Kafka will help a lot of users.
> > Here is the KIP that Mani Kumar wrote
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 80:+Kafka+Rest+Server.
> > There is a discussion thread in dev list that had differing opinions on
> > whether to include REST server in Apache Kafka or not. You can read more
> > about that in this thread
> > http://mail-archives.apache.org/mod_mbox/kafka-dev/201610.mbox/%3CCAMVt_
> > aymqeudm39znsxgktpdde46sowmqhsxop-+jmbcuv7...@mail.gmail.com%3E
> >
> >   This is a VOTE thread to check interest in the community for
> > adding REST Server implementation in Apache Kafka.
> >
> > Thanks,
> > Harsha
> >
>

[jira] [Commented] (KAFKA-2700) delete topic should remove the corresponding ACL and configs

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15493958#comment-15493958
 ] 

Parth Brahmbhatt commented on KAFKA-2700:
-

all yours.

> delete topic should remove the corresponding ACL and configs
> 
>
> Key: KAFKA-2700
> URL: https://issues.apache.org/jira/browse/KAFKA-2700
> Project: Kafka
>  Issue Type: Bug
>  Components: core
>Affects Versions: 0.9.0.0
>Reporter: Jun Rao
>    Assignee: Parth Brahmbhatt
>
> After a topic is successfully deleted, we should also remove any ACL, configs 
> and perhaps committed offsets associated with topic.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: [DISCUSS] KIP-48 Support for delegation tokens as an authentication mechanism

[parth brahmbhatt]

Hi,

I am suggesting that we will only allow the renewal by users that
authenticated using *non* delegation token mechanism. For example, If user
Alice authenticated using kerberos and requested delegation tokens, only
user Alice authenticated via non delegation token mechanism can renew.
Clients that have  access to delegation tokens can not issue renewal
request for renewing their own token and this is primarily important to
reduce the time window for which a compromised token will be valid.

To clarify, Yes any authenticated user can request delegation tokens but
even here I would recommend to avoid creating a chain where a client
authenticated via delegation token request for more delegation tokens.
Basically anyone can request delegation token, as long as they authenticate
via a non delegation token mechanism.

Aren't classes listed here
<https://cwiki.apache.org/confluence/display/KAFKA/KIP-48+Delegation+token+support+for+Kafka#KIP-48DelegationtokensupportforKafka-PublicInterfaces>
sufficient?

Thanks
Parth



On Tue, Jun 21, 2016 at 4:33 PM, Jun Rao <j...@confluent.io> wrote:

> Parth,
>
> Thanks for the reply. A couple of comments inline below.
>
> On Tue, Jun 21, 2016 at 10:36 AM, parth brahmbhatt <
> brahmbhatt.pa...@gmail.com> wrote:
>
> > 1. Who / how are tokens renewed? By original requester only? or using
> > Kerberos
> > auth only?
> > My recommendation is to do this only using Kerberos auth and only threw
> the
> > renewer specified during the acquisition request.
> >
> >
> Hmm, not sure that I follow this. Are you saying that any client
> authenticated with the delegation token can renew, i.e. there is no renewer
> needed?
>
> Also, just to be clear, any authenticated client (either through SASL or
> SSL) can request a delegation token for the authenticated user, right?
>
>
> > 2. Are tokens stored on each broker or in ZK?
> > My recommendation is still to store in ZK or not store them at all. The
> > whole controller based distribution is too much overhead with not much to
> > achieve.
> >
> > 3. How are tokens invalidated / expired?
> > Either by expiration time out or through an explicit request to
> invalidate.
> >
> > 4. Which encryption algorithm is used?
> > SCRAM
> >
> > 5. What is the impersonation proposal (it wasn't in the KIP but was
> > discussed
> > in this thread)?
> > There is no imperonation proposal. I tried and explained how its a
> > different problem and why its not really necessary to discuss that as
> part
> > of this KIP.  This KIP will not support any impersonation, it will just
> be
> > another way to authenticate.
> >
> > 6. Do we need new ACLs, if so - for what actions?
> > We do not need new ACLs.
> >
> >
> Could we document the format of the new request/response and their
> associated Resource and Operation for ACL?
>
>
> > 7. How would the delegation token be configured in the client?
> > Should be through config. I wasn't planning on supporting JAAS for
> tokens.
> > I don't believe hadoop does this either.
> >
> > Thanks
> > Parth
> >
> >
> >
> > On Thu, Jun 16, 2016 at 4:03 PM, Jun Rao <j...@confluent.io> wrote:
> >
> > > Harsha,
> > >
> > > Another question.
> > >
> > > 9. How would the delegation token be configured in the client? The
> > standard
> > > way is to do this through JAAS. However, we will need to think through
> if
> > > this is convenient in a shared environment. For example, when a new
> task
> > is
> > > added to a Storm worker node, do we need to dynamically add a new
> section
> > > in the JAAS file? It may be more convenient if we can pass in the token
> > > through the config directly w/o going through JAAS.
> > >
> > > Are you or Parth still actively working on this KIP?
> > >
> > > Thanks,
> > >
> > > Jun
> > >
> > >
> > >
> > > On Sun, Jun 12, 2016 at 2:18 PM, Jun Rao <j...@confluent.io> wrote:
> > >
> > > > Just to add on that list.
> > > >
> > > > 2. It would be good to document the format of the data stored in ZK.
> > > > 7. Earlier, there was a discussion on whether the tokens should be
> > > > propagated through ZK like config/acl/quota, or through the
> controller.
> > > > Currently, the controller is only designed for propagating topic
> > > metadata,
> > > > but not other data.
> > > > 8. Should we use SCRAM to send the token instead of DIGEST-MD5 since
> &

Re: [DISCUSS] KIP-48 Support for delegation tokens as an authentication mechanism

[parth brahmbhatt]

1. Who / how are tokens renewed? By original requester only? or using Kerberos
auth only?
My recommendation is to do this only using Kerberos auth and only threw the
renewer specified during the acquisition request.

2. Are tokens stored on each broker or in ZK?
My recommendation is still to store in ZK or not store them at all. The
whole controller based distribution is too much overhead with not much to
achieve.

3. How are tokens invalidated / expired?
Either by expiration time out or through an explicit request to invalidate.

4. Which encryption algorithm is used?
SCRAM

5. What is the impersonation proposal (it wasn't in the KIP but was discussed
in this thread)?
There is no imperonation proposal. I tried and explained how its a
different problem and why its not really necessary to discuss that as part
of this KIP.  This KIP will not support any impersonation, it will just be
another way to authenticate.

6. Do we need new ACLs, if so - for what actions?
We do not need new ACLs.

7. How would the delegation token be configured in the client?
Should be through config. I wasn't planning on supporting JAAS for tokens.
I don't believe hadoop does this either.

Thanks
Parth



On Thu, Jun 16, 2016 at 4:03 PM, Jun Rao <j...@confluent.io> wrote:

> Harsha,
>
> Another question.
>
> 9. How would the delegation token be configured in the client? The standard
> way is to do this through JAAS. However, we will need to think through if
> this is convenient in a shared environment. For example, when a new task is
> added to a Storm worker node, do we need to dynamically add a new section
> in the JAAS file? It may be more convenient if we can pass in the token
> through the config directly w/o going through JAAS.
>
> Are you or Parth still actively working on this KIP?
>
> Thanks,
>
> Jun
>
>
>
> On Sun, Jun 12, 2016 at 2:18 PM, Jun Rao <j...@confluent.io> wrote:
>
> > Just to add on that list.
> >
> > 2. It would be good to document the format of the data stored in ZK.
> > 7. Earlier, there was a discussion on whether the tokens should be
> > propagated through ZK like config/acl/quota, or through the controller.
> > Currently, the controller is only designed for propagating topic
> metadata,
> > but not other data.
> > 8. Should we use SCRAM to send the token instead of DIGEST-MD5 since it's
> > deprecated?
> >
> > Also, the images in the wiki seem broken.
> >
> > Thanks,
> >
> > Jun
> >
> > On Fri, Jun 10, 2016 at 10:02 AM, Gwen Shapira <g...@confluent.io>
> wrote:
> >
> >> From what I can see, remaining questions are:
> >>
> >> 1. Who / how are tokens renewed? By original requester only? or using
> >> Kerberos auth only?
> >> 2. Are tokens stored on each broker or in ZK?
> >> 3. How are tokens invalidated / expired?
> >> 4. Which encryption algorithm is used?
> >> 5. What is the impersonation proposal (it wasn't in the KIP but was
> >> discussed in this thread)?
> >> 6. Do we need new ACLs, if so - for what actions?
> >>
> >> Gwen
> >>
> >> On Thu, Jun 9, 2016 at 7:48 PM, Harsha <ka...@harsha.io> wrote:
> >> > Jun & Ismael,
> >> >  Unfortunately I couldn't attend the KIP
> meeting
> >> >  when delegation tokens discussed. Appreciate
> if
> >> >  you can update the thread if you have any
> >> >  further questions.
> >> > Thanks,
> >> > Harsha
> >> >
> >> > On Tue, May 24, 2016, at 11:32 AM, Liquan Pei wrote:
> >> >> It seems that the links to images in the KIP are broken.
> >> >>
> >> >> Liquan
> >> >>
> >> >> On Tue, May 24, 2016 at 9:33 AM, parth brahmbhatt <
> >> >> brahmbhatt.pa...@gmail.com> wrote:
> >> >>
> >> >> > 110. What does getDelegationTokenAs mean?
> >> >> > In the current proposal we only allow a user to get delegation
> token
> >> for
> >> >> > the identity that it authenticated as using another mechanism, i.e.
> >> A user
> >> >> > that authenticate using a keytab for principal us...@example.com
> >> will get
> >> >> > delegation tokens for that user only. In future I think we will
> have
> >> to
> >> >> > extend support such that we allow some set of users (
> >> >> > kafka-rest-u...@example.com, storm-nim...@example.com) to acquire
> >> >> > delegation tokens on

Re: [DISCUSS] KIP-48 Support for delegation tokens as an authentication mechanism

[parth brahmbhatt]

110. What does getDelegationTokenAs mean?
In the current proposal we only allow a user to get delegation token for
the identity that it authenticated as using another mechanism, i.e. A user
that authenticate using a keytab for principal us...@example.com will get
delegation tokens for that user only. In future I think we will have to
extend support such that we allow some set of users (
kafka-rest-u...@example.com, storm-nim...@example.com) to acquire
delegation tokens on behalf of other users whose identity they have
verified independently.  Kafka brokers will have ACLs to control which
users are allowed to impersonate other users and get tokens on behalf of
them. Overall Impersonation is a whole different problem in my opinion and
I think we can tackle it in separate KIP.

111. What's the typical rate of getting and renewing delegation tokens?
Typically this should be very very low, 1 request per minute is a
relatively high estimate. However it depends on the token expiration. I am
less worried about the extra load it puts on controller vs the added
complexity and the value it offers.

Thanks
Parth



On Tue, May 24, 2016 at 7:30 AM, Ismael Juma <ism...@juma.me.uk> wrote:

> Thanks Rajini. It would probably require a separate KIP as it will
> introduce user visible changes. We could also update KIP-48 to have this
> information, but it seems cleaner to do it separately. We can discuss that
> in the KIP call today.
>
> Ismael
>
> On Tue, May 24, 2016 at 3:19 PM, Rajini Sivaram <
> rajinisiva...@googlemail.com> wrote:
>
> > Ismael,
> >
> > I have created a JIRA (https://issues.apache.org/jira/browse/KAFKA-3751)
> > for adding SCRAM as a SASL mechanism. Would that need another KIP? If
> > KIP-48 will use this mechanism, can this just be a JIRA that gets
> reviewed
> > when the PR is ready?
> >
> > Thank you,
> >
> > Rajini
> >
> > On Tue, May 24, 2016 at 2:46 PM, Ismael Juma <ism...@juma.me.uk> wrote:
> >
> > > Thanks Rajini, SCRAM seems like a good candidate.
> > >
> > > Gwen had independently mentioned this as a SASL mechanism that might be
> > > useful for Kafka and I have been meaning to check it in more detail.
> Good
> > > to know that you are willing to contribute an implementation. Maybe we
> > > should file a separate JIRA for this?
> > >
> > > Ismael
> > >
> > > On Tue, May 24, 2016 at 2:12 PM, Rajini Sivaram <
> > > rajinisiva...@googlemail.com> wrote:
> > >
> > > > SCRAM (Salted Challenge Response Authentication Mechanism) is a
> better
> > > > mechanism than Digest-MD5. Java doesn't come with a built-in SCRAM
> > > > SaslServer or SaslClient, but I will be happy to add support in Kafka
> > > since
> > > > it would be a useful mechanism to support anyway.
> > > > https://tools.ietf.org/html/rfc7677 describes the protocol for
> > > > SCRAM-SHA-256.
> > > >
> > > > On Tue, May 24, 2016 at 2:37 AM, Jun Rao <j...@confluent.io> wrote:
> > > >
> > > > > Parth,
> > > > >
> > > > > Thanks for the explanation. A couple of more questions.
> > > > >
> > > > > 110. What does getDelegationTokenAs mean?
> > > > >
> > > > > 111. What's the typical rate of getting and renewing delegation
> > tokens?
> > > > > That may have an impact on whether they should be directed to the
> > > > > controller.
> > > > >
> > > > > Jun
> > > > >
> > > > > On Mon, May 23, 2016 at 1:19 PM, parth brahmbhatt <
> > > > > brahmbhatt.pa...@gmail.com> wrote:
> > > > >
> > > > > > Hi Jun,
> > > > > >
> > > > > > Thanks for reviewing.
> > > > > >
> > > > > > * We could add a Cluster action to add acls on who can request
> > > > delegation
> > > > > > tokens. I don't see the use case for that yet but down the line
> > when
> > > we
> > > > > > start supporting getDelegationTokenAs it will be necessary.
> > > > > > * Yes we recommend tokens to be only used/distributed over secure
> > > > > channels.
> > > > > > * Depending on what design we end up choosing Invalidation will
> be
> > > > > > responsibility of every broker or controller.
> > > > > > * I am not sure if I documented somewhere that invalidation will
> > > > directly
> > > > > > go through zookeeper but that

Re: [DISCUSS] KIP-48 Support for delegation tokens as an authentication mechanism

[parth brahmbhatt]

Hi Jun,

Thanks for reviewing.

* We could add a Cluster action to add acls on who can request delegation
tokens. I don't see the use case for that yet but down the line when we
start supporting getDelegationTokenAs it will be necessary.
* Yes we recommend tokens to be only used/distributed over secure channels.
* Depending on what design we end up choosing Invalidation will be
responsibility of every broker or controller.
* I am not sure if I documented somewhere that invalidation will directly
go through zookeeper but that is not the intent. Invalidation will either
be request based or due to expiration. No direct zookeeper interaction from
any client.
* "Broker also stores the DelegationToken without the hmac in the
zookeeper." : Sorry about the confusion. The sole purpose of zookeeper in
this design is as distribution channel for tokens between all brokers and a
layer that ensures only tokens that were generated by making a request to a
broker will be accepted (more on this in second paragraph). The token
consists of few elements (owner, renewer, uuid , expiration, hmac) , one of
which is the finally generated hmac but hmac it self is derivable if you
have all the other elements of the token + secret key to generate hmac.
Given zookeeper does not provide SSL support we do not want the entire
token to be wire transferred to zookeeper as that will be an insecure wire
transfer. Instead we only store all the other elements of a delegation
tokens. Brokers can read these elements and because they also have access
to secret key they will be able to generate hmac on their end.

One of the alternative proposed is to avoid zookeeper altogether. A Client
will call broker with required information (owner, renwer, expiration) and
get back (signed hmac, uuid). Broker won't store this in zookeeper. From
this point a client can contact any broker with all the delegation token
info (owner, rewner, expiration, hmac, uuid) the borker will regenerate the
hmac and as long as it matches with hmac presented by client , broker will
allow the request to authenticate.  Only problem with this approach is if
the secret key is compromised any client can now generate random tokens and
they will still be able to authenticate as any user they like. with
zookeeper we guarantee that only tokens acquired via a broker (using some
auth scheme other than delegation token) will be accepted. We need to
discuss which proposal makes more sense and we can go over it in tomorrow's
meeting.

Also, can you forward the invite to me?

Thanks
Parth



On Mon, May 23, 2016 at 10:35 AM, Jun Rao <j...@confluent.io> wrote:

> Thanks for the KIP. A few comments.
>
> 100. This potentially can be useful for Kafka Connect and Kafka rest proxy
> where a worker agent will need to run a task on behalf of a client. We will
> likely need to change how those services use Kafka clients
> (producer/consumer). Instead of a shared client per worker, we will need a
> client per user task since the authentication happens at the connection
> level. For Kafka Connect, the renewer will be the workers. So, we probably
> need to allow multiple renewers. For Kafka rest proxy, the renewer can
> probably just be the creator of the token.
>
> 101. Do we need new acl on who can request delegation tokens?
>
> 102. Do we recommend people to send delegation tokens in an encrypted
> channel?
>
> 103. Who is responsible for expiring tokens, every broker?
>
> 104. For invalidating tokens, would it be better to do it in a request
> instead of going to ZK directly?
>
> 105. The terminology of client in the wiki sometimes refers to the end
> client and some other times refers to the client using the delegation
> tokens. It would be useful to distinguish between the two.
>
> 106. Could you explain the sentence "Broker also stores the DelegationToken
> without the hmac in the zookeeper." a bit more? I thought the delegation
> token is the hmac.
>
> Thanks,
>
> Jun
>
>
> On Mon, May 23, 2016 at 9:22 AM, Jun Rao <j...@confluent.io> wrote:
>
> > Hi, Harsha,
> >
> > Just sent out a KIP meeting invite. We can discuss this in the meeting
> > tomorrow.
> >
> > Thanks,
> >
> > Jun
> >
> > On Thu, May 19, 2016 at 8:47 AM, Harsha <ka...@harsha.io> wrote:
> >
> >> Hi All,
> >>Can we have a KIP meeting around this. The KIP is up for
> >>sometime and if there are any questions lets quickly hash out
> >>details.
> >>
> >> Thanks,
> >> Harsha
> >>
> >> On Thu, May 19, 2016, at 08:40 AM, parth brahmbhatt wrote:
> >> > That is what the hadoop echo system uses so no good reason really. We
> >> > could
> >> > change it to whatever is the newest

Re: [DISCUSS] KIP-48 Support for delegation tokens as an authentication mechanism

[parth brahmbhatt]

yer the the request content in a pretty ugly
> > > way. Perhaps limiting renewer to non-owner is better.
> > >
> > > *I feel this is a necessary evil. While this will make the kafka code
> > > pretty straight forward , forcing  renewer to non-owner pushes the code
> > > ugliness to client and makes it even harder to integrate.  *
> >
> > As mentioned before, I don't think the "renewal by other" approach is
> > that ugly for the clients we expect to use delegation tokens since
> > they will have an app-master of some sort who requested the token to
> > begin with.
> >
> > >
> > > The response for my question on how multiple identities will be
> > > handled wasn't super clear to me - AFAIK, we don't authenticate each
> > > request, we authenticate connections.
> > >
> > > *We authenticate connections, and only when they are being established.
> > Let
> > > me try to phrase this as a question, in absence of delegation tokens if
> > we
> > > had to support the use case using user TGT's how would we do it? My
> point
> > > was it would be no different with delegation tokens. The use case you
> are
> > > describing seems more like impersonation.*
> >
> > Yeah, I thought that one of the things that delegation tokens handled.
> > Maybe I got it wrong :)
> >
> > Thanks for the detailed answers.
> >
> > Gwen
> >
> >
> > > Thanks
> > > Parth
> > >
> > > On Fri, May 13, 2016 at 12:19 AM, Gwen Shapira <g...@confluent.io>
> > wrote:
> > >
> > >> Hi Parth and Harsha,
> > >>
> > >> Few more comments:
> > >>
> > >> * The API / RequestResponse section doesn't seem to have good
> > >> description of the changes to the Kafka Protocol. Sounds like you are
> > >> proposing new DelegationTokenRequest and RenewTokenRequest (and
> > >> matching responses), without detailing the contents of the requests
> > >> and responses? Or rather, you show the class interface, but not the
> > >> underlying protocol. This could be seen as an implementation detail,
> > >> but since the binary protocol is what we provide to non-Java clients,
> > >> we need to show the changes there.
> > >>
> > >> * getDelegationToken sounds like delegationTokenRequestHandler? Is it
> > >> planned to be part of KafkaApi? or Client? Its unclear...
> > >>
> > >> * I want to emphasize that even though delegation tokens are a Hadoop
> > >> innovation, I feel very strongly about not adding dependency on Hadoop
> > >> when implementing delegation tokens for Kafka. The KIP doesn't imply
> > >> such dependency, but if you can clarify...
> > >>
> > >> * Can we get delegation token at any time after authenticating? only
> > >> immediately after?
> > >>
> > >> * My understanding is that tokens will propagate via ZK but without
> > >> additional changes to UpdateMetadata protocol, correct? Clients
> > >> currently don't retry on SASL auth failure (IIRC), but since the
> > >> tokens propagate between brokers asynch, we will need to retry a bit
> > >> to avoid clients failing auth due to timing issues.
> > >>
> > >> * Strongly agreeing on clients not touching ZK directly :)
> > >>
> > >> * I liked Ashish's suggestion of having just the controller issue the
> > >> delegation tokens, to avoid syncing a shared secret. Not sure if we
> > >> want to continue the discussion here or on the wiki. I think that we
> > >> can decouple the problem of "token distribution" from "shared secret
> > >> distribution" and use the controller as the only token generator to
> > >> solve the second issue, while still using ZK async to distribute
> > >> tokens.
> > >>
> > >> * I am also uncomfortable with infinite lifetime of tokens (and hoped
> > >> to hear from others in the community) - but having the option and
> > >> documenting it as less secure, allows users to configure their system
> > >> as the wish.
> > >>
> > >> * While I like the idea of forcing kerberos auth for renwal, I think
> > >> it mixes the transport layer the the request content in a pretty ugly
> > >> way. Perhaps limiting renewer to non-owner is better.
> > >>
> > >> Things I'd still like to see:
> > >&g

Re: [DISCUSS] KIP-48 Support for delegation tokens as an authentication mechanism

[parth brahmbhatt]

gt; but since the binary protocol is what we provide to non-Java clients,
> we need to show the changes there.
>
> * getDelegationToken sounds like delegationTokenRequestHandler? Is it
> planned to be part of KafkaApi? or Client? Its unclear...
>
> * I want to emphasize that even though delegation tokens are a Hadoop
> innovation, I feel very strongly about not adding dependency on Hadoop
> when implementing delegation tokens for Kafka. The KIP doesn't imply
> such dependency, but if you can clarify...
>
> * Can we get delegation token at any time after authenticating? only
> immediately after?
>
> * My understanding is that tokens will propagate via ZK but without
> additional changes to UpdateMetadata protocol, correct? Clients
> currently don't retry on SASL auth failure (IIRC), but since the
> tokens propagate between brokers asynch, we will need to retry a bit
> to avoid clients failing auth due to timing issues.
>
> * Strongly agreeing on clients not touching ZK directly :)
>
> * I liked Ashish's suggestion of having just the controller issue the
> delegation tokens, to avoid syncing a shared secret. Not sure if we
> want to continue the discussion here or on the wiki. I think that we
> can decouple the problem of "token distribution" from "shared secret
> distribution" and use the controller as the only token generator to
> solve the second issue, while still using ZK async to distribute
> tokens.
>
> * I am also uncomfortable with infinite lifetime of tokens (and hoped
> to hear from others in the community) - but having the option and
> documenting it as less secure, allows users to configure their system
> as the wish.
>
> * While I like the idea of forcing kerberos auth for renwal, I think
> it mixes the transport layer the the request content in a pretty ugly
> way. Perhaps limiting renewer to non-owner is better.
>
> Things I'd still like to see:
>
> * More detailed explanation on what we plan to do for the java clients
> specifically - new configuration? new APIs?
> The response for my question on how multiple identities will be
> handled wasn't super clear to me - AFAIK, we don't authenticate each
> request, we authenticate connections.
>
> * Alternatives: Delegation tokens are only used in the Hadoop
> ecosystem. I'm wondering if there are alternatives in other ecosystems
> (Mesos? Tachyon? Cassandra?) and whether there are some advantages
> there.
>
> Gwen
>
> On Thu, May 12, 2016 at 1:05 PM, Harsha <ka...@harsha.io> wrote:
> > Hi Gwen,
> >Can you look at Parth's last reply. Does it answer your
> >concerns.
> >
> > Thanks,
> > Harsha
> >
> > On Wed, May 4, 2016, at 09:25 AM, parth brahmbhatt wrote:
> >> Thanks for reviewing Gwen. The wiki already has details on token
> >> expiration
> >> under token acquisition process
> >> <
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-48+Delegation+token+support+for+Kafka#KIP-48DelegationtokensupportforKafka-Tokenacquisition
> >.
> >> Current proposal is that tokens will expire based on a server side
> >> configuration (default 24 hours) unless renewed. Renewal is only allowed
> >> until the max life time of token. Alternatively we could also make that
> >> an
> >> optional param and the server side default can serve as the upper bound.
> >>
> >> To your second point it will be done exactly the same way we would
> >> support
> >> multiple keytabs. The calling client will have to put the tokens it
> wants
> >> to use in the subject instance and call produce/consume inside
> >> subject.doas. Each caller will have to keep track of its own subject. I
> >> will have to look at the code to see if we support this feature right
> now
> >> but my understanding is delegation token shouldn't need any special
> >> treatment as its just another type of Credential in the subject.
> >>
> >> I would also like to know what is your opinion about infinite renewal
> (my
> >> recommendation is to not support this), tokens renewing them self(my
> >> recommendation is to not support this) and most importantly your choice
> >> between the alternatives listed on this thread
> >> <
> http://apache.markmail.org/message/ca3iakt3m6c4yygp?q=KIP-48+Support+for+delegation+tokens+as+an+authentication+mechanism
> >
> >> ( I am leaning towards the alternative-2 minus controller distributing
> >> secret). Thanks again for reviewing.
> >>
> >> Thanks
> >> Parth
> >>
> >>
> >>
> >> On Wed, May 4, 

Re: Reg: Kafka-Acls

[parth brahmbhatt]

Try the following
/bin/kafka-acls.sh --topic permissiontopic --allow-host {host}
--allow-principal
User:dev --producer --add --authorizer-properties
zookeeper.connect={host:port}


Thanks
Parth

On Thu, May 5, 2016 at 4:26 PM, BigData dev <bigdatadev...@gmail.com> wrote:

> Hi,
> Thanks for Info.
> It worked.
> Acls are correctly set, but when i run the producer is throwing error,
> even if acl's are correctlt set.
>
> bin/kafka-console-producer.sh --broker-list bdavm1222.svl.ibm.com:6667
> --topic permissiontopic --producer.config producer.properties
> jj
> [2016-05-05 16:02:23,308] WARN Error while fetching metadata with
> correlation id 0 : {permissiontopic=TOPIC_AUTHORIZATION_FAILED}
> (org.apache.kafka.clients.NetworkClient)
> [2016-05-05 16:02:23,309] ERROR Error when sending message to topic
> permissiontopic with key: null, value: 2 bytes with error: Not authorized
> to access topics: [permissiontopic]
> (org.apache.kafka.clients.producer.internals.ErrorLoggingCallback)
> ^C[2016-05-05 16:02:45,754] WARN TGT renewal thread has been interrupted
> and will exit. (org.apache.kafka.common.security.kerberos.Login)
> producer is throwing error
>
> Anythoughts on this.
>
>
> Regards,
> Bharat
>
>
>
> On Thu, May 5, 2016 at 4:19 PM, parth brahmbhatt <
> brahmbhatt.pa...@gmail.com> wrote:
>
>> Acls will be written in zookeeper but you are using getAcl , what you need
>>  is get  /kafka-acl/Topic/permissiontopic
>>
>> Thanks
>> Parth
>>
>> On Thu, May 5, 2016 at 3:28 PM, BigData dev <bigdatadev...@gmail.com>
>> wrote:
>>
>> > Hi,
>> > When I run the command
>> >  /bin/kafka-acls.sh --topic permissiontopic --add --allow-host {host}
>> > --allow-principal User:dev --operation Write --authorizer-properties
>> > zookeeper.connect={host:port}
>> >
>> > I am getting output as acls are set.
>> >
>> > But when i check under zookeeper using below command, it is not showing
>> the
>> > acls which I have set for user dev.
>> >
>> > [zk: (CONNECTED) 13] getAcl /kafka-acl/Topic/permissiontopic
>> > 'world,'anyone
>> > : r
>> > 'sasl,'kafka
>> > : cdrwa
>> >
>> > Is my understanding correct kafka-acls will be written to zookeeper
>> node.
>> >
>> >
>> > This is causing when i run producer, it is failing as topic
>> authorization
>> > failed.
>> >
>> > If any one has used this, can you please provide the inputs
>> >
>> > Regards,
>> > Bharat
>> >
>>
>
>

Re: Reg: Kafka-Acls

[parth brahmbhatt]

Acls will be written in zookeeper but you are using getAcl , what you need
 is get  /kafka-acl/Topic/permissiontopic

Thanks
Parth

On Thu, May 5, 2016 at 3:28 PM, BigData dev  wrote:

> Hi,
> When I run the command
>  /bin/kafka-acls.sh --topic permissiontopic --add --allow-host {host}
> --allow-principal User:dev --operation Write --authorizer-properties
> zookeeper.connect={host:port}
>
> I am getting output as acls are set.
>
> But when i check under zookeeper using below command, it is not showing the
> acls which I have set for user dev.
>
> [zk: (CONNECTED) 13] getAcl /kafka-acl/Topic/permissiontopic
> 'world,'anyone
> : r
> 'sasl,'kafka
> : cdrwa
>
> Is my understanding correct kafka-acls will be written to zookeeper node.
>
>
> This is causing when i run producer, it is failing as topic authorization
> failed.
>
> If any one has used this, can you please provide the inputs
>
> Regards,
> Bharat
>

Re: [DISCUSS] KIP-48 Support for delegation tokens as an authentication mechanism

[parth brahmbhatt]

Thanks for reviewing Gwen. The wiki already has details on token expiration
under token acquisition process
<https://cwiki.apache.org/confluence/display/KAFKA/KIP-48+Delegation+token+support+for+Kafka#KIP-48DelegationtokensupportforKafka-Tokenacquisition>.
Current proposal is that tokens will expire based on a server side
configuration (default 24 hours) unless renewed. Renewal is only allowed
until the max life time of token. Alternatively we could also make that an
optional param and the server side default can serve as the upper bound.

To your second point it will be done exactly the same way we would support
multiple keytabs. The calling client will have to put the tokens it wants
to use in the subject instance and call produce/consume inside
subject.doas. Each caller will have to keep track of its own subject. I
will have to look at the code to see if we support this feature right now
but my understanding is delegation token shouldn't need any special
treatment as its just another type of Credential in the subject.

I would also like to know what is your opinion about infinite renewal (my
recommendation is to not support this), tokens renewing them self(my
recommendation is to not support this) and most importantly your choice
between the alternatives listed on this thread
<http://apache.markmail.org/message/ca3iakt3m6c4yygp?q=KIP-48+Support+for+delegation+tokens+as+an+authentication+mechanism>
( I am leaning towards the alternative-2 minus controller distributing
secret). Thanks again for reviewing.

Thanks
Parth



On Wed, May 4, 2016 at 6:17 AM, Gwen Shapira <g...@confluent.io> wrote:

> Harsha,
>
> I was thinking of the Rest Proxy. I didn't see your design yet, but in
> our proxy, we have a set of producers, which will serve multiple users
> going through the proxy. Since these users will have different
> privileges, they'll need to authenticate separately, and can't share a
> token.
>
> Am I missing anything?
>
> Gwen
>
> On Tue, May 3, 2016 at 2:11 PM, Harsha <ka...@harsha.io> wrote:
> > Gwen,
> >On your second point. Can you describe a usecase where
> >mutliple clients ended up sharing a producer and even if they
> >do why can't they not use single token that producer
> >captures. Why would we need multiple clients with different
> >tokens sharing a single instance of producer.  Also in this
> >case other clients have access all the tokens no?
> >
> > Thanks,
> > Harsha
> >
> >
> > On Tue, May 3, 2016, at 11:49 AM, Gwen Shapira wrote:
> >> Sorry for the delay:
> >>
> >> Two questions that we didn't see in the wiki:
> >> 1. Is there an expiration for delegation tokens? Renewal? How do we
> >> revoke them?
> >> 2. If we want to use delegation tokens for "do-as" (say, submit Storm
> >> job as my user), we will need a producer for every job (we can't share
> >> them between multiple jobs running on same node), since we only
> >> authenticate when connecting. Is there a plan to change this for
> >> delegation tokens, in order to allow multiple users with different
> >> tokens to share a client?
> >>
> >> Gwen
> >>
> >> On Tue, May 3, 2016 at 9:12 AM, parth brahmbhatt
> >> <brahmbhatt.pa...@gmail.com> wrote:
> >> > Bumping this up one more time, can other committers review?
> >> >
> >> > Thanks
> >> > Parth
> >> >
> >> > On Tue, Apr 26, 2016 at 9:07 AM, Harsha <ka...@harsha.io> wrote:
> >> >
> >> >> Parth,
> >> >>   Overall current design looks good to me. I am +1 on the
> KIP.
> >> >>
> >> >> Gwen , Jun can you review this as well.
> >> >>
> >> >> -Harsha
> >> >>
> >> >> On Tue, Apr 19, 2016, at 09:57 AM, parth brahmbhatt wrote:
> >> >> > Thanks for review Jitendra.
> >> >> >
> >> >> > I don't like the idea of infinite lifetime but I see the Streaming
> use
> >> >> > case. Even for Streaming use case I was hoping there will be some
> notion
> >> >> > of
> >> >> > master/driver that can get new delegation tokens at fixed interval
> and
> >> >> > distribute to workers. If that is not the case for we can discuss
> >> >> > delegation tokens renewing them self and the security implications
> of the
> >> >> > same.
> >> >> >
> >> >> > I did not want clients to fetch tokens from zookeeper, overall I
> think
&g

Re: [DISCUSS] KIP-48 Support for delegation tokens as an authentication mechanism

[parth brahmbhatt]

Bumping this up one more time, can other committers review?

Thanks
Parth

On Tue, Apr 26, 2016 at 9:07 AM, Harsha <ka...@harsha.io> wrote:

> Parth,
>   Overall current design looks good to me. I am +1 on the KIP.
>
> Gwen , Jun can you review this as well.
>
> -Harsha
>
> On Tue, Apr 19, 2016, at 09:57 AM, parth brahmbhatt wrote:
> > Thanks for review Jitendra.
> >
> > I don't like the idea of infinite lifetime but I see the Streaming use
> > case. Even for Streaming use case I was hoping there will be some notion
> > of
> > master/driver that can get new delegation tokens at fixed interval and
> > distribute to workers. If that is not the case for we can discuss
> > delegation tokens renewing them self and the security implications of the
> > same.
> >
> > I did not want clients to fetch tokens from zookeeper, overall I think
> > its
> > better if clients don't rely on our metadata store and I think we are
> > moving in that direction with all the KIP-4 improvements.  I chose
> > zookeeper as in this case the client will still just talk to broker , its
> > the brokers that will use zookeeper which we already do for a lot of
> > other
> > usecases + ease of development + and the ability so tokens will survive
> > even a rolling restart/cluster failure. if a majority agrees the added
> > complexity to have controller forwarding keys to all broker is justified
> > as
> > it provides tighter security , I am fine with that option too.
> >
> > Given zookeeper does not support SSL we can not store master keys in
> > zookeeper as master keys will be exposed on wire. To support rotation
> > without affecting current clients is something I need to put more thought
> > in. My current proposal assumes the rotation will invalidate all current
> > tokens.
> >
> > I request committers to also review and post their comments so we can
> > make
> > progress on this KIP.
> >
> > Thanks
> > Parth
> >
> > On Tue, Apr 19, 2016 at 8:39 AM, Ashish Singh <asi...@cloudera.com>
> > wrote:
> >
> > > On Mon, Apr 18, 2016 at 11:26 AM, Harsha <ka...@harsha.io> wrote:
> > >
> > > > Unifying the two discussion threads on this KIP.
> > > >
> > > > Here is the response from Jitendra
> > > >
> > > > "The need for a large number of clients that are running all over the
> > > > cluster that authenticate with Kafka brokers, is very similar to the
> > > > Hadoop use case of large number of tasks running across the cluster
> that
> > > > need authentication to Hdfs Namenode. Therefore, the delegation token
> > > > approach does seem like a good fit for this use case as we have seen
> it
> > > > working at large scale in HDFS and YARN.
> > > >
> > > >   The proposed design is very much inline with Hadoop approach. A few
> > > >   comments:
> > > >
> > > > 1) Why do you guys want to allow infinite renewable lifetime for a
> > > > token? HDFS restricts a token to a max life time (default 7 days).  A
> > > > token's vulnerability is believed to increase with time.
> > > >
> > > I agree that having infinite lifetime might not be the best idea.
> > >
> > > >
> > > > 2) As I understand the tokens are stored in zookeeper as well, and
> can
> > > > be updated there. This is clever as it can allow replacing the tokens
> > > > once they run out of max life time, and clients can download new
> tokens
> > > > from zookeeper. It shouldn't be a big load on zookeeper as a client
> will
> > > > need to get a new token once in several days. In this approach you
> don't
> > > > need infinite lifetime on the token even for long running clients.
> > > >
> > > > 3) The token password are generated using a master key. The master
> key
> > > > should also be periodically changed. In Hadoop, the default renewal
> > > > period is 1 day.?
> > > >
> > > IIUC, this will require brokers maintaining a list of X most recent
> master
> > > keys. This list will have to be persisted somewhere, as if a broker
> goes
> > > down it will have to get that list again and storing master keys on ZK
> is
> > > not the best idea. However, if a broker goes down then we have much
> bigger
> > > issue to deal with and client can always re-authenticate is such
> events.
> > >
> > > Did you happen to take a look at other 

[jira] [Commented] (KAFKA-3294) Kafka REST API

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-3294?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15261562#comment-15261562
 ] 

Parth Brahmbhatt commented on KAFKA-3294:
-

Here is a version we are working on 
https://github.com/Parth-Brahmbhatt/kafka-rest. Its still work in progress but 
Its better to start the discussion around getting more clarity on requirements, 
how we want to package and distribute it and what does the community need from 
this feature.

> Kafka REST API
> --
>
> Key: KAFKA-3294
> URL: https://issues.apache.org/jira/browse/KAFKA-3294
> Project: Kafka
>  Issue Type: New Feature
>Reporter: Sriharsha Chintalapani
>    Assignee: Parth Brahmbhatt
>
> This JIRA is to build Kafka REST API for producer, consumer and also any 
> administrative tasks such as create topic, delete topic. We do have lot of 
> kafka client api support in different languages but having REST API for 
> producer and consumer will make it easier for users to read or write Kafka. 
> Also having administrative API will help in managing a cluster or building 
> administrative dashboards.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: [DISCUSS] KIP-48 Support for delegation tokens as an authentication mechanism

[parth brahmbhatt]

se,
> > > >> > i.e,
> > > >> > >when a new broker comes up it will have to get all delegation
> > tokens
> > > >> > from
> > > >> > >the controller.
> > > >> > >3. In catastrophic failures where all brokers go down, the
> > tokens will
> > > >> > >be lost even if servers are restarted as tokens are not
> > persisted
> > > >> > anywhere.
> > > >> > >If this happens, then there are more important things to
> worry
> > about
> > > >> > and
> > > >> > >maybe it is better to re-authenticate.
> > > >> > >
> > > >> > > (Alternative 2) Do not distribute DT to other brokers at all.
> > > >> > >
> > > >> > >1. Client authenticates with a broker.
> > > >> > >2. Once a client is authenticated, it will make a broker side
> > call to
> > > >> > >issue a delegation token.
> > > >> > >3. The broker generates DT of form, [hmac + (owner, renewer,
> > > >> > >maxLifeTime, id, hmac, expirationTime)] and passes back this
> > DT to
> > > >> > client.
> > > >> > >hmac is generated via {HMAC-SHA256(owner, renewer,
> > maxLifeTime, id,
> > > >> > hmac,
> > > >> > >expirationTime) using SecretKey}. Note that all brokers have
> > this
> > > >> > SecretKey.
> > > >> > >4. Client then goes to any broker and to authenticate sends
> > the DT.
> > > >> > >Broker recalculates hmac using (owner, renewer, maxLifeTime,
> > id, hmac,
> > > >> > >expirationTime) info from DT and its SecretKey. If it matches
> > with
> > > >> > hmac of
> > > >> > >DT, client is authenticated. Yes, it will do other obvious
> > checks of
> > > >> > >timestamp expiry and such.
> > > >> > >
> > > >> > > Note that secret key will be generated by controller and passed
> to
> > > >> > brokers
> > > >> > > periodically.
> > > >> > > Probable issues and fixes
> > > >> > >
> > > >> > >1. How to delete a DT? Yes, that is a downside here. However,
> > this can
> > > >> > >be handled with brokers maintaining a blacklist of DTs, DTs
> > from this
> > > >> > list
> > > >> > >can be removed after expiry.
> > > >> > >2. In catastrophic failures where all brokers go down, the
> > tokens will
> > > >> > >be lost even if servers are restarted as tokens are not
> > persisted
> > > >> > anywhere.
> > > >> > >If this happens, then there are more important things to
> worry
> > about
> > > >> > and
> > > >> > >maybe it is better to re-authenticate.
> > > >> > >
> > > >> > >
> > > >> > >
> > > >> > > On Fri, Feb 26, 2016 at 1:58 PM, Parth Brahmbhatt <
> > > >> > > pbrahmbh...@hortonworks.com> wrote:
> > > >> > >
> > > >> > >> Hi,
> > > >> > >>
> > > >> > >> I have filed KIP-48 so we can offer hadoop like delegation
> > tokens in
> > > >> > >> kafka. You can review the design
> > > >> > >>
> > > >> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-48+Delegation+token+support+for+Kafka
> > > >> > .
> > > >> > >> This KIP depends on KIP-43 and we have also discussed an
> > alternative to
> > > >> > >> proposed design here<
> > > >> > >>
> > > >> >
> >
> https://issues.apache.org/jira/browse/KAFKA-1696?focusedCommentId=15167800=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15167800
> > > >> > >> >.
> > > >> > >>
> > > >> > >> Thanks
> > > >> > >> Parth
> > > >> > >>
> > > >> > >
> > > >> > >
> > > >> > >
> > > >> > > --
> > > >> > >
> > > >> > > Regards,
> > > >> > > Ashish
> > > >> >
> >
>
>
>
> --
>
> Regards,
> Ashish
>

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

[parth brahmbhatt]

+1

Thanks
Parth

On Wed, Mar 30, 2016 at 8:22 PM, Ewen Cheslack-Postava 
wrote:

> +1
>
> -Ewen
>
> On Wed, Mar 30, 2016 at 7:20 PM, Gwen Shapira  wrote:
>
> > So my +1 is back :)
> >
> > On Wed, Mar 30, 2016 at 4:51 PM, Ashish Singh 
> wrote:
> >
> > > My bad, I moved the config option to rejected alternatives. The new
> > method
> > > getSupportedPrincipalTypes is what the KIP is proposing.
> > > ​
> > >
> > > On Wed, Mar 30, 2016 at 4:43 PM, Gwen Shapira 
> wrote:
> > >
> > > > My appologies, but I need to retract my vote.
> > > >
> > > > The KIP currently specifies two alternatives (none rejected). So I'm
> > not
> > > > sure what I'm voting on...
> > > >
> > > > For reference, my original +1 was for the new method:
> > > > getSupportedPrincipalTypes()
> > > > I'm not even sure how would a config work in that case.
> > > >
> > > > Anyway, we need to resolve this before voting.
> > > >
> > > > Gwen
> > > >
> > > > On Wed, Mar 30, 2016 at 4:34 PM, Gwen Shapira 
> > wrote:
> > > >
> > > > > +1.
> > > > >
> > > > > It's a small change and I can see how this will help improve
> > usability
> > > > for
> > > > > some of the authorizers.
> > > > >
> > > > > Gwen
> > > > >
> > > > > On Wed, Mar 30, 2016 at 11:02 AM, Ashish Singh <
> asi...@cloudera.com>
> > > > > wrote:
> > > > >
> > > > >> Hi Guys,
> > > > >>
> > > > >> I would like to open the vote for KIP-50.
> > > > >>
> > > > >> KIP:
> > > > >>
> > > > >>
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-50+-+Enhance+Authorizer+interface+to+be+aware+of+supported+Principal+Types
> > > > >>
> > > > >> Discuss thread: here
> > > > >> <
> > > > >>
> > > >
> > >
> >
> http://mail-archives.apache.org/mod_mbox/kafka-dev/201603.mbox/%3CCAGQG9cUCLDO0owdziDcL9iStXNF1wURyVNcEZedQJg%3DUuC7j%3DQ%40mail.gmail.com%3E
> > > > >> >
> > > > >>
> > > > >> JIRA: https://issues.apache.org/jira/browse/KAFKA-3186
> > > > >>
> > > > >> PR: https://github.com/apache/kafka/pull/861
> > > > >> ​
> > > > >> --
> > > > >>
> > > > >> Regards,
> > > > >> Ashish
> > > > >>
> > > > >
> > > > >
> > > >
> > >
> > >
> > >
> > > --
> > >
> > > Regards,
> > > Ashish
> > >
> >
>
>
>
> --
> Thanks,
> Ewen
>

[DISCUSS] KIP-48 Support for delegation tokens as an authentication mechanism

[Parth Brahmbhatt]

Hi,

I have filed KIP-48 so we can offer hadoop like delegation tokens in kafka. You 
can review the design 
https://cwiki.apache.org/confluence/display/KAFKA/KIP-48+Delegation+token+support+for+Kafka.
 This KIP depends on KIP-43 and we have also discussed an alternative to 
proposed design 
here.

Thanks
Parth

[GitHub] kafka pull request: KAFKA-3291: DumpLogSegment tool should also pr...

[Parth-Brahmbhatt]

GitHub user Parth-Brahmbhatt opened a pull request:

https://github.com/apache/kafka/pull/975

KAFKA-3291: DumpLogSegment tool should also provide an option to only…

… verify index sanity.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Parth-Brahmbhatt/kafka KAFKA-3291

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/975.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #975


commit fdc52b7edf1d9a9f73cf7b42ee4c30382e8471eb
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2016-02-26T00:14:34Z

KAFKA-3291: DumpLogSegment tool should also provide an option to only 
verify index sanity.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Created] (KAFKA-3291) DumpLogSegment tool should also provide an option to only verify index sanity.

[Parth Brahmbhatt (JIRA)]

Parth Brahmbhatt created KAFKA-3291:
---

 Summary: DumpLogSegment tool should also provide an option to only 
verify index sanity.
 Key: KAFKA-3291
 URL: https://issues.apache.org/jira/browse/KAFKA-3291
 Project: Kafka
  Issue Type: Improvement
  Components: tools
Reporter: Parth Brahmbhatt
Assignee: Parth Brahmbhatt


DumpLogSegment tool should call index.sanityCheck function as part of index 
sanity check as that function determines if an index will be rebuilt on restart 
or not. This is a cheap check as it only checks the file size and can help in 
scenarios where customer is trying to figure out which index files will be 
rebuilt on startup which directly affects the broker bootstrap time. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-1696) Kafka should be able to generate Hadoop delegation tokens

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-1696?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15168010#comment-15168010
 ] 

Parth Brahmbhatt commented on KAFKA-1696:
-

[~gwenshap] I think that discussion is happening as part of another KIP and no 
matter what we chose I don't think it affects the delegation token design. 

> Kafka should be able to generate Hadoop delegation tokens
> -
>
> Key: KAFKA-1696
> URL: https://issues.apache.org/jira/browse/KAFKA-1696
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Jay Kreps
>Assignee: Parth Brahmbhatt
>
> For access from MapReduce/etc jobs run on behalf of a user.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-1696) Kafka should be able to generate Hadoop delegation tokens

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-1696?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15167928#comment-15167928
 ] 

Parth Brahmbhatt commented on KAFKA-1696:
-

[~gwenshap] [~sriharsha] Please share your preference here so I can update the 
KIP accordingly and start a discussion thread. I wanted to try SASL over MD5 to 
ensure that this design will work as is but I don't see myself writing that 
prototype soon so might as well start the KIP discussion.

> Kafka should be able to generate Hadoop delegation tokens
> -
>
> Key: KAFKA-1696
> URL: https://issues.apache.org/jira/browse/KAFKA-1696
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Jay Kreps
>Assignee: Parth Brahmbhatt
>
> For access from MapReduce/etc jobs run on behalf of a user.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Comment Edited] (KAFKA-1696) Kafka should be able to generate Hadoop delegation tokens

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-1696?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15167800#comment-15167800
 ] 

Parth Brahmbhatt edited comment on KAFKA-1696 at 2/25/16 8:23 PM:
--

So here is how that request path would work in my mind:

* Client sends request for token acquisition to any broker.
* Broker forwards the request to the controller.
* Controller generates the token and pushes the tokens to all brokers. (Will 
need a new API)
* Controller responds back to original broker with the token.
* Broker responds back to client with the token.

Renewal is pretty much the same.

The race condition you are describing can still happen in the above case during 
renewal because controller may have pushed the renewal information to a subset 
of broker and die. The clients depending on which broker it connects to may get 
an exception or success. I do agree though that given controller would not have 
responded back with success the original renew request should be retried and 
most likely the scenario can be avoided.

If the above steps seems right , here are the advantages of this approach:

Advantage:
* Token generation/renewal will not involve zookeeper. I am not too worried 
about the load on zookeeper added due to this but it definitely seems more 
secure and follows the Hadoop model more closely. However zookeeper needs to be 
secure for lot of other things in kafka so not sure if this should really be a 
concern.
* Clients will get better consistency.

Disadvantage:
* We will have to add new APIs to support controller pushing tokens to brokers 
on top of the minimal APIs that are currently proposed. I like the publicly 
available APIs to be minimal and I like them to be something that we expect 
clients to use + this adds more development complexity. Overall this seems like 
a more philosophical thing so depending on who you ask they may see this as 
disadvantage or not. 
* We will also have to add APIs to support the bootstrapping case. What I mean 
is , when a new broker comes up it will have to get all delegation tokens from 
the controller so we will again need to add new APIs like getAllTokens. Again 
some of us may see that as disadvantage and some may not.
* In catastrophic failures where all brokers go down, the tokens will be lost 
even if servers are restarted as tokens are not persisted anywhere. Granted if 
something like this happens customer has bigger things to worry about but if 
they don't have to regenerate/redistribute tokens that is one less thing.

I don't see strong reasons to go one way or another so I would still like to go 
with zookeeper but don't really feel strongly about it. If you think I have 
mischaracterized what you were proposing feel free to add more details or list 
and other advantages/disadvantages.



was (Author: parth.brahmbhatt):
So here is how that request path would work in my mind:

* Client sends request for token acquisition to any broker.
* Broker forwards the request to the controller.
* Controller generates the token and pushes the tokens to all brokers. (Will 
need a new API)
* Controller responds back to original broker with the token.
* Broker responds back to client with the token.

Renewal is pretty much the same.

The race condition you are describing can still happen in the above case during 
renewal because controller may have pushed the renewal information to a subset 
of broker and die. The clients depending on which broker it connects to may get 
an exception or success. I do agree though that given controller would not have 
responded back with success the original renew request should be retried and 
most likely the scenario can be avoided.

If the above steps seems right , here are the advantages of this approach:

Advantage:
* Token generation/renewal will not involve zookeeper. I am not too worried 
about the load on zookeeper added due to this but it definitely seems more 
secure and follows the Hadoop model more closely. However zookeeper needs to be 
secure for lot of other things in kafka so not sure if this should really be a 
concern.

Disadvantage:
* We will have to add new APIs to support controller pushing tokens to brokers 
on top of the minimal APIs that are currently proposed. I like the publicly 
available APIs to be minimal and I like them to be something that we expect 
clients to use + this adds more development complexity. Overall this seems like 
a more philosophical thing so depending on who you ask they may see this as 
disadvantage or not. 
* We will also have to add APIs to support the bootstrapping case. What I mean 
is , when a new broker comes up it will have to get all delegation tokens from 
the controller so we will again need to add new APIs like getAllTokens. Again 
some of us may see that as disadvantage and some may not.
* In catastrophic failures where all brokers go down, the tokens

[jira] [Commented] (KAFKA-1696) Kafka should be able to generate Hadoop delegation tokens

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-1696?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15167800#comment-15167800
 ] 

Parth Brahmbhatt commented on KAFKA-1696:
-

So here is how that request path would work in my mind:

* Client sends request for token acquisition to any broker.
* Broker forwards the request to the controller.
* Controller generates the token and pushes the tokens to all brokers. (Will 
need a new API)
* Controller responds back to original broker with the token.
* Broker responds back to client with the token.

Renewal is pretty much the same.

The race condition you are describing can still happen in the above case during 
renewal because controller may have pushed the renewal information to a subset 
of broker and die. The clients depending on which broker it connects to may get 
an exception or success. I do agree though that given controller would not have 
responded back with success the original renew request should be retried and 
most likely the scenario can be avoided.

If the above steps seems right , here are the advantages of this approach:

Advantage:
* Token generation/renewal will not involve zookeeper. I am not too worried 
about the load on zookeeper added due to this but it definitely seems more 
secure and follows the Hadoop model more closely. However zookeeper needs to be 
secure for lot of other things in kafka so not sure if this should really be a 
concern.

Disadvantage:
* We will have to add new APIs to support controller pushing tokens to brokers 
on top of the minimal APIs that are currently proposed. I like the publicly 
available APIs to be minimal and I like them to be something that we expect 
clients to use + this adds more development complexity. Overall this seems like 
a more philosophical thing so depending on who you ask they may see this as 
disadvantage or not. 
* We will also have to add APIs to support the bootstrapping case. What I mean 
is , when a new broker comes up it will have to get all delegation tokens from 
the controller so we will again need to add new APIs like getAllTokens. Again 
some of us may see that as disadvantage and some may not.
* In catastrophic failures where all brokers go down, the tokens will be lost 
even if servers are restarted as tokens are not persisted anywhere. Granted if 
something like this happens customer has bigger things to worry about but if 
they don't have to regenerate/redistribute tokens that is one less thing.

I don't see strong reasons to go one way or another so I would still like to go 
with zookeeper but don't really feel strongly about it. If you think I have 
mischaracterized what you were proposing feel free to add more details or list 
and other advantages/disadvantages.


> Kafka should be able to generate Hadoop delegation tokens
> -
>
> Key: KAFKA-1696
> URL: https://issues.apache.org/jira/browse/KAFKA-1696
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Jay Kreps
>Assignee: Parth Brahmbhatt
>
> For access from MapReduce/etc jobs run on behalf of a user.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-1696) Kafka should be able to generate Hadoop delegation tokens

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-1696?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15167696#comment-15167696
 ] 

Parth Brahmbhatt commented on KAFKA-1696:
-

[~singhashish] Thanks for taking the time to review. I will add the 
errorcode/exception to the KIP and sample CLI.

For point 2, can you elaborate why it might be a better idea to communicate via 
coordinator? When I was thinking about distributing the tokens I felt the 
coordinator will just add extra complexity and wont buy us much. We already 
have multiple things ACLs/Configs for which we rely on zookeeper watchers so it 
seemed  like a natural choice. 

> Kafka should be able to generate Hadoop delegation tokens
> -
>
> Key: KAFKA-1696
> URL: https://issues.apache.org/jira/browse/KAFKA-1696
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Jay Kreps
>Assignee: Parth Brahmbhatt
>
> For access from MapReduce/etc jobs run on behalf of a user.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-1696) Kafka should be able to generate Hadoop delegation tokens

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-1696?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15149659#comment-15149659
 ] 

Parth Brahmbhatt commented on KAFKA-1696:
-

[~gwenshap] [~harsha_ch] [~singhashish] I posted an initial KIP Draft 
https://cwiki.apache.org/confluence/display/KAFKA/KIP-48+Delegation+token+support+for+Kafka.
 I haven't yet opened a Discuss thread as I need to verify some assumptions I 
have made. 

> Kafka should be able to generate Hadoop delegation tokens
> -
>
> Key: KAFKA-1696
> URL: https://issues.apache.org/jira/browse/KAFKA-1696
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Jay Kreps
>Assignee: Parth Brahmbhatt
>
> For access from MapReduce/etc jobs run on behalf of a user.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-1696) Kafka should be able to generate Hadoop delegation tokens

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-1696?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15113295#comment-15113295
 ] 

Parth Brahmbhatt commented on KAFKA-1696:
-

I will assign it to my self and file a KIP 

> Kafka should be able to generate Hadoop delegation tokens
> -
>
> Key: KAFKA-1696
> URL: https://issues.apache.org/jira/browse/KAFKA-1696
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Jay Kreps
>Assignee: Sriharsha Chintalapani
>
> For access from MapReduce/etc jobs run on behalf of a user.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Assigned] (KAFKA-1696) Kafka should be able to generate Hadoop delegation tokens

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-1696?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Parth Brahmbhatt reassigned KAFKA-1696:
---

Assignee: Parth Brahmbhatt  (was: Sriharsha Chintalapani)

> Kafka should be able to generate Hadoop delegation tokens
> -
>
> Key: KAFKA-1696
> URL: https://issues.apache.org/jira/browse/KAFKA-1696
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Jay Kreps
>Assignee: Parth Brahmbhatt
>
> For access from MapReduce/etc jobs run on behalf of a user.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-1696) Kafka should be able to generate Hadoop delegation tokens

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-1696?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15106023#comment-15106023
 ] 

Parth Brahmbhatt commented on KAFKA-1696:
-

[~gwenshap] Are you still working on this? We have some customers that needs 
this feature and if you have not started the design work I would like to take 
this over. 

> Kafka should be able to generate Hadoop delegation tokens
> -
>
> Key: KAFKA-1696
> URL: https://issues.apache.org/jira/browse/KAFKA-1696
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Jay Kreps
>Assignee: Sriharsha Chintalapani
>
> For access from MapReduce/etc jobs run on behalf of a user.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] kafka pull request: KAFKA-2000: Delete topic should also delete co...

[Parth-Brahmbhatt]

GitHub user Parth-Brahmbhatt opened a pull request:

https://github.com/apache/kafka/pull/704

KAFKA-2000: Delete topic should also delete consumer offsets.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Parth-Brahmbhatt/kafka KAFKA-2000

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/704.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #704


commit ceae0b7031d297a7db6664b435bb3cdc55228646
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2015-12-18T20:35:32Z

KAFKA-2000: Delete topic should also delete consumer offsets.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (KAFKA-2000) Delete consumer offsets from kafka once the topic is deleted

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2000?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15064564#comment-15064564
 ] 

Parth Brahmbhatt commented on KAFKA-2000:
-

Talked to [~harsha_ch] , I will submit an updated patch for this one today.

> Delete consumer offsets from kafka once the topic is deleted
> 
>
> Key: KAFKA-2000
> URL: https://issues.apache.org/jira/browse/KAFKA-2000
> Project: Kafka
>  Issue Type: Bug
>Reporter: Sriharsha Chintalapani
>Assignee: Sriharsha Chintalapani
>  Labels: newbie++
> Fix For: 0.9.1.0
>
> Attachments: KAFKA-2000.patch, KAFKA-2000_2015-05-03_10:39:11.patch
>
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Work started] (KAFKA-2547) Make DynamicConfigManager to use the ZkNodeChangeNotificationListener introduced as part of KAFKA-2211

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2547?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Work on KAFKA-2547 started by Parth Brahmbhatt.
---
> Make DynamicConfigManager to use the ZkNodeChangeNotificationListener 
> introduced as part of KAFKA-2211
> --
>
> Key: KAFKA-2547
> URL: https://issues.apache.org/jira/browse/KAFKA-2547
> Project: Kafka
>  Issue Type: Improvement
>    Reporter: Parth Brahmbhatt
>Assignee: Parth Brahmbhatt
>
> As part of KAFKA-2211 (https://github.com/apache/kafka/pull/195/files) we 
> introduced a reusable ZkNodeChangeNotificationListener to ensure node changes 
> can be processed in a loss less way. This was pretty much the same code in 
> DynamicConfigManager with little bit of refactoring so it can be reused. We 
> now need to make DynamicConfigManager itself to use this new class once 
> KAFKA-2211 is committed to avoid code duplication.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] kafka pull request: KAFKA-2547: Make DynamicConfigManager to use t...

[Parth-Brahmbhatt]

GitHub user Parth-Brahmbhatt opened a pull request:

https://github.com/apache/kafka/pull/679

KAFKA-2547: Make DynamicConfigManager to use the ZkNodeChangeNotifica…

…tionListener introduced as part of KAFKA-2211

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Parth-Brahmbhatt/kafka KAFKA-2547

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/679.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #679


commit 10071378d38bab1bcf00aefa0ef678fdb77c8f5b
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2015-12-15T21:15:22Z

KAFKA-2547: Make DynamicConfigManager to use the 
ZkNodeChangeNotificationListener introduced as part of KAFKA-2211




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (KAFKA-2794) Add group support for authorizer acls

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2794?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15056934#comment-15056934
 ] 

Parth Brahmbhatt commented on KAFKA-2794:
-

https://github.com/apache/kafka/pull/483

> Add group support for authorizer acls
> -
>
> Key: KAFKA-2794
> URL: https://issues.apache.org/jira/browse/KAFKA-2794
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Affects Versions: 0.9.0.0
>Reporter: Parth Brahmbhatt
>    Assignee: Parth Brahmbhatt
>
> Currently out of box kafka authorizer and ACLs only support single 
> principal/users. This is kind of a two part jira where we add support to 
> convert kerberos principal to unix user names and then from unix user name to 
> group. 
> With this feature , authorizer will be able to support Group level acls so 
> admins wont have to add support for individual users. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-2854) Make KerberosName implement PrincipalToLocal plugin so authorizer and authenticator can share this.

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2854?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15056930#comment-15056930
 ] 

Parth Brahmbhatt commented on KAFKA-2854:
-

[~ijuma][~harsha_ch] Can you review https://github.com/apache/kafka/pull/547

> Make KerberosName implement PrincipalToLocal plugin so authorizer and 
> authenticator can share this.
> ---
>
> Key: KAFKA-2854
> URL: https://issues.apache.org/jira/browse/KAFKA-2854
> Project: Kafka
>  Issue Type: Improvement
>Affects Versions: 0.9.0.0
>Reporter: Parth Brahmbhatt
>    Assignee: Parth Brahmbhatt
>
> Both Authorizer and SASL Authenticator needs a way to convert kerberos 
> principal into a local identity. This jira proposes to add an interface and a 
> config for this conversion so users can inject their own implementation. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Work started] (KAFKA-2854) Make KerberosName implement PrincipalToLocal plugin so authorizer and authenticator can share this.

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2854?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Work on KAFKA-2854 started by Parth Brahmbhatt.
---
> Make KerberosName implement PrincipalToLocal plugin so authorizer and 
> authenticator can share this.
> ---
>
> Key: KAFKA-2854
> URL: https://issues.apache.org/jira/browse/KAFKA-2854
> Project: Kafka
>  Issue Type: Improvement
>Affects Versions: 0.9.0.0
>Reporter: Parth Brahmbhatt
>    Assignee: Parth Brahmbhatt
>
> Both Authorizer and SASL Authenticator needs a way to convert kerberos 
> principal into a local identity. This jira proposes to add an interface and a 
> config for this conversion so users can inject their own implementation. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Work started] (KAFKA-2794) Add group support for authorizer acls

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2794?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Work on KAFKA-2794 started by Parth Brahmbhatt.
---
> Add group support for authorizer acls
> -
>
> Key: KAFKA-2794
> URL: https://issues.apache.org/jira/browse/KAFKA-2794
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Affects Versions: 0.9.0.0
>Reporter: Parth Brahmbhatt
>    Assignee: Parth Brahmbhatt
>
> Currently out of box kafka authorizer and ACLs only support single 
> principal/users. This is kind of a two part jira where we add support to 
> convert kerberos principal to unix user names and then from unix user name to 
> group. 
> With this feature , authorizer will be able to support Group level acls so 
> admins wont have to add support for individual users. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Created] (KAFKA-2852) Kafka Authroizer CLI should use consistent way to specify multiple values for all config options.

[Parth Brahmbhatt (JIRA)]

Parth Brahmbhatt created KAFKA-2852:
---

 Summary: Kafka Authroizer CLI should use consistent way to specify 
multiple values for all config options.
 Key: KAFKA-2852
 URL: https://issues.apache.org/jira/browse/KAFKA-2852
 Project: Kafka
  Issue Type: Bug
Reporter: Parth Brahmbhatt


Currently, AclCommand uses inconsistent conventions.

For principals, we take 
--allow-principal User:Bob --allow-principal User:Alice

For other options like hosts, we take
--allow-hosts Host1,Host2

We should have a consistent way to specify this.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] kafka pull request: Kafka-2852:Updating the Authorizer CLI to use ...

[Parth-Brahmbhatt]

GitHub user Parth-Brahmbhatt opened a pull request:

https://github.com/apache/kafka/pull/545

Kafka-2852:Updating the Authorizer CLI to use a consistent way to specify a 
list of values for a config options.

…ecify a list of values for a config options.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Parth-Brahmbhatt/kafka KAFKA-2852

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/545.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #545


commit 5f84fa80cd171f6e95e37a2a5fdfca42c98cd8a5
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2015-11-17T18:58:47Z

KAFKA-2852: Updating the Authorizer CLI to use a consistent way to specify 
a list of values for a config options.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] kafka pull request: Kafka-2854: Making KerberosShortNamer implemen...

[Parth-Brahmbhatt]

GitHub user Parth-Brahmbhatt opened a pull request:

https://github.com/apache/kafka/pull/547

Kafka-2854: Making KerberosShortNamer implement an interface and making it 
pluggable.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Parth-Brahmbhatt/kafka KAFKA-2854

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/547.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #547


commit 693598565a1fee0178ce9f432ec9799eba10df00
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2015-11-18T01:27:31Z

KAFKA-2854: Making KerberosShortNamer implement an interface and making it 
pluggable.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Created] (KAFKA-2854) Make KerberosName implement PrincipalToLocal plugin so authorizer and authenticator can share this.

[Parth Brahmbhatt (JIRA)]

Parth Brahmbhatt created KAFKA-2854:
---

 Summary: Make KerberosName implement PrincipalToLocal plugin so 
authorizer and authenticator can share this.
 Key: KAFKA-2854
 URL: https://issues.apache.org/jira/browse/KAFKA-2854
 Project: Kafka
  Issue Type: Improvement
Affects Versions: 0.9.0.0
Reporter: Parth Brahmbhatt
Assignee: Parth Brahmbhatt
 Fix For: 0.9.0.0


Both Authorizer and SASL Authenticator needs a way to convert kerberos 
principal into a local identity. This jira proposes to add an interface and a 
config for this conversion so users can inject their own implementation. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-2808) Support auto.create.topics.enable with automatic WRITE permissions for creator

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2808?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15000985#comment-15000985
 ] 

Parth Brahmbhatt commented on KAFKA-2808:
-

[~tgraves] your last comment is what I had in mind. Basically any time a topic 
is created , using CLI, AdminUtils or through auto create, in secure mode we 
should be able to derive the identity of the user who is creating the topic 
(from JAAS Login or if creation is  through auto create using the caller's 
session on server side) and assign him as the owner.  

Namespace can solve the problem and I believe 
https://cwiki.apache.org/confluence/display/KAFKA/KIP-37+-+Add+Namespaces+to+Kafka
 is addressing it.
I am assuming by WildCardTopics you mean something that supports regex, which 
wont be that different from namespacing it self.

> Support auto.create.topics.enable with automatic WRITE permissions for 
> creator 
> ---
>
> Key: KAFKA-2808
> URL: https://issues.apache.org/jira/browse/KAFKA-2808
> Project: Kafka
>  Issue Type: Improvement
>  Components: core
>Affects Versions: 0.9.0.0
>Reporter: Thomas Graves
>
> we have a user that wants to use the topic auto create functionality and 
> automatically have it give WRITE permissions so that they don't have to 
> explicitly create and grant acls ahead of time or make explicit call. 
> it seems like if you have auto.create.topics.enable enabled and the user has 
> CREATE acls we could automatically just give WRITE acls to the user who 
> creates the topic. Without that the auto create topics with acls doesn't add 
> much benefit.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-2808) Support auto.create.topics.enable with automatic WRITE permissions for creator

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2808?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15000865#comment-15000865
 ] 

Parth Brahmbhatt commented on KAFKA-2808:
-

Ideally we would always give admin access to a topic to the creator of the 
topic, problem is kafka does not have the concept of topic owner. I filed a 
jira to introduce the concept of topic owner 
https://issues.apache.org/jira/browse/KAFKA-2145 which should set the ground to 
support your request. However the PR is kind of stuck in review for more than 3 
months now so I don't expect this request to be fulfilled in the upcoming 
release.

> Support auto.create.topics.enable with automatic WRITE permissions for 
> creator 
> ---
>
> Key: KAFKA-2808
> URL: https://issues.apache.org/jira/browse/KAFKA-2808
> Project: Kafka
>  Issue Type: Improvement
>  Components: core
>Affects Versions: 0.9.0.0
>Reporter: Thomas Graves
>
> we have a user that wants to use the topic auto create functionality and 
> automatically have it give WRITE permissions so that they don't have to 
> explicitly create and grant acls ahead of time or make explicit call. 
> it seems like if you have auto.create.topics.enable enabled and the user has 
> CREATE acls we could automatically just give WRITE acls to the user who 
> creates the topic. Without that the auto create topics with acls doesn't add 
> much benefit.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-2788) allow comma when specifying principals in AclCommand

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14998905#comment-14998905
 ] 

Parth Brahmbhatt commented on KAFKA-2788:
-

I have started working on it, will send a PR sometime today.

> allow comma when specifying principals in AclCommand
> 
>
> Key: KAFKA-2788
> URL: https://issues.apache.org/jira/browse/KAFKA-2788
> Project: Kafka
>  Issue Type: Bug
>  Components: core
>Affects Versions: 0.9.0.0
>Reporter: Jun Rao
>    Assignee: Parth Brahmbhatt
>Priority: Blocker
> Fix For: 0.9.0.0
>
>
> Currently, comma doesn't seem to be allowed in AclCommand when specifying 
> allow-principals and deny-principals. However, when using ssl authentication, 
> by default, the client will look like the following and one can't pass that 
> in through AclCommand.
> "CN=writeuser,OU=Unknown,O=Unknown,L=Unknown,ST=Unknown,C=Unknown"



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] kafka pull request: KAFKA-2788: Allow specifying principals with c...

[Parth-Brahmbhatt]

GitHub user Parth-Brahmbhatt opened a pull request:

https://github.com/apache/kafka/pull/489

KAFKA-2788: Allow specifying principals with comman in ACL CLI.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Parth-Brahmbhatt/kafka KAFKA-2788

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/489.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #489


commit d94881e86096cfbc512b2bbc2ffdb5508cf4fb8e
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2015-11-10T21:13:07Z

KAFKA-2788: Allow specifying principals with comman in ACL CLI.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (KAFKA-2788) allow comma when specifying principals in AclCommand

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2788?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14998035#comment-14998035
 ] 

Parth Brahmbhatt commented on KAFKA-2788:
-

Looks like we already have a valid use case. I will add the support for it 
which will mean we will either have to change the separator(My Preferred way 
but inconsistent with all the other CLIs) or just stop supporting more than one 
principal in CLI(Users will probably hate this given they will have to script 
it up).

> allow comma when specifying principals in AclCommand
> 
>
> Key: KAFKA-2788
> URL: https://issues.apache.org/jira/browse/KAFKA-2788
> Project: Kafka
>  Issue Type: Bug
>  Components: core
>Affects Versions: 0.9.0.0
>Reporter: Jun Rao
>    Assignee: Parth Brahmbhatt
> Fix For: 0.9.0.0
>
>
> Currently, comma doesn't seem to be allowed in AclCommand when specifying 
> allow-principals and deny-principals. However, when using ssl authentication, 
> by default, the client will look like the following and one can't pass that 
> in through AclCommand.
> "CN=writeuser,OU=Unknown,O=Unknown,L=Unknown,ST=Unknown,C=Unknown"



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Created] (KAFKA-2794) Add group support for authorizer acls

[Parth Brahmbhatt (JIRA)]

Parth Brahmbhatt created KAFKA-2794:
---

 Summary: Add group support for authorizer acls
 Key: KAFKA-2794
 URL: https://issues.apache.org/jira/browse/KAFKA-2794
 Project: Kafka
  Issue Type: Sub-task
  Components: security
Affects Versions: 0.9.0.0
Reporter: Parth Brahmbhatt
Assignee: Parth Brahmbhatt


Currently out of box kafka authorizer and ACLs only support single 
principal/users. This is kind of a two part jira where we add support to 
convert kerberos principal to unix user names and then from unix user name to 
group. 

With this feature , authorizer will be able to support Group level acls so 
admins wont have to add support for individual users. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] kafka pull request: Kafka-2794: Added group support to authorizer.

[Parth-Brahmbhatt]

GitHub user Parth-Brahmbhatt opened a pull request:

https://github.com/apache/kafka/pull/483

Kafka-2794: Added group support to authorizer.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Parth-Brahmbhatt/kafka KAFKA-2794

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/483.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #483


commit b40118ab30170a9b060da15091900b09623396b6
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2015-11-03T22:49:20Z

Adding PrincipalToLocal mapping for backwards compatibility.

commit 46cd2505ec9d7cb39ea906f8a84ad5384c45a85a
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2015-11-10T06:18:34Z

KAFKA-2794: Add group support to Authorizer.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Assigned] (KAFKA-2754) Support defining ACLs at topic creation time

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2754?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Parth Brahmbhatt reassigned KAFKA-2754:
---

Assignee: Parth Brahmbhatt

> Support defining ACLs at topic creation time
> 
>
> Key: KAFKA-2754
> URL: https://issues.apache.org/jira/browse/KAFKA-2754
> Project: Kafka
>  Issue Type: Improvement
>Affects Versions: 0.9.0.0
>Reporter: Thomas Graves
>Assignee: Parth Brahmbhatt
>
> With a secured kafka cluster we want to be able have certain users create 
> topics and set the acls at the same time.   We have a use case where topics 
> will be dynamically created and we don't know the names ahead of time so it 
> would be really nice to create the topic and set the acls at the same time.  
> We want to be able to do this programmatically.
> Ideally it would be nice to have a way to set the acls with the auto create 
> topics enabled also but that might be a separate jira.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Assigned] (KAFKA-2700) delete topic should remove the corresponding ACL and configs

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2700?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Parth Brahmbhatt reassigned KAFKA-2700:
---

Assignee: Parth Brahmbhatt

> delete topic should remove the corresponding ACL and configs
> 
>
> Key: KAFKA-2700
> URL: https://issues.apache.org/jira/browse/KAFKA-2700
> Project: Kafka
>  Issue Type: Bug
>  Components: core
>Affects Versions: 0.9.0.0
>Reporter: Jun Rao
>    Assignee: Parth Brahmbhatt
>
> After a topic is successfully deleted, we should also remove any ACL, configs 
> and perhaps committed offsets associated with topic.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: [DISCUSS] KIP-38: ZooKeeper Authentication

[Parth Brahmbhatt]

I have 2 suggestions:

1) We need to document how does one move from secure to non secure
environment: 
1) change the config on all brokers to zookeeper.set.acl = false and do 
a
rolling upgrade.
2) Run the migration script with the jass config file so it is sasl
authenticated with zookeeper and change the acls on all subtrees back to
World modifiable.
3) remove the jaas config / or only the zookeeper section from the jaas,
and restart all brokers.

2) I am not sure if we should force users trying to move from unsecure to
secure environment to execute the migration script. In the second step
once the zookeeper.set.acl is set to true, we can secure all the subtrees
by calling ensureCorrectAcls as part of broker initialization (just after
makesurePersistentPathExists). Not sure why we want to add one more
manual/admin step when it can be automated. This also has the added
advantage that migration script will not have to take a flag as input to
figure out if it should set the acls to secure or unsecure given it will
always be used to move from secure to unsecure.

Given we are assuming all the information in zookeeper is world readable ,
I don¹t see SSL support as a must have or a blocker for this KIP.

Thanks
Parth



On 10/21/15, 9:56 AM, "Flavio Junqueira"  wrote:

>
>> On 21 Oct 2015, at 17:47, Todd Palino  wrote:
>> 
>> There seems to be a bit of detail lacking in the KIP. Specifically, I'd
>> like to understand:
>> 
>> 1) What znodes are the brokers going to secure? Is this configurable?
>>How?
>
>Currently it is securing all paths here except the consumers one:
>
>https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/utils
>/ZkUtils.scala#L56
>s/ZkUtils.scala#L56>
>
>This isn't configurable at the moment.
>
>> 2) What ACL is the broker going to apply? Is this configurable?
>
>The default is CREATOR_ALL_ACL + READ_ACL_UNSAFE, which means that an
>authenticated client can manipulate secured znodes and everyone can read
>znodes. The API of ZkUtils accommodates other ACLs, but the current code
>is using the default.
>
>> 3) How will the admin tools (such as preferred replica election and
>> partition reassignment) interact with this?
>> 
>
>Currently, you need to set a system property passing the login config
>file to be able to authenticate the client and perform writes to ZK.
>
>-Flavio
>
>> -Todd
>> 
>> 
>> On Wed, Oct 21, 2015 at 9:16 AM, Ismael Juma  wrote:
>> 
>>> On Wed, Oct 21, 2015 at 5:04 PM, Flavio Junqueira 
>>>wrote:
>>> 
 Bringing the points Grant brought to this thread:
 
> Is it worth mentioning the follow up steps that were discussed in the
>>> KIP
> call in this KIP document? Some of them were:
> 
>  - Adding SSL support for Zookeeper
>  - Removing the "world readable" assumption
> 
 
 Grant, how would you do it? I see three options:
 
 1- Add to the existing KIP, but then the functionality we should be
 checking in soon won't include it, so the KIP will remain incomplete
 
>>> 
>>> A "Future work" section would make sense to me, but I don't know how
>>>this
>>> is normally handled.
>>> 
>>> Ismael
>>> 
>

[jira] [Assigned] (KAFKA-2682) Authorization section in official docs

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2682?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Parth Brahmbhatt reassigned KAFKA-2682:
---

Assignee: Parth Brahmbhatt

> Authorization section in official docs
> --
>
> Key: KAFKA-2682
> URL: https://issues.apache.org/jira/browse/KAFKA-2682
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Ismael Juma
>Assignee: Parth Brahmbhatt
> Fix For: 0.9.0.0
>
>
> We need to add a section in the official documentation regarding 
> authorization:
> http://kafka.apache.org/documentation.html



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-2212) KafkaAuthorizer: Add CLI for Acl management.

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14959503#comment-14959503
 ] 

Parth Brahmbhatt commented on KAFKA-2212:
-

[~junrao] [~ijuma]

# we already have a PR for KAFKA-2598 https://github.com/apache/kafka/pull/300
# Here is the 
[https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Authorization+Command+Line+Interface
 wiki], please review and you can update the wiki or comment on this jira if 
you want me to add more details.
# As part of the PR for KAKFA-2598 , I have removed the unused import.


> KafkaAuthorizer: Add CLI for Acl management. 
> -
>
> Key: KAFKA-2212
> URL: https://issues.apache.org/jira/browse/KAFKA-2212
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>    Reporter: Parth Brahmbhatt
>Assignee: Parth Brahmbhatt
>Priority: Blocker
> Fix For: 0.9.0.0
>
> Attachments: KAFKA-2212.patch
>
>
> This is subtask-3 for Kafka-1688.
> Please see KIP-11 for details on CLI for Authorizer. 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] kafka pull request: KAFKA-2598: Adding integration test for the au...

[Parth-Brahmbhatt]

GitHub user Parth-Brahmbhatt opened a pull request:

https://github.com/apache/kafka/pull/300

KAFKA-2598: Adding integration test for the authorizer at API level. …

…Some bug fixes that I encountered while running the tests.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Parth-Brahmbhatt/kafka KAFKA-2598

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/300.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #300


commit 70d5b6a47085587fd5c30d3696e5dd1c6f740b19
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2015-10-12T22:46:59Z

KAFKA-2598: Adding integration test for the authorizer at API level. Some 
bug fixes that I encountered while running the tests.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (KAFKA-2598) Add Test with authorizer for producer and consumer

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2598?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14947417#comment-14947417
 ] 

Parth Brahmbhatt commented on KAFKA-2598:
-

I just started yesterday, should have an initial patch by EOD with producer and 
consumer. It is hard to write a test when we don't have miniKDC dependency as 
currently there is no way to establish identity and without multiple identities 
some test cases can not be written. 

I am trying to write the basic test cases for producer and consumer by granting 
access to "ANNONYMOUS" user to Write and Read which is the principal we get 
when no identity is established.

> Add Test with authorizer for producer and consumer
> --
>
> Key: KAFKA-2598
> URL: https://issues.apache.org/jira/browse/KAFKA-2598
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security, unit tests
>Affects Versions: 0.8.2.2
>Reporter: Parth Brahmbhatt
>Assignee: Parth Brahmbhatt
>Priority: Blocker
> Fix For: 0.9.0.0
>
>
> Now that we have all the authorizer code merged into trunk we should add a 
> test that enables authorizer and tests that only authorized users can 
> produce/consume from topics or issue cluster actions.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] kafka pull request: Kafka-2587: Only notification handler will upd...

[Parth-Brahmbhatt]

GitHub user Parth-Brahmbhatt opened a pull request:

https://github.com/apache/kafka/pull/277

Kafka-2587:  Only notification handler will update the cache and all 
verifications will use waitUntilTrue.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Parth-Brahmbhatt/kafka KAFKA-2587

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/277.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #277


commit bd2622297db321418e592db2edf642083d571bbb
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2015-09-29T02:09:03Z

KAFKA-2587:Increasing timeout for the test verification.

commit 736e1325201db750a378e59b6fea362ffeda8672
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2015-10-05T18:14:06Z

Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into 
KAFKA-2587

commit 4830bc82a40e9fa71304281fbf04ffec0ed49f1e
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2015-10-05T18:24:03Z

KAFKA-2587: Only notification handler will update the cache and all 
verifications will use waitUntilTrue.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (KAFKA-2587) Transient test failure: `SimpleAclAuthorizerTest`

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14943792#comment-14943792
 ] 

Parth Brahmbhatt commented on KAFKA-2587:
-

https://github.com/apache/kafka/pull/277/files

> Transient test failure: `SimpleAclAuthorizerTest`
> -
>
> Key: KAFKA-2587
> URL: https://issues.apache.org/jira/browse/KAFKA-2587
> Project: Kafka
>  Issue Type: Sub-task
>Reporter: Ismael Juma
>    Assignee: Parth Brahmbhatt
> Fix For: 0.9.0.0
>
>
> I've seen `SimpleAclAuthorizerTest ` fail a couple of times since its recent 
> introduction. Here's one such build:
> https://builds.apache.org/job/kafka-trunk-git-pr/576/console
> [~parth.brahmbhatt], can you please take a look and see if it's an easy fix?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-2587) Transient test failure: `SimpleAclAuthorizerTest`

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14941474#comment-14941474
 ] 

Parth Brahmbhatt commented on KAFKA-2587:
-

I looked at the code to reason around why this can happen. The state reported 
is indeed one of the valid states during our test 
https://github.com/apache/kafka/blob/5764e54de147af81aac85acc00687c23e9646a5c/core/src/test/scala/unit/kafka/security/auth/SimpleAclAuthorizerTest.scala#L217

After that line we actually remove all acls for that resource, add one acl back 
to it and remove that one acl. All those steps pass verification. 
https://github.com/apache/kafka/blob/5764e54de147af81aac85acc00687c23e9646a5c/core/src/test/scala/unit/kafka/security/auth/SimpleAclAuthorizerTest.scala#L225
 and 
https://github.com/apache/kafka/blob/5764e54de147af81aac85acc00687c23e9646a5c/core/src/test/scala/unit/kafka/security/auth/SimpleAclAuthorizerTest.scala#L226

Given we are using the same instance of the authorizer the cache of that 
instance is immediately updated for both add and remove. 
https://github.com/apache/kafka/blob/5764e54de147af81aac85acc00687c23e9646a5c/core/src/main/scala/kafka/security/auth/SimpleAclAuthorizer.scala#L171
https://github.com/apache/kafka/blob/5764e54de147af81aac85acc00687c23e9646a5c/core/src/main/scala/kafka/security/auth/SimpleAclAuthorizer.scala#L189

The only other place that can update the cache is notification handler as part 
of handling acl-changed notification. 
https://github.com/apache/kafka/blob/5764e54de147af81aac85acc00687c23e9646a5c/core/src/main/scala/kafka/security/auth/SimpleAclAuthorizer.scala#L269

However in that case we read the data from zookeeper and then update the cache. 
If the notifications processing was delayed for some reason, it should still 
read the acls from zk and then update the cache. 
There are pathological cases that can lead to this failure , for example:
1) Notification handler starts, reads acls from zk and a thread switch happens 
before it can update the cache
2) All the other cache updates go through (remove resource, add the acl, remove 
the acl). 
3) Before verification finishes for the last "remove one acl" a thread switch 
happens and notification handler update the cache with stale acls that it read 
before. 

Even with this case there should be follow up notifications about adding an acl 
and removing an acl which should again cause the notification process to read 
state from zookeeper and update the cache to correct state. Plus this seems 
unlikely enough that it would not happen every other day.

I will continue to look into this. In the meantime if this is a continuous dev 
pain, we can remove the last 3 lines of test that removes the last acl and 
tries to verify that the zookeeper path is deleted. 

> Transient test failure: `SimpleAclAuthorizerTest`
> -
>
> Key: KAFKA-2587
> URL: https://issues.apache.org/jira/browse/KAFKA-2587
> Project: Kafka
>  Issue Type: Sub-task
>Reporter: Ismael Juma
>    Assignee: Parth Brahmbhatt
> Fix For: 0.9.0.0
>
>
> I've seen `SimpleAclAuthorizerTest ` fail a couple of times since its recent 
> introduction. Here's one such build:
> https://builds.apache.org/job/kafka-trunk-git-pr/576/console
> [~parth.brahmbhatt], can you please take a look and see if it's an easy fix?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-2587) Transient test failure: `SimpleAclAuthorizerTest`

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14941533#comment-14941533
 ] 

Parth Brahmbhatt commented on KAFKA-2587:
-

Hey Ismael,

As mentioned above, in production given all notifications will be processed 
sequentially and as part of each notification we read the acls from zookeeper 
(source of truth) this issue should not happen in production as long as 
zookeeper guarantees read after write consistency. We may run into this issue 
in the case where we observe inconsistency due to zookeeper not supporting 
"Simultaneously Consistent Cross-Client Views". Please see Notes section in 
http://zookeeper.apache.org/doc/trunk/zookeeperProgrammers.html#ch_zkGuarantees.
 

I looked at the zkClient code to see if we can call sync before read to avoid 
any consistency concerns but zkClient does not support sync.

> Transient test failure: `SimpleAclAuthorizerTest`
> -
>
> Key: KAFKA-2587
> URL: https://issues.apache.org/jira/browse/KAFKA-2587
> Project: Kafka
>  Issue Type: Sub-task
>Reporter: Ismael Juma
>    Assignee: Parth Brahmbhatt
> Fix For: 0.9.0.0
>
>
> I've seen `SimpleAclAuthorizerTest ` fail a couple of times since its recent 
> introduction. Here's one such build:
> https://builds.apache.org/job/kafka-trunk-git-pr/576/console
> [~parth.brahmbhatt], can you please take a look and see if it's an easy fix?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-2598) Add Test with authorizer for producer and consumer

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2598?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14935589#comment-14935589
 ] 

Parth Brahmbhatt commented on KAFKA-2598:
-

I should be able to pick this one up next week.

> Add Test with authorizer for producer and consumer
> --
>
> Key: KAFKA-2598
> URL: https://issues.apache.org/jira/browse/KAFKA-2598
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security, unit tests
>Affects Versions: 0.8.2.2
>Reporter: Parth Brahmbhatt
>    Assignee: Parth Brahmbhatt
> Fix For: 0.9.0.0
>
>
> Now that we have all the authorizer code merged into trunk we should add a 
> test that enables authorizer and tests that only authorized users can 
> produce/consume from topics or issue cluster actions.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Created] (KAFKA-2598) Add Test with authorizer for producer and consumer

[Parth Brahmbhatt (JIRA)]

Parth Brahmbhatt created KAFKA-2598:
---

 Summary: Add Test with authorizer for producer and consumer
 Key: KAFKA-2598
 URL: https://issues.apache.org/jira/browse/KAFKA-2598
 Project: Kafka
  Issue Type: Bug
  Components: security, unit tests
Affects Versions: 0.8.2.2
Reporter: Parth Brahmbhatt
Assignee: Parth Brahmbhatt
 Fix For: 0.8.2.2


Now that we have all the authorizer code merged into trunk we should add a test 
that enables authorizer and tests that only authorized users can 
produce/consume from topics or issue cluster actions.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-2587) Transient test failure: `SimpleAclAuthorizerTest`

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14934498#comment-14934498
 ] 

Parth Brahmbhatt commented on KAFKA-2587:
-

[~ijuma] I tried to reproduce this locally by running the test in loop for 50 
times and it never failed. 

{code}
for i in `seq 1 50`; do ./gradlew clean core:test 
-Dtest.single=SimpleAclAuthorizerTest; done
{code}

Just analyzing the failure shows that the transient failure is reported during 
removal of the acl on this line. 
https://github.com/apache/kafka/blob/0990b6ba6a28276f79a1a8bfaf48455c6eddfa1f/core/src/test/scala/unit/kafka/security/auth/SimpleAclAuthorizerTest.scala#L190

Looking at the code given we update the local authorizer's cache immediately I 
am not entirely sure why would we see transient failures. I have modified the 
failure message to indicate a little bit more debug info and I have bumped up 
the timeout from 5 seconds to 10 seconds in case notification processing delay 
interleaved with the changes made to cache is somehow resulting in this failure.
 
{code}
kafka.security.auth.SimpleAclAuthorizerTest > testAclManagementAPIs FAILED
java.lang.AssertionError: changes not propagated in timeout period.
at org.junit.Assert.fail(Assert.java:88)
at kafka.utils.TestUtils$.waitUntilTrue(TestUtils.scala:655)
at 
kafka.security.auth.SimpleAclAuthorizerTest.changeAclAndVerify(SimpleAclAuthorizerTest.scala:229)
at 
kafka.security.auth.SimpleAclAuthorizerTest.testAclManagementAPIs(SimpleAclAuthorizerTest.scala:190)
{code}

> Transient test failure: `SimpleAclAuthorizerTest`
> -
>
> Key: KAFKA-2587
> URL: https://issues.apache.org/jira/browse/KAFKA-2587
> Project: Kafka
>  Issue Type: Sub-task
>Reporter: Ismael Juma
>Assignee: Parth Brahmbhatt
>
> I've seen `SimpleAclAuthorizerTest ` fail a couple of times since its recent 
> introduction. Here's one such build:
> https://builds.apache.org/job/kafka-trunk-git-pr/576/console
> [~parth.brahmbhatt], can you please take a look and see if it's an easy fix?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Work started] (KAFKA-2587) Transient test failure: `SimpleAclAuthorizerTest`

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2587?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Work on KAFKA-2587 started by Parth Brahmbhatt.
---
> Transient test failure: `SimpleAclAuthorizerTest`
> -
>
> Key: KAFKA-2587
> URL: https://issues.apache.org/jira/browse/KAFKA-2587
> Project: Kafka
>  Issue Type: Sub-task
>Reporter: Ismael Juma
>    Assignee: Parth Brahmbhatt
>
> I've seen `SimpleAclAuthorizerTest ` fail a couple of times since its recent 
> introduction. Here's one such build:
> https://builds.apache.org/job/kafka-trunk-git-pr/576/console
> [~parth.brahmbhatt], can you please take a look and see if it's an easy fix?



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] kafka pull request: KAFKA-2587:Increasing timeout for the test ver...

[Parth-Brahmbhatt]

GitHub user Parth-Brahmbhatt opened a pull request:

https://github.com/apache/kafka/pull/260

KAFKA-2587:Increasing timeout for the test verification.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Parth-Brahmbhatt/kafka KAFKA-2587

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/260.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #260


commit bd2622297db321418e592db2edf642083d571bbb
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2015-09-29T02:09:03Z

KAFKA-2587:Increasing timeout for the test verification.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] kafka pull request: KAFKA-2212: Authorizer CLI implementation.

[Parth-Brahmbhatt]

GitHub user Parth-Brahmbhatt opened a pull request:

https://github.com/apache/kafka/pull/230

KAFKA-2212: Authorizer CLI implementation.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Parth-Brahmbhatt/kafka KAFKA-2212

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/230.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #230


commit c3f3ff0aed93175c115e300b9ba76d25ad9063bc
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2015-09-22T20:42:09Z

KAFKA-2212: Authorizer CLI implementation.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Updated] (KAFKA-2212) KafkaAuthorizer: Add CLI for Acl management.

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2212?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Parth Brahmbhatt updated KAFKA-2212:

Status: In Progress  (was: Patch Available)

> KafkaAuthorizer: Add CLI for Acl management. 
> -
>
> Key: KAFKA-2212
> URL: https://issues.apache.org/jira/browse/KAFKA-2212
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>    Reporter: Parth Brahmbhatt
>Assignee: Parth Brahmbhatt
>Priority: Blocker
> Fix For: 0.9.0.0
>
> Attachments: KAFKA-2212.patch
>
>
> This is subtask-3 for Kafka-1688.
> Please see KIP-11 for details on CLI for Authorizer. 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] kafka pull request: KAFKA-2549: Fixing checkstyle failure resultin...

[Parth-Brahmbhatt]

GitHub user Parth-Brahmbhatt opened a pull request:

https://github.com/apache/kafka/pull/215

KAFKA-2549: Fixing checkstyle failure resulting due to unused imports…

… in Selector.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Parth-Brahmbhatt/kafka KAFKA-2549

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/215.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #215


commit 510b4ec6ef327448d4825acd84d24486054bc9ca
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2015-09-14T21:44:18Z

KAFKA-2549: Fixing checkstyle failure resulting due to unused imports in 
Selector.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Commented] (KAFKA-2549) Checkstyle reporting failure in trunk due to unused imports in Selector.java

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14744368#comment-14744368
 ] 

Parth Brahmbhatt commented on KAFKA-2549:
-

[~ijuma] You are right, my bad, edited the description.

> Checkstyle reporting failure in trunk due to unused imports in Selector.java
> 
>
> Key: KAFKA-2549
> URL: https://issues.apache.org/jira/browse/KAFKA-2549
> Project: Kafka
>  Issue Type: Bug
>    Reporter: Parth Brahmbhatt
>    Assignee: Parth Brahmbhatt
>Priority: Blocker
>
> Introduced in 
> https://github.com/apache/kafka/commit/d02ca36ca1cccdb6962191b97f54ce96b9d75abc#diff-db8f8be6ef2f1c81515d1ed83b3ab107
>  in which the Selector.java was modified with some unused imports so the 
> trunk can not execute test targets as it fails in client section during 
> checkstyle stage.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Updated] (KAFKA-2549) Checkstyle reporting failure in trunk due to unused imports in Selector.java

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2549?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Parth Brahmbhatt updated KAFKA-2549:

Description: Introduced in 
https://github.com/apache/kafka/commit/d02ca36ca1cccdb6962191b97f54ce96b9d75abc#diff-db8f8be6ef2f1c81515d1ed83b3ab107
 in which the Selector.java was modified with some unused imports so the trunk 
can not execute test targets as it fails in client section during checkstyle 
stage.  (was: Again introduced in 
https://github.com/apache/kafka/commit/d02ca36ca1cccdb6962191b97f54ce96b9d75abc#diff-db8f8be6ef2f1c81515d1ed83b3ab107
 in which the Selector.java was modified with some unused imports so the trunk 
can not execute test targets as it fails in client section during checkstyle 
stage.)

> Checkstyle reporting failure in trunk due to unused imports in Selector.java
> 
>
> Key: KAFKA-2549
> URL: https://issues.apache.org/jira/browse/KAFKA-2549
> Project: Kafka
>  Issue Type: Bug
>    Reporter: Parth Brahmbhatt
>    Assignee: Parth Brahmbhatt
>Priority: Blocker
>
> Introduced in 
> https://github.com/apache/kafka/commit/d02ca36ca1cccdb6962191b97f54ce96b9d75abc#diff-db8f8be6ef2f1c81515d1ed83b3ab107
>  in which the Selector.java was modified with some unused imports so the 
> trunk can not execute test targets as it fails in client section during 
> checkstyle stage.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Created] (KAFKA-2547) Make DynamicConfigManager to use the ZkNodeChangeNotificationListener introduced as part of KAFKA-2211

[Parth Brahmbhatt (JIRA)]

Parth Brahmbhatt created KAFKA-2547:
---

 Summary: Make DynamicConfigManager to use the 
ZkNodeChangeNotificationListener introduced as part of KAFKA-2211
 Key: KAFKA-2547
 URL: https://issues.apache.org/jira/browse/KAFKA-2547
 Project: Kafka
  Issue Type: Improvement
Reporter: Parth Brahmbhatt
Assignee: Parth Brahmbhatt


As part of KAFKA-2211 (https://github.com/apache/kafka/pull/195/files) we 
introduced a reusable ZkNodeChangeNotificationListener to ensure node changes 
can be processed in a loss less way. This was pretty much the same code in 
DynamicConfigManager with little bit of refactoring so it can be reused. We now 
need to make DynamicConfigManager itself to use this new class once KAFKA-2211 
is committed to avoid code duplication.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Assigned] (KAFKA-2538) Compilation in trunk is failing due to https://github.com/apache/kafka/commit/845514d62329be8382e6d02b8041fc858718d534

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2538?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Parth Brahmbhatt reassigned KAFKA-2538:
---

Assignee: Parth Brahmbhatt

> Compilation in trunk is failing due to 
> https://github.com/apache/kafka/commit/845514d62329be8382e6d02b8041fc858718d534
> --
>
> Key: KAFKA-2538
> URL: https://issues.apache.org/jira/browse/KAFKA-2538
> Project: Kafka
>  Issue Type: Bug
>    Reporter: Parth Brahmbhatt
>Assignee: Parth Brahmbhatt
>Priority: Blocker
>
> Getting 
> /Users/pbrahmbhatt/repo/kafka/core/src/main/scala/kafka/tools/EndToEndLatency.scala:82:
>  value commit is not a member of 
> org.apache.kafka.clients.consumer.KafkaConsumer[Array[Byte],Array[Byte]]
>   consumer.commit(CommitType.SYNC)
>^
> Which I believe was missed when committing KAFKA-2389 which replaces all 
> occurrences of commit(mode) with commit(Sync/Async). This is resulting in 
> other PRS reporting as bad by jenkins like 
> https://github.com/apache/kafka/pull/195 where 2 failures were reported by 
> jenkins https://builds.apache.org/job/kafka-trunk-git-pr/410/ and 
> https://builds.apache.org/job/kafka-trunk-git-pr/411/ 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Created] (KAFKA-2538) Compilation in trunk is failing due to https://github.com/apache/kafka/commit/845514d62329be8382e6d02b8041fc858718d534

[Parth Brahmbhatt (JIRA)]

Parth Brahmbhatt created KAFKA-2538:
---

 Summary: Compilation in trunk is failing due to 
https://github.com/apache/kafka/commit/845514d62329be8382e6d02b8041fc858718d534
 Key: KAFKA-2538
 URL: https://issues.apache.org/jira/browse/KAFKA-2538
 Project: Kafka
  Issue Type: Bug
Reporter: Parth Brahmbhatt
Priority: Blocker


Getting 
/Users/pbrahmbhatt/repo/kafka/core/src/main/scala/kafka/tools/EndToEndLatency.scala:82:
 value commit is not a member of 
org.apache.kafka.clients.consumer.KafkaConsumer[Array[Byte],Array[Byte]]
  consumer.commit(CommitType.SYNC)
   ^

Which I believe was missed when committing KAFKA-2389 which replaces all 
occurrences of commit(mode) with commit(Sync/Async). This is resulting in other 
PRS reporting as bad by jenkins like https://github.com/apache/kafka/pull/195 
where 2 failures were reported by jenkins 
https://builds.apache.org/job/kafka-trunk-git-pr/410/ and 
https://builds.apache.org/job/kafka-trunk-git-pr/411/ 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] kafka pull request: KAFKA-2538: Fixing a compilation error in trun...

[Parth-Brahmbhatt]

GitHub user Parth-Brahmbhatt opened a pull request:

https://github.com/apache/kafka/pull/208

KAFKA-2538: Fixing a compilation error in trunk.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Parth-Brahmbhatt/kafka KAFKA-2538

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/208.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #208


commit 24b4e01c251c51e0896887dbf65d92d101750b45
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2015-09-12T02:33:08Z

KAFKA-2538: Fixing a compilation error in trunk.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[jira] [Work started] (KAFKA-2211) KafkaAuthorizer: Add simpleACLAuthorizer implementation.

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2211?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Work on KAFKA-2211 started by Parth Brahmbhatt.
---
> KafkaAuthorizer: Add simpleACLAuthorizer implementation.
> 
>
> Key: KAFKA-2211
> URL: https://issues.apache.org/jira/browse/KAFKA-2211
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>    Reporter: Parth Brahmbhatt
>Assignee: Parth Brahmbhatt
>Priority: Blocker
> Fix For: 0.8.3
>
> Attachments: KAFKA-2211.patch
>
>
> Subtask-2 for Kafka-1688. 
> Please see KIP-11 to get details on out of box SimpleACLAuthorizer 
> implementation 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Updated] (KAFKA-2211) KafkaAuthorizer: Add simpleACLAuthorizer implementation.

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2211?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Parth Brahmbhatt updated KAFKA-2211:

Status: Patch Available  (was: In Progress)

> KafkaAuthorizer: Add simpleACLAuthorizer implementation.
> 
>
> Key: KAFKA-2211
> URL: https://issues.apache.org/jira/browse/KAFKA-2211
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>    Reporter: Parth Brahmbhatt
>Assignee: Parth Brahmbhatt
>Priority: Blocker
> Fix For: 0.8.3
>
> Attachments: KAFKA-2211.patch
>
>
> Subtask-2 for Kafka-1688. 
> Please see KIP-11 to get details on out of box SimpleACLAuthorizer 
> implementation 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[GitHub] kafka pull request: KAFKA-2211: Adding simpleAclAuthorizer impleme...

[Parth-Brahmbhatt]

GitHub user Parth-Brahmbhatt opened a pull request:

https://github.com/apache/kafka/pull/195

KAFKA-2211: Adding simpleAclAuthorizer implementation and test cases.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/Parth-Brahmbhatt/kafka KAFKA-2211

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/kafka/pull/195.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #195


commit 7be79e12a4106f7c9963e5cd6fcc1d5438f60e46
Author: Parth Brahmbhatt <brahmbhatt.pa...@gmail.com>
Date:   2015-09-04T22:21:06Z

KAFKA-2211: Adding simpleAclAuthorizer implementation and test cases.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Re: Review Request 34493: Patch for KAFKA-2211

[Parth Brahmbhatt]

> On Aug. 21, 2015, 2:31 p.m., Ismael Juma wrote:
> > Thanks for this Parth. I did an initial pass where I left a number comments 
> > (many of them style-related, see http://kafka.apache.org/coding-guide.html 
> > for reference). I know, we should have a tool that checks some of these 
> > things automatically. That is my main priority after we get the security 
> > stuff in shape.
> > 
> > I think it would be useful if you had a look and made the changes (if you 
> > agree) to the cases I pointed out and similar ones. I noticed that 
> > KAFKA-2212 has some similar issues too, it may be worth taking a pass there 
> > too.
> > 
> > I will look at these two patches again early next week.
> 
> Parth Brahmbhatt wrote:
> Hey , thanks for reviewing this. This patch needs to be updated with all 
> the changes that we have made in 2210. As 2210 was moving slow and was kind 
> of a moving target I did not update this patch. I think I have gained little 
> more understanding around scala styles from the 2210 review so I will 
> incorporate all those in this patch.
> 
> Ismael Juma wrote:
> Thank you Parth!

Is it ok if I post a pull request with the new changes instead of using review 
board?


> On Aug. 21, 2015, 2:31 p.m., Ismael Juma wrote:
> > core/src/main/scala/kafka/security/auth/SimpleAclAuthorizer.scala, line 118
> > <https://reviews.apache.org/r/34493/diff/1/?file=965666#file965666line118>
> >
> > In Scala, `return` is normally avoided because it makes it harder to 
> > reason about some code in isolation. Could this code be rewritten in a more 
> > readable way without using `return`? The answer may well be no, interested 
> > in your thoughts.

I think I have addressed all your comments but this one. This seems one of 
those cases where the code will be more redable with return statement given 
there are so many different checks and at end of each of those checks we might 
have found the final result.


- Parth


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34493/#review96042
---


On May 20, 2015, 8:03 p.m., Parth Brahmbhatt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34493/
> ---
> 
> (Updated May 20, 2015, 8:03 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2211
> https://issues.apache.org/jira/browse/KAFKA-2211
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> KAFKA-2211: Out of box implementation for authorizer interface.
> 
> 
> Diffs
> -
> 
>   core/src/main/scala/kafka/security/auth/SimpleAclAuthorizer.scala 
> PRE-CREATION 
>   core/src/test/resources/authorizer-config.properties PRE-CREATION 
>   core/src/test/scala/unit/kafka/security/auth/SimpleAclAuthorizerTest.scala 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/34493/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Parth Brahmbhatt
> 
>

Re: Review Request 34493: Patch for KAFKA-2211

[Parth Brahmbhatt]

> On Aug. 23, 2015, 9:28 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/security/auth/SimpleAclAuthorizer.scala, lines 
> > 55-57
> > <https://reviews.apache.org/r/34493/diff/1/?file=965666#file965666line55>
> >
> > To be consistent with how we pass in configs for pluggable components, 
> > we need to get the external properties directly through 
> > KafkaConfig.originals() instead of from an external property file.

Again, all this code is too old. We now have Authorizer extending configurable 
so we dont get KafkaConfig instance here. We actually get the original 
properties map and we initiliaze the variables from that map. We dont expect a 
new properties file anymore.


> On Aug. 23, 2015, 9:28 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/security/auth/SimpleAclAuthorizer.scala, lines 
> > 66-68
> > <https://reviews.apache.org/r/34493/diff/1/?file=965666#file965666line66>
> >
> > Perhaps this can be moved to ZkUtils.setupCommonPaths()?

This path is kind of optional as we only need it if someone wants to use 
authorizer + if they want to use our authorizer. I think creating this path as 
part of commonPaths will be confusing.


> On Aug. 23, 2015, 9:28 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/security/auth/SimpleAclAuthorizer.scala, lines 
> > 70-72
> > <https://reviews.apache.org/r/34493/diff/1/?file=965666#file965666line70>
> >
> > Hmm, not sure why we need the scheduler. It seems that it's enough to 
> > just read every acl from ZK first on initialization. That's how 
> > topic/client config changes works. What's the re-connection issue that you 
> > mentioned?

Please read http://zookeeper.apache.org/doc/trunk/zookeeperProgrammers.html 
section "Things to Remember about Watches". Bottomline is there are 
pathological cases where a watcher can get missed. All I really need is set a 
TTL On cache entry, I could add that or keep the scheduler so it expires all 
cache entries every hour.


> On Aug. 23, 2015, 9:28 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/security/auth/SimpleAclAuthorizer.scala, lines 
> > 110-111
> > <https://reviews.apache.org/r/34493/diff/1/?file=965666#file965666line110>
> >
> > Is this right? It seems the expansion should happen on the acl side. 
> > Basically, if you configure an ACL to read from a topic, you automatically 
> > get an ACL to describe the topic.

I am not sure what you mean by "the expansion should happen on the acl side". 
Do you mean we should add the describe acl when someone adds an acl for read or 
write?  That does not seem right because in that case when they remove the READ 
or WRITE acl, do we also remove describe acl? or removal will have to be 
explicit? Seems confusing. I think the current way is cleaner.


> On Aug. 23, 2015, 9:28 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/security/auth/SimpleAclAuthorizer.scala, line 134
> > <https://reviews.apache.org/r/34493/diff/1/?file=965666#file965666line134>
> >
> > Hmm, the acl should only match if operation is a subset of 
> > acl.operations, right?

again code has changed quite a bit given acl only stores one operation now. 

The original code was doing the right thing as the list only had > 1 element in 
describe case and in that case the match method was checking if the 
acl.oprations had atleast one of the 3 DESCRIBE,READ or WRITE.


> On Aug. 23, 2015, 9:28 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/security/auth/SimpleAclAuthorizer.scala, lines 
> > 220-227
> > <https://reviews.apache.org/r/34493/diff/1/?file=965666#file965666line220>
> >
> > Not sure why we need to read from ZK during reads since writes should 
> > only be propagated from the ZK listner. You can take a look at how 
> > ConfigChangeListener is implemented.

Again my understanding of how zookeeper watchers work with zkClient is that it 
is possible to miss watchers so I don't completely rely on the watchers always 
firing. If that understading is incorrect i can remove the scheduler thread and 
change the code to only read from zookeeper as part of handling the watcher 
notification.


- Parth


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34493/#review96128
---


On May 20, 2015, 8:03 p.m., Parth Brahmbhatt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34493/
> ---
> 
> (Updated May 20, 2015, 8:03 p.m.)
&

[jira] [Updated] (KAFKA-2210) KafkaAuthorizer: Add all public entities, config changes and changes to KafkaAPI and kafkaServer to allow pluggable authorizer implementation.

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2210?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Parth Brahmbhatt updated KAFKA-2210:

Attachment: KAFKA-2210_2015-09-02_17:32:06.patch

> KafkaAuthorizer: Add all public entities, config changes and changes to 
> KafkaAPI and kafkaServer to allow pluggable authorizer implementation.
> --
>
> Key: KAFKA-2210
> URL: https://issues.apache.org/jira/browse/KAFKA-2210
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Parth Brahmbhatt
>    Assignee: Parth Brahmbhatt
>Priority: Blocker
> Fix For: 0.8.3
>
> Attachments: KAFKA-2210.patch, KAFKA-2210_2015-06-03_16:36:11.patch, 
> KAFKA-2210_2015-06-04_16:07:39.patch, KAFKA-2210_2015-07-09_18:00:34.patch, 
> KAFKA-2210_2015-07-14_10:02:19.patch, KAFKA-2210_2015-07-14_14:13:19.patch, 
> KAFKA-2210_2015-07-20_16:42:18.patch, KAFKA-2210_2015-07-21_17:08:21.patch, 
> KAFKA-2210_2015-08-10_18:31:54.patch, KAFKA-2210_2015-08-20_11:27:18.patch, 
> KAFKA-2210_2015-08-25_17:59:22.patch, KAFKA-2210_2015-08-26_14:29:02.patch, 
> KAFKA-2210_2015-09-01_15:36:02.patch, KAFKA-2210_2015-09-02_14:50:29.patch, 
> KAFKA-2210_2015-09-02_17:32:06.patch
>
>
> This is the first subtask for Kafka-1688. As Part of this jira we intend to 
> agree on all the public entities, configs and changes to existing kafka 
> classes to allow pluggable authorizer implementation.
> Please see KIP-11 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface
>  for detailed design. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-2210) KafkaAuthorizer: Add all public entities, config changes and changes to KafkaAPI and kafkaServer to allow pluggable authorizer implementation.

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14728293#comment-14728293
 ] 

Parth Brahmbhatt commented on KAFKA-2210:
-

Updated reviewboard https://reviews.apache.org/r/34492/diff/
 against branch origin/trunk

> KafkaAuthorizer: Add all public entities, config changes and changes to 
> KafkaAPI and kafkaServer to allow pluggable authorizer implementation.
> --
>
> Key: KAFKA-2210
> URL: https://issues.apache.org/jira/browse/KAFKA-2210
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Parth Brahmbhatt
>    Assignee: Parth Brahmbhatt
>Priority: Blocker
> Fix For: 0.8.3
>
> Attachments: KAFKA-2210.patch, KAFKA-2210_2015-06-03_16:36:11.patch, 
> KAFKA-2210_2015-06-04_16:07:39.patch, KAFKA-2210_2015-07-09_18:00:34.patch, 
> KAFKA-2210_2015-07-14_10:02:19.patch, KAFKA-2210_2015-07-14_14:13:19.patch, 
> KAFKA-2210_2015-07-20_16:42:18.patch, KAFKA-2210_2015-07-21_17:08:21.patch, 
> KAFKA-2210_2015-08-10_18:31:54.patch, KAFKA-2210_2015-08-20_11:27:18.patch, 
> KAFKA-2210_2015-08-25_17:59:22.patch, KAFKA-2210_2015-08-26_14:29:02.patch, 
> KAFKA-2210_2015-09-01_15:36:02.patch, KAFKA-2210_2015-09-02_14:50:29.patch, 
> KAFKA-2210_2015-09-02_17:32:06.patch
>
>
> This is the first subtask for Kafka-1688. As Part of this jira we intend to 
> agree on all the public entities, configs and changes to existing kafka 
> classes to allow pluggable authorizer implementation.
> Please see KIP-11 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface
>  for detailed design. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: Review Request 34492: Patch for KAFKA-2210

[Parth Brahmbhatt]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34492/
---

(Updated Sept. 3, 2015, 12:32 a.m.)


Review request for kafka.


Bugs: KAFKA-2210
https://issues.apache.org/jira/browse/KAFKA-2210


Repository: kafka


Description (updated)
---

Addressing review comments from Jun.


Adding CREATE check for offset topic only if the topic does not exist already.


Addressing some more comments.


Removing acl.json file


Moving PermissionType to trait instead of enum. Following the convention for 
defining constants.


Adding authorizer.config.path back.


Addressing more comments from Jun.


Addressing more comments.


Now addressing Ismael's comments. Case sensitive checks.


Addressing Jun's comments.


Merge remote-tracking branch 'origin/trunk' into az

Conflicts:
core/src/main/scala/kafka/server/KafkaApis.scala
core/src/main/scala/kafka/server/KafkaServer.scala

Deleting KafkaConfigDefTest


Addressing comments from Ismael.


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az


Consolidating KafkaPrincipal.


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az

Conflicts:

clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java

clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
core/src/main/scala/kafka/server/KafkaApis.scala

Making Acl structure take only one principal, operation and host.


Merge remote-tracking branch 'origin/trunk' into az


Reverting uninteded new line change.


Addressing comments from Jun.


Merge remote-tracking branch 'origin/trunk' into az


Various tweaks that make the code more readable

Conflicts:
core/src/main/scala/kafka/server/KafkaApis.scala

Fixing compilation errors after cherry-pocking.


Diffs (updated)
-

  
clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java
 35d41685dd178bbdf77b2476e03ad51fc4adcbb6 
  clients/src/main/java/org/apache/kafka/common/protocol/Errors.java 
641afa1b2474150fa1002e9fedca13ff55175a7e 
  
clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java 
b640ea0f4bdb694fc5524ef594aa125cc1ba4cf3 
  
clients/src/test/java/org/apache/kafka/common/security/auth/KafkaPrincipalTest.java
 PRE-CREATION 
  core/src/main/scala/kafka/api/OffsetRequest.scala 
f418868046f7c99aefdccd9956541a0cb72b1500 
  core/src/main/scala/kafka/common/AuthorizationException.scala PRE-CREATION 
  core/src/main/scala/kafka/common/ErrorMapping.scala 
c75c68589681b2c9d6eba2b440ce5e58cddf6370 
  core/src/main/scala/kafka/security/auth/Acl.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Authorizer.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Operation.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/PermissionType.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Resource.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/ResourceType.scala PRE-CREATION 
  core/src/main/scala/kafka/server/KafkaApis.scala 
a3a8df0545c3f9390e0e04b8d2fab0134f5fd019 
  core/src/main/scala/kafka/server/KafkaConfig.scala 
d547a01cf7098f216a3775e1e1901c5794e1b24c 
  core/src/main/scala/kafka/server/KafkaServer.scala 
756cf775cadbcaf01df7f691d8d01d9ff75db291 
  core/src/test/scala/unit/kafka/security/auth/AclTest.scala PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/OperationTest.scala PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/PermissionTypeTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/ResourceTypeTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 
3da666f73227fc7ef7093e3790546344065f6825 

Diff: https://reviews.apache.org/r/34492/diff/


Testing
---


Thanks,

Parth Brahmbhatt

Re: Review Request 34492: Patch for KAFKA-2210

[Parth Brahmbhatt]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34492/
---

(Updated Sept. 3, 2015, 12:36 a.m.)


Review request for kafka.


Bugs: KAFKA-2210
https://issues.apache.org/jira/browse/KAFKA-2210


Repository: kafka


Description (updated)
---

Addressing review comments from Jun.


Adding CREATE check for offset topic only if the topic does not exist already.


Addressing some more comments.


Removing acl.json file


Moving PermissionType to trait instead of enum. Following the convention for 
defining constants.


Adding authorizer.config.path back.


Addressing more comments from Jun.


Addressing more comments.


Now addressing Ismael's comments. Case sensitive checks.


Addressing Jun's comments.


Merge remote-tracking branch 'origin/trunk' into az

Conflicts:
core/src/main/scala/kafka/server/KafkaApis.scala
core/src/main/scala/kafka/server/KafkaServer.scala

Deleting KafkaConfigDefTest


Addressing comments from Ismael.


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az


Consolidating KafkaPrincipal.


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az

Conflicts:

clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java

clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
core/src/main/scala/kafka/server/KafkaApis.scala

Making Acl structure take only one principal, operation and host.


Merge remote-tracking branch 'origin/trunk' into az


Reverting uninteded new line change.


Addressing comments from Jun.


Merge remote-tracking branch 'origin/trunk' into az


Various tweaks that make the code more readable

Conflicts:
core/src/main/scala/kafka/server/KafkaApis.scala

Fixing compilation errors after cherry-pocking.


Removing FIXME.


Diffs (updated)
-

  
clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java
 35d41685dd178bbdf77b2476e03ad51fc4adcbb6 
  clients/src/main/java/org/apache/kafka/common/protocol/Errors.java 
641afa1b2474150fa1002e9fedca13ff55175a7e 
  
clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java 
b640ea0f4bdb694fc5524ef594aa125cc1ba4cf3 
  
clients/src/test/java/org/apache/kafka/common/security/auth/KafkaPrincipalTest.java
 PRE-CREATION 
  core/src/main/scala/kafka/api/OffsetRequest.scala 
f418868046f7c99aefdccd9956541a0cb72b1500 
  core/src/main/scala/kafka/common/AuthorizationException.scala PRE-CREATION 
  core/src/main/scala/kafka/common/ErrorMapping.scala 
c75c68589681b2c9d6eba2b440ce5e58cddf6370 
  core/src/main/scala/kafka/security/auth/Acl.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Authorizer.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Operation.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/PermissionType.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Resource.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/ResourceType.scala PRE-CREATION 
  core/src/main/scala/kafka/server/KafkaApis.scala 
a3a8df0545c3f9390e0e04b8d2fab0134f5fd019 
  core/src/main/scala/kafka/server/KafkaConfig.scala 
d547a01cf7098f216a3775e1e1901c5794e1b24c 
  core/src/main/scala/kafka/server/KafkaServer.scala 
756cf775cadbcaf01df7f691d8d01d9ff75db291 
  core/src/test/scala/unit/kafka/security/auth/AclTest.scala PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/OperationTest.scala PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/PermissionTypeTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/ResourceTypeTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 
3da666f73227fc7ef7093e3790546344065f6825 

Diff: https://reviews.apache.org/r/34492/diff/


Testing
---


Thanks,

Parth Brahmbhatt

Re: Review Request 34492: Patch for KAFKA-2210

[Parth Brahmbhatt]

> On Sept. 2, 2015, 4:36 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/common/ErrorMapping.scala, lines 55-57
> > <https://reviews.apache.org/r/34492/diff/14/?file=1061760#file1061760line55>
> >
> > Could you add the missing error cords 23-28 in the comment?

copied the error constant from Error.java and added in comments. I am not sure 
if you already have a jira to actually fix this. If you do can you assign that 
to me? If you don't have a jira yet can I create one and assign it to myself?


> On Sept. 2, 2015, 4:36 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/KafkaApis.scala, line 680
> > <https://reviews.apache.org/r/34492/diff/14/?file=1061767#file1061767line680>
> >
> > Unused val?

unused call given we agreed for offset topic creation we are not going to check 
for authorization.


> On Sept. 2, 2015, 4:36 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/server/KafkaServer.scala, line 187
> > <https://reviews.apache.org/r/34492/diff/14/?file=1061769#file1061769line187>
> >
> > space after if

fixed.


- Parth


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34492/#review97466
---


On Sept. 3, 2015, 12:36 a.m., Parth Brahmbhatt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34492/
> ---
> 
> (Updated Sept. 3, 2015, 12:36 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2210
> https://issues.apache.org/jira/browse/KAFKA-2210
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Addressing review comments from Jun.
> 
> 
> Adding CREATE check for offset topic only if the topic does not exist already.
> 
> 
> Addressing some more comments.
> 
> 
> Removing acl.json file
> 
> 
> Moving PermissionType to trait instead of enum. Following the convention for 
> defining constants.
> 
> 
> Adding authorizer.config.path back.
> 
> 
> Addressing more comments from Jun.
> 
> 
> Addressing more comments.
> 
> 
> Now addressing Ismael's comments. Case sensitive checks.
> 
> 
> Addressing Jun's comments.
> 
> 
> Merge remote-tracking branch 'origin/trunk' into az
> 
> Conflicts:
>   core/src/main/scala/kafka/server/KafkaApis.scala
>   core/src/main/scala/kafka/server/KafkaServer.scala
> 
> Deleting KafkaConfigDefTest
> 
> 
> Addressing comments from Ismael.
> 
> 
> Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az
> 
> 
> Consolidating KafkaPrincipal.
> 
> 
> Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az
> 
> Conflicts:
>   
> clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
>   core/src/main/scala/kafka/server/KafkaApis.scala
> 
> Making Acl structure take only one principal, operation and host.
> 
> 
> Merge remote-tracking branch 'origin/trunk' into az
> 
> 
> Reverting uninteded new line change.
> 
> 
> Addressing comments from Jun.
> 
> 
> Merge remote-tracking branch 'origin/trunk' into az
> 
> 
> Various tweaks that make the code more readable
> 
> Conflicts:
>   core/src/main/scala/kafka/server/KafkaApis.scala
> 
> Fixing compilation errors after cherry-pocking.
> 
> 
> Removing FIXME.
> 
> 
> Diffs
> -
> 
>   
> clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java
>  35d41685dd178bbdf77b2476e03ad51fc4adcbb6 
>   clients/src/main/java/org/apache/kafka/common/protocol/Errors.java 
> 641afa1b2474150fa1002e9fedca13ff55175a7e 
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
>  b640ea0f4bdb694fc5524ef594aa125cc1ba4cf3 
>   
> clients/src/test/java/org/apache/kafka/common/security/auth/KafkaPrincipalTest.java
>  PRE-CREATION 
>   core/src/main/scala/kafka/api/OffsetRequest.scala 
> f418868046f7c99aefdccd9956541a0cb72b1500 
>   core/src/main/scala/kafka/common/AuthorizationException.scala PRE-CREATION 
>   core/src/main/scala/kafka/common/ErrorMapping.scala 
> c75c68589681b2c9d6eba2b440ce5e58cddf6370 
>   core/src/main/scala/kafka/security/auth/Acl.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/Authorizer.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/au

Re: Review Request 34492: Patch for KAFKA-2210

[Parth Brahmbhatt]

> On Sept. 2, 2015, 1:11 p.m., Ismael Juma wrote:
> > Hi Parth, I finally had a bit of time to look into this in more detail and 
> > I pushed a commit with my suggestions to make the `KafkaApis` code a bit 
> > more readable:
> > 
> > https://github.com/ijuma/kafka/commit/7737a9feb0c6d8cb4be3fe22992f8dc10b657154
> > 
> > As you said, it's difficult to do much better given the lack of common 
> > interfaces.
> > 
> > Please incorporate the changes if you agree. Also, note that I added a 
> > FIXME in one case where we don't seem to use the data produced by the 
> > `partition` call.

Cherry picked. The FIXME does not need any change if you see 
https://github.com/Parth-Brahmbhatt/kafka/blob/az/core/src/main/scala/kafka/server/KafkaApis.scala#L195
 it uses unauthorized partiton in constructing respoinse. where as the 
authorized part gets used 
https://github.com/Parth-Brahmbhatt/kafka/blob/az/core/src/main/scala/kafka/server/KafkaApis.scala#L214
 and 
https://github.com/Parth-Brahmbhatt/kafka/blob/az/core/src/main/scala/kafka/server/KafkaApis.scala#L255
 and the actual call back finally gets called here 
https://github.com/Parth-Brahmbhatt/kafka/blob/az/core/src/main/scala/kafka/server/KafkaApis.scala#L273.


- Parth


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34492/#review97432
-------


On Sept. 3, 2015, 12:36 a.m., Parth Brahmbhatt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34492/
> ---
> 
> (Updated Sept. 3, 2015, 12:36 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2210
> https://issues.apache.org/jira/browse/KAFKA-2210
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Addressing review comments from Jun.
> 
> 
> Adding CREATE check for offset topic only if the topic does not exist already.
> 
> 
> Addressing some more comments.
> 
> 
> Removing acl.json file
> 
> 
> Moving PermissionType to trait instead of enum. Following the convention for 
> defining constants.
> 
> 
> Adding authorizer.config.path back.
> 
> 
> Addressing more comments from Jun.
> 
> 
> Addressing more comments.
> 
> 
> Now addressing Ismael's comments. Case sensitive checks.
> 
> 
> Addressing Jun's comments.
> 
> 
> Merge remote-tracking branch 'origin/trunk' into az
> 
> Conflicts:
>   core/src/main/scala/kafka/server/KafkaApis.scala
>   core/src/main/scala/kafka/server/KafkaServer.scala
> 
> Deleting KafkaConfigDefTest
> 
> 
> Addressing comments from Ismael.
> 
> 
> Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az
> 
> 
> Consolidating KafkaPrincipal.
> 
> 
> Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az
> 
> Conflicts:
>   
> clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
>   core/src/main/scala/kafka/server/KafkaApis.scala
> 
> Making Acl structure take only one principal, operation and host.
> 
> 
> Merge remote-tracking branch 'origin/trunk' into az
> 
> 
> Reverting uninteded new line change.
> 
> 
> Addressing comments from Jun.
> 
> 
> Merge remote-tracking branch 'origin/trunk' into az
> 
> 
> Various tweaks that make the code more readable
> 
> Conflicts:
>   core/src/main/scala/kafka/server/KafkaApis.scala
> 
> Fixing compilation errors after cherry-pocking.
> 
> 
> Removing FIXME.
> 
> 
> Diffs
> -
> 
>   
> clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java
>  35d41685dd178bbdf77b2476e03ad51fc4adcbb6 
>   clients/src/main/java/org/apache/kafka/common/protocol/Errors.java 
> 641afa1b2474150fa1002e9fedca13ff55175a7e 
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
>  b640ea0f4bdb694fc5524ef594aa125cc1ba4cf3 
>   
> clients/src/test/java/org/apache/kafka/common/security/auth/KafkaPrincipalTest.java
>  PRE-CREATION 
>   core/src/main/scala/kafka/api/OffsetRequest.scala 
> f418868046f7c99aefdccd9956541a0cb72b1500 
>   core/src/main/scala/kafka/common/AuthorizationException.scala PRE-CREATION 
>   core/src/main/scala/kafka/common/ErrorMapping.scala 
> c75c68589681b2c9d6eba2b440ce5e58cddf6370 
>   core/src/main/scala/kafka/security/auth/A

[jira] [Commented] (KAFKA-2210) KafkaAuthorizer: Add all public entities, config changes and changes to KafkaAPI and kafkaServer to allow pluggable authorizer implementation.

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14728299#comment-14728299
 ] 

Parth Brahmbhatt commented on KAFKA-2210:
-

Updated reviewboard https://reviews.apache.org/r/34492/diff/
 against branch origin/trunk

> KafkaAuthorizer: Add all public entities, config changes and changes to 
> KafkaAPI and kafkaServer to allow pluggable authorizer implementation.
> --
>
> Key: KAFKA-2210
> URL: https://issues.apache.org/jira/browse/KAFKA-2210
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Parth Brahmbhatt
>    Assignee: Parth Brahmbhatt
>Priority: Blocker
> Fix For: 0.8.3
>
> Attachments: KAFKA-2210.patch, KAFKA-2210_2015-06-03_16:36:11.patch, 
> KAFKA-2210_2015-06-04_16:07:39.patch, KAFKA-2210_2015-07-09_18:00:34.patch, 
> KAFKA-2210_2015-07-14_10:02:19.patch, KAFKA-2210_2015-07-14_14:13:19.patch, 
> KAFKA-2210_2015-07-20_16:42:18.patch, KAFKA-2210_2015-07-21_17:08:21.patch, 
> KAFKA-2210_2015-08-10_18:31:54.patch, KAFKA-2210_2015-08-20_11:27:18.patch, 
> KAFKA-2210_2015-08-25_17:59:22.patch, KAFKA-2210_2015-08-26_14:29:02.patch, 
> KAFKA-2210_2015-09-01_15:36:02.patch, KAFKA-2210_2015-09-02_14:50:29.patch, 
> KAFKA-2210_2015-09-02_17:32:06.patch, KAFKA-2210_2015-09-02_17:36:47.patch
>
>
> This is the first subtask for Kafka-1688. As Part of this jira we intend to 
> agree on all the public entities, configs and changes to existing kafka 
> classes to allow pluggable authorizer implementation.
> Please see KIP-11 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface
>  for detailed design. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: Review Request 34492: Patch for KAFKA-2210

[Parth Brahmbhatt]

> On Sept. 2, 2015, 11:13 p.m., Jun Rao wrote:
> > Hmm, did you attach the right patch? It doesn't seem to apply to trunk and 
> > my last round of minor comments didn't seem to get addressed. Also, one 
> > more suggestion below.

Can you take a look now? I am not sure why the patch looked weird the first 
time around. I have cherry picked Ismeal's change but I had to fix some 
compilation errors so you will see couple more commits on top of his commit.


- Parth


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34492/#review97554
---


On Sept. 3, 2015, 12:36 a.m., Parth Brahmbhatt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34492/
> ---
> 
> (Updated Sept. 3, 2015, 12:36 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2210
> https://issues.apache.org/jira/browse/KAFKA-2210
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Addressing review comments from Jun.
> 
> 
> Adding CREATE check for offset topic only if the topic does not exist already.
> 
> 
> Addressing some more comments.
> 
> 
> Removing acl.json file
> 
> 
> Moving PermissionType to trait instead of enum. Following the convention for 
> defining constants.
> 
> 
> Adding authorizer.config.path back.
> 
> 
> Addressing more comments from Jun.
> 
> 
> Addressing more comments.
> 
> 
> Now addressing Ismael's comments. Case sensitive checks.
> 
> 
> Addressing Jun's comments.
> 
> 
> Merge remote-tracking branch 'origin/trunk' into az
> 
> Conflicts:
>   core/src/main/scala/kafka/server/KafkaApis.scala
>   core/src/main/scala/kafka/server/KafkaServer.scala
> 
> Deleting KafkaConfigDefTest
> 
> 
> Addressing comments from Ismael.
> 
> 
> Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az
> 
> 
> Consolidating KafkaPrincipal.
> 
> 
> Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az
> 
> Conflicts:
>   
> clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
>   core/src/main/scala/kafka/server/KafkaApis.scala
> 
> Making Acl structure take only one principal, operation and host.
> 
> 
> Merge remote-tracking branch 'origin/trunk' into az
> 
> 
> Reverting uninteded new line change.
> 
> 
> Addressing comments from Jun.
> 
> 
> Merge remote-tracking branch 'origin/trunk' into az
> 
> 
> Various tweaks that make the code more readable
> 
> Conflicts:
>   core/src/main/scala/kafka/server/KafkaApis.scala
> 
> Fixing compilation errors after cherry-pocking.
> 
> 
> Removing FIXME.
> 
> 
> Diffs
> -
> 
>   
> clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java
>  35d41685dd178bbdf77b2476e03ad51fc4adcbb6 
>   clients/src/main/java/org/apache/kafka/common/protocol/Errors.java 
> 641afa1b2474150fa1002e9fedca13ff55175a7e 
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
>  b640ea0f4bdb694fc5524ef594aa125cc1ba4cf3 
>   
> clients/src/test/java/org/apache/kafka/common/security/auth/KafkaPrincipalTest.java
>  PRE-CREATION 
>   core/src/main/scala/kafka/api/OffsetRequest.scala 
> f418868046f7c99aefdccd9956541a0cb72b1500 
>   core/src/main/scala/kafka/common/AuthorizationException.scala PRE-CREATION 
>   core/src/main/scala/kafka/common/ErrorMapping.scala 
> c75c68589681b2c9d6eba2b440ce5e58cddf6370 
>   core/src/main/scala/kafka/security/auth/Acl.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/Authorizer.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/Operation.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/PermissionType.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/Resource.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/ResourceType.scala PRE-CREATION 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> a3a8df0545c3f9390e0e04b8d2fab0134f5fd019 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 
> d547a01cf7098f216a3775e1e1901c5794e1b24c 
>   core/src/main/scala/kafka/server/KafkaServer.scala 
> 756cf775cadbcaf01df7f691d8d01d9ff75db291 
>   core/src/test/scala/unit/kafka/security/auth/AclTest.scala PRE-CREATION 
>   

[jira] [Updated] (KAFKA-2210) KafkaAuthorizer: Add all public entities, config changes and changes to KafkaAPI and kafkaServer to allow pluggable authorizer implementation.

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2210?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Parth Brahmbhatt updated KAFKA-2210:

Attachment: KAFKA-2210_2015-09-02_17:36:47.patch

> KafkaAuthorizer: Add all public entities, config changes and changes to 
> KafkaAPI and kafkaServer to allow pluggable authorizer implementation.
> --
>
> Key: KAFKA-2210
> URL: https://issues.apache.org/jira/browse/KAFKA-2210
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Parth Brahmbhatt
>    Assignee: Parth Brahmbhatt
>Priority: Blocker
> Fix For: 0.8.3
>
> Attachments: KAFKA-2210.patch, KAFKA-2210_2015-06-03_16:36:11.patch, 
> KAFKA-2210_2015-06-04_16:07:39.patch, KAFKA-2210_2015-07-09_18:00:34.patch, 
> KAFKA-2210_2015-07-14_10:02:19.patch, KAFKA-2210_2015-07-14_14:13:19.patch, 
> KAFKA-2210_2015-07-20_16:42:18.patch, KAFKA-2210_2015-07-21_17:08:21.patch, 
> KAFKA-2210_2015-08-10_18:31:54.patch, KAFKA-2210_2015-08-20_11:27:18.patch, 
> KAFKA-2210_2015-08-25_17:59:22.patch, KAFKA-2210_2015-08-26_14:29:02.patch, 
> KAFKA-2210_2015-09-01_15:36:02.patch, KAFKA-2210_2015-09-02_14:50:29.patch, 
> KAFKA-2210_2015-09-02_17:32:06.patch, KAFKA-2210_2015-09-02_17:36:47.patch
>
>
> This is the first subtask for Kafka-1688. As Part of this jira we intend to 
> agree on all the public entities, configs and changes to existing kafka 
> classes to allow pluggable authorizer implementation.
> Please see KIP-11 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface
>  for detailed design. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: Review Request 34492: Patch for KAFKA-2210

[Parth Brahmbhatt]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34492/
---

(Updated Sept. 2, 2015, 9:50 p.m.)


Review request for kafka.


Bugs: KAFKA-2210
https://issues.apache.org/jira/browse/KAFKA-2210


Repository: kafka


Description (updated)
---

Addressing review comments from Jun.


Adding CREATE check for offset topic only if the topic does not exist already.


Addressing some more comments.


Removing acl.json file


Moving PermissionType to trait instead of enum. Following the convention for 
defining constants.


Adding authorizer.config.path back.


Addressing more comments from Jun.


Addressing more comments.


Now addressing Ismael's comments. Case sensitive checks.


Addressing Jun's comments.


Merge remote-tracking branch 'origin/trunk' into az

Conflicts:
core/src/main/scala/kafka/server/KafkaApis.scala
core/src/main/scala/kafka/server/KafkaServer.scala

Deleting KafkaConfigDefTest


Addressing comments from Ismael.


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az


Consolidating KafkaPrincipal.


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az

Conflicts:

clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java

clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
core/src/main/scala/kafka/server/KafkaApis.scala

Making Acl structure take only one principal, operation and host.


Merge remote-tracking branch 'origin/trunk' into az


Reverting uninteded new line change.


Diffs (updated)
-

  
clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java
 35d41685dd178bbdf77b2476e03ad51fc4adcbb6 
  clients/src/main/java/org/apache/kafka/common/protocol/Errors.java 
e17e390c507eca0eba28a2763c0e35d66077d1f2 
  
clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java 
b640ea0f4bdb694fc5524ef594aa125cc1ba4cf3 
  
clients/src/test/java/org/apache/kafka/common/security/auth/KafkaPrincipalTest.java
 PRE-CREATION 
  core/src/main/scala/kafka/api/OffsetRequest.scala 
f418868046f7c99aefdccd9956541a0cb72b1500 
  core/src/main/scala/kafka/common/AuthorizationException.scala PRE-CREATION 
  core/src/main/scala/kafka/common/ErrorMapping.scala 
c75c68589681b2c9d6eba2b440ce5e58cddf6370 
  core/src/main/scala/kafka/security/auth/Acl.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Authorizer.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Operation.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/PermissionType.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Resource.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/ResourceType.scala PRE-CREATION 
  core/src/main/scala/kafka/server/KafkaApis.scala 
a3a8df0545c3f9390e0e04b8d2fab0134f5fd019 
  core/src/main/scala/kafka/server/KafkaConfig.scala 
d547a01cf7098f216a3775e1e1901c5794e1b24c 
  core/src/main/scala/kafka/server/KafkaServer.scala 
039c7eb919edeedbf8f1342c6e2fdf4736e224cd 
  core/src/test/scala/unit/kafka/security/auth/AclTest.scala PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/OperationTest.scala PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/PermissionTypeTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/ResourceTypeTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 
3da666f73227fc7ef7093e3790546344065f6825 

Diff: https://reviews.apache.org/r/34492/diff/


Testing
---


Thanks,

Parth Brahmbhatt

Re: Review Request 34492: Patch for KAFKA-2210

[Parth Brahmbhatt]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34492/#review97481
---



core/src/main/scala/kafka/server/KafkaApis.scala (line 296)
<https://reviews.apache.org/r/34492/#comment153374>

nope. reverted.


- Parth Brahmbhatt


On Sept. 2, 2015, 9:50 p.m., Parth Brahmbhatt wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34492/
> ---
> 
> (Updated Sept. 2, 2015, 9:50 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2210
> https://issues.apache.org/jira/browse/KAFKA-2210
> 
> 
> Repository: kafka
> 
> 
> Description
> ---
> 
> Addressing review comments from Jun.
> 
> 
> Adding CREATE check for offset topic only if the topic does not exist already.
> 
> 
> Addressing some more comments.
> 
> 
> Removing acl.json file
> 
> 
> Moving PermissionType to trait instead of enum. Following the convention for 
> defining constants.
> 
> 
> Adding authorizer.config.path back.
> 
> 
> Addressing more comments from Jun.
> 
> 
> Addressing more comments.
> 
> 
> Now addressing Ismael's comments. Case sensitive checks.
> 
> 
> Addressing Jun's comments.
> 
> 
> Merge remote-tracking branch 'origin/trunk' into az
> 
> Conflicts:
>   core/src/main/scala/kafka/server/KafkaApis.scala
>   core/src/main/scala/kafka/server/KafkaServer.scala
> 
> Deleting KafkaConfigDefTest
> 
> 
> Addressing comments from Ismael.
> 
> 
> Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az
> 
> 
> Consolidating KafkaPrincipal.
> 
> 
> Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az
> 
> Conflicts:
>   
> clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
>   core/src/main/scala/kafka/server/KafkaApis.scala
> 
> Making Acl structure take only one principal, operation and host.
> 
> 
> Merge remote-tracking branch 'origin/trunk' into az
> 
> 
> Reverting uninteded new line change.
> 
> 
> Diffs
> -
> 
>   
> clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java
>  35d41685dd178bbdf77b2476e03ad51fc4adcbb6 
>   clients/src/main/java/org/apache/kafka/common/protocol/Errors.java 
> e17e390c507eca0eba28a2763c0e35d66077d1f2 
>   
> clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
>  b640ea0f4bdb694fc5524ef594aa125cc1ba4cf3 
>   
> clients/src/test/java/org/apache/kafka/common/security/auth/KafkaPrincipalTest.java
>  PRE-CREATION 
>   core/src/main/scala/kafka/api/OffsetRequest.scala 
> f418868046f7c99aefdccd9956541a0cb72b1500 
>   core/src/main/scala/kafka/common/AuthorizationException.scala PRE-CREATION 
>   core/src/main/scala/kafka/common/ErrorMapping.scala 
> c75c68589681b2c9d6eba2b440ce5e58cddf6370 
>   core/src/main/scala/kafka/security/auth/Acl.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/Authorizer.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/Operation.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/PermissionType.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/Resource.scala PRE-CREATION 
>   core/src/main/scala/kafka/security/auth/ResourceType.scala PRE-CREATION 
>   core/src/main/scala/kafka/server/KafkaApis.scala 
> a3a8df0545c3f9390e0e04b8d2fab0134f5fd019 
>   core/src/main/scala/kafka/server/KafkaConfig.scala 
> d547a01cf7098f216a3775e1e1901c5794e1b24c 
>   core/src/main/scala/kafka/server/KafkaServer.scala 
> 039c7eb919edeedbf8f1342c6e2fdf4736e224cd 
>   core/src/test/scala/unit/kafka/security/auth/AclTest.scala PRE-CREATION 
>   core/src/test/scala/unit/kafka/security/auth/OperationTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/security/auth/PermissionTypeTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/security/auth/ResourceTypeTest.scala 
> PRE-CREATION 
>   core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 
> 3da666f73227fc7ef7093e3790546344065f6825 
> 
> Diff: https://reviews.apache.org/r/34492/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Parth Brahmbhatt
> 
>

[jira] [Updated] (KAFKA-2210) KafkaAuthorizer: Add all public entities, config changes and changes to KafkaAPI and kafkaServer to allow pluggable authorizer implementation.

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2210?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Parth Brahmbhatt updated KAFKA-2210:

Attachment: KAFKA-2210_2015-09-02_14:50:29.patch

> KafkaAuthorizer: Add all public entities, config changes and changes to 
> KafkaAPI and kafkaServer to allow pluggable authorizer implementation.
> --
>
> Key: KAFKA-2210
> URL: https://issues.apache.org/jira/browse/KAFKA-2210
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Parth Brahmbhatt
>    Assignee: Parth Brahmbhatt
>Priority: Blocker
> Fix For: 0.8.3
>
> Attachments: KAFKA-2210.patch, KAFKA-2210_2015-06-03_16:36:11.patch, 
> KAFKA-2210_2015-06-04_16:07:39.patch, KAFKA-2210_2015-07-09_18:00:34.patch, 
> KAFKA-2210_2015-07-14_10:02:19.patch, KAFKA-2210_2015-07-14_14:13:19.patch, 
> KAFKA-2210_2015-07-20_16:42:18.patch, KAFKA-2210_2015-07-21_17:08:21.patch, 
> KAFKA-2210_2015-08-10_18:31:54.patch, KAFKA-2210_2015-08-20_11:27:18.patch, 
> KAFKA-2210_2015-08-25_17:59:22.patch, KAFKA-2210_2015-08-26_14:29:02.patch, 
> KAFKA-2210_2015-09-01_15:36:02.patch, KAFKA-2210_2015-09-02_14:50:29.patch
>
>
> This is the first subtask for Kafka-1688. As Part of this jira we intend to 
> agree on all the public entities, configs and changes to existing kafka 
> classes to allow pluggable authorizer implementation.
> Please see KIP-11 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface
>  for detailed design. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-2210) KafkaAuthorizer: Add all public entities, config changes and changes to KafkaAPI and kafkaServer to allow pluggable authorizer implementation.

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14728093#comment-14728093
 ] 

Parth Brahmbhatt commented on KAFKA-2210:
-

Updated reviewboard https://reviews.apache.org/r/34492/diff/
 against branch origin/trunk

> KafkaAuthorizer: Add all public entities, config changes and changes to 
> KafkaAPI and kafkaServer to allow pluggable authorizer implementation.
> --
>
> Key: KAFKA-2210
> URL: https://issues.apache.org/jira/browse/KAFKA-2210
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Parth Brahmbhatt
>    Assignee: Parth Brahmbhatt
>Priority: Blocker
> Fix For: 0.8.3
>
> Attachments: KAFKA-2210.patch, KAFKA-2210_2015-06-03_16:36:11.patch, 
> KAFKA-2210_2015-06-04_16:07:39.patch, KAFKA-2210_2015-07-09_18:00:34.patch, 
> KAFKA-2210_2015-07-14_10:02:19.patch, KAFKA-2210_2015-07-14_14:13:19.patch, 
> KAFKA-2210_2015-07-20_16:42:18.patch, KAFKA-2210_2015-07-21_17:08:21.patch, 
> KAFKA-2210_2015-08-10_18:31:54.patch, KAFKA-2210_2015-08-20_11:27:18.patch, 
> KAFKA-2210_2015-08-25_17:59:22.patch, KAFKA-2210_2015-08-26_14:29:02.patch, 
> KAFKA-2210_2015-09-01_15:36:02.patch, KAFKA-2210_2015-09-02_14:50:29.patch
>
>
> This is the first subtask for Kafka-1688. As Part of this jira we intend to 
> agree on all the public entities, configs and changes to existing kafka 
> classes to allow pluggable authorizer implementation.
> Please see KIP-11 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface
>  for detailed design. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: Review Request 34492: Patch for KAFKA-2210

[Parth Brahmbhatt]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34492/
---

(Updated Sept. 1, 2015, 10:36 p.m.)


Review request for kafka.


Bugs: KAFKA-2210
https://issues.apache.org/jira/browse/KAFKA-2210


Repository: kafka


Description (updated)
---

Addressing review comments from Jun.


Adding CREATE check for offset topic only if the topic does not exist already.


Addressing some more comments.


Removing acl.json file


Moving PermissionType to trait instead of enum. Following the convention for 
defining constants.


Adding authorizer.config.path back.


Addressing more comments from Jun.


Addressing more comments.


Now addressing Ismael's comments. Case sensitive checks.


Addressing Jun's comments.


Merge remote-tracking branch 'origin/trunk' into az

Conflicts:
core/src/main/scala/kafka/server/KafkaApis.scala
core/src/main/scala/kafka/server/KafkaServer.scala

Deleting KafkaConfigDefTest


Addressing comments from Ismael.


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az


Consolidating KafkaPrincipal.


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az

Conflicts:

clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java

clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
core/src/main/scala/kafka/server/KafkaApis.scala

Making Acl structure take only one principal, operation and host.


Diffs (updated)
-

  
clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java
 35d41685dd178bbdf77b2476e03ad51fc4adcbb6 
  clients/src/main/java/org/apache/kafka/common/protocol/Errors.java 
e17e390c507eca0eba28a2763c0e35d66077d1f2 
  
clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java 
b640ea0f4bdb694fc5524ef594aa125cc1ba4cf3 
  
clients/src/test/java/org/apache/kafka/common/security/auth/KafkaPrincipalTest.java
 PRE-CREATION 
  core/src/main/scala/kafka/api/OffsetRequest.scala 
f418868046f7c99aefdccd9956541a0cb72b1500 
  core/src/main/scala/kafka/common/AuthorizationException.scala PRE-CREATION 
  core/src/main/scala/kafka/common/ErrorMapping.scala 
c75c68589681b2c9d6eba2b440ce5e58cddf6370 
  core/src/main/scala/kafka/security/auth/Acl.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Authorizer.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Operation.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/PermissionType.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Resource.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/ResourceType.scala PRE-CREATION 
  core/src/main/scala/kafka/server/KafkaApis.scala 
a3a8df0545c3f9390e0e04b8d2fab0134f5fd019 
  core/src/main/scala/kafka/server/KafkaConfig.scala 
d547a01cf7098f216a3775e1e1901c5794e1b24c 
  core/src/main/scala/kafka/server/KafkaServer.scala 
17db4fa3c3a146f03a35dd7671ad1b06d122bb59 
  core/src/test/scala/unit/kafka/security/auth/AclTest.scala PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/OperationTest.scala PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/PermissionTypeTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/ResourceTypeTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 
3da666f73227fc7ef7093e3790546344065f6825 

Diff: https://reviews.apache.org/r/34492/diff/


Testing
---


Thanks,

Parth Brahmbhatt

[jira] [Updated] (KAFKA-2210) KafkaAuthorizer: Add all public entities, config changes and changes to KafkaAPI and kafkaServer to allow pluggable authorizer implementation.

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2210?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Parth Brahmbhatt updated KAFKA-2210:

Attachment: KAFKA-2210_2015-09-01_15:36:02.patch

> KafkaAuthorizer: Add all public entities, config changes and changes to 
> KafkaAPI and kafkaServer to allow pluggable authorizer implementation.
> --
>
> Key: KAFKA-2210
> URL: https://issues.apache.org/jira/browse/KAFKA-2210
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Parth Brahmbhatt
>    Assignee: Parth Brahmbhatt
> Fix For: 0.8.3
>
> Attachments: KAFKA-2210.patch, KAFKA-2210_2015-06-03_16:36:11.patch, 
> KAFKA-2210_2015-06-04_16:07:39.patch, KAFKA-2210_2015-07-09_18:00:34.patch, 
> KAFKA-2210_2015-07-14_10:02:19.patch, KAFKA-2210_2015-07-14_14:13:19.patch, 
> KAFKA-2210_2015-07-20_16:42:18.patch, KAFKA-2210_2015-07-21_17:08:21.patch, 
> KAFKA-2210_2015-08-10_18:31:54.patch, KAFKA-2210_2015-08-20_11:27:18.patch, 
> KAFKA-2210_2015-08-25_17:59:22.patch, KAFKA-2210_2015-08-26_14:29:02.patch, 
> KAFKA-2210_2015-09-01_15:36:02.patch
>
>
> This is the first subtask for Kafka-1688. As Part of this jira we intend to 
> agree on all the public entities, configs and changes to existing kafka 
> classes to allow pluggable authorizer implementation.
> Please see KIP-11 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface
>  for detailed design. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-2210) KafkaAuthorizer: Add all public entities, config changes and changes to KafkaAPI and kafkaServer to allow pluggable authorizer implementation.

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14723866#comment-14723866
 ] 

Parth Brahmbhatt commented on KAFKA-2210:
-

[~junrao] yes that is correct sequencing and I haven't updated 2211 and 2212 as 
we were changing 2210. Once 2210 is merged I will update 2211 and 2212 and send 
new pull requests.

> KafkaAuthorizer: Add all public entities, config changes and changes to 
> KafkaAPI and kafkaServer to allow pluggable authorizer implementation.
> --
>
> Key: KAFKA-2210
> URL: https://issues.apache.org/jira/browse/KAFKA-2210
> Project: Kafka
>  Issue Type: Sub-task
>  Components: security
>Reporter: Parth Brahmbhatt
>    Assignee: Parth Brahmbhatt
> Fix For: 0.8.3
>
> Attachments: KAFKA-2210.patch, KAFKA-2210_2015-06-03_16:36:11.patch, 
> KAFKA-2210_2015-06-04_16:07:39.patch, KAFKA-2210_2015-07-09_18:00:34.patch, 
> KAFKA-2210_2015-07-14_10:02:19.patch, KAFKA-2210_2015-07-14_14:13:19.patch, 
> KAFKA-2210_2015-07-20_16:42:18.patch, KAFKA-2210_2015-07-21_17:08:21.patch, 
> KAFKA-2210_2015-08-10_18:31:54.patch, KAFKA-2210_2015-08-20_11:27:18.patch, 
> KAFKA-2210_2015-08-25_17:59:22.patch, KAFKA-2210_2015-08-26_14:29:02.patch
>
>
> This is the first subtask for Kafka-1688. As Part of this jira we intend to 
> agree on all the public entities, configs and changes to existing kafka 
> classes to allow pluggable authorizer implementation.
> Please see KIP-11 
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface
>  for detailed design. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Updated] (KAFKA-2210) KafkaAuthorizer: Add all public entities, config changes and changes to KafkaAPI and kafkaServer to allow pluggable authorizer implementation.

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2210?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Parth Brahmbhatt updated KAFKA-2210:

Attachment: KAFKA-2210_2015-08-26_14:29:02.patch

 KafkaAuthorizer: Add all public entities, config changes and changes to 
 KafkaAPI and kafkaServer to allow pluggable authorizer implementation.
 --

 Key: KAFKA-2210
 URL: https://issues.apache.org/jira/browse/KAFKA-2210
 Project: Kafka
  Issue Type: Sub-task
  Components: security
Reporter: Parth Brahmbhatt
Assignee: Parth Brahmbhatt
 Fix For: 0.8.3

 Attachments: KAFKA-2210.patch, KAFKA-2210_2015-06-03_16:36:11.patch, 
 KAFKA-2210_2015-06-04_16:07:39.patch, KAFKA-2210_2015-07-09_18:00:34.patch, 
 KAFKA-2210_2015-07-14_10:02:19.patch, KAFKA-2210_2015-07-14_14:13:19.patch, 
 KAFKA-2210_2015-07-20_16:42:18.patch, KAFKA-2210_2015-07-21_17:08:21.patch, 
 KAFKA-2210_2015-08-10_18:31:54.patch, KAFKA-2210_2015-08-20_11:27:18.patch, 
 KAFKA-2210_2015-08-25_17:59:22.patch, KAFKA-2210_2015-08-26_14:29:02.patch


 This is the first subtask for Kafka-1688. As Part of this jira we intend to 
 agree on all the public entities, configs and changes to existing kafka 
 classes to allow pluggable authorizer implementation.
 Please see KIP-11 
 https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface
  for detailed design. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: Review Request 34492: Patch for KAFKA-2210

[Parth Brahmbhatt]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34492/
---

(Updated Aug. 26, 2015, 9:29 p.m.)


Review request for kafka.


Bugs: KAFKA-2210
https://issues.apache.org/jira/browse/KAFKA-2210


Repository: kafka


Description (updated)
---

Addressing review comments from Jun.


Adding CREATE check for offset topic only if the topic does not exist already.


Addressing some more comments.


Removing acl.json file


Moving PermissionType to trait instead of enum. Following the convention for 
defining constants.


Adding authorizer.config.path back.


Addressing more comments from Jun.


Addressing more comments.


Now addressing Ismael's comments. Case sensitive checks.


Addressing Jun's comments.


Merge remote-tracking branch 'origin/trunk' into az

Conflicts:
core/src/main/scala/kafka/server/KafkaApis.scala
core/src/main/scala/kafka/server/KafkaServer.scala

Deleting KafkaConfigDefTest


Addressing comments from Ismael.


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az


Consolidating KafkaPrincipal.


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az

Conflicts:

clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java

clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
core/src/main/scala/kafka/server/KafkaApis.scala


Diffs (updated)
-

  
clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java
 35d41685dd178bbdf77b2476e03ad51fc4adcbb6 
  
clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java 
b640ea0f4bdb694fc5524ef594aa125cc1ba4cf3 
  
clients/src/test/java/org/apache/kafka/common/security/auth/KafkaPrincipalTest.java
 PRE-CREATION 
  core/src/main/scala/kafka/api/OffsetRequest.scala 
f418868046f7c99aefdccd9956541a0cb72b1500 
  core/src/main/scala/kafka/common/AuthorizationException.scala PRE-CREATION 
  core/src/main/scala/kafka/common/ErrorMapping.scala 
c75c68589681b2c9d6eba2b440ce5e58cddf6370 
  core/src/main/scala/kafka/security/auth/Acl.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Authorizer.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Operation.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/PermissionType.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Resource.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/ResourceType.scala PRE-CREATION 
  core/src/main/scala/kafka/server/KafkaApis.scala 
a3a8df0545c3f9390e0e04b8d2fab0134f5fd019 
  core/src/main/scala/kafka/server/KafkaConfig.scala 
d547a01cf7098f216a3775e1e1901c5794e1b24c 
  core/src/main/scala/kafka/server/KafkaServer.scala 
17db4fa3c3a146f03a35dd7671ad1b06d122bb59 
  core/src/test/scala/unit/kafka/security/auth/AclTest.scala PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/OperationTest.scala PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/PermissionTypeTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/ResourceTypeTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 
3da666f73227fc7ef7093e3790546344065f6825 

Diff: https://reviews.apache.org/r/34492/diff/


Testing
---


Thanks,

Parth Brahmbhatt

Re: Review Request 34492: Patch for KAFKA-2210

[Parth Brahmbhatt]

 On Aug. 23, 2015, 9:29 p.m., Jun Rao wrote:
  core/src/main/scala/kafka/security/auth/KafkaPrincipal.scala, line 21
  https://reviews.apache.org/r/34492/diff/11/?file=1045205#file1045205line21
 
  There is a java version of KafkaPrincipal. Could we consolidate?

consolidated. Converted Authorizer's principal to Java class and moved under 
client/common/security/auth. Let me know if I misunderstood what you meant by 
this comment.


- Parth


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34492/#review96129
---


On Aug. 26, 2015, 12:59 a.m., Parth Brahmbhatt wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34492/
 ---
 
 (Updated Aug. 26, 2015, 12:59 a.m.)
 
 
 Review request for kafka.
 
 
 Bugs: KAFKA-2210
 https://issues.apache.org/jira/browse/KAFKA-2210
 
 
 Repository: kafka
 
 
 Description
 ---
 
 Addressing review comments from Jun.
 
 
 Adding CREATE check for offset topic only if the topic does not exist already.
 
 
 Addressing some more comments.
 
 
 Removing acl.json file
 
 
 Moving PermissionType to trait instead of enum. Following the convention for 
 defining constants.
 
 
 Adding authorizer.config.path back.
 
 
 Addressing more comments from Jun.
 
 
 Addressing more comments.
 
 
 Now addressing Ismael's comments. Case sensitive checks.
 
 
 Addressing Jun's comments.
 
 
 Merge remote-tracking branch 'origin/trunk' into az
 
 Conflicts:
   core/src/main/scala/kafka/server/KafkaApis.scala
   core/src/main/scala/kafka/server/KafkaServer.scala
 
 Deleting KafkaConfigDefTest
 
 
 Addressing comments from Ismael.
 
 
 Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az
 
 
 Consolidating KafkaPrincipal.
 
 
 Diffs
 -
 
   
 clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java
  a3567af96e4f90d62587b31d5b14e7911ba9baf4 
   
 clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java
  277b6efb8cf4ad2c81fdcf694bf7c233e48cf214 
   
 clients/src/test/java/org/apache/kafka/common/security/auth/KafkaPrincipalTest.java
  PRE-CREATION 
   core/src/main/scala/kafka/api/OffsetRequest.scala 
 f418868046f7c99aefdccd9956541a0cb72b1500 
   core/src/main/scala/kafka/common/AuthorizationException.scala PRE-CREATION 
   core/src/main/scala/kafka/common/ErrorMapping.scala 
 c75c68589681b2c9d6eba2b440ce5e58cddf6370 
   core/src/main/scala/kafka/security/auth/Acl.scala PRE-CREATION 
   core/src/main/scala/kafka/security/auth/Authorizer.scala PRE-CREATION 
   core/src/main/scala/kafka/security/auth/Operation.scala PRE-CREATION 
   core/src/main/scala/kafka/security/auth/PermissionType.scala PRE-CREATION 
   core/src/main/scala/kafka/security/auth/Resource.scala PRE-CREATION 
   core/src/main/scala/kafka/security/auth/ResourceType.scala PRE-CREATION 
   core/src/main/scala/kafka/server/KafkaApis.scala 
 67f0cad802f901f255825aa2158545d7f5e7cc3d 
   core/src/main/scala/kafka/server/KafkaConfig.scala 
 d547a01cf7098f216a3775e1e1901c5794e1b24c 
   core/src/main/scala/kafka/server/KafkaServer.scala 
 17db4fa3c3a146f03a35dd7671ad1b06d122bb59 
   core/src/test/scala/unit/kafka/security/auth/AclTest.scala PRE-CREATION 
   core/src/test/scala/unit/kafka/security/auth/OperationTest.scala 
 PRE-CREATION 
   core/src/test/scala/unit/kafka/security/auth/PermissionTypeTest.scala 
 PRE-CREATION 
   core/src/test/scala/unit/kafka/security/auth/ResourceTypeTest.scala 
 PRE-CREATION 
   core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 
 3da666f73227fc7ef7093e3790546344065f6825 
 
 Diff: https://reviews.apache.org/r/34492/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Parth Brahmbhatt
 

[jira] [Commented] (KAFKA-2210) KafkaAuthorizer: Add all public entities, config changes and changes to KafkaAPI and kafkaServer to allow pluggable authorizer implementation.

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2210?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14712282#comment-14712282
 ] 

Parth Brahmbhatt commented on KAFKA-2210:
-

Updated reviewboard https://reviews.apache.org/r/34492/diff/
 against branch origin/trunk

 KafkaAuthorizer: Add all public entities, config changes and changes to 
 KafkaAPI and kafkaServer to allow pluggable authorizer implementation.
 --

 Key: KAFKA-2210
 URL: https://issues.apache.org/jira/browse/KAFKA-2210
 Project: Kafka
  Issue Type: Sub-task
  Components: security
Reporter: Parth Brahmbhatt
Assignee: Parth Brahmbhatt
 Fix For: 0.8.3

 Attachments: KAFKA-2210.patch, KAFKA-2210_2015-06-03_16:36:11.patch, 
 KAFKA-2210_2015-06-04_16:07:39.patch, KAFKA-2210_2015-07-09_18:00:34.patch, 
 KAFKA-2210_2015-07-14_10:02:19.patch, KAFKA-2210_2015-07-14_14:13:19.patch, 
 KAFKA-2210_2015-07-20_16:42:18.patch, KAFKA-2210_2015-07-21_17:08:21.patch, 
 KAFKA-2210_2015-08-10_18:31:54.patch, KAFKA-2210_2015-08-20_11:27:18.patch, 
 KAFKA-2210_2015-08-25_17:59:22.patch


 This is the first subtask for Kafka-1688. As Part of this jira we intend to 
 agree on all the public entities, configs and changes to existing kafka 
 classes to allow pluggable authorizer implementation.
 Please see KIP-11 
 https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface
  for detailed design. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: Review Request 34492: Patch for KAFKA-2210

[Parth Brahmbhatt]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34492/
---

(Updated Aug. 26, 2015, 12:59 a.m.)


Review request for kafka.


Bugs: KAFKA-2210
https://issues.apache.org/jira/browse/KAFKA-2210


Repository: kafka


Description (updated)
---

Addressing review comments from Jun.


Adding CREATE check for offset topic only if the topic does not exist already.


Addressing some more comments.


Removing acl.json file


Moving PermissionType to trait instead of enum. Following the convention for 
defining constants.


Adding authorizer.config.path back.


Addressing more comments from Jun.


Addressing more comments.


Now addressing Ismael's comments. Case sensitive checks.


Addressing Jun's comments.


Merge remote-tracking branch 'origin/trunk' into az

Conflicts:
core/src/main/scala/kafka/server/KafkaApis.scala
core/src/main/scala/kafka/server/KafkaServer.scala

Deleting KafkaConfigDefTest


Addressing comments from Ismael.


Merge branch 'trunk' of http://git-wip-us.apache.org/repos/asf/kafka into az


Consolidating KafkaPrincipal.


Diffs (updated)
-

  
clients/src/main/java/org/apache/kafka/common/network/PlaintextTransportLayer.java
 a3567af96e4f90d62587b31d5b14e7911ba9baf4 
  
clients/src/main/java/org/apache/kafka/common/security/auth/KafkaPrincipal.java 
277b6efb8cf4ad2c81fdcf694bf7c233e48cf214 
  
clients/src/test/java/org/apache/kafka/common/security/auth/KafkaPrincipalTest.java
 PRE-CREATION 
  core/src/main/scala/kafka/api/OffsetRequest.scala 
f418868046f7c99aefdccd9956541a0cb72b1500 
  core/src/main/scala/kafka/common/AuthorizationException.scala PRE-CREATION 
  core/src/main/scala/kafka/common/ErrorMapping.scala 
c75c68589681b2c9d6eba2b440ce5e58cddf6370 
  core/src/main/scala/kafka/security/auth/Acl.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Authorizer.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Operation.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/PermissionType.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/Resource.scala PRE-CREATION 
  core/src/main/scala/kafka/security/auth/ResourceType.scala PRE-CREATION 
  core/src/main/scala/kafka/server/KafkaApis.scala 
67f0cad802f901f255825aa2158545d7f5e7cc3d 
  core/src/main/scala/kafka/server/KafkaConfig.scala 
d547a01cf7098f216a3775e1e1901c5794e1b24c 
  core/src/main/scala/kafka/server/KafkaServer.scala 
17db4fa3c3a146f03a35dd7671ad1b06d122bb59 
  core/src/test/scala/unit/kafka/security/auth/AclTest.scala PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/OperationTest.scala PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/PermissionTypeTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/security/auth/ResourceTypeTest.scala 
PRE-CREATION 
  core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala 
3da666f73227fc7ef7093e3790546344065f6825 

Diff: https://reviews.apache.org/r/34492/diff/


Testing
---


Thanks,

Parth Brahmbhatt

[jira] [Updated] (KAFKA-2210) KafkaAuthorizer: Add all public entities, config changes and changes to KafkaAPI and kafkaServer to allow pluggable authorizer implementation.

[Parth Brahmbhatt (JIRA)]

 [ 
https://issues.apache.org/jira/browse/KAFKA-2210?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Parth Brahmbhatt updated KAFKA-2210:

Attachment: KAFKA-2210_2015-08-25_17:59:22.patch

 KafkaAuthorizer: Add all public entities, config changes and changes to 
 KafkaAPI and kafkaServer to allow pluggable authorizer implementation.
 --

 Key: KAFKA-2210
 URL: https://issues.apache.org/jira/browse/KAFKA-2210
 Project: Kafka
  Issue Type: Sub-task
  Components: security
Reporter: Parth Brahmbhatt
Assignee: Parth Brahmbhatt
 Fix For: 0.8.3

 Attachments: KAFKA-2210.patch, KAFKA-2210_2015-06-03_16:36:11.patch, 
 KAFKA-2210_2015-06-04_16:07:39.patch, KAFKA-2210_2015-07-09_18:00:34.patch, 
 KAFKA-2210_2015-07-14_10:02:19.patch, KAFKA-2210_2015-07-14_14:13:19.patch, 
 KAFKA-2210_2015-07-20_16:42:18.patch, KAFKA-2210_2015-07-21_17:08:21.patch, 
 KAFKA-2210_2015-08-10_18:31:54.patch, KAFKA-2210_2015-08-20_11:27:18.patch, 
 KAFKA-2210_2015-08-25_17:59:22.patch


 This is the first subtask for Kafka-1688. As Part of this jira we intend to 
 agree on all the public entities, configs and changes to existing kafka 
 classes to allow pluggable authorizer implementation.
 Please see KIP-11 
 https://cwiki.apache.org/confluence/display/KAFKA/KIP-11+-+Authorization+Interface
  for detailed design. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

[jira] [Commented] (KAFKA-2145) An option to add topic owners.

[Parth Brahmbhatt (JIRA)]

[ 
https://issues.apache.org/jira/browse/KAFKA-2145?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14712319#comment-14712319
 ] 

Parth Brahmbhatt commented on KAFKA-2145:
-

[~junrao] Pinging again for review. 

 An option to add topic owners. 
 ---

 Key: KAFKA-2145
 URL: https://issues.apache.org/jira/browse/KAFKA-2145
 Project: Kafka
  Issue Type: Improvement
Reporter: Parth Brahmbhatt
Assignee: Parth Brahmbhatt

 We need to expose a way so users can identify users/groups that share 
 ownership of topic. We discussed adding this as part of 
 https://issues.apache.org/jira/browse/KAFKA-2035 and agreed that it will be 
 simpler to add owner as a logconfig. 
 The owner field can be used for auditing and also by authorization layer to 
 grant access without having to explicitly configure acls. 



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Re: Review Request 34493: Patch for KAFKA-2211

[Parth Brahmbhatt]

 On Aug. 21, 2015, 2:31 p.m., Ismael Juma wrote:
  Thanks for this Parth. I did an initial pass where I left a number comments 
  (many of them style-related, see http://kafka.apache.org/coding-guide.html 
  for reference). I know, we should have a tool that checks some of these 
  things automatically. That is my main priority after we get the security 
  stuff in shape.
  
  I think it would be useful if you had a look and made the changes (if you 
  agree) to the cases I pointed out and similar ones. I noticed that 
  KAFKA-2212 has some similar issues too, it may be worth taking a pass there 
  too.
  
  I will look at these two patches again early next week.

Hey , thanks for reviewing this. This patch needs to be updated with all the 
changes that we have made in 2210. As 2210 was moving slow and was kind of a 
moving target I did not update this patch. I think I have gained little more 
understanding around scala styles from the 2210 review so I will incorporate 
all those in this patch.


- Parth


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34493/#review96042
---


On May 20, 2015, 8:03 p.m., Parth Brahmbhatt wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/34493/
 ---
 
 (Updated May 20, 2015, 8:03 p.m.)
 
 
 Review request for kafka.
 
 
 Bugs: KAFKA-2211
 https://issues.apache.org/jira/browse/KAFKA-2211
 
 
 Repository: kafka
 
 
 Description
 ---
 
 KAFKA-2211: Out of box implementation for authorizer interface.
 
 
 Diffs
 -
 
   core/src/main/scala/kafka/security/auth/SimpleAclAuthorizer.scala 
 PRE-CREATION 
   core/src/test/resources/authorizer-config.properties PRE-CREATION 
   core/src/test/scala/unit/kafka/security/auth/SimpleAclAuthorizerTest.scala 
 PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/34493/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Parth Brahmbhatt