SOLR-16743: Auto reload keystore/truststore on change #2100
Conversation
| @@ -279,12 +282,20 @@ public void init(PluginInfo info) { | |||
| int soTimeout = | |||
| getParameter(args, HttpClientUtil.PROP_SO_TIMEOUT, HttpClientUtil.DEFAULT_SO_TIMEOUT, sb); | |||
|
|
|||
| int keystoreReloadIntervalSecs = | |||
There was a problem hiding this comment.
I think this should be handled on the Http2SolrClient (and HttpSolrClient) itself, kind of like the keystore/truststore/etc are configured through sysProps by default. There are numerous usages of Solr clients throughout Solr, and presumably, all of them would need to offer this. (Though I know most are short-lived).
There was a problem hiding this comment.
I'm fine either way. I didn't like much to "leak" the server-side system properties into the client, but it's true that it's what we doing with other settings, so maybe better to favor consistency.
I did explicitly disabled this for some of those short-lived clients to prevent them from starting and stopping the scanner.
Also, I was missing the setting for the update client, this new code you suggested is handling that. Added a test for it
| SOLR_URL_SCHEME=https | ||
| if [ -n "$SOLR_SSL_KEY_STORE" ]; then | ||
| SOLR_SSL_OPTS+=" -Dsolr.jetty.keystore=$SOLR_SSL_KEY_STORE" | ||
| if [ "${SOLR_SSL_RELOAD_ENABLED:-true}" == "true" ] && [ "${SOLR_SECURITY_MANAGER_ENABLED:-true}" == "true" ]; then | ||
| # In this case we need to allow reads from the parent directory of the keystore | ||
| SOLR_SSL_OPTS+=" -Dsolr.jetty.keystoreParentPath=$SOLR_SSL_KEY_STORE/.." |
There was a problem hiding this comment.
Do we need to expand this out? I'm surprised the security manager works with /..
There was a problem hiding this comment.
Yes, without this, the tests with SSL + Security Manager will fail. I believe the reason is that the KeyStoreScanner uses the parent file here
Add Jetty module to scan and automatically reload key store and trust store changes. Also, add scanner to the SolrJ library to do the same on the client side if configured
Marking this as a draft for now, since it's missing the documentation changes. Also, I wasn't able to test the Windows start script changes