Hi Clement, Kafka does special handling for keystore/truststore file changes when an AlterConfig request is processed, but that is not easy to extend to custom configs. I was thinking we could just add a custom config to trigger reconfiguration. For example, a config like `my.custom.keystore.version` that is incremented when the custom implementation detects a change in keystore. And the custom implementation would invoke admin client to update `my.custom.keystore.version`. Kafka would do the rest to reconfigure SslFactory. And the custom implementation can then create the new builder.
For an example of custom config reconfiguration, this test may be useful: https://github.com/apache/kafka/blob/d3559f628b2ccb23a9faf531796675376ac06abb/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala#L792 On Thu, Sep 12, 2019 at 3:24 PM Pellerin, Clement <clement_pelle...@ibi.com> wrote: > For the push notification, Rajini prefers if the trigger to reconfigure > comes from an admin client. He says the admin client can reconfigure even > though none of the config values have changed. This allows your custom impl > to discover other conditions that have changed, for example the contents of > the keystore. > > @Rajini, can you point us to an existing example of a Kafka extension > point that implements reconfigurable custom configs. This way we could > study it and do the same thing. Consistency is good. It would be nice if > there was a KIP that describes that design pattern. > > -----Original Message----- > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > Sent: Thursday, September 12, 2019 2:24 AM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > Thanks Clement and Rajini. Let me digest what both of you said. Clearly I > need to understand more about the configurations - dynamic, custom etc. > > On the 'push notification' question Clement asked, > The way I see is simple - Kafka defines the interface for SslEngineFactory. > Implementation of that interface is owned by the Kafka operator who > customized the implementation. Let us say, my SslEngineFactoryImpl ONLY > customizes loading of keys/certs where it knows when they are updated. How > is Kafka going to know that? You said 'next time it loads' but if there is > NO additional configuration that was needed by my SslEngineFactoryImpl, > there won't be any trigger coming from Kafka to reconfigure and SslFactory > will never re-create my SslEngineFactoryImpl code which will help Kafka use > new keys/certs in the next calls. Please let me know if this makes my 'push > notification' needs clearer. > > Thanks > Maulin > > > > > > > On Wed, Sep 11, 2019 at 2:31 PM Pellerin, Clement < > clement_pelle...@ibi.com> > wrote: > > > 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. > > > > > > > > > >
