Skip to content

Commit f110c25

Browse files
[DBMON-6490] Fix ClickHouse TLS setting in the connection pool manager (#23450)
* Fix ClickHouse TLS setting in the connection pool manager * Fix CI * Update change log * Fixing TLS files * Fix TLS container crash: remove init.sql that requires missing macros The init.sql uses ReplicatedMergeTree with {layer}/{shard}/{replica} macros not present in the single-node TLS container, causing the entrypoint to exit with code 139 and the container to never start. The TLS integration tests only verify connection behaviour (verify=True/False) and do not need the test tables created by init.sql. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Skip E2E test in TLS flavor The E2E test connects to the non-TLS HTTP port (8128) which does not exist in the TLS flavor environment. TLS-specific behaviour is already covered by the integration tests in test_tls.py. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Adding a few more tests * Supporting TLS for v22.7+ and folding TLS containers to main docker-compose * Update * Cleaning up TLS compose profiles --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 8dc09ce commit f110c25

11 files changed

Lines changed: 193 additions & 7 deletions

File tree

clickhouse/changelog.d/23450.fixed

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fix SSL certificate verification being ignored when `verify: false` is set. The shared connection pool was created without TLS settings, causing `verify=False` to be silently ignored when clickhouse-connect skips its own TLS pool creation for a pre-supplied pool manager.

clickhouse/datadog_checks/clickhouse/clickhouse.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,16 @@ def __init__(self, name, init_config, instances):
7777
# We'll connect on the first check run
7878
self._client = None
7979

80-
# Shared HTTP connection pool for all ClickHouse clients (main + DBM jobs)
81-
# This reduces connection overhead while maintaining client isolation
82-
# See: https://clickhouse.com/docs/integrations/language-clients/python/advanced-usage#customizing-the-http-connection-pool
83-
self._pool_manager = httputil.get_pool_manager(maxsize=8, num_pools=4)
80+
# Shared HTTP connection pool for all ClickHouse clients (main + DBM jobs).
81+
# TLS settings must be baked in here: when pool_mgr is provided to get_client(),
82+
# clickhouse-connect assigns it immediately and skips its own TLS pool creation,
83+
# so verify=False would be silently ignored if the pool was created without it.
84+
self._pool_manager = httputil.get_pool_manager(
85+
maxsize=8,
86+
num_pools=4,
87+
verify=self._config.verify,
88+
ca_cert=self._config.tls_ca_cert,
89+
)
8490

8591
self._query_manager = QueryManager(
8692
self,

clickhouse/tests/common.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@
66
from datadog_checks.dev import get_docker_hostname, get_here
77

88
HERE = get_here()
9-
COMPOSE_FILE = os.path.join(HERE, 'docker', 'docker-compose.yaml')
9+
COMPOSE_FILE_PATH = os.path.join(HERE, 'docker', 'docker-compose.yaml')
10+
SERVER_CERT_PATH = os.path.join(HERE, 'docker', 'certs', 'server.crt')
1011

1112
HOST = get_docker_hostname()
1213
HTTP_START_PORT = 8128
1314
TCP_START_PORT = 9001
15+
HTTPS_PORT = 8134
1416
CLICKHOUSE_VERSION = os.environ['CLICKHOUSE_VERSION']
1517

1618
CONFIG = {
@@ -20,3 +22,12 @@
2022
'password': 'Datadog123!',
2123
'tags': ['foo:bar'],
2224
}
25+
26+
TLS_CONFIG = {
27+
'server': HOST,
28+
'port': HTTPS_PORT,
29+
'username': 'datadog',
30+
'password': 'Datadog123!',
31+
'tls_verify': True,
32+
'tags': ['foo:bar'],
33+
}

clickhouse/tests/conftest.py

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,19 @@ def dd_environment():
3535
)
3636
)
3737

38-
with docker_run(common.COMPOSE_FILE, conditions=conditions, sleep=10, attempts=2):
38+
conditions.append(
39+
WaitFor(
40+
ping_clickhouse(
41+
common.TLS_CONFIG['server'],
42+
common.TLS_CONFIG['port'],
43+
common.TLS_CONFIG['username'],
44+
common.TLS_CONFIG['password'],
45+
secure=True,
46+
)
47+
)
48+
)
49+
50+
with docker_run(common.COMPOSE_FILE_PATH, conditions=conditions, sleep=10, attempts=2):
3951
yield common.CONFIG
4052

