Hi Maulin,

Thanks for writing that up. I think we are getting close.

As an example interface (which can be adjusted as necessary, but just
including something here to refer to), I think we would have an interface
 along these lines:

public interface SslEngineFactory extends Configurable, Closeable {

    Set<String> reconfigurableConfigs();
    boolean shouldBeRebuilt(Map<String, Object> nextConfigs);

    SSLEngine createSslEngine(Mode mode, String peerHost, int
peerPort, String endpointIdentification);
    SSLContext sslContext();
    KeyStore keystore();
    KeyStore truststore();
}

A) *Creation:*

   - We typically tend to include configs in constructor for non-pluggable
   classes, but use Configurable for pluggable classes. So Option 2 works
   better

B) *Returning reconfigurable configs: *

   - Custom SslEngineFactory implementations will only return what they
   care about in their implementation of reconfigurableConfigs().
   - SslChannelBuilder will delegate to SslFactory as you mentioned.
   - SslFactory.reconfigurableConfigs() will return
   SslConfigs.RECONFIGURABLE_CONFIGS plus SslEngineFactory.
    reconfigurableConfigs(). So one day if we make endpoint validation
   reconfigurable, it would all just work. We can easily find a different way
   of continuing to reconfigure SslFactory without config changes if we
   needed to since it is not a pluggable class.

C) *Triggering reconfiguration:*

   - We continue to use AdminClient for reconfiguration in brokers with
   validation. That goes through SslEngineFactory.shouldBeRebuilt() and
   creates a new SslEngineFactory instance if needed.

D) *Handling push notification kind of needs*

   - For brokers, we want SslFactory to manage reconfiguration since
   configs may change. Also, AdminClient requests in brokers give a clear
   auditable path for config updates where update failures are returned to the
   caller. C) enables this.
   - For client-side, custom SslEngineFactory implementations could
   reconfigure themselves, we don't really need SslFactory to be involved
   at all.



On Sat, Sep 14, 2019 at 8:17 AM Maulin Vasavada <maulin.vasav...@gmail.com>
wrote:

