Handle secretsManager comments part 2

Author: EhabY

src/commands.ts

@@ -79,7 +79,7 @@ export class Commands {
 	 */
 	public async login(args?: {
 		url?: string;
-		label?: string;
+		safeHostname?: string;
 		autoLogin?: boolean;
 	}): Promise<void> {
 		if (this.deploymentManager.isAuthenticated()) {
@@ -98,13 +98,13 @@ export class Commands {
 		}
 
 		// It is possible that we are trying to log into an old-style host, in which
-		// case we want to write with the provided blank label instead of generating
-		// a host label.
-		const label = args?.label ?? toSafeHost(url);
-		this.logger.info("Using deployment label", label);
+		// case we want to write with the provided blank hostname instead of
+		// generating one.
+		const safeHostname = args?.safeHostname ?? toSafeHost(url);
+		this.logger.info("Using hostname", safeHostname);
 
 		const result = await this.loginCoordinator.promptForLogin({
-			label,
+			safeHostname,
 			url,
 			autoLogin: args?.autoLogin,
 		});
@@ -115,7 +115,7 @@ export class Commands {
 
 		await this.deploymentManager.changeDeployment({
 			url,
-			label,
+			safeHostname,
 			token: result.token,
 			user: result.user,
 		});
@@ -307,13 +307,13 @@ export class Commands {
 
 					// If workspace_name is provided, run coder ssh before the command
 					const baseUrl = this.requireExtensionBaseUrl();
-					const label = toSafeHost(baseUrl);
+					const safeHost = toSafeHost(baseUrl);
 					const binary = await this.cliManager.fetchBinary(
 						this.extensionClient,
-						label,
+						safeHost,
 					);
 
-					const configDir = this.pathResolver.getGlobalConfigDir(label);
+					const configDir = this.pathResolver.getGlobalConfigDir(safeHost);
 					const globalFlags = getGlobalFlags(
 						vscode.workspace.getConfiguration(),
 						configDir,

src/core/cliManager.ts

@@ -33,16 +33,19 @@ export class CliManager {
 
 	/**
 	 * Download and return the path to a working binary for the deployment with
-	 * the provided label using the provided client.  If the label is empty, use
-	 * the old deployment-unaware path instead.
+	 * the provided hostname using the provided client.  If the hostname is empty,
+	 * use the old deployment-unaware path instead.
 	 *
 	 * If there is already a working binary and it matches the server version,
 	 * return that, skipping the download.  If it does not match but downloads are
 	 * disabled, return whatever we have and log a warning.  Otherwise throw if
 	 * unable to download a working binary, whether because of network issues or
 	 * downloads being disabled.
 	 */
-	public async fetchBinary(restClient: Api, label: string): Promise<string> {
+	public async fetchBinary(
+		restClient: Api,
+		safeHostname: string,
+	): Promise<string> {
 		const cfg = vscode.workspace.getConfiguration("coder");
 		// Settings can be undefined when set to their defaults (true in this case),
 		// so explicitly check against false.
@@ -64,7 +67,7 @@ export class CliManager {
 		// is valid and matches the server, or if it does not match the server but
 		// downloads are disabled, we can return early.
 		const binPath = path.join(
-			this.pathResolver.getBinaryCachePath(label),
+			this.pathResolver.getBinaryCachePath(safeHostname),
 			cliUtils.name(),
 		);
 		this.output.info("Using binary path", binPath);
@@ -693,48 +696,48 @@ export class CliManager {
 	}
 
 	/**
-	 * Configure the CLI for the deployment with the provided label.
+	 * Configure the CLI for the deployment with the provided hostname.
 	 *
 	 * Falsey URLs and null tokens are a no-op; we avoid unconfiguring the CLI to
 	 * avoid breaking existing connections.
 	 */
 	public async configure(
-		label: string,
+		safeHostname: string,
 		url: string | undefined,
 		token: string | null,
 	) {
 		await Promise.all([
-			this.updateUrlForCli(label, url),
-			this.updateTokenForCli(label, token),
+			this.updateUrlForCli(safeHostname, url),
+			this.updateTokenForCli(safeHostname, token),
 		]);
 	}
 
 	/**
-	 * Update the URL for the deployment with the provided label on disk which can
-	 * be used by the CLI via --url-file.  If the URL is falsey, do nothing.
+	 * Update the URL for the deployment with the provided hostname on disk which
+	 * can be used by the CLI via --url-file.  If the URL is falsey, do nothing.
 	 *
-	 * If the label is empty, read the old deployment-unaware config instead.
+	 * If the hostname is empty, read the old deployment-unaware config instead.
 	 */
 	private async updateUrlForCli(
-		label: string,
+		safeHostname: string,
 		url: string | undefined,
 	): Promise<void> {
 		if (url) {
-			const urlPath = this.pathResolver.getUrlPath(label);
+			const urlPath = this.pathResolver.getUrlPath(safeHostname);
 			await this.atomicWriteFile(urlPath, url);
 		}
 	}
 
 	/**
-	 * Update the session token for a deployment with the provided label on disk
-	 * which can be used by the CLI via --session-token-file.  If the token is
-	 * null, do nothing.
+	 * Update the session token for a deployment with the provided hostname on
+	 * disk which can be used by the CLI via --session-token-file.  If the token
+	 * is null, do nothing.
 	 *
-	 * If the label is empty, read the old deployment-unaware config instead.
+	 * If the hostname is empty, read the old deployment-unaware config instead.
 	 */
-	private async updateTokenForCli(label: string, token: string | null) {
+	private async updateTokenForCli(safeHostname: string, token: string | null) {
 		if (token !== null) {
-			const tokenPath = this.pathResolver.getSessionTokenPath(label);
+			const tokenPath = this.pathResolver.getSessionTokenPath(safeHostname);
 			await this.atomicWriteFile(tokenPath, token);
 		}
 	}

src/core/container.ts

@@ -35,6 +35,7 @@ export class ServiceContainer implements vscode.Disposable {
 		this.secretsManager = new SecretsManager(
 			context.secrets,
 			context.globalState,
+			this.logger,
 		);
 		this.cliManager = new CliManager(
 			this.vscodeProposed,

src/core/pathResolver.ts

@@ -1,4 +1,4 @@
-import * as path from "path";
+import * as path from "node:path";
 import * as vscode from "vscode";
 
 export class PathResolver {
@@ -8,26 +8,28 @@ export class PathResolver {
 	) {}
 
 	/**
-	 * Return the directory for the deployment with the provided label to where
-	 * the global Coder configs are stored.
+	 * Return the directory for the deployment with the provided hostname to
+	 * where the global Coder configs are stored.
 	 *
-	 * If the label is empty, read the old deployment-unaware config instead.
+	 * If the hostname is empty, read the old deployment-unaware config instead.
 	 *
 	 * The caller must ensure this directory exists before use.
 	 */
-	public getGlobalConfigDir(label: string): string {
-		return label ? path.join(this.basePath, label) : this.basePath;
+	public getGlobalConfigDir(safeHostname: string): string {
+		return safeHostname
+			? path.join(this.basePath, safeHostname)
+			: this.basePath;
 	}
 
 	/**
-	 * Return the directory for a deployment with the provided label to where its
-	 * binary is cached.
+	 * Return the directory for a deployment with the provided hostname to where
+	 * its binary is cached.
 	 *
-	 * If the label is empty, read the old deployment-unaware config instead.
+	 * If the hostname is empty, read the old deployment-unaware config instead.
 	 *
 	 * The caller must ensure this directory exists before use.
 	 */
-	public getBinaryCachePath(label: string): string {
+	public getBinaryCachePath(safeHostname: string): string {
 		const settingPath = vscode.workspace
 			.getConfiguration()
 			.get<string>("coder.binaryDestination")
@@ -36,7 +38,7 @@ export class PathResolver {
 			settingPath || process.env.CODER_BINARY_DESTINATION?.trim();
 		return binaryPath
 			? path.normalize(binaryPath)
-			: path.join(this.getGlobalConfigDir(label), "bin");
+			: path.join(this.getGlobalConfigDir(safeHostname), "bin");
 	}
 
 	/**
@@ -69,39 +71,39 @@ export class PathResolver {
 	}
 
 	/**
-	 * Return the directory for the deployment with the provided label to where
-	 * its session token is stored.
+	 * Return the directory for the deployment with the provided hostname to
+	 * where its session token is stored.
 	 *
-	 * If the label is empty, read the old deployment-unaware config instead.
+	 * If the hostname is empty, read the old deployment-unaware config instead.
 	 *
 	 * The caller must ensure this directory exists before use.
 	 */
-	public getSessionTokenPath(label: string): string {
-		return path.join(this.getGlobalConfigDir(label), "session");
+	public getSessionTokenPath(safeHostname: string): string {
+		return path.join(this.getGlobalConfigDir(safeHostname), "session");
 	}
 
 	/**
-	 * Return the directory for the deployment with the provided label to where
-	 * its session token was stored by older code.
+	 * Return the directory for the deployment with the provided hostname to
+	 * where its session token was stored by older code.
 	 *
-	 * If the label is empty, read the old deployment-unaware config instead.
+	 * If the hostname is empty, read the old deployment-unaware config instead.
 	 *
 	 * The caller must ensure this directory exists before use.
 	 */
-	public getLegacySessionTokenPath(label: string): string {
-		return path.join(this.getGlobalConfigDir(label), "session_token");
+	public getLegacySessionTokenPath(safeHostname: string): string {
+		return path.join(this.getGlobalConfigDir(safeHostname), "session_token");
 	}
 
 	/**
-	 * Return the directory for the deployment with the provided label to where
-	 * its url is stored.
+	 * Return the directory for the deployment with the provided hostname to
+	 * where its url is stored.
 	 *
-	 * If the label is empty, read the old deployment-unaware config instead.
+	 * If the hostname is empty, read the old deployment-unaware config instead.
 	 *
 	 * The caller must ensure this directory exists before use.
 	 */
-	public getUrlPath(label: string): string {
-		return path.join(this.getGlobalConfigDir(label), "url");
+	public getUrlPath(safeHostname: string): string {
+		return path.join(this.getGlobalConfigDir(safeHostname), "url");
 	}
 
 	/**