SSL Root Certificate Examples#2872
Draft
chernser wants to merge 1 commit into
Draft
Conversation
|
Repository collaborators can run the JMH benchmark suite against this PR by commenting: Optional regression threshold override (Δ% on Time or Alloc/op; defaults to 10%): Only one benchmark run per PR is active at a time — issuing a new |
|
Client V2 CoverageCoverage Report
Class Coverage
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
Contributor
Author
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 26caa45. Configure here.
| rows.get(0).getString("version")); | ||
| } catch (Exception e) { | ||
| log.error("Secure connection with a custom root CA certificate failed", e); | ||
| } |
There was a problem hiding this comment.
Example exits zero on failure
Medium Severity
The SSL examples treat a failed secure connection like success for the process exit code. connectWithCustomRootCertificate in client-v2 swallows all exceptions after logging, and local mode still calls Runtime.exit(0) afterward; JDBC standalone logs SQLException and returns normally. Scripts or CI that rely on exit status will not detect TLS or query failures.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 26caa45. Configure here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 26caa45. Configure here.
| Runtime.getRuntime().exit(-1); | ||
| } | ||
| // Explicit exit: testcontainers keeps non-daemon threads alive after the scenario is done. | ||
| Runtime.getRuntime().exit(0); |
There was a problem hiding this comment.
Example exits zero on failure
Medium Severity
The SSL example programs treat a failed secure connection as success for process exit status. connectWithCustomRootCertificate catches errors and only logs them, while local client-v2 mode always calls Runtime.exit(0) after the try-with-resources block even when the connection step failed.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 26caa45. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
client-v2andjdbcprojects and use testcontainer with specially configured server. Also allows run against user configured endpointThis PR is part 1 in series of changes for SSL support and different configurations.
Checklist
Delete items not relevant to your PR:
Note
Low Risk
Production TLS path is a minor provider wiring change in HttpAPIClientHelper; most churn is test-scoped deps, examples, and docs.
Overview
Documents and validates custom root CA TLS for Client-v2 (
setRootCertificate) and JDBC (sslrootcert), without changing the JVM default trust store.HttpAPIClientHelpernow holds aClickHouseDefaultSslContextProviderinstance instead of resolving the SPI provider on eachcreateSSLContextcall; cert/keystore branching is unchanged but reads CA/client cert/key into locals first.Tests:
testCustomCaCertificatespins up WireMock HTTPS with a BouncyCastle-generated CA chain, asserts ping/query succeed with only the root CA (file path today), and fails without it.Examples: New
SSLExamples+SecureServerSupportin client-v2 and jdbc modules—local Docker ClickHouse with ephemeral certs, or standalone mode against a user host—plus README sections and manual OpenSSL/Docker setup notes. BouncyCastle (and testcontainers for examples) added to POMs.Reviewed by Cursor Bugbot for commit 26caa45. Bugbot is set up for automated code reviews on this repo. Configure here.