[SEA-NodeJS] Kernel backend: mTLS, custom HTTP headers & User-Agent#420
Open
msrathore-db wants to merge 1 commit into
Open
[SEA-NodeJS] Kernel backend: mTLS, custom HTTP headers & User-Agent#420msrathore-db wants to merge 1 commit into
msrathore-db wants to merge 1 commit into
Conversation
68dbb3d to
1607ce0
Compare
1607ce0 to
b0cf092
Compare
b0cf092 to
cdcf766
Compare
cdcf766 to
84924c9
Compare
Wire the SEA/kernel path's remaining TLS-adjacent connection options
through to the napi binding, matching the Python connector's use_kernel
path (session.py + backend/kernel/client.py):
- mTLS client identity: `clientCertPem` / `clientKeyPem` (PEM string or
Buffer), normalised to Buffers and routed to the kernel
`TlsConfig::client_cert_pem` / `client_key_pem`. Both-or-neither
enforced up front with an actionable error.
- Independent hostname-verify toggle: `checkServerCertificateHostname`
(kernel `skip_hostname_verification`) for full parity with Python's
`tls_verify_hostname` — skip only the hostname check while still
validating the chain. The master `checkServerCertificate=false` still
subsumes it.
- Custom HTTP headers + User-Agent: headers cross the FFI as an ordered
list (`Array<{name,value}>`, the napi `HeaderEntry` shape matching the
kernel core `Vec<(String,String)>` and Python's `List[Tuple]`): caller
`customHeaders` first, then the connector's composed `User-Agent`
appended last (always emitted; the kernel folds the last User-Agent into
its base `DatabricksJDBCDriverOSS/...` UA). Kernel-managed reserved names
`Authorization` / `x-databricks-org-id` are dropped before the FFI hop,
matching Python's `_KERNEL_MANAGED_HEADERS` double-wall.
Adds `buildSeaHttpOptions`, extends `buildSeaTlsOptions`/`SeaTlsOptions`,
and factors PEM normalisation into a shared helper. Bumps KERNEL_REV and
regenerates `native/sea/index.d.ts`. Unit tests cover mTLS
pairing/validation, the hostname toggle, ordered header pass-through,
reserved-name dropping, and User-Agent composition/ordering; verified the
real native binding marshals every new field across the FFI and rejects a
wrong header shape.
Depends on the kernel napi change exposing clientCertPem / clientKeyPem /
customHeaders / checkServerCertificateHostname; KERNEL_REV must be
repointed to that commit once merged.
Co-authored-by: Isaac
Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
84924c9 to
cfe3b3f
Compare
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.
What
Wires the SEA/kernel path's remaining TLS-adjacent connection options through to the napi binding, completing the TLS surface started in #416 (
checkServerCertificate+customCaCert). Shapes and semantics mirror the Python connector'suse_kernelpath (session.py+backend/kernel/client.py, latest main #825).clientCertPem/clientKeyPem(PEM string orBuffer), normalised toBuffers and routed toTlsConfig::client_cert_pem/client_key_pem. Both-or-neither enforced up front. (Python forwardstls_client_cert/tls_client_key; encrypted-key password unsupported in both — the kernel has no field for it.)checkServerCertificateHostname(kernelskip_hostname_verification), for full parity with Python'stls_verify_hostname: skip only the hostname-vs-SNI check while still validating the chain. The mastercheckServerCertificate=falsesubsumes it (disables everything), exactly like Python'stls_verify=False.Array<{name,value}>, the napiHeaderEntryshape matching the kernel coreVec<(String,String)>and Python'sList[Tuple]): callercustomHeadersfirst, then the connector's composedUser-Agentappended last (always emitted via the samebuildUserAgentStringthe Thrift path uses, withuserAgentEntryfolded in; the kernel folds the lastUser-Agentinto its baseDatabricksJDBCDriverOSS/...UA, preserving the result-disposition gating token). Reserved namesAuthorization/x-databricks-org-idare dropped before the FFI hop, matching Python's_KERNEL_MANAGED_HEADERSdouble-wall.Adds
buildSeaHttpOptions, extendsbuildSeaTlsOptions/SeaTlsOptions, factors PEM normalisation into a shared helper, and adds the new fields toInternalConnectionOptions.Parity
Verified against the latest
databricks-sql-pythonmain (#825), which wires TLS/mTLS/http_headers/User-Agent throughKernelDatabricksClient. Server-cert verification now has full granular parity: master toggle (checkServerCertificate≈tls_no_verify) plus the independentcheckServerCertificateHostname≈tls_verify_hostname.Dependency
Depends on databricks/databricks-sql-kernel#127 (exposes
clientCertPem/clientKeyPem/customHeaders/checkServerCertificateHostname).KERNEL_REVis bumped to that branch's HEAD andnative/sea/index.d.tsregenerated;KERNEL_REVmust be repointed to the squashed kernel commit once #127 merges.Testing
npm test -- tests/unit/sea/connectionOptions.test.ts— 33 passing (mTLS pairing/validation, hostname toggle, ordered header pass-through, reserved-name dropping, User-Agent composition/ordering).tsc --noEmit+eslintclean on changed files. Additionally ran an FFI smoke test against the rebuilt native binding: every new field (mTLS Buffers, orderedcustomHeaders, both TLS toggles) marshals across the boundary, and a wrong header shape (plain map) is rejected by the binding ("Given napi value is not an array on ConnectionOptions.customHeaders"). (Pre-existingexamples/+ optional-lz4failures are unrelated.)This pull request and its description were written by Isaac.