You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by "Jason Brown (JIRA)" <ji...@apache.org> on 2018/02/09 02:11:01 UTC
[jira] [Comment Edited] (CASSANDRA-14222) Add hot reloading of SSL
Certificates capability to Cassandra
[ https://issues.apache.org/jira/browse/CASSANDRA-14222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16357828#comment-16357828 ]
Jason Brown edited comment on CASSANDRA-14222 at 2/9/18 2:10 AM:
-----------------------------------------------------------------
{{SSLFactory#checkCertFilesForHotReloading}} takes references to two {{EncryptionOptions}}, but they are passed as part of lambda, meaning you'll be holding onto a reference that was created when the process started. Normally, this is not a problem, especially as we don't make immutable instances of {{EncryptionOptions}}, and we typically don't reload properties (to change the the encryption options). So I don't think we have a problem now, I'm not sure if this could cause some future problem and somebody looking real hard to figure why a value isn't propagating. So I see a few options:
- Document and leave as it is (at least we leave a bread crumb trail)
- Always call out to {{DatabaseDescriptor}} from {{SSLFactory#checkCertFilesForHotReloading}} when calling {{SSLFactory#initHotReloadableFiles()}}. That way you don't retain references to potentially 'stale' versions.
- don't check {{SSLFactory#isHotReloadingInitialized}} and eliminate the call to {{SSLFactory#initHotReloadableFiles()}} from the background task. I don't think it's unreasonable to have the correct state setup in {{SSLFactory}} before having {{SSLFactory#checkCertFilesForHotReloading}} do any real work. That way you don't keep references to any {{EncryptionOptions}}.
I'm in favor of the last, but I could be persuaded otherwise.
nits:
- remove import of {{javax.xml.crypto.Data}} from {{MessagingService}}
- {{#initHotReloadableFiles()}} I'm wondering if a simple {{ArrayList}} is sufficient vs a {{CopyOnWriteArrayList}}. You end up wrapping it in a guava ImmutableList, so it should be safe.
- -you can eliminate the member field {{SSLFactory#hotReloadableFiles}} and simply pass the list into {{checkCertFilesForHotReloading}}, as it's not used anywhere else. Your choice on this one though-. Nope, you need this for the JMX-invoked code path, so ignore this
was (Author: jasobrown):
{{SSLFactory#checkCertFilesForHotReloading}} takes references to two {{EncryptionOptions}}, but they are passed as part of lambda, meaning you'll be holding onto a reference that was created when the process started. Normally, this is not a problem, especially as we don't make immutable instances of {{EncryptionOptions}}, and we typically don't reload properties (to change the the encryption options). So I don't think we have a problem now, I'm not sure if this could cause some future problem and somebody looking real hard to figure why a value isn't propagating. So I see a few options:
- Document and leave as it is (at least we leave a bread crumb trail)
- Always call out to {{DatabaseDescriptor}} from {{SSLFactory#checkCertFilesForHotReloading}} when calling {{SSLFactory#initHotReloadableFiles()}}. That way you don't retain references to potentially 'stale' versions.
- don't check {{SSLFactory#isHotReloadingInitialized}} and eliminate the call to {{SSLFactory#initHotReloadableFiles()}} from the background task. I don't think it's unreasonable to have the correct state setup in {{SSLFactory}} before having {{SSLFactory#checkCertFilesForHotReloading}} do any real work. That way you don't keep references to any {{EncryptionOptions}}.
I'm in favor of the last, but I could be persuaded otherwise.
nits:
- remove import of {{javax.xml.crypto.Data}} from {{MessagingService}}
- {{#initHotReloadableFiles()}} I'm wondering if a simple {{ArrayList}} is sufficient vs a {{CopyOnWriteArrayList}}. You end up wrapping it in a guava ImmutableList, so it should be safe.
- you can eliminate the member field {{SSLFactory#hotReloadableFiles}} and simply pass the list into {{checkCertFilesForHotReloading}}, as it's not used anywhere else. Your choice on this one though.
> Add hot reloading of SSL Certificates capability to Cassandra
> -------------------------------------------------------------
>
> Key: CASSANDRA-14222
> URL: https://issues.apache.org/jira/browse/CASSANDRA-14222
> Project: Cassandra
> Issue Type: New Feature
> Reporter: Dinesh Joshi
> Assignee: Dinesh Joshi
> Priority: Major
> Fix For: 4.x
>
>
> Cassandra does not currently hot reload SSL certificates. For ease of operation it would be useful if we add this capability. This patch would watch changes to the truststore & keystore and would reload them when they're changed.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org