4153

@@ -44,13 +56,20 @@ def instance():
4456
return deepcopy(common.CONFIG)
4557

4658

47-
def ping_clickhouse(host, port, username, password):
59+
@pytest.fixture
60+
def tls_instance():
61+
return deepcopy(common.TLS_CONFIG)
62+
63+
64+
def ping_clickhouse(host, port, username, password, secure=False):
4865
def _ping_clickhouse():
4966
client = clickhouse_connect.get_client(
5067
host=host,
5168
port=port,
5269
username=username,
5370
password=password,
71+
secure=secure,
72+
verify=False,
5473
)
5574
return client.ping()
5675

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
-----BEGIN DH PARAMETERS-----
2+
MIIBDAKCAQEA6NjHG4vP50LpyuzuyREzF6dpBzQ6rieSCdT5zU4Ku2Cb5hGzsTfp
3+
q9TLROgeua2WgRE949/ObBOdqRgc+uU4xSI/h7BRvyakvxx482iWKIefQB3kf+gZ
4+
wlQo0EZRnjARDMVVapgUxsiMmQHaj+w2trrsrn14hvcCjR9mi9j/VGXTwt36RzUS
5+
CN9jB8wEankSA2FGlicARjCSPz+X839as7Q8l9NgxM2aFY0ddSkoBCQ7ruxGOB+6
6+
qBd3YaWCO5CNy1ITq7ZjcqXmlLAbLD70xoIknhahDoDHetsGfuDG18I1tS1OzsS7
7+
3p2C+FQwPJ0Fl0v37+KweBNHQiR60i5UfwIBAgICAOE=
8+
-----END DH PARAMETERS-----
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIDUzCCAjugAwIBAgIUPQgoFbWsXK5ce7Eq11vx96y87WIwDQYJKoZIhvcNAQEL
3+
BQAwKzESMBAGA1UEAwwJbG9jYWxob3N0MRUwEwYDVQQKDAxEYXRhZG9nIFRlc3Qw
4+
HhcNMjYwNDIzMjAxMTE4WhcNMzYwNDIwMjAxMTE4WjArMRIwEAYDVQQDDAlsb2Nh
5+
bGhvc3QxFTATBgNVBAoMDERhdGFkb2cgVGVzdDCCASIwDQYJKoZIhvcNAQEBBQAD
6+
ggEPADCCAQoCggEBAIm3vNOye0w3gZj+NJ4spjk1SmL2wxqfFow/G7jKUxeIoF+X
7+
2UbfDgrkLBd64mkQqPXlfvzYPuLu+0R+4cSkoM/Pfkf55lm/ekTKPrv2B9gR28q9
8+
J/HNLsNufGfxs5XRD9YHyF0xms5MoZrSaMmpA6LsYemRet8tCcI1PgVn2ugeDVQo
9+
wZ/Eko6dToHgrxT59OPwenyQrKmkW/xVLj3yN7+XYZY93fvlpyb9RzOLV6IiErrw
10+
RgyMrwBZyWltY4bkfluNAmAi0dyoAhSt4NQgaDEYOtXdcdpd0Q6bcJIsVtnx/C+f
11+
yXlbSxtMHY1cjQmNv4ShralNuxU/Ap7YNyletGECAwEAAaNvMG0wHQYDVR0OBBYE
12+
FEjXZiVh3FKPFAwPYHD1IYYEjAJxMB8GA1UdIwQYMBaAFEjXZiVh3FKPFAwPYHD1
13+
IYYEjAJxMA8GA1UdEwEB/wQFMAMBAf8wGgYDVR0RBBMwEYIJbG9jYWxob3N0hwR/
14+
AAABMA0GCSqGSIb3DQEBCwUAA4IBAQAQJ1tugGV8FlFriqhxW1lUCjo+3J74jIKl
15+
xbI762sPZ3sZYufzxq9eE3RtZlbnrEAyfxlW2+EqDTxSuzhdezcaWSWeryLjriY0
16+
pHTFZ2vQ9y6vS+xKa54qlict6YUZjeiskN8XM0IP5TmpO+Kd6YYgh5oz5+mVZ0Jp
17+
zijXZt4zg27SqzX5SBpMBKi8B1yR+eyGgO4Jen3DEc0HXa+yB94wa5KSa1KQpR6B
18+
2bOQ0mBa9NkIF859YNNBdw+pCTAzmai3X5ptzrVBy8PEk+cH9La+UYl1guq43cap
19+
uLyZlwBpFBtx2IBNBIc91b0jR3rhI9DsnNzANtrDKFHE/NWj1Pzr
20+
-----END CERTIFICATE-----
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
-----BEGIN PRIVATE KEY-----
2+
MIIEvwIBADANBgkqhkiG9w0BAQEFAASCBKkwggSlAgEAAoIBAQCJt7zTsntMN4GY
3+
/jSeLKY5NUpi9sManxaMPxu4ylMXiKBfl9lG3w4K5CwXeuJpEKj15X782D7i7vtE
4+
fuHEpKDPz35H+eZZv3pEyj679gfYEdvKvSfxzS7Dbnxn8bOV0Q/WB8hdMZrOTKGa
5+
0mjJqQOi7GHpkXrfLQnCNT4FZ9roHg1UKMGfxJKOnU6B4K8U+fTj8Hp8kKyppFv8
6+
VS498je/l2GWPd375acm/Uczi1eiIhK68EYMjK8AWclpbWOG5H5bjQJgItHcqAIU
7+
reDUIGgxGDrV3XHaXdEOm3CSLFbZ8fwvn8l5W0sbTB2NXI0Jjb+Eoa2pTbsVPwKe
8+
2DcpXrRhAgMBAAECggEAAVbDOyY3Kfy4g1/WBZm2AMFbvjIx5i1yZ4Kh6TPN3dRk
9+
9aldHVuKYdkAFDC1/wfTsoBrG3StT737Y0bhK+nRrq2KZJmf6sLVRevDszNk77Qu
10+
PqAkl8nEqCJuV+FQodHvutc2GtC/KLbtfT42Reu+APiWFw80SmAzW5msCk5J/knw
11+
KCH0gWyKESeuSsradac3YEY6o9zJ7gDkIw1ScOFHStBA1MlJ0o6IKwyXvy9JQ9Mb
12+
mPcbz78CO42BLuHs+07Zl697NghEBhrM0C0dZWea9NSEnDkru6s1SvtCA6y5eQa1
13+
mzA0ZqC7TefDWY2NbWTlrvjaXbP9lCO3Jz18uO+G0QKBgQC+vHhPxBryPyLXvrPd
14+
cvLIUrUpdmdxbTH9/MK0t3zryRVP9d8oBRGbgXg6ob/3WjC+StQ4KsJfiD3HufvJ
15+
lQBPYMid+wfD2PmSBDL8qItwvngd33isbQmam5N8rFJ/BdTABjbzoknOExAiFuRA
16+
cg+5u2GgnjP6TIdSaLlisb/E0QKBgQC41x2eVrCqgL9n3K9aELAzqFEnDSaHDOM2
17+
GWINho0DUvb9RoDy93G8EkHodBHHQ2YA1UFsmFJKtBXdcJTd9TvG0tcyabSSq1rw
18+
vo+GGbEoOtfngkqc114aE+ORbaHlY26HB5SF+RYLtb8i/yPT25wGSmiClQEfA8qz
19+
U/YnklMakQKBgQCpOok4pJMP6gqdBvuI+KsGJ7PK1N4U2kBnzHJoawBwJxo8trbH
20+
SDAGHAozYz3yVdg3jibaqr2Ii1XdNlt60DPVOdblrEuIbaOI9SbT2yrU3nmub48e
21+
1SOXRH/INxB+2QqZUSpCYfQ/3dQLmunrcAgZz3JgUt1lL69E49vQAEtYcQKBgQCD
22+
zjm7qBuwacCTgoW0FsRfAb5kt3bqxKDc+fq2pGS3oxinAPiAyTxcHTTMWsA2tHPv
23+
UiKmGnRL+uDAmEA/ilkG7WAw/bmbAQAHPgvAV1/BzTwaRQmJj/X1aItVL7VLirzN
24+
IvWhFz7GOOLEROW6dBZtUe/2deRnxl9rwwp7q1cBUQKBgQCtXlEAQBBSJDnLggRo
25+
aoCCvnqyVEOj6iG9Cr+6QKVlczf68Qvn23IAFRIveI+fSIw3W/UqEVEnAPZTKNji
26+
ZuqC8CoOWWKq8reTFuGfPVaJ1SumQYO/erp4/7qrt7dTOirMZdFk39naOp9ldOEG
27+
Jnx53Ea7KGbeKmcuZumGwUYWfw==
28+
-----END PRIVATE KEY-----

