1) I agree that signature pattern is cleaner. To support this, we have to give
up inheriting Reconfigurable.
That's exactly what you propose when you want to keep reconfiguration in the
non-pluggable class.
2) I agree reconfiguration and the option to validate keystores against
truststores do not belong in a pluggable SSL Factory.
I was planning to ignore those features in my custom SslFactory
implementation, but I now see there is no reason to if Kafka does it all for me.
Reconfiguration creates a new SSLContext anyway, so that can be handled
like the initial creation by the non-pluggable class.
Back to the drawing board to keep a non-pluggable class and split a pluggable
SSLContext factory out of it.
Thanks for the feedback.
-----Original Message-----
From: Rajini Sivaram [mailto:rajinisiva...@gmail.com]
Sent: Tuesday, October 23, 2018 7:54 AM
To: dev
Subject: Re: [DISCUSS] KIP-383 Pluggable interface for SSL Factory
Thanks for the PR. A couple of comments:
1) For other public interfaces, we use parameters to configure instead of
internal config names going into the same list of configs. Kafka has public
config names and custom config names that don't conflict with public config
names. These internal config names fall into neither category.
An example of how we do this in other classes is Deserializer:
void configure(Map<String, ?> configs, boolean isKey);
2) I am not sure `SslFactory` as it stands today is the right class to turn
into a pluggable class. I can see why we want to have a custom class to
configure SSLEngines. But I don't think we want a custom class to decide
the logic for dynamic reconfiguration or decide whether to validate against
trust stores. It would perhaps be better to have a class to just create
SSLContext and possibly configure SSLEngine with additional options is
there is a good use case for that. But the non-pluggable class in Kafka
should deal with all the reconfiguration and validation logic.
Thoughts?
>
