Make log dir optional (#322)

Co-authored-by: Asher <[email protected]>

CHANGELOG.md

@@ -2,6 +2,13 @@
 
 ## Unreleased
 
+### Changed
+
+- Previously, the extension would always log SSH proxy diagnostics to a fixed
+  directory. Now this must be explicitly enabled by configuring a new setting
+  `coder.proxyLogDirectory`. If you are having connectivity issues, please
+  configure this setting and gather the logs before submitting an issue.
+
 ## [v1.3.1](https://github.com/coder/vscode-coder/releases/tag/v1.3.1) (2024-07-15)
 
 ### Fixed

README.md

@@ -26,3 +26,13 @@ ext install coder.coder-remote
 
 Alternatively, manually install the VSIX from the
 [latest release](https://github.com/coder/vscode-coder/releases/latest).
+
+#### Variables Reference
+
+Coder uses
+${userHome} from VS Code's
+[variables reference](https://code.visualstudio.com/docs/editor/variables-reference).
+Use this when formatting paths in the Coder extension settings rather than ~ or
+$HOME.
+
+Example: ${userHome}/foo/bar.baz

package.json

@@ -88,6 +88,11 @@
           "type": "string",
           "default": ""
         },
+        "coder.proxyLogDirectory": {
+          "markdownDescription": "If set, the Coder CLI will output extra SSH information into this directory, which can be helpful for debugging connectivity issues.",
+          "type": "string",
+          "default": ""
+        },
         "coder.proxyBypass": {
           "markdownDescription": "If not set, will inherit from the `no_proxy` or `NO_PROXY` environment variables. `http.proxySupport` must be set to `on` or `off`, otherwise VS Code will override the proxy agent set by the plugin.",
           "type": "string",

src/api.ts

@@ -1,20 +1,14 @@
 import { Api } from "coder/site/src/api/api"
 import { ProvisionerJobLog, Workspace } from "coder/site/src/api/typesGenerated"
 import fs from "fs/promises"
-import * as os from "os"
 import { ProxyAgent } from "proxy-agent"
 import * as vscode from "vscode"
 import * as ws from "ws"
 import { errToStr } from "./api-helper"
 import { CertificateError } from "./error"
 import { getProxyForUrl } from "./proxy"
 import { Storage } from "./storage"
-
-// expandPath will expand ${userHome} in the input string.
-function expandPath(input: string): string {
-  const userHome = os.homedir()
-  return input.replace(/\${userHome}/g, userHome)
-}
+import { expandPath } from "./util"
 
 async function createHttpAgent(): Promise<ProxyAgent> {
   const cfg = vscode.workspace.getConfiguration()

src/error.test.ts

@@ -52,7 +52,7 @@ async function startServer(certName: string): Promise<string> {
   disposers.push(() => server.close())
   return new Promise<string>((resolve, reject) => {
     server.on("error", reject)
-    server.listen(0, "localhost", () => {
+    server.listen(0, "127.0.0.1", () => {
       const address = server.address()
       if (!address) {
         throw new Error("Server has no address")

src/remote.ts

@@ -17,7 +17,7 @@ import { getHeaderCommand } from "./headers"
 import { SSHConfig, SSHValues, mergeSSHConfigValues } from "./sshConfig"
 import { computeSSHProperties, sshSupportsSetEnv } from "./sshSupport"
 import { Storage } from "./storage"
-import { AuthorityPrefix, parseRemoteAuthority } from "./util"
+import { AuthorityPrefix, expandPath, parseRemoteAuthority } from "./util"
 import { supportsCoderAgentLogDirFlag } from "./version"
 import { WorkspaceAction } from "./workspaceAction"
 
@@ -33,6 +33,7 @@ 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<boolean> {
@@ -195,13 +196,13 @@ export class Remote {
 
     // First thing is to check the version.
     const buildInfo = await workspaceRestClient.getBuildInfo()
-    const parsedVersion = semver.parse(buildInfo.version)
+    this.coderVersion = semver.parse(buildInfo.version)
     // Server versions before v0.14.1 don't support the vscodessh command!
     if (
-      parsedVersion?.major === 0 &&
-      parsedVersion?.minor <= 14 &&
-      parsedVersion?.patch < 1 &&
-      parsedVersion?.prerelease.length === 0
+      this.coderVersion?.major === 0 &&
+      this.coderVersion?.minor <= 14 &&
+      this.coderVersion?.patch < 1 &&
+      this.coderVersion?.prerelease.length === 0
     ) {
       await this.vscodeProposed.window.showErrorMessage(
         "Incompatible Server",
@@ -215,7 +216,6 @@ export class Remote {
       await this.closeRemote()
       return
     }
-    const hasCoderLogs = supportsCoderAgentLogDirFlag(parsedVersion)
 
     // Next is to find the workspace from the URI scheme provided.
     let workspace: Workspace
@@ -501,7 +501,7 @@ export class Remote {
     // "Host not found".
     try {
       this.storage.writeToCoderOutputChannel("Updating SSH config...")
-      await this.updateSSHConfig(workspaceRestClient, parts.label, parts.host, hasCoderLogs)
+      await this.updateSSHConfig(workspaceRestClient, parts.label, parts.host)
     } catch (error) {
       this.storage.writeToCoderOutputChannel(`Failed to configure SSH: ${error}`)
       throw error
@@ -541,9 +541,29 @@ export class Remote {
     }
   }
 
+  /**
+   * Format's the --log-dir argument for the ProxyCommand
+   */
+  private async formatLogArg(): Promise<string> {
+    if (!supportsCoderAgentLogDirFlag(this.coderVersion)) {
+      return ""
+    }
+
+    // If the proxyLogDirectory is not set in the extension settings we don't send one.
+    // Question for Asher: How do VSCode extension settings behave in terms of semver for the extension?
+    const logDir = expandPath(String(vscode.workspace.getConfiguration().get("coder.proxyLogDirectory") ?? "").trim())
+    if (!logDir) {
+      return ""
+    }
+
+    await fs.mkdir(logDir, { recursive: true })
+    this.storage.writeToCoderOutputChannel(`SSH proxy diagnostics are being written to ${logDir}`)
+    return ` --log-dir ${escape(logDir)}`
+  }
+
   // updateSSHConfig updates the SSH configuration with a wildcard that handles
   // all Coder entries.
-  private async updateSSHConfig(restClient: Api, label: string, hostName: string, hasCoderLogs = false) {
+  private async updateSSHConfig(restClient: Api, label: string, hostName: string) {
     let deploymentSSHConfig = {}
     try {
       const deploymentConfig = await restClient.getDeploymentSSHConfig()
@@ -634,16 +654,12 @@ export class Remote {
     if (typeof headerCommand === "string" && headerCommand.trim().length > 0) {
       headerArg = ` --header-command ${escapeSubcommand(headerCommand)}`
     }
-    let logArg = ""
-    if (hasCoderLogs) {
-      await fs.mkdir(this.storage.getLogPath(), { recursive: true })
-      logArg = ` --log-dir ${escape(this.storage.getLogPath())}`
-    }
+
     const sshValues: SSHValues = {
       Host: label ? `${AuthorityPrefix}.${label}--*` : `${AuthorityPrefix}--*`,
       ProxyCommand: `${escape(binaryPath)}${headerArg} vscodessh --network-info-dir ${escape(
         this.storage.getNetworkInfoPath(),
-      )}${logArg} --session-token-file ${escape(this.storage.getSessionTokenPath(label))} --url-file ${escape(
+      )}${await this.formatLogArg()} --session-token-file ${escape(this.storage.getSessionTokenPath(label))} --url-file ${escape(
         this.storage.getUrlPath(label),
       )} %h`,
       ConnectTimeout: "0",

src/util.ts

@@ -1,3 +1,4 @@
+import * as os from "os"
 import url from "url"
 
 export interface AuthorityParts {
@@ -58,3 +59,13 @@ export function toSafeHost(rawUrl: string): string {
   // should already have thrown in that case.
   return url.domainToASCII(u.hostname) || u.hostname
 }
+
+/**
+ * Expand a path with ${userHome} in the input string
+ * @param input string
+ * @returns string
+ */
+export function expandPath(input: string): string {
+  const userHome = os.homedir()
+  return input.replace(/\${userHome}/g, userHome)
+}