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<String, ?> principalConfigs) is to create a new Principal from command line parameters, perhaps Properties or Map<String, String> would be more appropriate? Regards, Rajini On Sat, Jan 21, 2017 at 5:50 PM, radai <radai.rosenbl...@gmail.com> 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 <ism...@juma.me.uk> > 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 > > >
