Hi Edoardo, Yes, you're right, this can be just a JIRA now as there are no publicly facing changes.
Thanks, Ismael On Tue, Dec 13, 2016 at 9:07 AM, Edoardo Comar <eco...@uk.ibm.com> wrote: > Thanks for your review, Ismael. > > First, I am no longer sure KIP-83 is worth keeping as KIP, I created it > just before Rajini's > https://cwiki.apache.org/confluence/display/KAFKA/KIP-85%3A+Dynamic+JAAS+ > configuration+for+Kafka+clients > With KIP-85 as presented, my proposal has become a simple JIRA, there are > no interface changes on top of KIP-85. > So I'll have no objection if you want to retire it as part of your > cleanup. > > As for your comments : > 1) We can change the map to use the Password object as a key in the > LoginManager cache, so logging its content won't leak the key. > Though I can't see why we would log the content of the cache. > > 2) If two clients use the same Jaas Config value, they will obtain the > same LoginManager. > No new concurrency issue would arise as this happens today with any two > clients (Producers/Consumers) in the same process. > > 3) Based on most jaas.config samples I have seen for kerberos and > sasl/plain, the text used as key should be no larger than 0.5k. > > Please let us know of any other concerns you may have, as > IBM Message Hub is very eager to have the issue > https://issues.apache.org/jira/browse/KAFKA-4180 merged in the next > release (February timeframe 0.10.2 ? 0.11 ?). > so we're waiting for Rajini's > https://issues.apache.org/jira/browse/KAFKA-4259 on which our changes are > based. > > thanks > Edo > -------------------------------------------------- > Edoardo Comar > IBM MessageHub > eco...@uk.ibm.com > IBM UK Ltd, Hursley Park, SO21 2JN > > IBM United Kingdom Limited Registered in England and Wales with number > 741598 Registered office: PO Box 41, North Harbour, Portsmouth, Hants. PO6 > 3AU > > > > From: Ismael Juma <ism...@juma.me.uk> > To: dev@kafka.apache.org > Date: 13/12/2016 12:49 > Subject: Re: [DISCUSS] KIP-83 - Allow multiple SASL PLAIN > authenticated Java clients in a single JVM process > Sent by: isma...@gmail.com > > > > Thanks for the KIP. A few comments: > > 1. The suggestion is to use the JAAS config value as the key to the map in > `LoginManager`. The config value can include passwords, so we could > potentially end up leaking them if we log the keys of `LoginManager`. This > seems a bit dangerous. > > 2. If someone uses the same JAAS config value in two clients, they'll get > the same `JaasConfig`, which seems fine, but worth mentioning (it means > that the `JaasConfig` has to be thread-safe). > > 3. How big can a JAAS config get? Is it an issue to use it as a map key? > Probably not given how this is used, but worth covering in the KIP as > well. > > Ismael > > On Tue, Sep 27, 2016 at 10:15 AM, Edoardo Comar <eco...@uk.ibm.com> wrote: > > > Hi, > > I had a go at a KIP that addresses this JIRA > > https://issues.apache.org/jira/browse/KAFKA-4180 > > "Shared authentification with multiple actives Kafka > producers/consumers" > > > > which is a limitation of the current Java client that we (IBM > MessageHub) > > get asked quite often lately. > > > > We will have a go at a PR soon, just as a proof of concept, but as it > > introduces new public interfaces it needs a KIP. > > > > I'll welcome your input. > > > > Edo > > -------------------------------------------------- > > Edoardo Comar > > MQ Cloud Technologies > > eco...@uk.ibm.com > > +44 (0)1962 81 5576 > > IBM UK Ltd, Hursley Park, SO21 2JN > > > > IBM United Kingdom Limited Registered in England and Wales with number > > 741598 Registered office: PO Box 41, North Harbour, Portsmouth, Hants. > PO6 > > 3AU > > Unless stated otherwise above: > > IBM United Kingdom Limited - Registered in England and Wales with number > > 741598. > > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 > 3AU > > > > > > Unless stated otherwise above: > IBM United Kingdom Limited - Registered in England and Wales with number > 741598. > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU >