Skip to content

Commit 58d02a4

Browse files
committed
Pass DATABRICKS_CLI_PATH and resolve the platform-specific CLI binary
*Why*: * On Windows, `databricks bundle` deploys failed with "databricks CLI not found" because the SDK/Terraform provider could not locate the bundled CLI. * The forwarded path was extensionless (`bin/databricks`), but the bundled Windows binary is `databricks.exe`; the Go SDK/Terraform do a literal file lookup and do not auto-append `.exe` the way Windows CreateProcess does. * The legacy project.json persisted the bundled CLI path (`databricksPath`), which `fromJSON` preferred over the freshly resolved one. That snapshot is version-pinned and, on Windows, extensionless, so it overrode the corrected path during legacy-to-bundle migration. *What:* * Forward the resolved CLI path to subprocesses via the DATABRICKS_CLI_PATH env var so they don't fall back to a PATH search. * Make CliWrapper.cliPath return the platform-specific binary name (`databricks.exe` on win32, `databricks` elsewhere). * In AuthProvider.fromJSON, always use the freshly resolved bundled CLI path and ignore the persisted `databricksPath`, since it is an extension-managed, version-pinned value that is stale after any upgrade. *Verification:* * Added unit tests for the platform-specific binary name, for toEnv() exposing DATABRICKS_CLI_PATH, and for fromJSON ignoring a stale persisted databricksPath; suite passes in the VS Code test host (14 relevant tests green). * Built the win32-x64 VSIX and confirmed the bundled extension contains both the env var and the `.exe` path, with `bin/databricks.exe` packaged. Co-authored-by: Isaac
1 parent 79ef3eb commit 58d02a4

4 files changed

Lines changed: 117 additions & 2 deletions

File tree

packages/databricks-vscode/src/cli/CliWrapper.test.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,33 @@ describe(__filename, function () {
5353
assert.ok(result.stdout.indexOf("databricks") > 0);
5454
});
5555

