From 43e8275452c76c6bde9e9620ea2e2bdb7c4a5fb4 Mon Sep 17 00:00:00 2001 From: Harshit Jindal Date: Fri, 7 Nov 2025 04:25:18 +0530 Subject: [PATCH 1/5] fix(clickhouseexporter): respect TLS configuration when cert_file/key_file are missing and add test case (#43911) --- exporter/clickhouseexporter/config.go | 9 ++++++- exporter/clickhouseexporter/config_test.go | 30 ++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/exporter/clickhouseexporter/config.go b/exporter/clickhouseexporter/config.go index 55a032c68c3c6..7a3c0096236ac 100644 --- a/exporter/clickhouseexporter/config.go +++ b/exporter/clickhouseexporter/config.go @@ -210,7 +210,14 @@ func (cfg *Config) buildClickHouseOptions() (*clickhouse.Options, error) { return nil, fmt.Errorf("failed to parse DSN: %w", err) } - if cfg.TLS.CertFile != "" || cfg.TLS.KeyFile != "" { + // Load TLS config if any TLS-related field is set (not just cert/key). + if cfg.TLS.CertFile != "" || + cfg.TLS.KeyFile != "" || + cfg.TLS.CAFile != "" || + cfg.TLS.ServerName != "" || + cfg.TLS.Insecure || + cfg.TLS.InsecureSkipVerify { + opt.TLS, err = cfg.TLS.LoadTLSConfig(context.Background()) if err != nil { return nil, fmt.Errorf("failed to load TLS config: %w", err) diff --git a/exporter/clickhouseexporter/config_test.go b/exporter/clickhouseexporter/config_test.go index a70e49c145ed4..3e83dcf20ce79 100644 --- a/exporter/clickhouseexporter/config_test.go +++ b/exporter/clickhouseexporter/config_test.go @@ -5,6 +5,7 @@ package clickhouseexporter import ( "fmt" + "os" "path/filepath" "testing" "time" @@ -756,3 +757,32 @@ func TestConfigDatabase(t *testing.T) { }) } } + +// TestBuildClickHouseOptions_WithCAFileOnly verifies that when only a CAFile is provided, +// buildClickHouseOptions returns a clean TLS error instead of crashing. +func TestBuildClickHouseOptions_WithCAFileOnly(t *testing.T) { + cfg := createDefaultConfig().(*Config) + + // Use a valid DSN to ensure parsing succeeds before TLS setup. + cfg.Endpoint = "clickhouse://default:password@localhost:9000/default?secure=true" + + // Create a dummy CA file (intentionally invalid to trigger TLS error). + tmpFile, err := os.CreateTemp("", "ca*.pem") + require.NoError(t, err) + defer os.Remove(tmpFile.Name()) + + _, err = tmpFile.WriteString("-----BEGIN CERTIFICATE-----\nINVALID\n-----END CERTIFICATE-----") + require.NoError(t, err) + tmpFile.Close() + + cfg.TLS.CAFile = tmpFile.Name() + + // Run buildClickHouseOptions. + opt, err := cfg.buildClickHouseOptions() + + // We expect an error since the CA file is invalid. + require.Error(t, err, "expected error due to invalid CA file") + + // No panic, but options may be nil since TLS setup failed early. + require.Nil(t, opt, "expected nil options when TLS setup fails cleanly") +} From ebbbfcbb14d3c6282a0745e029e9c63eaba2854d Mon Sep 17 00:00:00 2001 From: Harshit Jindal Date: Fri, 7 Nov 2025 17:40:28 +0530 Subject: [PATCH 2/5] changelog: add entry for ClickHouse TLS config fix (#43911) --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 525dc4008c1a8..350d3ca2fd05c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,6 +75,7 @@ If you are looking for developer-facing changes, check out [CHANGELOG-API.md](./ ### 🧰 Bug fixes 🧰 +- `ClickHouse Exporter`: Fix TLS configuration being ignored when only `ca_file` is set. (#43911) - `exporter/clickhouse`: Fix a bug in the exporter factory resulting in a nil dereference panic when the clickhouse.json feature gate is enabled (#43733) - `exporter/kafka`: franz-go: Fix underreported kafka_exporter_write_latency metric (#43803) - `exporter/loadbalancing`: Fix high cardinality issue in loadbalancing exporter by moving endpoint from exporter ID to attributes (#43719) From 8f3f10c30fd5c96dbeb5a6ae3791c2e677937b61 Mon Sep 17 00:00:00 2001 From: Harshit Jindal Date: Sat, 8 Nov 2025 00:15:36 +0530 Subject: [PATCH 3/5] revert: remove direct changelog edit --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 350d3ca2fd05c..525dc4008c1a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,7 +75,6 @@ If you are looking for developer-facing changes, check out [CHANGELOG-API.md](./ ### 🧰 Bug fixes 🧰 -- `ClickHouse Exporter`: Fix TLS configuration being ignored when only `ca_file` is set. (#43911) - `exporter/clickhouse`: Fix a bug in the exporter factory resulting in a nil dereference panic when the clickhouse.json feature gate is enabled (#43733) - `exporter/kafka`: franz-go: Fix underreported kafka_exporter_write_latency metric (#43803) - `exporter/loadbalancing`: Fix high cardinality issue in loadbalancing exporter by moving endpoint from exporter ID to attributes (#43719) From 109d187b9b54edb7815916ed090a0e74733916fa Mon Sep 17 00:00:00 2001 From: Harshit Jindal Date: Sat, 8 Nov 2025 00:27:56 +0530 Subject: [PATCH 4/5] chore(changelog): add entry for ClickHouse TLS fix (#43911) --- .chloggen/fix-clickhouse-server-tls.yaml | 27 ++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 .chloggen/fix-clickhouse-server-tls.yaml diff --git a/.chloggen/fix-clickhouse-server-tls.yaml b/.chloggen/fix-clickhouse-server-tls.yaml new file mode 100644 index 0000000000000..8266a243fff1b --- /dev/null +++ b/.chloggen/fix-clickhouse-server-tls.yaml @@ -0,0 +1,27 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: bug_fix + +# The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog) +component: exporter/clickhouse + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Fix TLS configuration being ignored when only ca_file is provided and no cert/key files are set. + +# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists. +issues: [43911] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: This change ensures server-side TLS validation works correctly even without client certificates. + +# If your change doesn't affect end users or the exported elements of any package, +# you should instead start your pull request title with [chore] or use the "Skip Changelog" label. +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [] From db0164c89a2cd1cc0be9640ef89ff30385f814f0 Mon Sep 17 00:00:00 2001 From: Harshit Jindal Date: Thu, 13 Nov 2025 13:59:15 +0530 Subject: [PATCH 5/5] chore(lint): fix CreateTemp usage and remove extra whitespace --- exporter/clickhouseexporter/config.go | 1 - exporter/clickhouseexporter/config_test.go | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/exporter/clickhouseexporter/config.go b/exporter/clickhouseexporter/config.go index 7a3c0096236ac..1195e0fbfb8f4 100644 --- a/exporter/clickhouseexporter/config.go +++ b/exporter/clickhouseexporter/config.go @@ -217,7 +217,6 @@ func (cfg *Config) buildClickHouseOptions() (*clickhouse.Options, error) { cfg.TLS.ServerName != "" || cfg.TLS.Insecure || cfg.TLS.InsecureSkipVerify { - opt.TLS, err = cfg.TLS.LoadTLSConfig(context.Background()) if err != nil { return nil, fmt.Errorf("failed to load TLS config: %w", err) diff --git a/exporter/clickhouseexporter/config_test.go b/exporter/clickhouseexporter/config_test.go index 3e83dcf20ce79..614650b4f66da 100644 --- a/exporter/clickhouseexporter/config_test.go +++ b/exporter/clickhouseexporter/config_test.go @@ -767,7 +767,7 @@ func TestBuildClickHouseOptions_WithCAFileOnly(t *testing.T) { cfg.Endpoint = "clickhouse://default:password@localhost:9000/default?secure=true" // Create a dummy CA file (intentionally invalid to trigger TLS error). - tmpFile, err := os.CreateTemp("", "ca*.pem") + tmpFile, err := os.CreateTemp(t.TempDir(), "ca*.pem") require.NoError(t, err) defer os.Remove(tmpFile.Name())