Re: [DISCUSS] KIP-111 : Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.

[Mayuresh Gharat]

Hi All,

If there are no more concerns, I will like to start vote for this KIP.

Thanks,

Mayuresh

On Wed, Feb 1, 2017 at 8:38 PM, Mayuresh Gharat 
wrote:

> Hi Dong,
>
> What I meant was "Right now Kafka just extracts the name out of the
> Principal that is generated by the PrincipalBuilder. Instead of doing that
> if it preserves the Principal itself, this issue can be addressed".
>
> May be I should have used the word "preserve" instead of "stores". I have
> updated the wording in the KIP.
>
> Thanks,
>
> Mayuresh
>
> On Wed, Feb 1, 2017 at 8:30 PM, Dong Lin  wrote:
>
>> The last paragraph of the motivation section is a bit confusing. I guess
>> you want to say "This issue can be addressed if the Session class stores
>> the Principal object extracted from a request".
>>
>> I like the approach of changing Session class to be case class
>> *Session(principal:
>> KafkaPrincipal, clientAddress: InetAddress)* under the assumption that the
>> Session class doesn't really need principalType of the KafkaPrincipal. I
>> am
>> wondering if anyone in the open source mailing list knows why we need to
>> have principalType in KafkaPrincipal.
>>
>> For the record, I actually prefer that we use the existing configure() to
>> provide properties to PrincipalBuilder instead of adding the method
>> *buildPrincipal(Map> ?> principalConfigs)* in the PrincipalBuilder interface. But this is not a
>> blocking issue for me.
>>
>>
>>
>>
>> On Wed, Feb 1, 2017 at 2:54 PM, Mayuresh Gharat <
>> gharatmayures...@gmail.com>
>> wrote:
>>
>> > Hi All,
>> >
>> > I have updated the KIP as per our discussion here.
>> > It would be great if you can take another look and let me know if there
>> are
>> > any concerns.
>> >
>> > Thanks,
>> >
>> > Mayuresh
>> >
>> > On Sat, Jan 28, 2017 at 6:10 PM, Mayuresh Gharat <
>> > gharatmayures...@gmail.com
>> > > wrote:
>> >
>> > > I had offline discussions with Joel, Dong and Radai.
>> > >
>> > > I agree that we can replace the KafkaPrincipal in Session with the
>> > > ChannelPrincipal.
>> > > KafkaPrincipal can be provided as an out of box implementation.
>> > >
>> > > The only gotcha will be users will have to implement there own
>> > Authorizer,
>> > > if they decide to use there own PrincipalBuilder in kafka-acls.sh.
>> > >
>> > > I will update the KIP accordingly.
>> > >
>> > > Thanks,
>> > >
>> > > Mayuresh
>> > >
>> > > On Thu, Jan 26, 2017 at 6:01 PM, Mayuresh Gharat <
>> > > gharatmayures...@gmail.com> wrote:
>> > >
>> > >> Hi Dong,
>> > >>
>> > >> Thanks for the review. Please see the replies inline.
>> > >>
>> > >>
>> > >> 1. I am not sure we need to add the method
>> buildPrincipal(Map
>> > >> principalConfigs). It seems that user can simply do
>> > >> principalBuilder.configure(...).buildPrincipal(...) without using
>> that
>> > >> method.
>> > >> -> I am not sure if I understand the question.
>> > >> buildPrincipal(Map principalConfigs) will be used to build
>> > >> individual Principals from the passed in configs. Each Principal can
>> be
>> > >> different type and the PrincipalBuilder is responsible for handling
>> > those
>> > >> configs correctly and build those Principals.
>> > >>
>> > >> 2. Is there any reason specific reason that we should put the
>> > >> channelPrincipal in KafkaPrincipal class instead of the Session
>> class?
>> > If
>> > >> they work equally well to serve the use-case of this KIP, then it
>> seems
>> > >> better to put this field in the Session class to avoid changing
>> > interface
>> > >> that needs to be implemented by custom principal.
>> > >> -> Doing this might be backwards incompatible as we need to
>> > >> preserve the existing behavior of kafka-acls.sh. Also as we have
>> field
>> > of
>> > >> PrincipalType which can be used in future if Kafka decides to support
>> > >> different Principal types (currently it just says "User"), we might
>> > loose
>> > >> that functionality.
>> > >>
>> > >> Thanks,
>> > >>
>> > >> Mayuresh
>> > >>
>> > >>
>> > >> On Tue, Jan 24, 2017 at 3:35 PM, Dong Lin 
>> wrote:
>> > >>
>> > >>> Hey Mayuresh,
>> > >>>
>> > >>> Thanks for the KIP. I actually like the suggestions by Ismael and
>> Jun.
>> > >>> Here
>> > >>> are my comments:
>> > >>>
>> > >>> 1. I am not sure we need to add the method
>> buildPrincipal(Map> > ?>
>> > >>> principalConfigs). It seems that user can simply do
>> > >>> principalBuilder.configure(...).buildPrincipal(...) without using
>> that
>> > >>> method.
>> > >>>
>> > >>> 2. Is there any reason specific reason that we should put the
>> > >>> channelPrincipal in KafkaPrincipal class instead of the Session
>> class?
>> > If
>> > >>> they work equally well to serve the use-case of this KIP, then it
>> seems
>> > >>> better to put this field in the Session class to avoid changing
>> > interface
>> > >>> that needs to be implemented by custom principal.
>> > >>>
>> > >>> Dong
>> > 

Re: [DISCUSS] KIP-111 : Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.

[Mayuresh Gharat]

Hi Dong,

What I meant was "Right now Kafka just extracts the name out of the
Principal that is generated by the PrincipalBuilder. Instead of doing that
if it preserves the Principal itself, this issue can be addressed".

May be I should have used the word "preserve" instead of "stores". I have
updated the wording in the KIP.

Thanks,

Mayuresh

On Wed, Feb 1, 2017 at 8:30 PM, Dong Lin  wrote:

