-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fix CassandraContainer wait strategy when SSL is configured #9419
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
and reduce error logging while trying to connect to Cassandra database at container startup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @maximevw, sorry for the delay. I think the wait was worth because I recently learnt about SSL_CERTFILE
env var which could avoid writing a cqlshrc
file.
Let me know if you can take look at this. Otherwise, I can tackle it.
Thanks again!
modules/cassandra/src/main/java/org/testcontainers/cassandra/CassandraContainer.java
Outdated
Show resolved
Hide resolved
@@ -0,0 +1,7 @@ | |||
[ssl] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SSL_CERTFILE
env var can be set with value. I recently learnt it from scylladb module. This will avoid creating a cqlshrc
file. See https://github.com/testcontainers/testcontainers-java/blob/main/modules/scylladb/src/main/java/org/testcontainers/scylladb/ScyllaDBContainer.java#L81
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @eddumelendez,
As stated in my previous message and after testing the different use cases, if client_encryption_options.require_client_auth
is set to true
in cassandra.yaml
configuration file, the cqlshrc
file becomes required because the environment variable SSL_CERTFILE
is not sufficient to configure SSL properly on client-side and other required parameters ssl.usercert
and ssl.userkey
don't have equivalent env variables.
See:
That's why I used cqlshrc
way: because it covers more possible configurations.
So, what we could do to avoid writing a cqlshrc
in most use cases (by default, client auth is not required by server) is to have 2 methods withSsl
:
withSsl(String clientCertFile, String clientKeyFile)
using theSSL_CERTFILE
env variable for the default use case whereclient_encryption_options.require_client_auth
isfalse
incassandra.yaml
.withSsl(String clientCertFile, String clientKeyFile, boolean clientAuthRequired)
using the appropriatecqlshrc
fileclient_encryption_options.require_client_auth
istrue
incassandra.yaml
.
Let me know the solution you prefer and I'll implement it.
modules/cassandra/src/test/java/org/testcontainers/cassandra/CassandraContainerTest.java
Show resolved
Hide resolved
Hello @eddumelendez, Thank you for your review. I'll try to take a look in the next days. Regarding the |
Add test using simple cqlsh command with SSL configuration
This fixes issue #9410 and reduce error logging while trying to connect to Cassandra database at container startup (as discussed in #9337 (comment)).
The proposed solution introduces a method
withSslClientConfig(certFile, keyFile)
to use when a secured connection (TLS) is required by the Cassandra server configuration. It allows to specify the client certificate and key to use when connection checks and init script execution are performed.The tests and documentation have been updated as well.