Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion packages/databricks-vscode/src/cli/CliWrapper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@ import {ChildProcess, ChildProcessWithoutNullStreams} from "child_process";
import {Readable} from "stream";

const execFile = promisify(execFileCb);
const cliPath = path.join(__dirname, "../../bin/databricks");
// Mirror CliWrapper.cliPath: the bundled binary is `databricks.exe` on Windows.
const cliPath = path.join(
__dirname,
"../../bin/" +
(process.platform === "win32" ? "databricks.exe" : "databricks")
);

// eslint-disable-next-line @typescript-eslint/no-var-requires
const extensionVersion = require("../../package.json").version;
Expand Down Expand Up @@ -53,6 +58,33 @@ describe(__filename, function () {
assert.ok(result.stdout.indexOf("databricks") > 0);
});

it("should resolve the platform-specific CLI binary name", () => {
const cli = createCliWrapper();
const originalPlatform = process.platform;
const setPlatform = (platform: NodeJS.Platform) =>
Object.defineProperty(process, "platform", {value: platform});
try {
// On Windows the bundled binary is `databricks.exe`. The `.exe` is
// required because cliPath is forwarded to the SDK/Terraform via
// DATABRICKS_CLI_PATH, which does a literal (no auto-`.exe`) lookup.
setPlatform("win32");
assert.ok(
cli.cliPath.endsWith(path.join("bin", "databricks.exe")),
`expected win32 cliPath to end with bin/databricks.exe, got ${cli.cliPath}`
);

for (const platform of ["linux", "darwin"] as NodeJS.Platform[]) {
setPlatform(platform);
assert.ok(
cli.cliPath.endsWith(path.join("bin", "databricks")),
`expected ${platform} cliPath to end with bin/databricks, got ${cli.cliPath}`
);
}
} finally {
setPlatform(originalPlatform);
}
});

let mocks: any[] = [];
afterEach(() => {
mocks.forEach((mock) => reset(mock));
Expand Down
10 changes: 9 additions & 1 deletion packages/databricks-vscode/src/cli/CliWrapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,15 @@ export class CliWrapper {
}

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

getLoggingArguments(): string[] {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import * as assert from "assert";
import {instance, mock, when} from "ts-mockito";
import {AuthProvider, DatabricksCliAuthProvider} from "./AuthProvider";
import {CliWrapper} from "../../cli/CliWrapper";

describe(__filename, () => {
describe("DatabricksCliAuthProvider.toEnv", () => {
const host = new URL("https://test.cloud.databricks.com");
const cliPath = "/path/to/bin/databricks";

function createProvider(profile?: string, workspaceId?: string) {
return new DatabricksCliAuthProvider(
host,
cliPath,
instance(mock(CliWrapper)),
profile,
workspaceId
);
}

it("should expose DATABRICKS_CLI_PATH so the SDK/Terraform provider can locate the bundled CLI", () => {
const env = createProvider("dev").toEnv();

assert.equal(env["DATABRICKS_CLI_PATH"], cliPath);
assert.equal(env["DATABRICKS_HOST"], host.toString());
assert.equal(env["DATABRICKS_AUTH_TYPE"], "databricks-cli");
assert.equal(env["DATABRICKS_CONFIG_PROFILE"], "dev");
});

it("should include DATABRICKS_CLI_PATH even without a profile or workspace id", () => {
const env = createProvider().toEnv();

assert.equal(env["DATABRICKS_CLI_PATH"], cliPath);
assert.ok(!("DATABRICKS_CONFIG_PROFILE" in env));
assert.ok(!("DATABRICKS_WORKSPACE_ID" in env));
});
});

describe("AuthProvider.fromJSON", () => {
it("should ignore a persisted databricksPath and use the freshly resolved bundled CLI path", () => {
// Simulate an upgraded install: the new extension resolves a
// versioned, platform-correct path, while project.json still holds
// an old, extensionless snapshot from the previous version.
const freshPath =
"/ext/databricks.databricks-2.12.0/bin/databricks.exe";
const stalePath =
"/ext/databricks.databricks-2.11.0/bin/databricks";

const cliMock = mock(CliWrapper);
when(cliMock.cliPath).thenReturn(freshPath);

const provider = AuthProvider.fromJSON(
{
host: "https://test.cloud.databricks.com",
authType: "databricks-cli",
databricksPath: stalePath,
profile: "dev",
},
instance(cliMock)
);

assert.ok(provider instanceof DatabricksCliAuthProvider);
assert.equal(
(provider as DatabricksCliAuthProvider).cliPath,
freshPath
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,14 @@ export abstract class AuthProvider {
case "databricks-cli":
return new DatabricksCliAuthProvider(
host,
json.databricksPath ?? cli.cliPath,
// Always use the freshly resolved bundled CLI path and
// ignore any persisted `databricksPath`. That field is a
// snapshot of a previous install's bundled binary: it is
// version-pinned (e.g. .../databricks-2.11.0/bin/...) and,
// on Windows, was stored without the `.exe` suffix, so a
// persisted value is stale/wrong and must not win over the
// path computed by the current extension.
cli.cliPath,
cli,
json.profile,
json.workspaceId
Expand Down Expand Up @@ -355,6 +362,10 @@ export class DatabricksCliAuthProvider extends AuthProvider {
const env: Record<string, string> = {
DATABRICKS_HOST: this.host.toString(),
DATABRICKS_AUTH_TYPE: "databricks-cli",
// Point the SDK/Terraform provider at the bundled CLI so they don't
// fall back to searching PATH (and fail with "databricks CLI not
// found") in subprocesses that don't inherit our resolved path.
DATABRICKS_CLI_PATH: this.cliPath,
};
if (this.profile) {
env["DATABRICKS_CONFIG_PROFILE"] = this.profile;
Expand Down
Loading