From d293554c42aada1a91fcd942783a2914149559f7 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Tue, 18 Oct 2022 11:45:30 -0400 Subject: [PATCH] Get tests passing Signed-off-by: Sebastian Malton --- package.json | 2 +- ...ath-mode.global-override-for-injectable.ts | 11 ++ src/common/fs/change-path-mode.injectable.ts | 16 +++ ...opy-file.global-override-for-injectable.ts | 11 ++ src/common/fs/copy-file.injectable.ts | 15 +++ .../fs/create-write-file-stream.injectable.ts | 17 +++ ...e-stream.global-override-for-injectable.ts | 11 ++ ...irectory.global-override-for-injectable.ts | 11 ++ ...able.ts => ensure-directory.injectable.ts} | 7 +- ...elimiter.global-override-for-injectable.ts | 10 ++ src/common/path/delimiter.injectable.ts | 14 ++ src/common/utils/async-result.ts | 4 + src/common/utils/wait.ts | 4 +- .../extension-discovery.injectable.ts | 4 +- .../extension-discovery.ts | 2 +- .../opening-terminal-tab.test.tsx.snap | 8 -- .../terminal/opening-terminal-tab.test.tsx | 103 ++++++++++++++- src/jest.setup.ts | 3 - src/main/__test__/kubeconfig-manager.test.ts | 11 +- .../kubeconfig-manager/kubeconfig-manager.ts | 13 +- src/main/kubectl/create-kubectl.injectable.ts | 18 +++ src/main/kubectl/kubectl.ts | 112 +++++++++------- .../get-cluster-for-request.injectable.ts | 37 ++++-- src/main/lens-proxy/lens-proxy.ts | 11 +- .../shell-api-request.injectable.ts | 11 +- .../local-shell-session.ts | 16 +-- .../local-shell-session/open.injectable.ts | 10 +- .../node-shell-session/open.injectable.ts | 12 +- .../shell-environment-cache.injectable.ts | 34 +++++ .../shell-processes.injectable.ts | 84 ++++++++++++ src/main/shell-session/shell-session.ts | 120 +++++------------- .../clean-up-shell-sessions.injectable.ts | 14 +- .../start-kube-config-sync.injectable.ts | 4 +- .../api/create-terminal-api.injectable.ts | 7 + src/renderer/api/terminal-api.ts | 35 +++-- src/renderer/api/websocket-api.ts | 2 +- yarn.lock | 33 ++++- 37 files changed, 612 insertions(+), 225 deletions(-) create mode 100644 src/common/fs/change-path-mode.global-override-for-injectable.ts create mode 100644 src/common/fs/change-path-mode.injectable.ts create mode 100644 src/common/fs/copy-file.global-override-for-injectable.ts create mode 100644 src/common/fs/copy-file.injectable.ts create mode 100644 src/common/fs/create-write-file-stream.injectable.ts create mode 100644 src/common/fs/create-write-stream.global-override-for-injectable.ts create mode 100644 src/common/fs/ensure-directory.global-override-for-injectable.ts rename src/common/fs/{ensure-dir.injectable.ts => ensure-directory.injectable.ts} (66%) create mode 100644 src/common/path/delimiter.global-override-for-injectable.ts create mode 100644 src/common/path/delimiter.injectable.ts create mode 100644 src/main/shell-session/shell-environment-cache.injectable.ts create mode 100644 src/main/shell-session/shell-processes.injectable.ts diff --git a/package.json b/package.json index e3cf0d98d7..a36828cb4d 100644 --- a/package.json +++ b/package.json @@ -449,7 +449,7 @@ "webpack-dev-server": "^4.11.1", "webpack-node-externals": "^3.0.0", "xterm": "^5.0.0", - "xterm-addon-fit": "^0.5.0", + "xterm-addon-fit": "^0.6.0", "xterm-addon-search": "^0.10.0", "xterm-addon-web-links": "^0.7.0", "xterm-addon-webgl": "^0.13.0", diff --git a/src/common/fs/change-path-mode.global-override-for-injectable.ts b/src/common/fs/change-path-mode.global-override-for-injectable.ts new file mode 100644 index 0000000000..a441c2155e --- /dev/null +++ b/src/common/fs/change-path-mode.global-override-for-injectable.ts @@ -0,0 +1,11 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ + +import { getGlobalOverride } from "../test-utils/get-global-override"; +import changePathModeInjectable from "./change-path-mode.injectable"; + +export default getGlobalOverride(changePathModeInjectable, () => () => { + throw new Error("tried to change path mode without override"); +}); diff --git a/src/common/fs/change-path-mode.injectable.ts b/src/common/fs/change-path-mode.injectable.ts new file mode 100644 index 0000000000..f4820bf119 --- /dev/null +++ b/src/common/fs/change-path-mode.injectable.ts @@ -0,0 +1,16 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ + +import { getInjectable } from "@ogre-tools/injectable"; +import fsInjectable from "./fs.injectable"; + +export type ChangePathMode = (path: string, newMode: number) => Promise; + +const changePathModeInjectable = getInjectable({ + id: "change-path-mode", + instantiate: (di): ChangePathMode => di.inject(fsInjectable).chmod, +}); + +export default changePathModeInjectable; diff --git a/src/common/fs/copy-file.global-override-for-injectable.ts b/src/common/fs/copy-file.global-override-for-injectable.ts new file mode 100644 index 0000000000..b58f736537 --- /dev/null +++ b/src/common/fs/copy-file.global-override-for-injectable.ts @@ -0,0 +1,11 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ + +import { getGlobalOverride } from "../test-utils/get-global-override"; +import copyFileInjectable from "./copy-file.injectable"; + +export default getGlobalOverride(copyFileInjectable, () => () => { + throw new Error("tried to copy a file without override"); +}); diff --git a/src/common/fs/copy-file.injectable.ts b/src/common/fs/copy-file.injectable.ts new file mode 100644 index 0000000000..86cdfe4203 --- /dev/null +++ b/src/common/fs/copy-file.injectable.ts @@ -0,0 +1,15 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import fsInjectable from "./fs.injectable"; + +export type CopyFile = (fromPath: string, toPath: string) => Promise; + +const copyFileInjectable = getInjectable({ + id: "copy-file", + instantiate: (di): CopyFile => di.inject(fsInjectable).copyFile, +}); + +export default copyFileInjectable; diff --git a/src/common/fs/create-write-file-stream.injectable.ts b/src/common/fs/create-write-file-stream.injectable.ts new file mode 100644 index 0000000000..dde1710c32 --- /dev/null +++ b/src/common/fs/create-write-file-stream.injectable.ts @@ -0,0 +1,17 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ + +import { getInjectable } from "@ogre-tools/injectable"; +import type { createWriteStream } from "fs"; +import fsInjectable from "./fs.injectable"; + +export type CreateWriteFileStream = typeof createWriteStream; + +const createWriteFileStreamInjectable = getInjectable({ + id: "create-write-file-stream", + instantiate: (di) => di.inject(fsInjectable).createWriteStream, +}); + +export default createWriteFileStreamInjectable; diff --git a/src/common/fs/create-write-stream.global-override-for-injectable.ts b/src/common/fs/create-write-stream.global-override-for-injectable.ts new file mode 100644 index 0000000000..242bceaa38 --- /dev/null +++ b/src/common/fs/create-write-stream.global-override-for-injectable.ts @@ -0,0 +1,11 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ + +import { getGlobalOverride } from "../test-utils/get-global-override"; +import createWriteFileStreamInjectable from "./create-write-file-stream.injectable"; + +export default getGlobalOverride(createWriteFileStreamInjectable, () => () => { + throw new Error("tried to create a file write stream without override"); +}); diff --git a/src/common/fs/ensure-directory.global-override-for-injectable.ts b/src/common/fs/ensure-directory.global-override-for-injectable.ts new file mode 100644 index 0000000000..b5a9f73112 --- /dev/null +++ b/src/common/fs/ensure-directory.global-override-for-injectable.ts @@ -0,0 +1,11 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ + +import { getGlobalOverride } from "../test-utils/get-global-override"; +import ensureDirectoryInjectable from "./ensure-directory.injectable"; + +export default getGlobalOverride(ensureDirectoryInjectable, () => async () => { + throw new Error("tried to ensure directory without override"); +}); diff --git a/src/common/fs/ensure-dir.injectable.ts b/src/common/fs/ensure-directory.injectable.ts similarity index 66% rename from src/common/fs/ensure-dir.injectable.ts rename to src/common/fs/ensure-directory.injectable.ts index 78ec4d91dc..9b7353a912 100644 --- a/src/common/fs/ensure-dir.injectable.ts +++ b/src/common/fs/ensure-directory.injectable.ts @@ -3,11 +3,12 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ import { getInjectable } from "@ogre-tools/injectable"; +import type { EnsureOptions } from "fs-extra"; import fsInjectable from "./fs.injectable"; -export type EnsureDirectory = (dirPath: string) => Promise; +export type EnsureDirectory = (dirPath: string, options?: number | EnsureOptions) => Promise; -const ensureDirInjectable = getInjectable({ +const ensureDirectoryInjectable = getInjectable({ id: "ensure-dir", // TODO: Remove usages of ensureDir from business logic. @@ -15,4 +16,4 @@ const ensureDirInjectable = getInjectable({ instantiate: (di): EnsureDirectory => di.inject(fsInjectable).ensureDir, }); -export default ensureDirInjectable; +export default ensureDirectoryInjectable; diff --git a/src/common/path/delimiter.global-override-for-injectable.ts b/src/common/path/delimiter.global-override-for-injectable.ts new file mode 100644 index 0000000000..bdc0409552 --- /dev/null +++ b/src/common/path/delimiter.global-override-for-injectable.ts @@ -0,0 +1,10 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ + +import path from "path"; +import { getGlobalOverride } from "../test-utils/get-global-override"; +import pathDelimiterInjectable from "./delimiter.injectable"; + +export default getGlobalOverride(pathDelimiterInjectable, () => path.posix.delimiter); diff --git a/src/common/path/delimiter.injectable.ts b/src/common/path/delimiter.injectable.ts new file mode 100644 index 0000000000..267208fcda --- /dev/null +++ b/src/common/path/delimiter.injectable.ts @@ -0,0 +1,14 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import path from "path"; + +const pathDelimiterInjectable = getInjectable({ + id: "path-delimiter", + instantiate: () => path.delimiter, + causesSideEffects: true, +}); + +export default pathDelimiterInjectable; diff --git a/src/common/utils/async-result.ts b/src/common/utils/async-result.ts index 69927f3275..abf9ca7117 100644 --- a/src/common/utils/async-result.ts +++ b/src/common/utils/async-result.ts @@ -9,3 +9,7 @@ export type AsyncResult = : { callWasSuccessful: true; response: Response } ) | { callWasSuccessful: false; error: Error }; + +export type Result = + | { callWasSuccessful: true; response: Response } + | { callWasSuccessful: false; error: Error }; diff --git a/src/common/utils/wait.ts b/src/common/utils/wait.ts index 402d556b5d..b4a694d0c3 100644 --- a/src/common/utils/wait.ts +++ b/src/common/utils/wait.ts @@ -2,11 +2,11 @@ * Copyright (c) OpenLens Authors. All rights reserved. * Licensed under MIT License. See LICENSE in root directory for more information. */ -import type { IComputedValue } from "mobx"; +import type { IComputedValue, IObservableValue } from "mobx"; import { runInAction, when } from "mobx"; import type { Disposer } from "./disposer"; -export async function waitUntilDefined(getter: (() => T | null | undefined) | IComputedValue, opts?: { timeout?: number }): Promise { +export async function waitUntilDefined(getter: (() => T | null | undefined) | IComputedValue | IObservableValue, opts?: { timeout?: number }): Promise { return new Promise((resolve, reject) => { when( () => { diff --git a/src/extensions/extension-discovery/extension-discovery.injectable.ts b/src/extensions/extension-discovery/extension-discovery.injectable.ts index 378f519bb7..d5ef981c64 100644 --- a/src/extensions/extension-discovery/extension-discovery.injectable.ts +++ b/src/extensions/extension-discovery/extension-discovery.injectable.ts @@ -16,7 +16,7 @@ import pathExistsInjectable from "../../common/fs/path-exists.injectable"; import watchInjectable from "../../common/fs/watch/watch.injectable"; import accessPathInjectable from "../../common/fs/access-path.injectable"; import copyInjectable from "../../common/fs/copy.injectable"; -import ensureDirInjectable from "../../common/fs/ensure-dir.injectable"; +import ensureDirectoryInjectable from "../../common/fs/ensure-directory.injectable"; import isProductionInjectable from "../../common/vars/is-production.injectable"; import lstatInjectable from "../../common/fs/lstat.injectable"; import readDirectoryInjectable from "../../common/fs/read-directory.injectable"; @@ -47,7 +47,7 @@ const extensionDiscoveryInjectable = getInjectable({ accessPath: di.inject(accessPathInjectable), copy: di.inject(copyInjectable), removePath: di.inject(removePathInjectable), - ensureDirectory: di.inject(ensureDirInjectable), + ensureDirectory: di.inject(ensureDirectoryInjectable), isProduction: di.inject(isProductionInjectable), lstat: di.inject(lstatInjectable), readDirectory: di.inject(readDirectoryInjectable), diff --git a/src/extensions/extension-discovery/extension-discovery.ts b/src/extensions/extension-discovery/extension-discovery.ts index c9646a1c6c..efac752073 100644 --- a/src/extensions/extension-discovery/extension-discovery.ts +++ b/src/extensions/extension-discovery/extension-discovery.ts @@ -21,7 +21,7 @@ import type { Watch } from "../../common/fs/watch/watch.injectable"; import type { Stats } from "fs"; import type { LStat } from "../../common/fs/lstat.injectable"; import type { ReadDirectory } from "../../common/fs/read-directory.injectable"; -import type { EnsureDirectory } from "../../common/fs/ensure-dir.injectable"; +import type { EnsureDirectory } from "../../common/fs/ensure-directory.injectable"; import type { AccessPath } from "../../common/fs/access-path.injectable"; import type { Copy } from "../../common/fs/copy.injectable"; import type { JoinPaths } from "../../common/path/join-paths.injectable"; diff --git a/src/features/terminal/__snapshots__/opening-terminal-tab.test.tsx.snap b/src/features/terminal/__snapshots__/opening-terminal-tab.test.tsx.snap index da17a3d55f..e3f36a584c 100644 --- a/src/features/terminal/__snapshots__/opening-terminal-tab.test.tsx.snap +++ b/src/features/terminal/__snapshots__/opening-terminal-tab.test.tsx.snap @@ -645,14 +645,6 @@ exports[`test for opening terminal tab within cluster frame when new terminal ta
- -
diff --git a/src/features/terminal/opening-terminal-tab.test.tsx b/src/features/terminal/opening-terminal-tab.test.tsx index e8c48bf76f..ca4959be35 100644 --- a/src/features/terminal/opening-terminal-tab.test.tsx +++ b/src/features/terminal/opening-terminal-tab.test.tsx @@ -4,6 +4,16 @@ */ import type { RenderResult } from "@testing-library/react"; +import { prettyDOM, waitFor } from "@testing-library/react"; +import assert from "assert"; +import type { IObservableValue } from "mobx"; +import { observable } from "mobx"; +import type { IPty } from "node-pty"; +import { waitUntilDefined } from "../../common/utils"; +import createKubeconfigManagerInjectable from "../../main/kubeconfig-manager/create-kubeconfig-manager.injectable"; +import type { KubeconfigManager } from "../../main/kubeconfig-manager/kubeconfig-manager"; +import type { SpawnPty } from "../../main/shell-session/spawn-pty.injectable"; +import spawnPtyInjectable from "../../main/shell-session/spawn-pty.injectable"; import type { ApplicationBuilder } from "../../renderer/components/test-utils/get-application-builder"; import { getApplicationBuilder } from "../../renderer/components/test-utils/get-application-builder"; import type { FindByTextWithMarkup } from "../../test-utils/findByTextWithMarkup"; @@ -13,11 +23,29 @@ describe("test for opening terminal tab within cluster frame", () => { let builder: ApplicationBuilder; let result: RenderResult; let findByTextWithMarkup: FindByTextWithMarkup; + let spawnPtyMock: jest.MockedFunction; + + beforeAll(() => { + jest.spyOn(window, "requestAnimationFrame").mockImplementation(function IAmAMockRequestAnimationFrame(cb) { + return window.setTimeout(() => cb(Date.now())); + }); + }); beforeEach(async () => { builder = getApplicationBuilder(); + + builder.mainDi.override(createKubeconfigManagerInjectable, () => (cluster) => { + return { + getPath: async () => `/some-kubeconfig-managed-path-for-${cluster.id}`, + clear: async () => {}, + } as KubeconfigManager; + }); + builder.setEnvironmentToClusterFrame(); + spawnPtyMock = jest.fn(); + builder.mainDi.override(spawnPtyInjectable, () => spawnPtyMock); + result = await builder.render(); findByTextWithMarkup = findByTextWithMarkupFor(result); }); @@ -45,12 +73,81 @@ describe("test for opening terminal tab within cluster frame", () => { await findByTextWithMarkup("Connecting ..."); }); - it.skip("connects websocket to main", () => { + describe("when the websocket connection is established", () => { + let pty: IObservableValue; + let sendData: (e: string) => any; - }); + beforeEach(async () => { + pty = observable.box(undefined, { + deep: false, + }); - it.skip("displays the values on screen", () => { + spawnPtyMock.mockImplementationOnce(() => { + const val: IPty = { + cols: 80, + handleFlowControl: false, + kill: jest.fn(), + onData: (handler) => { + sendData = handler; + return { + dispose: () => {}, + }; + }, + onExit: jest.fn(), + pause: jest.fn(), + pid: 12345, + process: "my-term", + resize: jest.fn(), + resume: jest.fn(), + rows: 40, + write: jest.fn(), + on: jest.fn(), + }; + + pty.set(val); + + return val; + }); + + await waitUntilDefined(pty); + }); + + describe("when the first data is sent", () => { + beforeEach(() => { + sendData(""); + }); + + it("clears the screen", async () => { + await waitFor(() => hasNoTextContent(result.baseElement, ".xterm-rows")); + }); + + describe("when the next data is sent", () => { + beforeEach(() => { + sendData("I am a prompt"); + }); + + it("renders the new data", async () => { + await findByTextWithMarkup("I am a prompt"); + }); + }); + }); }); }); }); + +function hasNoTextContent(baseElement: HTMLElement, selector: string) { + const root = baseElement.querySelector(selector); + + assert(root, `Did not find "${selector}" in:\n${prettyDOM(baseElement)}`); + + const assertChildrenHasNoTextContent = (elem: HTMLElement | Element) => { + for (const child of elem.children) { + expect(child.textContent?.trim()).toBe(""); + assertChildrenHasNoTextContent(child); + } + }; + + expect(root.textContent?.trim()).toBe(""); + assertChildrenHasNoTextContent(root); +} diff --git a/src/jest.setup.ts b/src/jest.setup.ts index cd7316ec70..73b741dee4 100644 --- a/src/jest.setup.ts +++ b/src/jest.setup.ts @@ -9,7 +9,6 @@ import { TextEncoder, TextDecoder as TextDecoderNode } from "util"; import glob from "glob"; import path from "path"; import { enableMapSet, setAutoFreeze } from "immer"; -import { WebSocket } from "ws"; declare global { interface InjectablePaths { @@ -53,8 +52,6 @@ global.ResizeObserver = class { disconnect = () => {}; }; -global.WebSocket = WebSocket as any; - jest.mock("./renderer/components/monaco-editor/monaco-editor"); jest.mock("./renderer/components/tooltip/withTooltip"); diff --git a/src/main/__test__/kubeconfig-manager.test.ts b/src/main/__test__/kubeconfig-manager.test.ts index 689ef13ce8..c58d98eb91 100644 --- a/src/main/__test__/kubeconfig-manager.test.ts +++ b/src/main/__test__/kubeconfig-manager.test.ts @@ -3,7 +3,7 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ import { getDiForUnitTesting } from "../getDiForUnitTesting"; -import { KubeconfigManager } from "../kubeconfig-manager/kubeconfig-manager"; +import type { KubeconfigManager } from "../kubeconfig-manager/kubeconfig-manager"; import type { Cluster } from "../../common/cluster/cluster"; import createKubeconfigManagerInjectable from "../kubeconfig-manager/create-kubeconfig-manager.injectable"; import { createClusterInjectionToken } from "../../common/cluster/create-cluster-injection-token"; @@ -30,6 +30,7 @@ import removePathInjectable from "../../common/fs/remove.injectable"; import pathExistsSyncInjectable from "../../common/fs/path-exists-sync.injectable"; import readJsonSyncInjectable from "../../common/fs/read-json-sync.injectable"; import writeJsonSyncInjectable from "../../common/fs/write-json-sync.injectable"; +import lensProxyPortInjectable from "../lens-proxy/lens-proxy-port.injectable"; const clusterServerUrl = "https://192.168.64.3:8443"; @@ -90,6 +91,8 @@ describe("kubeconfig manager tests", () => { ensureServer: ensureServerMock, })); + di.inject(lensProxyPortInjectable).set(9191); + const createCluster = di.inject(createClusterInjectionToken); createKubeconfigManager = di.inject(createKubeconfigManagerInjectable); @@ -102,8 +105,6 @@ describe("kubeconfig manager tests", () => { clusterServerUrl, }); - jest.spyOn(KubeconfigManager.prototype, "resolveProxyUrl", "get").mockReturnValue("https://127.0.0.1:9191/foo"); - kubeConfManager = createKubeconfigManager(clusterFake); }); @@ -174,7 +175,7 @@ describe("kubeconfig manager tests", () => { beforeEach(async () => { await writeFileMock.resolveSpecific( [ - "/some-directory-for-temp/kubeconfig-foo", + "/some-directory-for-temp/kubeconfig-foo", "apiVersion: v1\nkind: Config\npreferences: {}\ncurrent-context: minikube\nclusters:\n - name: minikube\n cluster:\n certificate-authority-data: PGNhLWRhdGE+\n server: https://127.0.0.1:9191/foo\n insecure-skip-tls-verify: false\ncontexts:\n - name: minikube\n context:\n cluster: minikube\n user: proxy\nusers:\n - name: proxy\n user:\n username: lens\n password: fake\n", ], ); @@ -303,7 +304,7 @@ describe("kubeconfig manager tests", () => { beforeEach(async () => { await writeFileMock.resolveSpecific( [ - "/some-directory-for-temp/kubeconfig-foo", + "/some-directory-for-temp/kubeconfig-foo", "apiVersion: v1\nkind: Config\npreferences: {}\ncurrent-context: minikube\nclusters:\n - name: minikube\n cluster:\n certificate-authority-data: PGNhLWRhdGE+\n server: https://127.0.0.1:9191/foo\n insecure-skip-tls-verify: false\ncontexts:\n - name: minikube\n context:\n cluster: minikube\n user: proxy\nusers:\n - name: proxy\n user:\n username: lens\n password: fake\n", ], ); diff --git a/src/main/kubeconfig-manager/kubeconfig-manager.ts b/src/main/kubeconfig-manager/kubeconfig-manager.ts index 0486521e21..9d26fcccb2 100644 --- a/src/main/kubeconfig-manager/kubeconfig-manager.ts +++ b/src/main/kubeconfig-manager/kubeconfig-manager.ts @@ -29,6 +29,11 @@ export interface KubeconfigManagerDependencies { certificate: SelfSignedCert; } +export interface KubeconfigManager { + getPath(): Promise; + clear(): Promise; +} + export class KubeconfigManager { /** * The path to the temp config file @@ -40,7 +45,7 @@ export class KubeconfigManager { protected readonly contextHandler: ClusterContextHandler; - constructor(private readonly dependencies: KubeconfigManagerDependencies, protected cluster: Cluster) { + constructor(private readonly dependencies: KubeconfigManagerDependencies, protected readonly cluster: Cluster) { this.contextHandler = cluster.contextHandler; } @@ -87,10 +92,6 @@ export class KubeconfigManager { } } - get resolveProxyUrl() { - return `https://127.0.0.1:${this.dependencies.lensProxyPort.get()}/${this.cluster.id}`; - } - /** * Creates new "temporary" kubeconfig that point to the kubectl-proxy. * This way any user of the config does not need to know anything about the auth etc. details. @@ -109,7 +110,7 @@ export class KubeconfigManager { clusters: [ { name: contextName, - server: this.resolveProxyUrl, + server: `http://127.0.0.1:${this.dependencies.lensProxyPort.get()}/${this.cluster.id}`, skipTLSVerify: false, caData: Buffer.from(certificate.cert).toString("base64"), }, diff --git a/src/main/kubectl/create-kubectl.injectable.ts b/src/main/kubectl/create-kubectl.injectable.ts index adfce0922f..09d3e0cae0 100644 --- a/src/main/kubectl/create-kubectl.injectable.ts +++ b/src/main/kubectl/create-kubectl.injectable.ts @@ -18,6 +18,15 @@ import getDirnameOfPathInjectable from "../../common/path/get-dirname.injectable import joinPathsInjectable from "../../common/path/join-paths.injectable"; import getBasenameOfPathInjectable from "../../common/path/get-basename.injectable"; import loggerInjectable from "../../common/logger.injectable"; +import changePathModeInjectable from "../../common/fs/change-path-mode.injectable"; +import copyFileInjectable from "../../common/fs/copy-file.injectable"; +import createWriteFileStreamInjectable from "../../common/fs/create-write-file-stream.injectable"; +import execFileInjectable from "../../common/fs/exec-file.injectable"; +import pathExistsInjectable from "../../common/fs/path-exists.injectable"; +import removePathInjectable from "../../common/fs/remove.injectable"; +import writeFileInjectable from "../../common/fs/write-file.injectable"; +import ensureDirectoryInjectable from "../../common/fs/ensure-directory.injectable"; +import fetchInjectable from "../../common/fetch/fetch.injectable"; const createKubectlInjectable = getInjectable({ id: "create-kubectl", @@ -37,6 +46,15 @@ const createKubectlInjectable = getInjectable({ getDirnameOfPath: di.inject(getDirnameOfPathInjectable), joinPaths: di.inject(joinPathsInjectable), getBasenameOfPath: di.inject(getBasenameOfPathInjectable), + changePathMode: di.inject(changePathModeInjectable), + copyFile: di.inject(copyFileInjectable), + createWriteFileStream: di.inject(createWriteFileStreamInjectable), + ensureDirectory: di.inject(ensureDirectoryInjectable), + execFile: di.inject(execFileInjectable), + pathExists: di.inject(pathExistsInjectable), + removePath: di.inject(removePathInjectable), + writeFile: di.inject(writeFileInjectable), + fetch: di.inject(fetchInjectable), }; return (clusterVersion: string) => new Kubectl(dependencies, clusterVersion); diff --git a/src/main/kubectl/kubectl.ts b/src/main/kubectl/kubectl.ts index 15773a2ef7..c62b25db1e 100644 --- a/src/main/kubectl/kubectl.ts +++ b/src/main/kubectl/kubectl.ts @@ -3,23 +3,28 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ -import fs from "fs"; -import { promiseExecFile } from "../../common/utils/promise-exec"; -import { ensureDir, pathExists } from "fs-extra"; import * as lockFile from "proper-lockfile"; import { SemVer, coerce } from "semver"; import { defaultPackageMirror, packageMirrors } from "../../common/user-store/preferences-helpers"; -import got from "got/dist/source"; -import { promisify } from "util"; import stream from "stream"; -import { noop } from "lodash/fp"; import type { JoinPaths } from "../../common/path/join-paths.injectable"; import type { GetDirnameOfPath } from "../../common/path/get-dirname.injectable"; import type { GetBasenameOfPath } from "../../common/path/get-basename.injectable"; import type { NormalizedPlatform } from "../../common/vars/normalized-platform.injectable"; import type { Logger } from "../../common/logger"; +import type { PathExists } from "../../common/fs/path-exists.injectable"; +import type { ExecFile } from "../../common/fs/exec-file.injectable"; +import type { EnsureDirectory } from "../../common/fs/ensure-directory.injectable"; +import { promisify } from "util"; +import type { ChangePathMode } from "../../common/fs/change-path-mode.injectable"; +import type { WriteFile } from "../../common/fs/write-file.injectable"; +import type { CopyFile } from "../../common/fs/copy-file.injectable"; +import type { CreateWriteFileStream } from "../../common/fs/create-write-file-stream.injectable"; +import type { Fetch } from "../../common/fetch/fetch.injectable"; +import type { RemovePath } from "../../common/fs/remove.injectable"; const initScriptVersionString = "# lens-initscript v3"; +const pipeline = promisify(stream.pipeline); export interface KubectlDependencies { readonly directoryForKubectlBinaries: string; @@ -40,6 +45,15 @@ export interface KubectlDependencies { joinPaths: JoinPaths; getDirnameOfPath: GetDirnameOfPath; getBasenameOfPath: GetBasenameOfPath; + removePath: RemovePath; + pathExists: PathExists; + execFile: ExecFile; + ensureDirectory: EnsureDirectory; + changePathMode: ChangePathMode; + writeFile: WriteFile; + copyFile: CopyFile; + createWriteFileStream: CreateWriteFileStream; + fetch: Fetch; } export class Kubectl { @@ -143,37 +157,41 @@ export class Kubectl { } public async checkBinary(path: string, checkVersion = true) { - const exists = await pathExists(path); + const exists = await this.dependencies.pathExists(path); if (exists) { - try { - const args = [ - "version", - "--client", - "--output", "json", - ]; - const { stdout } = await promiseExecFile(path, args); - const output = JSON.parse(stdout); + const result = await this.dependencies.execFile(path, [ + "version", + "--client", + "--output", "json", + ]); - if (!checkVersion) { - return true; - } - let version: string = output.clientVersion.gitVersion; + if (!result.callWasSuccessful) { + this.dependencies.logger.error(`Local kubectl failed to run properly (${result.error}), removing`); + await this.dependencies.removePath(this.path); - if (version[0] === "v") { - version = version.slice(1); - } - - if (version === this.kubectlVersion) { - this.dependencies.logger.debug(`Local kubectl is version ${this.kubectlVersion}`); - - return true; - } - this.dependencies.logger.error(`Local kubectl is version ${version}, expected ${this.kubectlVersion}, unlinking`); - } catch (error) { - this.dependencies.logger.error(`Local kubectl failed to run properly (${error}), unlinking`); + return false; } - await fs.promises.unlink(this.path); + + const output = JSON.parse(result.response); + + if (!checkVersion) { + return true; + } + let version: string = output.clientVersion.gitVersion; + + if (version[0] === "v") { + version = version.slice(1); + } + + if (version === this.kubectlVersion) { + this.dependencies.logger.debug(`Local kubectl is version ${this.kubectlVersion}`); + + return true; + } + + this.dependencies.logger.error(`Local kubectl is version ${version}, expected ${this.kubectlVersion}, removing`); + await this.dependencies.removePath(this.path); } return false; @@ -182,11 +200,11 @@ export class Kubectl { protected async checkBundled(): Promise { if (this.kubectlVersion === this.dependencies.bundledKubectlVersion) { try { - const exist = await pathExists(this.path); + const exist = await this.dependencies.pathExists(this.path); if (!exist) { - await fs.promises.copyFile(this.getBundledPath(), this.path); - await fs.promises.chmod(this.path, 0o755); + await this.dependencies.copyFile(this.getBundledPath(), this.path); + await this.dependencies.changePathMode(this.path, 0o755); } return true; @@ -211,7 +229,7 @@ export class Kubectl { return false; } - await ensureDir(this.dirname, 0o755); + await this.dependencies.ensureDirectory(this.dirname, 0o755); try { const release = await lockFile.lock(this.dirname); @@ -253,20 +271,24 @@ export class Kubectl { } public async downloadKubectl() { - await ensureDir(this.dependencies.getDirnameOfPath(this.path), 0o755); + await this.dependencies.ensureDirectory(this.dependencies.getDirnameOfPath(this.path), 0o755); this.dependencies.logger.info(`Downloading kubectl ${this.kubectlVersion} from ${this.url} to ${this.path}`); - const downloadStream = got.stream({ url: this.url, decompress: true }); - const fileWriteStream = fs.createWriteStream(this.path, { mode: 0o755 }); - const pipeline = promisify(stream.pipeline); + const response = await this.dependencies.fetch(this.url, { compress: true }); + + if (!response.body || !response.body.readable) { + throw new Error("Body missing or not readable"); + } + + const fileWriteStream = this.dependencies.createWriteFileStream(this.path, { mode: 0o755 }); try { - await pipeline(downloadStream, fileWriteStream); - await fs.promises.chmod(this.path, 0o755); + await pipeline(response.body, fileWriteStream); + await this.dependencies.changePathMode(this.path, 0o755); this.dependencies.logger.debug("kubectl binary download finished"); } catch (error) { - await fs.promises.unlink(this.path).catch(noop); + await this.dependencies.removePath(this.path); throw error; } } @@ -332,8 +354,8 @@ export class Kubectl { ].join("\n"); await Promise.all([ - fs.promises.writeFile(bashScriptPath, bashScript, { mode: 0o644 }), - fs.promises.writeFile(zshScriptPath, zshScript, { mode: 0o644 }), + this.dependencies.writeFile(bashScriptPath, bashScript, { mode: 0o644 }), + this.dependencies.writeFile(zshScriptPath, zshScript, { mode: 0o644 }), ]); } diff --git a/src/main/lens-proxy/get-cluster-for-request.injectable.ts b/src/main/lens-proxy/get-cluster-for-request.injectable.ts index d2406479c0..6a00bf9512 100644 --- a/src/main/lens-proxy/get-cluster-for-request.injectable.ts +++ b/src/main/lens-proxy/get-cluster-for-request.injectable.ts @@ -17,30 +17,43 @@ const getClusterForRequestInjectable = getInjectable({ const getClusterById = di.inject(getClusterByIdInjectable); return (req) => { - if (!req.headers.host) { + const { host } = req.headers; + + if (!host || !req.url) { return undefined; } // lens-server is connecting to 127.0.0.1:/ - if (req.url && req.headers.host.startsWith("127.0.0.1")) { - const clusterId = req.url.split("/")[1]; - const cluster = getClusterById(clusterId); + if (host.startsWith("127.0.0.1") || host.startsWith("localhost")) { + { + const clusterId = req.url.split("/")[1]; + const cluster = getClusterById(clusterId); - if (cluster) { - // we need to swap path prefix so that request is proxied to kube api - req.url = req.url.replace(`/${clusterId}`, apiKubePrefix); + if (cluster) { + // we need to swap path prefix so that request is proxied to kube api + req.url = req.url.replace(`/${clusterId}`, apiKubePrefix); + + return cluster; + } } - return cluster; + { + const searchParams = new URLSearchParams(req.url); + const clusterId = searchParams.get("clusterId"); + + if (clusterId) { + return getClusterById(clusterId); + } + } } - const clusterId = getClusterIdFromHost(req.headers.host); + const clusterId = getClusterIdFromHost(host); - if (!clusterId) { - return undefined; + if (clusterId) { + return getClusterById(clusterId); } - return getClusterById(clusterId); + return undefined; }; }, }); diff --git a/src/main/lens-proxy/lens-proxy.ts b/src/main/lens-proxy/lens-proxy.ts index 59b052f9a8..99fefd5c45 100644 --- a/src/main/lens-proxy/lens-proxy.ts +++ b/src/main/lens-proxy/lens-proxy.ts @@ -68,7 +68,7 @@ export class LensProxy { protected retryCounters = new Map(); constructor(private readonly dependencies: Dependencies) { - this.configureProxy(dependencies.proxy); + this.configureProxy(this.dependencies.proxy); this.proxyServer = https.createServer( { @@ -91,8 +91,13 @@ export class LensProxy { const isInternal = req.url.startsWith(`${apiPrefix}?`); const reqHandler = isInternal ? this.dependencies.shellApiRequest : this.dependencies.kubeApiUpgradeRequest; - (async () => reqHandler({ req, socket, head, cluster }))() - .catch(error => this.dependencies.logger.error("[LENS-PROXY]: failed to handle proxy upgrade", error)); + (async () => { + try { + await reqHandler({ req, socket, head, cluster }); + } catch (error) { + this.dependencies.logger.error("[LENS-PROXY]: failed to handle proxy upgrade", error); + } + })(); } }); } diff --git a/src/main/lens-proxy/proxy-functions/shell-api-request.injectable.ts b/src/main/lens-proxy/proxy-functions/shell-api-request.injectable.ts index 05785541df..ddc2aa4745 100644 --- a/src/main/lens-proxy/proxy-functions/shell-api-request.injectable.ts +++ b/src/main/lens-proxy/proxy-functions/shell-api-request.injectable.ts @@ -27,14 +27,21 @@ const shellApiRequestInjectable = getInjectable({ const nodeName = searchParams.get("node"); const shellToken = searchParams.get("shellToken"); + console.log("got shell api request", { tabId, clusterId: cluster?.id, nodeName }); + if (!tabId || !cluster || !shellApiAuthenticator.authenticate(cluster.id, tabId, shellToken)) { socket.write("Invalid shell request"); socket.end(); } else { new WebSocketServer({ noServer: true }) .handleUpgrade(req, socket, head, (websocket) => { - openShellSession({ websocket, cluster, tabId, nodeName }) - .catch(error => logger.error(`[SHELL-SESSION]: failed to open a ${nodeName ? "node" : "local"} shell`, error)); + (async () => { + try { + await openShellSession({ websocket, cluster, tabId, nodeName }); + } catch (error) { + logger.error(`[SHELL-SESSION]: failed to open a ${nodeName ? "node" : "local"} shell`, error); + } + })(); }); } }; diff --git a/src/main/shell-session/local-shell-session/local-shell-session.ts b/src/main/shell-session/local-shell-session/local-shell-session.ts index cf2ec2d1db..b3dd91f313 100644 --- a/src/main/shell-session/local-shell-session/local-shell-session.ts +++ b/src/main/shell-session/local-shell-session/local-shell-session.ts @@ -37,23 +37,21 @@ export class LocalShellSession extends ShellSession { } public async open() { - // extensions can modify the env - const env = this.dependencies.modifyTerminalShellEnv(this.cluster.id, await this.getCachedShellEnv()); + const cachedShellEnv = await this.getCachedShellEnv(); + const env = this.dependencies.modifyTerminalShellEnv(this.cluster.id, cachedShellEnv); const shell = env.PTYSHELL; - if (!shell) { + if (shell) { + const args = await this.getShellArgs(shell); + + await this.openShellProcess(shell, args, env); + } else { this.send({ type: TerminalChannels.ERROR, data: "PTYSHELL is not defined with the environment", }); this.dependencies.logger.warn(`[LOCAL-SHELL-SESSION]: PTYSHELL env var is not defined for ${this.terminalId}`); - - return; } - - const args = await this.getShellArgs(shell); - - await this.openShellProcess(shell, args, env); } protected async getShellArgs(shell: string): Promise { diff --git a/src/main/shell-session/local-shell-session/open.injectable.ts b/src/main/shell-session/local-shell-session/open.injectable.ts index 084aa2e17c..457c357e01 100644 --- a/src/main/shell-session/local-shell-session/open.injectable.ts +++ b/src/main/shell-session/local-shell-session/open.injectable.ts @@ -18,12 +18,16 @@ import getDirnameOfPathInjectable from "../../../common/path/get-dirname.injecta import joinPathsInjectable from "../../../common/path/join-paths.injectable"; import getBasenameOfPathInjectable from "../../../common/path/get-basename.injectable"; import computeShellEnvironmentInjectable from "../../../features/shell-sync/main/compute-shell-environment.injectable"; -import spawnPtyInjectable from "../spawn-pty.injectable"; import userShellSettingInjectable from "../../../common/user-store/shell-setting.injectable"; import appNameInjectable from "../../../common/vars/app-name.injectable"; import buildVersionInjectable from "../../vars/build-version/build-version.injectable"; import emitAppEventInjectable from "../../../common/app-event-bus/emit-event.injectable"; import statInjectable from "../../../common/fs/stat.injectable"; +import shellEnvironmentCacheInjectable from "../shell-environment-cache.injectable"; +import shellProcessesInjectable from "../shell-processes.injectable"; +import homeDirectoryPathInjectable from "../../../common/os/home-directory-path.injectable"; +import pathDelimiterInjectable from "../../../common/path/delimiter.injectable"; +import spawnPtyInjectable from "../spawn-pty.injectable"; export interface OpenLocalShellSessionArgs { websocket: WebSocket; @@ -55,6 +59,10 @@ const openLocalShellSessionInjectable = getInjectable({ computeShellEnvironment: di.inject(computeShellEnvironmentInjectable), spawnPty: di.inject(spawnPtyInjectable), stat: di.inject(statInjectable), + shellEnvironmentCache: di.inject(shellEnvironmentCacheInjectable), + shellProcesses: di.inject(shellProcessesInjectable), + homeDirectory: di.inject(homeDirectoryPathInjectable), + pathDelimiter: di.inject(pathDelimiterInjectable), }; return (args) => { diff --git a/src/main/shell-session/node-shell-session/open.injectable.ts b/src/main/shell-session/node-shell-session/open.injectable.ts index cce3fa5f36..3aedc5f155 100644 --- a/src/main/shell-session/node-shell-session/open.injectable.ts +++ b/src/main/shell-session/node-shell-session/open.injectable.ts @@ -13,13 +13,18 @@ import isWindowsInjectable from "../../../common/vars/is-windows.injectable"; import loggerInjectable from "../../../common/logger.injectable"; import createKubeJsonApiForClusterInjectable from "../../../common/k8s-api/create-kube-json-api-for-cluster.injectable"; import computeShellEnvironmentInjectable from "../../../features/shell-sync/main/compute-shell-environment.injectable"; -import spawnPtyInjectable from "../spawn-pty.injectable"; import userShellSettingInjectable from "../../../common/user-store/shell-setting.injectable"; import appNameInjectable from "../../../common/vars/app-name.injectable"; import buildVersionInjectable from "../../vars/build-version/build-version.injectable"; import emitAppEventInjectable from "../../../common/app-event-bus/emit-event.injectable"; import statInjectable from "../../../common/fs/stat.injectable"; import createKubeApiInjectable from "../../../common/k8s-api/create-kube-api.injectable"; +import getBasenameOfPathInjectable from "../../../common/path/get-basename.injectable"; +import homeDirectoryPathInjectable from "../../../common/os/home-directory-path.injectable"; +import pathDelimiterInjectable from "../../../common/path/delimiter.injectable"; +import shellEnvironmentCacheInjectable from "../shell-environment-cache.injectable"; +import shellProcessesInjectable from "../shell-processes.injectable"; +import spawnPtyInjectable from "../spawn-pty.injectable"; export interface NodeShellSessionArgs { websocket: WebSocket; @@ -47,6 +52,11 @@ const openNodeShellSessionInjectable = getInjectable({ emitAppEvent: di.inject(emitAppEventInjectable), stat: di.inject(statInjectable), createKubeApi: di.inject(createKubeApiInjectable), + getBasenameOfPath: di.inject(getBasenameOfPathInjectable), + homeDirectory: di.inject(homeDirectoryPathInjectable), + pathDelimiter: di.inject(pathDelimiterInjectable), + shellEnvironmentCache: di.inject(shellEnvironmentCacheInjectable), + shellProcesses: di.inject(shellProcessesInjectable), }; return async (args) => { diff --git a/src/main/shell-session/shell-environment-cache.injectable.ts b/src/main/shell-session/shell-environment-cache.injectable.ts new file mode 100644 index 0000000000..0e13d03bda --- /dev/null +++ b/src/main/shell-session/shell-environment-cache.injectable.ts @@ -0,0 +1,34 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import type { IAsyncComputed } from "@ogre-tools/injectable-react"; +import { asyncComputed } from "@ogre-tools/injectable-react"; +import { when } from "mobx"; +import { now } from "mobx-utils"; +import type { ClusterId } from "../../common/cluster-types"; +import { getOrInsert } from "../../common/utils"; + +export type ShellEnvironmentCache = (clusterId: string, builder: () => Promise>>) => Promise>>; + +const shellEnvironmentCacheInjectable = getInjectable({ + id: "shell-environment-cache", + instantiate: (): ShellEnvironmentCache => { + const cache = new Map>>>(); + + return async (clusterId, builder) => { + const cacheLine = getOrInsert(cache, clusterId, asyncComputed(() => { + now(1000 * 60 * 10); // update every 10 minutes + + return builder(); + })); + + await when(() => !cacheLine.pending.get()); + + return cacheLine.value.get(); + }; + }, +}); + +export default shellEnvironmentCacheInjectable; diff --git a/src/main/shell-session/shell-processes.injectable.ts b/src/main/shell-session/shell-processes.injectable.ts new file mode 100644 index 0000000000..6073d227fc --- /dev/null +++ b/src/main/shell-session/shell-processes.injectable.ts @@ -0,0 +1,84 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; +import type { IPty } from "node-pty"; +import loggerInjectable from "../../common/logger.injectable"; +import { getOrInsertWith } from "../../common/utils"; +import type { AsyncResult } from "../../common/utils/async-result"; +import spawnPtyInjectable from "./spawn-pty.injectable"; + +export interface StartOrResuemArgs { + terminalId: string; + shell: string; + args: string[]; + env: Partial>; + cwd: string; +} + +export interface ShellProcesses { + startOrResume: (args: StartOrResuemArgs) => AsyncResult<{ shellProcess: IPty; resume: boolean }>; + cleanup: () => void; + clear: (terminalId: string) => void; +} + +const shellProcessesInjectable = getInjectable({ + id: "shell-processes", + instantiate: (di): ShellProcesses => { + const spawnPty = di.inject(spawnPtyInjectable); + const logger = di.inject(loggerInjectable); + + const processes = new Map(); + + return { + startOrResume: ({ terminalId, shell, args, env, cwd }) => { + try { + const resume = processes.has(terminalId); + + const shellProcess = getOrInsertWith(processes, terminalId, () => ( + spawnPty(shell, args, { + rows: 30, + cols: 80, + cwd, + env, + name: "xterm-256color", + // TODO: Something else is broken here so we need to force the use of winPty on windows + useConpty: false, + }) + )); + + logger.info(`[SHELL-SESSION]: PTY for ${terminalId} is ${resume ? "resumed" : "started"} with PID=${shellProcess.pid}`); + + return { + callWasSuccessful: true, + response: { shellProcess, resume }, + }; + } catch (error) { + logger.warn(`[SHELL-SESSION]: Failed to start PTY for ${terminalId}: ${error}`, { shell }); + + return { + callWasSuccessful: false, + error: String(error), + }; + } + }, + cleanup: () => { + for (const shellProcess of processes.values()) { + try { + process.kill(shellProcess.pid); + } catch { + // ignore error + } + } + + processes.clear(); + }, + clear: (terminalId) => { + processes.delete(terminalId); + }, + }; + }, +}); + +export default shellProcessesInjectable; diff --git a/src/main/shell-session/shell-session.ts b/src/main/shell-session/shell-session.ts index 8a48ae6ef1..440932d144 100644 --- a/src/main/shell-session/shell-session.ts +++ b/src/main/shell-session/shell-session.ts @@ -7,18 +7,17 @@ import type { Cluster } from "../../common/cluster/cluster"; import type { Kubectl } from "../kubectl/kubectl"; import type WebSocket from "ws"; import { clearKubeconfigEnvVars } from "../utils/clear-kube-env-vars"; -import path from "path"; -import os from "os"; -import type * as pty from "node-pty"; -import { getOrInsertWith } from "../../common/utils"; import { type TerminalMessage, TerminalChannels } from "../../common/terminal/channels"; import type { Logger } from "../../common/logger"; import type { ComputeShellEnvironment } from "../../features/shell-sync/main/compute-shell-environment.injectable"; -import type { SpawnPty } from "./spawn-pty.injectable"; import type { InitializableState } from "../../common/initializable-state/create"; import type { EmitAppEvent } from "../../common/app-event-bus/emit-event.injectable"; import type { Stat } from "../../common/fs/stat.injectable"; import type { IComputedValue } from "mobx"; +import type { ShellProcesses } from "./shell-processes.injectable"; +import type { ShellEnvironmentCache } from "./shell-environment-cache.injectable"; +import type { GetBasenameOfPath } from "../../common/path/get-basename.injectable"; +import type { SpawnPty } from "./spawn-pty.injectable"; export class ShellOpenError extends Error { constructor(message: string, options?: ErrorOptions) { @@ -109,12 +108,17 @@ export interface ShellSessionDependencies { readonly isMac: boolean; readonly logger: Logger; readonly userShellSetting: IComputedValue; + readonly homeDirectory: string; readonly appName: string; readonly buildVersion: InitializableState; + readonly shellProcesses: ShellProcesses; + readonly pathDelimiter: string; computeShellEnvironment: ComputeShellEnvironment; spawnPty: SpawnPty; emitAppEvent: EmitAppEvent; stat: Stat; + shellEnvironmentCache: ShellEnvironmentCache; + getBasenameOfPath: GetBasenameOfPath; } export interface ShellSessionArgs { @@ -127,25 +131,6 @@ export interface ShellSessionArgs { export abstract class ShellSession { abstract readonly ShellType: string; - private static readonly shellEnvs = new Map>(); - private static readonly processes = new Map(); - - /** - * Kill all remaining shell backing processes. Should be called when about to - * quit - */ - public static cleanup(): void { - for (const shellProcess of this.processes.values()) { - try { - process.kill(shellProcess.pid); - } catch { - // ignore error - } - } - - this.processes.clear(); - } - protected running = false; protected readonly kubectlBinDirP: Promise; protected readonly kubeconfigPathP: Promise; @@ -156,36 +141,6 @@ export abstract class ShellSession { protected abstract get cwd(): string | undefined; - protected ensureShellProcess(shell: string, args: string[], env: Partial>, cwd: string): { shellProcess: pty.IPty; resume: boolean } | null { - try { - const resume = ShellSession.processes.has(this.terminalId); - - const shellProcess = getOrInsertWith(ShellSession.processes, this.terminalId, () => ( - this.dependencies.spawnPty(shell, args, { - rows: 30, - cols: 80, - cwd, - env, - name: "xterm-256color", - // TODO: Something else is broken here so we need to force the use of winPty on windows - useConpty: false, - }) - )); - - this.dependencies.logger.info(`[SHELL-SESSION]: PTY for ${this.terminalId} is ${resume ? "resumed" : "started"} with PID=${shellProcess.pid}`); - - return { shellProcess, resume }; - } catch (error) { - this.send({ - type: TerminalChannels.ERROR, - data: `Failed to start shell (${shell}): ${error}`, - }); - this.dependencies.logger.warn(`[SHELL-SESSION]: Failed to start PTY for ${this.terminalId}: ${error}`, { shell }); - - return null; - } - } - constructor(protected readonly dependencies: ShellSessionDependencies, { kubectl, websocket, cluster, tabId: terminalId }: ShellSessionArgs) { this.kubectl = kubectl; this.websocket = websocket; @@ -205,13 +160,13 @@ export abstract class ShellSession { if (this.dependencies.isWindows) { cwdOptions.push( env.USERPROFILE, - os.homedir(), + this.dependencies.homeDirectory, "C:\\", ); } else { cwdOptions.push( env.HOME, - os.homedir(), + this.dependencies.homeDirectory, ); if (this.dependencies.isMac) { @@ -242,20 +197,34 @@ export abstract class ShellSession { protected async openShellProcess(shell: string, args: string[], env: Record) { const cwd = await this.getCwd(env); - const ensured = this.ensureShellProcess(shell, args, env, cwd); + const result = this.dependencies.shellProcesses.startOrResume({ + terminalId: this.terminalId, + shell, + args, + env, + cwd, + }); + + if (!result.callWasSuccessful) { + this.send({ + type: TerminalChannels.ERROR, + data: `Failed to start shell (${shell}): ${result.error}`, + }); - if (!ensured) { return; } - const { shellProcess, resume } = ensured; + const { shellProcess, resume } = result.response; if (resume) { this.send({ type: TerminalChannels.CONNECTED }); } this.running = true; - shellProcess.onData(data => this.send({ type: TerminalChannels.STDOUT, data })); + shellProcess.onData(data => { + console.log("sending", { data }); + this.send({ type: TerminalChannels.STDOUT, data }); + }); shellProcess.onExit(({ exitCode }) => { this.dependencies.logger.info(`[SHELL-SESSION]: shell has exited for ${this.terminalId} closed with exitcode=${exitCode}`); @@ -322,8 +291,8 @@ export abstract class ShellSession { try { this.dependencies.logger.info(`[SHELL-SESSION]: Killing shell process (pid=${shellProcess.pid}) for ${this.terminalId}`); + this.dependencies.shellProcesses.clear(this.terminalId); shellProcess.kill(); - ShellSession.processes.delete(this.terminalId); } catch (error) { this.dependencies.logger.warn(`[SHELL-SESSION]: failed to kill shell process (pid=${shellProcess.pid}) for ${this.terminalId}`, error); } @@ -338,21 +307,7 @@ export abstract class ShellSession { } protected async getCachedShellEnv() { - const { id: clusterId } = this.cluster; - - let env = ShellSession.shellEnvs.get(clusterId); - - if (!env) { - env = await this.getShellEnv(); - ShellSession.shellEnvs.set(clusterId, env); - } else { - // refresh env in the background - this.getShellEnv().then((shellEnv: any) => { - ShellSession.shellEnvs.set(clusterId, shellEnv); - }); - } - - return env; + return this.dependencies.shellEnvironmentCache(this.cluster.id, () => this.getShellEnv()); } protected async getShellEnv() { @@ -367,13 +322,13 @@ export abstract class ShellSession { })(); const env = clearKubeconfigEnvVars(JSON.parse(JSON.stringify(rawEnv))); - const pathStr = [await this.kubectlBinDirP, ...this.getPathEntries(), env.PATH].join(path.delimiter); + const pathStr = [await this.kubectlBinDirP, ...this.getPathEntries(), env.PATH].join(this.dependencies.pathDelimiter); delete env.DEBUG; // don't pass DEBUG into shells + env.PTYSHELL = shell; + env.PATH = pathStr; if (this.dependencies.isWindows) { - env.PTYSHELL = shell || "powershell.exe"; - env.PATH = pathStr; env.LENS_SESSION = "true"; env.WSLENV = [ env.WSLENV, @@ -381,14 +336,9 @@ export abstract class ShellSession { ] .filter(Boolean) .join(":"); - } else if (shell !== undefined) { - env.PTYSHELL = shell; - env.PATH = pathStr; - } else { - env.PTYSHELL = ""; // blank runs the system default shell } - if (path.basename(env.PTYSHELL) === "zsh") { + if (this.dependencies.getBasenameOfPath(env.PTYSHELL) === "zsh") { env.OLD_ZDOTDIR = env.ZDOTDIR || env.HOME; env.ZDOTDIR = await this.kubectlBinDirP; env.DISABLE_AUTO_UPDATE = "true"; diff --git a/src/main/start-main-application/runnables/clean-up-shell-sessions.injectable.ts b/src/main/start-main-application/runnables/clean-up-shell-sessions.injectable.ts index 46afa5e692..2db4f0eccf 100644 --- a/src/main/start-main-application/runnables/clean-up-shell-sessions.injectable.ts +++ b/src/main/start-main-application/runnables/clean-up-shell-sessions.injectable.ts @@ -4,15 +4,19 @@ */ import { getInjectable } from "@ogre-tools/injectable"; import { beforeQuitOfBackEndInjectionToken } from "../runnable-tokens/before-quit-of-back-end-injection-token"; -import { ShellSession } from "../../shell-session/shell-session"; +import shellProcessesInjectable from "../../shell-session/shell-processes.injectable"; const cleanUpShellSessionsInjectable = getInjectable({ id: "clean-up-shell-sessions", - instantiate: () => ({ - id: "clean-up-shell-sessions", - run: () => void ShellSession.cleanup(), - }), + instantiate: (di) => { + const shellProcesses = di.inject(shellProcessesInjectable); + + return { + id: "clean-up-shell-sessions", + run: () => void shellProcesses.cleanup(), + }; + }, injectionToken: beforeQuitOfBackEndInjectionToken, }); diff --git a/src/main/start-main-application/runnables/kube-config-sync/start-kube-config-sync.injectable.ts b/src/main/start-main-application/runnables/kube-config-sync/start-kube-config-sync.injectable.ts index 1dbadc4246..da517b0a21 100644 --- a/src/main/start-main-application/runnables/kube-config-sync/start-kube-config-sync.injectable.ts +++ b/src/main/start-main-application/runnables/kube-config-sync/start-kube-config-sync.injectable.ts @@ -5,7 +5,7 @@ import { getInjectable } from "@ogre-tools/injectable"; import { afterApplicationIsLoadedInjectionToken } from "../../runnable-tokens/after-application-is-loaded-injection-token"; import directoryForKubeConfigsInjectable from "../../../../common/app-paths/directory-for-kube-configs/directory-for-kube-configs.injectable"; -import ensureDirInjectable from "../../../../common/fs/ensure-dir.injectable"; +import ensureDirectoryInjectable from "../../../../common/fs/ensure-directory.injectable"; import kubeconfigSyncManagerInjectable from "../../../catalog-sources/kubeconfig-sync/manager.injectable"; import addKubeconfigSyncAsEntitySourceInjectable from "./add-source.injectable"; @@ -15,7 +15,7 @@ const startKubeConfigSyncInjectable = getInjectable({ instantiate: (di) => { const directoryForKubeConfigs = di.inject(directoryForKubeConfigsInjectable); const kubeConfigSyncManager = di.inject(kubeconfigSyncManagerInjectable); - const ensureDir = di.inject(ensureDirInjectable); + const ensureDir = di.inject(ensureDirectoryInjectable); return { id: "start-kubeconfig-sync", diff --git a/src/renderer/api/create-terminal-api.injectable.ts b/src/renderer/api/create-terminal-api.injectable.ts index fca8c75100..cd8610bcba 100644 --- a/src/renderer/api/create-terminal-api.injectable.ts +++ b/src/renderer/api/create-terminal-api.injectable.ts @@ -3,8 +3,10 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ import { getInjectable } from "@ogre-tools/injectable"; +import assert from "assert"; import loggerInjectable from "../../common/logger.injectable"; import requestShellApiTokenInjectable from "../../features/terminal/renderer/request-shell-api-token.injectable"; +import hostedClusterIdInjectable from "../cluster-frame-context/hosted-cluster-id.injectable"; import currentLocationInjectable from "./current-location.injectable"; import defaultWebsocketApiParamsInjectable from "./default-websocket-params.injectable"; import type { TerminalApiDependencies, TerminalApiQuery } from "./terminal-api"; @@ -15,11 +17,16 @@ export type CreateTerminalApi = (query: TerminalApiQuery) => TerminalApi; const createTerminalApiInjectable = getInjectable({ id: "create-terminal-api", instantiate: (di): CreateTerminalApi => { + const hostedClusterId = di.inject(hostedClusterIdInjectable); + + assert(hostedClusterId, "Can only create Terminal APIs within cluster frames"); + const deps: TerminalApiDependencies = { requestShellApiToken: di.inject(requestShellApiTokenInjectable), defaultParams: di.inject(defaultWebsocketApiParamsInjectable), logger: di.inject(loggerInjectable), currentLocation: di.inject(currentLocationInjectable), + hostedClusterId, }; return (query) => new TerminalApi(deps, query); diff --git a/src/renderer/api/terminal-api.ts b/src/renderer/api/terminal-api.ts index fd0de319b5..2785e294ae 100644 --- a/src/renderer/api/terminal-api.ts +++ b/src/renderer/api/terminal-api.ts @@ -6,13 +6,13 @@ import type { WebSocketApiDependencies, WebSocketEvents } from "./websocket-api"; import { WebSocketApi } from "./websocket-api"; import isEqual from "lodash/isEqual"; -import url from "url"; import { makeObservable, observable } from "mobx"; import type { Logger } from "../../common/logger"; import { once } from "lodash"; import { type TerminalMessage, TerminalChannels } from "../../common/terminal/channels"; import type { RequestShellApiToken } from "../../features/terminal/renderer/request-shell-api-token.injectable"; import type { CurrentLocation } from "./current-location.injectable"; +import type { ClusterId } from "../../common/cluster-types"; enum TerminalColor { RED = "\u001b[31m", @@ -26,10 +26,9 @@ enum TerminalColor { NO_COLOR = "\u001b[0m", } -export interface TerminalApiQuery extends Record { +export interface TerminalApiQuery { id: string; node?: string; - type?: string; } export interface TerminalEvents extends WebSocketEvents { @@ -41,6 +40,7 @@ export interface TerminalEvents extends WebSocketEvents { export interface TerminalApiDependencies extends WebSocketApiDependencies { readonly logger: Logger; readonly currentLocation: CurrentLocation; + readonly hostedClusterId: ClusterId; requestShellApiToken: RequestShellApiToken; } @@ -59,10 +59,6 @@ export class TerminalApi extends WebSocketApi { pingInterval: 30, }); makeObservable(this); - - if (query.node) { - query.type ||= "node"; - } } async connect() { @@ -75,18 +71,19 @@ export class TerminalApi extends WebSocketApi { } const authTokenArray = await this.dependencies.requestShellApiToken(this.query.id); + const { hostname, protocol, port } = this.dependencies.currentLocation; - const socketUrl = url.format({ - protocol: protocol.includes("https") ? "wss" : "ws", - hostname, - port, - pathname: "/api", - query: { - ...this.query, - shellToken: Buffer.from(authTokenArray).toString("base64"), - }, - slashes: true, - }); + const socketProtocol = protocol.includes("https") ? "wss" : "ws"; + + const socketUrl = new URL(`${socketProtocol}://${hostname}:${port}/api`); + + socketUrl.searchParams.append("id", this.query.id); + socketUrl.searchParams.append("shellToken", Buffer.from(authTokenArray).toString("base64")); + socketUrl.searchParams.append("clusterId", this.dependencies.hostedClusterId); + + if (this.query.node) { + socketUrl.searchParams.append("node", this.query.node); + } const onReady = once((data?: string) => { this.isReady = true; @@ -111,7 +108,7 @@ export class TerminalApi extends WebSocketApi { this.prependListener("data", onReady); this.prependListener("connected", onReady); - this.connectTo(socketUrl); + this.connectTo(socketUrl.toString()); } sendMessage(message: TerminalMessage) { diff --git a/src/renderer/api/websocket-api.ts b/src/renderer/api/websocket-api.ts index 96ce7585d7..2abcdfd92c 100644 --- a/src/renderer/api/websocket-api.ts +++ b/src/renderer/api/websocket-api.ts @@ -164,7 +164,7 @@ export class WebSocketApi extends (EventEmitter this.writeLog("%cOPEN", "color:green;font-weight:bold;", evt); } - protected _onMessage({ data }: MessageEvent): void { + protected _onMessage({ data }: MessageEvent): void { (this as TypedEventEmitter).emit("data", data); this.writeLog("%cMESSAGE", "color:black;font-weight:bold;", data); } diff --git a/yarn.lock b/yarn.lock index 72c9a750f3..142c919d0a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1848,7 +1848,7 @@ dependencies: defer-to-connect "^2.0.0" -"@testing-library/dom@>=7", "@testing-library/dom@^8.0.0": +"@testing-library/dom@>=7": version "8.13.0" resolved "https://registry.yarnpkg.com/@testing-library/dom/-/dom-8.13.0.tgz#bc00bdd64c7d8b40841e27a70211399ad3af46f5" integrity sha512-9VHgfIatKNXQNaZTtLnalIy0jNZzY35a4S3oi08YAt9Hv1VsfZ/DfA45lM8D/UhtHBGJ4/lGwp0PZkVndRkoOQ== @@ -1876,6 +1876,20 @@ lz-string "^1.4.4" pretty-format "^26.6.2" +"@testing-library/dom@^8.0.0": + version "8.19.0" + resolved "https://registry.yarnpkg.com/@testing-library/dom/-/dom-8.19.0.tgz#bd3f83c217ebac16694329e413d9ad5fdcfd785f" + integrity sha512-6YWYPPpxG3e/xOo6HIWwB/58HukkwIVTOaZ0VwdMVjhRUX/01E4FtQbck9GazOOj7MXHc5RBzMrU86iBJHbI+A== + dependencies: + "@babel/code-frame" "^7.10.4" + "@babel/runtime" "^7.12.5" + "@types/aria-query" "^4.2.0" + aria-query "^5.0.0" + chalk "^4.1.0" + dom-accessibility-api "^0.5.9" + lz-string "^1.4.4" + pretty-format "^27.0.2" + "@testing-library/jest-dom@^5.16.5": version "5.16.5" resolved "https://registry.yarnpkg.com/@testing-library/jest-dom/-/jest-dom-5.16.5.tgz#3912846af19a29b2dbf32a6ae9c31ef52580074e" @@ -2457,7 +2471,14 @@ dependencies: "@types/react" "*" -"@types/react-dom@<18.0.0", "@types/react-dom@^17.0.16": +"@types/react-dom@<18.0.0": + version "17.0.17" + resolved "https://registry.yarnpkg.com/@types/react-dom/-/react-dom-17.0.17.tgz#2e3743277a793a96a99f1bf87614598289da68a1" + integrity sha512-VjnqEmqGnasQKV0CWLevqMTXBYG9GbwuE6x3VetERLh0cq2LTptFE73MrQi2S7GkKXCf2GgwItB/melLnxfnsg== + dependencies: + "@types/react" "^17" + +"@types/react-dom@^17.0.16": version "17.0.16" resolved "https://registry.yarnpkg.com/@types/react-dom/-/react-dom-17.0.16.tgz#7caba93cf2806c51e64d620d8dff4bae57e06cc4" integrity sha512-DWcXf8EbMrO/gWnQU7Z88Ws/p16qxGpPyjTKTpmBSFKeE+HveVubqGO1CVK7FrwlWD5MuOcvh8gtd0/XO38NdQ== @@ -14029,10 +14050,10 @@ xtend@^4.0.0, xtend@^4.0.2, xtend@~4.0.1: resolved "https://registry.yarnpkg.com/xtend/-/xtend-4.0.2.tgz#bb72779f5fa465186b1f438f674fa347fdb5db54" integrity sha512-LKYU1iAXJXUgAXn9URjiu+MWhyUXHsvfp7mcuYm9dSUKK0/CjtrUwFAxD82/mCWbtLsGjFIad0wIsod4zrTAEQ== -xterm-addon-fit@^0.5.0: - version "0.5.0" - resolved "https://registry.yarnpkg.com/xterm-addon-fit/-/xterm-addon-fit-0.5.0.tgz#2d51b983b786a97dcd6cde805e700c7f913bc596" - integrity sha512-DsS9fqhXHacEmsPxBJZvfj2la30Iz9xk+UKjhQgnYNkrUIN5CYLbw7WEfz117c7+S86S/tpHPfvNxJsF5/G8wQ== +xterm-addon-fit@^0.6.0: + version "0.6.0" + resolved "https://registry.yarnpkg.com/xterm-addon-fit/-/xterm-addon-fit-0.6.0.tgz#142e1ce181da48763668332593fc440349c88c34" + integrity sha512-9/7A+1KEjkFam0yxTaHfuk9LEvvTSBi0PZmEkzJqgafXPEXL9pCMAVV7rB09sX6ATRDXAdBpQhZkhKj7CGvYeg== xterm-addon-search@^0.10.0: version "0.10.0"