> Hi Clement/Rajini
>
> Based on what I get from my own understanding and your latest inputs, I'll
> write down what my proposal is here and let us see if we can discuss on
> that,
>
> A) Creation
>
> 1. SslEngineFactory interface will not extend Configurable. Instead it will
> require the implementation to have constructor with config map
> 2. Currently, the caller is co-ordinating the call sequences between
> configure() and reconfigurableConfigs() in existing code base for
> SslChannelBuilder, SslFactory, ChannelBuilders and SocketServer. Hence I am
> not sure if we should get stuck arguing having a constructor vs having
> configure() to initialize the object. Personally I don't have a preference.
>
> B) Returning reconfigurable configs
>
> 1. SslEngineFactory interface will have reconfigurableConfigs() method
> which will return Set<String> with values for custom (or with existing)
> config keys that it cares about.
> Here I agree that all existing SSL configurations may not at all be
> applicable to the pluggable engine. I don't see a point in keep referring
> to existing SSL Configs. Of course, the default implementation of the
> SslEngineFactory will return current SSL Configs since it relies on them to
> initialize keys/certs.
>
> 2. SslFacotry's reconfigurableConfigs() will delegate the call to the
> SslEngineFactory's reconfigurableConfigs(). The similar delegation will
> need to be done by SslChannelBuilder/SaslChannelBuilder to SslFactory for
> the same method (currently that are returning SslConfigs.
> RECONFIGURABLE_CONFIGS)
>
> C) Triggering reconfiguration
>
> 1. We can use AdminClient to trigger AlterConfig for any configs needed by
> the SslEngineFactory's implementation. For the default implementation it
> will follow the SSL Configs it uses today. For the custom implementation
> everything will funnel via 'custom config' as it is dealt with in below
> method,
>
>
> https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/server/DynamicBrokerConfig.scala#L507
>
> Btw, I don't see any check like if the reconfiguration needs to be
> triggered at the below line for Listeners,
>
> https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/server/DynamicBrokerConfig.scala#L515
>
> We do seem to have a check on Line#518 for other reconfigurables.
>
> D) Handling push notification kind of needs
>
> As noted by both of you, the reconfiguration will come via AdminClient
> probably from the SslEngineFactory's implementation itself when it knows
> the keys/certs changed and I need to ask Kafka to re-create myself.
> However, that approach ONLY works for Brokers. With keys/cert rotations we
> should address Client side challenge also. As you would have noticed in
> KIP-486 motivation the deployment challenge on Broker and Client side both
> makes it challenging to manage key/cert rotations etc. Hence I feel we
> should consider option of method in the SslEngineFactory to take a
> reconfigurable and call it when it knows it needs to be reconfigured.
>
> Overall, I feel we should avoid any coupling with existing handling of SSL
> keystore and truststore via the SSL Configs. When we are allowing
> customization for the whole SSLEngine then we don't want it to rely on
> existing configs to assume/necessarily-reuse any mechanism for loading
> keystore and Truststore.
>
> For MetricReporters I feel it is using custom configs like this except that
> MetricReporter interface defines init() and extends Configurable. Please
> clarify if I am missing something.
>
> Thanks
> Maulin
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> On Fri, Sep 13, 2019 at 12:15 PM Maulin Vasavada <
> maulin.vasav...@gmail.com>
> wrote:
>
> > Hi Clement/Rajini
> >
> > I've gone through the code to understand how reconfigruation work
> > currently. I sent both of you a note separately also. Let us reconvene
> next
> > week and proceed.
> >
> > Thanks
> > Maulin
> >
> > On Thu, Sep 12, 2019 at 12:25 PM Pellerin, Clement <
> > clement_pelle...@ibi.com> wrote:
> >
> >> You are proposing a different design pattern for the SSL reconfigurable
> >> extension point than the one already implemented for MetricsReporter.
> You
> >> need a good reason to justify this decision. It is as if you consider
> >> SslFactory the extension point. This is indeed something I proposed
> >> multiple times and was always shut down.
> >>
> >> Going back to your latest proposal, notice you cannot call
> >> reconfigurableConfigs() until configure() is called because you need to
> >> instantiate SslEngineFactory() first. There is no way to enforce this in
> >> the API.
> >>
> >> You did not react to my observation that ConfigDef does a better job
> >> validating/casting configs based on the ConfigKey declarations compared
> to
> >> ad hoc code you force people to write in their extension point
> >> implementations.
> >>
> >> You suggest not to augment ConfigDef with custom configs, so that takes
> >> care of the recursive dependency.
> >> I just noticed reconfigurableConfigs() returns Set<String> and that does
> >> not force the creation of a ConfigKey for custom configs.
> >>
> >> >> SslFactory.reconfigurableConfigs() returns all standard
> reconfigurable
> >> SSL configs as well as MySslEngineFactory.reconfigurableConfigs().
> >> That's no good because that implementation does not use the standard
> >> property for the keystore location. For that particular use case, it
> would
> >> probably be better to reuse the standard keystore location config and
> >> change its semantics to a URL. Regardless, my point is the custom
> >> implementation should decide all the reconfigurable properties. By the
> way,
> >> my original use case for KIP-383 was to replace all SSL configs with a
> >> single name.
> >>
> >> It is still not clear in your email if the keystore/truststore exception
> >> is handled locally in SslFactory or by the initiator of the whole
> >> AlterConfig. That determines whether "AlterConfig without config
> changes"
> >> always goes through or is usually blocked early by the initiator.
> >>
> >>
> >> -----Original Message-----
> >> From: Rajini Sivaram [mailto:rajinisiva...@gmail.com]
> >> Sent: Thursday, September 12, 2019 2:05 PM
> >> To: dev
> >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration
> >> extensible
> >>
> >> Sorry about the confusion, my notes were conflicting!
> >>
> >> Let me give an example to clarify. Let's say KIP-519 adds a new
> interface
> >> SslEngineFactory and I have a custom implementation MySslEngineFactory
> >> that
> >> gets keystore material from a web service using a custom config `
> >> my.ssl.keystore.url`. MySslEngineFactory.reconfigurableConfigs() returns
> >> {`
> >> my.ssl.keystore.url`}. We don't want to add this custom config to Kafka.
> >> My
> >> SslEngineFactory will be managed by the (non-pluggable) Reconfigurable
> >> SslFactory that invokes appropriate methods on MySslEngineFactory to
> >> create
> >> a new SslEngine when required. SslFactory.reconfigurableConfigs()
> returns
> >> all standard reconfigurable SSL configs as well as
> >> MySslEngineFactory.reconfigurableConfigs().
> >>
> >> The existing code in the broker is sufficient for both validation and
> >> reconfiguration. We can support custom configs as well as
> reconfiguration
> >> without config change. The only non-SSL change we require is a fix for
> >> KAFKA-7588.
> >>
> >> 1) *Initial validation*: SslFactory.configure() invokes
> >> MySslEngineFactory#configure() which validates the custom config `
> >> my.ssl.keystore.url`. Broker will fail to start if the URL is invalid
> even
> >> though broker knows nothing about this config because the custom
> >> implementation fails its `configure()`.
> >> 2) *Validation during reconfiguration*: AdminClient sends an AlterConfig
> >> request to update `my.ssl.keystore.url`  Broker invokes.
> >> SslFactory.validateReconfiguration()  which will invoke
> >> MySslEngineFactory.validateReconfiguration() and that can fail
> >> reconfiguration if the provided URL is invalid. And AdminClient sees
> this
> >> validation failure in its response. This validation is more flexible
> than
> >> that provided by ConfigDef. During reconfiguration, we are not just
> >> validating that the value is of the right type, we may want to verify
> that
> >> you can connect to the remote web service for example. This validation
> >> path
> >> for custom configs is already supported.
> >> 3) *Reconfiguration without config change*: This is supported
> specifically
> >> for two configs `ssl.keystore.location` and `ssl.truststore.location`.
> >> SslFactory is given the opportunity to reconfigure whenever there is an
> >> AlterConfig request from AdminClient since it uses these configs. This
> is
> >> regardless of whether there is a config change. SslFactory will invoke
> >> MySslEngineFactory.shouldBeRebuilt(). This gives MySslEngineFactory the
> >> opportunity to provide a new SslEngine whenever there is an AdminClient
> >> request to alter configs, regardless of whether any config changed or
> not.
> >>
> >> Hope that helps,
> >>
> >> Rajini
> >>
> >> On Thu, Sep 12, 2019 at 5:26 PM Pellerin, Clement <
> >> clement_pelle...@ibi.com>
> >> wrote:
> >>
> >> > I'm confused. Can you launch a reconfiguration without a config change
> >> or
> >> > not?
> >> >
> >> > If I understand the test case correctly, the design pattern to
> >> implement a
> >> > reconfigurable extension point in Kafka is to implement the
> >> Reconfigurable
> >> > interface. This means SslEngineFactory would be created once and
> mutate
> >> > with reconfigure. There is no ConfigKey created for the custom config
> >> and
> >> > therefore there is no validation by ConfigDef.
> >> >
> >> > Optionally, to expose the built-in validation, it might be worthwhile
> to
> >> > consider making ConfigKey a public API and move an individual config
> >> parse
> >> > from ConfigDef to ConfigKey. It would be more object oriented anyway.
> >> >
> >> > One of the use case of custom configs in SslEngineFactory is to remove
> >> the
> >> > need to specify the keystore and truststore locations. The special
> >> handling
> >> > to detect changes in keystore/truststore should be pushed to
> >> > DefaultSslEngineFactory and all calls to reconfigure should reach the
> >> > SslEngineFactory instance. Am I missing something?
> >> >
> >> > -----Original Message-----
> >> > From: Rajini Sivaram [mailto:rajinisiva...@gmail.com]
> >> > Sent: Thursday, September 12, 2019 12:01 PM
> >> > To: dev
> >> > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration
> >> > extensible
> >> >
> >> > 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.
> >> > >> > > >
> >> > >> > >
> >> > >> >
> >> > >>
> >> > >
> >> >
> >>
> >
>

Reply via email to