This is a good start. Please document the constants for the config you are creating. You will notice you need to make the name of your default implementation public.
How do the configs get into your factory instance? Is it through the constructor or you will add method arguments? Please define your constructor in the KIP even if it is the default constructor. You will also need to document the constructor in the class comment of your interface. In your implementation, I suggest to default the config value and always call the constructor dynamically even for the default implementation. I don't understand why this is a factory for both SSLContext and SslEngine. The name of the factory depends on this choice. I am not a fan of the method name shouldBeRebuilt() This was OK in a private implementation before but this will become a public API. What would be a better name that fits with the naming conventions in SslFactory? Please mention how your interface could handle custom configs. I would assume you plan to merge the SslFactory reconfigurableConfigs with those of the SslEngineFactory. How can you be sure SslFactory always creates its SslEngineFactory before any calls to SslFactory.reconfigurableConfigs()? Your Rejected Alternatives need to be beefed up. In particular, why is SslFactory not the pluggable interface directly? Hint: because we want to reuse all the complex validation code and make it mandatory. This is also the place to argue against KIP-383. Hint: because it does not handle reconfiguration in the presence of custom configs. When I wrote KIP-383, I felt I needed a prototype before I could solidify the proposal. That's part of the reason why there was never a third iteration. -----Original Message----- From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] Sent: Monday, September 9, 2019 6:41 PM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-486 Support for pluggable KeyStore and TrustStore 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
