Harsha, No, ZK data format for acls or SimplaAclAthorizer implementation is not proposed to be changed.
Gwen, The concern I was pointing at is that if we choose to go with Option 2, i.e., move interface to a separate package, authorizer implementation will have to be updated along with broker upgrade and authorizer implementations need to make changes. I am not sure how this affects rolling upgrade. Harsha, do you mind sharing your concerns on rolling upgrade? On Thu, Apr 7, 2016 at 12:52 PM, Harsha <ka...@harsha.io> wrote: > Ashish, > Thanks for the details. We are not changing any of the zookeeper > data format for acls right? > > Thanks, > Harsha > > On Thu, Apr 7, 2016, at 11:25 AM, Gwen Shapira wrote: > > Can you guys go into details on what will happen during a rolling upgrade > > exactly? > > > > Gwen > > > > On Thu, Apr 7, 2016 at 11:21 AM, Ashish Singh <asi...@cloudera.com> > > wrote: > > > > > Hello Harsha, > > > > > > On Thu, Apr 7, 2016 at 11:03 AM, Harsha <m...@harsha.io> wrote: > > > > > > "My only ask is to have this in 0.10. As Jay pointed out, right now > > > > there > > > > are not many implementations out there, we might want to fix it > ASAP." > > > > > > > > Probably there aren't many implementations but there are lot of users > > > > using these implementations in production clusters. > > > > Isn't this going to break the rolling upgrade? > > > > > > It will and it is a concern, in my previous mail I have mentioned this > as > > > an issue if we choose to go this route. However, if we actually decide > to > > > do this, I would say it is better to do it sooner than later, as fewer > > > implementations will be affected. Below is excerpt from my previous > mail. > > > > > > Increase scope of KIP-50 to move authorizer and related classes to a > > > separate package. The new package will have java interface. This will > allow > > > implementations to not depend on kafka core and just on authorizer > package, > > > make authorization interface follow kafka’s coding standards and will > allow > > > java implementations to be cleaner. We can either completely drop scala > > > interface, which might be a pain for existing implementations, or we > can > > > have scala interface wrap java interface. Later allows a cleaner > > > deprecation path for existing scala authorizer interface, however it > may or > > > may not be feasible as Kafka server will have to somehow decide which > > > interface it should be looking for while loading authorizer > implementation, > > > this can probably be solved with a config or some reflection. If we > choose > > > to go this route, I can dig deeper. > > > > > > If we go with option 2 and commit on getting this in ASAP, preferably > in > > > 0.10, there will be fewer implementations that will be affected. > > > > > > and also moving to Java , > > > > a authorizer implementation going to run inside a KafkaBroker and I > > > > don't see why this is necessary to move to clients package. > > > > Are we planning on introducing common module to have it independent > of > > > > broker and client code? > > > > > > > Yes, I think that would take away the requirement of depending on Kafka > > > core from authorizer implementations. Do you suggest otherwise? > > > > > > > > > > -Harsha > > > > > > > > On Thu, Apr 7, 2016, at 10:52 AM, Ashish Singh wrote: > > > > > We might want to take a call here. Following are the options. > > > > > > > > > > 1. Let KIP-50 be the way it is, i.e., just add getDescription() > to > > > > > existing scala authorizer interface. It will break binary > > > > > compatibility > > > > > (only when using CLI and/or AdminCommand from >= 0.10 against > > > > > authorizer > > > > > implementations based on 0.9.). If we go this route, it is a > simpler > > > > > change > > > > > and existing implementations won’t have to change anything on > their > > > > > end. > > > > > 2. Increase scope of KIP-50 to move authorizer and related > classes > > > to > > > > > a > > > > > separate package. The new package will have java interface. This > > > will > > > > > allow > > > > > implementations to not depend on kafka core and just on > authorizer > > > > > package, > > > > > make authorization interface follow kafka’s coding standards and > > > will > > > > > allow > > > > > java implementations to be cleaner. We can either completely > drop > > > > > scala > > > > > interface, which might be a pain for existing implementations, > or we > > > > > can > > > > > have scala interface wrap java interface. Later allows a cleaner > > > > > deprecation path for existing scala authorizer interface, > however it > > > > > may or > > > > > may not be feasible as Kafka server will have to somehow decide > > > which > > > > > interface it should be looking for while loading authorizer > > > > > implementation, > > > > > this can probably be solved with a config or some reflection. > If we > > > > > choose > > > > > to go this route, I can dig deeper. > > > > > > > > > > If we decide to go with option 1, I think it would be fair to say > that > > > > > scala authorizer interface will be around for some time, as there > will > > > be > > > > > more implementations relying on it. If we go with option 2 and > commit > > > on > > > > > getting this in ASAP, preferably in 0.10, there will be fewer > > > > > implementations that will be affected. > > > > > > > > > > *Another thing to notice is that scala authorizer interface is not > > > > > annotated as unstable.* > > > > > > > > > > > > > > > On Wed, Apr 6, 2016 at 9:41 AM, Ashish Singh <asi...@cloudera.com> > > > > wrote: > > > > > > > > > > > I see value in minimizing breaking changes and I do not oppose > the > > > > idea of > > > > > > increasing scope of KIP-50 to move auth interface to java. > > > > > > > > > > > > As authorizer implementations do not really need to depend on > Kafka > > > > core, > > > > > > I would suggest that we keep authorizer interface and its > components > > > > in a > > > > > > separate package. I share the concern that right now using > Resource, > > > > > > Operation, etc, in java implementations is messy. I had to deal > with > > > > lot of > > > > > > it while writing Apache Sentry plugin. > > > > > > > > > > > > My only ask is to have this in 0.10. As Jay pointed out, right > now > > > > there > > > > > > are not many implementations out there, we might want to fix it > ASAP. > > > > I can > > > > > > only speak of Sentry integration and I think 0.10 will be the > best > > > for > > > > such > > > > > > a change, as I should be able to adopt the changes in Sentry > > > > integration > > > > > > before a lot of users start using it. > > > > > > > > > > > > On Wed, Apr 6, 2016 at 9:25 AM, Ismael Juma <ism...@juma.me.uk> > > > wrote: > > > > > > > > > > > >> It is small, but breaks binary compatibility. > > > > > >> > > > > > >> Ismael > > > > > >> > > > > > >> On Wed, Apr 6, 2016 at 5:20 PM, Grant Henke < > ghe...@cloudera.com> > > > > wrote: > > > > > >> > > > > > >> > KIP-50 as defined is very small. I don't see any harm in > putting > > > it > > > > in > > > > > >> as > > > > > >> > is and then tackling the follow up work. > > > > > >> > > > > > > >> > > > > > > >> > On Wed, Apr 6, 2016 at 11:16 AM, Ismael Juma < > ism...@juma.me.uk> > > > > wrote: > > > > > >> > > > > > > >> > > Thanks Grant. I wonder if KIP-50 should just be done as > part of > > > > this > > > > > >> > work. > > > > > >> > > > > > > > >> > > Ismael > > > > > >> > > > > > > > >> > > On Wed, Apr 6, 2016 at 5:12 PM, Grant Henke < > > > ghe...@cloudera.com> > > > > > >> wrote: > > > > > >> > > > > > > > >> > > > My work with KIP-4 found that many of the Scala classes > used > > > in > > > > the > > > > > >> > > > Authorizer interface are needed in the Clients package > when > > > > adding > > > > > >> the > > > > > >> > > > various ACL requests and responses. I also found that we > don't > > > > have > > > > > >> > > > standard Exceptions defined for the authorizer interface. > This > > > > means > > > > > >> > that > > > > > >> > > > when I add the Authorizer calls to the broker and wire > > > > protocols all > > > > > >> > > > exceptions will be reported as an "Unknown Error" back to > the > > > > user > > > > > >> via > > > > > >> > > the > > > > > >> > > > wire protocol. > > > > > >> > > > > > > > > >> > > > I have written more about it on the KIP-4 wiki and created > > > > jiras to > > > > > >> > track > > > > > >> > > > those issues (See below). I think we should wrap up this > KIP > > > as > > > > is > > > > > >> and > > > > > >> > > > tackle the Java/Exception changes as a part of those > > > jiras/kips. > > > > > >> > > > > > > > > >> > > > - KIP-4 "Follow Up Changes" > > > > > >> > > > < > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-4+-+Command+line+and+centralized+administrative+operations#KIP-4-Commandlineandcentralizedadministrativeoperations-FollowUpChangesfollow-up-changes > > > > > >> > > > > > > > > > >> > > > - KAFKA-3509 < > > > > https://issues.apache.org/jira/browse/KAFKA-3509>: > > > > > >> > > > Provide > > > > > >> > > > an Authorizer interface using the Java client > enumerator > > > > classes > > > > > >> > > > - KAFKA-3507 < > > > > https://issues.apache.org/jira/browse/KAFKA-3507>: > > > > > >> > > Define > > > > > >> > > > standard exceptions for the Authorizer interface > > > > > >> > > > > > > > > >> > > > Thank you, > > > > > >> > > > Grant > > > > > >> > > > > > > > > >> > > > On Wed, Apr 6, 2016 at 10:58 AM, Jay Kreps < > j...@confluent.io> > > > > > >> wrote: > > > > > >> > > > > > > > > >> > > > > Hey Ismael, > > > > > >> > > > > > > > > > >> > > > > Yeah I think this is a minor cleanliness thing. Since > this > > > is > > > > kind > > > > > >> > of a > > > > > >> > > > > power user interface I don't feel strongly either way. > > > > > >> > > > > > > > > > >> > > > > My motivation with Scala is just that we've tried to > move to > > > > > >> having > > > > > >> > the > > > > > >> > > > > public interfaces be Java, and as a group we definitely > > > > struggled > > > > > >> a > > > > > >> > lot > > > > > >> > > > > with understanding and maintaining Scala compatibility > in > > > the > > > > > >> older > > > > > >> > > > > clients. > > > > > >> > > > > > > > > > >> > > > > -Jay > > > > > >> > > > > > > > > > >> > > > > On Tue, Apr 5, 2016 at 11:46 PM, Ismael Juma < > > > > ism...@juma.me.uk> > > > > > >> > > wrote: > > > > > >> > > > > > > > > > >> > > > > > Hi Jay, > > > > > >> > > > > > > > > > > >> > > > > > On Wed, Apr 6, 2016 at 3:48 AM, Jay Kreps < > > > j...@confluent.io > > > > > > > > > > >> > wrote: > > > > > >> > > > > > > > > > > >> > > > > > > Given that we're breaking compatibility anyway > should > > > we: > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > We are not breaking source compatibility since the new > > > > method > > > > > >> has a > > > > > >> > > > > default > > > > > >> > > > > > implementation. I take it that you mean binary > > > > compatibility? > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > 1. Remove the get prefix on this method and the > existing > > > > one > > > > > >> > which > > > > > >> > > > > > violate > > > > > >> > > > > > > our own code style guidelines (Oops! Kind of sad we > went > > > > > >> through > > > > > >> > > the > > > > > >> > > > > > whole > > > > > >> > > > > > > KIP process and no one even flagged this) > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > I did flag this during the discussion and Ashish said > he > > > > would > > > > > >> > change > > > > > >> > > > it > > > > > >> > > > > if > > > > > >> > > > > > other people felt that it should be changed. > > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > 2. Move the interface out of scala to be a normal > Java > > > > > >> interface > > > > > >> > > > > > > > > > > > >> > > > > > > This breaks source compatibility but probably what > we > > > > should > > > > > >> have > > > > > >> > > > done > > > > > >> > > > > > > originally I suspect. Probably there are few enough > > > > > >> > implementations > > > > > >> > > > of > > > > > >> > > > > > this > > > > > >> > > > > > > that it is better to just rip the bandaid off. > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > >> > > > > > Can you please explain the motivation? It did come up > in > > > > > >> previous > > > > > >> > > > > > discussions that some things like Operation and > > > ResourceType > > > > > >> should > > > > > >> > > be > > > > > >> > > > in > > > > > >> > > > > > the clients library, but not Authorizer itself. Are we > > > > saying > > > > > >> that > > > > > >> > > any > > > > > >> > > > > > pluggable interface should be in Java so that users > can > > > > > >> implement > > > > > >> > it > > > > > >> > > > > > without including the Scala library? > > > > > >> > > > > > > > > > > >> > > > > > Grant, you originally suggested that some things would > > > have > > > > to > > > > > >> be > > > > > >> > in > > > > > >> > > > the > > > > > >> > > > > > Java side as well. Can you please elaborate on this? > > > > > >> > > > > > > > > > > >> > > > > > Ismael > > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > -- > > > > > >> > > > Grant Henke > > > > > >> > > > Software Engineer | Cloudera > > > > > >> > > > gr...@cloudera.com | twitter.com/gchenke | > > > > > >> linkedin.com/in/granthenke > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > -- > > > > > >> > Grant Henke > > > > > >> > Software Engineer | Cloudera > > > > > >> > gr...@cloudera.com | twitter.com/gchenke | > > > > linkedin.com/in/granthenke > > > > > >> > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > > Regards, > > > > > > Ashish > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > Regards, > > > > > Ashish > > > > > > > > > > -- > > > > > > Regards, > > > Ashish > > > > -- Regards, Ashish