> The last paragraph of the motivation section is a bit confusing. I guess
> you want to say "This issue can be addressed if the Session class stores
> the Principal object extracted from a request".
>
> I like the approach of changing Session class to be case class
> *Session(principal:
> KafkaPrincipal, clientAddress: InetAddress)* under the assumption that the
> Session class doesn't really need principalType of the KafkaPrincipal. I am
> wondering if anyone in the open source mailing list knows why we need to
> have principalType in KafkaPrincipal.
>
> For the record, I actually prefer that we use the existing configure() to
> provide properties to PrincipalBuilder instead of adding the method
> *buildPrincipal(Map ?> principalConfigs)* in the PrincipalBuilder interface. But this is not a
> blocking issue for me.
>
>
>
>
> On Wed, Feb 1, 2017 at 2:54 PM, Mayuresh Gharat <
> gharatmayures...@gmail.com>
> wrote:
>
> > Hi All,
> >
> > I have updated the KIP as per our discussion here.
> > It would be great if you can take another look and let me know if there
> are
> > any concerns.
> >
> > Thanks,
> >
> > Mayuresh
> >
> > On Sat, Jan 28, 2017 at 6:10 PM, Mayuresh Gharat <
> > gharatmayures...@gmail.com
> > > wrote:
> >
> > > I had offline discussions with Joel, Dong and Radai.
> > >
> > > I agree that we can replace the KafkaPrincipal in Session with the
> > > ChannelPrincipal.
> > > KafkaPrincipal can be provided as an out of box implementation.
> > >
> > > The only gotcha will be users will have to implement there own
> > Authorizer,
> > > if they decide to use there own PrincipalBuilder in kafka-acls.sh.
> > >
> > > I will update the KIP accordingly.
> > >
> > > Thanks,
> > >
> > > Mayuresh
> > >
> > > On Thu, Jan 26, 2017 at 6:01 PM, Mayuresh Gharat <
> > > gharatmayures...@gmail.com> wrote:
> > >
> > >> Hi Dong,
> > >>
> > >> Thanks for the review. Please see the replies inline.
> > >>
> > >>
> > >> 1. I am not sure we need to add the method buildPrincipal(Map ?>
> > >> principalConfigs). It seems that user can simply do
> > >> principalBuilder.configure(...).buildPrincipal(...) without using
> that
> > >> method.
> > >> -> I am not sure if I understand the question.
> > >> buildPrincipal(Map principalConfigs) will be used to build
> > >> individual Principals from the passed in configs. Each Principal can
> be
> > >> different type and the PrincipalBuilder is responsible for handling
> > those
> > >> configs correctly and build those Principals.
> > >>
> > >> 2. Is there any reason specific reason that we should put the
> > >> channelPrincipal in KafkaPrincipal class instead of the Session class?
> > If
> > >> they work equally well to serve the use-case of this KIP, then it
> seems
> > >> better to put this field in the Session class to avoid changing
> > interface
> > >> that needs to be implemented by custom principal.
> > >> -> Doing this might be backwards incompatible as we need to
> > >> preserve the existing behavior of kafka-acls.sh. Also as we have field
> > of
> > >> PrincipalType which can be used in future if Kafka decides to support
> > >> different Principal types (currently it just says "User"), we might
> > loose
> > >> that functionality.
> > >>
> > >> Thanks,
> > >>
> > >> Mayuresh
> > >>
> > >>
> > >> On Tue, Jan 24, 2017 at 3:35 PM, Dong Lin 
> wrote:
> > >>
> > >>> Hey Mayuresh,
> > >>>
> > >>> Thanks for the KIP. I actually like the suggestions by Ismael and
> Jun.
> > >>> Here
> > >>> are my comments:
> > >>>
> > >>> 1. I am not sure we need to add the method buildPrincipal(Map > ?>
> > >>> principalConfigs). It seems that user can simply do
> > >>> principalBuilder.configure(...).buildPrincipal(...) without using
> that
> > >>> method.
> > >>>
> > >>> 2. Is there any reason specific reason that we should put the
> > >>> channelPrincipal in KafkaPrincipal class instead of the Session
> class?
> > If
> > >>> they work equally well to serve the use-case of this KIP, then it
> seems
> > >>> better to put this field in the Session class to avoid changing
> > interface
> > >>> that needs to be implemented by custom principal.
> > >>>
> > >>> Dong
> > >>>
> > >>>
> > >>> On Mon, Jan 23, 2017 at 5:55 PM, Mayuresh Gharat <
> > >>> gharatmayures...@gmail.com
> > >>> > wrote:
> > >>>
> > >>> > Hi Rajini,
> > >>> >
> > >>> > Thanks a lot for the review. Please see the comments inline :
> > >>> >
> > >>> > It feels like the goal is to expose custom Principal as an
> > >>> > opaque object between 

Re: [DISCUSS] KIP-111 : Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.

[Dong Lin]

The last paragraph of the motivation section is a bit confusing. I guess
you want to say "This issue can be addressed if the Session class stores
the Principal object extracted from a request".

I like the approach of changing Session class to be case class
*Session(principal:
KafkaPrincipal, clientAddress: InetAddress)* under the assumption that the
Session class doesn't really need principalType of the KafkaPrincipal. I am
wondering if anyone in the open source mailing list knows why we need to
have principalType in KafkaPrincipal.

For the record, I actually prefer that we use the existing configure() to
provide properties to PrincipalBuilder instead of adding the method
*buildPrincipal(Map principalConfigs)* in the PrincipalBuilder interface. But this is not a
blocking issue for me.




On Wed, Feb 1, 2017 at 2:54 PM, Mayuresh Gharat 
wrote:

