From a6a3c57b4fc9e852eaa4cd9c899d269de857654e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 29 Mar 2023 10:42:25 -0500 Subject: [PATCH 1/9] feat: Add extension settings for customizing behavior --- package.json | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index 1553bdff..6ffb9e02 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,21 @@ ], "main": "./dist/extension.js", "contributes": { + "configuration": { + "title": "Coder", + "properties": { + "coder.sshConfig": { + "markdownDescription": "These values will be included in the ssh config file. Eg: `'ConnectTimeout=10'` will set the timeout to 10 seconds. Any values included here will override anything provided by default or by the deployment. To unset a value that is written by default, set the value to the empty string, Eg: `'ConnectTimeout='` will unset it.", + "type": "array", + "items": { + "title": "SSH Config Value", + "type": "string" + }, + "scope": "machine", + "default": [] + } + } + }, "viewsContainers": { "activitybar": [ { @@ -140,4 +155,4 @@ "ws": "^8.11.0", "yaml": "^1.10.0" } -} +} \ No newline at end of file From 8c4e6f47e17d761bc227920c94b4b582fb9c8755 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 29 Mar 2023 11:19:06 -0500 Subject: [PATCH 2/9] Make settings override the deployment config --- package.json | 3 +- src/remote.ts | 31 ++++++++++++--- src/sshConfig.test.ts | 19 ++++----- src/sshConfig.ts | 90 +++++++++++++++++++++++-------------------- 4 files changed, 84 insertions(+), 59 deletions(-) diff --git a/package.json b/package.json index 6ffb9e02..972f3693 100644 --- a/package.json +++ b/package.json @@ -37,7 +37,8 @@ "type": "array", "items": { "title": "SSH Config Value", - "type": "string" + "type": "string", + "pattern": "^[a-zA-Z0-9-]+[=\\s].*$" }, "scope": "machine", "default": [] diff --git a/src/remote.ts b/src/remote.ts index 8c11c1aa..f2510628 100644 --- a/src/remote.ts +++ b/src/remote.ts @@ -19,7 +19,7 @@ import prettyBytes from "pretty-bytes" import * as semver from "semver" import * as vscode from "vscode" import * as ws from "ws" -import { SSHConfig, defaultSSHConfigResponse } from "./sshConfig" +import { SSHConfig, defaultSSHConfigResponse, mergeSSHConfigValues } from "./sshConfig" import { Storage } from "./storage" export class Remote { @@ -441,9 +441,10 @@ export class Remote { // updateSSHConfig updates the SSH configuration with a wildcard that handles // all Coder entries. private async updateSSHConfig() { - let deploymentConfig: SSHConfigResponse = defaultSSHConfigResponse + let deploymentSSHConfig = defaultSSHConfigResponse try { - deploymentConfig = await getDeploymentSSHConfig() + const deploymentConfig = await getDeploymentSSHConfig() + deploymentSSHConfig = deploymentConfig.ssh_config_options } catch (error) { if (!axios.isAxiosError(error)) { throw error @@ -452,7 +453,6 @@ export class Remote { case 404: { // Deployment does not support overriding ssh config yet. Likely an // older version, just use the default. - deploymentConfig = defaultSSHConfigResponse break } case 401: { @@ -464,6 +464,27 @@ export class Remote { } } + // deploymentConfig is now set from the remote coderd deployment. + // Now override with the user's config. + const userConfigSSH = vscode.workspace.getConfiguration("coder").get("sshConfig") || [] + // Parse the user's config into a Record. + const userConfig = userConfigSSH.reduce((acc, line) => { + let i = line.indexOf("=") + if (i === -1) { + i = line.indexOf(" ") + if (i === -1) { + // This line is malformed. The setting is incorrect, and does not match + // the pattern regex in the settings schema. + return acc + } + } + const key = line.slice(0, i) + const value = line.slice(i + 1) + acc[key] = value + return acc + }, {} as Record) + const sshConfigOverrides = mergeSSHConfigValues(deploymentSSHConfig, userConfig) + let sshConfigFile = vscode.workspace.getConfiguration().get("remote.SSH.configFile") if (!sshConfigFile) { sshConfigFile = path.join(os.homedir(), ".ssh", "config") @@ -504,7 +525,7 @@ export class Remote { SetEnv: "CODER_SSH_SESSION_TYPE=vscode", } - await sshConfig.update(sshValues, deploymentConfig) + await sshConfig.update(sshValues, sshConfigOverrides) } // showNetworkUpdates finds the SSH process ID that is being used by this diff --git a/src/sshConfig.test.ts b/src/sshConfig.test.ts index 2c20d520..bfeaa749 100644 --- a/src/sshConfig.test.ts +++ b/src/sshConfig.test.ts @@ -182,17 +182,14 @@ it("override values", async () => { LogLevel: "ERROR", }, { - ssh_config_options: { - loglevel: "DEBUG", // This tests case insensitive - ConnectTimeout: "500", - ExtraKey: "ExtraValue", - Foo: "bar", - Buzz: "baz", - // Remove this key - StrictHostKeyChecking: "", - ExtraRemove: "", - }, - hostname_prefix: "", + loglevel: "DEBUG", // This tests case insensitive + ConnectTimeout: "500", + ExtraKey: "ExtraValue", + Foo: "bar", + Buzz: "baz", + // Remove this key + StrictHostKeyChecking: "", + ExtraRemove: "", }, ) diff --git a/src/sshConfig.ts b/src/sshConfig.ts index 21c5a2e7..b83916af 100644 --- a/src/sshConfig.ts +++ b/src/sshConfig.ts @@ -31,10 +31,47 @@ const defaultFileSystem: FileSystem = { writeFile, } -export const defaultSSHConfigResponse: SSHConfigResponse = { - ssh_config_options: {}, - // The prefix is not used by the vscode-extension - hostname_prefix: "coder.", +export const defaultSSHConfigResponse: Record = {} + +// mergeSSHConfigValues will take a given ssh config and merge it with the overrides +// provided. The merge handles key case insensitivity, so casing in the "key" does +// not matter. +export function mergeSSHConfigValues( + config: Record, + overrides: Record, +): Record { + const merged: Record = {} + + // We need to do a case insensitive match for the overrides as ssh config keys are case insensitive. + // To get the correct key:value, use: + // key = caseInsensitiveOverrides[key.toLowerCase()] + // value = overrides[key] + const caseInsensitiveOverrides: Record = {} + Object.keys(overrides).forEach((key) => { + caseInsensitiveOverrides[key.toLowerCase()] = key + }) + + Object.keys(config).forEach((key) => { + const lower = key.toLowerCase() + // If the key is in overrides, use the override value. + if (caseInsensitiveOverrides[lower]) { + const correctCaseKey = caseInsensitiveOverrides[lower] + const value = overrides[correctCaseKey] + + // If the value is empty, do not add the key. It is being removed. + if (value === "") { + return + } + merged[correctCaseKey] = value + return + } + // If no override, take the original value. + if (config[key] !== "") { + merged[key] = config[key] + } + }) + + return merged } export class SSHConfig { @@ -58,7 +95,7 @@ export class SSHConfig { } } - async update(values: SSHValues, overrides: SSHConfigResponse = defaultSSHConfigResponse) { + async update(values: SSHValues, overrides: Record = defaultSSHConfigResponse) { // We should remove this in March 2023 because there is not going to have // old configs this.cleanUpOldConfig() @@ -66,7 +103,7 @@ export class SSHConfig { if (block) { this.eraseBlock(block) } - this.appendBlock(values, overrides.ssh_config_options) + this.appendBlock(values, overrides) await this.save() } @@ -122,44 +159,13 @@ export class SSHConfig { */ private appendBlock({ Host, ...otherValues }: SSHValues, overrides: Record) { const lines = [this.startBlockComment, `Host ${Host}`] - // We need to do a case insensitive match for the overrides as ssh config keys are case insensitive. - // To get the correct key:value, use: - // key = caseInsensitiveOverrides[key.toLowerCase()] - // value = overrides[key] - const caseInsensitiveOverrides: Record = {} - Object.keys(overrides).forEach((key) => { - caseInsensitiveOverrides[key.toLowerCase()] = key - }) - const keys = Object.keys(otherValues) as Array + // configValues is the merged values of the defaults and the overrides. + const configValues = mergeSSHConfigValues(otherValues, overrides) + + const keys = Object.keys(configValues) as Array keys.forEach((key) => { - const lower = key.toLowerCase() - if (caseInsensitiveOverrides[lower]) { - const correctCaseKey = caseInsensitiveOverrides[lower] - const value = overrides[correctCaseKey] - // Remove the key from the overrides so we don't write it again. - delete caseInsensitiveOverrides[lower] - if (value === "") { - // If the value is empty, don't write it. Prevent writing the default - // value as well. - return - } - // If the key is in overrides, use the override value. - // Doing it this way maintains the default order of the keys. - lines.push(this.withIndentation(`${key} ${value}`)) - return - } - lines.push(this.withIndentation(`${key} ${otherValues[key]}`)) - }) - // Write remaining overrides that have not been written yet. Sort to maintain deterministic order. - const remainingKeys = (Object.keys(caseInsensitiveOverrides) as Array).sort() - remainingKeys.forEach((key) => { - const correctKey = caseInsensitiveOverrides[key] - const value = overrides[correctKey] - // Only write the value if it is not empty. - if (value !== "") { - lines.push(this.withIndentation(`${correctKey} ${value}`)) - } + lines.push(this.withIndentation(`${key} ${configValues[key]}`)) }) lines.push(this.endBlockComment) From dc1fc31344f10e5dbf145ba0685acd65af245b38 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 29 Mar 2023 12:02:02 -0500 Subject: [PATCH 3/9] Include remaining overrides --- src/sshConfig.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/sshConfig.ts b/src/sshConfig.ts index b83916af..dc33de21 100644 --- a/src/sshConfig.ts +++ b/src/sshConfig.ts @@ -57,6 +57,7 @@ export function mergeSSHConfigValues( if (caseInsensitiveOverrides[lower]) { const correctCaseKey = caseInsensitiveOverrides[lower] const value = overrides[correctCaseKey] + delete caseInsensitiveOverrides[lower] // If the value is empty, do not add the key. It is being removed. if (value === "") { @@ -71,6 +72,12 @@ export function mergeSSHConfigValues( } }) + // Add remaining overrides. + Object.keys(caseInsensitiveOverrides).forEach((lower) => { + const correctCaseKey = caseInsensitiveOverrides[lower] + merged[correctCaseKey] = overrides[correctCaseKey] + }) + return merged } From 3c3d33adf24205f6eb80014893766d2a42368146 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 29 Mar 2023 12:02:55 -0500 Subject: [PATCH 4/9] fixup! Include remaining overrides --- src/sshConfig.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/sshConfig.ts b/src/sshConfig.ts index dc33de21..9b94c4ac 100644 --- a/src/sshConfig.ts +++ b/src/sshConfig.ts @@ -75,7 +75,9 @@ export function mergeSSHConfigValues( // Add remaining overrides. Object.keys(caseInsensitiveOverrides).forEach((lower) => { const correctCaseKey = caseInsensitiveOverrides[lower] - merged[correctCaseKey] = overrides[correctCaseKey] + if (overrides[correctCaseKey] !== "") { + merged[correctCaseKey] = overrides[correctCaseKey] + } }) return merged From 70fabed938e8e83dc0ebe29a799f0f9a3b61ec97 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 29 Mar 2023 12:46:23 -0500 Subject: [PATCH 5/9] fixup! fixup! Include remaining overrides --- src/sshConfig.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/sshConfig.ts b/src/sshConfig.ts index 9b94c4ac..28992fc4 100644 --- a/src/sshConfig.ts +++ b/src/sshConfig.ts @@ -75,9 +75,7 @@ export function mergeSSHConfigValues( // Add remaining overrides. Object.keys(caseInsensitiveOverrides).forEach((lower) => { const correctCaseKey = caseInsensitiveOverrides[lower] - if (overrides[correctCaseKey] !== "") { - merged[correctCaseKey] = overrides[correctCaseKey] - } + merged[correctCaseKey] = overrides[correctCaseKey] }) return merged @@ -174,7 +172,10 @@ export class SSHConfig { const keys = Object.keys(configValues) as Array keys.forEach((key) => { - lines.push(this.withIndentation(`${key} ${configValues[key]}`)) + const value = configValues[key] + if (value !== "") { + lines.push(this.withIndentation(`${key} ${value}`)) + } }) lines.push(this.endBlockComment) From 773d548674bb397dd14d7934f617894abd43ae77 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 29 Mar 2023 16:02:35 -0500 Subject: [PATCH 6/9] Sort final keys --- src/remote.ts | 2 +- src/sshConfig.test.ts | 2 +- src/sshConfig.ts | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/remote.ts b/src/remote.ts index f2510628..d4c8cd4c 100644 --- a/src/remote.ts +++ b/src/remote.ts @@ -8,7 +8,7 @@ import { startWorkspace, getDeploymentSSHConfig, } from "coder/site/src/api/api" -import { ProvisionerJobLog, SSHConfigResponse, Workspace, WorkspaceAgent } from "coder/site/src/api/typesGenerated" +import { ProvisionerJobLog, Workspace, WorkspaceAgent } from "coder/site/src/api/typesGenerated" import EventSource from "eventsource" import find from "find-process" import * as fs from "fs/promises" diff --git a/src/sshConfig.test.ts b/src/sshConfig.test.ts index bfeaa749..2b10bdab 100644 --- a/src/sshConfig.test.ts +++ b/src/sshConfig.test.ts @@ -198,7 +198,7 @@ Host coder-vscode--* ProxyCommand some-command-here ConnectTimeout 500 UserKnownHostsFile /dev/null - LogLevel DEBUG + loglevel DEBUG Buzz baz ExtraKey ExtraValue Foo bar diff --git a/src/sshConfig.ts b/src/sshConfig.ts index 28992fc4..63b55f51 100644 --- a/src/sshConfig.ts +++ b/src/sshConfig.ts @@ -170,7 +170,8 @@ export class SSHConfig { // configValues is the merged values of the defaults and the overrides. const configValues = mergeSSHConfigValues(otherValues, overrides) - const keys = Object.keys(configValues) as Array + // keys is the sorted keys of the merged values. + const keys = (Object.keys(configValues) as Array).sort() keys.forEach((key) => { const value = configValues[key] if (value !== "") { From dc45cb17771e89641de4f42dfbe6b317b1892fa7 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 29 Mar 2023 16:03:47 -0500 Subject: [PATCH 7/9] Sort final keys in test --- src/sshConfig.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/sshConfig.test.ts b/src/sshConfig.test.ts index 2b10bdab..97a3e1cb 100644 --- a/src/sshConfig.test.ts +++ b/src/sshConfig.test.ts @@ -195,13 +195,13 @@ it("override values", async () => { const expectedOutput = `# --- START CODER VSCODE --- Host coder-vscode--* - ProxyCommand some-command-here - ConnectTimeout 500 - UserKnownHostsFile /dev/null - loglevel DEBUG Buzz baz + ConnectTimeout 500 ExtraKey ExtraValue Foo bar + LogLevel DEBUG + ProxyCommand some-command-here + UserKnownHostsFile /dev/null # --- END CODER VSCODE ---` expect(mockFileSystem.readFile).toBeCalledWith(sshFilePath, expect.anything()) From c31600c89fd60c2e1e3b7645baf361395585a926 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 29 Mar 2023 16:12:59 -0500 Subject: [PATCH 8/9] Sorting --- src/sshConfig.test.ts | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/sshConfig.test.ts b/src/sshConfig.test.ts index 97a3e1cb..44de50c2 100644 --- a/src/sshConfig.test.ts +++ b/src/sshConfig.test.ts @@ -30,11 +30,11 @@ it("creates a new file and adds the config", async () => { const expectedOutput = `# --- START CODER VSCODE --- Host coder-vscode--* - ProxyCommand some-command-here ConnectTimeout 0 + LogLevel ERROR + ProxyCommand some-command-here StrictHostKeyChecking no UserKnownHostsFile /dev/null - LogLevel ERROR # --- END CODER VSCODE ---` expect(mockFileSystem.readFile).toBeCalledWith(sshFilePath, expect.anything()) @@ -43,12 +43,12 @@ Host coder-vscode--* it("adds a new coder config in an existent SSH configuration", async () => { const existentSSHConfig = `Host coder.something - HostName coder.something ConnectTimeout=0 - StrictHostKeyChecking=no - UserKnownHostsFile=/dev/null LogLevel ERROR - ProxyCommand command` + HostName coder.something + ProxyCommand command + StrictHostKeyChecking=no + UserKnownHostsFile=/dev/null` mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig) const sshConfig = new SSHConfig(sshFilePath, mockFileSystem) @@ -66,11 +66,11 @@ it("adds a new coder config in an existent SSH configuration", async () => { # --- START CODER VSCODE --- Host coder-vscode--* - ProxyCommand some-command-here ConnectTimeout 0 + LogLevel ERROR + ProxyCommand some-command-here StrictHostKeyChecking no UserKnownHostsFile /dev/null - LogLevel ERROR # --- END CODER VSCODE ---` expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath, expectedOutput, { @@ -90,11 +90,11 @@ it("updates an existent coder config", async () => { # --- START CODER VSCODE --- Host coder-vscode--* - ProxyCommand some-command-here ConnectTimeout 0 + LogLevel ERROR + ProxyCommand some-command-here StrictHostKeyChecking no UserKnownHostsFile /dev/null - LogLevel ERROR # --- END CODER VSCODE ---` mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig) @@ -119,11 +119,11 @@ Host coder-vscode--* # --- START CODER VSCODE --- Host coder--updated--vscode--* - ProxyCommand some-command-here ConnectTimeout 0 + LogLevel ERROR + ProxyCommand some-command-here StrictHostKeyChecking no UserKnownHostsFile /dev/null - LogLevel ERROR # --- END CODER VSCODE ---` expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath, expectedOutput, { @@ -134,12 +134,12 @@ Host coder--updated--vscode--* it("removes old coder SSH config and adds the new one", async () => { const existentSSHConfig = `Host coder-vscode--* - HostName coder.something ConnectTimeout=0 - StrictHostKeyChecking=no - UserKnownHostsFile=/dev/null + HostName coder.something LogLevel ERROR - ProxyCommand command` + ProxyCommand command + StrictHostKeyChecking=no + UserKnownHostsFile=/dev/null` mockFileSystem.readFile.mockResolvedValueOnce(existentSSHConfig) const sshConfig = new SSHConfig(sshFilePath, mockFileSystem) @@ -155,11 +155,11 @@ it("removes old coder SSH config and adds the new one", async () => { const expectedOutput = `# --- START CODER VSCODE --- Host coder-vscode--* - ProxyCommand some-command-here ConnectTimeout 0 + LogLevel ERROR + ProxyCommand some-command-here StrictHostKeyChecking no UserKnownHostsFile /dev/null - LogLevel ERROR # --- END CODER VSCODE ---` expect(mockFileSystem.writeFile).toBeCalledWith(sshFilePath, expectedOutput, { @@ -199,9 +199,9 @@ Host coder-vscode--* ConnectTimeout 500 ExtraKey ExtraValue Foo bar - LogLevel DEBUG ProxyCommand some-command-here UserKnownHostsFile /dev/null + logLevel DEBUG # --- END CODER VSCODE ---` expect(mockFileSystem.readFile).toBeCalledWith(sshFilePath, expect.anything()) From 4c8429e0ab0c4579a6c94db13d97a64d41a0090a Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 29 Mar 2023 16:13:48 -0500 Subject: [PATCH 9/9] fixup! Sorting --- src/sshConfig.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sshConfig.test.ts b/src/sshConfig.test.ts index 44de50c2..ff89c315 100644 --- a/src/sshConfig.test.ts +++ b/src/sshConfig.test.ts @@ -201,7 +201,7 @@ Host coder-vscode--* Foo bar ProxyCommand some-command-here UserKnownHostsFile /dev/null - logLevel DEBUG + loglevel DEBUG # --- END CODER VSCODE ---` expect(mockFileSystem.readFile).toBeCalledWith(sshFilePath, expect.anything())