Skip to content

Commit f2b04e7

Browse files
committed
Self-review
1 parent cdf59fd commit f2b04e7

File tree

5 files changed

+89
-77
lines changed

5 files changed

+89
-77
lines changed

src/commands.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import { maybeAskAgent, maybeAskUrl } from "./promptUtils";
2626
import {
2727
RECOMMENDED_SSH_SETTINGS,
2828
applySettingOverrides,
29-
} from "./remote/userSettings";
29+
} from "./remote/sshOverrides";
3030
import { getGlobalFlags, resolveCliAuth } from "./settings/cli";
3131
import { escapeCommandArg, toRemoteAuthority, toSafeHost } from "./util";
3232
import { vscodeProposed } from "./vscodeProposed";

src/remote/remote.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,13 @@ import {
5858
parseCoderSshOptions,
5959
parseSshConfig,
6060
} from "./sshConfig";
61-
import { SshProcessMonitor } from "./sshProcess";
62-
import { computeSshProperties, sshSupportsSetEnv } from "./sshSupport";
6361
import {
6462
applySettingOverrides,
6563
buildSshOverrides,
6664
isActiveRemoteCommand,
67-
} from "./userSettings";
65+
} from "./sshOverrides";
66+
import { SshProcessMonitor } from "./sshProcess";
67+
import { computeSshProperties, sshSupportsSetEnv } from "./sshSupport";
6868
import { WorkspaceStateMachine } from "./workspaceStateMachine";
6969