> Hi All,
>
> I have updated the KIP as per our discussion here.
> It would be great if you can take another look and let me know if there are
> any concerns.
>
> Thanks,
>
> Mayuresh
>
> On Sat, Jan 28, 2017 at 6:10 PM, Mayuresh Gharat <
> gharatmayures...@gmail.com
> > wrote:
>
> > I had offline discussions with Joel, Dong and Radai.
> >
> > I agree that we can replace the KafkaPrincipal in Session with the
> > ChannelPrincipal.
> > KafkaPrincipal can be provided as an out of box implementation.
> >
> > The only gotcha will be users will have to implement there own
> Authorizer,
> > if they decide to use there own PrincipalBuilder in kafka-acls.sh.
> >
> > I will update the KIP accordingly.
> >
> > Thanks,
> >
> > Mayuresh
> >
> > On Thu, Jan 26, 2017 at 6:01 PM, Mayuresh Gharat <
> > gharatmayures...@gmail.com> wrote:
> >
> >> Hi Dong,
> >>
> >> Thanks for the review. Please see the replies inline.
> >>
> >>
> >> 1. I am not sure we need to add the method buildPrincipal(Map
> >> principalConfigs). It seems that user can simply do
> >> principalBuilder.configure(...).buildPrincipal(...) without using that
> >> method.
> >> -> I am not sure if I understand the question.
> >> buildPrincipal(Map principalConfigs) will be used to build
> >> individual Principals from the passed in configs. Each Principal can be
> >> different type and the PrincipalBuilder is responsible for handling
> those
> >> configs correctly and build those Principals.
> >>
> >> 2. Is there any reason specific reason that we should put the
> >> channelPrincipal in KafkaPrincipal class instead of the Session class?
> If
> >> they work equally well to serve the use-case of this KIP, then it seems
> >> better to put this field in the Session class to avoid changing
> interface
> >> that needs to be implemented by custom principal.
> >> -> Doing this might be backwards incompatible as we need to
> >> preserve the existing behavior of kafka-acls.sh. Also as we have field
> of
> >> PrincipalType which can be used in future if Kafka decides to support
> >> different Principal types (currently it just says "User"), we might
> loose
> >> that functionality.
> >>
> >> Thanks,
> >>
> >> Mayuresh
> >>
> >>
> >> On Tue, Jan 24, 2017 at 3:35 PM, Dong Lin  wrote:
> >>
> >>> Hey Mayuresh,
> >>>
> >>> Thanks for the KIP. I actually like the suggestions by Ismael and Jun.
> >>> Here
> >>> are my comments:
> >>>
> >>> 1. I am not sure we need to add the method buildPrincipal(Map ?>
> >>> principalConfigs). It seems that user can simply do
> >>> principalBuilder.configure(...).buildPrincipal(...) without using that
> >>> method.
> >>>
> >>> 2. Is there any reason specific reason that we should put the
> >>> channelPrincipal in KafkaPrincipal class instead of the Session class?
> If
> >>> they work equally well to serve the use-case of this KIP, then it seems
> >>> better to put this field in the Session class to avoid changing
> interface
> >>> that needs to be implemented by custom principal.
> >>>
> >>> Dong
> >>>
> >>>
> >>> On Mon, Jan 23, 2017 at 5:55 PM, Mayuresh Gharat <
> >>> gharatmayures...@gmail.com
> >>> > wrote:
> >>>
> >>> > Hi Rajini,
> >>> >
> >>> > Thanks a lot for the review. Please see the comments inline :
> >>> >
> >>> > It feels like the goal is to expose custom Principal as an
> >>> > opaque object between PrincipalBuilder and Authorizer so that Kafka
> >>> doesn't
> >>> > really need to know anything about additional stuff added for
> >>> > customization. But kafka-acls.sh is expecting a key-value map from
> >>> which
> >>> > Principal is constructed. This is a breaking change to the
> >>> PrincipalBuilder
> >>> > interface - and I am not sure what it achieves.
> >>> > -> kafka-acls is a commandline tool where in currently we just
> >>> specify
> >>> > the "names" of the principal that are allowed or denied.
> >>> > The Principal generated by PrincipalBuilder is still opaque and Kafka
> >>> as
> >>> > such does not need to know the details.
> >>> > The key-value map that is been 

Re: [DISCUSS] KIP-111 : Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.

[Mayuresh Gharat]

Hi All,

I have updated the KIP as per our discussion here.
It would be great if you can take another look and let me know if there are
any concerns.

Thanks,

Mayuresh

On Sat, Jan 28, 2017 at 6:10 PM, Mayuresh Gharat  wrote:

> I had offline discussions with Joel, Dong and Radai.
>
> I agree that we can replace the KafkaPrincipal in Session with the
> ChannelPrincipal.
> KafkaPrincipal can be provided as an out of box implementation.
>
> The only gotcha will be users will have to implement there own Authorizer,
> if they decide to use there own PrincipalBuilder in kafka-acls.sh.
>
> I will update the KIP accordingly.
>
> Thanks,
>
> Mayuresh
>
> On Thu, Jan 26, 2017 at 6:01 PM, Mayuresh Gharat <
> gharatmayures...@gmail.com> wrote:
>
>> Hi Dong,
>>
>> Thanks for the review. Please see the replies inline.
>>
>>
>> 1. I am not sure we need to add the method buildPrincipal(Map
>> principalConfigs). It seems that user can simply do
>> principalBuilder.configure(...).buildPrincipal(...) without using that
>> method.
>> -> I am not sure if I understand the question.
>> buildPrincipal(Map principalConfigs) will be used to build
>> individual Principals from the passed in configs. Each Principal can be
>> different type and the PrincipalBuilder is responsible for handling those
>> configs correctly and build those Principals.
>>
>> 2. Is there any reason specific reason that we should put the
>> channelPrincipal in KafkaPrincipal class instead of the Session class? If
>> they work equally well to serve the use-case of this KIP, then it seems
>> better to put this field in the Session class to avoid changing interface
>> that needs to be implemented by custom principal.
>> -> Doing this might be backwards incompatible as we need to
>> preserve the existing behavior of kafka-acls.sh. Also as we have field of
>> PrincipalType which can be used in future if Kafka decides to support
>> different Principal types (currently it just says "User"), we might loose
>> that functionality.
>>
>> Thanks,
>>
>> Mayuresh
>>
>>
>> On Tue, Jan 24, 2017 at 3:35 PM, Dong Lin  wrote:
>>
>>> Hey Mayuresh,
>>>
>>> Thanks for the KIP. I actually like the suggestions by Ismael and Jun.
>>> Here
>>> are my comments:
>>>
>>> 1. I am not sure we need to add the method buildPrincipal(Map
>>> principalConfigs). It seems that user can simply do
>>> principalBuilder.configure(...).buildPrincipal(...) without using that
>>> method.
>>>
>>> 2. Is there any reason specific reason that we should put the
>>> channelPrincipal in KafkaPrincipal class instead of the Session class? If
>>> they work equally well to serve the use-case of this KIP, then it seems
>>> better to put this field in the Session class to avoid changing interface
>>> that needs to be implemented by custom principal.
>>>
>>> Dong
>>>
>>>
>>> On Mon, Jan 23, 2017 at 5:55 PM, Mayuresh Gharat <
>>> gharatmayures...@gmail.com
>>> > wrote:
>>>
>>> > Hi Rajini,
>>> >
>>> > Thanks a lot for the review. Please see the comments inline :
>>> >
>>> > It feels like the goal is to expose custom Principal as an
>>> > opaque object between PrincipalBuilder and Authorizer so that Kafka
>>> doesn't
>>> > really need to know anything about additional stuff added for
>>> > customization. But kafka-acls.sh is expecting a key-value map from
>>> which
>>> > Principal is constructed. This is a breaking change to the
>>> PrincipalBuilder
>>> > interface - and I am not sure what it achieves.
>>> > -> kafka-acls is a commandline tool where in currently we just
>>> specify
>>> > the "names" of the principal that are allowed or denied.
>>> > The Principal generated by PrincipalBuilder is still opaque and Kafka
>>> as
>>> > such does not need to know the details.
>>> > The key-value map that is been passed in, will be used specifically by
>>> the
>>> > user PrincipalBuilder to create the Principal. The main motivation of
>>> the
>>> > KIP is that, the Principal built by the PrincipalBuilder can have other
>>> > fields apart from the "name", which are ignored currently. Allowing a
>>> > key-value pair to be passed in will enable the PrincipalBuilder to
>>> create
>>> > such type of Principal.
>>> >
>>> > 1. A custom Principal is (a) created during authentication using custom
>>> > PrincipalBuilder (b) checked during authorization using
>>> Principal.equals()
>>> > and (c) stored in Zookeeper using Principal.toString(). Is that
>>> correct?
>>> > -> The authorization will be done as per the user supplied
>>> Authorizer.
>>> > As not everyone might be using zookeeper for storing ACLs, its storage
>>> is
>>> > again Authorizer  implementation dependent.
>>> >
>>> > 2. Is the reason for the new parameters in kafka-acls.sh and the
>>> breaking
>>> > change in PrincipalBuilder interface to enable users to specify a
>>> Principal
>>> > using properties rather than create the String in 1c) themselves?
>>> > 

