[SEA-NodeJS] Kernel-parity batch: mTLS + custom headers/UA + retry/backoff + operation-status fields#420
Open
msrathore-db wants to merge 8 commits into
Open
[SEA-NodeJS] Kernel-parity batch: mTLS + custom headers/UA + retry/backoff + operation-status fields#420msrathore-db wants to merge 8 commits into
msrathore-db wants to merge 8 commits into
Conversation
msrathore-db
force-pushed
the
msrathore/sea-mtls-headers-useragent
branch
from
68dbb3d to
1607ce0
Compare
msrathore-db
force-pushed
the
msrathore/sea-mtls-headers-useragent
branch
from
1607ce0 to
b0cf092
Compare
msrathore-db
force-pushed
the
msrathore/sea-mtls-headers-useragent
branch
from
b0cf092 to
cdcf766
Compare
msrathore-db
force-pushed
the
msrathore/sea-mtls-headers-useragent
branch
from
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>
msrathore-db
force-pushed
the
msrathore/sea-mtls-headers-useragent
branch
from
84924c9 to
cfe3b3f
Compare
Kernel #126 (logging bridge) and #127 (mTLS identity + custom HTTP headers) are both merged to kernel main. Pin KERNEL_REV to the unified main SHA 80b68e1eef3b613910183a50dfa4dace854d50dd and regenerate native/sea/index.* from it. The contract now carries both feature surfaces (gains the logging exports from #126). Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
Same alignment as the logging PR: kernel #131/#135 renamed the published napi package @databricks/sql-kernel -> @databricks/databricks-sql-kernel. Update the packaging test, version-test hint, SeaNativeLoader install hint, and README to match the regenerated router, fixing the native-packaging unit tests under the KERNEL_REV bump to kernel main 80b68e1. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
The kernel owns the retry loop on the SEA/use_kernel path, so forward the driver's existing ClientConfig retry knobs (the same ones the Thrift HttpRetryPolicy reads) onto the napi ConnectionOptions retry kwargs — keeping SEA and Thrift governed by one retry config. Mirrors Python connector #820. - buildSeaRetryOptions(config): ms -> whole seconds, clamped to napi u32. retryDelayMin->retryMinWaitSecs, retryDelayMax->retryMaxWaitSecs, retriesTimeout->retryOverallTimeoutSecs, retryMaxAttempts passes through as a TOTAL attempt count (the kernel converts to retries-after-first). - SeaBackend.connect() merges it into the native options from the client config. - Adds SeaSessionDefaults retry fields + unit tests (mapping, rounding, clamp). Requires kernel napi retry kwargs (databricks-sql-kernel #141). KERNEL_REV is pinned to #141's branch HEAD as a placeholder — MUST be re-pinned to #141's squash-merge SHA before this merges (orphan-SHA risk otherwise). Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
…o msrathore/sea-mtls-headers-useragent Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com> # Conflicts: # tests/unit/sea/connectionOptions.test.ts
…essage, diagnosticInfo, errorDetailsJson) Ports the async rich-status work (was #422) onto the consolidated branch: the napi Statement.status() fields the kernel already exposes are now surfaced through getOperationStatus instead of a flat Succeeded (M1 item). Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
Consolidation fixups: - buildSeaRetryOptions now OMITS any knob the client config didn't set to a finite number (was emitting NaN across the FFI when getConfig() lacked retry fields, e.g. the fake test context). Finite negatives still clamp to 0. - Repair the brace balance in connectionOptions.test.ts after merging the mTLS and retry test blocks. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
|
Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase ( |
msrathore-db
changed the title
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.
What
Consolidated PR landing every kernel-supported capability that was missing on the Node SEA path — the items the kernel core (and the Python connector) already support but the Node driver did not yet wire. Folds in the previously-separate #427 and #422.
Features
clientCertPem/clientKeyPem,customHeaders,userAgentEntry→ kernelTlsConfig/HttpConfig.custom_headers. Kernel: ✅ · Python: ✅ #819/#823.buildSeaRetryOptionsmaps the driver'sClientConfigretry knobs (ms) → kernelretry{Min,Max}WaitSecs/retryMaxAttempts/retryOverallTimeoutSecs(whole secs), merged inSeaBackend.connect(). Unset knobs are omitted (kernel default stands; no NaN across the FFI). Kernel: ✅ (napi kwargs added in kernel Patch-Package security vulnerability #141) · Python: ✅ #820.numModifiedRows/displayMessage/diagnosticInfo/errorDetailsJsonsurfaced throughgetOperationStatusinstead of a flatSucceeded(the M1 item inSeaOperationLifecycle). Kernel: ✅ · Python: ✅ #825.Plus the napi package-name alignment to
@databricks/databricks-sql-kernel-*(kernel #131/#135).Verification
KERNEL_REVregenerated against the unified kernel SHA carrying mTLS + retry.KERNEL_REVis pinned to #141's branch HEAD (fcc459b) as a placeholder — it carries the napi retry kwargs. Re-pin to #141's squash-merge SHA before merge (orphan-SHA risk otherwise). Kernel #141 is armed for auto-merge.Supersedes
This pull request and its description were written by Isaac.