From 2d6ae426c83fe6d0ac64f448f4bdd757668123cc Mon Sep 17 00:00:00 2001 From: Josh Vawdrey Date: Fri, 3 Nov 2023 16:39:11 +1100 Subject: [PATCH 1/8] feat: fallback to env var if configuration not set --- .DS_Store | Bin 0 -> 6148 bytes package.json | 4 ++-- src/remote.ts | 3 ++- src/storage.ts | 8 +++++++- 4 files changed, 11 insertions(+), 4 deletions(-) create mode 100644 .DS_Store diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..cdc8eda77d0d917ed493938a3994ae7aca486ea1 GIT binary patch literal 6148 zcmeH~Jr2S!425ml0g0s}V-^m;4I%_5-~tF3kvaf-j?VMXLSaS~dY0@jc51bKLsN^0 z?w;4J$RHw1xKTD1CZ@-};&kK@fsZi}Q9;Db!|bDN+7RDcRl0V+TRW~4wK z 0) { headerArg = ` --header-command ${escape(headerCommand)}` } diff --git a/src/storage.ts b/src/storage.ts index 8bd2ceb8..d208aeae 100644 --- a/src/storage.ts +++ b/src/storage.ts @@ -397,7 +397,13 @@ export class Storage { } public async getHeaders(url = this.getURL()): Promise> { - return getHeaders(url, vscode.workspace.getConfiguration().get("coder.headerCommand"), this) + console.log(process.env) + + return getHeaders( + url, + vscode.workspace.getConfiguration().get("coder.headerCommand") || process.env.CODER_HEADER_COMMAND, + this, + ) } } From 4e4144bc7c3cf949bcaca8900418189ca02c84b9 Mon Sep 17 00:00:00 2001 From: Josh Vawdrey Date: Fri, 3 Nov 2023 16:39:21 +1100 Subject: [PATCH 2/8] commit --- .DS_Store | Bin 6148 -> 0 bytes 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 .DS_Store diff --git a/.DS_Store b/.DS_Store deleted file mode 100644 index cdc8eda77d0d917ed493938a3994ae7aca486ea1..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeH~Jr2S!425ml0g0s}V-^m;4I%_5-~tF3kvaf-j?VMXLSaS~dY0@jc51bKLsN^0 z?w;4J$RHw1xKTD1CZ@-};&kK@fsZi}Q9;Db!|bDN+7RDcRl0V+TRW~4wK z Date: Fri, 3 Nov 2023 16:48:29 +1100 Subject: [PATCH 3/8] chore: remove console.log --- src/storage.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/storage.ts b/src/storage.ts index d208aeae..9d86673d 100644 --- a/src/storage.ts +++ b/src/storage.ts @@ -397,8 +397,6 @@ export class Storage { } public async getHeaders(url = this.getURL()): Promise> { - console.log(process.env) - return getHeaders( url, vscode.workspace.getConfiguration().get("coder.headerCommand") || process.env.CODER_HEADER_COMMAND, From 9d566e290c0fc18ea55877c6bdc444421d6808d1 Mon Sep 17 00:00:00 2001 From: Josh Vawdrey Date: Fri, 3 Nov 2023 17:07:09 +1100 Subject: [PATCH 4/8] chore: add tests --- src/headers.test.ts | 125 +++++++++++++++++++++++++++++--------------- src/headers.ts | 10 ++++ src/remote.ts | 4 +- src/storage.ts | 8 +-- 4 files changed, 98 insertions(+), 49 deletions(-) diff --git a/src/headers.test.ts b/src/headers.test.ts index 422e23ec..8397eeec 100644 --- a/src/headers.test.ts +++ b/src/headers.test.ts @@ -1,6 +1,7 @@ import * as os from "os" -import { it, expect } from "vitest" -import { getHeaders } from "./headers" +import { it, expect, describe, beforeEach, afterEach, vi } from "vitest" +import { WorkspaceConfiguration } from "vscode" +import { getHeaderCommand, getHeaders } from "./headers" const logger = { writeToCoderOutputChannel() { @@ -8,50 +9,92 @@ const logger = { }, } -it("should return no headers", async () => { - await expect(getHeaders(undefined, undefined, logger)).resolves.toStrictEqual({}) - await expect(getHeaders("localhost", undefined, logger)).resolves.toStrictEqual({}) - await expect(getHeaders(undefined, "command", logger)).resolves.toStrictEqual({}) - await expect(getHeaders("localhost", "", logger)).resolves.toStrictEqual({}) - await expect(getHeaders("", "command", logger)).resolves.toStrictEqual({}) - await expect(getHeaders("localhost", " ", logger)).resolves.toStrictEqual({}) - await expect(getHeaders(" ", "command", logger)).resolves.toStrictEqual({}) -}) +describe("getHeaders", () => { + it("should return no headers", async () => { + await expect(getHeaders(undefined, undefined, logger)).resolves.toStrictEqual({}) + await expect(getHeaders("localhost", undefined, logger)).resolves.toStrictEqual({}) + await expect(getHeaders(undefined, "command", logger)).resolves.toStrictEqual({}) + await expect(getHeaders("localhost", "", logger)).resolves.toStrictEqual({}) + await expect(getHeaders("", "command", logger)).resolves.toStrictEqual({}) + await expect(getHeaders("localhost", " ", logger)).resolves.toStrictEqual({}) + await expect(getHeaders(" ", "command", logger)).resolves.toStrictEqual({}) + }) -it("should return headers", async () => { - await expect(getHeaders("localhost", "printf 'foo=bar\\nbaz=qux'", logger)).resolves.toStrictEqual({ - foo: "bar", - baz: "qux", + it("should return headers", async () => { + await expect(getHeaders("localhost", "printf 'foo=bar\\nbaz=qux'", logger)).resolves.toStrictEqual({ + foo: "bar", + baz: "qux", + }) + await expect(getHeaders("localhost", "printf 'foo=bar\\r\\nbaz=qux'", logger)).resolves.toStrictEqual({ + foo: "bar", + baz: "qux", + }) + await expect(getHeaders("localhost", "printf 'foo=bar\\r\\n'", logger)).resolves.toStrictEqual({ foo: "bar" }) + await expect(getHeaders("localhost", "printf 'foo=bar'", logger)).resolves.toStrictEqual({ foo: "bar" }) + await expect(getHeaders("localhost", "printf 'foo=bar='", logger)).resolves.toStrictEqual({ foo: "bar=" }) + await expect(getHeaders("localhost", "printf 'foo=bar=baz'", logger)).resolves.toStrictEqual({ foo: "bar=baz" }) + await expect(getHeaders("localhost", "printf 'foo='", logger)).resolves.toStrictEqual({ foo: "" }) }) - await expect(getHeaders("localhost", "printf 'foo=bar\\r\\nbaz=qux'", logger)).resolves.toStrictEqual({ - foo: "bar", - baz: "qux", + + it("should error on malformed or empty lines", async () => { + await expect(getHeaders("localhost", "printf 'foo=bar\\r\\n\\r\\n'", logger)).rejects.toMatch(/Malformed/) + await expect(getHeaders("localhost", "printf '\\r\\nfoo=bar'", logger)).rejects.toMatch(/Malformed/) + await expect(getHeaders("localhost", "printf '=foo'", logger)).rejects.toMatch(/Malformed/) + await expect(getHeaders("localhost", "printf 'foo'", logger)).rejects.toMatch(/Malformed/) + await expect(getHeaders("localhost", "printf ' =foo'", logger)).rejects.toMatch(/Malformed/) + await expect(getHeaders("localhost", "printf 'foo =bar'", logger)).rejects.toMatch(/Malformed/) + await expect(getHeaders("localhost", "printf 'foo foo=bar'", logger)).rejects.toMatch(/Malformed/) + await expect(getHeaders("localhost", "printf ''", logger)).rejects.toMatch(/Malformed/) }) - await expect(getHeaders("localhost", "printf 'foo=bar\\r\\n'", logger)).resolves.toStrictEqual({ foo: "bar" }) - await expect(getHeaders("localhost", "printf 'foo=bar'", logger)).resolves.toStrictEqual({ foo: "bar" }) - await expect(getHeaders("localhost", "printf 'foo=bar='", logger)).resolves.toStrictEqual({ foo: "bar=" }) - await expect(getHeaders("localhost", "printf 'foo=bar=baz'", logger)).resolves.toStrictEqual({ foo: "bar=baz" }) - await expect(getHeaders("localhost", "printf 'foo='", logger)).resolves.toStrictEqual({ foo: "" }) -}) -it("should error on malformed or empty lines", async () => { - await expect(getHeaders("localhost", "printf 'foo=bar\\r\\n\\r\\n'", logger)).rejects.toMatch(/Malformed/) - await expect(getHeaders("localhost", "printf '\\r\\nfoo=bar'", logger)).rejects.toMatch(/Malformed/) - await expect(getHeaders("localhost", "printf '=foo'", logger)).rejects.toMatch(/Malformed/) - await expect(getHeaders("localhost", "printf 'foo'", logger)).rejects.toMatch(/Malformed/) - await expect(getHeaders("localhost", "printf ' =foo'", logger)).rejects.toMatch(/Malformed/) - await expect(getHeaders("localhost", "printf 'foo =bar'", logger)).rejects.toMatch(/Malformed/) - await expect(getHeaders("localhost", "printf 'foo foo=bar'", logger)).rejects.toMatch(/Malformed/) - await expect(getHeaders("localhost", "printf ''", logger)).rejects.toMatch(/Malformed/) -}) + it("should have access to environment variables", async () => { + const coderUrl = "dev.coder.com" + await expect( + getHeaders(coderUrl, os.platform() === "win32" ? "printf url=%CODER_URL%" : "printf url=$CODER_URL", logger), + ).resolves.toStrictEqual({ url: coderUrl }) + }) -it("should have access to environment variables", async () => { - const coderUrl = "dev.coder.com" - await expect( - getHeaders(coderUrl, os.platform() === "win32" ? "printf url=%CODER_URL%" : "printf url=$CODER_URL", logger), - ).resolves.toStrictEqual({ url: coderUrl }) + it("should error on non-zero exit", async () => { + await expect(getHeaders("localhost", "exit 10", logger)).rejects.toMatch(/exited unexpectedly with code 10/) + }) }) -it("should error on non-zero exit", async () => { - await expect(getHeaders("localhost", "exit 10", logger)).rejects.toMatch(/exited unexpectedly with code 10/) +describe("getHeaderCommand", () => { + beforeEach(() => { + vi.stubEnv("CODER_HEADER_COMMAND", "") + }) + + afterEach(() => { + vi.unstubAllEnvs() + }) + + it("should return undefined if coder.headerCommand is not set in config", () => { + const config = { + get: () => undefined, + } as unknown as WorkspaceConfiguration + + expect(getHeaderCommand(config)).toBeUndefined() + }) + + it("should return coder.headerCommand if set in config", () => { + vi.stubEnv("CODER_HEADER_COMMAND", "printf 'x=y'") + + const config = { + get: () => "printf 'foo=bar'", + } as unknown as WorkspaceConfiguration + + expect(getHeaderCommand(config)).toBe("printf 'foo=bar'") + }) + + it("should return CODER_HEADER_COMMAND if coder.headerCommand is not set in config and CODER_HEADER_COMMAND is set in environment", () => { + vi.stubEnv("CODER_HEADER_COMMAND", "printf 'x=y'") + + const config = { + get: () => undefined, + } as unknown as WorkspaceConfiguration + + expect(getHeaderCommand(config)).toBe("printf 'x=y'") + + delete process.env.CODER_HEADER_COMMAND + }) }) diff --git a/src/headers.ts b/src/headers.ts index f9da6168..259ad4a4 100644 --- a/src/headers.ts +++ b/src/headers.ts @@ -1,6 +1,8 @@ import * as cp from "child_process" import * as util from "util" +import { WorkspaceConfiguration } from "vscode" + export interface Logger { writeToCoderOutputChannel(message: string): void } @@ -15,6 +17,14 @@ function isExecException(err: unknown): err is ExecException { return typeof (err as ExecException).code !== "undefined" } +export function getHeaderCommand(config: WorkspaceConfiguration): string | undefined { + const cmd = config.get("coder.headerCommand") || process.env.CODER_HEADER_COMMAND + if (!cmd || typeof cmd !== "string") { + return undefined + } + return cmd +} + // TODO: getHeaders might make more sense to directly implement on Storage // but it is difficult to test Storage right now since we use vitest instead of // the standard extension testing framework which would give us access to vscode diff --git a/src/remote.ts b/src/remote.ts index 633f0f23..3e072dd9 100644 --- a/src/remote.ts +++ b/src/remote.ts @@ -20,6 +20,7 @@ import prettyBytes from "pretty-bytes" import * as semver from "semver" import * as vscode from "vscode" import * as ws from "ws" +import { getHeaderCommand } from "./headers" import { SSHConfig, SSHValues, defaultSSHConfigResponse, mergeSSHConfigValues } from "./sshConfig" import { computeSSHProperties, sshSupportsSetEnv } from "./sshSupport" import { Storage } from "./storage" @@ -537,8 +538,7 @@ export class Remote { // Add headers from the header command. let headerArg = "" - const headerCommand = - vscode.workspace.getConfiguration().get("coder.headerCommand") || process.env.CODER_HEADER_COMMAND + const headerCommand = getHeaderCommand(vscode.workspace.getConfiguration()) if (typeof headerCommand === "string" && headerCommand.trim().length > 0) { headerArg = ` --header-command ${escape(headerCommand)}` } diff --git a/src/storage.ts b/src/storage.ts index 9d86673d..8506464f 100644 --- a/src/storage.ts +++ b/src/storage.ts @@ -11,7 +11,7 @@ import os from "os" import path from "path" import prettyBytes from "pretty-bytes" import * as vscode from "vscode" -import { getHeaders } from "./headers" +import { getHeaderCommand, getHeaders } from "./headers" export class Storage { public workspace?: Workspace @@ -397,11 +397,7 @@ export class Storage { } public async getHeaders(url = this.getURL()): Promise> { - return getHeaders( - url, - vscode.workspace.getConfiguration().get("coder.headerCommand") || process.env.CODER_HEADER_COMMAND, - this, - ) + return getHeaders(url, getHeaderCommand(vscode.workspace.getConfiguration()), this) } } From 06c4364a4096dbf59e1197949d1c6489f3a1cfc8 Mon Sep 17 00:00:00 2001 From: Josh Vawdrey Date: Fri, 3 Nov 2023 17:07:38 +1100 Subject: [PATCH 5/8] chore: add tests --- src/headers.test.ts | 84 ++++++++++++++++++++++----------------------- 1 file changed, 41 insertions(+), 43 deletions(-) diff --git a/src/headers.test.ts b/src/headers.test.ts index 8397eeec..3cf21866 100644 --- a/src/headers.test.ts +++ b/src/headers.test.ts @@ -9,54 +9,52 @@ const logger = { }, } -describe("getHeaders", () => { - it("should return no headers", async () => { - await expect(getHeaders(undefined, undefined, logger)).resolves.toStrictEqual({}) - await expect(getHeaders("localhost", undefined, logger)).resolves.toStrictEqual({}) - await expect(getHeaders(undefined, "command", logger)).resolves.toStrictEqual({}) - await expect(getHeaders("localhost", "", logger)).resolves.toStrictEqual({}) - await expect(getHeaders("", "command", logger)).resolves.toStrictEqual({}) - await expect(getHeaders("localhost", " ", logger)).resolves.toStrictEqual({}) - await expect(getHeaders(" ", "command", logger)).resolves.toStrictEqual({}) - }) +it("should return no headers", async () => { + await expect(getHeaders(undefined, undefined, logger)).resolves.toStrictEqual({}) + await expect(getHeaders("localhost", undefined, logger)).resolves.toStrictEqual({}) + await expect(getHeaders(undefined, "command", logger)).resolves.toStrictEqual({}) + await expect(getHeaders("localhost", "", logger)).resolves.toStrictEqual({}) + await expect(getHeaders("", "command", logger)).resolves.toStrictEqual({}) + await expect(getHeaders("localhost", " ", logger)).resolves.toStrictEqual({}) + await expect(getHeaders(" ", "command", logger)).resolves.toStrictEqual({}) +}) - it("should return headers", async () => { - await expect(getHeaders("localhost", "printf 'foo=bar\\nbaz=qux'", logger)).resolves.toStrictEqual({ - foo: "bar", - baz: "qux", - }) - await expect(getHeaders("localhost", "printf 'foo=bar\\r\\nbaz=qux'", logger)).resolves.toStrictEqual({ - foo: "bar", - baz: "qux", - }) - await expect(getHeaders("localhost", "printf 'foo=bar\\r\\n'", logger)).resolves.toStrictEqual({ foo: "bar" }) - await expect(getHeaders("localhost", "printf 'foo=bar'", logger)).resolves.toStrictEqual({ foo: "bar" }) - await expect(getHeaders("localhost", "printf 'foo=bar='", logger)).resolves.toStrictEqual({ foo: "bar=" }) - await expect(getHeaders("localhost", "printf 'foo=bar=baz'", logger)).resolves.toStrictEqual({ foo: "bar=baz" }) - await expect(getHeaders("localhost", "printf 'foo='", logger)).resolves.toStrictEqual({ foo: "" }) +it("should return headers", async () => { + await expect(getHeaders("localhost", "printf 'foo=bar\\nbaz=qux'", logger)).resolves.toStrictEqual({ + foo: "bar", + baz: "qux", }) - - it("should error on malformed or empty lines", async () => { - await expect(getHeaders("localhost", "printf 'foo=bar\\r\\n\\r\\n'", logger)).rejects.toMatch(/Malformed/) - await expect(getHeaders("localhost", "printf '\\r\\nfoo=bar'", logger)).rejects.toMatch(/Malformed/) - await expect(getHeaders("localhost", "printf '=foo'", logger)).rejects.toMatch(/Malformed/) - await expect(getHeaders("localhost", "printf 'foo'", logger)).rejects.toMatch(/Malformed/) - await expect(getHeaders("localhost", "printf ' =foo'", logger)).rejects.toMatch(/Malformed/) - await expect(getHeaders("localhost", "printf 'foo =bar'", logger)).rejects.toMatch(/Malformed/) - await expect(getHeaders("localhost", "printf 'foo foo=bar'", logger)).rejects.toMatch(/Malformed/) - await expect(getHeaders("localhost", "printf ''", logger)).rejects.toMatch(/Malformed/) + await expect(getHeaders("localhost", "printf 'foo=bar\\r\\nbaz=qux'", logger)).resolves.toStrictEqual({ + foo: "bar", + baz: "qux", }) + await expect(getHeaders("localhost", "printf 'foo=bar\\r\\n'", logger)).resolves.toStrictEqual({ foo: "bar" }) + await expect(getHeaders("localhost", "printf 'foo=bar'", logger)).resolves.toStrictEqual({ foo: "bar" }) + await expect(getHeaders("localhost", "printf 'foo=bar='", logger)).resolves.toStrictEqual({ foo: "bar=" }) + await expect(getHeaders("localhost", "printf 'foo=bar=baz'", logger)).resolves.toStrictEqual({ foo: "bar=baz" }) + await expect(getHeaders("localhost", "printf 'foo='", logger)).resolves.toStrictEqual({ foo: "" }) +}) - it("should have access to environment variables", async () => { - const coderUrl = "dev.coder.com" - await expect( - getHeaders(coderUrl, os.platform() === "win32" ? "printf url=%CODER_URL%" : "printf url=$CODER_URL", logger), - ).resolves.toStrictEqual({ url: coderUrl }) - }) +it("should error on malformed or empty lines", async () => { + await expect(getHeaders("localhost", "printf 'foo=bar\\r\\n\\r\\n'", logger)).rejects.toMatch(/Malformed/) + await expect(getHeaders("localhost", "printf '\\r\\nfoo=bar'", logger)).rejects.toMatch(/Malformed/) + await expect(getHeaders("localhost", "printf '=foo'", logger)).rejects.toMatch(/Malformed/) + await expect(getHeaders("localhost", "printf 'foo'", logger)).rejects.toMatch(/Malformed/) + await expect(getHeaders("localhost", "printf ' =foo'", logger)).rejects.toMatch(/Malformed/) + await expect(getHeaders("localhost", "printf 'foo =bar'", logger)).rejects.toMatch(/Malformed/) + await expect(getHeaders("localhost", "printf 'foo foo=bar'", logger)).rejects.toMatch(/Malformed/) + await expect(getHeaders("localhost", "printf ''", logger)).rejects.toMatch(/Malformed/) +}) - it("should error on non-zero exit", async () => { - await expect(getHeaders("localhost", "exit 10", logger)).rejects.toMatch(/exited unexpectedly with code 10/) - }) +it("should have access to environment variables", async () => { + const coderUrl = "dev.coder.com" + await expect( + getHeaders(coderUrl, os.platform() === "win32" ? "printf url=%CODER_URL%" : "printf url=$CODER_URL", logger), + ).resolves.toStrictEqual({ url: coderUrl }) +}) + +it("should error on non-zero exit", async () => { + await expect(getHeaders("localhost", "exit 10", logger)).rejects.toMatch(/exited unexpectedly with code 10/) }) describe("getHeaderCommand", () => { From 60c2c485c644e86a3353d4d2d5c4cb90b1492a06 Mon Sep 17 00:00:00 2001 From: Josh Vawdrey Date: Fri, 3 Nov 2023 17:09:52 +1100 Subject: [PATCH 6/8] commit --- src/headers.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/headers.test.ts b/src/headers.test.ts index 3cf21866..39b55fc3 100644 --- a/src/headers.test.ts +++ b/src/headers.test.ts @@ -74,6 +74,14 @@ describe("getHeaderCommand", () => { expect(getHeaderCommand(config)).toBeUndefined() }) + it("should return undefined if coder.headerCommand is not a string", () => { + const config = { + get: () => 1234, + } as unknown as WorkspaceConfiguration + + expect(getHeaderCommand(config)).toBeUndefined() + }) + it("should return coder.headerCommand if set in config", () => { vi.stubEnv("CODER_HEADER_COMMAND", "printf 'x=y'") From dd7e83c47b5dd72da21c9789b21f1b1d0b56bb7f Mon Sep 17 00:00:00 2001 From: Josh Vawdrey Date: Fri, 3 Nov 2023 18:37:45 +1100 Subject: [PATCH 7/8] Update headers.test.ts --- src/headers.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/headers.test.ts b/src/headers.test.ts index 39b55fc3..dba294d1 100644 --- a/src/headers.test.ts +++ b/src/headers.test.ts @@ -101,6 +101,5 @@ describe("getHeaderCommand", () => { expect(getHeaderCommand(config)).toBe("printf 'x=y'") - delete process.env.CODER_HEADER_COMMAND }) }) From 983eb7059a7c78f41d3c11a82ff1e28f11528754 Mon Sep 17 00:00:00 2001 From: Josh Vawdrey Date: Fri, 3 Nov 2023 21:45:28 +1100 Subject: [PATCH 8/8] Update headers.test.ts --- src/headers.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/headers.test.ts b/src/headers.test.ts index dba294d1..1fa5f759 100644 --- a/src/headers.test.ts +++ b/src/headers.test.ts @@ -100,6 +100,5 @@ describe("getHeaderCommand", () => { } as unknown as WorkspaceConfiguration expect(getHeaderCommand(config)).toBe("printf 'x=y'") - }) })