Hi Clement, I think my note was probably not clear.
Having SslEngineFactory separate from SslFactory is good. It simplifies reconfiguration since at least some of the logic from SslFactory can be retained. My question was around what custom configs `SslEngineFactory` implementations may have in addition to standard Kafka configs and whether these are likely to need reconfiguration. If there are unlikely to be any custom configs for SslEngineFactory that need reconfiguration, then the existing interface is sufficient. When standard SSL configs change, SslFactory gets notified. But if you wanted to have custom reconfigurable configs for SslEngineFactory, then you need to make SslEngineFactory reconfigurable so that it can provide its custom configs to the broker and get notified when the config changes. For example, the current interface is sufficient to handle changes to `ssl,keystore.location`. But if your SslEngineFactory had a config named `my.keystore.alias`, then you cannot change it without restarting the broker unless SslEngineFactory extended Reconfigurable. If you don't think that we need to support custom configs, then it will be good to mention that in the KIP (and the interface doesn't need to change if that is the case), On Fri, Jan 25, 2019 at 5:28 PM Pellerin, Clement <clement_pelle...@ibi.com> wrote: > That's what my previous solution was and it got vetoed. > I was told to refactor my solution to keep the reconfiguration internal to > Kafka. > Indeed, the reconfiguration checks are not trivial and it is best if all > pluggable implementations reuse it. > Therefore it became an implicit goal not to let the SslEngineFactory do > any reconfiguration. > > We could delegate reconfiguration to the pluggable factory under some > circumstances if you insist. > We would have to define the rules more precisely though. > Notice I don't think a pluggable factory can have its own custom configs > at the moment. > This would be desirable, but without it, your use case is mostly > inapplicable at the moment. > My KIP says this is left for another KIP, though we might want to think > about it now. > > -----Original Message----- > From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] > Sent: Friday, January 25, 2019 10:54 AM > To: dev > Subject: Re: [VOTE] [REMINDER] KIP-383 Pluggable interface for SSL Factory > > Not yet a vote, but almost there. Just wanted to clarify reconfiguration > before voting. > > So the advantage of making `SslEngineFactory` reconfigurable is that it can > define custom configs which may be reconfigured. Basically SslEngineFactory > can define a set of custom reconfigurable configs and it will get notified > when any of them changes. SslFactory on the other hand knows only of Kafka > built-in configs. So perhaps a single reconfigurable instance of > SslEngineFactory would be useful? > > > On Fri, Jan 25, 2019 at 2:13 PM Pellerin, Clement < > clement_pelle...@ibi.com> > wrote: > > > Is this a +1 binding vote? > > > > The KIP says: > > Since SslFactory will handle reconfiguration, the idea is to make the > > configuration immutable in the pluggable factory. SslFactory would > create a > > new pluggable factory every time the configuration changes. The pluggable > > factory creates its SSLContext when it is configured and never changes > it. > > It turns out SslFactory does not really need the SSLContext, so it can > use > > the new pluggable factory as an SSLEngine factory instead. > > > > I started implementing the KIP but I never finished. I did not see the > > point without the necessary binding votes. > > I missed the KIP freeze for 2.2.0 yesterday, which means I will not be > > able to use that feature in my project even if I implement it. > > I was planning to orphan that KIP today, but as you suggest, I will > finish > > the work and attach a PR before I consider doing that. > > > > -----Original Message----- > > From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] > > Sent: Friday, January 25, 2019 4:35 AM > > To: dev > > Subject: Re: [VOTE] [REMINDER] KIP-383 Pluggable interface for SSL > Factory > > > > Hi Clement, > > > > Thanks for the KIP, it is looking good. Do you by any chance have a PR > that > > you can attach to the JIRA? It wasn't clear to me how the > SslEngineFactory > > got > > the new keystore/truststore if they were reconfigured. I am assuming the > > factory is creating the SSLContext and configuring the context. Doesn't > > SslEngineFactory need to be Reconfigurable? > > > > > > On Thu, Jan 24, 2019 at 9:10 PM Harsha <m...@harsha.io> wrote: > > > > > Hi Rajini, > > > Since you helped review the KIP if you don't mind can you > vote > > > on this KIP. > > > Thanks, > > > Harsha > > > > > > On Wed, Jan 9, 2019, at 8:05 AM, Harsha wrote: > > > > HI All, > > > > We are looking forward to this KIP. Appreciate if others can > > > > take a look at the kip and > > > > vote on this thread. > > > > > > > > Thanks > > > > Harsha > > > > > > > > On Fri, Dec 21, 2018, at 4:41 AM, Damian Guy wrote: > > > > > must be my gmail playing up. This appears to be the DISCUSS thread > to > > > me... > > > > > e > > > > > On Thu, 20 Dec 2018 at 18:42, Harsha <ka...@harsha.io> wrote: > > > > > > > > > > > Damian, > > > > > > This is the VOTE thread. There is a DISCUSS thread > > > which > > > > > > concluded in it. > > > > > > > > > > > > -Harsha > > > > > > > > > > > > > > > > > > On Wed, Dec 19, 2018, at 5:04 AM, Pellerin, Clement wrote: > > > > > > > I did that and nobody came. > > > > > > > > > > https://lists.apache.org/list.html?dev@kafka.apache.org:lte=1M:kip-383 > > > > > > > I don't understand why this feature is not more popular. > > > > > > > It's the solution to one Jira and a work-around for a handful > > more > > > Jiras. > > > > > > > > > > > > > > -----Original Message----- > > > > > > > From: Damian Guy [mailto:damian....@gmail.com] > > > > > > > Sent: Wednesday, December 19, 2018 7:38 AM > > > > > > > To: dev > > > > > > > Subject: Re: [VOTE] [REMINDER] KIP-383 Pluggable interface for > > SSL > > > > > > Factory > > > > > > > > > > > > > > Hi Clement, > > > > > > > > > > > > > > You should start a separate thread for the vote, i.e., one > with a > > > subject > > > > > > > of [VOTE] KIP-383 ... > > > > > > > > > > > > > > Looks like you haven't done this? > > > > > > > > > > > >
