From 2770ea71d9043c18856a20ffe18a311ba5681b4e Mon Sep 17 00:00:00 2001 From: Benjamin Peinhardt Date: Thu, 25 Jul 2024 16:13:09 +0000 Subject: [PATCH 1/5] use cli version for featureset detection --- package.json | 3 ++- src/remote.ts | 24 +++++++++++++++++++++--- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 0d4c6b18..321a6659 100644 --- a/package.json +++ b/package.json @@ -304,5 +304,6 @@ "semver": "7.6.2", "trim": "0.0.3", "word-wrap": "1.2.5" - } + }, + "packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e" } diff --git a/src/remote.ts b/src/remote.ts index 415f1731..9cc2d714 100644 --- a/src/remote.ts +++ b/src/remote.ts @@ -12,6 +12,7 @@ import * as semver from "semver" import * as vscode from "vscode" import { makeCoderSdk, startWorkspace, waitForBuild } from "./api" import { extractAgents } from "./api-helper" +import * as cli from "./cliManager" import { Commands } from "./commands" import { getHeaderCommand } from "./headers" import { SSHConfig, SSHValues, mergeSSHConfigValues } from "./sshConfig" @@ -541,11 +542,28 @@ export class Remote { } } + /** + * Tries to get the install CLI version. If that fails, defaults + * to the remote coder version. For the most part these should be in sync. + */ + private async probableCoderVersion(binaryPath: string): Promise { + try { + const version = await cli.version(binaryPath) + const parsedVersion = semver.parse(version) + if (!parsedVersion) { + return this.coderVersion + } + return parsedVersion + } catch (e) { + return this.coderVersion + } + } + /** * Format's the --log-dir argument for the ProxyCommand */ - private async formatLogArg(): Promise { - if (!supportsCoderAgentLogDirFlag(this.coderVersion)) { + private async formatLogArg(binpath: string): Promise { + if (!supportsCoderAgentLogDirFlag(await this.probableCoderVersion(binpath))) { return "" } @@ -659,7 +677,7 @@ export class Remote { Host: label ? `${AuthorityPrefix}.${label}--*` : `${AuthorityPrefix}--*`, ProxyCommand: `${escape(binaryPath)}${headerArg} vscodessh --network-info-dir ${escape( this.storage.getNetworkInfoPath(), - )}${await this.formatLogArg()} --session-token-file ${escape(this.storage.getSessionTokenPath(label))} --url-file ${escape( + )}${await this.formatLogArg(binaryPath)} --session-token-file ${escape(this.storage.getSessionTokenPath(label))} --url-file ${escape( this.storage.getUrlPath(label), )} %h`, ConnectTimeout: "0", From da073b66aaf91f1ccf6416ef1b54a84432925e67 Mon Sep 17 00:00:00 2001 From: Benjamin Peinhardt Date: Fri, 26 Jul 2024 00:03:46 +0000 Subject: [PATCH 2/5] use featureset object --- src/featureSet.ts | 21 ++++++++++++ src/remote.ts | 86 ++++++++++++++++++++++------------------------- 2 files changed, 62 insertions(+), 45 deletions(-) create mode 100644 src/featureSet.ts diff --git a/src/featureSet.ts b/src/featureSet.ts new file mode 100644 index 00000000..d0284249 --- /dev/null +++ b/src/featureSet.ts @@ -0,0 +1,21 @@ +import * as semver from "semver" + +export type FeatureSet = { + coderExtension: boolean + proxyLogDirectory: boolean +} + +/** + * Builds and returns a FeatureSet object for a given coder version. + */ +export function featureSetForVersion(version: semver.SemVer | null): FeatureSet { + return { + coderExtension: + version?.major === 0 && version?.minor <= 14 && version?.patch < 1 && version?.prerelease.length === 0, + + // CLI versions before 2.3.3 don't support the --log-dir flag! + // If this check didn't exist, VS Code connections would fail on + // older versions because of an unknown CLI argument. + proxyLogDirectory: (version?.compare("2.3.3") || 0) > 0 || version?.prerelease[0] === "devel", + } +} diff --git a/src/remote.ts b/src/remote.ts index 9cc2d714..c2154c9e 100644 --- a/src/remote.ts +++ b/src/remote.ts @@ -14,12 +14,12 @@ import { makeCoderSdk, startWorkspace, waitForBuild } from "./api" import { extractAgents } from "./api-helper" import * as cli from "./cliManager" import { Commands } from "./commands" +import { featureSetForVersion, FeatureSet } from "./featureSet" import { getHeaderCommand } from "./headers" import { SSHConfig, SSHValues, mergeSSHConfigValues } from "./sshConfig" import { computeSSHProperties, sshSupportsSetEnv } from "./sshSupport" import { Storage } from "./storage" import { AuthorityPrefix, expandPath, parseRemoteAuthority } from "./util" -import { supportsCoderAgentLogDirFlag } from "./version" import { WorkspaceAction } from "./workspaceAction" export interface RemoteDetails extends vscode.Disposable { @@ -34,7 +34,6 @@ export class Remote { private readonly storage: Storage, private readonly commands: Commands, private readonly mode: vscode.ExtensionMode, - private coderVersion: semver.SemVer | null = null, ) {} private async confirmStart(workspaceName: string): Promise { @@ -195,16 +194,38 @@ export class Remote { // Store for use in commands. this.commands.workspaceRestClient = workspaceRestClient + let binaryPath: string | undefined + if (this.mode === vscode.ExtensionMode.Production) { + binaryPath = await this.storage.fetchBinary(workspaceRestClient, parts.label) + } else { + try { + // In development, try to use `/tmp/coder` as the binary path. + // This is useful for debugging with a custom bin! + binaryPath = path.join(os.tmpdir(), "coder") + await fs.stat(binaryPath) + } catch (ex) { + binaryPath = await this.storage.fetchBinary(workspaceRestClient, parts.label) + } + } + // First thing is to check the version. const buildInfo = await workspaceRestClient.getBuildInfo() - this.coderVersion = semver.parse(buildInfo.version) + + let version: semver.SemVer | null = null + try { + let v = await cli.version(binaryPath) + if (!v) { + v = buildInfo.version + } + version = semver.parse(v) + } catch (e) { + version = semver.parse(buildInfo.version) + } + + const featureSet = featureSetForVersion(version) + // Server versions before v0.14.1 don't support the vscodessh command! - if ( - this.coderVersion?.major === 0 && - this.coderVersion?.minor <= 14 && - this.coderVersion?.patch < 1 && - this.coderVersion?.prerelease.length === 0 - ) { + if (featureSet.coderExtension) { await this.vscodeProposed.window.showErrorMessage( "Incompatible Server", { @@ -502,7 +523,7 @@ export class Remote { // "Host not found". try { this.storage.writeToCoderOutputChannel("Updating SSH config...") - await this.updateSSHConfig(workspaceRestClient, parts.label, parts.host) + await this.updateSSHConfig(workspaceRestClient, parts.label, parts.host, binaryPath, featureSet) } catch (error) { this.storage.writeToCoderOutputChannel(`Failed to configure SSH: ${error}`) throw error @@ -542,28 +563,11 @@ export class Remote { } } - /** - * Tries to get the install CLI version. If that fails, defaults - * to the remote coder version. For the most part these should be in sync. - */ - private async probableCoderVersion(binaryPath: string): Promise { - try { - const version = await cli.version(binaryPath) - const parsedVersion = semver.parse(version) - if (!parsedVersion) { - return this.coderVersion - } - return parsedVersion - } catch (e) { - return this.coderVersion - } - } - /** * Format's the --log-dir argument for the ProxyCommand */ - private async formatLogArg(binpath: string): Promise { - if (!supportsCoderAgentLogDirFlag(await this.probableCoderVersion(binpath))) { + private async formatLogArg(featureSet: FeatureSet): Promise { + if (!featureSet.proxyLogDirectory) { return "" } @@ -581,7 +585,13 @@ export class Remote { // updateSSHConfig updates the SSH configuration with a wildcard that handles // all Coder entries. - private async updateSSHConfig(restClient: Api, label: string, hostName: string) { + private async updateSSHConfig( + restClient: Api, + label: string, + hostName: string, + binaryPath: string, + featureSet: FeatureSet, + ) { let deploymentSSHConfig = {} try { const deploymentConfig = await restClient.getDeploymentSSHConfig() @@ -642,20 +652,6 @@ export class Remote { const sshConfig = new SSHConfig(sshConfigFile) await sshConfig.load() - let binaryPath: string | undefined - if (this.mode === vscode.ExtensionMode.Production) { - binaryPath = await this.storage.fetchBinary(restClient, label) - } else { - try { - // In development, try to use `/tmp/coder` as the binary path. - // This is useful for debugging with a custom bin! - binaryPath = path.join(os.tmpdir(), "coder") - await fs.stat(binaryPath) - } catch (ex) { - binaryPath = await this.storage.fetchBinary(restClient, label) - } - } - const escape = (str: string): string => `"${str.replace(/"/g, '\\"')}"` // Escape a command line to be executed by the Coder binary, so ssh doesn't substitute variables. const escapeSubcommand: (str: string) => string = @@ -677,7 +673,7 @@ export class Remote { Host: label ? `${AuthorityPrefix}.${label}--*` : `${AuthorityPrefix}--*`, ProxyCommand: `${escape(binaryPath)}${headerArg} vscodessh --network-info-dir ${escape( this.storage.getNetworkInfoPath(), - )}${await this.formatLogArg(binaryPath)} --session-token-file ${escape(this.storage.getSessionTokenPath(label))} --url-file ${escape( + )}${await this.formatLogArg(featureSet)} --session-token-file ${escape(this.storage.getSessionTokenPath(label))} --url-file ${escape( this.storage.getUrlPath(label), )} %h`, ConnectTimeout: "0", From b273261a062c9b9682349f168a34a251aec6a1f5 Mon Sep 17 00:00:00 2001 From: Benjamin Peinhardt Date: Fri, 26 Jul 2024 00:31:47 +0000 Subject: [PATCH 3/5] migrate tests for new file + remove unneeded version module --- src/featureSet.test.ts | 14 ++++++++++++++ src/version.test.ts | 13 ------------- src/version.ts | 8 -------- 3 files changed, 14 insertions(+), 21 deletions(-) create mode 100644 src/featureSet.test.ts delete mode 100644 src/version.test.ts delete mode 100644 src/version.ts diff --git a/src/featureSet.test.ts b/src/featureSet.test.ts new file mode 100644 index 00000000..4fa594ce --- /dev/null +++ b/src/featureSet.test.ts @@ -0,0 +1,14 @@ +import * as semver from "semver" +import { describe, expect, it } from "vitest" +import { featureSetForVersion } from "./featureSet" + +describe("check version support", () => { + it("has logs", () => { + ;["v1.3.3+e491217", "v2.3.3+e491217"].forEach((v: string) => { + expect(featureSetForVersion(semver.parse(v)).proxyLogDirectory).toBeFalsy() + }) + ;["v2.3.4+e491217", "v5.3.4+e491217", "v5.0.4+e491217"].forEach((v: string) => { + expect(featureSetForVersion(semver.parse(v)).proxyLogDirectory).toBeTruthy() + }) + }) +}) diff --git a/src/version.test.ts b/src/version.test.ts deleted file mode 100644 index c9cc71e6..00000000 --- a/src/version.test.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { parse } from "semver" -import { describe, expect, it } from "vitest" -import { supportsCoderAgentLogDirFlag } from "./version" - -describe("check version support", () => { - it("has logs", () => { - expect(supportsCoderAgentLogDirFlag(parse("v1.3.3+e491217"))).toBeFalsy() - expect(supportsCoderAgentLogDirFlag(parse("v2.3.3+e491217"))).toBeFalsy() - expect(supportsCoderAgentLogDirFlag(parse("v2.3.4+e491217"))).toBeTruthy() - expect(supportsCoderAgentLogDirFlag(parse("v5.3.4+e491217"))).toBeTruthy() - expect(supportsCoderAgentLogDirFlag(parse("v5.0.4+e491217"))).toBeTruthy() - }) -}) diff --git a/src/version.ts b/src/version.ts deleted file mode 100644 index d4a2199b..00000000 --- a/src/version.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { SemVer } from "semver" - -// CLI versions before 2.3.3 don't support the --log-dir flag! -// If this check didn't exist, VS Code connections would fail on -// older versions because of an unknown CLI argument. -export const supportsCoderAgentLogDirFlag = (ver: SemVer | null): boolean => { - return (ver?.compare("2.3.3") || 0) > 0 || ver?.prerelease[0] === "devel" -} From 86067f84357ea7d6b01760f9952f8c75387856b4 Mon Sep 17 00:00:00 2001 From: Benjamin Peinhardt Date: Fri, 26 Jul 2024 00:42:17 +0000 Subject: [PATCH 4/5] fix featureset.CoderExtension to be sematically correct --- src/featureSet.ts | 8 ++++++-- src/remote.ts | 8 ++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/featureSet.ts b/src/featureSet.ts index d0284249..b3704635 100644 --- a/src/featureSet.ts +++ b/src/featureSet.ts @@ -10,8 +10,12 @@ export type FeatureSet = { */ export function featureSetForVersion(version: semver.SemVer | null): FeatureSet { return { - coderExtension: - version?.major === 0 && version?.minor <= 14 && version?.patch < 1 && version?.prerelease.length === 0, + coderExtension: !( + version?.major === 0 && + version?.minor <= 14 && + version?.patch < 1 && + version?.prerelease.length === 0 + ), // CLI versions before 2.3.3 don't support the --log-dir flag! // If this check didn't exist, VS Code connections would fail on diff --git a/src/remote.ts b/src/remote.ts index c2154c9e..bc5ebe14 100644 --- a/src/remote.ts +++ b/src/remote.ts @@ -213,11 +213,7 @@ export class Remote { let version: semver.SemVer | null = null try { - let v = await cli.version(binaryPath) - if (!v) { - v = buildInfo.version - } - version = semver.parse(v) + version = semver.parse(await cli.version(binaryPath)) } catch (e) { version = semver.parse(buildInfo.version) } @@ -225,7 +221,7 @@ export class Remote { const featureSet = featureSetForVersion(version) // Server versions before v0.14.1 don't support the vscodessh command! - if (featureSet.coderExtension) { + if (!featureSet.coderExtension) { await this.vscodeProposed.window.showErrorMessage( "Incompatible Server", { From 174cc5a4a2a695d6d04122a217152e59bee878fc Mon Sep 17 00:00:00 2001 From: Benjamin Peinhardt Date: Fri, 26 Jul 2024 17:06:37 +0000 Subject: [PATCH 5/5] rename coderExtension field on featureSet to vscodessh --- src/featureSet.ts | 4 ++-- src/remote.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/featureSet.ts b/src/featureSet.ts index b3704635..62ff0c2b 100644 --- a/src/featureSet.ts +++ b/src/featureSet.ts @@ -1,7 +1,7 @@ import * as semver from "semver" export type FeatureSet = { - coderExtension: boolean + vscodessh: boolean proxyLogDirectory: boolean } @@ -10,7 +10,7 @@ export type FeatureSet = { */ export function featureSetForVersion(version: semver.SemVer | null): FeatureSet { return { - coderExtension: !( + vscodessh: !( version?.major === 0 && version?.minor <= 14 && version?.patch < 1 && diff --git a/src/remote.ts b/src/remote.ts index bc5ebe14..07fb94bb 100644 --- a/src/remote.ts +++ b/src/remote.ts @@ -221,7 +221,7 @@ export class Remote { const featureSet = featureSetForVersion(version) // Server versions before v0.14.1 don't support the vscodessh command! - if (!featureSet.coderExtension) { + if (!featureSet.vscodessh) { await this.vscodeProposed.window.showErrorMessage( "Incompatible Server", {