From a9da02612e6002407fe7fe2fdc62c8aa68fbccd8 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 19 Jun 2026 10:25:14 +0000 Subject: [PATCH] acc: make acceptance tests work in Databricks development environments The acceptance harness blocks external network access during tests. In some Databricks-internal development environments, local tooling issues network calls that trip the sandbox and fail the suite even though CI passes. Ignore the local probes that are not test traffic and disable the background beacons so tests behave the same as on CI. Also assert the external toolchain (jq, uv, ruff) and provision python via uv up front, so a missing or stale tool fails fast with a clear message. Co-authored-by: Isaac --- .github/workflows/check.yml | 10 +++--- Taskfile.yml | 6 ++-- acceptance/acceptance_test.go | 23 ++++++++++++++ acceptance/internal/jq.go | 32 ++++++++++++++++++++ acceptance/internal/jq_test.go | 17 +++++++++++ acceptance/internal/python.go | 42 ++++++++++++++++++++++++++ acceptance/internal/python_test.go | 23 ++++++++++++++ acceptance/internal/rejecting_proxy.go | 8 +++++ acceptance/internal/ruff.go | 38 +++++++++++++++++++++++ acceptance/internal/ruff_test.go | 17 +++++++++++ acceptance/internal/uv.go | 32 ++++++++++++++++++++ acceptance/internal/uv_test.go | 16 ++++++++++ libs/testserver/server.go | 26 ++++++++++++++++ libs/testserver/server_test.go | 35 +++++++++++++++++++++ 14 files changed, 318 insertions(+), 7 deletions(-) create mode 100644 acceptance/internal/jq.go create mode 100644 acceptance/internal/jq_test.go create mode 100644 acceptance/internal/python.go create mode 100644 acceptance/internal/python_test.go create mode 100644 acceptance/internal/ruff.go create mode 100644 acceptance/internal/ruff_test.go create mode 100644 acceptance/internal/uv.go create mode 100644 acceptance/internal/uv_test.go create mode 100644 libs/testserver/server_test.go diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 5bff4cf9935..b8b1208686c 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -35,6 +35,11 @@ jobs: - name: Fail if go.mod/go.sum changed run: git diff --exit-code + - name: Install uv + uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 + with: + version: "0.8.9" + - name: Run Go lint checks (does not include formatting checks) run: go tool -modfile=tools/task/go.mod task lint @@ -44,11 +49,6 @@ jobs: version: "0.9.1" args: "format --check" - - name: Install uv - uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 - with: - version: "0.8.9" - - name: "task fmt: Python and Go formatting" # Python formatting is already checked above, but this also checks Go and YAML formatting run: | diff --git a/Taskfile.yml b/Taskfile.yml index 488e2284cc6..d6417f271e7 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -12,8 +12,10 @@ vars: # a separate static `**/testdata/**` glob, not this script. # Limitation: git grep only scans tracked files; new //go:embed directives in # untracked files are missed until the file is staged or committed. + # Run via uv with a pinned interpreter floor so the helper works on hosts whose + # default python3 is older than the 3.11 this repo's scripts target. EMBED_SOURCES: - sh: 'python3 tools/list_embeds.py' + sh: 'uv run -p ">=3.11" --no-project python tools/list_embeds.py' # pydabs-* tasks live in python/Taskfile.yml so `task pydabs-foo` works when # run from python/. Flattened so they keep their `pydabs-` names at the root. @@ -948,7 +950,7 @@ tasks: generates: - bundle/direct/dresources/apitypes.generated.yml cmds: - - "sh -c 'python3 bundle/direct/tools/generate_apitypes.py .codegen/cli.json acceptance/bundle/refschema/out.fields.txt > bundle/direct/dresources/apitypes.generated.yml'" + - "sh -c 'uv run -p \">=3.11\" --no-project python bundle/direct/tools/generate_apitypes.py .codegen/cli.json acceptance/bundle/refschema/out.fields.txt > bundle/direct/dresources/apitypes.generated.yml'" generate-direct-resources: desc: Generate direct engine resources YAML diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 2d3326b23d6..80658e1806c 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -229,6 +229,14 @@ func testAccept(t *testing.T, inprocessMode bool, singleTest string) int { os.Unsetenv(v) //nolint:usetesting // t.Setenv cannot unset } + // Verify external tool prerequisites before doing any work, so a stale + // toolchain fails fast with an actionable message instead of producing + // confusing diffs deep into the run. + internal.RequireModernJq(t) + internal.RequireModernUv(t) + internal.RequireModernRuff(t) + internal.EnsureModernPython(t) + buildDir := getBuildDir(t, cwd, runtime.GOOS, runtime.GOARCH) // Set up terraform for tests. Skip on DBR - tests with RunsOnDbr only use direct deployment. @@ -759,6 +767,21 @@ func runTest(t *testing.T, // into compared output. Tests can override this via [Env] in test.toml. cmd.Env = append(cmd.Env, "DATABRICKS_CLI_DISABLE_UPDATE_CHECK=true") + // Neutralize Databricks-internal development-environment interference so + // acceptance tests behave the same as on CI (which has none of this). Two + // sources both reach the blocking proxy on every git invocation: + // + // 1. A command-timing shim that wraps git (ahead of the real binary on + // PATH) and POSTs per-command metrics over the network. + // COMMAND_TIMER_DISABLE=1 makes it pass through without the beacon. + // 2. A managed global git config installs a core.hooksPath whose hooks + // (secret scanning, etc.) also beacon metrics. Ignoring the global and + // system git config disables those hooks and keeps tests hermetic; tests + // configure the repos they create via git-repo-init locally. + cmd.Env = append(cmd.Env, "COMMAND_TIMER_DISABLE=1") + cmd.Env = append(cmd.Env, "GIT_CONFIG_GLOBAL="+os.DevNull) + cmd.Env = append(cmd.Env, "GIT_CONFIG_SYSTEM="+os.DevNull) + for _, kv := range testEnv { key, value, _ := strings.Cut(kv, "=") // Only add replacement by default if value is part of EnvMatrix with more than 1 option and length is 4 or more chars diff --git a/acceptance/internal/jq.go b/acceptance/internal/jq.go new file mode 100644 index 00000000000..3fe7805d82a --- /dev/null +++ b/acceptance/internal/jq.go @@ -0,0 +1,32 @@ +package internal + +import ( + "fmt" + "os/exec" + "strings" + "testing" +) + +// RequireModernJq fails the run if jq is missing or older than 1.7. Acceptance +// scripts use jq 1.7 features (the pick/1 builtin and the `.foo.[]` iteration +// syntax); an older jq compiles them as errors and produces spurious diffs +// across many tests rather than one clear failure. +func RequireModernJq(t *testing.T) { + out, err := exec.Command("jq", "--version").Output() + if err != nil { + t.Fatalf("jq not found on PATH (acceptance tests require jq >= 1.7): %v", err) + } + version := strings.TrimSpace(string(out)) + if !jqVersionOK(version) { + t.Fatalf("acceptance tests require jq >= 1.7 (found %q); install a newer jq", version) + } +} + +// jqVersionOK reports whether `jq --version` output (e.g. "jq-1.7.1") is >= 1.7. +func jqVersionOK(version string) bool { + var major, minor int + if _, err := fmt.Sscanf(version, "jq-%d.%d", &major, &minor); err != nil { + return false + } + return major > 1 || (major == 1 && minor >= 7) +} diff --git a/acceptance/internal/jq_test.go b/acceptance/internal/jq_test.go new file mode 100644 index 00000000000..bb1aba9f4cd --- /dev/null +++ b/acceptance/internal/jq_test.go @@ -0,0 +1,17 @@ +package internal + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestJqVersionOK(t *testing.T) { + assert.True(t, jqVersionOK("jq-1.7")) + assert.True(t, jqVersionOK("jq-1.7.1")) + assert.True(t, jqVersionOK("jq-1.8.1")) + assert.True(t, jqVersionOK("jq-2.0")) + assert.False(t, jqVersionOK("jq-1.6")) + assert.False(t, jqVersionOK("jq version 1.7")) + assert.False(t, jqVersionOK("")) +} diff --git a/acceptance/internal/python.go b/acceptance/internal/python.go new file mode 100644 index 00000000000..d69f3a899f8 --- /dev/null +++ b/acceptance/internal/python.go @@ -0,0 +1,42 @@ +package internal + +import ( + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// EnsureModernPython makes `python3` on PATH resolve to a Python >= 3.11, the +// version this repo's scripts target. Acceptance scripts invoke `python3` +// directly and some import stdlib modules added in 3.11 (e.g. tomllib in +// acceptance/bundle/resources/permissions/analyze_requests.py), but a host's +// default python3 may be older. uv (already required for building the +// databricks-bundles wheel) discovers or provisions a suitable interpreter; we +// symlink it as python3/python into a temp dir prepended to PATH so every +// script and build step resolves it. Fails hard if uv is missing or has no +// suitable Python. +func EnsureModernPython(t *testing.T) { + // Windows runners already ship a python3 >= 3.11, and os.Symlink needs extra + // privileges there, so don't provision: use the interpreter already on PATH. + if runtime.GOOS == "windows" { + return + } + + out, err := exec.Command("uv", "python", "find", ">=3.11").Output() + if err != nil { + t.Fatalf("uv could not find python >= 3.11: %v", err) + } + python := strings.TrimSpace(string(out)) + + binDir := t.TempDir() + for _, link := range []string{"python3", "python"} { + if err := os.Symlink(python, filepath.Join(binDir, link)); err != nil { + t.Fatalf("failed to symlink %s as %s: %v", python, link, err) + } + } + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) //nolint:forbidigo // acceptance test harness; no ctx for libs/env + t.Logf("acceptance tests: using %s (via uv) as python3", python) +} diff --git a/acceptance/internal/python_test.go b/acceptance/internal/python_test.go new file mode 100644 index 00000000000..ad066ed789c --- /dev/null +++ b/acceptance/internal/python_test.go @@ -0,0 +1,23 @@ +package internal + +import ( + "os/exec" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestEnsureModernPython(t *testing.T) { + if _, err := exec.LookPath("uv"); err != nil { + t.Skip("uv not installed") + } + + EnsureModernPython(t) + + // After setup, the python3 resolved from PATH must satisfy the floor. + out, err := exec.Command("python3", "-c", "import sys; print(sys.version_info >= (3, 11))").Output() + require.NoError(t, err) + assert.Equal(t, "True", strings.TrimSpace(string(out))) +} diff --git a/acceptance/internal/rejecting_proxy.go b/acceptance/internal/rejecting_proxy.go index 569fa490bc8..591954ed15a 100644 --- a/acceptance/internal/rejecting_proxy.go +++ b/acceptance/internal/rejecting_proxy.go @@ -7,6 +7,8 @@ import ( "net/http" "strings" "testing" + + "github.com/databricks/cli/libs/testserver" ) // StartRejectingProxy starts an HTTP proxy server bound to a loopback port and @@ -93,6 +95,12 @@ func handleBlockedConnection(t *testing.T, conn net.Conn, hint string) { if isLoopback || isReserved { // Expected unreachable fixture or local test server — log only, don't fail. t.Logf("blocking proxy: blocked loopback/reserved host: %s", detail) + } else if testserver.IsLocalhostProbe(req) { + // Some Databricks-internal development environments run a port watcher + // that auto-forwards every new localhost listener and probes it with + // `HEAD / Host: localhost`. This is not the CLI-under-test reaching the + // internet, so log it instead of failing the test. + t.Logf("blocking proxy: ignored localhost port-classification probe: %s", detail) } else { t.Errorf("internet access blocked by proxy: %s%s", detail, hint) } diff --git a/acceptance/internal/ruff.go b/acceptance/internal/ruff.go new file mode 100644 index 00000000000..2b9e165d27d --- /dev/null +++ b/acceptance/internal/ruff.go @@ -0,0 +1,38 @@ +package internal + +import ( + "fmt" + "os/exec" + "strings" + "testing" +) + +// RequireModernRuff fails the run if ruff is missing or older than 0.9.1, the +// version pinned across the repo (python/pyproject.toml, Taskfile.yml). The +// pydabs check-formatting acceptance test runs `ruff format` and its golden +// output assumes that formatter behavior. +func RequireModernRuff(t *testing.T) { + out, err := exec.Command("ruff", "--version").Output() + if err != nil { + t.Fatalf("ruff not found on PATH (acceptance tests require ruff >= 0.9.1): %v", err) + } + version := strings.TrimSpace(string(out)) + if !ruffVersionOK(version) { + t.Fatalf("acceptance tests require ruff >= 0.9.1 (found %q); install a newer ruff", version) + } +} + +// ruffVersionOK reports whether `ruff --version` output (e.g. "ruff 0.9.1") is >= 0.9.1. +func ruffVersionOK(version string) bool { + var major, minor, patch int + if _, err := fmt.Sscanf(version, "ruff %d.%d.%d", &major, &minor, &patch); err != nil { + return false + } + if major != 0 { + return major > 0 + } + if minor != 9 { + return minor > 9 + } + return patch >= 1 +} diff --git a/acceptance/internal/ruff_test.go b/acceptance/internal/ruff_test.go new file mode 100644 index 00000000000..99840755646 --- /dev/null +++ b/acceptance/internal/ruff_test.go @@ -0,0 +1,17 @@ +package internal + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestRuffVersionOK(t *testing.T) { + assert.True(t, ruffVersionOK("ruff 0.9.1")) + assert.True(t, ruffVersionOK("ruff 0.9.2")) + assert.True(t, ruffVersionOK("ruff 0.11.0")) + assert.True(t, ruffVersionOK("ruff 1.0.0")) + assert.False(t, ruffVersionOK("ruff 0.9.0")) + assert.False(t, ruffVersionOK("ruff 0.8.5")) + assert.False(t, ruffVersionOK("")) +} diff --git a/acceptance/internal/uv.go b/acceptance/internal/uv.go new file mode 100644 index 00000000000..bba765e4e0d --- /dev/null +++ b/acceptance/internal/uv.go @@ -0,0 +1,32 @@ +package internal + +import ( + "fmt" + "os/exec" + "strings" + "testing" +) + +// RequireModernUv fails the run if uv is missing or older than 0.4. uv builds +// the databricks-bundles wheel and provides the test interpreter via +// `uv python find` (see EnsureModernPython), which landed in the 0.3 line; 0.4 +// is a small margin above that. +func RequireModernUv(t *testing.T) { + out, err := exec.Command("uv", "--version").Output() + if err != nil { + t.Fatalf("uv not found on PATH (acceptance tests require uv >= 0.4): %v", err) + } + version := strings.TrimSpace(string(out)) + if !uvVersionOK(version) { + t.Fatalf("acceptance tests require uv >= 0.4 (found %q); install a newer uv", version) + } +} + +// uvVersionOK reports whether `uv --version` output (e.g. "uv 0.11.22 (abc)") is >= 0.4. +func uvVersionOK(version string) bool { + var major, minor int + if _, err := fmt.Sscanf(version, "uv %d.%d", &major, &minor); err != nil { + return false + } + return major > 0 || minor >= 4 +} diff --git a/acceptance/internal/uv_test.go b/acceptance/internal/uv_test.go new file mode 100644 index 00000000000..2b378ec6575 --- /dev/null +++ b/acceptance/internal/uv_test.go @@ -0,0 +1,16 @@ +package internal + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestUvVersionOK(t *testing.T) { + assert.True(t, uvVersionOK("uv 0.4.0")) + assert.True(t, uvVersionOK("uv 0.11.22 (abcdef 2025-01-01)")) + assert.True(t, uvVersionOK("uv 1.0.0")) + assert.False(t, uvVersionOK("uv 0.3.5")) + assert.False(t, uvVersionOK("0.4.0")) + assert.False(t, uvVersionOK("")) +} diff --git a/libs/testserver/server.go b/libs/testserver/server.go index 9dbfe32c5fa..10a283eeadc 100644 --- a/libs/testserver/server.go +++ b/libs/testserver/server.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "maps" + "net" "net/http" "net/http/httptest" "net/url" @@ -24,6 +25,24 @@ const testPidKey = "test-pid" var testPidRegex = regexp.MustCompile(testPidKey + `/(\d+)`) +// IsLocalhostProbe reports whether r is an external port-classification probe +// rather than traffic from the CLI-under-test or its helper scripts. +// +// Some Databricks-internal development environments run a port watcher that +// auto-forwards every new localhost listener and probes it to decide whether it +// speaks HTTP or HTTPS, connecting back and sending `HEAD / HTTP/1.0` with +// `Host: localhost`. All legitimate test traffic is configured against +// 127.0.0.1:PORT, so the Host is the reliable discriminator: a request whose +// host is bare "localhost" never originates from the test. The method and path +// checks keep the match tight so a genuinely misdirected request still surfaces. +func IsLocalhostProbe(r *http.Request) bool { + host := r.Host + if h, _, err := net.SplitHostPort(host); err == nil { + host = h + } + return host == "localhost" && r.Method == http.MethodHead && r.URL.Path == "/" +} + func ExtractPidFromHeaders(headers http.Header) int { ua := headers.Get("User-Agent") matches := testPidRegex.FindStringSubmatch(ua) @@ -243,6 +262,13 @@ func New(t testutil.TestingT) *Server { // Set up the not found handler as fallback notFoundFunc := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Answer external port-classification probes benignly instead of failing + // the test with a spurious "No handler" error. See IsLocalhostProbe. + if IsLocalhostProbe(r) { + w.WriteHeader(http.StatusOK) + return + } + pattern := r.Method + " " + r.URL.Path bodyBytes, err := io.ReadAll(r.Body) var body string diff --git a/libs/testserver/server_test.go b/libs/testserver/server_test.go new file mode 100644 index 00000000000..6c6dfe8c160 --- /dev/null +++ b/libs/testserver/server_test.go @@ -0,0 +1,35 @@ +package testserver_test + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/databricks/cli/libs/testserver" + "github.com/stretchr/testify/assert" +) + +func TestIsLocalhostProbe(t *testing.T) { + tests := []struct { + name string + method string + target string + host string + want bool + }{ + {"localhost probe", http.MethodHead, "/", "localhost", true}, + {"localhost probe with port", http.MethodHead, "/", "localhost:8080", true}, + {"cli request to loopback ip", http.MethodGet, "/api/2.0/jobs/list", "127.0.0.1:12345", false}, + {"head to loopback ip", http.MethodHead, "/", "127.0.0.1:12345", false}, + {"get to localhost root", http.MethodGet, "/", "localhost", false}, + {"head to localhost non-root", http.MethodHead, "/api/2.0/jobs/list", "localhost", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := httptest.NewRequest(tt.method, tt.target, nil) + r.Host = tt.host + assert.Equal(t, tt.want, testserver.IsLocalhostProbe(r)) + }) + } +}