ppkarwasz commented on PR #2767:
URL: https://github.com/apache/logging-log4j2/pull/2767#issuecomment-2256772362

   > @ppkarwasz, in the light of your feedback, I revised my proposal as 
follows:
   > 
   >     1. Introduce `getId()` to `SslConfiguration` and use it in the `name` 
provided to `getManager()` in `SslSocketManager`. This will ensure 
`SslSocketManager` will evict old connections upon a reconfiguration attempt 
with a different `SslConfiguration#getId()` value.
   
   Sounds good to me. We can generate the id based on the issuers and serial 
numbers of the certificates in the two key stores.
   
   >     2. Introduce `reloadInterval` to `SslConfiguration` and use it with 
`Configuration#getScheduler().schedule()` in `SocketAppender` to periodically 
call `Configuration#getLoggerContext().reconfigure()` – iff, `reloadInterval` 
is provided and `protocol` is SSL.
   
   Do we really need to poll for configuration changes? Apache Tomcat does not 
poll the key stores for changes (see [Can Tomcat reload its SSL certificate 
without being 
restarted?](https://serverfault.com/questions/328533/can-tomcat-reload-its-ssl-certificate-without-being-restarted))
 forcing users to either restart the server when the server certificate changes 
or to use JMX.
   
   Since the OP of `LOG4J2-2988` only asks to reload the keystores **after** a 
reconfiguration, I think we don't need to introduce `reloadInterval`.
   
   I am not strongly opinionated about this, but I believe that `touch`ing the 
configuration file after the keystore has been modified might be enough.
   
   > > Reload the TLS socket. The easiest way to do it is to trigger a Log4j 
Core reconfiguration.
   > 
   > @ppkarwasz, is it so? Can't we instead create a new 
`DelegatingOutputStreamManager` extending from `SslSocketManager` that allows 
gracefully swapping (i.e., start the new, set the new, stop the old) the 
underlying `SslSocketManager` with another one?
   
   `TcpSocketManager` already has a `Reconnector` thread that asynchronously 
reconnects the channel. I guess we could use `Reconnector` to establish a new 
socket after the contents of the key stores changes.
   
   I admit, I have mixed feelings about this:
   
   * on one hand we put a lot of effort to have a reliable reconfiguration 
process. This process dictates half of Log4j Core architecture. We also 
discourage users from modifying the configuration in code.
   * on the other hand we have more and more components that modify their 
configuration at runtime, e.g. `MutableThreadContextMapFilter`.


-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to