clickhouse/tests/docker/docker-compose.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ services:
1515
hard: 262144
1616
ports:
1717
- "8128:8123"
18+
- "8134:8443"
1819
- "9001:9000"
1920
volumes:
2021
- ./override_configs/zookeeper.xml:/etc/clickhouse-server/config.d/zookeeper.xml
@@ -26,6 +27,10 @@ services:
2627
- ./init.sql:/docker-entrypoint-initdb.d/init.sql
2728
- ./test_dictionary.xml:/etc/clickhouse-server/test_dictionary.xml
2829
- ./test.csv:/opt/dictionaries/test.csv
30+
- ./tls_config.xml:/etc/clickhouse-server/config.d/tls.xml
31+
- ./certs/server.crt:/etc/clickhouse-server/ssl/server.crt
32+
- ./certs/server.key:/etc/clickhouse-server/ssl/server.key
33+
- ./certs/dhparam.pem:/etc/clickhouse-server/dhparam.pem
2934
depends_on:
3035
clickhouse-zookeeper:
3136
condition: service_healthy
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<yandex>
2+
<https_port>8443</https_port>
3+
<openSSL>
4+
<server>
5+
<privateKeyFile>/etc/clickhouse-server/ssl/server.key</privateKeyFile>
6+
<certificateFile>/etc/clickhouse-server/ssl/server.crt</certificateFile>
7+
<dhParamsFile>/etc/clickhouse-server/dhparam.pem</dhParamsFile>
8+
<verificationMode>none</verificationMode>
9+
<cacheSessions>true</cacheSessions>
10+
</server>
11+
</openSSL>
12+
</yandex>

