-
-
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
Open
maximevw
wants to merge
4
commits into
testcontainers:main
Choose a base branch
from
maximevw:fix/issue-9410
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
[ssl] | ||
certfile = ssl/user_cert.pem | ||
usercert = ssl/user_cert.pem | ||
userkey = ssl/user_key.pem | ||
|
||
[connection] | ||
factory = cqlshlib.ssl.ssl_transport_factory |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Binary file added
BIN
+882 Bytes
modules/cassandra/src/test/resources/cassandra-ssl-configuration/cassandra.cer
Binary file not shown.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 acqlshrc
file. See https://github.com/testcontainers/testcontainers-java/blob/main/modules/scylladb/src/main/java/org/testcontainers/scylladb/ScyllaDBContainer.java#L81There 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 totrue
incassandra.yaml
configuration file, thecqlshrc
file becomes required because the environment variableSSL_CERTFILE
is not sufficient to configure SSL properly on client-side and other required parametersssl.usercert
andssl.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 methodswithSsl
: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.