-
Notifications
You must be signed in to change notification settings - Fork 255
feat: add TLS support for Keeper connections #1312
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
Conversation
Slach
left a comment
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.
please add parsing /*/client/* from /var/lib/clickhouse/preprocessed_configs/config.xml
when isSecure true instead of using clickhouse TLS connection setting
moreover, could you add zookeeper and keeper tls settings to /tests/integration/docker-compose*.yml and /tests/integration/dynamic_settings.sh
and test it via
RUN_TESTS=^TestS3$ ./test/integration/run.sh
|
thanks you for your contribution |
Pull Request Test Coverage Report for Build 20569484255Details
💛 - Coveralls |
|
@loomts any news about requested changes? |
Slach
left a comment
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.
thanks a lot for your contribution, looks great!
Resolved #1311 , to simplify the config, I just reuse the tls config of clickhouse.
According to https://clickhouse.com/docs/operations/ssl-zookeeper, the sample use OS RootCAs, which can also be handle.