clickhouse/tests/test_tls.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# (C) Datadog, Inc. 2026-present
2+
# All rights reserved
3+
# Licensed under a 3-clause BSD style license (see LICENSE)
4+
import pytest
5+
6+
from datadog_checks.clickhouse import ClickhouseCheck
7+
8+
from . import common
9+
10+
pytestmark = [pytest.mark.integration, pytest.mark.usefixtures('dd_environment')]
11+
12+
13+
def test_connect_with_verify_false(aggregator, tls_instance, dd_run_check):
14+
"""Regression: verify: false must allow connection to a self-signed cert.
15+
16+
Integration-level proof that the pool manager fix works end-to-end: the shared
17+
pool must be created with verify=False so clickhouse-connect doesn't override it
18+
with a cert_reqs=CERT_REQUIRED pool when pool_mgr is pre-supplied.
19+
"""
20+
tls_instance['verify'] = False
21+
check = ClickhouseCheck('clickhouse', {}, [tls_instance])
22+
dd_run_check(check)
23+
aggregator.assert_service_check('clickhouse.can_connect', status=ClickhouseCheck.OK)
24+
25+
26+
def test_connect_ssl_verify_true_fails(tls_instance, dd_run_check):
27+
"""Sanity: verify=True (default) must reject a self-signed cert.
28+
29+
Confirms TLS is actually active — without this passing, test_connect_with_verify_false
30+
would be vacuous (the server might just be accepting plain HTTP).
31+
"""
32+
tls_instance['verify'] = True
33+
check = ClickhouseCheck('clickhouse', {}, [tls_instance])
34+
with pytest.raises(Exception):
35+
dd_run_check(check)
36+
37+
38+
def test_connect_verify_true_with_ca_cert(aggregator, tls_instance, dd_run_check):
39+
"""Production path: verify=True + tls_ca_cert pointing at a trusted CA must succeed.
40+
41+
Most TLS-using DBM customers configure the integration this way — TLS on, cert
42+
validation on, with a custom CA bundle that trusts the server's cert. Pins that
43+
path against the self-signed server so the ca_cert plumbing in the shared pool
44+
manager doesn't silently regress.
45+
"""
46+
tls_instance['verify'] = True
47+
tls_instance['tls_ca_cert'] = common.SERVER_CERT_PATH
48+
check = ClickhouseCheck('clickhouse', {}, [tls_instance])
49+
dd_run_check(check)
50+
aggregator.assert_service_check('clickhouse.can_connect', status=ClickhouseCheck.OK)

0 commit comments

Comments
 (0)