From 9b2031159dd529595002ad78953a077f633fc632 Mon Sep 17 00:00:00 2001 From: Sebastian Malton Date: Mon, 12 Jul 2021 12:05:05 -0400 Subject: [PATCH] Ignore extensions' clusters when adding to kubeConfigSync (#3313) --- src/common/__tests__/user-store.test.ts | 39 +++++-- src/common/utils/__tests__/paths.test.ts | 130 ++++++++++++++++++++++ src/common/utils/paths.ts | 32 ++++++ src/migrations/user-store/5.0.3-beta.1.ts | 13 ++- 4 files changed, 201 insertions(+), 13 deletions(-) create mode 100644 src/common/utils/__tests__/paths.test.ts diff --git a/src/common/__tests__/user-store.test.ts b/src/common/__tests__/user-store.test.ts index 6e5a6433ef..c5a15aec28 100644 --- a/src/common/__tests__/user-store.test.ts +++ b/src/common/__tests__/user-store.test.ts @@ -19,10 +19,6 @@ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -import { Console } from "console"; - -console = new Console(process.stdout, process.stderr); - import mockFs from "mock-fs"; jest.mock("electron", () => { @@ -37,27 +33,27 @@ jest.mock("electron", () => { }); import { UserStore } from "../user-store"; +import { Console } from "console"; import { SemVer } from "semver"; import electron from "electron"; import { stdout, stderr } from "process"; import { beforeEachWrapped } from "../../../integration/helpers/utils"; import { ThemeStore } from "../../renderer/theme.store"; +import type { ClusterStoreModel } from "../cluster-store"; console = new Console(stdout, stderr); describe("user store tests", () => { describe("for an empty config", () => { beforeEachWrapped(() => { - UserStore.resetInstance(); mockFs({ tmp: { "config.json": "{}", "kube_config": "{}" } }); (UserStore.createInstance() as any).refreshNewContexts = jest.fn(() => Promise.resolve()); - - UserStore.getInstance(); }); afterEach(() => { mockFs.restore(); + UserStore.resetInstance(); }); it("allows setting and retrieving lastSeenAppVersion", () => { @@ -99,14 +95,31 @@ describe("user store tests", () => { describe("migrations", () => { beforeEachWrapped(() => { - UserStore.resetInstance(); mockFs({ "tmp": { "config.json": JSON.stringify({ user: { username: "foobar" }, preferences: { colorTheme: "light" }, lastSeenAppVersion: "1.2.3" - }) + }), + "lens-cluster-store.json": JSON.stringify({ + clusters: [ + { + id: "foobar", + kubeConfigPath: "tmp/extension_data/foo/bar", + }, + { + id: "barfoo", + kubeConfigPath: "some/other/path", + }, + ] + } as ClusterStoreModel), + "extension_data": {}, + }, + "some": { + "other": { + "path": "is file", + } } }); @@ -114,6 +127,7 @@ describe("user store tests", () => { }); afterEach(() => { + UserStore.resetInstance(); mockFs.restore(); }); @@ -122,5 +136,12 @@ describe("user store tests", () => { expect(us.lastSeenAppVersion).toBe("0.0.0"); }); + + it.only("skips clusters for adding to kube-sync with files under extension_data/", () => { + const us = UserStore.getInstance(); + + expect(us.syncKubeconfigEntries.has("tmp/extension_data/foo/bar")).toBe(false); + expect(us.syncKubeconfigEntries.has("some/other/path")).toBe(true); + }); }); }); diff --git a/src/common/utils/__tests__/paths.test.ts b/src/common/utils/__tests__/paths.test.ts new file mode 100644 index 0000000000..0eec289fb9 --- /dev/null +++ b/src/common/utils/__tests__/paths.test.ts @@ -0,0 +1,130 @@ +/** + * Copyright (c) 2021 OpenLens Authors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER + * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +import { describeIf } from "../../../../integration/helpers/utils"; +import { isWindows } from "../../vars"; +import { isLogicalChildPath } from "../paths"; + +describe("isLogicalChildPath", () => { + describeIf(isWindows)("windows tests", () => { + it.each([ + { + parentPath: "C:\\Foo", + testPath: "C:\\Foo\\Bar", + expected: true, + }, + { + parentPath: "C:\\Foo", + testPath: "C:\\Bar", + expected: false, + }, + { + parentPath: "C:\\Foo", + testPath: "C:/Bar", + expected: false, + }, + { + parentPath: "C:\\Foo", + testPath: "C:/Foo/Bar", + expected: true, + }, + { + parentPath: "C:\\Foo", + testPath: "D:\\Foo\\Bar", + expected: false, + }, + ])("test %#", (testData) => { + expect(isLogicalChildPath(testData.parentPath, testData.testPath)).toBe(testData.expected); + }); + }); + + describeIf(!isWindows)("posix tests", () => { + it.each([ + { + parentPath: "/foo", + testPath: "/foo", + expected: false, + }, + { + parentPath: "/foo", + testPath: "/bar", + expected: false, + }, + { + parentPath: "/foo", + testPath: "/foobar", + expected: false, + }, + { + parentPath: "/foo", + testPath: "/foo/bar", + expected: true, + }, + { + parentPath: "/foo", + testPath: "/foo/../bar", + expected: false, + }, + { + parentPath: "/foo", + testPath: "/foo/./bar", + expected: true, + }, + { + parentPath: "/foo", + testPath: "/foo/.bar", + expected: true, + }, + { + parentPath: "/foo", + testPath: "/foo/..bar", + expected: true, + }, + { + parentPath: "/foo", + testPath: "/foo/...bar", + expected: true, + }, + { + parentPath: "/foo", + testPath: "/foo/..\\.bar", + expected: true, + }, + { + parentPath: "/bar/../foo", + testPath: "/foo/bar", + expected: true, + }, + { + parentPath: "/foo", + testPath: "/foo/\\bar", + expected: true, + }, + { + parentPath: "/foo", + testPath: "./bar", + expected: false, + }, + ])("test %#", (testData) => { + expect(isLogicalChildPath(testData.parentPath, testData.testPath)).toBe(testData.expected); + }); + }); +}); diff --git a/src/common/utils/paths.ts b/src/common/utils/paths.ts index 391e1392f2..68d8f61ffa 100644 --- a/src/common/utils/paths.ts +++ b/src/common/utils/paths.ts @@ -33,3 +33,35 @@ function resolveTilde(filePath: string) { export function resolvePath(filePath: string): string { return path.resolve(resolveTilde(filePath)); } + +/** + * Checks if `testPath` represents a potential filesystem entry that would be + * logically "within" the `parentPath` directory. + * + * This function will return `true` in the above case, and `false` otherwise. + * It will return `false` if the two paths are the same (after resolving them). + * + * The function makes no FS calls and is platform dependant. Meaning that the + * results are only guaranteed to be correct for the platform you are running + * on. + * @param parentPath The known path of a directory + * @param testPath The path that is to be tested + */ +export function isLogicalChildPath(parentPath: string, testPath: string): boolean { + parentPath = path.resolve(parentPath); + testPath = path.resolve(testPath); + + if (parentPath === testPath) { + return false; + } + + while (testPath.length >= parentPath.length) { + if (testPath === parentPath) { + return true; + } + + testPath = path.dirname(testPath); + } + + return false; +} diff --git a/src/migrations/user-store/5.0.3-beta.1.ts b/src/migrations/user-store/5.0.3-beta.1.ts index 57271e30a4..ef3448116d 100644 --- a/src/migrations/user-store/5.0.3-beta.1.ts +++ b/src/migrations/user-store/5.0.3-beta.1.ts @@ -20,19 +20,21 @@ */ import { app } from "electron"; -import { existsSync, readJsonSync } from "fs-extra"; +import { existsSync, readFileSync } from "fs"; import path from "path"; import os from "os"; import { ClusterStore, ClusterStoreModel } from "../../common/cluster-store"; import type { KubeconfigSyncEntry, UserPreferencesModel } from "../../common/user-store"; import { MigrationDeclaration, migrationLog } from "../helpers"; +import { isLogicalChildPath } from "../../common/utils"; export default { version: "5.0.3-beta.1", run(store) { try { const { syncKubeconfigEntries = [], ...preferences }: UserPreferencesModel = store.get("preferences") ?? {}; - const { clusters = [] }: ClusterStoreModel = readJsonSync(path.resolve(app.getPath("userData"), "lens-cluster-store.json")) ?? {}; + const { clusters = [] }: ClusterStoreModel = JSON.parse(readFileSync(path.resolve(app.getPath("userData"), "lens-cluster-store.json"), "utf-8")) ?? {}; + const extensionDataDir = path.resolve(app.getPath("userData"), "extension_data"); const syncPaths = new Set(syncKubeconfigEntries.map(s => s.filePath)); syncPaths.add(path.join(os.homedir(), ".kube")); @@ -50,6 +52,11 @@ export default { continue; } + if (isLogicalChildPath(extensionDataDir, cluster.kubeConfigPath)) { + migrationLog(`Skipping ${cluster.id} because kubeConfigPath is placed under an extension_data folder`); + continue; + } + if (!existsSync(cluster.kubeConfigPath)) { migrationLog(`Skipping ${cluster.id} because kubeConfigPath no longer exists`); continue; @@ -64,8 +71,6 @@ export default { migrationLog("Final list of synced paths", updatedSyncEntries); store.set("preferences", { ...preferences, syncKubeconfigEntries: updatedSyncEntries }); } catch (error) { - console.log(error); - if (error.code !== "ENOENT") { // ignore files being missing throw error;