Indeed, this is a general problem requiring a more general solution than KIP-519. I'm glad there was work done on this already.
So config.originals() still contains unknown configs but nothing has been validated and cast to the proper type. How does validation work for an extension point that receives config.originals()? Is there a public validator helper to handle this? Do we need to create ConfigKeys in the ConfigDef for custom configs only known to a custom SslEngineFactory implementation? Do we need to declare the standard SSL configs in ConfigDef if SslFactory needs to revalidate them anyway because it receives config.originals()? I assume there is such a thing as config.originals() also for a reconfigure()? If we implement KIP-519 and later change from config.values() to config.originals(), this will affect the contract for the constructor of the SslEngineFactory. We might need to add custom configs support to KIP-519 or delay KIP-519 until the change to config.originals(). -----Original Message----- From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] Sent: Wednesday, September 11, 2019 4:25 PM To: dev Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Kafka already has the notion of custom configs. And we support reconfigurable custom configs for some interfaces e.g. MetricsReporter. We also recently added custom reconfigurable configs for Authorizer under KIP-504. The issue with custom configs for SSL is described in https://issues.apache.org/jira/browse/KAFKA-7588. We currently don't pass in custom configs to ChannelBuilders. We need to fix this, not just for SSL but for other security plugins as well. So it needs to be a generic solution, not specific to KIP-519. Once KAFKA-7588 is fixed, the existing dynamic reconfiguration mechanism in brokers would simply work. Dynamic configs works exactly in the same way for custom configs as it does for other configs. The list of reconfigurable configs is returned by the implementation class and the class gets notified when any of those configs changes. This includes validateReconfiguration() as well the actual reconfigure(). For SSL alone, we have special handling of dynamic configs to enable reloading of keystores/truststores when the file changes, even though none of the config values have changed. Reconfiguration is triggered by an admin client request to alter configs. In this case, none of the actual configs may have changed, but we check if the file has changed. This is currently done only for the standard keystore/truststore configs. With KIP-519, I guess we want the custom SslEngineFactory to be able to decide whether reconfiguration is required. The simplest way to achieve this would be to have a custom config that is updated when reconfiguration is required. I am not sure we need a separate mechanism to trigger reconfiguration that doesn't rely on admin clients since admin clients provide useful logging and auditability. Regards, Rajini On Wed, Sep 11, 2019 at 4:13 PM Pellerin, Clement <clement_pelle...@ibi.com> wrote: > I'm sorry if I divert the discussion, but without this issue, it would > have been pretty trivial to update KIP-383 to go as far as you did. I am > also happy to get a discussion going, the KIP-383 thread was a desolate > place. > > Kafka needs to know about custom configs because it validates the configs > before it passes them to (re)configure. Unknown configs are silently > removed by ConfigDef. We could keep unknown configs as strings without > validating them in ConfigDef, but I don't know if the Kafka community would > accept this weaker validation. > > It appears we are trying to invent the notion of a meta config. Anyway, I > think we have shown asking an instance of SslEngineFactory to contribute to > ConfigDef is way too late. > > For your push notification, would it be simpler to just let your > SslEngineFactory notice the change and make it effective the next time it > is called. SslFactory would not cache the SslEngine and always ask > SslEngineFactory for it. You don't even need an inner thread if > SslEngineFactory checks for a change when it is called. SslEngineFactory > would no longer be immutable, so maybe it is worth reconsidering how > reconfigure works for it. > > -----Original Message----- > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > Sent: Wednesday, September 11, 2019 3:29 AM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > Hi all, > > Since the "custom config" seems the main topic of interest let us talk > about it. > > 1. I want to confirm that I interpret the definition of 'custom config of > SslEngineFactory' the same way Clement is suggesting - "a config that does > not exist in Kafka but is specified by a custom implementation of > SslEngineFactory". If there is a disagreement to that we have to bring it > up here sooner. > > 2. I've been thinking about it and I question why we are trying to make a > custom config a first class citizen in standard config? > The reasoning for that question is- > Kafka wants to delegate creation of SSLEngine to a class which is "not" > part of Kafka's distribution. Since the interface for SSLEngine creator > will be defined by the public method of createSSLEngine(), why would Kafka > care what does the implementation do other than fulfilling the contract of > creation of SSLEngine. The implementation can use any special configs i.e. > configs coming from input Map OR configs defined in a new file only known > to itself. Making the configs part of the interface contract in any way is > not necessary. This way we keep it simple and straightforward. > > 3. Now, 2nd point raises a question - if we follow that suggestion - how > can we ever re-create the SSLEngineFactory object and allow new object to > be created when something changes in the implementation. That is a valid > question. If you noticed in the KIP section titled "Other challenge" - we > do have scenario where the SslEngineFactory implementation ONLY knows that > something changed - example: keystore got updated by a local daemon process > only known to the specific implementation. This means we have a need of > "push notification" from the SslEngineFactory's implementation to the > SslFactory actually. I feel if we build the "push notification" via adding > a method in the interface as "public void > registerReconfigurableListener(Reconfigurable r)" and make SslFactory > register itself with the SslEngineFactory's impl class, we can trigger the > reconfiguration of SslEngineFactory implementation based on its own terms > and conditions without getting into making custom configs complicated. > > I am just thinking out loud here and expressing my opinion not to avoid > addressing custom configs BUT what I genuinely believe might be a better > approach. > > Thanks > Maulin > > > > > > > > > > > > > > > > > > > > On Tue, Sep 10, 2019 at 9:06 PM Pellerin, Clement < > clement_pelle...@ibi.com> > wrote: > > > Regarding what I labeled the simplest solution below, SslConfigs could > > instantiate the custom interface only if the yet to be validated configs > > were passed in to the call to get the list of known SSL configs. > > > > -----Original Message----- > > From: Pellerin, Clement > > Sent: Tuesday, September 10, 2019 11:36 AM > > To: dev@kafka.apache.org > > Subject: [EXTERNAL]RE: [DISCUSS] KIP-519: Make SSL context/engine > > configuration extensible > > > > Another solution could be a new standard ssl config that holds a list of > > extra custom configs to accept. > > Using a custom SslEngineFactory with custom configs would require setting > > two configs, one for the class name and another for the list of custom > > configs. > > In SslConfigs, we see that declaring a single config takes 5 values, so > > I'm not sure how it would work exactly. > > > > We could also declare a new interface to return the list of custom ssl > > configs and the extra standard ssl config I'm proposing holds the name of > > the implementation class instead. The reason a different interface is > > needed is because it would be instantiated by SslConfigs, not SslFactory. > > This seems the simplest solution. > > > > Anyway, the point of this exercise is to prove an acceptable solution for > > custom configs is not affecting the public API in KIP-519. > > > > > > -----Original Message----- > > From: Pellerin, Clement > > Sent: Tuesday, September 10, 2019 9:35 AM > > To: dev@kafka.apache.org > > Subject: [EXTERNAL]RE: [DISCUSS] KIP-519: Make SSL context/engine > > configuration extensible > > > > Custom config is a term I invented to mean a config that does not exist > in > > Kafka but is specified by a custom implementation of SslEngineFactory. > > The problem with custom configs (as I remember it) is that the list of > > configs is static in SslConfigs and config names are checked before > > SslFactory is created. > > ==> You must solve this problem because that is what killed KIP-383 and > > therefore is the sole reason why KIP-519 exists. > > ==> Your KIP does not have to implement the solution since it can be done > > in a future KIP, but your KIP must be compatible with the solution found. > > ==> A method to return the list of configs would help. This cannot be a > > static method in the interface since it cannot be overridden. > > ==> You could require a static method in the implementation class by > > convention, just like the constructor you require. > > > > This email did not originate from inside Information Builders. > > >
