Skip to content

fix: align Keeper TLS verification with ClickHouse settings#1366

Merged
Slach merged 2 commits intoAltinity:masterfrom
loomts:fix/keeper-tls
May 7, 2026
Merged

fix: align Keeper TLS verification with ClickHouse settings#1366
Slach merged 2 commits intoAltinity:masterfrom
loomts:fix/keeper-tls

Conversation

@loomts
Copy link
Copy Markdown
Contributor

@loomts loomts commented May 7, 2026

Follow-up to #1312.

This fixes Keeper TLS verification to match ClickHouse openSSL/client settings more closely.
Previously the Keeper TLS code effectively collapsed verification into one skipVerify flag. That is not quite right: extendedVerification=false should skip hostname verification, but should still keep certificate chain verification enabled.

This PR:

  • parses Keeper TLS verification settings more explicitly
  • uses the configured Keeper host as TLS ServerName
  • updates the integration Keeper TLS fixture
  • adds unit tests and a TestKeeperTLS integration check for the expected behavior

Tested locally:

go test ./pkg/...
KEEPER_TLS_ENABLED=1 RUN_TESTS='^TestKeeperTLS$' ./test/integration/run.sh

TestKeeperTLS passed with ClickHouse 26.3 and KEEPER_TLS_ENABLED=1.

@coveralls
Copy link
Copy Markdown

Coverage Report for CI Build 25507910574

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage at 66.807% (no base build to compare)

Details

  • Coverage remained the same as the base build.
  • Patch coverage: 45 uncovered changes across 1 file (174 of 219 lines covered, 79.45%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
pkg/keeper/keeper.go 219 174 79.45%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 18100
Covered Lines: 12092
Line Coverage: 66.81%
Coverage Strength: 31752.24 hits per line

💛 - Coveralls

@Slach Slach merged commit d10b579 into Altinity:master May 7, 2026
53 of 54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants