feat(kernel): richer TLS Go options — CA bundle + independent hostname skip#400
Merged
mani-mathur-arch merged 3 commits intoJul 15, 2026
Merged
Conversation
mani-mathur-arch
force-pushed
the
mani/sea-kernel-consolidated
branch
from
July 14, 2026 15:36
543c7a2 to
ba466d3
Compare
mani-mathur-arch
force-pushed
the
mani/sea-kernel-richer-tls
branch
from
July 14, 2026 15:36
96ba29c to
ab90eff
Compare
mani-mathur-arch
force-pushed
the
mani/sea-kernel-consolidated
branch
from
July 14, 2026 18:42
ba466d3 to
f35b387
Compare
mani-mathur-arch
force-pushed
the
mani/sea-kernel-richer-tls
branch
from
July 14, 2026 18:42
ab90eff to
0abfb38
Compare
mani-mathur-arch
force-pushed
the
mani/sea-kernel-consolidated
branch
from
July 14, 2026 18:48
f35b387 to
d0c999e
Compare
mani-mathur-arch
force-pushed
the
mani/sea-kernel-richer-tls
branch
from
July 14, 2026 18:48
0abfb38 to
89c9b36
Compare
mani-mathur-arch
force-pushed
the
mani/sea-kernel-consolidated
branch
from
July 14, 2026 18:53
d0c999e to
2caa354
Compare
mani-mathur-arch
force-pushed
the
mani/sea-kernel-richer-tls
branch
from
July 14, 2026 18:53
89c9b36 to
b147d0e
Compare
Collaborator
Author
There was a problem hiding this comment.
Summary
Solid, focused PR — minor suggestions.
The experimental-option idiom (KernelExperimental off Config, WithKernel* prefix, fail-loud on Thrift) is the right design and consistent with the rest of the kernel backend work. cBytes / applyKernelTLS / trySetKernelTLS follow existing patterns well, and the reflective field-classification guard is a nice maintenance hook.
Main gap: no end-to-end test that Connect() actually rejects on the Thrift path (see inline on kernel_experimental_test.go). Everything else is polish / nice-to-have.
mani-mathur-arch
marked this pull request as ready for review
July 15, 2026 09:54
…e skip
Expose the two kernel-only TLS knobs whose C-ABI setters already exist on
kernel main (no kernel change needed), via an experimental-option idiom:
- WithKernelTrustedCerts(pem) -> kernel_session_config_set_tls_trusted_certs,
adding a PEM CA bundle on top of the system roots. Required because the
kernel's rustls stack ignores SSL_CERT_FILE, so a custom CA (corporate
re-signing proxy / on-prem CA) must be handed over explicitly.
- WithKernelSkipHostnameVerify() -> set_tls_skip_hostname_verification,
skipping only the hostname check while keeping chain validation
(finer-grained than the blanket WithSkipTLSHostVerify).
The knobs live on a non-exported config.KernelExperimentalConfig off
config.Config (not UserConfig), so they stay off the stable DSN surface
(mirroring Node's InternalConnectionOptions / Python's underscore kwargs).
The Thrift path rejects a non-nil block loudly at connect rather than
silently ignoring it, so a caller who forgets WithUseKernel learns the
option had no effect. OpenSession forwards each to the kernel C ABI via a
byte-buffer helper (cBytes); a reflective guard
(TestKernelExperimentalFieldsClassified) keeps a new field from slipping
either path unclassified.
mTLS client cert/key (needs an absent kernel C-ABI setter, K5) and the
CloudFetch on/off toggle (K3) are deliberately out of scope here — they are
tracked separately (PECOBLR-3652 / PECOBLR-3653).
Closes PECOBLR-3651.
Co-authored-by: Isaac
Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
…ift-reject test Follow-up on the four review findings for the richer-TLS kernel options: - Add ErrRequiresKernelBackend sentinel (mirror of ErrNotSupportedByKernel) and wrap the Thrift-path rejection with %w, so callers detect it via errors.Is instead of message text. Documented in doc.go. - WithKernelTrustedCerts copies the PEM defensively (matching DeepCopy) so a caller mutating the slice between NewConnector and Connect can't change the trust store. - Add a MITM WARNING to WithKernelSkipHostnameVerify, consistent with WithSkipTLSHostVerify. - Add TestWithKernelOptionsRejectedOnThriftPath: end-to-end Connect() reject on the Thrift path, asserting on the sentinel. Co-authored-by: Isaac
…erage Non-blocking follow-ups: - doc.go: mention ErrRequiresKernelBackend in the main Errors section, symmetric with ErrNotSupportedByKernel. - Add TestWithKernelTrustedCertsCopiesPEM: mutating the caller's slice after the option must not change stored config (option-set counterpart to the DeepCopy aliasing test). - TestConfig_DeepCopy: set KernelExperimental in the all-values case so Config.DeepCopy's wiring of that field is exercised, and assert it's copied not aliased. Co-authored-by: Isaac
mani-mathur-arch
force-pushed
the
mani/sea-kernel-richer-tls
branch
from
July 15, 2026 12:39
8c108b4 to
1e6d921
Compare
mani-mathur-arch
merged commit Jul 15, 2026
1e6d921
into
mani/sea-kernel-consolidated
5 checks passed
mani-mathur-arch
added a commit
that referenced
this pull request
Jul 17, 2026
…tly workflows for e2e tests (#399) ## What Extends the SEA-via-kernel backend for the Go driver with the full DAIS gap-closure + PuPr feature set: authentication, session setup, type rendering, query execution, telemetry, richer TLS, kernel-log routing, and CI. This PR consolidates what was a 5-PR stack (#400 richer-TLS, #401 log-unify, #402 nightly-e2e, #403 c2-dispatch) into a single reviewable PR. The stack was linear, so the branches were fast-forwarded into this one — every change is preserved as a **distinct, logically-scoped commit** (23 commits), so it reads commit-by-commit rather than as one blob. #400–#403 now show as merged (their commits live here). ## Features **Authentication** - **OAuth M2M / U2M** — the kernel drives its own auth flow from `cfg.Authenticator` (mirroring pyo3/napi). Adds a `kernel.Auth` descriptor + `resolveKernelAuth`, wired through the `set_auth_pat` / `_m2m` / `_u2m` C-ABI setters. `cfg.Authenticator` is the single source of truth (last-writer-wins, matching Thrift). - **Unsupported auth is sentinel-wrapped** — token-provider / external / federated authenticators reject with `%w`-wrapped `ErrNotSupportedByKernel`, the same programmatic-fallback contract every other unsupported kernel option follows. **Session setup** - **Initial namespace** — post-connect `USE CATALOG` / `USE SCHEMA` with identifier quoting (the kernel C ABI has no namespace setter). - **Metric-view metadata** — `config.EffectiveSessionParams()` folds the server conf in backend-neutrally, so the kernel path sends the identical conf Thrift does. **Type rendering** - **INTERVAL** day-time & year-month rendering in `internal/arrowscan` (kernel returns native arrow values; formatted Go-side to the Thrift path's string form). Handles the `math.MinInt64` / `math.MinInt32` negation bound without overflow. - Parity coverage for **TIMESTAMP vs TIMESTAMP_NTZ** and **VARIANT / GEOMETRY** (live-probed on both backends). **Query execution & telemetry** - **Bound query parameters** via the kernel raw-param C ABI (`kernel_statement_bind_parameter`). The positional/named + SQL-NULL/empty-string decision is a pure `paramBindArg` seam, unit-tested under `CGO_ENABLED=0`. - **Kernel errors surfaced as `DBExecutionError`** carrying sqlstate + server query id, so `errors.As` → `SqlState()` / `QueryId()` works on the kernel path. The execute path deliberately reports **non-retryable** (a sent statement may have committed — no double-write), matching Thrift. - **Server query id** via a real `StatementID()` accessor (`kernel_executed_statement_query_id`), threaded into EXECUTE_STATEMENT telemetry, plus CLOSE_STATEMENT telemetry on the result-read path. - **Cancelled execute** evicts a session-fatal conn and preserves the kernel error metadata (multi-`%w`: `errors.Is(DeadlineExceeded)` AND `errors.As(*KernelError)` both hold). **Richer TLS (Go options)** - CA-bundle + independent hostname-skip setters mapped to the kernel's own TLS knobs (defensive PEM copy; the wholesale custom `WithTransport` stays rejected). **Kernel logging** - Kernel log output routed through the driver log level with connId/corrId/queryId correlation; allocation-free above Debug (guards the Arrow-batch hot path). - **scope note (please read before flagging):** - This unifies the *control knob*, not the *sink*, and that boundary is intentional. `DATABRICKS_LOG_LEVEL`/`SetLogLevel` now drives both the Go binding lines and the kernel's Rust lines from one level. But they stay two physical streams: Go lines go through `logger.Logger` (and honor `logger.SetLogOutput`), while the kernel's Rust lines go straight to stderr — the kernel C ABI exposes **no sink/callback hook**, so there is currently no way to route them through the Go logger. Consequences a reviewer will notice, all expected: - A custom `SetLogOutput`/file captures only the Go lines; Rust lines stay on stderr. - With the default sink both land on stderr but only *race* there — not a single ordered, merged stream. - The Rust subscriber's level is sampled once, at the first kernel connect. Fully merging both into one ordered sink requires a kernel log callback and is tracked separately (<TICKET>); it's out of scope here because it's a kernel-side ABI change, not a driver one. **CI** - A scheduled + dispatchable **nightly E2E** workflow that runs the credential-gated e2e + Thrift-vs-kernel parity suites against a real warehouse (the ordinary CI injects no warehouse secrets, so those tests otherwise skip). SEA-via-kernel integration leg gated behind a kernel label. ## Notable behavior - Bound query parameters are now supported (previously rejected at connect). - **Staging operations remain rejected** — the kernel path can't perform the local file transfer and the C ABI surfaces no staging signal. ## Testing - **Default pure-Go** (`CGO_ENABLED=0`): `go build` / `go vet` / `go test ./...` — 24 packages ok. - **Tagged kernel path** (`CGO_ENABLED=1 -tags databricks_kernel`, linked against a locally-built kernel `.a` at `KERNEL_REV`): 24 packages ok. - **Live e2e + parity** against a real warehouse: all pass — data types (incl. variant/geometry/decimal), NTZ, params-vs-Thrift (10/10), CloudFetch, cancellation, connection pool (40/40), StatementID, initial namespace, and **M2M** (OAuth client-credentials). - `gofmt` clean; Isaac Review clean (0 final comments). ## Merge gate - **Re-pin `KERNEL_REV` at merge time.** It currently points at the kernel dependency's PR-head SHA (GC-able once that kernel change merges); bump it to the resulting kernel `main` SHA, re-sync the cgo drift assertions in `cgo.go` if any signatures changed, and run `make test-kernel` against the new rev. This pull request and its description were written by Isaac. --------- Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
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
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.
Context for reviewers
Part of the SEA-via-kernel backend for the Go driver: an alternative to the Thrift backend that talks to Databricks SQL through the Rust
databricks-sql-kernelstatic library over a C ABI (built into the driver behind thedatabricks_kernelbuild tag; the default pure-Go build is unaffected). This PR adds two TLS knobs to that backend.Stack position (each stacked on the one below):
How to read the diff: the new public surface is the two
With…options inconnector.go+ theirconfigfields; the interesting logic is (a) the Thrift-path reject and (b) theOpenSessionforwarding via the newcBytescgo helper. Everything else is wiring + tests.What
Exposes two kernel-only TLS options via an experimental-option idiom — no kernel change (both C-ABI setters already exist on kernel main, verified against the
origin/mainkernel header):WithKernelTrustedCerts(pem []byte)→kernel_session_config_set_tls_trusted_certs: adds a PEM CA bundle to the kernel's trust store on top of the system roots (corporate re-signing proxy / on-prem CA). Required rather than relying onSSL_CERT_FILE, which the kernel's rustls stack ignores.WithKernelSkipHostnameVerify()→kernel_session_config_set_tls_skip_hostname_verification: skips only the certificate hostname check while keeping chain validation (finer-grained than the blanketWithSkipTLSHostVerify).Closes PECOBLR-3651.
Design
config.KernelExperimentalConfigoffconfig.Config(NOTUserConfig), so they stay off the stable DSN/exported surface — same treatment asTLSConfig, mirroring Node's non-exportedInternalConnectionOptions.DeepCopycopies the CA byte slice.WithUseKernellearns the option had no effect rather than connecting with a weaker-than-intended trust store.OpenSessionforwards each to the kernel C ABI via a new byte-buffer helper (cBytes, mirroringcStr). A reflective guard (TestKernelExperimentalFieldsClassified) fails if a new experimental field isn't classified forwarded/rejected — so a future option can't silently be neither forwarded nor rejected.Scope boundary (deliberately excluded, deferred)
Bundle 1b only (the no-kernel-dependency part of the richer-TLS work). The following were confirmed absent from kernel main, so they need a kernel C-ABI setter first and are correctly deferred:
set_tls_client_certificate) → PECOBLR-3652 (K5).set_cloudfetch_enabled) → PECOBLR-3653 (K3).Testing
CGO_ENABLED=0: build + vet +go test ./...— 24 pkg ok, 0 fail.CGO_ENABLED=1 -tags databricks_kernel(linked against a locally-built kernel.a): build + vet + test — 24 pkg ok, 0 fail. NewTestSetKernelTLSdrives the real cgo setters; untaggedkernel_experimental_test.gocovers the option→config wiring, the classification guard, andDeepCopy.[]bytenormalization, and the idempotent double hostname-skip call when bothWithSkipTLSHostVerify+WithKernelSkipHostnameVerifyare set.Stacking / merge order
Stacked on #399 (
mani/sea-kernel-consolidated). NoKERNEL_REVchange here (inherits #399's pin). #401 (kernel logging) is stacked on top of this.This pull request and its description were written by Isaac.