You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "jyothsnakonisa (via GitHub)" <gi...@apache.org> on 2026/02/11 00:00:14 UTC
[PR] CASSSIDECAR-401 Incorrect SSL Configuration Keys in CdcPublisher.secretsProvider() [cassandra-sidecar]
jyothsnakonisa opened a new pull request, #315:
URL: https://github.com/apache/cassandra-sidecar/pull/315
(no comment)
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-401 Incorrect SSL Configuration Keys in CdcPublisher.secretsProvider() [cassandra-sidecar]
Posted by "jyothsnakonisa (via GitHub)" <gi...@apache.org>.
jyothsnakonisa merged PR #315:
URL: https://github.com/apache/cassandra-sidecar/pull/315
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-401 Incorrect SSL Configuration Keys in CdcPublisher.secretsProvider() [cassandra-sidecar]
Posted by "jyothsnakonisa (via GitHub)" <gi...@apache.org>.
jyothsnakonisa commented on code in PR #315:
URL: https://github.com/apache/cassandra-sidecar/pull/315#discussion_r2794921998
##########
server/src/main/java/org/apache/cassandra/sidecar/cdc/CdcPublisher.java:
##########
@@ -165,27 +153,20 @@ public SecretsProvider secretsProvider()
Map<String, String> sslConfigMap = new HashMap<>();
- sslConfigMap.put(SSL_ENABLED_KEY, sslConfiguration.enabled() + "");
- sslConfigMap.put(SSL_PREFER_OPENSSL_KEY, sslConfiguration.preferOpenSSL() + "");
- sslConfigMap.put(SSL_CLIENT_AUTH_KEY, sslConfiguration.clientAuth());
- sslConfigMap.put(SSL_CIPHER_SUITES_KEY, String.join(",", sslConfiguration.cipherSuites()));
- sslConfigMap.put(SSL_SECURE_TRANSPORT_PROTOCOLS_KEY, String.join(",", sslConfiguration.secureTransportProtocols()));
- sslConfigMap.put(SSL_HANDSHAKE_TIMEOUT_KEY, sslConfiguration.handshakeTimeout().toString());
-
if (sslConfiguration.isKeystoreConfigured())
{
KeyStoreConfiguration keystore = sslConfiguration.keystore();
- sslConfigMap.put(SSL_KEYSTORE_PATH_KEY, keystore.path());
- sslConfigMap.put(SSL_KEYSTORE_PASSWORD_KEY, keystore.password());
- sslConfigMap.put(SSL_KEYSTORE_TYPE_KEY, keystore.type());
+ sslConfigMap.put(MapUtils.lowerCaseKey(SslConfig.KEYSTORE_PATH), keystore.path());
Review Comment:
Thanks for the suggestion to use a caseinsensitive map, that's much cleaner.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-401 Incorrect SSL Configuration Keys in CdcPublisher.secretsProvider() [cassandra-sidecar]
Posted by "jmckenzie-dev (via GitHub)" <gi...@apache.org>.
jmckenzie-dev commented on code in PR #315:
URL: https://github.com/apache/cassandra-sidecar/pull/315#discussion_r2794238485
##########
server/src/main/java/org/apache/cassandra/sidecar/cdc/CdcPublisher.java:
##########
@@ -165,27 +153,20 @@ public SecretsProvider secretsProvider()
Map<String, String> sslConfigMap = new HashMap<>();
- sslConfigMap.put(SSL_ENABLED_KEY, sslConfiguration.enabled() + "");
- sslConfigMap.put(SSL_PREFER_OPENSSL_KEY, sslConfiguration.preferOpenSSL() + "");
- sslConfigMap.put(SSL_CLIENT_AUTH_KEY, sslConfiguration.clientAuth());
- sslConfigMap.put(SSL_CIPHER_SUITES_KEY, String.join(",", sslConfiguration.cipherSuites()));
- sslConfigMap.put(SSL_SECURE_TRANSPORT_PROTOCOLS_KEY, String.join(",", sslConfiguration.secureTransportProtocols()));
- sslConfigMap.put(SSL_HANDSHAKE_TIMEOUT_KEY, sslConfiguration.handshakeTimeout().toString());
-
if (sslConfiguration.isKeystoreConfigured())
{
KeyStoreConfiguration keystore = sslConfiguration.keystore();
- sslConfigMap.put(SSL_KEYSTORE_PATH_KEY, keystore.path());
- sslConfigMap.put(SSL_KEYSTORE_PASSWORD_KEY, keystore.password());
- sslConfigMap.put(SSL_KEYSTORE_TYPE_KEY, keystore.type());
+ sslConfigMap.put(MapUtils.lowerCaseKey(SslConfig.KEYSTORE_PATH), keystore.path());
Review Comment:
Might be worth adding a comment here as to why we're moving things lowercase like this since we have a split in locality between 2 codebases w/this formatting dependency. And/or just use the CaseInsensitiveStringMap on our `sslConfigMap` here if you wanted to. 🤷🏻
Not a hard requirement at all - PR is good the way it is. Just trying to think through how to simplify things.
##########
server/src/main/java/org/apache/cassandra/sidecar/cdc/CdcPublisher.java:
##########
@@ -165,27 +153,20 @@ public SecretsProvider secretsProvider()
Map<String, String> sslConfigMap = new HashMap<>();
- sslConfigMap.put(SSL_ENABLED_KEY, sslConfiguration.enabled() + "");
- sslConfigMap.put(SSL_PREFER_OPENSSL_KEY, sslConfiguration.preferOpenSSL() + "");
- sslConfigMap.put(SSL_CLIENT_AUTH_KEY, sslConfiguration.clientAuth());
- sslConfigMap.put(SSL_CIPHER_SUITES_KEY, String.join(",", sslConfiguration.cipherSuites()));
- sslConfigMap.put(SSL_SECURE_TRANSPORT_PROTOCOLS_KEY, String.join(",", sslConfiguration.secureTransportProtocols()));
- sslConfigMap.put(SSL_HANDSHAKE_TIMEOUT_KEY, sslConfiguration.handshakeTimeout().toString());
-
if (sslConfiguration.isKeystoreConfigured())
{
KeyStoreConfiguration keystore = sslConfiguration.keystore();
- sslConfigMap.put(SSL_KEYSTORE_PATH_KEY, keystore.path());
- sslConfigMap.put(SSL_KEYSTORE_PASSWORD_KEY, keystore.password());
- sslConfigMap.put(SSL_KEYSTORE_TYPE_KEY, keystore.type());
+ sslConfigMap.put(MapUtils.lowerCaseKey(SslConfig.KEYSTORE_PATH), keystore.path());
Review Comment:
My own ignorance here - why do we `lowerCaseKey` them here? Asking claude indicates Spark's DataSource V2 API uses CaseInsensitiveStringMap which tracks from poking around at that code. Makes me wonder why we have all the `MapUtils.java` stuff in analytics instead of just using [CaseInsensitiveStringMap](https://commons.apache.org/proper/commons-collections/apidocs/org/apache/commons/collections4/map/CaseInsensitiveMap.html) ourselves...
Certainly beyond the scope of this patch but might be worth some follow up thought if it'd clean up the codebase.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
Re: [PR] CASSSIDECAR-401 Incorrect SSL Configuration Keys in CdcPublisher.secretsProvider() [cassandra-sidecar]
Posted by "jyothsnakonisa (via GitHub)" <gi...@apache.org>.
jyothsnakonisa commented on code in PR #315:
URL: https://github.com/apache/cassandra-sidecar/pull/315#discussion_r2794789911
##########
server/src/main/java/org/apache/cassandra/sidecar/cdc/CdcPublisher.java:
##########
@@ -165,27 +153,20 @@ public SecretsProvider secretsProvider()
Map<String, String> sslConfigMap = new HashMap<>();
- sslConfigMap.put(SSL_ENABLED_KEY, sslConfiguration.enabled() + "");
- sslConfigMap.put(SSL_PREFER_OPENSSL_KEY, sslConfiguration.preferOpenSSL() + "");
- sslConfigMap.put(SSL_CLIENT_AUTH_KEY, sslConfiguration.clientAuth());
- sslConfigMap.put(SSL_CIPHER_SUITES_KEY, String.join(",", sslConfiguration.cipherSuites()));
- sslConfigMap.put(SSL_SECURE_TRANSPORT_PROTOCOLS_KEY, String.join(",", sslConfiguration.secureTransportProtocols()));
- sslConfigMap.put(SSL_HANDSHAKE_TIMEOUT_KEY, sslConfiguration.handshakeTimeout().toString());
-
if (sslConfiguration.isKeystoreConfigured())
{
KeyStoreConfiguration keystore = sslConfiguration.keystore();
- sslConfigMap.put(SSL_KEYSTORE_PATH_KEY, keystore.path());
- sslConfigMap.put(SSL_KEYSTORE_PASSWORD_KEY, keystore.password());
- sslConfigMap.put(SSL_KEYSTORE_TYPE_KEY, keystore.type());
+ sslConfigMap.put(MapUtils.lowerCaseKey(SslConfig.KEYSTORE_PATH), keystore.path());
Review Comment:
`MapUtils.getOrDefault` method looks for lowercase keys and hence we have to convert keys to lowercase and pass it to SslConfig class in `cassandra-analytics`. I don't see a reason why MapUtils class should do that, may be we can clean these things up in the future.
--
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: pr-unsubscribe@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org