ci: fan out pure-Go driver tests to macOS and Windows#421
Open
mani-mathur-arch wants to merge 10 commits into
Open
ci: fan out pure-Go driver tests to macOS and Windows#421mani-mathur-arch wants to merge 10 commits into
mani-mathur-arch wants to merge 10 commits into
Conversation
Add a build-and-test-cross-os job that builds and unit-tests the default pure-Go driver (CGO_ENABLED=0, Thrift backend) on hosted macos-latest (arm64) and windows-latest (amd64), across Go 1.25.x/1.26.x. The Linux build-and-test job already covers linux/amd64 on the protected runners; this extends the cross-platform surface without touching it. The pure-Go build has no cgo, no kernel .a, and no private module deps, so it needs no Rust/C toolchain, no JFrog proxy, and no protected runner — it runs on plain hosted runners, mirroring the sibling databricks-adbc Go driver. The SEA-via-kernel backend stays Linux-only for now (added per-OS later). Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
mani-mathur-arch
force-pushed
the
mani-mathur_data/mani/ci-cross-os-pure-go
branch
from
July 22, 2026 21:00
e78b856 to
397d1e8
Compare
localPathIsAllowed guarded against staging PUT/GET escaping the configured stagingAllowedLocalPaths by rejecting a relative path containing the literal "../". filepath.Rel returns OS-native separators, so on Windows an escaping path is "..\\x", which the "../" substring never matches — the guard returned true (allowed) for a path outside the allowlist. This is a fail-open path-traversal check on Windows, present since the driver's first commit. Compare the relative path against filepath.Separator instead: a path is inside the allowed dir iff it is neither ".." nor starts with ".."+separator. Using a prefix check (not Contains) also stops a sibling like "..foo" from being mistaken for an escape. Only the default Thrift path reaches this code; the kernel/SEA backend rejects staging statements before execStagingOperation. Surfaced by the new macOS/Windows CI matrix — the existing TestPathAllowed now passes on Windows. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
Adds build-and-test-kernel-macos, extending kernel/SEA CI coverage to its second OS (after linux/amd64). This is the first time cgo_darwin.go's link flags (positional .a for Apple ld64, -lc++, @loader_path) are compiled and linked in CI — a green run validates the darwin link contract. Structurally it is the linux build-and-test-kernel job minus the two JFrog steps: it runs on a hosted macos-latest (arm64) runner with open egress, and the kernel's deps plus the driver's go.mod deps are all public, so cargo uses crates.io and go uses the default proxy directly. The GitHub App token step is kept because the kernel repo is private (repo auth, not crate resolution). macos-latest is arm64 (matches cgo_darwin.go's `darwin && arm64` tag), so host == target and kernel-lib.sh needs no --target; aarch64-apple-darwin is a Rust tier-1 target and Xcode CLT supplies cc + libc++. No warehouse creds, so only the tagged unit tests run (live e2e/parity self-skip, as on linux). Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
Add cgo_linux_arm64.go with the linux/arm64 link flags (same GNU ld form as cgo_linux.go: -l:libdatabricks_sql_kernel.a -lstdc++ -lm -ldl, from lib/linux_arm64), and narrow cgo_unsupported.go's build constraint to admit linux/arm64 so it no longer compile-blocks that target. Verified via `go list -tags databricks_kernel` that each supported target now selects exactly one link file (linux/amd64, linux/arm64, darwin/arm64, windows/amd64) with no overlap, and darwin/amd64 still falls through to the unsupported guard. Not yet exercised in CI — no allow-listed linux/arm64 runner exists in the org today; the arm64 .a is built and linked only on a native arm64 host. Flags are the intended shape; validate on an arm64 runner before a release relies on it. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
… build kernel-lib.sh gains an optional KERNEL_CARGO_TARGET: when set it passes `cargo build --target <triple>` and reads the archive from target/<triple>/release/ instead of target/release/. The Makefile sets it to x86_64-pc-windows-gnu for GOOS=windows and leaves it empty elsewhere. Windows cgo links via the mingw/gcc toolchain, which needs a GNU archive (.a), but the default x86_64-pc-windows-MSVC host triple emits an MSVC import lib. This is an ABI selection on the same os/arch (host==target still holds, so the cross-build guard is not tripped), not a cross-OS build. Linux/macOS keep the original no-`--target`, target/release/ behavior byte-for-byte (empty override). Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
Adds build-and-test-kernel-macos, extending kernel/SEA CI toward its second OS. A green run validates cgo_darwin.go's link flags (positional .a for Apple ld64, -lc++, @loader_path), otherwise never linked in CI. GATED OFF by default (vars.KERNEL_MACOS_CI_ENABLED == 'true') because it is blocked by org infrastructure, not the driver code: the kernel repo is private, so building its .a needs a GitHub App token, but GitHub-hosted macos-latest runners have public IPs that are not on the databricks org IP allow list, so create-github-app-token fails before cargo/cgo run. No allow-listed macOS runner exists in the org today (the protected group offers windows-server-latest but no macOS label). Turn on once either an allow-listed macOS runner exists or the kernel publishes a downloadable darwin_arm64 .a (the distribution download path). The job body is otherwise ready: the linux kernel job minus the two JFrog steps (hosted mac has open egress; kernel + go deps are all public), keeping the App token for the private clone. macos-latest is arm64 = cgo_darwin.go's tag, so host==target (no --target); aarch64-apple-darwin is Rust tier-1; Xcode CLT gives cc + libc++. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
Adds build-and-test-kernel-windows, the third kernel OS. A green run validates cgo_windows.go's link flags (-l:libdatabricks_sql_kernel.a -lws2_32 -lwsock32 -lrstrtmgr), otherwise never linked in CI, plus the windows-gnu target plumbing. Runs on the self-hosted, allow-listed databricks-protected-runner-group with the windows-server-latest label (as databricks-jdbc does), NOT a hosted runner: create-github-app-token for the private kernel repo must run from an allow-listed IP, and hosted windows-latest is not allow-listed (same org IP-allow-list block that gates the macOS kernel job). Being on the protected runner, crates.io is blocked, so — unlike the hosted-macOS job — this keeps the JFrog Go proxy + cargo→JFrog shim, like the linux kernel job. Windows specifics: install the x86_64-pc-windows-gnu Rust target + mingw-w64 (cgo links GNU archives via gcc, not the MSVC lib the default -msvc target emits; the Makefile passes --target for GOOS=windows), and enable core.longpaths before checkout. No warehouse creds, so live e2e/parity self-skip. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
The first windows-server-latest CI run got through the windows-gnu kernel build and the cgo link, then failed with two undefined references: BCryptGenRandom (Rust getrandom crate) -> bcrypt.dll NtCreateNamedPipeFile (Rust std child_pipe, NT API) -> ntdll.dll mingw does not auto-link these transitively, so add the two system libs to cgo_windows.go's LDFLAGS alongside the existing Winsock / Restart Manager libs. The rest of the Windows path (App token on the allow-listed runner, JFrog cargo proxy, the windows-gnu targeted build producing the .a, and the cgo link invocation itself) worked on the first run; this was the only gap. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
EXPERIMENTAL testing scaffold for this PR: adds on-demand-e2e.yml, triggered by adding the `run-e2e` label to the PR, running the live-warehouse E2E suites on every OS we can actually reach: Thrift E2E (pure-Go, creds only): linux + macOS + Windows (hosted) Kernel E2E (needs private kernel): linux + Windows (protected runners) macOS kernel E2E is intentionally absent — a hosted macOS runner can't mint the GitHub App token for the private kernel repo (org IP allow list) and no allow-listed macOS runner exists in the org. pull_request (not schedule/dispatch) so the PR branch's workflow + code run, and #421 is a same-repo branch so it receives the DATABRICKS_PECOTESTING_* secrets. Each job hard-fails loudly if a credential is missing rather than skipping green. Testing scaffold, expected to be removed or folded into nightly-e2e before merge. Co-authored-by: Isaac Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
The Thrift E2E on windows-latest failed: the DATABRICKS_PECOTESTING_HTTP_PATH2
secret starts with "/" (e.g. /sql/1.0/warehouses/...), and git-bash's MSYS layer
auto-converted that leading-slash value into a Windows path
("C:/Program Files/Git/sql/..."), so the driver POSTed to a mangled path and got
404. Secrets were reachable and the driver was correct — only the shell mangled
the value. linux/macOS are unaffected.
Set MSYS2_ARG_CONV_EXCL='*' and MSYS_NO_PATHCONV=1 on the E2E test steps so the
path reaches Go verbatim. Both are MSYS-only, so they are no-ops on ubuntu/macOS
and on the linux kernel job.
Co-authored-by: Isaac
Signed-off-by: Mani Kaustubh Mathur <mani.mathur@databricks.com>
mani-mathur-arch
added
run-e2e
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
Extends CI to exercise the driver across all supported OSes, so cross-platform stability is verified before release. This is the umbrella OS-CI PR — all OS coverage (pure-Go and kernel/SEA) lands here.
Coverage matrix
Commits (each independently cherry-pickable)
ci: fan out pure-Go driver tests to macOS and Windows—build-and-test-cross-osjob: pure-Go (CGO_ENABLED=0) build + unit tests on hostedmacos-latest/windows-latest× Go 1.25/1.26. No JFrog/Rust/C toolchain needed (all deps public). Mirrors the siblingdatabricks-adbcGo driver.fix: make staging path-traversal check separator-aware— security fix surfaced by the new Windows CI.localPathIsAllowed(connection.go) checked for the literal"../", butfilepath.Relreturns..\on Windows, so the staging allowlist failed open on Windows (allowed a local file escapingstagingAllowedLocalPaths). Present since the driver's first commit (Sept 2023), default Thrift path only (kernel rejects staging before this guard). Fixed with a separator-aware check. (This commit is a standalone security fix — a candidate to cherry-pick into its own PR; see fix: make staging path-traversal check separator-aware (fail-open on Windows) #422.)feat(kernel): enable linux/arm64 target— addscgo_linux_arm64.goand narrowscgo_unsupported.goso linux/arm64 is a supported kernel target. Verified each target selects exactly one link file. Not yet CI-exercised (no allow-listed arm64 runner in the org).build(kernel): support an explicit cargo --target for windows-gnu—kernel-lib.sh+MakefilegainKERNEL_CARGO_TARGET, set tox86_64-pc-windows-gnuforGOOS=windows(cgo links GNU archives, not the MSVC lib the default-msvctarget emits). Linux/macOS behavior unchanged.ci: add gated SEA/kernel test job on macOS— see the infra blocker below.ci: add SEA/kernel test job on Windows— kernel/SEA on the protected runner group +windows-server-latest(allow-listed), installing the windows-gnu Rust target + mingw-w64.Known infra blocker — macOS kernel job is gated OFF
The macOS kernel job (
build-and-test-kernel-macos) is gated behindvars.KERNEL_MACOS_CI_ENABLED == 'true'and does not run by default. Reason: the kernel repo (databricks-sql-kernel) is private, so building its.aneeds a GitHub App token — but thedatabricksorg enforces an IP allow list, and GitHub-hostedmacos-latestrunners have public IPs that are not allow-listed, socreate-github-app-tokenfails before cargo/cgo ever run. The Linux/Windows kernel jobs avoid this only because they run on the self-hosted, allow-listeddatabricks-protected-runner-group— which offers awindows-server-latestlabel but no macOS label anywhere in the org today.Two ways to enable it (then flip the repo var to
'true'):runs-onat it; ordarwin_arm64.a(the distribution download path) — then drop the App-token + private clone and run on hostedmacos-latest.The job body is otherwise complete and validated by review; only the runner/artifact is missing. The one code-level unknown it would confirm is
cgo_darwin.go's never-CI-linked flags (positional.afor ld64,-lc++,@loader_path).Not merging yet
Building this out into a comprehensive cross-OS release gate. linux/arm64 kernel (Part D) is code-ready but has no runner. macOS kernel (Part B) is code-ready but infra-blocked as above.
This pull request and its description were written by Isaac.
Update: Windows kernel link fix (commit
fix(kernel): add bcrypt and ntdll ...)First
windows-server-latestkernel run validated most of Part C on the first try — the allow-listed runner minted the App token, the JFrog cargo proxy worked, the--target x86_64-pc-windows-gnubuild produced the.a, and cgo invoked the mingw link with the intended flags. It failed only on two undefined references (BCryptGenRandom→bcrypt,NtCreateNamedPipeFile→ntdll) that mingw doesn't auto-link. Added-lbcrypt -lntdlltocgo_windows.go; re-running.