From 7c2018c566c433798cb5c318e617e95650a5e35a Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 19 Feb 2025 10:48:57 -0500 Subject: [PATCH 1/6] test: add error code to test connection failure --- packages/core/src/awsService/ec2/model.ts | 8 ++++---- packages/core/src/shared/extensions/ssh.ts | 4 ++-- packages/core/src/test/shared/extensions/ssh.test.ts | 8 ++++---- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/core/src/awsService/ec2/model.ts b/packages/core/src/awsService/ec2/model.ts index 9a5e13feaef..8610ea2ce2e 100644 --- a/packages/core/src/awsService/ec2/model.ts +++ b/packages/core/src/awsService/ec2/model.ts @@ -26,7 +26,7 @@ import { SshError, startSshAgent, startVscodeRemote, - testSshConnection, + testSsmConnection, } from '../../shared/extensions/ssh' import { getLogger } from '../../shared/logger/logger' import { CancellationError, Timeout } from '../../shared/utilities/timeoutUtils' @@ -36,7 +36,7 @@ import { SshKeyPair } from './sshKeyPair' import { Ec2SessionTracker } from './remoteSessionManager' import { getEc2SsmEnv } from './utils' -export type Ec2ConnectErrorCode = 'EC2SSMStatus' | 'EC2SSMPermission' | 'EC2SSMConnect' | 'EC2SSMAgentStatus' +export type Ec2ConnectErrorCode = 'EC2SSMStatus' | 'EC2SSMPermission' | 'EC2SSMTestConnect' | 'EC2SSMAgentStatus' export interface Ec2RemoteEnv extends VscodeRemoteConnection { selection: Ec2Selection @@ -200,7 +200,7 @@ export class Ec2Connecter implements vscode.Disposable { const remoteEnv = await this.prepareEc2RemoteEnvWithProgress(selection, remoteUser) const testSession = await this.ssmClient.startSession(selection.instanceId, 'AWS-StartSSHSession') try { - await testSshConnection( + await testSsmConnection( remoteEnv.SessionProcess, remoteEnv.hostname, remoteEnv.sshPath, @@ -216,7 +216,7 @@ export class Ec2Connecter implements vscode.Disposable { ) } catch (err) { const message = err instanceof SshError ? 'Testing SSH connection to instance failed' : '' - this.throwConnectionError(message, selection, err as Error) + this.throwConnectionError(message, selection, { ...(err as Error), code: 'EC2SSMTestConnect' }) } finally { await this.ssmClient.terminateSession(testSession) } diff --git a/packages/core/src/shared/extensions/ssh.ts b/packages/core/src/shared/extensions/ssh.ts index ba349a902c2..d20d12cfaa8 100644 --- a/packages/core/src/shared/extensions/ssh.ts +++ b/packages/core/src/shared/extensions/ssh.ts @@ -130,7 +130,7 @@ export class RemoteSshSettings extends Settings.define('remote.SSH', remoteSshTy } } -export async function testSshConnection( +export async function testSsmConnection( ProcessClass: typeof ChildProcess, hostname: string, sshPath: string, @@ -150,7 +150,7 @@ export async function testSshConnection( }) return result } catch (error) { - throw new SshError('SSH connection test failed', { cause: error as Error }) + throw new SshError('SSH connection test failed', { cause: error as Error, code: 'SSMTestConnectionFailed' }) } } diff --git a/packages/core/src/test/shared/extensions/ssh.test.ts b/packages/core/src/test/shared/extensions/ssh.test.ts index 38874e2df68..d6737773a02 100644 --- a/packages/core/src/test/shared/extensions/ssh.test.ts +++ b/packages/core/src/test/shared/extensions/ssh.test.ts @@ -4,7 +4,7 @@ */ import * as assert from 'assert' import { ChildProcess } from '../../../shared/utilities/processUtils' -import { startSshAgent, testSshConnection } from '../../../shared/extensions/ssh' +import { startSshAgent, testSsmConnection } from '../../../shared/extensions/ssh' import { createBoundProcess } from '../../../shared/remoteSession' import { createExecutableFile, createTestWorkspaceFolder } from '../../testUtil' import { WorkspaceFolder } from 'vscode' @@ -74,7 +74,7 @@ describe('testSshConnection', function () { } as SSM.StartSessionResponse await createExecutableFile(sshPath, echoEnvVarsCmd(['MY_VAR'])) - const r = await testSshConnection(process, 'localhost', sshPath, 'test-user', session) + const r = await testSsmConnection(process, 'localhost', sshPath, 'test-user', session) assertOutputContains(r.stdout, 'yes') }) @@ -97,7 +97,7 @@ describe('testSshConnection', function () { const process = createBoundProcess(envProvider) await createExecutableFile(sshPath, echoEnvVarsCmd(['SESSION_ID', 'STREAM_URL', 'TOKEN'])) - const r = await testSshConnection(process, 'localhost', sshPath, 'test-user', newSession) + const r = await testSsmConnection(process, 'localhost', sshPath, 'test-user', newSession) assertOutputContains(r.stdout, `${newSession.SessionId} ${newSession.StreamUrl} ${newSession.TokenValue}`) }) @@ -105,7 +105,7 @@ describe('testSshConnection', function () { const executableFileContent = isWin() ? `echo "%1 %2"` : `echo "$1 $2"` const process = createBoundProcess(async () => ({})) await createExecutableFile(sshPath, executableFileContent) - const r = await testSshConnection(process, 'localhost', sshPath, 'test-user', {} as SSM.StartSessionResponse) + const r = await testSsmConnection(process, 'localhost', sshPath, 'test-user', {} as SSM.StartSessionResponse) assertOutputContains(r.stdout, '-T') assertOutputContains(r.stdout, 'test-user@localhost') }) From a59da214c0d156dea554653660445f556fc412e9 Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 19 Feb 2025 11:28:34 -0500 Subject: [PATCH 2/6] refactor: simplify test ssm connection --- packages/core/src/shared/extensions/ssh.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/core/src/shared/extensions/ssh.ts b/packages/core/src/shared/extensions/ssh.ts index d20d12cfaa8..b4907e446b6 100644 --- a/packages/core/src/shared/extensions/ssh.ts +++ b/packages/core/src/shared/extensions/ssh.ts @@ -137,20 +137,16 @@ export async function testSsmConnection( user: string, session: SSM.StartSessionResponse ): Promise { + const env = { SESSION_ID: session.SessionId, STREAM_URL: session.StreamUrl, TOKEN: session.TokenValue } + const process = new ProcessClass(sshPath, ['-T', `${user}@${hostname}`, 'echo "test connection succeeded" && exit']) try { - const env = { SESSION_ID: session.SessionId, STREAM_URL: session.StreamUrl, TOKEN: session.TokenValue } - const result = await new ProcessClass(sshPath, [ - '-T', - `${user}@${hostname}`, - 'echo "test connection succeeded" && exit', - ]).run({ + return await process.run({ spawnOptions: { env, }, }) - return result } catch (error) { - throw new SshError('SSH connection test failed', { cause: error as Error, code: 'SSMTestConnectionFailed' }) + throw new SshError('SSH connection test failed', { cause: error as Error }) } } From 88cfab939fe38474467fed53e8665303f83332ff Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 19 Feb 2025 11:57:37 -0500 Subject: [PATCH 3/6] feat: pipe error from spawned process into message --- packages/core/src/awsService/ec2/model.ts | 7 ++++--- packages/core/src/shared/extensions/ssh.ts | 5 +++-- packages/core/src/test/shared/extensions/ssh.test.ts | 8 ++++---- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/core/src/awsService/ec2/model.ts b/packages/core/src/awsService/ec2/model.ts index 8610ea2ce2e..df5aa5f4392 100644 --- a/packages/core/src/awsService/ec2/model.ts +++ b/packages/core/src/awsService/ec2/model.ts @@ -26,7 +26,7 @@ import { SshError, startSshAgent, startVscodeRemote, - testSsmConnection, + testSshConnection, } from '../../shared/extensions/ssh' import { getLogger } from '../../shared/logger/logger' import { CancellationError, Timeout } from '../../shared/utilities/timeoutUtils' @@ -200,7 +200,7 @@ export class Ec2Connecter implements vscode.Disposable { const remoteEnv = await this.prepareEc2RemoteEnvWithProgress(selection, remoteUser) const testSession = await this.ssmClient.startSession(selection.instanceId, 'AWS-StartSSHSession') try { - await testSsmConnection( + await testSshConnection( remoteEnv.SessionProcess, remoteEnv.hostname, remoteEnv.sshPath, @@ -215,7 +215,8 @@ export class Ec2Connecter implements vscode.Disposable { remoteUser.name ) } catch (err) { - const message = err instanceof SshError ? 'Testing SSH connection to instance failed' : '' + const message = + err instanceof SshError ? `Testing SSM connection to instance failed with error: ${err.message}` : '' this.throwConnectionError(message, selection, { ...(err as Error), code: 'EC2SSMTestConnect' }) } finally { await this.ssmClient.terminateSession(testSession) diff --git a/packages/core/src/shared/extensions/ssh.ts b/packages/core/src/shared/extensions/ssh.ts index b4907e446b6..a2376e7d668 100644 --- a/packages/core/src/shared/extensions/ssh.ts +++ b/packages/core/src/shared/extensions/ssh.ts @@ -130,7 +130,7 @@ export class RemoteSshSettings extends Settings.define('remote.SSH', remoteSshTy } } -export async function testSsmConnection( +export async function testSshConnection( ProcessClass: typeof ChildProcess, hostname: string, sshPath: string, @@ -146,7 +146,8 @@ export async function testSsmConnection( }, }) } catch (error) { - throw new SshError('SSH connection test failed', { cause: error as Error }) + const errorMessage = [process.result()?.stderr ?? ''].join('\n') + throw new SshError(errorMessage, { cause: error as Error }) } } diff --git a/packages/core/src/test/shared/extensions/ssh.test.ts b/packages/core/src/test/shared/extensions/ssh.test.ts index d6737773a02..38874e2df68 100644 --- a/packages/core/src/test/shared/extensions/ssh.test.ts +++ b/packages/core/src/test/shared/extensions/ssh.test.ts @@ -4,7 +4,7 @@ */ import * as assert from 'assert' import { ChildProcess } from '../../../shared/utilities/processUtils' -import { startSshAgent, testSsmConnection } from '../../../shared/extensions/ssh' +import { startSshAgent, testSshConnection } from '../../../shared/extensions/ssh' import { createBoundProcess } from '../../../shared/remoteSession' import { createExecutableFile, createTestWorkspaceFolder } from '../../testUtil' import { WorkspaceFolder } from 'vscode' @@ -74,7 +74,7 @@ describe('testSshConnection', function () { } as SSM.StartSessionResponse await createExecutableFile(sshPath, echoEnvVarsCmd(['MY_VAR'])) - const r = await testSsmConnection(process, 'localhost', sshPath, 'test-user', session) + const r = await testSshConnection(process, 'localhost', sshPath, 'test-user', session) assertOutputContains(r.stdout, 'yes') }) @@ -97,7 +97,7 @@ describe('testSshConnection', function () { const process = createBoundProcess(envProvider) await createExecutableFile(sshPath, echoEnvVarsCmd(['SESSION_ID', 'STREAM_URL', 'TOKEN'])) - const r = await testSsmConnection(process, 'localhost', sshPath, 'test-user', newSession) + const r = await testSshConnection(process, 'localhost', sshPath, 'test-user', newSession) assertOutputContains(r.stdout, `${newSession.SessionId} ${newSession.StreamUrl} ${newSession.TokenValue}`) }) @@ -105,7 +105,7 @@ describe('testSshConnection', function () { const executableFileContent = isWin() ? `echo "%1 %2"` : `echo "$1 $2"` const process = createBoundProcess(async () => ({})) await createExecutableFile(sshPath, executableFileContent) - const r = await testSsmConnection(process, 'localhost', sshPath, 'test-user', {} as SSM.StartSessionResponse) + const r = await testSshConnection(process, 'localhost', sshPath, 'test-user', {} as SSM.StartSessionResponse) assertOutputContains(r.stdout, '-T') assertOutputContains(r.stdout, 'test-user@localhost') }) From f9b88d6d5d98e4cb8b10ce7bc81404fa3a541b5b Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 19 Feb 2025 12:00:55 -0500 Subject: [PATCH 4/6] doc: add doc string and improve default message --- packages/core/src/shared/extensions/ssh.ts | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/core/src/shared/extensions/ssh.ts b/packages/core/src/shared/extensions/ssh.ts index a2376e7d668..44af313d108 100644 --- a/packages/core/src/shared/extensions/ssh.ts +++ b/packages/core/src/shared/extensions/ssh.ts @@ -130,6 +130,15 @@ export class RemoteSshSettings extends Settings.define('remote.SSH', remoteSshTy } } +/** + * Test a SSH connection over SSM. + * @param ProcessClass given process to test the connection within. + * @param hostname + * @param sshPath + * @param user + * @param session SSM session credentials. These cannot be reused, so it may be required to create a seperate session for the test connection. + * @returns + */ export async function testSshConnection( ProcessClass: typeof ChildProcess, hostname: string, @@ -146,8 +155,9 @@ export async function testSshConnection( }, }) } catch (error) { - const errorMessage = [process.result()?.stderr ?? ''].join('\n') - throw new SshError(errorMessage, { cause: error as Error }) + throw new SshError(process.result()?.stderr ?? 'An unknown error occurred when testing the connection', { + cause: error as Error, + }) } } From 49ba773ee125affe7056516fa66011819ed9796d Mon Sep 17 00:00:00 2001 From: Hweinstock <42325418+Hweinstock@users.noreply.github.com> Date: Wed, 19 Feb 2025 15:51:14 -0500 Subject: [PATCH 5/6] refactor: make message more concise. Co-authored-by: Justin M. Keyes --- packages/core/src/awsService/ec2/model.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/awsService/ec2/model.ts b/packages/core/src/awsService/ec2/model.ts index df5aa5f4392..2dd191a1b76 100644 --- a/packages/core/src/awsService/ec2/model.ts +++ b/packages/core/src/awsService/ec2/model.ts @@ -216,7 +216,7 @@ export class Ec2Connecter implements vscode.Disposable { ) } catch (err) { const message = - err instanceof SshError ? `Testing SSM connection to instance failed with error: ${err.message}` : '' + err instanceof SshError ? `Testing SSM connection to instance failed: ${err.message}` : '' this.throwConnectionError(message, selection, { ...(err as Error), code: 'EC2SSMTestConnect' }) } finally { await this.ssmClient.terminateSession(testSession) From 65881b1096559089f60d6a95b2f7febeaed71472 Mon Sep 17 00:00:00 2001 From: hkobew Date: Wed, 19 Feb 2025 16:08:53 -0500 Subject: [PATCH 6/6] fix: add space to avoid lint issue --- packages/core/src/awsService/ec2/model.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/core/src/awsService/ec2/model.ts b/packages/core/src/awsService/ec2/model.ts index 2dd191a1b76..42d69828aa7 100644 --- a/packages/core/src/awsService/ec2/model.ts +++ b/packages/core/src/awsService/ec2/model.ts @@ -215,8 +215,7 @@ export class Ec2Connecter implements vscode.Disposable { remoteUser.name ) } catch (err) { - const message = - err instanceof SshError ? `Testing SSM connection to instance failed: ${err.message}` : '' + const message = err instanceof SshError ? `Testing SSM connection to instance failed: ${err.message}` : '' this.throwConnectionError(message, selection, { ...(err as Error), code: 'EC2SSMTestConnect' }) } finally { await this.ssmClient.terminateSession(testSession)