Correction on my previous email. Custom implementations can use AlterConfig request without a config change by including `ssl.keystore.location` or `ssl.truststore.location` in their `reconfigurabkeConfigs()`. This will trigger the same codepath as we use for keystore updates when the file has changed.
On Thu, Sep 12, 2019 at 4:43 PM Rajini Sivaram <rajinisiva...@gmail.com> wrote: > 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. >> > > > >> > > >> > >> >
