Hi all I created a KIP-519 for pluggability of SSLContext/Engine object. Looking forward for a great discussion and feedback on the same. I'll keep KIP-486 in discussion state until we reach to some good conclusion on how to allow custom key/trust stores after KIP-519 work is done. Based on that, we will update the KIP-486 appropriately.
Thanks Maulin On Sun, Sep 8, 2019 at 11:08 PM Maulin Vasavada <maulin.vasav...@gmail.com> wrote: > Thank you Clement and Colin for shading light on the process for me. I > will probably start working on a new kip for pluggable SslEngine. > > Side note: The pull request is from my fork to my fork to make it easier > to see what changes I am making. I have no intention to getting it reviewed > as a pull request. > > Okay, let me start with a new KIP and we will go from there. I'll keep > this KIP-486 since I feel ultimately we need that kip. > > Thanks > Maulin > > On Fri, Sep 6, 2019 at 2:52 PM Colin McCabe <cmcc...@apache.org> wrote: > >> Sorry, I meant to write "removing an API typically requires a new major >> release of Kafka". Deprecating an API can be done in a minor release. >> >> regards, >> Colin >> >> >> On Fri, Sep 6, 2019, at 14:49, Colin McCabe wrote: >> > Hi Maulin, >> > >> > Clement brought up a good point, which is that we should have agreement >> > on the overall design before we start looking at PRs. In Kafka, that >> > means starting a discussion on a KIP, and then getting that KIP voted >> > in. >> > >> > There is more information about the KIP process here: >> > >> https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals >> > Feel free to ask more detailed questions about the process on the >> > mailing list as well :) >> > >> > It sounds like Clement is proposing that you create a new KIP rather >> > than reusing KIP-383. So you should probably either repurpose KIP-486 >> > for this, or choose a new number. Both options seem reasonable here. >> > As far as I can see, once we implement a pluggable SslEngineBuilder, >> > KIP-383 would no longer be needed, and so we would put that one into >> > "rejected KIPs." >> > >> > In general, coming up with good APIs can be harder than it might seem >> > at first. Once we add an API, we have to assume that we're going to >> > support it forever. Deprecating an API can take a very long time, and >> > typically requires a new major release of Kafka. Therefore, sometimes >> > it's worth making users go through a little bit of copying and pasting >> > in order to avoid creating an API that would constrain the project in >> > the future. Hopefully, we can come up with something good here that >> > will be useful to everyone. >> > >> > best, >> > Colin >> > >> > >> > On Fri, Sep 6, 2019, at 07:48, Pellerin, Clement wrote: >> > > This is the way I see it, which is in no way authoritative. >> > > >> > > First you have to find a complete solution, then you need to document >> > > it in a KIP and that KIP needs to pass a vote. Any votes before the >> KIP >> > > vote starts is meaningless. >> > > >> > > As for the ownership and authorship of the KIPs, I don't plan to work >> > > on this, so KIP-383 is better left the way it is. I would prefer if >> you >> > > would update your KIP or maybe create a new one. We can mark KIP-383 >> as >> > > obsolete after your KIP passes its vote. >> > > >> > > -----Original Message----- >> > > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] >> > > Sent: Friday, September 6, 2019 2:36 AM >> > > To: dev@kafka.apache.org >> > > Subject: Re: [DISCUSS] KIP-486 Support for pluggable KeyStore and >> TrustStore >> > > >> > > Hi all >> > > >> > > Any input on my last question about the process? >> > > >> > > Also, I have updated the code based on the feedback so far at: >> > > https://github.com/maulin-vasavada/kafka/pull/2/files. Still have to >> figure >> > > out how to plug keys/certs loading while using >> DefaultSslEngineFactory's >> > > implementation (still need to make it final). >> > > >> > > Thanks >> > > Maulin >> > > >> > > >> > > On Thu, Sep 5, 2019 at 4:34 PM Maulin Vasavada < >> maulin.vasav...@gmail.com> >> > > wrote: >> > > >> > > > Hi all >> > > > >> > > > It seems we are in some sort of agreement so far apart from code >> > > > review/comments. However, I have a fundamental question - going >> forward how >> > > > this work from the process standpoint? What do we do with this >> KIP-486 vs >> > > > KIP-383? I feel that both needs to come together since in order to >> make >> > > > Pluggable key/trust store on top of making SslEngineBuilder >> pluggable we >> > > > will need changes suggested by KIP-486 with some differences to the >> > > > original proposal. It would great if someone can help us clarify >> the next >> > > > steps. >> > > > >> > > > Thanks >> > > > Maulin >> > > > >> > > > On Wed, Sep 4, 2019 at 1:54 PM Maulin Vasavada < >> maulin.vasav...@gmail.com> >> > > > wrote: >> > > > >> > > >> Do you guys think it would be easier if you can provide comments on >> > > >> GitHub and we can continue there and summarize the conclusion here? >> > > >> >> > > >> We should not lose addressing any comments. >> > > >> >> > > >> On Wed, Sep 4, 2019 at 12:34 PM Pellerin, Clement < >> > > >> clement_pelle...@ibi.com> wrote: >> > > >> >> > > >>> The proposed interface does not look like the Builder pattern I >> am used >> > > >>> to. >> > > >>> Should SslEngineBuilder be called SslEngineFactory instead? >> > > >>> >> > > >>> On Mon, Sep 2, 2019, at 03:33, Rajini Sivaram wrote: >> > > >>> > I would expect SslEngineBuilder interface to look something >> like this, >> > > >>> > perhaps with some tweaking: >> > > >>> > >> > > >>> > public interface SslEngineBuilder extends Configurable, >> Closeable { >> > > >>> > >> > > >>> > Set<String> reconfigurableConfigs(); >> > > >>> > >> > > >>> > boolean shouldBeRebuilt(Map<String, Object> nextConfigs); >> > > >>> > >> > > >>> > SSLEngine createSslEngine(Mode mode, String peerHost, int >> > > >>> > peerPort, String endpointIdentification); >> > > >>> > >> > > >>> > } >> > > >>> >> > > >> >> > > >> > >> >
