frankgh commented on code in PR #79:
URL: https://github.com/apache/cassandra-sidecar/pull/79#discussion_r1396068731
##########
src/test/java/org/apache/cassandra/sidecar/config/SidecarConfigurationTest.java:
##########
@@ -338,6 +339,15 @@ void validateDefaultSslConfiguration(SslConfiguration
config)
assertThat(config.truststore().password()).isEqualTo("password");
assertThat(config.truststore().reloadStore()).isFalse();
assertThat(config.truststore().checkIntervalInSeconds()).isEqualTo(-1);
+ assertThat(config.secureTransportProtocols()).contains("TLSv1.3");
Review Comment:
> so validateDefaultSslConfiguration is a terribly-named method
Yeah, good point. I will rename it.
> can we please add something that tests the defaults
Yeah, I guess the default is that SSL is null, so maybe we need defaults
when SSL is explicitly provided, so maybe it belongs in a unit test for the
SSLConfiguration object instead?
> maybe add some default cipher suites
I didn't explicitly add any default cipher suites, I'm defaulting to the
existing behavior in vertx which doesn't provide any defaults.
> please use containsExactly to make sure you aren't getting the defaults
above
yeah, good point, will change it
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
