Skip to content

Commit f45d00e

Browse files
authored
Merge branch 'master' into dependabot/npm_and_yarn/master/vscode-lsp-02ec881dff
2 parents a1c4b05 + 2e1a219 commit f45d00e

File tree

3 files changed

+69
-11
lines changed

3 files changed

+69
-11
lines changed

packages/core/src/awsService/sagemaker/commands.ts

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -191,32 +191,60 @@ export async function openRemoteConnect(
191191
void vscode.window.showErrorMessage(ConnectFromRemoteWorkspaceMessage)
192192
return
193193
}
194+
195+
const spaceName = node.spaceApp.SpaceName!
194196
await tryRefreshNode(node)
197+
198+
// for Stopped SM spaces - check instance type before showing progress
195199
if (node.getStatus() === 'Stopped') {
196200
// In case of SMUS, we pass in a SM Client and for SM AI, it creates a new SM Client.
197201
const client = sageMakerClient ? sageMakerClient : new SagemakerClient(node.regionCode)
198202

199203
try {
200-
await client.startSpace(node.spaceApp.SpaceName!, node.spaceApp.DomainId!)
204+
await client.startSpace(spaceName, node.spaceApp.DomainId!)
201205
await tryRefreshNode(node)
202206
const appType = node.spaceApp.SpaceSettingsSummary?.AppType
203207
if (!appType) {
204208
throw new ToolkitError('AppType is undefined for the selected space. Cannot start remote connection.', {
205209
code: 'undefinedAppType',
206210
})
207211
}
208-
await client.waitForAppInService(node.spaceApp.DomainId!, node.spaceApp.SpaceName!, appType)
209-
await tryRemoteConnection(node, ctx)
212+
213+
// Only start showing progress after instance type validation
214+
return await vscode.window.withProgress(
215+
{
216+
location: vscode.ProgressLocation.Notification,
217+
cancellable: false,
218+
title: `Connecting to ${spaceName}`,
219+
},
220+
async (progress) => {
221+
progress.report({ message: 'Starting the space.' })
222+
await client.waitForAppInService(node.spaceApp.DomainId!, spaceName, appType)
223+
await tryRemoteConnection(node, ctx, progress)
224+
}
225+
)
210226
} catch (err: any) {
211227
// Ignore InstanceTypeError since it means the user decided not to use an instanceType with more memory
212-
if (err.code !== InstanceTypeError) {
213-
throw new ToolkitError(`Remote connection failed: ${(err as Error).message}`, {
214-
cause: err as Error,
215-
code: err.code,
216-
})
228+
// just return without showing progress
229+
if (err.code === InstanceTypeError) {
230+
return
217231
}
232+
throw new ToolkitError(`Remote connection failed: ${(err as Error).message}`, {
233+
cause: err as Error,
234+
code: err.code,
235+
})
218236
}
219237
} else if (node.getStatus() === 'Running') {
220-
await tryRemoteConnection(node, ctx)
238+
// For running spaces, show progress
239+
return await vscode.window.withProgress(
240+
{
241+
location: vscode.ProgressLocation.Notification,
242+
cancellable: false,
243+
title: `Connecting to ${spaceName}`,
244+
},
245+
async (progress) => {
246+
await tryRemoteConnection(node, ctx, progress)
247+
}
248+
)
221249
}
222250
}

packages/core/src/awsService/sagemaker/model.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,14 @@ const logger = getLogger('sagemaker')
2727

2828
export async function tryRemoteConnection(
2929
node: SagemakerSpaceNode | SagemakerUnifiedStudioSpaceNode,
30-
ctx: vscode.ExtensionContext
30+
ctx: vscode.ExtensionContext,
31+
progress: vscode.Progress<{ message?: string; increment?: number }>
3132
) {
3233
const spaceArn = (await node.getSpaceArn()) as string
3334
const isSMUS = node instanceof SagemakerUnifiedStudioSpaceNode
3435
const remoteEnv = await prepareDevEnvConnection(spaceArn, ctx, 'sm_lc', isSMUS, node)
3536
try {
37+
progress.report({ message: 'Opening remote session' })
3638
await startVscodeRemote(
3739
remoteEnv.SessionProcess,
3840
remoteEnv.hostname,
@@ -226,7 +228,13 @@ export async function removeKnownHost(hostname: string): Promise<void> {
226228
throw ToolkitError.chain(err, 'Failed to read known_hosts file')
227229
}
228230

229-
const updatedLines = lines.filter((line) => !line.split(' ')[0].split(',').includes(hostname))
231+
const updatedLines = lines.filter((line) => {
232+
const entryHostname = line.split(' ')[0].split(',')
233+
// Hostnames in the known_hosts file seem to be always lowercase, but keeping the case-sensitive check just in
234+
// case. Originally we were only doing the case-sensitive check which caused users to get a host
235+
// identification error when reconnecting to a Space after it was restarted.
236+
return !entryHostname.includes(hostname) && !entryHostname.includes(hostname.toLowerCase())
237+
})
230238

231239
if (updatedLines.length !== lines.length) {
232240
try {

packages/core/src/test/awsService/sagemaker/model.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,28 @@ describe('SageMaker Model', () => {
211211
assertLogsContain(`Removed '${hostname}' from known_hosts`, false, 'debug')
212212
})
213213

214+
it('removes case-sensitive hostname when entry in known_hosts is lowercase', async function () {
215+
const mixedCaseHostname = 'Test.Host.Com'
216+
217+
sandbox.stub(fs, 'existsFile').resolves(true)
218+
219+
const inputContent = `test.host.com ssh-rsa AAAA\nsome.other.com ssh-rsa BBBB`
220+
const expectedOutput = `some.other.com ssh-rsa BBBB`
221+
222+
sandbox.stub(fs, 'readFileText').resolves(inputContent)
223+
const writeStub = sandbox.stub(fs, 'writeFile').resolves()
224+
225+
await removeKnownHost(mixedCaseHostname)
226+
227+
sinon.assert.calledWith(
228+
writeStub,
229+
path.join(os.homedir(), '.ssh', 'known_hosts'),
230+
sinon.match((value: string) => value.trim() === expectedOutput),
231+
{ atomic: true }
232+
)
233+
assertLogsContain(`Removed '${mixedCaseHostname}' from known_hosts`, false, 'debug')
234+
})
235+
214236
it('handles hostname in comma-separated list', async function () {
215237
sandbox.stub(fs, 'existsFile').resolves(true)
216238

0 commit comments

Comments
 (0)