Re: [DISCUSS] KIP-111 : Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.

[Mayuresh Gharat]

I had offline discussions with Joel, Dong and Radai.

I agree that we can replace the KafkaPrincipal in Session with the
ChannelPrincipal.
KafkaPrincipal can be provided as an out of box implementation.

The only gotcha will be users will have to implement there own Authorizer,
if they decide to use there own PrincipalBuilder in kafka-acls.sh.

I will update the KIP accordingly.

Thanks,

Mayuresh

On Thu, Jan 26, 2017 at 6:01 PM, Mayuresh Gharat  wrote:

> Hi Dong,
>
> Thanks for the review. Please see the replies inline.
>
>
> 1. I am not sure we need to add the method buildPrincipal(Map
> principalConfigs). It seems that user can simply do
> principalBuilder.configure(...).buildPrincipal(...) without using that
> method.
> -> I am not sure if I understand the question.
> buildPrincipal(Map principalConfigs) will be used to build
> individual Principals from the passed in configs. Each Principal can be
> different type and the PrincipalBuilder is responsible for handling those
> configs correctly and build those Principals.
>
> 2. Is there any reason specific reason that we should put the
> channelPrincipal in KafkaPrincipal class instead of the Session class? If
> they work equally well to serve the use-case of this KIP, then it seems
> better to put this field in the Session class to avoid changing interface
> that needs to be implemented by custom principal.
> -> Doing this might be backwards incompatible as we need to
> preserve the existing behavior of kafka-acls.sh. Also as we have field of
> PrincipalType which can be used in future if Kafka decides to support
> different Principal types (currently it just says "User"), we might loose
> that functionality.
>
> Thanks,
>
> Mayuresh
>
>
> On Tue, Jan 24, 2017 at 3:35 PM, Dong Lin  wrote:
>
>> Hey Mayuresh,
>>
>> Thanks for the KIP. I actually like the suggestions by Ismael and Jun.
>> Here
>> are my comments:
>>
>> 1. I am not sure we need to add the method buildPrincipal(Map
>> principalConfigs). It seems that user can simply do
>> principalBuilder.configure(...).buildPrincipal(...) without using that
>> method.
>>
>> 2. Is there any reason specific reason that we should put the
>> channelPrincipal in KafkaPrincipal class instead of the Session class? If
>> they work equally well to serve the use-case of this KIP, then it seems
>> better to put this field in the Session class to avoid changing interface
>> that needs to be implemented by custom principal.
>>
>> Dong
>>
>>
>> On Mon, Jan 23, 2017 at 5:55 PM, Mayuresh Gharat <
>> gharatmayures...@gmail.com
>> > wrote:
>>
>> > Hi Rajini,
>> >
>> > Thanks a lot for the review. Please see the comments inline :
>> >
>> > It feels like the goal is to expose custom Principal as an
>> > opaque object between PrincipalBuilder and Authorizer so that Kafka
>> doesn't
>> > really need to know anything about additional stuff added for
>> > customization. But kafka-acls.sh is expecting a key-value map from which
>> > Principal is constructed. This is a breaking change to the
>> PrincipalBuilder
>> > interface - and I am not sure what it achieves.
>> > -> kafka-acls is a commandline tool where in currently we just
>> specify
>> > the "names" of the principal that are allowed or denied.
>> > The Principal generated by PrincipalBuilder is still opaque and Kafka as
>> > such does not need to know the details.
>> > The key-value map that is been passed in, will be used specifically by
>> the
>> > user PrincipalBuilder to create the Principal. The main motivation of
>> the
>> > KIP is that, the Principal built by the PrincipalBuilder can have other
>> > fields apart from the "name", which are ignored currently. Allowing a
>> > key-value pair to be passed in will enable the PrincipalBuilder to
>> create
>> > such type of Principal.
>> >
>> > 1. A custom Principal is (a) created during authentication using custom
>> > PrincipalBuilder (b) checked during authorization using
>> Principal.equals()
>> > and (c) stored in Zookeeper using Principal.toString(). Is that correct?
>> > -> The authorization will be done as per the user supplied
>> Authorizer.
>> > As not everyone might be using zookeeper for storing ACLs, its storage
>> is
>> > again Authorizer  implementation dependent.
>> >
>> > 2. Is the reason for the new parameters in kafka-acls.sh and the
>> breaking
>> > change in PrincipalBuilder interface to enable users to specify a
>> Principal
>> > using properties rather than create the String in 1c) themselves?
>> > -> Please see the explanation above.
>> >
>> > 3. Since the purpose of the new PrincipalBuilder method
>> > buildPrincipal(Map> > ?> principalConfigs) is to create a new Principal from command line
>> > parameters, perhaps Properties or Map would be more
>> > appropriate?
>> > -> Yes we can, but I actually prefer to keep it similar to
>> > configure(Map 

Re: [DISCUSS] KIP-111 : Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.

[Mayuresh Gharat]

Hi Dong,

Thanks for the review. Please see the replies inline.


1. I am not sure we need to add the method buildPrincipal(Map
principalConfigs). It seems that user can simply do
principalBuilder.configure(...).buildPrincipal(...) without using that
method.
-> I am not sure if I understand the question.
buildPrincipal(Map principalConfigs) will be used to build
individual Principals from the passed in configs. Each Principal can be
different type and the PrincipalBuilder is responsible for handling those
configs correctly and build those Principals.

2. Is there any reason specific reason that we should put the
channelPrincipal in KafkaPrincipal class instead of the Session class? If
they work equally well to serve the use-case of this KIP, then it seems
better to put this field in the Session class to avoid changing interface
that needs to be implemented by custom principal.
-> Doing this might be backwards incompatible as we need to
preserve the existing behavior of kafka-acls.sh. Also as we have field of
PrincipalType which can be used in future if Kafka decides to support
different Principal types (currently it just says "User"), we might loose
that functionality.

Thanks,

Mayuresh


On Tue, Jan 24, 2017 at 3:35 PM, Dong Lin  wrote:

> Hey Mayuresh,
>
> Thanks for the KIP. I actually like the suggestions by Ismael and Jun. Here
> are my comments:
>
> 1. I am not sure we need to add the method buildPrincipal(Map
> principalConfigs). It seems that user can simply do
> principalBuilder.configure(...).buildPrincipal(...) without using that
> method.
>
> 2. Is there any reason specific reason that we should put the
> channelPrincipal in KafkaPrincipal class instead of the Session class? If
> they work equally well to serve the use-case of this KIP, then it seems
> better to put this field in the Session class to avoid changing interface
> that needs to be implemented by custom principal.
>
> Dong
>
>
> On Mon, Jan 23, 2017 at 5:55 PM, Mayuresh Gharat <
> gharatmayures...@gmail.com
> > wrote:
>
> > Hi Rajini,
> >
> > Thanks a lot for the review. Please see the comments inline :
> >
> > It feels like the goal is to expose custom Principal as an
> > opaque object between PrincipalBuilder and Authorizer so that Kafka
> doesn't
> > really need to know anything about additional stuff added for
> > customization. But kafka-acls.sh is expecting a key-value map from which
> > Principal is constructed. This is a breaking change to the
> PrincipalBuilder
> > interface - and I am not sure what it achieves.
> > -> kafka-acls is a commandline tool where in currently we just
> specify
> > the "names" of the principal that are allowed or denied.
> > The Principal generated by PrincipalBuilder is still opaque and Kafka as
> > such does not need to know the details.
> > The key-value map that is been passed in, will be used specifically by
> the
> > user PrincipalBuilder to create the Principal. The main motivation of the
> > KIP is that, the Principal built by the PrincipalBuilder can have other
> > fields apart from the "name", which are ignored currently. Allowing a
> > key-value pair to be passed in will enable the PrincipalBuilder to create
> > such type of Principal.
> >
> > 1. A custom Principal is (a) created during authentication using custom
> > PrincipalBuilder (b) checked during authorization using
> Principal.equals()
> > and (c) stored in Zookeeper using Principal.toString(). Is that correct?
> > -> The authorization will be done as per the user supplied
> Authorizer.
> > As not everyone might be using zookeeper for storing ACLs, its storage is
> > again Authorizer  implementation dependent.
> >
> > 2. Is the reason for the new parameters in kafka-acls.sh and the breaking
> > change in PrincipalBuilder interface to enable users to specify a
> Principal
> > using properties rather than create the String in 1c) themselves?
> > -> Please see the explanation above.
> >
> > 3. Since the purpose of the new PrincipalBuilder method
> > buildPrincipal(Map > ?> principalConfigs) is to create a new Principal from command line
> > parameters, perhaps Properties or Map would be more
> > appropriate?
> > -> Yes we can, but I actually prefer to keep it similar to
> > configure(Map configs) API.
> >
> >
> > Hi Ismael,
> >
> > Thanks a lot for the review. Please see the comments inline.
> >
> > 1. PrincipalBuilder implements Configurable and gets a map of properties
> > via the `configure` method. Do we really need a new `buildPrincipal`
> method
> > given that?
> > --> The configure() API will actually be used to configure the
> > PrincipalBuilder in the same way as the Authorizer. The buildPrincipal()
> > API will be used by the PrincipalBuilder to build individual principals.
> > Each of these principals can be of different custom types like
> > GroupPrincipals, ServicePrincipals and so on, based 

