diff --git a/src/common/fs/read-file.injectable.ts b/src/common/fs/read-file.injectable.ts index b0a2b7233e..0dc539e1b1 100644 --- a/src/common/fs/read-file.injectable.ts +++ b/src/common/fs/read-file.injectable.ts @@ -5,11 +5,16 @@ import { getInjectable } from "@ogre-tools/injectable"; import fsInjectable from "./fs.injectable"; +export type ReadFile = (filePath: string) => Promise; + const readFileInjectable = getInjectable({ id: "read-file", - instantiate: (di) => (filePath: string) => - di.inject(fsInjectable).readFile(filePath, "utf-8"), + instantiate: (di): ReadFile => { + const { readFile } = di.inject(fsInjectable); + + return (filePath) => readFile(filePath, "utf-8"); + }, }); export default readFileInjectable; diff --git a/src/common/fs/write-file.injectable.ts b/src/common/fs/write-file.injectable.ts index 44774bea7b..faa5285ca1 100644 --- a/src/common/fs/write-file.injectable.ts +++ b/src/common/fs/write-file.injectable.ts @@ -3,21 +3,28 @@ * Licensed under MIT License. See LICENSE in root directory for more information. */ import { getInjectable } from "@ogre-tools/injectable"; +import type { WriteFileOptions } from "fs-extra"; import getDirnameOfPathInjectable from "../path/get-dirname.injectable"; import fsInjectable from "./fs.injectable"; +export type WriteFile = (filePath: string, content: string | Buffer, opts?: WriteFileOptions) => Promise; + const writeFileInjectable = getInjectable({ id: "write-file", - instantiate: (di) => { + instantiate: (di): WriteFile => { const { writeFile, ensureDir } = di.inject(fsInjectable); const getDirnameOfPath = di.inject(getDirnameOfPathInjectable); - return async (filePath: string, content: string | Buffer) => { - await ensureDir(getDirnameOfPath(filePath), { mode: 0o755 }); + return async (filePath, content, opts) => { + await ensureDir(getDirnameOfPath(filePath), { + mode: 0o755, + ...(opts ?? {}), + }); await writeFile(filePath, content, { encoding: "utf-8", + ...(opts ?? {}), }); }; }, diff --git a/src/main/__test__/kubeconfig-manager.test.ts b/src/main/__test__/kubeconfig-manager.test.ts index 9cac0063ee..6a33093cb4 100644 --- a/src/main/__test__/kubeconfig-manager.test.ts +++ b/src/main/__test__/kubeconfig-manager.test.ts @@ -4,12 +4,7 @@ */ import { getDiForUnitTesting } from "../getDiForUnitTesting"; import { KubeconfigManager } from "../kubeconfig-manager/kubeconfig-manager"; -import mockFs from "mock-fs"; import type { Cluster } from "../../common/cluster/cluster"; -import fse from "fs-extra"; -import { loadYaml } from "@kubernetes/client-node"; -import { Console } from "console"; -import * as path from "path"; import createKubeconfigManagerInjectable from "../kubeconfig-manager/create-kubeconfig-manager.injectable"; import { createClusterInjectionToken } from "../../common/cluster/create-cluster-injection-token"; import directoryForTempInjectable from "../../common/app-paths/directory-for-temp/directory-for-temp.injectable"; @@ -18,31 +13,53 @@ import type { DiContainer } from "@ogre-tools/injectable"; import { parse } from "url"; import loggerInjectable from "../../common/logger.injectable"; import type { Logger } from "../../common/logger"; -import assert from "assert"; import directoryForUserDataInjectable from "../../common/app-paths/directory-for-user-data/directory-for-user-data.injectable"; import normalizedPlatformInjectable from "../../common/vars/normalized-platform.injectable"; import kubectlBinaryNameInjectable from "../kubectl/binary-name.injectable"; import kubectlDownloadingNormalizedArchInjectable from "../kubectl/normalized-arch.injectable"; - -console = new Console(process.stdout, process.stderr); // fix mockFS +import type { ReadFile } from "../../common/fs/read-file.injectable"; +import readFileInjectable from "../../common/fs/read-file.injectable"; +import type { AsyncFnMock } from "@async-fn/jest"; +import asyncFn from "@async-fn/jest"; +import type { WriteFile } from "../../common/fs/write-file.injectable"; +import writeFileInjectable from "../../common/fs/write-file.injectable"; +import type { PathExists } from "../../common/fs/path-exists.injectable"; +import pathExistsInjectable from "../../common/fs/path-exists.injectable"; +import type { DeleteFile } from "../../common/fs/delete-file.injectable"; +import deleteFileInjectable from "../../common/fs/delete-file.injectable"; const clusterServerUrl = "https://192.168.64.3:8443"; describe("kubeconfig manager tests", () => { let clusterFake: Cluster; - let createKubeconfigManager: (cluster: Cluster) => KubeconfigManager | undefined; + let createKubeconfigManager: (cluster: Cluster) => KubeconfigManager; let di: DiContainer; let loggerMock: jest.Mocked; + let readFileMock: AsyncFnMock; + let deleteFileMock: AsyncFnMock; + let writeFileMock: AsyncFnMock; + let pathExistsMock: AsyncFnMock; + let kubeConfManager: KubeconfigManager; + let ensureServerMock: AsyncFnMock<() => Promise>; beforeEach(async () => { di = getDiForUnitTesting({ doGeneralOverrides: true }); - di.override(directoryForTempInjectable, () => "some-directory-for-temp"); - di.override(directoryForUserDataInjectable, () => "some-directory-for-user-data"); + di.override(directoryForTempInjectable, () => "/some-directory-for-temp"); + di.override(directoryForUserDataInjectable, () => "/some-directory-for-user-data"); di.override(kubectlBinaryNameInjectable, () => "kubectl"); di.override(kubectlDownloadingNormalizedArchInjectable, () => "amd64"); di.override(normalizedPlatformInjectable, () => "darwin"); + readFileMock = asyncFn(); + di.override(readFileInjectable, () => readFileMock); + writeFileMock = asyncFn(); + di.override(writeFileInjectable, () => writeFileMock); + pathExistsMock = asyncFn(); + di.override(pathExistsInjectable, () => pathExistsMock); + deleteFileMock = asyncFn(); + di.override(deleteFileInjectable, () => deleteFileMock); + loggerMock = { warn: jest.fn(), debug: jest.fn(), @@ -53,29 +70,7 @@ describe("kubeconfig manager tests", () => { di.override(loggerInjectable, () => loggerMock); - mockFs({ - "minikube-config.yml": JSON.stringify({ - apiVersion: "v1", - clusters: [{ - name: "minikube", - cluster: { - server: clusterServerUrl, - }, - }], - contexts: [{ - context: { - cluster: "minikube", - user: "minikube", - }, - name: "minikube", - }], - users: [{ - name: "minikube", - }], - kind: "Config", - preferences: {}, - }), - }); + ensureServerMock = asyncFn(); di.override(createContextHandlerInjectable, () => (cluster) => ({ restartServer: jest.fn(), @@ -86,7 +81,7 @@ describe("kubeconfig manager tests", () => { resolveAuthProxyCa: jest.fn(), resolveAuthProxyUrl: jest.fn(), setupPrometheus: jest.fn(), - ensureServer: jest.fn(), + ensureServer: ensureServerMock, })); const createCluster = di.inject(createClusterInjectionToken); @@ -96,48 +91,222 @@ describe("kubeconfig manager tests", () => { clusterFake = createCluster({ id: "foo", contextName: "minikube", - kubeConfigPath: "minikube-config.yml", + kubeConfigPath: "/minikube-config.yml", }, { clusterServerUrl, }); jest.spyOn(KubeconfigManager.prototype, "resolveProxyUrl", "get").mockReturnValue("http://127.0.0.1:9191/foo"); + + kubeConfManager = createKubeconfigManager(clusterFake); }); - afterEach(() => { - mockFs.restore(); + describe("when calling clear", () => { + it("should resolve immediately", async () => { + await kubeConfManager.clear(); + }); + + it("being called several times shouldn't throw", async () => { + await kubeConfManager.clear(); + await kubeConfManager.clear(); + await kubeConfManager.clear(); + }); }); - it("should create 'temp' kube config with proxy", async () => { - const kubeConfManager = createKubeconfigManager(clusterFake); + describe("when getPath() is called initially", () => { + let getPathPromise: Promise; - assert(kubeConfManager, "should actually create one"); + beforeEach(async () => { + getPathPromise = kubeConfManager.getPath(); + }); - expect(loggerMock.error).not.toBeCalled(); - expect(await kubeConfManager.getPath()).toBe(`some-directory-for-temp${path.sep}kubeconfig-foo`); - // this causes an intermittent "ENXIO: no such device or address, read" error - // const file = await fse.readFile(await kubeConfManager.getPath()); - const file = fse.readFileSync(await kubeConfManager.getPath()); - const yml = loadYaml(file.toString()); + it("should not call pathExists()", () => { + expect(pathExistsMock).not.toBeCalled(); + }); - expect(yml["current-context"]).toBe("minikube"); - expect(yml["clusters"][0]["cluster"]["server"].endsWith("/foo")).toBe(true); - expect(yml["users"][0]["name"]).toBe("proxy"); - }); + it("should call ensureServer on the cluster context", () => { + expect(ensureServerMock).toBeCalledTimes(1); + }); - it("should remove 'temp' kube config on unlink and remove reference from inside class", async () => { - const kubeConfManager = createKubeconfigManager(clusterFake); + describe("when ensureServer resolves", () => { + beforeEach(async () => { + await ensureServerMock.resolve(); - assert(kubeConfManager, "should actually create one"); + // clear state of calls + ensureServerMock.mock.calls.length = 0; + }); - const configPath = await kubeConfManager.getPath(); + describe("when reading cluster's kubeconfig resolves", () => { + beforeEach(async () => { + await readFileMock.resolveSpecific( + ["/minikube-config.yml"], + JSON.stringify({ + apiVersion: "v1", + clusters: [{ + name: "minikube", + cluster: { + server: clusterServerUrl, + }, + }], + contexts: [{ + context: { + cluster: "minikube", + user: "minikube", + }, + name: "minikube", + }], + users: [{ + name: "minikube", + }], + kind: "Config", + preferences: {}, + }), + ); + }); - expect(await fse.pathExists(configPath)).toBe(true); - await kubeConfManager.clear(); - expect(await fse.pathExists(configPath)).toBe(false); - await kubeConfManager.clear(); // doesn't throw - expect(async () => { - await kubeConfManager.getPath(); - }).rejects.toThrow("already unlinked"); + describe("when writing out new proxy kubeconfig resolves", () => { + beforeEach(async () => { + await writeFileMock.resolveSpecific( + ["/some-directory-for-temp/kubeconfig-foo", "apiVersion: v1\nkind: Config\npreferences: {}\ncurrent-context: minikube\nclusters:\n - name: minikube\n cluster:\n server: http://127.0.0.1:9191/foo\ncontexts:\n - name: minikube\n context:\n cluster: minikube\n user: proxy\nusers:\n - name: proxy\n user: {}\n"], + ); + }); + + it("should allow getPath to resolve with the path to the kubeconfig", async () => { + expect(await getPathPromise).toBe("/some-directory-for-temp/kubeconfig-foo"); + }); + + describe("when calling clear", () => { + let clearPromise: Promise; + + beforeEach(() => { + clearPromise = kubeConfManager.clear(); + }); + + it("should call deleteFile", () => { + expect(deleteFileMock).toBeCalledTimes(1); + }); + + describe("when deleteFile resolves", () => { + beforeEach(async () => { + await deleteFileMock.resolveSpecific( + ["/some-directory-for-temp/kubeconfig-foo"], + ); + }); + + it("should allow clear to resolve", async () => { + await clearPromise; + }); + }); + + describe("when deleteFile rejects with ENOENT", () => { + beforeEach(async () => { + await deleteFileMock.resolveSpecific( + ["/some-directory-for-temp/kubeconfig-foo"], + Promise.reject(Object.assign(new Error("file not found"), { + code: "ENOENT", + })), + ); + }); + + it("should allow clear to resolve", async () => { + await clearPromise; + }); + }); + + it("when deleteFile rejects with some other error; clear should also reject", async () => { + const expectPromise = expect(clearPromise).rejects.toBeDefined(); + + await deleteFileMock.reject(new Error("some other error")); + await expectPromise; + }); + }); + + describe("when calling getPath a second time", () => { + let getPathPromise: Promise; + + beforeEach(async () => { + getPathPromise = kubeConfManager.getPath(); + }); + + it("should call pathExists", () => { + expect(pathExistsMock).toBeCalledTimes(1); + }); + + describe("when pathExists resoves to true", () => { + beforeEach(async () => { + await pathExistsMock.resolveSpecific( + ["/some-directory-for-temp/kubeconfig-foo"], + true, + ); + }); + + it("always getPath to resolve with path", async () => { + expect(await getPathPromise).toBe("/some-directory-for-temp/kubeconfig-foo"); + }); + }); + + describe("when pathExists resoves to false", () => { + beforeEach(async () => { + await pathExistsMock.resolveSpecific( + ["/some-directory-for-temp/kubeconfig-foo"], + false, + ); + }); + + it("should call ensureServer on the cluster context", () => { + expect(ensureServerMock).toBeCalledTimes(1); + }); + + describe("when ensureServer resolves", () => { + beforeEach(async () => { + await ensureServerMock.resolve(); + }); + + describe("when reading cluster's kubeconfig resolves", () => { + beforeEach(async () => { + await readFileMock.resolveSpecific( + ["/minikube-config.yml"], + JSON.stringify({ + apiVersion: "v1", + clusters: [{ + name: "minikube", + cluster: { + server: clusterServerUrl, + }, + }], + contexts: [{ + context: { + cluster: "minikube", + user: "minikube", + }, + name: "minikube", + }], + users: [{ + name: "minikube", + }], + kind: "Config", + preferences: {}, + }), + ); + }); + + describe("when writing out new proxy kubeconfig resolves", () => { + beforeEach(async () => { + await writeFileMock.resolveSpecific( + ["/some-directory-for-temp/kubeconfig-foo", "apiVersion: v1\nkind: Config\npreferences: {}\ncurrent-context: minikube\nclusters:\n - name: minikube\n cluster:\n server: http://127.0.0.1:9191/foo\ncontexts:\n - name: minikube\n context:\n cluster: minikube\n user: proxy\nusers:\n - name: proxy\n user: {}\n"], + ); + }); + + it("should allow getPath to resolve with the path to the kubeconfig", async () => { + expect(await getPathPromise).toBe("/some-directory-for-temp/kubeconfig-foo"); + }); + }); + }); + }); + }); + }); + }); + }); + }); }); }); diff --git a/src/main/kubeconfig-manager/create-kubeconfig-manager.injectable.ts b/src/main/kubeconfig-manager/create-kubeconfig-manager.injectable.ts index 010e6e1446..ed75943398 100644 --- a/src/main/kubeconfig-manager/create-kubeconfig-manager.injectable.ts +++ b/src/main/kubeconfig-manager/create-kubeconfig-manager.injectable.ts @@ -11,6 +11,9 @@ import loggerInjectable from "../../common/logger.injectable"; import lensProxyPortInjectable from "../lens-proxy/lens-proxy-port.injectable"; import joinPathsInjectable from "../../common/path/join-paths.injectable"; import getDirnameOfPathInjectable from "../../common/path/get-dirname.injectable"; +import deleteFileInjectable from "../../common/fs/delete-file.injectable"; +import pathExistsInjectable from "../../common/fs/path-exists.injectable"; +import writeFileInjectable from "../../common/fs/write-file.injectable"; export interface KubeConfigManagerInstantiationParameter { cluster: Cluster; @@ -28,6 +31,9 @@ const createKubeconfigManagerInjectable = getInjectable({ lensProxyPort: di.inject(lensProxyPortInjectable), joinPaths: di.inject(joinPathsInjectable), getDirnameOfPath: di.inject(getDirnameOfPathInjectable), + deleteFile: di.inject(deleteFileInjectable), + pathExists: di.inject(pathExistsInjectable), + writeFile: di.inject(writeFileInjectable), }; return (cluster) => new KubeconfigManager(dependencies, cluster); diff --git a/src/main/kubeconfig-manager/kubeconfig-manager.ts b/src/main/kubeconfig-manager/kubeconfig-manager.ts index e2c1aa1f6d..a51599169e 100644 --- a/src/main/kubeconfig-manager/kubeconfig-manager.ts +++ b/src/main/kubeconfig-manager/kubeconfig-manager.ts @@ -6,13 +6,15 @@ import type { KubeConfig } from "@kubernetes/client-node"; import type { Cluster } from "../../common/cluster/cluster"; import type { ClusterContextHandler } from "../context-handler/context-handler"; -import fs from "fs-extra"; import { dumpConfigYaml } from "../../common/kube-helpers"; import { isErrnoException } from "../../common/utils"; import type { PartialDeep } from "type-fest"; import type { Logger } from "../../common/logger"; import type { JoinPaths } from "../../common/path/join-paths.injectable"; import type { GetDirnameOfPath } from "../../common/path/get-dirname.injectable"; +import type { PathExists } from "../../common/fs/path-exists.injectable"; +import type { DeleteFile } from "../../common/fs/delete-file.injectable"; +import type { WriteFile } from "../../common/fs/write-file.injectable"; export interface KubeconfigManagerDependencies { readonly directoryForTemp: string; @@ -20,6 +22,9 @@ export interface KubeconfigManagerDependencies { readonly lensProxyPort: { get: () => number }; joinPaths: JoinPaths; getDirnameOfPath: GetDirnameOfPath; + pathExists: PathExists; + deleteFile: DeleteFile; + writeFile: WriteFile; } export class KubeconfigManager { @@ -27,10 +32,9 @@ export class KubeconfigManager { * The path to the temp config file * * - if `string` then path - * - if `null` then not yet created - * - if `undefined` then unlinked by calling `clear()` + * - if `null` then not yet created or was cleared */ - protected tempFilePath: string | null | undefined = null; + protected tempFilePath: string | null = null; protected readonly contextHandler: ClusterContextHandler; @@ -43,11 +47,7 @@ export class KubeconfigManager { * @returns The path to the temporary kubeconfig */ async getPath(): Promise { - if (this.tempFilePath === undefined) { - throw new Error("kubeconfig is already unlinked"); - } - - if (this.tempFilePath === null || !(await fs.pathExists(this.tempFilePath))) { + if (this.tempFilePath === null || !(await this.dependencies.pathExists(this.tempFilePath))) { return await this.ensureFile(); } @@ -65,13 +65,13 @@ export class KubeconfigManager { this.dependencies.logger.info(`[KUBECONFIG-MANAGER]: Deleting temporary kubeconfig: ${this.tempFilePath}`); try { - await fs.unlink(this.tempFilePath); + await this.dependencies.deleteFile(this.tempFilePath); } catch (error) { if (isErrnoException(error) && error.code !== "ENOENT") { throw error; } } finally { - this.tempFilePath = undefined; + this.tempFilePath = null; } } @@ -81,7 +81,7 @@ export class KubeconfigManager { return this.tempFilePath = await this.createProxyKubeconfig(); } catch (error) { - throw Object.assign(new Error("Failed to creat temp config for auth-proxy"), { cause: error }); + throw new Error(`Failed to creat temp config for auth-proxy: ${error}`); } } @@ -124,8 +124,7 @@ export class KubeconfigManager { // write const configYaml = dumpConfigYaml(proxyConfig); - await fs.ensureDir(this.dependencies.getDirnameOfPath(tempFile)); - await fs.writeFile(tempFile, configYaml, { mode: 0o600 }); + await this.dependencies.writeFile(tempFile, configYaml, { mode: 0o600 }); this.dependencies.logger.debug(`[KUBECONFIG-MANAGER]: Created temp kubeconfig "${contextName}" at "${tempFile}": \n${configYaml}`); return tempFile;