[GitHub] [activemq-artemis] sebthom edited a comment on pull request #3459: ARTEMIS-3117 Provide CachingOpenSSLContextFactory

[GitBox <gi...@apache.org>]

sebthom edited a comment on pull request #3459:
URL: https://github.com/apache/activemq-artemis/pull/3459#issuecomment-781633827


   @jbertram @ehsavoie I have three questions:
   1. why was the `SSLContextFactory` lookup implemented via a ServiceLoader and using a priority-based selection algo. This seems unnecessarily complicated. why was it not implemented in a way that e.g. the fully qualified class of the context factory to be used can be specified in the transport settings of the `broker.xml` instead?
   1. why is `CachingSSLContextFactory` holding the cache in a static map and not an instance field? https://github.com/apache/activemq-artemis/blob/52263663c48082227916cc3477f8892d9f10134b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/ssl/CachingSSLContextFactory.java#L35
   1. the transport settings allow specification of an `sslContext` value, which is basically a fixed cachekey to set/lookup the SSLContext only evaluated by `CachingSSLContextFactory`. Since the `CachingSSLContextFactory` already is capable to calculate a cache key based on keystore/truststore path, why would one want to specify a cachekey anyway? I can only see the disadvantage that using a manually specified cachekey can result in unexpected results/issues if multiple acceptors/connectors are configured to use the same cachekey but configure different key-/truststores.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org