From 5376e1ae50afada4546aba5aa8e7ad562f8a4c54 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Mon, 24 Jul 2023 17:38:38 +0200 Subject: [PATCH 01/31] Add `generateAgentConfigPaths` function that creates config path based on `runtimeSettings` and `rootClusterUri`. Pass `rootClusterUri` instead of `profileName` `createAgentFile` --- .../mainProcess/createAgentConfigFile.test.ts | 50 ++++++++----------- .../src/mainProcess/createAgentConfigFile.ts | 45 ++++++++++++++--- .../teleterm/src/mainProcess/mainProcess.ts | 3 +- .../connectMyComputerService.ts | 6 +-- 4 files changed, 62 insertions(+), 42 deletions(-) diff --git a/web/packages/teleterm/src/mainProcess/createAgentConfigFile.test.ts b/web/packages/teleterm/src/mainProcess/createAgentConfigFile.test.ts index 19d5f2160ad22..8c35705cd5236 100644 --- a/web/packages/teleterm/src/mainProcess/createAgentConfigFile.test.ts +++ b/web/packages/teleterm/src/mainProcess/createAgentConfigFile.test.ts @@ -17,9 +17,13 @@ import childProcess from 'node:child_process'; import fs from 'node:fs/promises'; +import { RootClusterUri } from 'teleterm/ui/uri'; import { makeRuntimeSettings } from 'teleterm/mainProcess/fixtures/mocks'; -import { createAgentConfigFile } from './createAgentConfigFile'; +import { + createAgentConfigFile, + generateAgentConfigPaths, +} from './createAgentConfigFile'; jest.mock('node:child_process'); jest.mock('node:fs'); @@ -40,7 +44,7 @@ test('teleport configure is called with proper arguments', async () => { '/Users/test/Caches/Teleport Connect/teleport/teleport'; const token = '8f50fd5d-38e8-4e96-baea-e9b882bb433b'; const proxy = 'cluster.local:3080'; - const profileName = 'cluster.local'; + const rootClusterUri: RootClusterUri = '/clusters/cluster.local'; const labels = [ { name: 'teleport.dev/connect-my-computer/owner', @@ -61,7 +65,7 @@ test('teleport configure is called with proper arguments', async () => { { token, proxy, - profileName, + rootClusterUri, labels, } ) @@ -72,8 +76,8 @@ test('teleport configure is called with proper arguments', async () => { [ 'node', 'configure', - `--output=${userDataDir}/agents/${profileName}/config.yaml`, - `--data-dir=${userDataDir}/agents/${profileName}/data`, + `--output=${userDataDir}/agents/cluster.local/config.yaml`, + `--data-dir=${userDataDir}/agents/cluster.local/data`, `--proxy=${proxy}`, `--token=${token}`, `--labels=${labels[0].name}=${labels[0].value},${labels[1].name}=${labels[1].value}`, @@ -87,7 +91,7 @@ test('teleport configure is called with proper arguments', async () => { test('previous config file is removed before calling teleport configure', async () => { const userDataDir = '/Users/test/Application Data/Teleport Connect'; - const profileName = 'cluster.local'; + const rootClusterUri: RootClusterUri = '/clusters/cluster.local'; await expect( createAgentConfigFile( @@ -97,45 +101,35 @@ test('previous config file is removed before calling teleport configure', async { token: '', proxy: '', - profileName, + rootClusterUri, labels: [], } ) ).resolves.toBeUndefined(); expect(fs.rm).toHaveBeenCalledWith( - `${userDataDir}/agents/${profileName}/config.yaml` + `${userDataDir}/agents/cluster.local/config.yaml` ); }); -test('throws when profileName is not a valid path segment', async () => { - await expect( - createAgentConfigFile( +test('throws when rootClusterUri does not contain a valid path segment', () => { + expect(() => + generateAgentConfigPaths( makeRuntimeSettings({ userDataDir: '/Users/test/Application Data/Teleport Connect', }), - { - token: '', - proxy: '', - profileName: '/cluster', - labels: [], - } + '/clusters/../not_valid' ) - ).rejects.toThrow('The agent config file path is incorrect'); + ).toThrow('The agent config path is incorrect'); }); -test('throws when profileName is undefined', async () => { - await expect( - createAgentConfigFile( +test('throws when rootClusterUri is undefined', () => { + expect(() => + generateAgentConfigPaths( makeRuntimeSettings({ userDataDir: '/Users/test/Application Data/Teleport Connect', }), - { - token: '', - proxy: '', - profileName: undefined, - labels: [], - } + '/clusters/' ) - ).rejects.toThrow('The "path" argument must be of type string'); + ).toThrow('Incorrect root cluster URI'); }); diff --git a/web/packages/teleterm/src/mainProcess/createAgentConfigFile.ts b/web/packages/teleterm/src/mainProcess/createAgentConfigFile.ts index cc397c5c39f0c..4996fccac079e 100644 --- a/web/packages/teleterm/src/mainProcess/createAgentConfigFile.ts +++ b/web/packages/teleterm/src/mainProcess/createAgentConfigFile.ts @@ -19,12 +19,13 @@ import { execFile } from 'node:child_process'; import { rm } from 'node:fs/promises'; import path from 'node:path'; +import { RootClusterUri, routing } from 'teleterm/ui/uri'; import { RuntimeSettings } from 'teleterm/mainProcess/types'; import type * as tsh from 'teleterm/services/tshd/types'; export interface AgentConfigFileClusterProperties { - profileName: string; + rootClusterUri: RootClusterUri; proxy: string; token: string; labels: tsh.Label[]; @@ -35,14 +36,11 @@ export async function createAgentConfigFile( clusterProperties: AgentConfigFileClusterProperties ): Promise { const asyncExecFile = promisify(execFile); - const agentDirectory = getAgentDirectoryOrThrow( - runtimeSettings.userDataDir, - clusterProperties.profileName + const { configFile, dataDirectory } = generateAgentConfigPaths( + runtimeSettings, + clusterProperties.rootClusterUri ); - const configFile = path.resolve(agentDirectory, 'config.yaml'); - const dataDirectory = path.resolve(agentDirectory, 'data'); - // remove the config file if exists try { await rm(configFile); @@ -69,6 +67,37 @@ export async function createAgentConfigFile( ); } +/** + * Returns agent config paths. + * @param runtimeSettings must not come from the renderer process. + * Otherwise, the generated paths may point outside the user's data directory. + * @param rootClusterUri may be passed from the renderer process. + */ +export function generateAgentConfigPaths( + runtimeSettings: RuntimeSettings, + rootClusterUri: RootClusterUri +): { + configFile: string; + dataDirectory: string; +} { + const parsed = routing.parseClusterUri(rootClusterUri); + if (!parsed?.params?.rootClusterId) { + throw new Error(`Incorrect root cluster URI: ${rootClusterUri}`); + } + + const agentDirectory = getAgentDirectoryOrThrow( + runtimeSettings.userDataDir, + parsed.params.rootClusterId + ); + const configFile = path.resolve(agentDirectory, 'config.yaml'); + const dataDirectory = path.resolve(agentDirectory, 'data'); + + return { + configFile, + dataDirectory, + }; +} + function getAgentDirectoryOrThrow( userDataDir: string, profileName: string @@ -81,7 +110,7 @@ function getAgentDirectoryOrThrow( path.dirname(resolved) === agentsDirectory && path.basename(resolved) === profileName; if (!isValidPath) { - throw new Error(`The agent config file path is incorrect: ${resolved}`); + throw new Error(`The agent config path is incorrect: ${resolved}`); } return resolved; } diff --git a/web/packages/teleterm/src/mainProcess/mainProcess.ts b/web/packages/teleterm/src/mainProcess/mainProcess.ts index 97c3275c9a5c5..854444fae6cbb 100644 --- a/web/packages/teleterm/src/mainProcess/mainProcess.ts +++ b/web/packages/teleterm/src/mainProcess/mainProcess.ts @@ -306,13 +306,14 @@ export default class MainProcess { ipcMain.handle('main-process-connect-my-computer-download-agent', () => this.downloadAgentShared() ); + ipcMain.handle( 'main-process-connect-my-computer-create-agent-config-file', (_, args: AgentConfigFileClusterProperties) => createAgentConfigFile(this.settings, { proxy: args.proxy, token: args.token, - profileName: args.profileName, + rootClusterUri: args.rootClusterUri, labels: args.labels, }) ); diff --git a/web/packages/teleterm/src/ui/services/connectMyComputer/connectMyComputerService.ts b/web/packages/teleterm/src/ui/services/connectMyComputer/connectMyComputerService.ts index 9b5947fc4bcf8..9868d77ddbe0b 100644 --- a/web/packages/teleterm/src/ui/services/connectMyComputer/connectMyComputerService.ts +++ b/web/packages/teleterm/src/ui/services/connectMyComputer/connectMyComputerService.ts @@ -21,8 +21,6 @@ import { TshClient, } from 'teleterm/services/tshd/types'; -import { routing } from 'teleterm/ui/uri'; - import type * as uri from 'teleterm/ui/uri'; export class ConnectMyComputerService { @@ -42,13 +40,11 @@ export class ConnectMyComputerService { } async createAgentConfigFile(cluster: Cluster): Promise { - const { rootClusterId } = routing.parseClusterUri(cluster.uri).params; - const { token, labelsList } = await this.tshClient.createConnectMyComputerNodeToken(cluster.uri); await this.mainProcessClient.createAgentConfigFile({ - profileName: rootClusterId, + rootClusterUri: cluster.uri, proxy: cluster.proxyHost, token: token, labels: labelsList, From ca6a5b2f83c47700eb187923b64c1771aa306d2a Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Mon, 24 Jul 2023 17:41:32 +0200 Subject: [PATCH 02/31] Add functions to run agent and subscribe to its events --- .../src/mainProcess/agentRunner.test.ts | 121 ++++++++++++++++ .../teleterm/src/mainProcess/agentRunner.ts | 133 ++++++++++++++++++ .../src/mainProcess/fixtures/mocks.ts | 7 + .../teleterm/src/mainProcess/mainProcess.ts | 25 ++++ .../src/mainProcess/mainProcessClient.ts | 28 +++- .../teleterm/src/mainProcess/types.ts | 27 ++++ 6 files changed, 340 insertions(+), 1 deletion(-) create mode 100644 web/packages/teleterm/src/mainProcess/agentRunner.test.ts create mode 100644 web/packages/teleterm/src/mainProcess/agentRunner.ts diff --git a/web/packages/teleterm/src/mainProcess/agentRunner.test.ts b/web/packages/teleterm/src/mainProcess/agentRunner.test.ts new file mode 100644 index 0000000000000..f3a17ce64cd1b --- /dev/null +++ b/web/packages/teleterm/src/mainProcess/agentRunner.test.ts @@ -0,0 +1,121 @@ +/* +Copyright 2023 Gravitational, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import childProcess, { ChildProcess } from 'node:child_process'; +import { EventEmitter } from 'node:events'; +import { PassThrough } from 'node:stream'; + +import Logger, { NullService } from 'teleterm/logger'; +import { RootClusterUri } from 'teleterm/ui/uri'; + +import { makeRuntimeSettings } from './fixtures/mocks'; + +import { AgentRunner } from './agentRunner'; +import { AgentProcessState } from './types'; + +jest.mock('node:child_process'); + +let eventEmitter: EventEmitter; +let childProcessMock: ChildProcess; +beforeEach(() => { + Logger.init(new NullService()); + + eventEmitter = new EventEmitter(); + childProcessMock = { + stderr: new PassThrough(), + once: (event, listener) => { + eventEmitter.once(event, listener); + return this; + }, + on: (event, listener) => { + eventEmitter.on(event, listener); + return this; + }, + off: (event, listener) => { + eventEmitter.off(event, listener); + return this; + }, + kill: jest.fn().mockImplementation(() => { + eventEmitter.emit('exit', 0); + }), + } as unknown as ChildProcess; + + jest.spyOn(childProcess, 'spawn').mockReturnValue(childProcessMock); +}); + +const userDataDir = '/Users/test/Application Data/Teleport Connect'; +const agentBinaryPath = '/Users/test/Caches/Teleport Connect/teleport/teleport'; +const rootClusterUri: RootClusterUri = '/clusters/cluster.local'; + +test('agent process starts with correct arguments', () => { + const agentRunner = new AgentRunner( + makeRuntimeSettings({ + agentBinaryPath, + userDataDir, + }), + () => {} + ); + agentRunner.start(rootClusterUri); + + expect(childProcess.spawn).toHaveBeenCalledWith( + agentBinaryPath, + ['start', `--config=${userDataDir}/agents/cluster.local/config.yaml`], + expect.anything() + ); +}); + +test('previous agent process is killed when a new one is started', () => { + const agentRunner = new AgentRunner( + makeRuntimeSettings({ + agentBinaryPath, + userDataDir, + }), + () => {} + ); + agentRunner.start(rootClusterUri); + agentRunner.start(rootClusterUri); + + expect(childProcessMock.kill).toHaveBeenCalledWith('SIGKILL'); +}); + +test('status updates are sent', () => { + const updateSender = jest.fn(); + const agentRunner = new AgentRunner( + makeRuntimeSettings({ + agentBinaryPath, + userDataDir, + }), + updateSender + ); + + agentRunner.start(rootClusterUri); + expect(updateSender).toHaveBeenCalledWith(rootClusterUri, { + status: 'running', + } as AgentProcessState); + + const error = new Error('unknown error'); + eventEmitter.emit('error', error); + expect(updateSender).toHaveBeenCalledWith(rootClusterUri, { + status: 'error', + message: `${error}`, + } as AgentProcessState); + + agentRunner.kill(rootClusterUri); + expect(updateSender).toHaveBeenCalledWith(rootClusterUri, { + status: 'exited', + code: 0, + } as AgentProcessState); +}); diff --git a/web/packages/teleterm/src/mainProcess/agentRunner.ts b/web/packages/teleterm/src/mainProcess/agentRunner.ts new file mode 100644 index 0000000000000..8f6d69e1c894c --- /dev/null +++ b/web/packages/teleterm/src/mainProcess/agentRunner.ts @@ -0,0 +1,133 @@ +/** + * Copyright 2023 Gravitational, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { spawn, ChildProcess } from 'node:child_process'; +import os from 'node:os'; + +import Logger from 'teleterm/logger'; +import { RootClusterUri } from 'teleterm/ui/uri'; + +import { generateAgentConfigPaths } from './createAgentConfigFile'; +import { AgentProcessState, RuntimeSettings } from './types'; + +const MAX_STDERR_LINES = 10; + +export class AgentRunner { + private logger = new Logger('AgentRunner'); + private agentProcesses = new Map(); + + constructor( + private settings: RuntimeSettings, + private sendProcessState: ( + rootClusterUri: RootClusterUri, + state: AgentProcessState + ) => void + ) {} + + /** + * Starts a new agent process. + * If an existing process exists for the given root cluster, the old one will be killed with SIGKILL. + * To kill the old process gracefully before starting the new one, use `kill()`. + */ + start(rootClusterUri: RootClusterUri): void { + if (this.agentProcesses.has(rootClusterUri)) { + this.agentProcesses.get(rootClusterUri).kill('SIGKILL'); + this.logger.warn(`Forcefully killed agent process for ${rootClusterUri}`); + } + + const { agentBinaryPath } = this.settings; + const { configFile } = generateAgentConfigPaths( + this.settings, + rootClusterUri + ); + + const args = [ + 'start', + `--config=${configFile}`, + this.settings.appVersion === '1.0.0-dev' && '--skip-version-check', + ].filter(Boolean); + + this.logger.info( + `Starting agent from ${agentBinaryPath} with arguments ${args}` + ); + + const agentProcess = spawn(agentBinaryPath, args, { + windowsHide: true, + env: process.env, + }); + + this.sendProcessState(rootClusterUri, { + status: 'running', + }); + + this.addListeners(rootClusterUri, agentProcess); + this.agentProcesses.set(rootClusterUri, agentProcess); + } + + kill(rootClusterUri: RootClusterUri): void { + this.agentProcesses.get(rootClusterUri).kill('SIGTERM'); + this.agentProcesses.delete(rootClusterUri); + } + + killAll(): void { + this.agentProcesses.forEach((agent, rootClusterUri) => { + agent.kill('SIGTERM'); + this.agentProcesses.delete(rootClusterUri); + }); + } + + private addListeners( + rootClusterUri: RootClusterUri, + process: ChildProcess + ): void { + // Teleport logs output to stderr. + let stderrOutput = ''; + process.stderr.setEncoding('utf-8'); + process.stderr.on('data', error => { + stderrOutput += error; + stderrOutput = limitProcessOutputLines(stderrOutput); + }); + + const errorHandler = (error: Error) => { + this.sendProcessState(rootClusterUri, { + status: 'error', + message: `${error}`, + }); + }; + + const exitHandler = ( + code: number | null, + signal: NodeJS.Signals | null + ) => { + // Remove error handler when the process exits. + process.off('error', errorHandler); + + this.sendProcessState(rootClusterUri, { + status: 'exited', + code, + signal, + stackTrace: code !== 0 ? stderrOutput : undefined, + }); + }; + + process.once('error', errorHandler); + process.once('exit', exitHandler); + } +} + +function limitProcessOutputLines(output: string): string { + return output.split(os.EOL).slice(-MAX_STDERR_LINES).join(os.EOL); +} diff --git a/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts b/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts index 58bb938f41c05..2ebf81962e0fb 100644 --- a/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts +++ b/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts @@ -84,6 +84,13 @@ export class MockMainProcessClient implements MainProcessClient { createAgentConfigFile() { return Promise.resolve(); } + + runAgent(): Promise { + return Promise.resolve(); + } + subscribeToAgentUpdate() { + return { cleanup: () => undefined }; + } } export const makeRuntimeSettings = ( diff --git a/web/packages/teleterm/src/mainProcess/mainProcess.ts b/web/packages/teleterm/src/mainProcess/mainProcess.ts index 854444fae6cbb..c9f184d50cefb 100644 --- a/web/packages/teleterm/src/mainProcess/mainProcess.ts +++ b/web/packages/teleterm/src/mainProcess/mainProcess.ts @@ -36,6 +36,7 @@ import { subscribeToFileStorageEvents } from 'teleterm/services/fileStorage'; import { LoggerColor, createFileLoggerService } from 'teleterm/services/logger'; import { ChildProcessAddresses } from 'teleterm/mainProcess/types'; import { getAssetPath } from 'teleterm/mainProcess/runtimeSettings'; +import { RootClusterUri } from 'teleterm/ui/uri'; import Logger from 'teleterm/logger'; import { @@ -49,6 +50,7 @@ import { resolveNetworkAddress } from './resolveNetworkAddress'; import { WindowsManager } from './windowsManager'; import { downloadAgent, FileDownloader } from './agentDownloader'; import { createAgentConfigFile } from './createAgentConfigFile'; +import { AgentRunner } from './agentRunner'; import type { AgentConfigFileClusterProperties } from './createAgentConfigFile'; @@ -79,6 +81,7 @@ export default class MainProcess { process.env ) ); + private readonly agentRunner: AgentRunner; private constructor(opts: Options) { this.settings = opts.settings; @@ -87,6 +90,15 @@ export default class MainProcess { this.appStateFileStorage = opts.appStateFileStorage; this.configFileStorage = opts.configFileStorage; this.windowsManager = opts.windowsManager; + this.agentRunner = new AgentRunner(this.settings, (rootClusterUri, state) => + this.windowsManager + .getWindow() + .webContents.send( + 'main-process-connect-my-computer-agent-update', + rootClusterUri, + state + ) + ); } static create(opts: Options) { @@ -98,6 +110,7 @@ export default class MainProcess { dispose() { this.killTshdProcess(); this.sharedProcess.kill('SIGTERM'); + this.agentRunner.killAll(); const processesExit = Promise.all([ promisifyProcessExit(this.tshdProcess), promisifyProcessExit(this.sharedProcess), @@ -318,6 +331,18 @@ export default class MainProcess { }) ); + ipcMain.handle( + 'main-process-connect-my-computer-run-agent', + ( + _, + args: { + rootClusterUri: RootClusterUri; + } + ) => { + this.agentRunner.start(args.rootClusterUri); + } + ); + subscribeToTerminalContextMenuEvent(); subscribeToTabContextMenuEvent(); subscribeToConfigServiceEvents(this.configService); diff --git a/web/packages/teleterm/src/mainProcess/mainProcessClient.ts b/web/packages/teleterm/src/mainProcess/mainProcessClient.ts index 5865e1c198c6d..064f466ca88b7 100644 --- a/web/packages/teleterm/src/mainProcess/mainProcessClient.ts +++ b/web/packages/teleterm/src/mainProcess/mainProcessClient.ts @@ -18,13 +18,19 @@ import { ipcRenderer } from 'electron'; import { createFileStorageClient } from 'teleterm/services/fileStorage'; import { AgentConfigFileClusterProperties } from 'teleterm/mainProcess/createAgentConfigFile'; +import { RootClusterUri } from 'teleterm/ui/uri'; import { createConfigServiceClient } from '../services/config'; import { openTerminalContextMenu } from './contextMenus/terminalContextMenu'; -import { MainProcessClient, ChildProcessAddresses } from './types'; import { openTabContextMenu } from './contextMenus/tabContextMenu'; +import { + MainProcessClient, + ChildProcessAddresses, + AgentProcessState, +} from './types'; + export default function createMainProcessClient(): MainProcessClient { return { getRuntimeSettings() { @@ -80,5 +86,25 @@ export default function createMainProcessClient(): MainProcessClient { clusterProperties ); }, + runAgent(clusterProperties: { rootClusterUri: RootClusterUri }) { + return ipcRenderer.invoke( + 'main-process-connect-my-computer-run-agent', + clusterProperties + ); + }, + subscribeToAgentUpdate: listener => { + const onChange = ( + _, + rootClusterUri: RootClusterUri, + state: AgentProcessState + ) => { + listener(rootClusterUri, state); + }; + const channel = 'main-process-connect-my-computer-agent-update'; + ipcRenderer.addListener(channel, onChange); + return { + cleanup: () => ipcRenderer.removeListener(channel, onChange), + }; + }, }; } diff --git a/web/packages/teleterm/src/mainProcess/types.ts b/web/packages/teleterm/src/mainProcess/types.ts index 691b3524d1526..a23f5d0963c0e 100644 --- a/web/packages/teleterm/src/mainProcess/types.ts +++ b/web/packages/teleterm/src/mainProcess/types.ts @@ -15,6 +15,8 @@ */ import { AgentConfigFileClusterProperties } from 'teleterm/mainProcess/createAgentConfigFile'; +import { RootClusterUri } from 'teleterm/ui/uri'; + import { Kind } from 'teleterm/ui/services/workspacesService'; import { FileStorage } from 'teleterm/services/fileStorage'; @@ -90,6 +92,12 @@ export type MainProcessClient = { createAgentConfigFile( properties: AgentConfigFileClusterProperties ): Promise; + runAgent(args: { rootClusterUri: RootClusterUri }): Promise; + subscribeToAgentUpdate: ( + listener: (rootClusterUri: RootClusterUri, state: AgentProcessState) => void + ) => { + cleanup: () => void; + }; }; export type ChildProcessAddresses = { @@ -103,6 +111,25 @@ export type GrpcServerAddresses = ChildProcessAddresses & { export type Platform = NodeJS.Platform; +export type AgentProcessState = + | { + status: 'not-started'; + } + | { + status: 'running'; + } + | { + status: 'exited'; + code: number | null; + signal: NodeJS.Signals | null; + /** Fragment of a stack trace when code is other than 0. */ + stackTrace?: string; + } + | { + status: 'error'; + message: string; + }; + export interface ClusterContextMenuOptions { isClusterConnected: boolean; From da19a685a7c769ff6c523f7c62a7518d8fb1d891 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Mon, 24 Jul 2023 17:47:10 +0200 Subject: [PATCH 03/31] Clear attempts when restarting the process --- .../DocumentConnectMyComputerSetup.tsx | 77 +++++++++++-------- 1 file changed, 45 insertions(+), 32 deletions(-) diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx index a60a1267c2ebd..74a10f8d39b93 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx @@ -88,34 +88,39 @@ function AgentSetup() { const { rootClusterUri } = useWorkspaceContext(); const cluster = ctx.clustersService.findCluster(rootClusterUri); - const [setUpRolesAttempt, runSetUpRolesAttempt] = useAsync( - useCallback(async () => { - retryWithRelogin(ctx, rootClusterUri, async () => { - let certsReloaded = false; - - try { - const response = await ctx.connectMyComputerService.createRole( - rootClusterUri - ); - certsReloaded = response.certsReloaded; - } catch (error) { - if ((error.message as string)?.includes('access denied')) { - throw new Error( - 'Access denied. Contact your administrator for permissions to manage users and roles.' + const [createRoleAttempt, runCreateRoleAttempt, setCreateRoleAttempt] = + useAsync( + useCallback(async () => { + retryWithRelogin(ctx, rootClusterUri, async () => { + let certsReloaded = false; + + try { + const response = await ctx.connectMyComputerService.createRole( + rootClusterUri ); + certsReloaded = response.certsReloaded; + } catch (error) { + if ((error.message as string)?.includes('access denied')) { + throw new Error( + 'Access denied. Contact your administrator for permissions to manage users and roles.' + ); + } + throw error; } - throw error; - } - - // If tshd reloaded the certs to refresh the role list, the Electron app must resync details - // of the cluster to also update the role list in the UI. - if (certsReloaded) { - await ctx.clustersService.syncRootCluster(rootClusterUri); - } - }); - }, [ctx, rootClusterUri]) - ); - const [downloadAgentAttempt, runDownloadAgentAttempt] = useAsync( + + // If tshd reloaded the certs to refresh the role list, the Electron app must resync details + // of the cluster to also update the role list in the UI. + if (certsReloaded) { + await ctx.clustersService.syncRootCluster(rootClusterUri); + } + }); + }, [ctx, rootClusterUri]) + ); + const [ + downloadAgentAttempt, + runDownloadAgentAttempt, + setDownloadAgentAttempt, + ] = useAsync( useCallback( () => ctx.connectMyComputerService.downloadAgent(), [ctx.connectMyComputerService] @@ -138,7 +143,7 @@ function AgentSetup() { const steps = [ { name: 'Setting up the role', - attempt: setUpRolesAttempt, + attempt: createRoleAttempt, }, { name: 'Downloading the agent', @@ -155,9 +160,13 @@ function AgentSetup() { ]; const runSteps = useCallback(async () => { - // uncomment when implemented + setCreateRoleAttempt(makeEmptyAttempt()); + setDownloadAgentAttempt(makeEmptyAttempt()); + setGenerateConfigFileAttempt(makeEmptyAttempt()); + setJoinClusterAttempt(makeEmptyAttempt()); + const actions = [ - runSetUpRolesAttempt, + runCreateRoleAttempt, runDownloadAgentAttempt, runGenerateConfigFileAttempt, // runJoinClusterAttempt, @@ -169,7 +178,11 @@ function AgentSetup() { } } }, [ - runSetUpRolesAttempt, + setCreateRoleAttempt, + setDownloadAgentAttempt, + setGenerateConfigFileAttempt, + setJoinClusterAttempt, + runCreateRoleAttempt, runDownloadAgentAttempt, runGenerateConfigFileAttempt, runJoinClusterAttempt, @@ -178,7 +191,7 @@ function AgentSetup() { useEffect(() => { if ( [ - setUpRolesAttempt, + createRoleAttempt, downloadAgentAttempt, generateConfigFileAttempt, joinClusterAttempt, @@ -190,7 +203,7 @@ function AgentSetup() { downloadAgentAttempt, generateConfigFileAttempt, joinClusterAttempt, - setUpRolesAttempt, + createRoleAttempt, runSteps, ]); From ea2c34fa239cade7b0cf261133e8dc97c349e27b Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Mon, 24 Jul 2023 17:48:36 +0200 Subject: [PATCH 04/31] Run the agent from the UI and remove node token --- .../DocumentConnectMyComputerSetup.tsx | 44 +++++++---- .../connectMyComputerContext.tsx | 73 +++++++++++++++++++ .../src/ui/ConnectMyComputer/index.ts | 1 + .../src/ui/Documents/DocumentsRenderer.tsx | 19 +++-- .../connectMyComputerService.ts | 15 +++- 5 files changed, 131 insertions(+), 21 deletions(-) create mode 100644 web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx index 74a10f8d39b93..249f7885222bb 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import React, { useCallback, useEffect, useState } from 'react'; +import React, { useCallback, useEffect, useRef, useState } from 'react'; import { Box, ButtonPrimary, Flex, Text } from 'design'; import { useAsync } from 'shared/hooks/useAsync'; import { wait } from 'shared/utils/wait'; @@ -26,6 +26,9 @@ import { useAppContext } from 'teleterm/ui/appContextProvider'; import Document from 'teleterm/ui/Document'; import { useWorkspaceContext } from 'teleterm/ui/Documents'; import { retryWithRelogin } from 'teleterm/ui/utils'; +import { useConnectMyComputerContext } from 'teleterm/ui/ConnectMyComputer'; + +import type { AgentProcessState } from 'teleterm/mainProcess/types'; interface DocumentConnectMyComputerSetupProps { visible: boolean; @@ -86,7 +89,9 @@ function Information(props: { onSetUpAgentClick(): void }) { function AgentSetup() { const ctx = useAppContext(); const { rootClusterUri } = useWorkspaceContext(); + const connectMyComputerContext = useConnectMyComputerContext(); const cluster = ctx.clustersService.findCluster(rootClusterUri); + const nodeToken = useRef(); const [createRoleAttempt, runCreateRoleAttempt, setCreateRoleAttempt] = useAsync( @@ -126,19 +131,30 @@ function AgentSetup() { [ctx.connectMyComputerService] ) ); - const [generateConfigFileAttempt, runGenerateConfigFileAttempt] = useAsync( - useCallback( - () => - retryWithRelogin(ctx, rootClusterUri, () => - ctx.connectMyComputerService.createAgentConfigFile(cluster) - ), - [cluster, ctx, rootClusterUri] - ) - ); - const [joinClusterAttempt, runJoinClusterAttempt] = useAsync( - // TODO(gzdunek): delete node token after joining the cluster - useCallback(() => wait(1_000), []) + const [ + generateConfigFileAttempt, + runGenerateConfigFileAttempt, + setGenerateConfigFileAttempt, + ] = useAsync( + useCallback(async () => { + const { token } = await retryWithRelogin(ctx, rootClusterUri, () => + ctx.connectMyComputerService.createAgentConfigFile(cluster) + ); + nodeToken.current = token; + }, [cluster, ctx, rootClusterUri]) ); + const [joinClusterAttempt, runJoinClusterAttempt, setJoinClusterAttempt] = + useAsync( + useCallback(async () => { + if (!nodeToken.current) { + throw new Error('Node token is empty'); + } + await ctx.connectMyComputerService.runAgentAndDeleteToken( + cluster, + nodeToken.current + ); + }, [ctx.connectMyComputerService, cluster]) + ); const steps = [ { @@ -169,7 +185,7 @@ function AgentSetup() { runCreateRoleAttempt, runDownloadAgentAttempt, runGenerateConfigFileAttempt, - // runJoinClusterAttempt, + runJoinClusterAttempt, ]; for (const action of actions) { const [, error] = await action(); diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx new file mode 100644 index 0000000000000..87276f842f90c --- /dev/null +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx @@ -0,0 +1,73 @@ +/** + * Copyright 2023 Gravitational, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import React, { + useContext, + FC, + createContext, + useState, + useEffect, +} from 'react'; + +import { RootClusterUri } from 'teleterm/ui/uri'; +import { useAppContext } from 'teleterm/ui/appContextProvider'; + +import type { AgentProcessState } from 'teleterm/mainProcess/types'; + +export interface ConnectMyComputerContext { + state: AgentProcessState; +} + +const ConnectMyComputerContext = createContext(null); + +export const ConnectMyComputerContextProvider: FC<{ + rootClusterUri: RootClusterUri; +}> = props => { + const { mainProcessClient } = useAppContext(); + const [agentState, setAgentState] = useState(() => ({ + status: 'not-started', + })); + + useEffect(() => { + const { cleanup } = mainProcessClient.subscribeToAgentUpdate( + (rootClusterUri, state) => { + if (props.rootClusterUri === rootClusterUri) { + setAgentState(state); + } + } + ); + return cleanup; + }, [mainProcessClient, props.rootClusterUri]); + + return ( + + ); +}; + +export const useConnectMyComputerContext = () => { + const context = useContext(ConnectMyComputerContext); + + if (!context) { + throw new Error( + 'ConnectMyComputerContext requires ConnectMyComputerContextProvider context.' + ); + } + + return context; +}; diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/index.ts b/web/packages/teleterm/src/ui/ConnectMyComputer/index.ts index c01d682d018ee..e56a7adb3e84b 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/index.ts +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/index.ts @@ -15,4 +15,5 @@ limitations under the License. */ export * from './DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup'; +export * from './connectMyComputerContext'; export { NavigationMenu as ConnectMyComputerNavigationMenu } from './NavigationMenu'; diff --git a/web/packages/teleterm/src/ui/Documents/DocumentsRenderer.tsx b/web/packages/teleterm/src/ui/Documents/DocumentsRenderer.tsx index d5bce83781644..ee05f1801858d 100644 --- a/web/packages/teleterm/src/ui/Documents/DocumentsRenderer.tsx +++ b/web/packages/teleterm/src/ui/Documents/DocumentsRenderer.tsx @@ -32,7 +32,10 @@ import { import DocumentCluster from 'teleterm/ui/DocumentCluster'; import DocumentGateway from 'teleterm/ui/DocumentGateway'; import { DocumentTerminal } from 'teleterm/ui/DocumentTerminal'; -import { DocumentConnectMyComputerSetup } from 'teleterm/ui/ConnectMyComputer'; +import { + ConnectMyComputerContextProvider, + DocumentConnectMyComputerSetup, +} from 'teleterm/ui/ConnectMyComputer'; import Document from 'teleterm/ui/Document'; import { RootClusterUri } from 'teleterm/ui/uri'; @@ -75,11 +78,15 @@ export function DocumentsRenderer() { key={workspace.rootClusterUri} > - {workspace.documentsService.getDocuments().length ? ( - renderDocuments(workspace.documentsService) - ) : ( - - )} + + {workspace.documentsService.getDocuments().length ? ( + renderDocuments(workspace.documentsService) + ) : ( + + )} + ))} diff --git a/web/packages/teleterm/src/ui/services/connectMyComputer/connectMyComputerService.ts b/web/packages/teleterm/src/ui/services/connectMyComputer/connectMyComputerService.ts index 9868d77ddbe0b..5552209463e4e 100644 --- a/web/packages/teleterm/src/ui/services/connectMyComputer/connectMyComputerService.ts +++ b/web/packages/teleterm/src/ui/services/connectMyComputer/connectMyComputerService.ts @@ -39,7 +39,9 @@ export class ConnectMyComputerService { return this.tshClient.createConnectMyComputerRole(rootClusterUri); } - async createAgentConfigFile(cluster: Cluster): Promise { + async createAgentConfigFile(cluster: Cluster): Promise<{ + token: string; + }> { const { token, labelsList } = await this.tshClient.createConnectMyComputerNodeToken(cluster.uri); @@ -49,5 +51,16 @@ export class ConnectMyComputerService { token: token, labels: labelsList, }); + + return { token }; + } + + async runAgentAndDeleteToken(cluster: Cluster, token: string): Promise { + await this.mainProcessClient.runAgent({ + rootClusterUri: cluster.uri, + }); + + // invalidate token + await this.tshClient.deleteConnectMyComputerToken(cluster.uri, token); } } From 8da756d513f17965a27f41b5623837fe3036d8df Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Mon, 24 Jul 2023 17:50:01 +0200 Subject: [PATCH 05/31] Show errors from the process in the setup UI --- .../DocumentConnectMyComputerSetup.tsx | 50 +++++++++++++++++-- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx index 249f7885222bb..58008a12e5012 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx @@ -16,8 +16,12 @@ limitations under the License. import React, { useCallback, useEffect, useRef, useState } from 'react'; import { Box, ButtonPrimary, Flex, Text } from 'design'; -import { useAsync } from 'shared/hooks/useAsync'; -import { wait } from 'shared/utils/wait'; +import { + Attempt, + makeEmptyAttempt, + makeErrorAttempt, + useAsync, +} from 'shared/hooks/useAsync'; import * as Alerts from 'design/Alert'; import { CircleCheck, CircleCross, CirclePlay, Spinner } from 'design/Icon'; @@ -171,7 +175,12 @@ function AgentSetup() { }, { name: 'Joining the cluster', - attempt: joinClusterAttempt, + attempt: + // hide errors on setup restart + (joinClusterAttempt.status !== '' && + // errors from spawning the process take precedence over a regular attempt value + mapFailedAgentStateToAttempt(connectMyComputerContext.state)) || + joinClusterAttempt, }, ]; @@ -260,7 +269,14 @@ function AgentSetup() {
  • {step.name} {step.attempt.status === 'error' && ( - {step.attempt.statusText} + + {step.attempt.statusText} + )}
    • @@ -273,3 +289,29 @@ function AgentSetup() { ); } + +function mapFailedAgentStateToAttempt( + agentState: AgentProcessState +): Attempt { + if (agentState.status === 'exited') { + const { code, signal } = agentState; + const codeOrSignal = [ + // code can be 0, so we cannot just check it the same way as the signal. + code != null && `code ${code}`, + signal && `signal ${signal}`, + ] + .filter(Boolean) + .join(' '); + + return makeErrorAttempt( + [`Agent process exited with ${codeOrSignal}.`, agentState.stackTrace] + .filter(Boolean) + .join(' \n') + ); + } + if (agentState.status === 'error') { + return makeErrorAttempt( + ['Agent process failed to start.', agentState.message].join(' \n') + ); + } +} From 904e2535323bfc770f87ad5abf5408ba3f50ef8a Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Tue, 25 Jul 2023 18:40:21 +0200 Subject: [PATCH 06/31] Refactor reporting errors from the agent process --- .../src/mainProcess/mainProcessClient.ts | 10 +- .../teleterm/src/mainProcess/types.ts | 13 ++- .../DocumentConnectMyComputerSetup.tsx | 50 ++-------- .../connectMyComputerContext.tsx | 93 +++++++++++++++++-- .../connectMyComputerService.ts | 14 ++- 5 files changed, 118 insertions(+), 62 deletions(-) diff --git a/web/packages/teleterm/src/mainProcess/mainProcessClient.ts b/web/packages/teleterm/src/mainProcess/mainProcessClient.ts index 064f466ca88b7..36628750a7a84 100644 --- a/web/packages/teleterm/src/mainProcess/mainProcessClient.ts +++ b/web/packages/teleterm/src/mainProcess/mainProcessClient.ts @@ -92,13 +92,15 @@ export default function createMainProcessClient(): MainProcessClient { clusterProperties ); }, - subscribeToAgentUpdate: listener => { + subscribeToAgentUpdate: (rootClusterUri, listener) => { const onChange = ( _, - rootClusterUri: RootClusterUri, - state: AgentProcessState + eventRootClusterUri: RootClusterUri, + eventState: AgentProcessState ) => { - listener(rootClusterUri, state); + if (eventRootClusterUri === rootClusterUri) { + listener(eventState); + } }; const channel = 'main-process-connect-my-computer-agent-update'; ipcRenderer.addListener(channel, onChange); diff --git a/web/packages/teleterm/src/mainProcess/types.ts b/web/packages/teleterm/src/mainProcess/types.ts index a23f5d0963c0e..6039a011a48ec 100644 --- a/web/packages/teleterm/src/mainProcess/types.ts +++ b/web/packages/teleterm/src/mainProcess/types.ts @@ -93,11 +93,14 @@ export type MainProcessClient = { properties: AgentConfigFileClusterProperties ): Promise; runAgent(args: { rootClusterUri: RootClusterUri }): Promise; - subscribeToAgentUpdate: ( - listener: (rootClusterUri: RootClusterUri, state: AgentProcessState) => void - ) => { - cleanup: () => void; - }; + subscribeToAgentUpdate: SubscribeToAgentUpdate; +}; + +export type SubscribeToAgentUpdate = ( + rootClusterUri: RootClusterUri, + listener: (state: AgentProcessState) => void +) => { + cleanup: () => void; }; export type ChildProcessAddresses = { diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx index 58008a12e5012..de6c29d49e0a9 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx @@ -17,9 +17,7 @@ limitations under the License. import React, { useCallback, useEffect, useRef, useState } from 'react'; import { Box, ButtonPrimary, Flex, Text } from 'design'; import { - Attempt, makeEmptyAttempt, - makeErrorAttempt, useAsync, } from 'shared/hooks/useAsync'; import * as Alerts from 'design/Alert'; @@ -32,8 +30,6 @@ import { useWorkspaceContext } from 'teleterm/ui/Documents'; import { retryWithRelogin } from 'teleterm/ui/utils'; import { useConnectMyComputerContext } from 'teleterm/ui/ConnectMyComputer'; -import type { AgentProcessState } from 'teleterm/mainProcess/types'; - interface DocumentConnectMyComputerSetupProps { visible: boolean; doc: types.DocumentConnectMyComputerSetup; @@ -93,7 +89,7 @@ function Information(props: { onSetUpAgentClick(): void }) { function AgentSetup() { const ctx = useAppContext(); const { rootClusterUri } = useWorkspaceContext(); - const connectMyComputerContext = useConnectMyComputerContext(); + const { runAgentAndWaitForNodeToJoin } = useConnectMyComputerContext(); const cluster = ctx.clustersService.findCluster(rootClusterUri); const nodeToken = useRef(); @@ -153,11 +149,16 @@ function AgentSetup() { if (!nodeToken.current) { throw new Error('Node token is empty'); } - await ctx.connectMyComputerService.runAgentAndDeleteToken( - cluster, + await runAgentAndWaitForNodeToJoin(); + await ctx.connectMyComputerService.deleteToken( + cluster.uri, nodeToken.current ); - }, [ctx.connectMyComputerService, cluster]) + }, [ + runAgentAndWaitForNodeToJoin, + ctx.connectMyComputerService, + cluster.uri, + ]) ); const steps = [ @@ -175,12 +176,7 @@ function AgentSetup() { }, { name: 'Joining the cluster', - attempt: - // hide errors on setup restart - (joinClusterAttempt.status !== '' && - // errors from spawning the process take precedence over a regular attempt value - mapFailedAgentStateToAttempt(connectMyComputerContext.state)) || - joinClusterAttempt, + attempt: joinClusterAttempt, }, ]; @@ -289,29 +285,3 @@ function AgentSetup() { ); } - -function mapFailedAgentStateToAttempt( - agentState: AgentProcessState -): Attempt { - if (agentState.status === 'exited') { - const { code, signal } = agentState; - const codeOrSignal = [ - // code can be 0, so we cannot just check it the same way as the signal. - code != null && `code ${code}`, - signal && `signal ${signal}`, - ] - .filter(Boolean) - .join(' '); - - return makeErrorAttempt( - [`Agent process exited with ${codeOrSignal}.`, agentState.stackTrace] - .filter(Boolean) - .join(' \n') - ); - } - if (agentState.status === 'error') { - return makeErrorAttempt( - ['Agent process failed to start.', agentState.message].join(' \n') - ); - } -} diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx index 87276f842f90c..22be0554ddd61 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx @@ -20,15 +20,22 @@ import React, { createContext, useState, useEffect, + useCallback, } from 'react'; +import { wait } from 'shared/utils/wait'; + import { RootClusterUri } from 'teleterm/ui/uri'; import { useAppContext } from 'teleterm/ui/appContextProvider'; -import type { AgentProcessState } from 'teleterm/mainProcess/types'; +import type { + AgentProcessState, + SubscribeToAgentUpdate, +} from 'teleterm/mainProcess/types'; export interface ConnectMyComputerContext { state: AgentProcessState; + runAgentAndWaitForNodeToJoin(): Promise; } const ConnectMyComputerContext = createContext(null); @@ -36,25 +43,44 @@ const ConnectMyComputerContext = createContext(null); export const ConnectMyComputerContextProvider: FC<{ rootClusterUri: RootClusterUri; }> = props => { - const { mainProcessClient } = useAppContext(); + const { mainProcessClient, connectMyComputerService } = useAppContext(); const [agentState, setAgentState] = useState(() => ({ status: 'not-started', })); + const runAgentAndWaitForNodeToJoin = useCallback(async () => { + await connectMyComputerService.runAgent(props.rootClusterUri); + + // TODO(gzdunek): Replace with waiting for the node to join. + const waitForNodeToJoin = wait(1_000); + + await Promise.race([ + waitForNodeToJoin, + waitForAgentProcessErrors( + mainProcessClient.subscribeToAgentUpdate, + props.rootClusterUri + ), + ]); + }, [ + connectMyComputerService, + mainProcessClient.subscribeToAgentUpdate, + props.rootClusterUri, + ]); + useEffect(() => { const { cleanup } = mainProcessClient.subscribeToAgentUpdate( - (rootClusterUri, state) => { - if (props.rootClusterUri === rootClusterUri) { - setAgentState(state); - } - } + props.rootClusterUri, + state => setAgentState(state) ); return cleanup; }, [mainProcessClient, props.rootClusterUri]); return ( ); @@ -71,3 +97,54 @@ export const useConnectMyComputerContext = () => { return context; }; + +/** + * Waits for `error` and `exit` events from the agent process. + * If none of them happen within 20 seconds, the promise resolves. + */ +async function waitForAgentProcessErrors( + subscribeToAgentUpdate: SubscribeToAgentUpdate, + rootClusterUri: RootClusterUri +) { + let cleanupFn: () => void; + + try { + const errorPromise = new Promise((_, reject) => { + const { cleanup } = subscribeToAgentUpdate(rootClusterUri, agentState => { + if (agentState.status === 'exited') { + const { code, signal } = agentState; + const codeOrSignal = [ + // code can be 0, so we cannot just check it the same way as the signal. + code != null && `code ${code}`, + signal && `signal ${signal}`, + ] + .filter(Boolean) + .join(' '); + + reject( + new Error( + [ + `Agent process exited with ${codeOrSignal}.`, + agentState.stackTrace, + ] + .filter(Boolean) + .join('\n') + ) + ); + } + if (agentState.status === 'error') { + reject( + new Error( + ['Agent process failed to start.', agentState.message].join(' \n') + ) + ); + } + }); + + cleanupFn = cleanup; + }); + await Promise.race([errorPromise, wait(20_000)]); + } finally { + cleanupFn(); + } +} diff --git a/web/packages/teleterm/src/ui/services/connectMyComputer/connectMyComputerService.ts b/web/packages/teleterm/src/ui/services/connectMyComputer/connectMyComputerService.ts index 5552209463e4e..a482733a2b6b3 100644 --- a/web/packages/teleterm/src/ui/services/connectMyComputer/connectMyComputerService.ts +++ b/web/packages/teleterm/src/ui/services/connectMyComputer/connectMyComputerService.ts @@ -55,12 +55,16 @@ export class ConnectMyComputerService { return { token }; } - async runAgentAndDeleteToken(cluster: Cluster, token: string): Promise { - await this.mainProcessClient.runAgent({ - rootClusterUri: cluster.uri, + runAgent(rootClusterUri: uri.RootClusterUri): Promise { + return this.mainProcessClient.runAgent({ + rootClusterUri, }); + } - // invalidate token - await this.tshClient.deleteConnectMyComputerToken(cluster.uri, token); + deleteToken( + rootClusterUri: uri.RootClusterUri, + token: string + ): Promise { + return this.tshClient.deleteConnectMyComputerToken(rootClusterUri, token); } } From ff265fa26cabd750b5d5895a97b1e49eff39dd60 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Tue, 25 Jul 2023 18:55:29 +0200 Subject: [PATCH 07/31] Add `isLocalBuild` --- .../agentDownloader/agentDownloader.ts | 14 ++++++-------- .../teleterm/src/mainProcess/agentRunner.ts | 2 +- .../src/mainProcess/fixtures/mocks.ts | 12 ++++++++++-- .../src/mainProcess/runtimeSettings.ts | 19 +++++++++++-------- .../teleterm/src/mainProcess/types.ts | 5 +++++ 5 files changed, 33 insertions(+), 19 deletions(-) diff --git a/web/packages/teleterm/src/mainProcess/agentDownloader/agentDownloader.ts b/web/packages/teleterm/src/mainProcess/agentDownloader/agentDownloader.ts index 412ce300541ca..815572726ab5d 100644 --- a/web/packages/teleterm/src/mainProcess/agentDownloader/agentDownloader.ts +++ b/web/packages/teleterm/src/mainProcess/agentDownloader/agentDownloader.ts @@ -45,10 +45,8 @@ interface AgentBinary { /** * Downloads and unpacks the agent binary, if it has not already been downloaded. * - * The agent version to download is taken from settings.appVersion if it is not a dev version (1.0.0-dev). - * The settings.appVersion is set to a real version only for packaged apps that went through our CI build pipeline. - * In local builds, both for the development version and for packaged apps, settings.appVersion is set to 1.0.0-dev. - * In those cases, we fetch the latest available stable version of the agent. + * The agent version to download is taken from settings.appVersion if settings.isLocalBuild is false. + * If it isn't, we fetch the latest available stable version of the agent. * CONNECT_CMC_AGENT_VERSION is available as an escape hatch for cases where we want to fetch a different version. */ export async function downloadAgent( @@ -56,7 +54,7 @@ export async function downloadAgent( settings: RuntimeSettings, env: Record ): Promise { - const version = await calculateAgentVersion(settings.appVersion, env); + const version = await calculateAgentVersion(settings, env); if ( await isCorrectAgentVersionAlreadyDownloaded( @@ -87,11 +85,11 @@ export async function downloadAgent( } async function calculateAgentVersion( - appVersion: string, + settings: RuntimeSettings, env: Record ): Promise { - if (appVersion !== '1.0.0-dev') { - return appVersion; + if (!settings.isLocalBuild) { + return settings.appVersion; } if (env.CONNECT_CMC_AGENT_VERSION) { return env.CONNECT_CMC_AGENT_VERSION; diff --git a/web/packages/teleterm/src/mainProcess/agentRunner.ts b/web/packages/teleterm/src/mainProcess/agentRunner.ts index 8f6d69e1c894c..d85f68bf7c453 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner.ts @@ -57,7 +57,7 @@ export class AgentRunner { const args = [ 'start', `--config=${configFile}`, - this.settings.appVersion === '1.0.0-dev' && '--skip-version-check', + this.settings.isLocalBuild && '--skip-version-check', ].filter(Boolean); this.logger.info( diff --git a/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts b/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts index 2ebf81962e0fb..0bde8808f0e03 100644 --- a/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts +++ b/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts @@ -37,7 +37,10 @@ export class MockMainProcessClient implements MainProcessClient { } getResolvedChildProcessAddresses = () => - Promise.resolve({ tsh: '', shared: '' }); + Promise.resolve({ + tsh: '', + shared: '', + }); openTerminalContextMenu() {} @@ -46,7 +49,10 @@ export class MockMainProcessClient implements MainProcessClient { openTabContextMenu() {} showFileSaveDialog() { - return Promise.resolve({ canceled: false, filePath: '' }); + return Promise.resolve({ + canceled: false, + filePath: '', + }); } fileStorage = createMockFileStorage(); @@ -88,6 +94,7 @@ export class MockMainProcessClient implements MainProcessClient { runAgent(): Promise { return Promise.resolve(); } + subscribeToAgentUpdate() { return { cleanup: () => undefined }; } @@ -123,5 +130,6 @@ export const makeRuntimeSettings = ( arch: 'arm64', osVersion: '22.2.0', appVersion: '11.1.0', + isLocalBuild: runtimeSettings?.appVersion === '1.0.0-dev', ...runtimeSettings, }); diff --git a/web/packages/teleterm/src/mainProcess/runtimeSettings.ts b/web/packages/teleterm/src/mainProcess/runtimeSettings.ts index bb361f1b41e9c..d092a13fcb257 100644 --- a/web/packages/teleterm/src/mainProcess/runtimeSettings.ts +++ b/web/packages/teleterm/src/mainProcess/runtimeSettings.ts @@ -80,6 +80,15 @@ function getRuntimeSettings(): RuntimeSettings { requestedNetworkAddress: tshdEventsAddress, }; + // To start the app in dev mode, we run `electron path_to_main.js`. It means + // that the app is run without package.json context, so it can not read the version + // from it. + // The way we run Electron can be changed (`electron .`), but it has one major + // drawback - dev app and bundled app will use the same app data directory. + // + // A workaround is to read the version from `process.env.npm_package_version`. + const appVersion = dev ? process.env.npm_package_version : app.getVersion(); + if (isInsecure) { tshd.flags.unshift('--debug'); tshd.flags.unshift('--insecure'); @@ -104,14 +113,8 @@ function getRuntimeSettings(): RuntimeSettings { ), arch: os.arch(), osVersion: os.release(), - // To start the app in dev mode we run `electron path_to_main.js`. It means - // that app is run without package.json context, so it can not read the version - // from it. - // The way we run Electron can be changed (`electron .`), but it has one major - // drawback - dev app and bundled app will use the same app data directory. - // - // A workaround is to read the version from `process.env.npm_package_version`. - appVersion: dev ? process.env.npm_package_version : app.getVersion(), + appVersion, + isLocalBuild: appVersion === '1.0.0-dev', }; } diff --git a/web/packages/teleterm/src/mainProcess/types.ts b/web/packages/teleterm/src/mainProcess/types.ts index 6039a011a48ec..64a3cccb75145 100644 --- a/web/packages/teleterm/src/mainProcess/types.ts +++ b/web/packages/teleterm/src/mainProcess/types.ts @@ -51,6 +51,11 @@ export type RuntimeSettings = { arch: string; osVersion: string; appVersion: string; + /** + * The {@link appVersion} is set to a real version only for packaged apps that went through our CI build pipeline. + * In local builds, both for the development version and for packaged apps, settings.appVersion is set to 1.0.0-dev. + */ + isLocalBuild: boolean; }; export type MainProcessClient = { From 7ba35c4fe2119e58d0c0cdfae4971e33b23940e0 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Tue, 25 Jul 2023 18:58:27 +0200 Subject: [PATCH 08/31] Join arguments with space when logging --- web/packages/teleterm/src/mainProcess/agentRunner.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/packages/teleterm/src/mainProcess/agentRunner.ts b/web/packages/teleterm/src/mainProcess/agentRunner.ts index d85f68bf7c453..882e9f45ea39a 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner.ts @@ -61,7 +61,7 @@ export class AgentRunner { ].filter(Boolean); this.logger.info( - `Starting agent from ${agentBinaryPath} with arguments ${args}` + `Starting agent from ${agentBinaryPath} with arguments ${args.join(' ')}` ); const agentProcess = spawn(agentBinaryPath, args, { From 40a62319f487b40d1cc8ad8c30a2f7ce48e35715 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Wed, 26 Jul 2023 13:14:52 +0200 Subject: [PATCH 09/31] Add `killProcess` function that handles process closing --- .../teleterm/src/mainProcess/agentRunner.ts | 27 ++++---- .../teleterm/src/mainProcess/mainProcess.ts | 32 ++++----- .../src/mainProcess/processKiller/index.ts | 17 +++++ .../processKiller/processKiller.test.ts | 68 +++++++++++++++++++ .../processKiller/processKiller.ts | 67 ++++++++++++++++++ .../mainProcess/processKiller/testProcess.mjs | 26 +++++++ 6 files changed, 205 insertions(+), 32 deletions(-) create mode 100644 web/packages/teleterm/src/mainProcess/processKiller/index.ts create mode 100644 web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts create mode 100644 web/packages/teleterm/src/mainProcess/processKiller/processKiller.ts create mode 100644 web/packages/teleterm/src/mainProcess/processKiller/testProcess.mjs diff --git a/web/packages/teleterm/src/mainProcess/agentRunner.ts b/web/packages/teleterm/src/mainProcess/agentRunner.ts index 882e9f45ea39a..f690bbc923c52 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner.ts @@ -22,6 +22,7 @@ import { RootClusterUri } from 'teleterm/ui/uri'; import { generateAgentConfigPaths } from './createAgentConfigFile'; import { AgentProcessState, RuntimeSettings } from './types'; +import { killProcess } from './processKiller'; const MAX_STDERR_LINES = 10; @@ -39,13 +40,12 @@ export class AgentRunner { /** * Starts a new agent process. - * If an existing process exists for the given root cluster, the old one will be killed with SIGKILL. - * To kill the old process gracefully before starting the new one, use `kill()`. + * If an existing process exists for the given root cluster, the old one will be killed. */ - start(rootClusterUri: RootClusterUri): void { + async start(rootClusterUri: RootClusterUri): Promise { if (this.agentProcesses.has(rootClusterUri)) { - this.agentProcesses.get(rootClusterUri).kill('SIGKILL'); - this.logger.warn(`Forcefully killed agent process for ${rootClusterUri}`); + await this.kill(rootClusterUri); + this.logger.warn(`Killed agent process for ${rootClusterUri}`); } const { agentBinaryPath } = this.settings; @@ -77,16 +77,19 @@ export class AgentRunner { this.agentProcesses.set(rootClusterUri, agentProcess); } - kill(rootClusterUri: RootClusterUri): void { - this.agentProcesses.get(rootClusterUri).kill('SIGTERM'); + async kill(rootClusterUri: RootClusterUri): Promise { + await killProcess(this.agentProcesses.get(rootClusterUri)); this.agentProcesses.delete(rootClusterUri); } - killAll(): void { - this.agentProcesses.forEach((agent, rootClusterUri) => { - agent.kill('SIGTERM'); - this.agentProcesses.delete(rootClusterUri); - }); + async killAll(): Promise { + const processes = Array.from(this.agentProcesses.entries()); + await Promise.all( + processes.map(async ([rootClusterUri, agent]) => { + await killProcess(agent); + this.agentProcesses.delete(rootClusterUri); + }) + ); } private addListeners( diff --git a/web/packages/teleterm/src/mainProcess/mainProcess.ts b/web/packages/teleterm/src/mainProcess/mainProcess.ts index c9f184d50cefb..16d9599c63e66 100644 --- a/web/packages/teleterm/src/mainProcess/mainProcess.ts +++ b/web/packages/teleterm/src/mainProcess/mainProcess.ts @@ -29,7 +29,6 @@ import { nativeTheme, shell, } from 'electron'; -import { wait } from 'shared/utils/wait'; import { FileStorage, RuntimeSettings } from 'teleterm/types'; import { subscribeToFileStorageEvents } from 'teleterm/services/fileStorage'; @@ -51,6 +50,7 @@ import { WindowsManager } from './windowsManager'; import { downloadAgent, FileDownloader } from './agentDownloader'; import { createAgentConfigFile } from './createAgentConfigFile'; import { AgentRunner } from './agentRunner'; +import { killProcess } from './processKiller'; import type { AgentConfigFileClusterProperties } from './createAgentConfigFile'; @@ -107,19 +107,15 @@ export default class MainProcess { return instance; } - dispose() { - this.killTshdProcess(); - this.sharedProcess.kill('SIGTERM'); - this.agentRunner.killAll(); - const processesExit = Promise.all([ - promisifyProcessExit(this.tshdProcess), - promisifyProcessExit(this.sharedProcess), + async dispose(): Promise { + await Promise.all([ + // sending usage events on tshd shutdown has 10-seconds timeout + killProcess(this.tshdProcess, 10_000, () => { + this.gracefullyKillTshdProcess(); + }), + killProcess(this.sharedProcess), + this.agentRunner.killAll(), ]); - // sending usage events on tshd shutdown has 10 seconds timeout - const timeout = wait(10_000).then(() => - this.logger.error('Child process(es) did not exit within 10 seconds') - ); - return Promise.race([processesExit, timeout]); } private _init() { @@ -333,13 +329,13 @@ export default class MainProcess { ipcMain.handle( 'main-process-connect-my-computer-run-agent', - ( + async ( _, args: { rootClusterUri: RootClusterUri; } ) => { - this.agentRunner.start(args.rootClusterUri); + await this.agentRunner.start(args.rootClusterUri); } ); @@ -418,7 +414,7 @@ export default class MainProcess { * kill a process is to send Ctrl-Break to its console. This task is done by * `tsh daemon stop` program. On Unix, the standard `SIGTERM` signal is sent. */ - private killTshdProcess() { + private gracefullyKillTshdProcess() { if (this.settings.platform !== 'win32') { this.tshdProcess.kill('SIGTERM'); return; @@ -447,10 +443,6 @@ function openDocsUrl() { shell.openExternal(DOCS_URL); } -function promisifyProcessExit(childProcess: ChildProcess) { - return new Promise(resolve => childProcess.once('exit', resolve)); -} - /** Shares promise returned from `promiseFn` across multiple concurrent callers. */ function sharePromise(promiseFn: () => Promise): () => Promise { let pending: Promise | undefined = undefined; diff --git a/web/packages/teleterm/src/mainProcess/processKiller/index.ts b/web/packages/teleterm/src/mainProcess/processKiller/index.ts new file mode 100644 index 0000000000000..03fc40d5b3ec5 --- /dev/null +++ b/web/packages/teleterm/src/mainProcess/processKiller/index.ts @@ -0,0 +1,17 @@ +/** + * Copyright 2023 Gravitational, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export * from './processKiller'; diff --git a/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts b/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts new file mode 100644 index 0000000000000..e15927f06629a --- /dev/null +++ b/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts @@ -0,0 +1,68 @@ +/** + * Copyright 2023 Gravitational, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { fork } from 'node:child_process'; +import path from 'node:path'; + +import Logger, { NullService } from 'teleterm/logger'; + +import { killProcess } from './processKiller'; + +beforeAll(() => { + Logger.init(new NullService()); +}); + +test('kills a process gracefully when possible', async () => { + const process = fork(path.join(__dirname, 'testProcess.mjs'), { + silent: true, + }); + + await killProcess(process); + + expect(process.killed).toBeTruthy(); + expect(process.signalCode).toBe('SIGTERM'); +}); + +test('kills a process using SIGTERM when a graceful kill did not work', async () => { + const process = fork( + path.join(__dirname, 'testProcess.mjs'), + ['ignore-sigterm'], + { + silent: true, + } + ); + + // wait for the process to start and register callbacks + await new Promise(resolve => process.stdout.once('data', resolve)); + + await killProcess(process, 1_000); + + expect(process.killed).toBeTruthy(); + expect(process.signalCode).toBe('SIGKILL'); +}); + +test('killing a process that has already stopped is noop', async () => { + const process = fork(path.join(__dirname, 'testProcess-nonExisting.mjs'), { + silent: true, + }); + + // wait for the process + await new Promise(resolve => process.once('close', resolve)); + await killProcess(process, 1_000); + + expect(process.exitCode).toBe(1); + expect(process.signalCode).toBeNull(); +}); diff --git a/web/packages/teleterm/src/mainProcess/processKiller/processKiller.ts b/web/packages/teleterm/src/mainProcess/processKiller/processKiller.ts new file mode 100644 index 0000000000000..bfc6e4f594b9e --- /dev/null +++ b/web/packages/teleterm/src/mainProcess/processKiller/processKiller.ts @@ -0,0 +1,67 @@ +/** + * Copyright 2023 Gravitational, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { ChildProcess } from 'node:child_process'; +import { setTimeout } from 'node:timers/promises'; + +import Logger from 'teleterm/logger'; + +const logger = new Logger('processKiller'); + +/** + * Tries to kill a process in a graceful way - by sending a SIGTERM signal, or using + * {@link gracefullyKill} function if provided. + * If the process doesn't close within the specified {@link timeout}, a SIGKILL signal is sent. + */ +export async function killProcess( + process: ChildProcess, + timeout = 5_000, + gracefullyKill: (process: ChildProcess) => void = process => + process.kill('SIGTERM') +): Promise { + if (!isProcessRunning(process)) { + logger.info(`Process is not running. Nothing to kill.`); + return; + } + + const processClose = promisifyProcessClose(process); + + async function startKillingSequence(): Promise { + gracefullyKill(process); + + await setTimeout(timeout); + + if (isProcessRunning(process)) { + const timeoutInSeconds = timeout / 1_000; + logger.error( + `Process ${process.spawnfile} did not close within ${timeoutInSeconds} seconds. Sending SIGKILL.` + ); + process.kill('SIGKILL'); + } + } + + startKillingSequence(); + + await processClose; +} + +function promisifyProcessClose(childProcess: ChildProcess): Promise { + return new Promise(resolve => childProcess.once('close', resolve)); +} + +function isProcessRunning(process: ChildProcess): boolean { + return process.exitCode === null && process.signalCode === null; +} diff --git a/web/packages/teleterm/src/mainProcess/processKiller/testProcess.mjs b/web/packages/teleterm/src/mainProcess/processKiller/testProcess.mjs new file mode 100644 index 0000000000000..b52c51a778787 --- /dev/null +++ b/web/packages/teleterm/src/mainProcess/processKiller/testProcess.mjs @@ -0,0 +1,26 @@ +/** + * Copyright 2023 Gravitational, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import process from 'node:process'; +import { setTimeout } from 'node:timers/promises'; + +const ignoreSigterm = !!process.argv[2]; +if (ignoreSigterm) { + process.on('SIGTERM', () => {}); +} + +console.log('READY'); +await setTimeout(10_000); \ No newline at end of file From 1e3a5bd040172a4a55077efa55533bf84a8c89b9 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Wed, 26 Jul 2023 14:56:37 +0200 Subject: [PATCH 10/31] Spawn a real process in `agentRunner` tests --- .../src/mainProcess/agentRunner.test.ts | 126 ++++++++++-------- .../teleterm/src/mainProcess/agentRunner.ts | 22 ++- .../src/mainProcess/agentTestProcess.mjs | 21 +++ 3 files changed, 103 insertions(+), 66 deletions(-) create mode 100755 web/packages/teleterm/src/mainProcess/agentTestProcess.mjs diff --git a/web/packages/teleterm/src/mainProcess/agentRunner.test.ts b/web/packages/teleterm/src/mainProcess/agentRunner.test.ts index f3a17ce64cd1b..d71c40169b861 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner.test.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner.test.ts @@ -14,9 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import childProcess, { ChildProcess } from 'node:child_process'; -import { EventEmitter } from 'node:events'; -import { PassThrough } from 'node:stream'; +import path from 'node:path'; import Logger, { NullService } from 'teleterm/logger'; import { RootClusterUri } from 'teleterm/ui/uri'; @@ -26,41 +24,15 @@ import { makeRuntimeSettings } from './fixtures/mocks'; import { AgentRunner } from './agentRunner'; import { AgentProcessState } from './types'; -jest.mock('node:child_process'); - -let eventEmitter: EventEmitter; -let childProcessMock: ChildProcess; beforeEach(() => { Logger.init(new NullService()); - - eventEmitter = new EventEmitter(); - childProcessMock = { - stderr: new PassThrough(), - once: (event, listener) => { - eventEmitter.once(event, listener); - return this; - }, - on: (event, listener) => { - eventEmitter.on(event, listener); - return this; - }, - off: (event, listener) => { - eventEmitter.off(event, listener); - return this; - }, - kill: jest.fn().mockImplementation(() => { - eventEmitter.emit('exit', 0); - }), - } as unknown as ChildProcess; - - jest.spyOn(childProcess, 'spawn').mockReturnValue(childProcessMock); }); const userDataDir = '/Users/test/Application Data/Teleport Connect'; -const agentBinaryPath = '/Users/test/Caches/Teleport Connect/teleport/teleport'; +const agentBinaryPath = path.join(__dirname, 'agentTestProcess.mjs'); const rootClusterUri: RootClusterUri = '/clusters/cluster.local'; -test('agent process starts with correct arguments', () => { +test('agent process starts with correct arguments', async () => { const agentRunner = new AgentRunner( makeRuntimeSettings({ agentBinaryPath, @@ -68,16 +40,21 @@ test('agent process starts with correct arguments', () => { }), () => {} ); - agentRunner.start(rootClusterUri); - expect(childProcess.spawn).toHaveBeenCalledWith( - agentBinaryPath, - ['start', `--config=${userDataDir}/agents/cluster.local/config.yaml`], - expect.anything() - ); + try { + const agentProcess = await agentRunner.start(rootClusterUri); + + expect(agentProcess.spawnargs).toEqual([ + agentBinaryPath, + 'start', + `--config=${userDataDir}/agents/cluster.local/config.yaml`, + ]); + } finally { + await agentRunner.killAll(); + } }); -test('previous agent process is killed when a new one is started', () => { +test('previous agent process is killed when a new one is started', async () => { const agentRunner = new AgentRunner( makeRuntimeSettings({ agentBinaryPath, @@ -85,13 +62,18 @@ test('previous agent process is killed when a new one is started', () => { }), () => {} ); - agentRunner.start(rootClusterUri); - agentRunner.start(rootClusterUri); - expect(childProcessMock.kill).toHaveBeenCalledWith('SIGKILL'); + try { + const firstProcess = await agentRunner.start(rootClusterUri); + await agentRunner.start(rootClusterUri); + + expect(firstProcess.killed).toBeTruthy(); + } finally { + await agentRunner.killAll(); + } }); -test('status updates are sent', () => { +test('status updates are sent on a successful start', async () => { const updateSender = jest.fn(); const agentRunner = new AgentRunner( makeRuntimeSettings({ @@ -101,21 +83,47 @@ test('status updates are sent', () => { updateSender ); - agentRunner.start(rootClusterUri); - expect(updateSender).toHaveBeenCalledWith(rootClusterUri, { - status: 'running', - } as AgentProcessState); - - const error = new Error('unknown error'); - eventEmitter.emit('error', error); - expect(updateSender).toHaveBeenCalledWith(rootClusterUri, { - status: 'error', - message: `${error}`, - } as AgentProcessState); - - agentRunner.kill(rootClusterUri); - expect(updateSender).toHaveBeenCalledWith(rootClusterUri, { - status: 'exited', - code: 0, - } as AgentProcessState); + try { + const agentProcess = await agentRunner.start(rootClusterUri); + await new Promise(resolve => agentProcess.on('spawn', resolve)); + expect(updateSender).toHaveBeenCalledWith(rootClusterUri, { + status: 'running', + } as AgentProcessState); + + await agentRunner.kill(rootClusterUri); + expect(updateSender).toHaveBeenCalledWith(rootClusterUri, { + status: 'exited', + code: null, + signal: 'SIGTERM', + } as AgentProcessState); + + expect(updateSender).toHaveBeenCalledTimes(2); + } finally { + await agentRunner.killAll(); + } +}); + +test('status updates are sent on a failed start', async () => { + const updateSender = jest.fn(); + const nonExisingPath = path.join(__dirname, 'agentTestProcess-nonExisting.mjs'); + const agentRunner = new AgentRunner( + makeRuntimeSettings({ + agentBinaryPath: nonExisingPath, + userDataDir, + }), + updateSender + ); + + try { + const agentProcess = await agentRunner.start(rootClusterUri); + await new Promise(resolve => agentProcess.on('error', resolve)); + + expect(updateSender).toHaveBeenCalledTimes(1); + expect(updateSender).toHaveBeenCalledWith(rootClusterUri, { + status: 'error', + message: `Error: spawn ${nonExisingPath} ENOENT`, + } as AgentProcessState); + } finally { + await agentRunner.killAll(); + } }); diff --git a/web/packages/teleterm/src/mainProcess/agentRunner.ts b/web/packages/teleterm/src/mainProcess/agentRunner.ts index f690bbc923c52..310b51f233781 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner.ts @@ -42,7 +42,7 @@ export class AgentRunner { * Starts a new agent process. * If an existing process exists for the given root cluster, the old one will be killed. */ - async start(rootClusterUri: RootClusterUri): Promise { + async start(rootClusterUri: RootClusterUri): Promise { if (this.agentProcesses.has(rootClusterUri)) { await this.kill(rootClusterUri); this.logger.warn(`Killed agent process for ${rootClusterUri}`); @@ -69,12 +69,10 @@ export class AgentRunner { env: process.env, }); - this.sendProcessState(rootClusterUri, { - status: 'running', - }); - this.addListeners(rootClusterUri, agentProcess); this.agentProcesses.set(rootClusterUri, agentProcess); + + return agentProcess; } async kill(rootClusterUri: RootClusterUri): Promise { @@ -104,7 +102,15 @@ export class AgentRunner { stderrOutput = limitProcessOutputLines(stderrOutput); }); + const spawnHandler = () => { + this.sendProcessState(rootClusterUri, { + status: 'running', + }); + }; + const errorHandler = (error: Error) => { + process.off('spawn', spawnHandler); + this.sendProcessState(rootClusterUri, { status: 'error', message: `${error}`, @@ -115,17 +121,19 @@ export class AgentRunner { code: number | null, signal: NodeJS.Signals | null ) => { - // Remove error handler when the process exits. + // Remove handlers when the process exits. process.off('error', errorHandler); + process.off('spawn', spawnHandler); this.sendProcessState(rootClusterUri, { status: 'exited', code, signal, - stackTrace: code !== 0 ? stderrOutput : undefined, + stackTrace: signal !== 'SIGTERM' ? stderrOutput : undefined, }); }; + process.once('spawn', spawnHandler); process.once('error', errorHandler); process.once('exit', exitHandler); } diff --git a/web/packages/teleterm/src/mainProcess/agentTestProcess.mjs b/web/packages/teleterm/src/mainProcess/agentTestProcess.mjs new file mode 100755 index 0000000000000..bdf461abea9f1 --- /dev/null +++ b/web/packages/teleterm/src/mainProcess/agentTestProcess.mjs @@ -0,0 +1,21 @@ +#!/usr/bin/env node +/** + * Copyright 2023 Gravitational, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + + +import { setTimeout } from 'node:timers/promises'; + +await setTimeout(10_000); \ No newline at end of file From b32f4056bd889bb0307ceecafa881431bb73f9f4 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Wed, 26 Jul 2023 15:11:59 +0200 Subject: [PATCH 11/31] Keep `agentRunner` files in a single directory --- .../{ => agentRunner}/agentRunner.test.ts | 9 ++++++--- .../{ => agentRunner}/agentRunner.ts | 6 +++--- .../{ => agentRunner}/agentTestProcess.mjs | 0 .../src/mainProcess/agentRunner/index.ts | 17 +++++++++++++++++ 4 files changed, 26 insertions(+), 6 deletions(-) rename web/packages/teleterm/src/mainProcess/{ => agentRunner}/agentRunner.test.ts (94%) rename web/packages/teleterm/src/mainProcess/{ => agentRunner}/agentRunner.ts (95%) rename web/packages/teleterm/src/mainProcess/{ => agentRunner}/agentTestProcess.mjs (100%) create mode 100644 web/packages/teleterm/src/mainProcess/agentRunner/index.ts diff --git a/web/packages/teleterm/src/mainProcess/agentRunner.test.ts b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts similarity index 94% rename from web/packages/teleterm/src/mainProcess/agentRunner.test.ts rename to web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts index d71c40169b861..57d4a0466a7c4 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner.test.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts @@ -19,10 +19,10 @@ import path from 'node:path'; import Logger, { NullService } from 'teleterm/logger'; import { RootClusterUri } from 'teleterm/ui/uri'; -import { makeRuntimeSettings } from './fixtures/mocks'; +import { makeRuntimeSettings } from '../fixtures/mocks'; +import { AgentProcessState } from '../types'; import { AgentRunner } from './agentRunner'; -import { AgentProcessState } from './types'; beforeEach(() => { Logger.init(new NullService()); @@ -105,7 +105,10 @@ test('status updates are sent on a successful start', async () => { test('status updates are sent on a failed start', async () => { const updateSender = jest.fn(); - const nonExisingPath = path.join(__dirname, 'agentTestProcess-nonExisting.mjs'); + const nonExisingPath = path.join( + __dirname, + 'agentTestProcess-nonExisting.mjs' + ); const agentRunner = new AgentRunner( makeRuntimeSettings({ agentBinaryPath: nonExisingPath, diff --git a/web/packages/teleterm/src/mainProcess/agentRunner.ts b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts similarity index 95% rename from web/packages/teleterm/src/mainProcess/agentRunner.ts rename to web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts index 310b51f233781..380fbc3c2d946 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts @@ -20,9 +20,9 @@ import os from 'node:os'; import Logger from 'teleterm/logger'; import { RootClusterUri } from 'teleterm/ui/uri'; -import { generateAgentConfigPaths } from './createAgentConfigFile'; -import { AgentProcessState, RuntimeSettings } from './types'; -import { killProcess } from './processKiller'; +import { generateAgentConfigPaths } from '../createAgentConfigFile'; +import { AgentProcessState, RuntimeSettings } from '../types'; +import { killProcess } from '../processKiller'; const MAX_STDERR_LINES = 10; diff --git a/web/packages/teleterm/src/mainProcess/agentTestProcess.mjs b/web/packages/teleterm/src/mainProcess/agentRunner/agentTestProcess.mjs similarity index 100% rename from web/packages/teleterm/src/mainProcess/agentTestProcess.mjs rename to web/packages/teleterm/src/mainProcess/agentRunner/agentTestProcess.mjs diff --git a/web/packages/teleterm/src/mainProcess/agentRunner/index.ts b/web/packages/teleterm/src/mainProcess/agentRunner/index.ts new file mode 100644 index 0000000000000..a6e9f12447ef9 --- /dev/null +++ b/web/packages/teleterm/src/mainProcess/agentRunner/index.ts @@ -0,0 +1,17 @@ +/** + * Copyright 2023 Gravitational, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export * from './agentRunner'; From b94bd7eaf1ba430edfb187b6b3536474ea130016 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Wed, 26 Jul 2023 15:12:13 +0200 Subject: [PATCH 12/31] Catch errors from `deleteToken` --- .../DocumentConnectMyComputerSetup.tsx | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx index de6c29d49e0a9..1a3e89e72faab 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx @@ -16,10 +16,7 @@ limitations under the License. import React, { useCallback, useEffect, useRef, useState } from 'react'; import { Box, ButtonPrimary, Flex, Text } from 'design'; -import { - makeEmptyAttempt, - useAsync, -} from 'shared/hooks/useAsync'; +import { makeEmptyAttempt, useAsync } from 'shared/hooks/useAsync'; import * as Alerts from 'design/Alert'; import { CircleCheck, CircleCross, CirclePlay, Spinner } from 'design/Icon'; @@ -29,12 +26,15 @@ import Document from 'teleterm/ui/Document'; import { useWorkspaceContext } from 'teleterm/ui/Documents'; import { retryWithRelogin } from 'teleterm/ui/utils'; import { useConnectMyComputerContext } from 'teleterm/ui/ConnectMyComputer'; +import Logger from 'teleterm/logger'; interface DocumentConnectMyComputerSetupProps { visible: boolean; doc: types.DocumentConnectMyComputerSetup; } +const logger = new Logger('DocumentConnectMyComputerSetup'); + export function DocumentConnectMyComputerSetup( props: DocumentConnectMyComputerSetupProps ) { @@ -150,10 +150,14 @@ function AgentSetup() { throw new Error('Node token is empty'); } await runAgentAndWaitForNodeToJoin(); - await ctx.connectMyComputerService.deleteToken( - cluster.uri, - nodeToken.current - ); + try { + await ctx.connectMyComputerService.deleteToken( + cluster.uri, + nodeToken.current + ); + } catch (error) { + logger.error('Failed to delete token', error); + } }, [ runAgentAndWaitForNodeToJoin, ctx.connectMyComputerService, From 72eaf48cbeec6de3ba4ef61097fc0bfa3671e3ab Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Wed, 26 Jul 2023 15:13:16 +0200 Subject: [PATCH 13/31] Remove `env: process.env` --- web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts index 380fbc3c2d946..4b9b78a8480d8 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts @@ -66,7 +66,6 @@ export class AgentRunner { const agentProcess = spawn(agentBinaryPath, args, { windowsHide: true, - env: process.env, }); this.addListeners(rootClusterUri, agentProcess); From 50b01efe5f9298094c4092ee9ba36bbcf1c14784 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Fri, 28 Jul 2023 13:55:33 +0200 Subject: [PATCH 14/31] Match on "access denied" when checking error from `deleteToken` --- .../DocumentConnectMyComputerSetup.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx index 1a3e89e72faab..3f3a788025dd9 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/DocumentConnectMyComputerSetup/DocumentConnectMyComputerSetup.tsx @@ -105,7 +105,7 @@ function AgentSetup() { ); certsReloaded = response.certsReloaded; } catch (error) { - if ((error.message as string)?.includes('access denied')) { + if (isAccessDeniedError(error)) { throw new Error( 'Access denied. Contact your administrator for permissions to manage users and roles.' ); @@ -156,7 +156,11 @@ function AgentSetup() { nodeToken.current ); } catch (error) { - logger.error('Failed to delete token', error); + // the user may not have permissions to remove the token, but it will expire in a few minutes anyway + if (isAccessDeniedError(error)) { + logger.error('Access denied when deleting a token.', error); + } + throw error; } }, [ runAgentAndWaitForNodeToJoin, @@ -289,3 +293,7 @@ function AgentSetup() { ); } + +function isAccessDeniedError(error: Error): boolean { + return (error.message as string)?.includes('access denied'); +} From a1ca16fe783997bcacb41ce653cf716f67561d03 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Fri, 28 Jul 2023 14:03:55 +0200 Subject: [PATCH 15/31] Reject when an agent process fails to start in test --- .../src/mainProcess/agentRunner/agentRunner.test.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts index 57d4a0466a7c4..bdc10c6e6458d 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts @@ -85,7 +85,16 @@ test('status updates are sent on a successful start', async () => { try { const agentProcess = await agentRunner.start(rootClusterUri); - await new Promise(resolve => agentProcess.on('spawn', resolve)); + await new Promise((resolve, reject) => { + const timeout = setTimeout( + () => reject('Process start timed out.'), + 4_000 + ); + agentProcess.once('spawn', () => { + resolve(undefined); + clearTimeout(timeout); + }); + }); expect(updateSender).toHaveBeenCalledWith(rootClusterUri, { status: 'running', } as AgentProcessState); From cdc0e8fc353ad5add3afe447e3dfbdc1af3792f3 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Fri, 28 Jul 2023 14:04:19 +0200 Subject: [PATCH 16/31] Match only on "ENOENT" --- .../teleterm/src/mainProcess/agentRunner/agentRunner.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts index bdc10c6e6458d..ecb6ebb10b83b 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts @@ -133,7 +133,7 @@ test('status updates are sent on a failed start', async () => { expect(updateSender).toHaveBeenCalledTimes(1); expect(updateSender).toHaveBeenCalledWith(rootClusterUri, { status: 'error', - message: `Error: spawn ${nonExisingPath} ENOENT`, + message: expect.stringContaining('ENOENT'), } as AgentProcessState); } finally { await agentRunner.killAll(); From 00234d760f91252bfd62bcb609c0c0c3be42bc03 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Fri, 28 Jul 2023 14:05:02 +0200 Subject: [PATCH 17/31] Correct test name ("SIGTERM" -> "SIGKILL") --- .../src/mainProcess/processKiller/processKiller.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts b/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts index e15927f06629a..3c8759b15cd77 100644 --- a/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts +++ b/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts @@ -36,7 +36,7 @@ test('kills a process gracefully when possible', async () => { expect(process.signalCode).toBe('SIGTERM'); }); -test('kills a process using SIGTERM when a graceful kill did not work', async () => { +test('kills a process using SIGKILL when a graceful kill did not work', async () => { const process = fork( path.join(__dirname, 'testProcess.mjs'), ['ignore-sigterm'], From ec735919d5c9b2e4cbe5173b8b25358b9149a712 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Fri, 28 Jul 2023 15:08:07 +0200 Subject: [PATCH 18/31] Test terminating the process and then trying to kill it --- .../processKiller/processKiller.test.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts b/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts index 3c8759b15cd77..2c79bfb1d51bf 100644 --- a/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts +++ b/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts @@ -54,10 +54,11 @@ test('kills a process using SIGKILL when a graceful kill did not work', async () expect(process.signalCode).toBe('SIGKILL'); }); -test('killing a process that has already stopped is noop', async () => { +test('killing a process that failed to start is noop', async () => { const process = fork(path.join(__dirname, 'testProcess-nonExisting.mjs'), { silent: true, }); + jest.spyOn(process, 'kill'); // wait for the process await new Promise(resolve => process.once('close', resolve)); @@ -65,4 +66,20 @@ test('killing a process that has already stopped is noop', async () => { expect(process.exitCode).toBe(1); expect(process.signalCode).toBeNull(); + expect(process.kill).toHaveBeenCalledTimes(0); +}); + +test('killing a process that has been already killed is noop', async () => { + const process = fork(path.join(__dirname, 'testProcess.mjs'), { + silent: true, + }); + jest.spyOn(process, 'kill'); + + process.kill('SIGTERM'); + await new Promise(resolve => process.once('exit', resolve)); + expect(process.killed).toBeTruthy(); + expect(process.signalCode).toBe('SIGTERM'); + + await killProcess(process, 1_000); + expect(process.kill).toHaveBeenCalledTimes(1); // called only once, in the test }); From 0a6a5e63d48cee367b3aee1d7e876efaab66d7b4 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Fri, 28 Jul 2023 15:10:48 +0200 Subject: [PATCH 19/31] Wait for "exit" event instead of "close" --- .../processKiller/processKiller.test.ts | 2 +- .../src/mainProcess/processKiller/processKiller.ts | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts b/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts index 2c79bfb1d51bf..dd550ea49fe30 100644 --- a/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts +++ b/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts @@ -61,7 +61,7 @@ test('killing a process that failed to start is noop', async () => { jest.spyOn(process, 'kill'); // wait for the process - await new Promise(resolve => process.once('close', resolve)); + await new Promise(resolve => process.once('exit', resolve)); await killProcess(process, 1_000); expect(process.exitCode).toBe(1); diff --git a/web/packages/teleterm/src/mainProcess/processKiller/processKiller.ts b/web/packages/teleterm/src/mainProcess/processKiller/processKiller.ts index bfc6e4f594b9e..258748d574345 100644 --- a/web/packages/teleterm/src/mainProcess/processKiller/processKiller.ts +++ b/web/packages/teleterm/src/mainProcess/processKiller/processKiller.ts @@ -33,11 +33,13 @@ export async function killProcess( process.kill('SIGTERM') ): Promise { if (!isProcessRunning(process)) { - logger.info(`Process is not running. Nothing to kill.`); + logger.info( + `Process ${process.spawnfile} is not running. Nothing to kill.` + ); return; } - const processClose = promisifyProcessClose(process); + const processExit = promisifyProcessExit(process); async function startKillingSequence(): Promise { gracefullyKill(process); @@ -47,7 +49,7 @@ export async function killProcess( if (isProcessRunning(process)) { const timeoutInSeconds = timeout / 1_000; logger.error( - `Process ${process.spawnfile} did not close within ${timeoutInSeconds} seconds. Sending SIGKILL.` + `Process ${process.spawnfile} did not exit within ${timeoutInSeconds} seconds. Sending SIGKILL.` ); process.kill('SIGKILL'); } @@ -55,11 +57,11 @@ export async function killProcess( startKillingSequence(); - await processClose; + await processExit; } -function promisifyProcessClose(childProcess: ChildProcess): Promise { - return new Promise(resolve => childProcess.once('close', resolve)); +function promisifyProcessExit(childProcess: ChildProcess): Promise { + return new Promise(resolve => childProcess.once('exit', resolve)); } function isProcessRunning(process: ChildProcess): boolean { From 103483bcf0201d3249fdf07ae3364914a8e95b84 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Fri, 28 Jul 2023 15:12:39 +0200 Subject: [PATCH 20/31] Rename `killProcess` to `terminateWithTimeout` --- .../src/mainProcess/agentRunner/agentRunner.ts | 6 +++--- web/packages/teleterm/src/mainProcess/mainProcess.ts | 6 +++--- .../{processKiller => terminateWithTimeout}/index.ts | 2 +- .../terminateWithTimeout.test.ts} | 10 +++++----- .../terminateWithTimeout.ts} | 2 +- .../testProcess.mjs | 0 6 files changed, 13 insertions(+), 13 deletions(-) rename web/packages/teleterm/src/mainProcess/{processKiller => terminateWithTimeout}/index.ts (93%) rename web/packages/teleterm/src/mainProcess/{processKiller/processKiller.test.ts => terminateWithTimeout/terminateWithTimeout.test.ts} (90%) rename web/packages/teleterm/src/mainProcess/{processKiller/processKiller.ts => terminateWithTimeout/terminateWithTimeout.ts} (97%) rename web/packages/teleterm/src/mainProcess/{processKiller => terminateWithTimeout}/testProcess.mjs (100%) diff --git a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts index 4b9b78a8480d8..ef5ed6f09ecfd 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts @@ -22,7 +22,7 @@ import { RootClusterUri } from 'teleterm/ui/uri'; import { generateAgentConfigPaths } from '../createAgentConfigFile'; import { AgentProcessState, RuntimeSettings } from '../types'; -import { killProcess } from '../processKiller'; +import { terminateWithTimeout } from '../terminateWithTimeout'; const MAX_STDERR_LINES = 10; @@ -75,7 +75,7 @@ export class AgentRunner { } async kill(rootClusterUri: RootClusterUri): Promise { - await killProcess(this.agentProcesses.get(rootClusterUri)); + await terminateWithTimeout(this.agentProcesses.get(rootClusterUri)); this.agentProcesses.delete(rootClusterUri); } @@ -83,7 +83,7 @@ export class AgentRunner { const processes = Array.from(this.agentProcesses.entries()); await Promise.all( processes.map(async ([rootClusterUri, agent]) => { - await killProcess(agent); + await terminateWithTimeout(agent); this.agentProcesses.delete(rootClusterUri); }) ); diff --git a/web/packages/teleterm/src/mainProcess/mainProcess.ts b/web/packages/teleterm/src/mainProcess/mainProcess.ts index 16d9599c63e66..9f35c752b2939 100644 --- a/web/packages/teleterm/src/mainProcess/mainProcess.ts +++ b/web/packages/teleterm/src/mainProcess/mainProcess.ts @@ -50,7 +50,7 @@ import { WindowsManager } from './windowsManager'; import { downloadAgent, FileDownloader } from './agentDownloader'; import { createAgentConfigFile } from './createAgentConfigFile'; import { AgentRunner } from './agentRunner'; -import { killProcess } from './processKiller'; +import { terminateWithTimeout } from './terminateWithTimeout'; import type { AgentConfigFileClusterProperties } from './createAgentConfigFile'; @@ -110,10 +110,10 @@ export default class MainProcess { async dispose(): Promise { await Promise.all([ // sending usage events on tshd shutdown has 10-seconds timeout - killProcess(this.tshdProcess, 10_000, () => { + terminateWithTimeout(this.tshdProcess, 10_000, () => { this.gracefullyKillTshdProcess(); }), - killProcess(this.sharedProcess), + terminateWithTimeout(this.sharedProcess), this.agentRunner.killAll(), ]); } diff --git a/web/packages/teleterm/src/mainProcess/processKiller/index.ts b/web/packages/teleterm/src/mainProcess/terminateWithTimeout/index.ts similarity index 93% rename from web/packages/teleterm/src/mainProcess/processKiller/index.ts rename to web/packages/teleterm/src/mainProcess/terminateWithTimeout/index.ts index 03fc40d5b3ec5..a7f317e0c541e 100644 --- a/web/packages/teleterm/src/mainProcess/processKiller/index.ts +++ b/web/packages/teleterm/src/mainProcess/terminateWithTimeout/index.ts @@ -14,4 +14,4 @@ * limitations under the License. */ -export * from './processKiller'; +export * from './terminateWithTimeout'; diff --git a/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts b/web/packages/teleterm/src/mainProcess/terminateWithTimeout/terminateWithTimeout.test.ts similarity index 90% rename from web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts rename to web/packages/teleterm/src/mainProcess/terminateWithTimeout/terminateWithTimeout.test.ts index dd550ea49fe30..5fcf7110aeb9c 100644 --- a/web/packages/teleterm/src/mainProcess/processKiller/processKiller.test.ts +++ b/web/packages/teleterm/src/mainProcess/terminateWithTimeout/terminateWithTimeout.test.ts @@ -19,7 +19,7 @@ import path from 'node:path'; import Logger, { NullService } from 'teleterm/logger'; -import { killProcess } from './processKiller'; +import { terminateWithTimeout } from './terminateWithTimeout'; beforeAll(() => { Logger.init(new NullService()); @@ -30,7 +30,7 @@ test('kills a process gracefully when possible', async () => { silent: true, }); - await killProcess(process); + await terminateWithTimeout(process); expect(process.killed).toBeTruthy(); expect(process.signalCode).toBe('SIGTERM'); @@ -48,7 +48,7 @@ test('kills a process using SIGKILL when a graceful kill did not work', async () // wait for the process to start and register callbacks await new Promise(resolve => process.stdout.once('data', resolve)); - await killProcess(process, 1_000); + await terminateWithTimeout(process, 1_000); expect(process.killed).toBeTruthy(); expect(process.signalCode).toBe('SIGKILL'); @@ -62,7 +62,7 @@ test('killing a process that failed to start is noop', async () => { // wait for the process await new Promise(resolve => process.once('exit', resolve)); - await killProcess(process, 1_000); + await terminateWithTimeout(process, 1_000); expect(process.exitCode).toBe(1); expect(process.signalCode).toBeNull(); @@ -80,6 +80,6 @@ test('killing a process that has been already killed is noop', async () => { expect(process.killed).toBeTruthy(); expect(process.signalCode).toBe('SIGTERM'); - await killProcess(process, 1_000); + await terminateWithTimeout(process, 1_000); expect(process.kill).toHaveBeenCalledTimes(1); // called only once, in the test }); diff --git a/web/packages/teleterm/src/mainProcess/processKiller/processKiller.ts b/web/packages/teleterm/src/mainProcess/terminateWithTimeout/terminateWithTimeout.ts similarity index 97% rename from web/packages/teleterm/src/mainProcess/processKiller/processKiller.ts rename to web/packages/teleterm/src/mainProcess/terminateWithTimeout/terminateWithTimeout.ts index 258748d574345..0030ec74dca44 100644 --- a/web/packages/teleterm/src/mainProcess/processKiller/processKiller.ts +++ b/web/packages/teleterm/src/mainProcess/terminateWithTimeout/terminateWithTimeout.ts @@ -26,7 +26,7 @@ const logger = new Logger('processKiller'); * {@link gracefullyKill} function if provided. * If the process doesn't close within the specified {@link timeout}, a SIGKILL signal is sent. */ -export async function killProcess( +export async function terminateWithTimeout( process: ChildProcess, timeout = 5_000, gracefullyKill: (process: ChildProcess) => void = process => diff --git a/web/packages/teleterm/src/mainProcess/processKiller/testProcess.mjs b/web/packages/teleterm/src/mainProcess/terminateWithTimeout/testProcess.mjs similarity index 100% rename from web/packages/teleterm/src/mainProcess/processKiller/testProcess.mjs rename to web/packages/teleterm/src/mainProcess/terminateWithTimeout/testProcess.mjs From 53e2cc56912fc9fdd85fd82fc160a061a892360b Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Mon, 31 Jul 2023 12:56:45 +0200 Subject: [PATCH 21/31] Add `getAgentState` method to synchronously get the agent state --- .../agentRunner/agentRunner.test.ts | 22 +++++--- .../mainProcess/agentRunner/agentRunner.ts | 52 +++++++++++++++---- .../src/mainProcess/fixtures/mocks.ts | 5 ++ .../teleterm/src/mainProcess/mainProcess.ts | 12 +++++ .../src/mainProcess/mainProcessClient.ts | 6 +++ .../connectMyComputerContext.tsx | 11 ++-- 6 files changed, 88 insertions(+), 20 deletions(-) diff --git a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts index ecb6ebb10b83b..7224d3c4a8910 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts @@ -84,7 +84,11 @@ test('status updates are sent on a successful start', async () => { ); try { + expect(agentRunner.getState(rootClusterUri)).toBeUndefined(); const agentProcess = await agentRunner.start(rootClusterUri); + expect(agentRunner.getState(rootClusterUri)).toStrictEqual({ + status: 'not-started', + } as AgentProcessState); await new Promise((resolve, reject) => { const timeout = setTimeout( () => reject('Process start timed out.'), @@ -95,16 +99,18 @@ test('status updates are sent on a successful start', async () => { clearTimeout(timeout); }); }); - expect(updateSender).toHaveBeenCalledWith(rootClusterUri, { - status: 'running', - } as AgentProcessState); + const runningState: AgentProcessState = { status: 'running' }; + expect(agentRunner.getState(rootClusterUri)).toStrictEqual(runningState); + expect(updateSender).toHaveBeenCalledWith(rootClusterUri, runningState); await agentRunner.kill(rootClusterUri); - expect(updateSender).toHaveBeenCalledWith(rootClusterUri, { + const exitedState: AgentProcessState = { status: 'exited', code: null, signal: 'SIGTERM', - } as AgentProcessState); + }; + expect(agentRunner.getState(rootClusterUri)).toBeUndefined(); // the agent has been killed and removed + expect(updateSender).toHaveBeenCalledWith(rootClusterUri, exitedState); expect(updateSender).toHaveBeenCalledTimes(2); } finally { @@ -131,10 +137,12 @@ test('status updates are sent on a failed start', async () => { await new Promise(resolve => agentProcess.on('error', resolve)); expect(updateSender).toHaveBeenCalledTimes(1); - expect(updateSender).toHaveBeenCalledWith(rootClusterUri, { + const errorState: AgentProcessState = { status: 'error', message: expect.stringContaining('ENOENT'), - } as AgentProcessState); + }; + expect(agentRunner.getState(rootClusterUri)).toStrictEqual(errorState); + expect(updateSender).toHaveBeenCalledWith(rootClusterUri, errorState); } finally { await agentRunner.killAll(); } diff --git a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts index ef5ed6f09ecfd..fa8d0e548e100 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts @@ -28,7 +28,13 @@ const MAX_STDERR_LINES = 10; export class AgentRunner { private logger = new Logger('AgentRunner'); - private agentProcesses = new Map(); + private agentProcesses = new Map< + RootClusterUri, + { + process: ChildProcess; + state: AgentProcessState; + } + >(); constructor( private settings: RuntimeSettings, @@ -45,7 +51,6 @@ export class AgentRunner { async start(rootClusterUri: RootClusterUri): Promise { if (this.agentProcesses.has(rootClusterUri)) { await this.kill(rootClusterUri); - this.logger.warn(`Killed agent process for ${rootClusterUri}`); } const { agentBinaryPath } = this.settings; @@ -61,29 +66,45 @@ export class AgentRunner { ].filter(Boolean); this.logger.info( - `Starting agent from ${agentBinaryPath} with arguments ${args.join(' ')}` + `Starting agent for ${rootClusterUri} from ${agentBinaryPath} with arguments ${args.join( + ' ' + )}` ); const agentProcess = spawn(agentBinaryPath, args, { windowsHide: true, }); + this.agentProcesses.set(rootClusterUri, { + process: agentProcess, + state: { status: 'not-started' }, + }); this.addListeners(rootClusterUri, agentProcess); - this.agentProcesses.set(rootClusterUri, agentProcess); return agentProcess; } + getState(rootClusterUri: RootClusterUri): AgentProcessState | undefined { + return this.agentProcesses.get(rootClusterUri)?.state; + } + async kill(rootClusterUri: RootClusterUri): Promise { - await terminateWithTimeout(this.agentProcesses.get(rootClusterUri)); - this.agentProcesses.delete(rootClusterUri); + const agent = this.agentProcesses.get(rootClusterUri); + if (agent) { + await terminateWithTimeout( + this.agentProcesses.get(rootClusterUri).process + ); + this.agentProcesses.delete(rootClusterUri); + this.logger.info(`Killed agent for ${rootClusterUri}`); + } + this.logger.warn(`Cannot get an agent to kill for ${rootClusterUri}`); } async killAll(): Promise { const processes = Array.from(this.agentProcesses.entries()); await Promise.all( processes.map(async ([rootClusterUri, agent]) => { - await terminateWithTimeout(agent); + await terminateWithTimeout(agent.process); this.agentProcesses.delete(rootClusterUri); }) ); @@ -102,7 +123,7 @@ export class AgentRunner { }); const spawnHandler = () => { - this.sendProcessState(rootClusterUri, { + this.updateProcessState(rootClusterUri, { status: 'running', }); }; @@ -110,7 +131,7 @@ export class AgentRunner { const errorHandler = (error: Error) => { process.off('spawn', spawnHandler); - this.sendProcessState(rootClusterUri, { + this.updateProcessState(rootClusterUri, { status: 'error', message: `${error}`, }); @@ -124,7 +145,7 @@ export class AgentRunner { process.off('error', errorHandler); process.off('spawn', spawnHandler); - this.sendProcessState(rootClusterUri, { + this.updateProcessState(rootClusterUri, { status: 'exited', code, signal, @@ -136,6 +157,17 @@ export class AgentRunner { process.once('error', errorHandler); process.once('exit', exitHandler); } + + private updateProcessState( + rootClusterUri: RootClusterUri, + state: AgentProcessState + ): void { + const agent = this.agentProcesses.get(rootClusterUri); + if (agent) { + agent.state = state; + } + this.sendProcessState(rootClusterUri, state); + } } function limitProcessOutputLines(output: string): string { diff --git a/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts b/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts index 0bde8808f0e03..8eeae16e53533 100644 --- a/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts +++ b/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts @@ -20,6 +20,7 @@ import { createMockFileStorage } from 'teleterm/services/fileStorage/fixtures/mo // teleterm/services/config/index.ts reexports the config service client which depends on electron. // Importing electron breaks the fixtures if that's done from within storybook. import { createConfigService } from 'teleterm/services/config/configService'; +import { AgentProcessState } from 'teleterm/mainProcess/types'; export class MockMainProcessClient implements MainProcessClient { configService: ReturnType; @@ -95,6 +96,10 @@ export class MockMainProcessClient implements MainProcessClient { return Promise.resolve(); } + getAgentState(): AgentProcessState { + return { status: 'not-started' }; + } + subscribeToAgentUpdate() { return { cleanup: () => undefined }; } diff --git a/web/packages/teleterm/src/mainProcess/mainProcess.ts b/web/packages/teleterm/src/mainProcess/mainProcess.ts index 9f35c752b2939..f51992cca1fec 100644 --- a/web/packages/teleterm/src/mainProcess/mainProcess.ts +++ b/web/packages/teleterm/src/mainProcess/mainProcess.ts @@ -339,6 +339,18 @@ export default class MainProcess { } ); + ipcMain.on( + 'main-process-connect-my-computer-get-agent-state', + ( + event, + args: { + rootClusterUri: RootClusterUri; + } + ) => { + event.returnValue = this.agentRunner.getState(args.rootClusterUri); + } + ); + subscribeToTerminalContextMenuEvent(); subscribeToTabContextMenuEvent(); subscribeToConfigServiceEvents(this.configService); diff --git a/web/packages/teleterm/src/mainProcess/mainProcessClient.ts b/web/packages/teleterm/src/mainProcess/mainProcessClient.ts index 36628750a7a84..197300afb7f85 100644 --- a/web/packages/teleterm/src/mainProcess/mainProcessClient.ts +++ b/web/packages/teleterm/src/mainProcess/mainProcessClient.ts @@ -92,6 +92,12 @@ export default function createMainProcessClient(): MainProcessClient { clusterProperties ); }, + getAgentState(clusterProperties: { rootClusterUri: RootClusterUri }) { + return ipcRenderer.sendSync( + 'main-process-connect-my-computer-get-agent-state', + clusterProperties + ); + }, subscribeToAgentUpdate: (rootClusterUri, listener) => { const onChange = ( _, diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx index 22be0554ddd61..cefe18f73cede 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx @@ -44,9 +44,14 @@ export const ConnectMyComputerContextProvider: FC<{ rootClusterUri: RootClusterUri; }> = props => { const { mainProcessClient, connectMyComputerService } = useAppContext(); - const [agentState, setAgentState] = useState(() => ({ - status: 'not-started', - })); + const [agentState, setAgentState] = useState( + () => + mainProcessClient.getAgentState({ + rootClusterUri: props.rootClusterUri, + }) || { + status: 'not-started', + } + ); const runAgentAndWaitForNodeToJoin = useCallback(async () => { await connectMyComputerService.runAgent(props.rootClusterUri); From 040a50fabfc4a4d972ddb8408bd30723ccdc5fad Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Mon, 31 Jul 2023 17:24:15 +0200 Subject: [PATCH 22/31] Remove space before new line --- .../src/ui/ConnectMyComputer/connectMyComputerContext.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx index cefe18f73cede..51d5ec9e7e3de 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx @@ -140,7 +140,7 @@ async function waitForAgentProcessErrors( if (agentState.status === 'error') { reject( new Error( - ['Agent process failed to start.', agentState.message].join(' \n') + ['Agent process failed to start.', agentState.message].join('\n') ) ); } From ecdf232cec8744db7ddf916f49bf547b3f97eaa1 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Mon, 31 Jul 2023 17:25:36 +0200 Subject: [PATCH 23/31] Simplify the logic in `AgentRunner` --- .../agentRunner/agentRunner.test.ts | 3 ++- .../mainProcess/agentRunner/agentRunner.ts | 21 +++++++------------ 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts index 7224d3c4a8910..b7c839b09653d 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts @@ -107,9 +107,10 @@ test('status updates are sent on a successful start', async () => { const exitedState: AgentProcessState = { status: 'exited', code: null, + stackTrace: undefined, signal: 'SIGTERM', }; - expect(agentRunner.getState(rootClusterUri)).toBeUndefined(); // the agent has been killed and removed + expect(agentRunner.getState(rootClusterUri)).toStrictEqual(exitedState); expect(updateSender).toHaveBeenCalledWith(rootClusterUri, exitedState); expect(updateSender).toHaveBeenCalledTimes(2); diff --git a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts index fa8d0e548e100..30edb23893153 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts @@ -90,22 +90,19 @@ export class AgentRunner { async kill(rootClusterUri: RootClusterUri): Promise { const agent = this.agentProcesses.get(rootClusterUri); - if (agent) { - await terminateWithTimeout( - this.agentProcesses.get(rootClusterUri).process - ); - this.agentProcesses.delete(rootClusterUri); - this.logger.info(`Killed agent for ${rootClusterUri}`); + if (!agent) { + this.logger.warn(`Cannot get an agent to kill for ${rootClusterUri}`); + return; } - this.logger.warn(`Cannot get an agent to kill for ${rootClusterUri}`); + await terminateWithTimeout(agent.process); + this.logger.info(`Killed agent for ${rootClusterUri}`); } async killAll(): Promise { - const processes = Array.from(this.agentProcesses.entries()); + const processes = Array.from(this.agentProcesses.values()); await Promise.all( - processes.map(async ([rootClusterUri, agent]) => { + processes.map(async agent => { await terminateWithTimeout(agent.process); - this.agentProcesses.delete(rootClusterUri); }) ); } @@ -163,9 +160,7 @@ export class AgentRunner { state: AgentProcessState ): void { const agent = this.agentProcesses.get(rootClusterUri); - if (agent) { - agent.state = state; - } + agent.state = state; this.sendProcessState(rootClusterUri, state); } } From 086bc1687cc8b65fc55c95e92c3b721714119170 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Mon, 31 Jul 2023 17:25:48 +0200 Subject: [PATCH 24/31] Fix TS error --- web/packages/teleterm/src/mainProcess/types.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/web/packages/teleterm/src/mainProcess/types.ts b/web/packages/teleterm/src/mainProcess/types.ts index 64a3cccb75145..b5724b8f9c9a6 100644 --- a/web/packages/teleterm/src/mainProcess/types.ts +++ b/web/packages/teleterm/src/mainProcess/types.ts @@ -98,6 +98,7 @@ export type MainProcessClient = { properties: AgentConfigFileClusterProperties ): Promise; runAgent(args: { rootClusterUri: RootClusterUri }): Promise; + getAgentState(args: { rootClusterUri: RootClusterUri }): AgentProcessState; subscribeToAgentUpdate: SubscribeToAgentUpdate; }; From 0eebf95edace0bc3533bfaf04c8757e9aa23f23e Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Tue, 1 Aug 2023 17:36:12 +0200 Subject: [PATCH 25/31] Do not send agent updates to a destroyed window --- .../teleterm/src/mainProcess/mainProcess.ts | 15 ++++++++++----- .../terminateWithTimeout/terminateWithTimeout.ts | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/web/packages/teleterm/src/mainProcess/mainProcess.ts b/web/packages/teleterm/src/mainProcess/mainProcess.ts index f51992cca1fec..4b919323eef14 100644 --- a/web/packages/teleterm/src/mainProcess/mainProcess.ts +++ b/web/packages/teleterm/src/mainProcess/mainProcess.ts @@ -90,14 +90,19 @@ export default class MainProcess { this.appStateFileStorage = opts.appStateFileStorage; this.configFileStorage = opts.configFileStorage; this.windowsManager = opts.windowsManager; - this.agentRunner = new AgentRunner(this.settings, (rootClusterUri, state) => - this.windowsManager - .getWindow() - .webContents.send( + this.agentRunner = new AgentRunner( + this.settings, + (rootClusterUri, state) => { + const window = this.windowsManager.getWindow(); + if (window.isDestroyed()) { + return; + } + window.webContents.send( 'main-process-connect-my-computer-agent-update', rootClusterUri, state - ) + ); + } ); } diff --git a/web/packages/teleterm/src/mainProcess/terminateWithTimeout/terminateWithTimeout.ts b/web/packages/teleterm/src/mainProcess/terminateWithTimeout/terminateWithTimeout.ts index 0030ec74dca44..81a4908c614e9 100644 --- a/web/packages/teleterm/src/mainProcess/terminateWithTimeout/terminateWithTimeout.ts +++ b/web/packages/teleterm/src/mainProcess/terminateWithTimeout/terminateWithTimeout.ts @@ -19,7 +19,7 @@ import { setTimeout } from 'node:timers/promises'; import Logger from 'teleterm/logger'; -const logger = new Logger('processKiller'); +const logger = new Logger('terminateWithTimeout'); /** * Tries to kill a process in a graceful way - by sending a SIGTERM signal, or using From aef7d7ea8b61ed693d2c9b48ed707bfea72a708b Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Tue, 1 Aug 2023 17:37:09 +0200 Subject: [PATCH 26/31] Add logging cluster URI and updated state --- .../teleterm/src/mainProcess/agentRunner/agentRunner.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts index 30edb23893153..3d44a9bb03599 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts @@ -159,6 +159,10 @@ export class AgentRunner { rootClusterUri: RootClusterUri, state: AgentProcessState ): void { + this.logger.info( + `Updating agent state ${rootClusterUri}: ${JSON.stringify(state)}` + ); + const agent = this.agentProcesses.get(rootClusterUri); agent.state = state; this.sendProcessState(rootClusterUri, state); From d3b858a9e4b419570553c0c8f85ac151501bfa74 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Wed, 2 Aug 2023 12:30:08 +0200 Subject: [PATCH 27/31] Catch errors that are thrown while spawning the process --- .../connectMyComputerContext.test.tsx | 52 +++++++++++ .../connectMyComputerContext.tsx | 88 +++++++++++-------- 2 files changed, 101 insertions(+), 39 deletions(-) create mode 100644 web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.test.tsx diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.test.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.test.tsx new file mode 100644 index 0000000000000..c97b71d13cade --- /dev/null +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.test.tsx @@ -0,0 +1,52 @@ +import { EventEmitter } from 'node:events'; + +import React from 'react'; +import { renderHook } from '@testing-library/react-hooks'; + +import { MockAppContextProvider } from 'teleterm/ui/fixtures/MockAppContextProvider'; +import { MockAppContext } from 'teleterm/ui/fixtures/mocks'; +import { WorkspaceContextProvider } from 'teleterm/ui/Documents'; +import { AgentProcessState } from 'teleterm/mainProcess/types'; + +import { + ConnectMyComputerContextProvider, + useConnectMyComputerContext, +} from './connectMyComputerContext'; + +test('runAgentAndWaitForNodeToJoin re-throws errors that are thrown while spawning the process', async () => { + const mockedAppContext = new MockAppContext({}); + const eventEmitter = new EventEmitter(); + const errorStatus: AgentProcessState = { status: 'error', message: 'ENOENT' }; + jest + .spyOn(mockedAppContext.mainProcessClient, 'getAgentState') + .mockImplementation(() => errorStatus); + jest + .spyOn(mockedAppContext.connectMyComputerService, 'runAgent') + .mockImplementation(async () => { + // the error is emitted before the function resolves + eventEmitter.emit('', errorStatus); + return; + }); + jest + .spyOn(mockedAppContext.mainProcessClient, 'subscribeToAgentUpdate') + .mockImplementation((rootClusterUri, listener) => { + eventEmitter.on('', listener); + return { cleanup: () => eventEmitter.off('', listener) }; + }); + + const { result } = renderHook(() => useConnectMyComputerContext(), { + wrapper: ({ children }) => ( + + + + {children} + + + + ), + }); + + await expect(result.current.runAgentAndWaitForNodeToJoin).rejects.toThrow( + `Agent process failed to start.\nENOENT` + ); +}); diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx index 51d5ec9e7e3de..91abf287f8295 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.tsx @@ -30,7 +30,7 @@ import { useAppContext } from 'teleterm/ui/appContextProvider'; import type { AgentProcessState, - SubscribeToAgentUpdate, + MainProcessClient, } from 'teleterm/mainProcess/types'; export interface ConnectMyComputerContext { @@ -61,16 +61,9 @@ export const ConnectMyComputerContextProvider: FC<{ await Promise.race([ waitForNodeToJoin, - waitForAgentProcessErrors( - mainProcessClient.subscribeToAgentUpdate, - props.rootClusterUri - ), + waitForAgentProcessErrors(mainProcessClient, props.rootClusterUri), ]); - }, [ - connectMyComputerService, - mainProcessClient.subscribeToAgentUpdate, - props.rootClusterUri, - ]); + }, [connectMyComputerService, mainProcessClient, props.rootClusterUri]); useEffect(() => { const { cleanup } = mainProcessClient.subscribeToAgentUpdate( @@ -108,43 +101,31 @@ export const useConnectMyComputerContext = () => { * If none of them happen within 20 seconds, the promise resolves. */ async function waitForAgentProcessErrors( - subscribeToAgentUpdate: SubscribeToAgentUpdate, + mainProcessClient: MainProcessClient, rootClusterUri: RootClusterUri ) { let cleanupFn: () => void; try { const errorPromise = new Promise((_, reject) => { - const { cleanup } = subscribeToAgentUpdate(rootClusterUri, agentState => { - if (agentState.status === 'exited') { - const { code, signal } = agentState; - const codeOrSignal = [ - // code can be 0, so we cannot just check it the same way as the signal. - code != null && `code ${code}`, - signal && `signal ${signal}`, - ] - .filter(Boolean) - .join(' '); - - reject( - new Error( - [ - `Agent process exited with ${codeOrSignal}.`, - agentState.stackTrace, - ] - .filter(Boolean) - .join('\n') - ) - ); - } - if (agentState.status === 'error') { - reject( - new Error( - ['Agent process failed to start.', agentState.message].join('\n') - ) - ); + const { cleanup } = mainProcessClient.subscribeToAgentUpdate( + rootClusterUri, + agentProcessState => { + const error = isProcessInErrorOrExitState(agentProcessState); + if (error) { + reject(error); + } } + ); + + // the state may have changed before we started listening, we have to check the current state + const agentProcessState = mainProcessClient.getAgentState({ + rootClusterUri, }); + const error = isProcessInErrorOrExitState(agentProcessState); + if (error) { + reject(error); + } cleanupFn = cleanup; }); @@ -153,3 +134,32 @@ async function waitForAgentProcessErrors( cleanupFn(); } } + +function isProcessInErrorOrExitState( + agentProcessState: AgentProcessState +): Error | undefined { + if (agentProcessState.status === 'exited') { + const { code, signal } = agentProcessState; + const codeOrSignal = [ + // code can be 0, so we cannot just check it the same way as the signal. + code != null && `code ${code}`, + signal && `signal ${signal}`, + ] + .filter(Boolean) + .join(' '); + + return new Error( + [ + `Agent process exited with ${codeOrSignal}.`, + agentProcessState.stackTrace, + ] + .filter(Boolean) + .join('\n') + ); + } + if (agentProcessState.status === 'error') { + return new Error( + ['Agent process failed to start.', agentProcessState.message].join('\n') + ); + } +} From a57477a7f3235f3dc1bdf296dd21fe78f37c3c9a Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Wed, 2 Aug 2023 14:28:56 +0200 Subject: [PATCH 28/31] Strip ANSI codes --- jest.config.js | 3 ++ web/packages/teleterm/package.json | 1 + .../mainProcess/agentRunner/agentRunner.ts | 4 +- yarn.lock | 37 ++++++++++++++++++- 4 files changed, 43 insertions(+), 2 deletions(-) diff --git a/jest.config.js b/jest.config.js index ce5e333ef3685..699e0ad5b12ce 100644 --- a/jest.config.js +++ b/jest.config.js @@ -2,6 +2,8 @@ const config = require('@gravitational/build/jest/config'); process.env.TZ = 'UTC'; +const esModules = ['strip-ansi-stream', 'ansi-regex'].join('|'); + /** @type {import('@jest/types').Config.InitialOptions} */ module.exports = { ...config, @@ -13,6 +15,7 @@ module.exports = { // '**/packages/design/src/**/*.jsx', '**/packages/shared/components/**/*.jsx', ], + transformIgnorePatterns: [`/node_modules/(?!${esModules})`], coverageReporters: ['text-summary', 'lcov'], setupFilesAfterEnv: ['/web/packages/shared/setupTests.tsx'], }; diff --git a/web/packages/teleterm/package.json b/web/packages/teleterm/package.json index 60968a08b10e0..2f31b1ca0ca74 100644 --- a/web/packages/teleterm/package.json +++ b/web/packages/teleterm/package.json @@ -53,6 +53,7 @@ "react-dnd": "^14.0.4", "react-dnd-html5-backend": "^14.0.2", "split2": "4.1.0", + "strip-ansi-stream": "^2.0.1", "ts-loader": "^9.4.2", "winston": "^3.3.3", "xterm": "^5.0.0", diff --git a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts index 3d44a9bb03599..9eb8dff36f078 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts @@ -17,6 +17,8 @@ import { spawn, ChildProcess } from 'node:child_process'; import os from 'node:os'; +import stripAnsiStream from 'strip-ansi-stream'; + import Logger from 'teleterm/logger'; import { RootClusterUri } from 'teleterm/ui/uri'; @@ -114,7 +116,7 @@ export class AgentRunner { // Teleport logs output to stderr. let stderrOutput = ''; process.stderr.setEncoding('utf-8'); - process.stderr.on('data', error => { + process.stderr.pipe(stripAnsiStream()).on('data', (error: string) => { stderrOutput += error; stderrOutput = limitProcessOutputLines(stderrOutput); }); diff --git a/yarn.lock b/yarn.lock index 45d1ca831afc0..1f16b489cc609 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4711,6 +4711,11 @@ ansi-regex@^5.0.1: resolved "https://registry.yarnpkg.com/ansi-regex/-/ansi-regex-5.0.1.tgz#082cb2c89c9fe8659a311a53bd6a4dc5301db304" integrity sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ== +ansi-regex@^6.0.1: + version "6.0.1" + resolved "https://registry.yarnpkg.com/ansi-regex/-/ansi-regex-6.0.1.tgz#3183e38fae9a65d7cb5e53945cd5897d0260a06a" + integrity sha512-n5M855fKb2SsfMIiFFoVrABHJC8QtHwVx+mHWP3QcEqBHYienj5dHSgjbxtC0WEZXYt4wcD6zrQElDPhFuZgfA== + ansi-styles@^3.2.1: version "3.2.1" resolved "https://registry.yarnpkg.com/ansi-styles/-/ansi-styles-3.2.1.tgz#41fbb20243e50b12be0f04b8dedbf07520ce841d" @@ -7252,7 +7257,7 @@ escape-html@~1.0.3: resolved "https://registry.yarnpkg.com/escape-html/-/escape-html-1.0.3.tgz#0258eae4d3d0c0974de1c169188ef0051d1d1988" integrity sha1-Aljq5NPQwJdN4cFpGI7wBR0dGYg= -escape-string-regexp@^1.0.5: +escape-string-regexp@^1.0.3, escape-string-regexp@^1.0.5: version "1.0.5" resolved "https://registry.yarnpkg.com/escape-string-regexp/-/escape-string-regexp-1.0.5.tgz#1b61c0562190a8dff6ae3bb2cf0200ca130b86d4" integrity sha1-G2HAViGQqN/2rjuyzwIAyhMLhtQ= @@ -13110,6 +13115,19 @@ readable-stream@^2.0.1: string_decoder "~1.1.1" util-deprecate "~1.0.1" +readable-stream@^2.0.2: + version "2.3.8" + resolved "https://registry.yarnpkg.com/readable-stream/-/readable-stream-2.3.8.tgz#91125e8042bba1b9887f49345f6277027ce8be9b" + integrity sha512-8p0AUk4XODgIewSi0l8Epjs+EVnWiK7NoDIEGU0HhE7+ZyY8D1IMY7odu5lRrFXGg71L15KG8QrPmum45RTtdA== + dependencies: + core-util-is "~1.0.0" + inherits "~2.0.3" + isarray "~1.0.0" + process-nextick-args "~2.0.0" + safe-buffer "~5.1.1" + string_decoder "~1.1.1" + util-deprecate "~1.0.1" + readable-stream@^3.0.6, readable-stream@^3.4.0, readable-stream@^3.6.0: version "3.6.0" resolved "https://registry.yarnpkg.com/readable-stream/-/readable-stream-3.6.0.tgz#337bbda3adc0706bd3e024426a286d4b4b2c9198" @@ -13332,6 +13350,15 @@ repeat-string@^1.5.4, repeat-string@^1.6.1: resolved "https://registry.yarnpkg.com/repeat-string/-/repeat-string-1.6.1.tgz#8dcae470e1c88abc2d600fff4a776286da75e637" integrity sha1-jcrkcOHIirwtYA//Sndihtp15jc= +replacestream@^4.0.3: + version "4.0.3" + resolved "https://registry.yarnpkg.com/replacestream/-/replacestream-4.0.3.tgz#3ee5798092be364b1cdb1484308492cb3dff2f36" + integrity sha512-AC0FiLS352pBBiZhd4VXB1Ab/lh0lEgpP+GGvZqbQh8a5cmXVoTe5EX/YeTFArnp4SRGTHh1qCHu9lGs1qG8sA== + dependencies: + escape-string-regexp "^1.0.3" + object-assign "^4.0.1" + readable-stream "^2.0.2" + require-directory@^2.1.1: version "2.1.1" resolved "https://registry.yarnpkg.com/require-directory/-/require-directory-2.1.1.tgz#8c64ad5fd30dab1c976e2344ffe7f792a6a6df42" @@ -14338,6 +14365,14 @@ string_decoder@~1.1.1: dependencies: safe-buffer "~5.1.0" +strip-ansi-stream@^2.0.1: + version "2.0.1" + resolved "https://registry.yarnpkg.com/strip-ansi-stream/-/strip-ansi-stream-2.0.1.tgz#9a5f4ef2f29a6e22e685bf69bf106df118230b46" + integrity sha512-8obaZwnoFRHCgxzrqil2435OBiCcJBOtcPkmCpgHCIJ6Rb3/Ewfob9HOkyhgxVAAaXnKGIWDiV8X4XJSOdKMkg== + dependencies: + ansi-regex "^6.0.1" + replacestream "^4.0.3" + strip-ansi@^3.0.1: version "3.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-3.0.1.tgz#6a385fb8853d952d5ff05d0e8aaf94278dc63dcf" From d80094ddbe304ad0b637724db9ea6937568b9dba Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Wed, 2 Aug 2023 15:05:41 +0200 Subject: [PATCH 29/31] Add `exitedSuccessfully` property to `exited` state, so we won't have to check signal and code every time --- .../teleterm/src/mainProcess/agentRunner/agentRunner.test.ts | 1 + .../teleterm/src/mainProcess/agentRunner/agentRunner.ts | 5 ++++- web/packages/teleterm/src/mainProcess/types.ts | 3 ++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts index b7c839b09653d..213dcb99460bd 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.test.ts @@ -108,6 +108,7 @@ test('status updates are sent on a successful start', async () => { status: 'exited', code: null, stackTrace: undefined, + exitedSuccessfully: true, signal: 'SIGTERM', }; expect(agentRunner.getState(rootClusterUri)).toStrictEqual(exitedState); diff --git a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts index 9eb8dff36f078..ba0e766efafd0 100644 --- a/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts +++ b/web/packages/teleterm/src/mainProcess/agentRunner/agentRunner.ts @@ -144,11 +144,14 @@ export class AgentRunner { process.off('error', errorHandler); process.off('spawn', spawnHandler); + const exitedSuccessfully = code === 0 || signal === 'SIGTERM'; + this.updateProcessState(rootClusterUri, { status: 'exited', code, signal, - stackTrace: signal !== 'SIGTERM' ? stderrOutput : undefined, + exitedSuccessfully, + stackTrace: exitedSuccessfully ? undefined : stderrOutput, }); }; diff --git a/web/packages/teleterm/src/mainProcess/types.ts b/web/packages/teleterm/src/mainProcess/types.ts index 0205f4300560b..0c9e1b597d761 100644 --- a/web/packages/teleterm/src/mainProcess/types.ts +++ b/web/packages/teleterm/src/mainProcess/types.ts @@ -133,7 +133,8 @@ export type AgentProcessState = status: 'exited'; code: number | null; signal: NodeJS.Signals | null; - /** Fragment of a stack trace when code is other than 0. */ + exitedSuccessfully: boolean; + /** Fragment of a stack trace when the process did not exit successfully. */ stackTrace?: string; } | { From 0b55dfd2b4136435881df71041a2a43aa82402a6 Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Wed, 2 Aug 2023 16:30:39 +0200 Subject: [PATCH 30/31] Move `strip-ansi-stream` to `dependencies` --- web/packages/teleterm/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/packages/teleterm/package.json b/web/packages/teleterm/package.json index 2f31b1ca0ca74..d4828cdc2835d 100644 --- a/web/packages/teleterm/package.json +++ b/web/packages/teleterm/package.json @@ -31,6 +31,7 @@ "@types/tar-fs": "^2.0.1", "emittery": "^1.0.1", "node-pty": "0.11.0-beta29", + "strip-ansi-stream": "^2.0.1", "tar-fs": "^3.0.3" }, "devDependencies": { @@ -53,7 +54,6 @@ "react-dnd": "^14.0.4", "react-dnd-html5-backend": "^14.0.2", "split2": "4.1.0", - "strip-ansi-stream": "^2.0.1", "ts-loader": "^9.4.2", "winston": "^3.3.3", "xterm": "^5.0.0", From 5a466e266bd13e94fb2b10433f342a800764531e Mon Sep 17 00:00:00 2001 From: Grzegorz Zdunek Date: Wed, 2 Aug 2023 16:31:06 +0200 Subject: [PATCH 31/31] Fix license --- .../connectMyComputerContext.test.tsx | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.test.tsx b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.test.tsx index c97b71d13cade..86c863cb4f343 100644 --- a/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.test.tsx +++ b/web/packages/teleterm/src/ui/ConnectMyComputer/connectMyComputerContext.test.tsx @@ -1,3 +1,19 @@ +/** + * Copyright 2023 Gravitational, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + import { EventEmitter } from 'node:events'; import React from 'react';