Re: [DISCUSS] KIP-111 : Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.

[Dong Lin]

Hey Mayuresh,

Thanks for the KIP. I actually like the suggestions by Ismael and Jun. Here
are my comments:

1. I am not sure we need to add the method buildPrincipal(Map
principalConfigs). It seems that user can simply do
principalBuilder.configure(...).buildPrincipal(...) without using that
method.

2. Is there any reason specific reason that we should put the
channelPrincipal in KafkaPrincipal class instead of the Session class? If
they work equally well to serve the use-case of this KIP, then it seems
better to put this field in the Session class to avoid changing interface
that needs to be implemented by custom principal.

Dong


On Mon, Jan 23, 2017 at 5:55 PM, Mayuresh Gharat  wrote:

> Hi Rajini,
>
> Thanks a lot for the review. Please see the comments inline :
>
> It feels like the goal is to expose custom Principal as an
> opaque object between PrincipalBuilder and Authorizer so that Kafka doesn't
> really need to know anything about additional stuff added for
> customization. But kafka-acls.sh is expecting a key-value map from which
> Principal is constructed. This is a breaking change to the PrincipalBuilder
> interface - and I am not sure what it achieves.
> -> kafka-acls is a commandline tool where in currently we just specify
> the "names" of the principal that are allowed or denied.
> The Principal generated by PrincipalBuilder is still opaque and Kafka as
> such does not need to know the details.
> The key-value map that is been passed in, will be used specifically by the
> user PrincipalBuilder to create the Principal. The main motivation of the
> KIP is that, the Principal built by the PrincipalBuilder can have other
> fields apart from the "name", which are ignored currently. Allowing a
> key-value pair to be passed in will enable the PrincipalBuilder to create
> such type of Principal.
>
> 1. A custom Principal is (a) created during authentication using custom
> PrincipalBuilder (b) checked during authorization using Principal.equals()
> and (c) stored in Zookeeper using Principal.toString(). Is that correct?
> -> The authorization will be done as per the user supplied Authorizer.
> As not everyone might be using zookeeper for storing ACLs, its storage is
> again Authorizer  implementation dependent.
>
> 2. Is the reason for the new parameters in kafka-acls.sh and the breaking
> change in PrincipalBuilder interface to enable users to specify a Principal
> using properties rather than create the String in 1c) themselves?
> -> Please see the explanation above.
>
> 3. Since the purpose of the new PrincipalBuilder method
> buildPrincipal(Map ?> principalConfigs) is to create a new Principal from command line
> parameters, perhaps Properties or Map would be more
> appropriate?
> -> Yes we can, but I actually prefer to keep it similar to
> configure(Map configs) API.
>
>
> Hi Ismael,
>
> Thanks a lot for the review. Please see the comments inline.
>
> 1. PrincipalBuilder implements Configurable and gets a map of properties
> via the `configure` method. Do we really need a new `buildPrincipal` method
> given that?
> --> The configure() API will actually be used to configure the
> PrincipalBuilder in the same way as the Authorizer. The buildPrincipal()
> API will be used by the PrincipalBuilder to build individual principals.
> Each of these principals can be of different custom types like
> GroupPrincipals, ServicePrincipals and so on, based on the Map
> principalConfigs provided to the buildPrincipal() API.
>
> 2. Jun suggested in the JIRA that it may make sense to pass the
> `channelPrincipal` as a field in `Session` instead of `KafkaPrincipal`. It
> would be good to understand why this was rejected.
> -> Now I understand what Jun meant by "Perhaps, we could extend the
> Session object with channelPrincipal instead.". Actually thinking more on
> this, there is a PrincipalType in KafkaPrincipal, that was inserted for a
> specific purpose when it was created for the first time, I think. I thought
> that we should preserve it, if its useful for future.
>
> Thanks,
>
> Mayuresh
>
>
>
>
>
> On Mon, Jan 23, 2017 at 8:56 AM, Ismael Juma  wrote:
>
> > Hi Mayuresh,
> >
> > Thanks for updating the KIP. A couple of questions:
> >
> > 1. PrincipalBuilder implements Configurable and gets a map of properties
> > via the `configure` method. Do we really need a new `buildPrincipal`
> method
> > given that?
> >
> > 2. Jun suggested in the JIRA that it may make sense to pass the
> > `channelPrincipal` as a field in `Session` instead of `KafkaPrincipal`.
> It
> > would be good to understand why this was rejected.
> >
> > Ismael
> >
> > On Thu, Jan 12, 2017 at 7:07 PM, Ismael Juma  wrote:
> >
> > > Hi Mayuresh,
> > >
> > > Thanks for the KIP. A quick comment before I do a more detailed
> analysis,
> > > the KIP says:
> > >
> > > `This KIP is a pure addition to 

Re: [DISCUSS] KIP-111 : Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.

[Mayuresh Gharat]

Hi Rajini,

Thanks a lot for the review. Please see the comments inline :

It feels like the goal is to expose custom Principal as an
opaque object between PrincipalBuilder and Authorizer so that Kafka doesn't
really need to know anything about additional stuff added for
customization. But kafka-acls.sh is expecting a key-value map from which
Principal is constructed. This is a breaking change to the PrincipalBuilder
interface - and I am not sure what it achieves.
-> kafka-acls is a commandline tool where in currently we just specify
the "names" of the principal that are allowed or denied.
The Principal generated by PrincipalBuilder is still opaque and Kafka as
such does not need to know the details.
The key-value map that is been passed in, will be used specifically by the
user PrincipalBuilder to create the Principal. The main motivation of the
KIP is that, the Principal built by the PrincipalBuilder can have other
fields apart from the "name", which are ignored currently. Allowing a
key-value pair to be passed in will enable the PrincipalBuilder to create
such type of Principal.

1. A custom Principal is (a) created during authentication using custom
PrincipalBuilder (b) checked during authorization using Principal.equals()
and (c) stored in Zookeeper using Principal.toString(). Is that correct?
-> The authorization will be done as per the user supplied Authorizer.
As not everyone might be using zookeeper for storing ACLs, its storage is
again Authorizer  implementation dependent.

2. Is the reason for the new parameters in kafka-acls.sh and the breaking
change in PrincipalBuilder interface to enable users to specify a Principal
using properties rather than create the String in 1c) themselves?
-> Please see the explanation above.

3. Since the purpose of the new PrincipalBuilder method
buildPrincipal(Map principalConfigs) is to create a new Principal from command line
parameters, perhaps Properties or Map would be more
appropriate?
-> Yes we can, but I actually prefer to keep it similar to
configure(Map configs) API.


Hi Ismael,

Thanks a lot for the review. Please see the comments inline.

1. PrincipalBuilder implements Configurable and gets a map of properties
via the `configure` method. Do we really need a new `buildPrincipal` method
given that?
--> The configure() API will actually be used to configure the
PrincipalBuilder in the same way as the Authorizer. The buildPrincipal()
API will be used by the PrincipalBuilder to build individual principals.
Each of these principals can be of different custom types like
GroupPrincipals, ServicePrincipals and so on, based on the Map
principalConfigs provided to the buildPrincipal() API.

2. Jun suggested in the JIRA that it may make sense to pass the
`channelPrincipal` as a field in `Session` instead of `KafkaPrincipal`. It
would be good to understand why this was rejected.
-> Now I understand what Jun meant by "Perhaps, we could extend the
Session object with channelPrincipal instead.". Actually thinking more on
this, there is a PrincipalType in KafkaPrincipal, that was inserted for a
specific purpose when it was created for the first time, I think. I thought
that we should preserve it, if its useful for future.

Thanks,

Mayuresh





On Mon, Jan 23, 2017 at 8:56 AM, Ismael Juma  wrote:

> Hi Mayuresh,
>
> Thanks for updating the KIP. A couple of questions:
>
> 1. PrincipalBuilder implements Configurable and gets a map of properties
> via the `configure` method. Do we really need a new `buildPrincipal` method
> given that?
>
> 2. Jun suggested in the JIRA that it may make sense to pass the
> `channelPrincipal` as a field in `Session` instead of `KafkaPrincipal`. It
> would be good to understand why this was rejected.
>
> Ismael
>
> On Thu, Jan 12, 2017 at 7:07 PM, Ismael Juma  wrote:
>
> > Hi Mayuresh,
> >
> > Thanks for the KIP. A quick comment before I do a more detailed analysis,
> > the KIP says:
> >
> > `This KIP is a pure addition to existing functionality and does not
> > include any backward incompatible changes.`
> >
> > However, the KIP is proposing the addition of a method to the
> > PrincipalBuilder pluggable interface, which is not a compatible change.
> > Existing implementations would no longer compile, for example. It would
> be
> > good to make this clear in the KIP.
> >
> > Ismael
> >
> > On Thu, Jan 12, 2017 at 5:44 PM, Mayuresh Gharat <
> > gharatmayures...@gmail.com> wrote:
> >
> >> Hi all.
> >>
> >> We created KIP-111 to propose that Kafka should preserve the Principal
> >> generated by the PrincipalBuilder while processing the request received
> on
> >> socket channel, on the broker.
> >>
> >> Please find the KIP wiki in the link
> >> https://cwiki.apache.org/confluence/pages/viewpage.
> action?pageId=67638388
> >> .
> >> We would love to hear your comments and suggestions.
> >>
> >>
> >> Thanks,
> >>
> >> Mayuresh
> >>
> >

Re: [DISCUSS] KIP-111 : Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.

[Ismael Juma]

Hi Mayuresh,

Thanks for updating the KIP. A couple of questions:

1. PrincipalBuilder implements Configurable and gets a map of properties
via the `configure` method. Do we really need a new `buildPrincipal` method
given that?

2. Jun suggested in the JIRA that it may make sense to pass the
`channelPrincipal` as a field in `Session` instead of `KafkaPrincipal`. It
would be good to understand why this was rejected.

Ismael

On Thu, Jan 12, 2017 at 7:07 PM, Ismael Juma  wrote:

> Hi Mayuresh,
>
> Thanks for the KIP. A quick comment before I do a more detailed analysis,
> the KIP says:
>
> `This KIP is a pure addition to existing functionality and does not
> include any backward incompatible changes.`
>
> However, the KIP is proposing the addition of a method to the
> PrincipalBuilder pluggable interface, which is not a compatible change.
> Existing implementations would no longer compile, for example. It would be
> good to make this clear in the KIP.
>
> Ismael
>
> On Thu, Jan 12, 2017 at 5:44 PM, Mayuresh Gharat <
> gharatmayures...@gmail.com> wrote:
>
>> Hi all.
>>
>> We created KIP-111 to propose that Kafka should preserve the Principal
>> generated by the PrincipalBuilder while processing the request received on
>> socket channel, on the broker.
>>
>> Please find the KIP wiki in the link
>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=67638388
>> .
>> We would love to hear your comments and suggestions.
>>
>>
>> Thanks,
>>
>> Mayuresh
>>
>
>

Re: [DISCUSS] KIP-111 : Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.

[Rajini Sivaram]

Hi Mayuresh,

The part about exposing Principal from custom PrincipalBuilder to custom
Authorizer sounds good. The definition of ACLs using kafka-acls.sh is less
clear to me. It feels like the goal is to expose custom Principal as an
opaque object between PrincipalBuilder and Authorizer so that Kafka doesn't
really need to know anything about additional stuff added for
customization. But kafka-acls.sh is expecting a key-value map from which
Principal is constructed. This is a breaking change to the PrincipalBuilder
interface - and I am not sure what it achieves.

1. A custom Principal is (a) created during authentication using custom
PrincipalBuilder (b) checked during authorization using Principal.equals()
and (c) stored in Zookeeper using Principal.toString(). Is that correct?
2. Is the reason for the new parameters in kafka-acls.sh and the breaking
change in PrincipalBuilder interface to enable users to specify a Principal
using properties rather than create the String in 1c) themselves?
3. Since the purpose of the new PrincipalBuilder method
buildPrincipal(Map principalConfigs) is to create a new Principal from command line
parameters, perhaps Properties or Map would be more
appropriate?

Regards,

Rajini


On Sat, Jan 21, 2017 at 5:50 PM, radai  wrote:

> LGTM.
>
> Kafka currently allows setting both a custom PrincipalBuilder and a custom
> Authorizer (expected to act on the output of the principal builder) but
> makes the naive assumption that any and all information about a (custom)
> principal is solely contained in its name property. this kip addresses
> that.
>
> On Fri, Jan 20, 2017 at 4:15 PM, Mayuresh Gharat <
> gharatmayures...@gmail.com
> > wrote:
>
> > Hi,
> >
> > Just wanted to see if anyone had any concerns with this KIP.
> > I would like to put this to vote soon, if there are no concerns.
> >
> > Thanks,
> >
> > Mayuresh
> >
> > On Thu, Jan 12, 2017 at 11:21 AM, Mayuresh Gharat <
> > gharatmayures...@gmail.com> wrote:
> >
> > > Hi Ismael,
> > >
> > > Fair point. I will update it.
> > >
> > > Thanks,
> > >
> > > Mayuresh
> > >
> > > On Thu, Jan 12, 2017 at 11:07 AM, Ismael Juma 
> wrote:
> > >
> > >> Hi Mayuresh,
> > >>
> > >> Thanks for the KIP. A quick comment before I do a more detailed
> > analysis,
> > >> the KIP says:
> > >>
> > >> `This KIP is a pure addition to existing functionality and does not
> > >> include
> > >> any backward incompatible changes.`
> > >>
> > >> However, the KIP is proposing the addition of a method to the
> > >> PrincipalBuilder pluggable interface, which is not a compatible
> change.
> > >> Existing implementations would no longer compile, for example. It
> would
> > be
> > >> good to make this clear in the KIP.
> > >>
> > >> Ismael
> > >>
> > >> On Thu, Jan 12, 2017 at 5:44 PM, Mayuresh Gharat <
> > >> gharatmayures...@gmail.com
> > >> > wrote:
> > >>
> > >> > Hi all.
> > >> >
> > >> > We created KIP-111 to propose that Kafka should preserve the
> Principal
> > >> > generated by the PrincipalBuilder while processing the request
> > received
> > >> on
> > >> > socket channel, on the broker.
> > >> >
> > >> > Please find the KIP wiki in the link
> > >> > https://cwiki.apache.org/confluence/pages/viewpage.action?
> > >> pageId=67638388.
> > >> > We would love to hear your comments and suggestions.
> > >> >
> > >> >
> > >> > Thanks,
> > >> >
> > >> > Mayuresh
> > >> >
> > >>
> > >
> > >
> > >
> > > --
> > > -Regards,
> > > Mayuresh R. Gharat
> > > (862) 250-7125
> > >
> >
> >
> >
> > --
> > -Regards,
> > Mayuresh R. Gharat
> > (862) 250-7125
> >
>

Re: [DISCUSS] KIP-111 : Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.

[radai]

LGTM.

Kafka currently allows setting both a custom PrincipalBuilder and a custom
Authorizer (expected to act on the output of the principal builder) but
makes the naive assumption that any and all information about a (custom)
principal is solely contained in its name property. this kip addresses that.

On Fri, Jan 20, 2017 at 4:15 PM, Mayuresh Gharat  wrote:

> Hi,
>
> Just wanted to see if anyone had any concerns with this KIP.
> I would like to put this to vote soon, if there are no concerns.
>
> Thanks,
>
> Mayuresh
>
> On Thu, Jan 12, 2017 at 11:21 AM, Mayuresh Gharat <
> gharatmayures...@gmail.com> wrote:
>
> > Hi Ismael,
> >
> > Fair point. I will update it.
> >
> > Thanks,
> >
> > Mayuresh
> >
> > On Thu, Jan 12, 2017 at 11:07 AM, Ismael Juma  wrote:
> >
> >> Hi Mayuresh,
> >>
> >> Thanks for the KIP. A quick comment before I do a more detailed
> analysis,
> >> the KIP says:
> >>
> >> `This KIP is a pure addition to existing functionality and does not
> >> include
> >> any backward incompatible changes.`
> >>
> >> However, the KIP is proposing the addition of a method to the
> >> PrincipalBuilder pluggable interface, which is not a compatible change.
> >> Existing implementations would no longer compile, for example. It would
> be
> >> good to make this clear in the KIP.
> >>
> >> Ismael
> >>
> >> On Thu, Jan 12, 2017 at 5:44 PM, Mayuresh Gharat <
> >> gharatmayures...@gmail.com
> >> > wrote:
> >>
> >> > Hi all.
> >> >
> >> > We created KIP-111 to propose that Kafka should preserve the Principal
> >> > generated by the PrincipalBuilder while processing the request
> received
> >> on
> >> > socket channel, on the broker.
> >> >
> >> > Please find the KIP wiki in the link
> >> > https://cwiki.apache.org/confluence/pages/viewpage.action?
> >> pageId=67638388.
> >> > We would love to hear your comments and suggestions.
> >> >
> >> >
> >> > Thanks,
> >> >
> >> > Mayuresh
> >> >
> >>
> >
> >
> >
> > --
> > -Regards,
> > Mayuresh R. Gharat
> > (862) 250-7125
> >
>
>
>
> --
> -Regards,
> Mayuresh R. Gharat
> (862) 250-7125
>

Re: [DISCUSS] KIP-111 : Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.

[Mayuresh Gharat]

Hi,

Just wanted to see if anyone had any concerns with this KIP.
I would like to put this to vote soon, if there are no concerns.

Thanks,

Mayuresh

On Thu, Jan 12, 2017 at 11:21 AM, Mayuresh Gharat <
gharatmayures...@gmail.com> wrote:

> Hi Ismael,
>
> Fair point. I will update it.
>
> Thanks,
>
> Mayuresh
>
> On Thu, Jan 12, 2017 at 11:07 AM, Ismael Juma  wrote:
>
>> Hi Mayuresh,
>>
>> Thanks for the KIP. A quick comment before I do a more detailed analysis,
>> the KIP says:
>>
>> `This KIP is a pure addition to existing functionality and does not
>> include
>> any backward incompatible changes.`
>>
>> However, the KIP is proposing the addition of a method to the
>> PrincipalBuilder pluggable interface, which is not a compatible change.
>> Existing implementations would no longer compile, for example. It would be
>> good to make this clear in the KIP.
>>
>> Ismael
>>
>> On Thu, Jan 12, 2017 at 5:44 PM, Mayuresh Gharat <
>> gharatmayures...@gmail.com
>> > wrote:
>>
>> > Hi all.
>> >
>> > We created KIP-111 to propose that Kafka should preserve the Principal
>> > generated by the PrincipalBuilder while processing the request received
>> on
>> > socket channel, on the broker.
>> >
>> > Please find the KIP wiki in the link
>> > https://cwiki.apache.org/confluence/pages/viewpage.action?
>> pageId=67638388.
>> > We would love to hear your comments and suggestions.
>> >
>> >
>> > Thanks,
>> >
>> > Mayuresh
>> >
>>
>
>
>
> --
> -Regards,
> Mayuresh R. Gharat
> (862) 250-7125
>



-- 
-Regards,
Mayuresh R. Gharat
(862) 250-7125

Re: [DISCUSS] KIP-111 : Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.

[Mayuresh Gharat]

Hi Ismael,

Fair point. I will update it.

Thanks,

Mayuresh

On Thu, Jan 12, 2017 at 11:07 AM, Ismael Juma  wrote:

> Hi Mayuresh,
>
> Thanks for the KIP. A quick comment before I do a more detailed analysis,
> the KIP says:
>
> `This KIP is a pure addition to existing functionality and does not include
> any backward incompatible changes.`
>
> However, the KIP is proposing the addition of a method to the
> PrincipalBuilder pluggable interface, which is not a compatible change.
> Existing implementations would no longer compile, for example. It would be
> good to make this clear in the KIP.
>
> Ismael
>
> On Thu, Jan 12, 2017 at 5:44 PM, Mayuresh Gharat <
> gharatmayures...@gmail.com
> > wrote:
>
> > Hi all.
> >
> > We created KIP-111 to propose that Kafka should preserve the Principal
> > generated by the PrincipalBuilder while processing the request received
> on
> > socket channel, on the broker.
> >
> > Please find the KIP wiki in the link
> > https://cwiki.apache.org/confluence/pages/viewpage.
> action?pageId=67638388.
> > We would love to hear your comments and suggestions.
> >
> >
> > Thanks,
> >
> > Mayuresh
> >
>



-- 
-Regards,
Mayuresh R. Gharat
(862) 250-7125

Re: [DISCUSS] KIP-111 : Kafka should preserve the Principal generated by the PrincipalBuilder while processing the request received on socket channel, on the broker.

[Ismael Juma]

Hi Mayuresh,

Thanks for the KIP. A quick comment before I do a more detailed analysis,
the KIP says:

`This KIP is a pure addition to existing functionality and does not include
any backward incompatible changes.`

However, the KIP is proposing the addition of a method to the
PrincipalBuilder pluggable interface, which is not a compatible change.
Existing implementations would no longer compile, for example. It would be
good to make this clear in the KIP.

Ismael

On Thu, Jan 12, 2017 at 5:44 PM, Mayuresh Gharat  wrote:

> Hi all.
>
> We created KIP-111 to propose that Kafka should preserve the Principal
> generated by the PrincipalBuilder while processing the request received on
> socket channel, on the broker.
>
> Please find the KIP wiki in the link
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=67638388.
> We would love to hear your comments and suggestions.
>
>
> Thanks,
>
> Mayuresh
>