Hi Jun, Thanks for the call out to the KIP-4 Authorizer work. I will discuss it with Ashish and let him follow up here.
Thanks, Grant On Sun, Apr 3, 2016 at 10:37 AM, Jun Rao <j...@confluent.io> wrote: > Ashish, > > For the two benefits that you listed, fail fast can be done just in the > implementation w/o getSupportedPrincipalTypes(). For avoiding the same > check in all implementations, I think Grant is thinking of adding some ACL > RPC request/response btw the client and broker directly, instead of writing > the ACL to ZK directly in the future. If we do that, the right place to do > any sanity check is probably in each implementation instead of in > AclCommand since AclCommand won't necessarily be the only entry point for > changing ACLs. So, I am still not quite convinced that we should > getSupportedPrincipalTypes() > right now. It's easy to add a new method to the interface, but hard to > remove/change an existing method. Grant, could you comment on the latter? > > Thanks, > > Jun > > On Sat, Apr 2, 2016 at 11:43 AM, Ashish Singh <asi...@cloudera.com> wrote: > > > Hello Jun, > > > > > > On Fri, Apr 1, 2016 at 9:57 PM, Jun Rao <j...@confluent.io> wrote: > > > > > Ashish, > > > > > > Thanks for the KIP. > > > > > > It seems that a specific implementation of Authorizer can reject > invalid > > > user type in addAcl/removeAcl without needing the new > > > getSupportedPrincipalTypes() > > > method, right? It's probably useful to provide the supported user types > > as > > > information through CLI (e.g., when --help is specified). Then, there > may > > > other information that a specific authorizer may want to provide. So, > if > > > this is just informational, would it be better to add sth like > > > getDescription() in the Authorizer interface and expose that through > CLI? > > > > > Providing information is definitely an important reason, some other > reasons > > were to fail fast and to avoid same check in all implementations. I agree > > having a generic getDescription() will be handy for authorizer > > implementations to provide more implementation specific info, including > > supported principal types and more. However, do you think other two > reasons > > I mentioned can convince you for current proposal? > > > > > > > > Jun > > > > > > 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 > > > -- Grant Henke Software Engineer | Cloudera gr...@cloudera.com | twitter.com/gchenke | linkedin.com/in/granthenke