7070
export interface RemoteDetails extends vscode.Disposable {
@@ -485,7 +485,9 @@ export class Remote {
485485

486486
const remoteCommand = computedSshProperties.RemoteCommand;
487487
if (isActiveRemoteCommand(remoteCommand)) {
488-
this.logger.info("RemoteCommand detected in SSH config");
488+
this.logger.info(
489+
"RemoteCommand detected, skipping remotePlatform override",
490+
);
489491
}
490492

491493
this.logger.info("Modifying settings...");
@@ -755,7 +757,7 @@ export class Remote {
755757
logDir: string,
756758
featureSet: FeatureSet,
757759
cliAuth: CliAuth,
758-
) {
760+
): Promise<Record<string, string>> {
759761
let deploymentSSHConfig = {};
760762
try {
761763
const deploymentConfig = await restClient.getDeploymentSSHConfig();
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ export function buildSshOverrides(
7575
config: Pick<WorkspaceConfiguration, "get">,
7676
sshHost: string,
7777
agentOS: string,
78-
remoteCommand?: string,
78+
remoteCommand: string | undefined,
7979
): SettingOverride[] {
8080
const overrides: SettingOverride[] = [];
8181

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
applySettingOverrides,
77
buildSshOverrides,
88
isActiveRemoteCommand,
9-
} from "@/remote/userSettings";
9+
} from "@/remote/sshOverrides";
1010

1111
import {
1212
MockConfigurationProvider,
@@ -54,7 +54,7 @@ describe("buildSshOverrides", () => {
5454
config.set("remote.SSH.remotePlatform", { "other-host": "darwin" });
5555
expect(
5656
findOverride(
57-
buildSshOverrides(config, "new-host", "linux"),
57+
buildSshOverrides(config, "new-host", "linux", undefined),
5858
"remote.SSH.remotePlatform",
5959
),
6060
).toEqual({ "other-host": "darwin", "new-host": "linux" });
@@ -63,7 +63,7 @@ describe("buildSshOverrides", () => {
6363
config.set("remote.SSH.remotePlatform", { "my-host": "windows" });
6464
expect(
6565
findOverride(
66-
buildSshOverrides(config, "my-host", "linux"),
66+
buildSshOverrides(config, "my-host", "linux", undefined),
6767
"remote.SSH.remotePlatform",
6868
),
6969
).toEqual({ "my-host": "linux" });
@@ -74,7 +74,7 @@ describe("buildSshOverrides", () => {
7474
config.set("remote.SSH.remotePlatform", { "my-host": "linux" });
7575
expect(
7676
findOverride(
77-
buildSshOverrides(config, "my-host", "linux"),
77+
buildSshOverrides(config, "my-host", "linux", undefined),
7878
"remote.SSH.remotePlatform",
7979
),
8080
).toBeUndefined();
@@ -150,7 +150,7 @@ describe("buildSshOverrides", () => {
150150
}
151151
expect(
152152
findOverride(
153-
buildSshOverrides(config, "host", "linux"),
153+
buildSshOverrides(config, "host", "linux", undefined),
154154
"remote.SSH.connectTimeout",
155155
),
156156
).toBe(1800);
@@ -164,7 +164,7 @@ describe("buildSshOverrides", () => {
164164
config.set("remote.SSH.connectTimeout", timeout);
165165
expect(
166166
findOverride(
167-
buildSshOverrides(config, "host", "linux"),
167+
buildSshOverrides(config, "host", "linux", undefined),
168168
"remote.SSH.connectTimeout",
169169
),
170170
).toBeUndefined();
@@ -175,7 +175,12 @@ describe("buildSshOverrides", () => {
175175
it("defaults to 8 hours when not configured", () => {
176176
expect(
177177
findOverride(
178-
buildSshOverrides(new MockConfigurationProvider(), "host", "linux"),
178+
buildSshOverrides(
179+
new MockConfigurationProvider(),
180+
"host",
181+
"linux",
182+
undefined,
183+
),
179184
"remote.SSH.reconnectionGraceTime",
180185
),
181186
).toBe(28800);
@@ -186,7 +191,7 @@ describe("buildSshOverrides", () => {
186191
config.set("remote.SSH.reconnectionGraceTime", 3600);
187192
expect(
188193
findOverride(
189-
buildSshOverrides(config, "host", "linux"),
194+
buildSshOverrides(config, "host", "linux", undefined),
190195
"remote.SSH.reconnectionGraceTime",
191196
),
192197
).toBeUndefined();
@@ -201,6 +206,7 @@ describe("buildSshOverrides", () => {
201206
new MockConfigurationProvider(),
202207
"host",
203208
"linux",
209+
undefined,
204210
);
205211
expect(findOverride(overrides, key)).toBe(expected);
206212
});
@@ -212,7 +218,9 @@ describe("buildSshOverrides", () => {
212218
config.set("remote.SSH.reconnectionGraceTime", 7200);
213219
config.set("remote.SSH.serverShutdownTimeout", 600);
214220
config.set("remote.SSH.maxReconnectionAttempts", 4);
215-
expect(buildSshOverrides(config, "my-host", "linux")).toHaveLength(0);
221+
expect(
222+
buildSshOverrides(config, "my-host", "linux", undefined),
223+
).toHaveLength(0);
216224
});
217225
});
218226

Lines changed: 63 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { it, expect } from "vitest";
1+
import { describe, it, expect } from "vitest";
22

33
import {
44
computeSshProperties,
@@ -26,10 +26,11 @@ it("current shell supports ssh", () => {
2626
expect(sshSupportsSetEnv()).toBeTruthy();
2727
});
2828

29-
it("computes the config for a host", () => {
30-
const properties = computeSshProperties(
31-
"coder-vscode--testing",
32-
`Host *
29+
describe("computeSshProperties", () => {
30+
it("computes the config for a host", () => {
31+
const properties = computeSshProperties(
32+
"coder-vscode--testing",
33+
`Host *
3334
StrictHostKeyChecking yes
3435
3536
# --- START CODER VSCODE ---
@@ -39,19 +40,19 @@ Host coder-vscode--*
3940
ProxyCommand=/tmp/coder --header="X-FOO=bar" coder.dev
4041
# --- END CODER VSCODE ---
4142
`,
42-
);
43+
);
4344

44-
expect(properties).toEqual({
45-
Another: "true",
46-
StrictHostKeyChecking: "yes",
47-
ProxyCommand: '/tmp/coder --header="X-FOO=bar" coder.dev',
45+
expect(properties).toEqual({
46+
Another: "true",
47+
StrictHostKeyChecking: "yes",
48+
ProxyCommand: '/tmp/coder --header="X-FOO=bar" coder.dev',
49+
});
4850
});
49-
});
5051

51-
it("handles ? wildcards", () => {
52-
const properties = computeSshProperties(
53-
"coder-vscode--testing",
54-
`Host *
52+
it("handles ? wildcards", () => {
53+
const properties = computeSshProperties(
54+
"coder-vscode--testing",
55+
`Host *
5556
StrictHostKeyChecking yes
5657
5758
Host i-???????? i-?????????????????
@@ -67,19 +68,19 @@ Host coder-v?code--*
6768
ProxyCommand=/tmp/coder --header="X-BAR=foo" coder.dev
6869
# --- END CODER VSCODE ---
6970
`,
70-
);
71+
);
7172

72-
expect(properties).toEqual({
73-
Another: "true",
74-
StrictHostKeyChecking: "yes",
75-
ProxyCommand: '/tmp/coder --header="X-BAR=foo" coder.dev',
73+
expect(properties).toEqual({
74+
Another: "true",
75+
StrictHostKeyChecking: "yes",
76+
ProxyCommand: '/tmp/coder --header="X-BAR=foo" coder.dev',
77+
});
7678
});
77-
});
7879

79-
it("picks up RemoteCommand from a user Host block alongside a Coder block", () => {
80-
const props = computeSshProperties(
81-
"coder-vscode.example.com--user--ws",
82-
`# --- START CODER VSCODE example.com ---
80+
it("picks up RemoteCommand from a user Host block alongside a Coder block", () => {
81+
const props = computeSshProperties(
82+
"coder-vscode.example.com--user--ws",
83+
`# --- START CODER VSCODE example.com ---
8384
Host coder-vscode.example.com--*
8485
ProxyCommand /path/to/coder ssh --stdio %h
8586
StrictHostKeyChecking no
@@ -89,48 +90,48 @@ Host coder-vscode.example.com--*
8990
RequestTTY yes
9091
RemoteCommand exec /bin/bash -l
9192
`,
92-
);
93-
expect(props.RemoteCommand).toBe("exec /bin/bash -l");
94-
expect(props.ProxyCommand).toBe("/path/to/coder ssh --stdio %h");
95-
});
93+
);
94+
expect(props.RemoteCommand).toBe("exec /bin/bash -l");
95+
expect(props.ProxyCommand).toBe("/path/to/coder ssh --stdio %h");
96+
});
9697

97-
it("returns RemoteCommand none literally", () => {
98-
const props = computeSshProperties(
99-
"coder-vscode.example.com--user--ws",
100-
`Host coder-vscode.example.com--*
98+
it("returns RemoteCommand none literally", () => {
99+
const props = computeSshProperties(
100+
"coder-vscode.example.com--user--ws",
101+
`Host coder-vscode.example.com--*
101102
RemoteCommand none
102103
`,
103-
);
104-
expect(props.RemoteCommand).toBe("none");
105-
});
104+
);
105+
expect(props.RemoteCommand).toBe("none");
106+
});
106107

107-
it("inherits RemoteCommand from a Host * block", () => {
108-
const props = computeSshProperties(
109-
"coder-vscode.example.com--user--ws",
110-
`Host *
108+
it("inherits RemoteCommand from a Host * block", () => {
109+
const props = computeSshProperties(
110+
"coder-vscode.example.com--user--ws",
111+
`Host *
111112
RemoteCommand exec /bin/zsh -l
112113
113114
Host coder-vscode.example.com--*
114115
ProxyCommand /path/to/coder ssh --stdio %h
115116
`,
116-
);
117-
expect(props.RemoteCommand).toBe("exec /bin/zsh -l");
118-
});
117+
);
118+
expect(props.RemoteCommand).toBe("exec /bin/zsh -l");
119+
});
119120

120-
it("handles RemoteCommand with = delimiter", () => {
121-
const props = computeSshProperties(
122-
"coder-vscode.example.com--user--ws",
123-
`Host coder-vscode.example.com--*
121+
it("handles RemoteCommand with = delimiter", () => {
122+
const props = computeSshProperties(
123+
"coder-vscode.example.com--user--ws",
124+
`Host coder-vscode.example.com--*
124125
RemoteCommand=exec /bin/bash -l
125126
`,
126-
);
127-
expect(props.RemoteCommand).toBe("exec /bin/bash -l");
128-
});
127+
);
128+
expect(props.RemoteCommand).toBe("exec /bin/bash -l");
129+
});
129130

130-
it("properly escapes meaningful regex characters", () => {
131-
const properties = computeSshProperties(
132-
"coder-vscode.dev.coder.com--matalfi--dogfood",
133-
`Host *
131+
it("properly escapes meaningful regex characters", () => {
132+
const properties = computeSshProperties(
133+
"coder-vscode.dev.coder.com--matalfi--dogfood",
134+
`Host *
134135
StrictHostKeyChecking yes
135136
136137
# ------------START-CODER-----------
@@ -153,12 +154,13 @@ Host coder-vscode.dev.coder.com--*
153154
# --- END CODER VSCODE dev.coder.com ---%
154155
155156
`,
156-
);
157-
158-
expect(properties).toEqual({
159-
StrictHostKeyChecking: "yes",
160-
ProxyCommand:
161-
'"/Users/matifali/Library/Application Support/Code/User/globalStorage/coder.coder-remote/dev.coder.com/bin/coder-darwin-arm64" vscodessh --network-info-dir "/Users/matifali/Library/Application Support/Code/User/globalStorage/coder.coder-remote/net" --session-token-file "/Users/matifali/Library/Application Support/Code/User/globalStorage/coder.coder-remote/dev.coder.com/session" --url-file "/Users/matifali/Library/Application Support/Code/User/globalStorage/coder.coder-remote/dev.coder.com/url" %h',
162-
UserKnownHostsFile: "/dev/null",
157+
);
158+
159+
expect(properties).toEqual({
160+
StrictHostKeyChecking: "yes",
161+
ProxyCommand:
162+
'"/Users/matifali/Library/Application Support/Code/User/globalStorage/coder.coder-remote/dev.coder.com/bin/coder-darwin-arm64" vscodessh --network-info-dir "/Users/matifali/Library/Application Support/Code/User/globalStorage/coder.coder-remote/net" --session-token-file "/Users/matifali/Library/Application Support/Code/User/globalStorage/coder.coder-remote/dev.coder.com/session" --url-file "/Users/matifali/Library/Application Support/Code/User/globalStorage/coder.coder-remote/dev.coder.com/url" %h',
163+
UserKnownHostsFile: "/dev/null",
164+
});
163165
});
164166
});

0 commit comments

Comments
 (0)