56+
it("should resolve the platform-specific CLI binary name", () => {
57+
const cli = createCliWrapper();
58+
const originalPlatform = process.platform;
59+
const setPlatform = (platform: NodeJS.Platform) =>
60+
Object.defineProperty(process, "platform", {value: platform});
61+
try {
62+
// On Windows the bundled binary is `databricks.exe`. The `.exe` is
63+
// required because cliPath is forwarded to the SDK/Terraform via
64+
// DATABRICKS_CLI_PATH, which does a literal (no auto-`.exe`) lookup.
65+
setPlatform("win32");
66+
assert.ok(
67+
cli.cliPath.endsWith(path.join("bin", "databricks.exe")),
68+
`expected win32 cliPath to end with bin/databricks.exe, got ${cli.cliPath}`
69+
);
70+
71+
for (const platform of ["linux", "darwin"] as NodeJS.Platform[]) {
72+
setPlatform(platform);
73+
assert.ok(
74+
cli.cliPath.endsWith(path.join("bin", "databricks")),
75+
`expected ${platform} cliPath to end with bin/databricks, got ${cli.cliPath}`
76+
);
77+
}
78+
} finally {
79+
setPlatform(originalPlatform);
80+
}
81+
});
82+
5683
let mocks: any[] = [];
5784
afterEach(() => {
5885
mocks.forEach((mock) => reset(mock));

packages/databricks-vscode/src/cli/CliWrapper.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,15 @@ export class CliWrapper {
298298
}
299299

300300
get cliPath(): string {
301-
return this.extensionContext.asAbsolutePath("./bin/databricks");
301+
// The bundled binary is named `databricks.exe` on Windows. We must
302+
// include the extension here: while spawning the CLI ourselves works
303+
// without it (Windows' CreateProcess auto-appends `.exe`), this path is
304+
// also forwarded to the Databricks Go SDK / Terraform provider via the
305+
// DATABRICKS_CLI_PATH env var, and they do a literal file lookup that
306+
// fails on an extensionless path with "databricks CLI not found".
307+
const binName =
308+
process.platform === "win32" ? "databricks.exe" : "databricks";
309+
return this.extensionContext.asAbsolutePath(`./bin/${binName}`);
302310
}
303311

304312
getLoggingArguments(): string[] {
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import * as assert from "assert";
2+
import {instance, mock, when} from "ts-mockito";
3+
import {AuthProvider, DatabricksCliAuthProvider} from "./AuthProvider";
4+
import {CliWrapper} from "../../cli/CliWrapper";
5+
6+
describe(__filename, () => {
7+
describe("DatabricksCliAuthProvider.toEnv", () => {
8+
const host = new URL("https://test.cloud.databricks.com");
9+
const cliPath = "/path/to/bin/databricks";
10+
11+
function createProvider(profile?: string, workspaceId?: string) {
12+
return new DatabricksCliAuthProvider(
13+
host,
14+
cliPath,
15+
instance(mock(CliWrapper)),
16+
profile,
17+
workspaceId
18+
);
19+
}
20+
21+
it("should expose DATABRICKS_CLI_PATH so the SDK/Terraform provider can locate the bundled CLI", () => {
22+
const env = createProvider("dev").toEnv();
23+
24+
assert.equal(env["DATABRICKS_CLI_PATH"], cliPath);
25+
assert.equal(env["DATABRICKS_HOST"], host.toString());
26+
assert.equal(env["DATABRICKS_AUTH_TYPE"], "databricks-cli");
27+
assert.equal(env["DATABRICKS_CONFIG_PROFILE"], "dev");
28+
});
29+
30+
it("should include DATABRICKS_CLI_PATH even without a profile or workspace id", () => {
31+
const env = createProvider().toEnv();
32+
33+
assert.equal(env["DATABRICKS_CLI_PATH"], cliPath);
34+
assert.ok(!("DATABRICKS_CONFIG_PROFILE" in env));
35+
assert.ok(!("DATABRICKS_WORKSPACE_ID" in env));
36+
});
37+
});
38+
39+
describe("AuthProvider.fromJSON", () => {
40+
it("should ignore a persisted databricksPath and use the freshly resolved bundled CLI path", () => {
41+
// Simulate an upgraded install: the new extension resolves a
42+
// versioned, platform-correct path, while project.json still holds
43+
// an old, extensionless snapshot from the previous version.
44+
const freshPath =
45+
"/ext/databricks.databricks-2.12.0/bin/databricks.exe";
46+
const stalePath =
47+
"/ext/databricks.databricks-2.11.0/bin/databricks";
48+
49+
const cliMock = mock(CliWrapper);
50+
when(cliMock.cliPath).thenReturn(freshPath);
51+
52+
const provider = AuthProvider.fromJSON(
53+
{
54+
host: "https://test.cloud.databricks.com",
55+
authType: "databricks-cli",
56+
databricksPath: stalePath,
57+
profile: "dev",
58+
},
59+
instance(cliMock)
60+
);
61+
62+
assert.ok(provider instanceof DatabricksCliAuthProvider);
63+
assert.equal(
64+
(provider as DatabricksCliAuthProvider).cliPath,
65+
freshPath
66+
);
67+
});
68+
});
69+
});

packages/databricks-vscode/src/configuration/auth/AuthProvider.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,14 @@ export abstract class AuthProvider {
163163
case "databricks-cli":
164164
return new DatabricksCliAuthProvider(
165165
host,
166-
json.databricksPath ?? cli.cliPath,
166+
// Always use the freshly resolved bundled CLI path and
167+
// ignore any persisted `databricksPath`. That field is a
168+
// snapshot of a previous install's bundled binary: it is
169+
// version-pinned (e.g. .../databricks-2.11.0/bin/...) and,
170+
// on Windows, was stored without the `.exe` suffix, so a
171+
// persisted value is stale/wrong and must not win over the
172+
// path computed by the current extension.
173+
cli.cliPath,
167174
cli,
168175
json.profile,
169176
json.workspaceId
@@ -355,6 +362,10 @@ export class DatabricksCliAuthProvider extends AuthProvider {
355362
const env: Record<string, string> = {
356363
DATABRICKS_HOST: this.host.toString(),
357364
DATABRICKS_AUTH_TYPE: "databricks-cli",
365+
// Point the SDK/Terraform provider at the bundled CLI so they don't
366+
// fall back to searching PATH (and fail with "databricks CLI not
367+
// found") in subprocesses that don't inherit our resolved path.
368+
DATABRICKS_CLI_PATH: this.cliPath,
358369
};
359370
if (this.profile) {
360371
env["DATABRICKS_CONFIG_PROFILE"] = this.profile;

0 commit comments

Comments
 (0)