feat: add TLS support for Keeper connections #1312
Conversation
Slach
left a comment
Slach
left a comment
There was a problem hiding this comment.
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
| var conn *zk.Conn | ||
| if isSecure { | ||
| log.Info().Msgf("isSecure=%v, keeperHosts=%v, caPath=%v, certPath=%v, keyPath=%v, skipVerify=%v, use TLS for keeper connection", isSecure, keeperHosts, cfg.ClickHouse.TLSCa, cfg.ClickHouse.TLSCert, cfg.ClickHouse.TLSKey, cfg.ClickHouse.SkipVerify) | ||
| tlsConfig, err := utils.NewTLSConfig(cfg.ClickHouse.TLSCa, cfg.ClickHouse.TLSCert, cfg.ClickHouse.TLSKey, cfg.ClickHouse.SkipVerify) |
There was a problem hiding this comment.
Not sure that this is a good idea to use the same certificates for connecting clickHouse-backup -> clickhouse and clickhouse-backup -> keeper,
according to https://clickhouse.com/docs/operations/ssl-zookeeper
better parse /*/client/* from /var/lib/clickhouse/preprocessed_configs/config.xml
when isSecure true
There was a problem hiding this comment.
got it, I will fix it soon, thank you for you guidance.
|
thanks you for your contribution |
Pull Request Test Coverage Report for Build 20477145276Details
💛 - Coveralls |
3 participants
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.