Skip to content

fix(go): support tls connections and validate over tls#97

Merged
Mandukhai-Alimaa merged 5 commits into
adbc-drivers:mainfrom
Mandukhai-Alimaa:fix/tls-connections
Jul 3, 2026
Merged

fix(go): support tls connections and validate over tls#97
Mandukhai-Alimaa merged 5 commits into
adbc-drivers:mainfrom
Mandukhai-Alimaa:fix/tls-connections

Conversation

@Mandukhai-Alimaa

Copy link
Copy Markdown
Contributor

What's Changed

This PR fixes TLS handling and updates the validation to test self-signed TLS.

  • Make SSLVerification=NONE and SSLCertPath work correctly with the custom client and avoid TLS config leaking across connections.
  • Run the validation over TLS

@Mandukhai-Alimaa Mandukhai-Alimaa changed the title fix(go): support tls connections and validate over tls [WIP] fix(go): support tls connections and validate over tls Jun 30, 2026
Comment thread go/ci/docker/certs/generate-certs.sh

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Go Trino driver’s TLS handling (custom HTTP client + TLS config) and extends the Go validation environment/tests to exercise HTTPS against a self-signed local Trino Docker setup.

Changes:

  • Add per-connection custom HTTP client registration with explicit TLS configuration (trusted CA and “skip verification” modes).
  • Update Python URI validation tests/fixtures to construct HTTP/HTTPS URIs and validate TLS scenarios (including self-signed CA).
  • Extend local Docker Compose setup/scripts/config to expose HTTPS (8443) and generate/mount TLS assets.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
go/validation/tests/trino/test_uri.py Updates URI/DSN tests to use username-only auth and to validate multiple SSL/TLS modes (trusted CA, skip verify, plain HTTP).
go/validation/tests/trino/conftest.py Reworks fixtures to build URIs/DSNs from environment variables and support HTTP/HTTPS ports + cert path.
go/validation/README.md Adds pre-build step and documents HTTPS TRINO_DSN example for self-signed CA usage.
go/db_factory.go Implements TLS-aware custom HTTP client creation/registration and prevents TLS settings from leaking via config fields.
go/compose.yaml Exposes 8443 and mounts Trino HTTPS config + generated keystore/CA cert into the container.
go/ci/scripts/pre-build.sh New script to generate a local self-signed CA + PKCS#12 keystore for the Docker HTTPS setup.
go/ci/docker/trino/config.properties Enables HTTPS alongside HTTP in the local Trino container and points to the mounted keystore.
go/ci/docker/certs/openssl-server.cnf OpenSSL server certificate config (SANs for localhost/127.0.0.1) used to generate the keystore.
go/ci/docker/certs/openssl-ca.cnf OpenSSL config for generating the self-signed CA cert used for local trust.
go/.gitignore Ignores generated TLS artifacts (ca.crt, localhost.p12).
go/.env.linux Switches validation defaults to HTTPS (8443) and adds TRINO_SSL_* env vars used by URI-specific tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread go/db_factory.go Outdated
Comment thread go/db_factory.go Outdated
Comment thread go/ci/scripts/pre-build.sh Outdated
Comment thread go/validation/README.md
Comment thread go/validation/tests/trino/test_uri.py Outdated
Comment thread go/validation/tests/trino/test_uri.py
Comment thread go/validation/tests/trino/test_uri.py
@Mandukhai-Alimaa Mandukhai-Alimaa changed the title [WIP] fix(go): support tls connections and validate over tls fix(go): support tls connections and validate over tls Jul 1, 2026
@Mandukhai-Alimaa
Mandukhai-Alimaa requested a review from lidavidm July 1, 2026 22:30
Comment thread go/db_factory.go Outdated
Comment thread go/db_factory.go

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were you able to replicate the original issue?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the driver with a self-signed certificate and found an issue. This PR fixes it, but the error I saw was different from the original issue. The original error showed "schema does not exist," which I suspect may be a downstream effect of this bug. I asked the reporter to try a local build after the PR is merged. If the error persists, I will investigate further.

Comment thread go/db_factory.go Outdated
@Mandukhai-Alimaa
Mandukhai-Alimaa merged commit 47e1c25 into adbc-drivers:main Jul 3, 2026
15 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