You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by "frankgh (via GitHub)" <gi...@apache.org> on 2023/06/29 19:16:22 UTC
[GitHub] [cassandra-sidecar] frankgh opened a new pull request, #59: CASSANDRASC-63: Support credential rotation in JmxClient
frankgh opened a new pull request, #59:
URL: https://github.com/apache/cassandra-sidecar/pull/59
JMX credentials in a Cassandra instance can be rotated on a cadence, on every bounce, or by some other means. In those cases, the `JmxClient` will no longer be able to connect to the instance completely losing the ability to talk to that instance.
In this commit, we allow the `JmxClient` to support credential changes to be continue to talk to the Cassandra instance uninterrupted without any potential downtime to the Sidecar service.
--
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
[GitHub] [cassandra-sidecar] yifan-c commented on a diff in pull request #59: CASSANDRASC-63: Support credential rotation in JmxClient
Posted by "yifan-c (via GitHub)" <gi...@apache.org>.
yifan-c commented on code in PR #59:
URL: https://github.com/apache/cassandra-sidecar/pull/59#discussion_r1260113220
##########
common/src/main/java/org/apache/cassandra/sidecar/common/JmxClient.java:
##########
@@ -158,6 +162,14 @@ private void connect()
throw new RuntimeException(String.format("Failed to connect to JMX endpoint %s", jmxServiceURL),
iox);
}
+ catch (SecurityException securityException)
+ {
+ connected = false;
+ String errorMessage = securityException.getMessage() != null
+ ? securityException.getMessage()
+ : "JMX Authentication failed";
+ throw new JmxAuthenticationException(errorMessage, securityException);
+ }
Review Comment:
Theoretically, it also needs to catch `RuntimeException` since it _could_ be thrown from the suppliers.
--
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
[GitHub] [cassandra-sidecar] yifan-c commented on a diff in pull request #59: CASSANDRASC-63: Support credential rotation in JmxClient
Posted by "yifan-c (via GitHub)" <gi...@apache.org>.
yifan-c commented on code in PR #59:
URL: https://github.com/apache/cassandra-sidecar/pull/59#discussion_r1260306236
##########
common/src/main/java/org/apache/cassandra/sidecar/common/JmxClient.java:
##########
@@ -158,6 +162,14 @@ private void connect()
throw new RuntimeException(String.format("Failed to connect to JMX endpoint %s", jmxServiceURL),
iox);
}
+ catch (SecurityException securityException)
+ {
+ connected = false;
+ String errorMessage = securityException.getMessage() != null
+ ? securityException.getMessage()
+ : "JMX Authentication failed";
+ throw new JmxAuthenticationException(errorMessage, securityException);
+ }
Review Comment:
Talked offline. The thrown runtime exception needs to be wrapped in a dedicated exception type to avoid being caught and handled unexpectedly.
--
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
[GitHub] [cassandra-sidecar] frankgh commented on a diff in pull request #59: CASSANDRASC-63: Support credential rotation in JmxClient
Posted by "frankgh (via GitHub)" <gi...@apache.org>.
frankgh commented on code in PR #59:
URL: https://github.com/apache/cassandra-sidecar/pull/59#discussion_r1260290709
##########
common/src/main/java/org/apache/cassandra/sidecar/common/JmxClient.java:
##########
@@ -158,6 +162,14 @@ private void connect()
throw new RuntimeException(String.format("Failed to connect to JMX endpoint %s", jmxServiceURL),
iox);
}
+ catch (SecurityException securityException)
+ {
+ connected = false;
+ String errorMessage = securityException.getMessage() != null
+ ? securityException.getMessage()
+ : "JMX Authentication failed";
+ throw new JmxAuthenticationException(errorMessage, securityException);
+ }
Review Comment:
I think the desired behavior is to let `RuntimeException`s bubble up and be handled up the stack. We actually throw a `RuntimeException` on line 162, and that will bubble up
--
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
[GitHub] [cassandra-sidecar] yifan-c closed pull request #59: CASSANDRASC-63: Support credential rotation in JmxClient
Posted by "yifan-c (via GitHub)" <gi...@apache.org>.
yifan-c closed pull request #59: CASSANDRASC-63: Support credential rotation in JmxClient
URL: https://github.com/apache/cassandra-sidecar/pull/59
--
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