From a33327d0bb071ce4e565e583338a6317f761a040 Mon Sep 17 00:00:00 2001 From: Lauri Nevala Date: Thu, 14 Jan 2021 12:59:49 +0200 Subject: [PATCH 01/15] Ignore clusters with invalid kubeconfig Signed-off-by: Lauri Nevala --- src/common/__tests__/cluster-store.test.ts | 70 ++++++++++++++++++++++ src/common/cluster-store.ts | 13 ++-- 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/src/common/__tests__/cluster-store.test.ts b/src/common/__tests__/cluster-store.test.ts index fcbd5ebd6baf..147500871bfa 100644 --- a/src/common/__tests__/cluster-store.test.ts +++ b/src/common/__tests__/cluster-store.test.ts @@ -247,6 +247,76 @@ describe("config with existing clusters", () => { }); }); +describe("config with invalid cluster kubeconfig", () => { + beforeEach(() => { + const invalidKubeconfig = ` +apiVersion: v1 +clusters: +- cluster: + server: https://localhost + name: test2 +contexts: +- context: + cluster: test + user: test + name: test +current-context: test +kind: Config +preferences: {} +users: +- name: test + user: + token: kubeconfig-user-q4lm4:xxxyyyy +`; + + ClusterStore.resetInstance(); + const mockOpts = { + "tmp": { + "lens-cluster-store.json": JSON.stringify({ + __internal__: { + migrations: { + version: "99.99.99" + } + }, + clusters: [ + { + id: "cluster1", + kubeConfigPath: invalidKubeconfig, + contextName: "test", + preferences: { terminalCWD: "/foo" }, + workspace: "foo", + }, + { + id: "cluster2", + kubeConfig: "foo", + contextName: "foo", + preferences: { terminalCWD: "/foo" }, + workspace: "default" + }, + + ] + }) + } + }; + + mockFs(mockOpts); + clusterStore = ClusterStore.getInstance(); + + return clusterStore.load(); + }); + + afterEach(() => { + mockFs.restore(); + }); + + it("ignores clusters with invalid kubeconfig", () => { + const storedClusters = clusterStore.clustersList; + + expect(storedClusters.length).toBe(1); + expect(storedClusters[0].id).toBe("cluster2"); + }); +}); + describe("pre 2.0 config with an existing cluster", () => { beforeEach(() => { ClusterStore.resetInstance(); diff --git a/src/common/cluster-store.ts b/src/common/cluster-store.ts index d8bd28f1e84c..2792239a8489 100644 --- a/src/common/cluster-store.ts +++ b/src/common/cluster-store.ts @@ -321,10 +321,15 @@ export class ClusterStore extends BaseStore { if (cluster) { cluster.updateModel(clusterModel); } else { - cluster = new Cluster(clusterModel); - - if (!cluster.isManaged) { - cluster.enabled = true; + try { + cluster = new Cluster(clusterModel); + + if (!cluster.isManaged) { + cluster.enabled = true; + } + } catch (err) { + logger.error(`[CLUSTER-STORE] Failed to construct a cluster (context: ${clusterModel.contextName}, kubeconfig: ${clusterModel.kubeConfigPath})... Removing it from the app.`); + continue; } } newClusters.set(clusterModel.id, cluster); From be0ce02448cd89f58374168d77896328379db7e1 Mon Sep 17 00:00:00 2001 From: Lauri Nevala Date: Thu, 14 Jan 2021 13:16:51 +0200 Subject: [PATCH 02/15] Improve error message Signed-off-by: Lauri Nevala --- src/common/cluster-store.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/common/cluster-store.ts b/src/common/cluster-store.ts index 2792239a8489..01e6c0b7e536 100644 --- a/src/common/cluster-store.ts +++ b/src/common/cluster-store.ts @@ -328,7 +328,8 @@ export class ClusterStore extends BaseStore { cluster.enabled = true; } } catch (err) { - logger.error(`[CLUSTER-STORE] Failed to construct a cluster (context: ${clusterModel.contextName}, kubeconfig: ${clusterModel.kubeConfigPath})... Removing it from the app.`); + logger.error(err); + logger.error(`[CLUSTER-STORE] Failed to load a cluster (context: ${clusterModel.contextName}, kubeconfig: ${clusterModel.kubeConfigPath})... Removing it from the app. `); continue; } } From 7baaec5d275e6f09b0b1fb14184aaadf27dd3dc1 Mon Sep 17 00:00:00 2001 From: Lauri Nevala Date: Thu, 14 Jan 2021 14:57:25 +0200 Subject: [PATCH 03/15] Mark cluster as dead if kubeconfig loading fails Signed-off-by: Lauri Nevala --- src/common/__tests__/cluster-store.test.ts | 41 +++++++++++++++++----- src/common/cluster-store.ts | 16 +++------ src/main/cluster.ts | 15 ++++++-- 3 files changed, 50 insertions(+), 22 deletions(-) diff --git a/src/common/__tests__/cluster-store.test.ts b/src/common/__tests__/cluster-store.test.ts index 147500871bfa..f530030f68ed 100644 --- a/src/common/__tests__/cluster-store.test.ts +++ b/src/common/__tests__/cluster-store.test.ts @@ -6,6 +6,29 @@ import { ClusterStore } from "../cluster-store"; import { workspaceStore } from "../workspace-store"; const testDataIcon = fs.readFileSync("test-data/cluster-store-migration-icon.png"); +const kubeconfig = ` +apiVersion: v1 +clusters: +- cluster: + server: https://localhost + name: test +contexts: +- context: + cluster: test + user: test + name: foo +- context: + cluster: test + user: test + name: foo2 +current-context: test +kind: Config +preferences: {} +users: +- name: test + user: + token: kubeconfig-user-q4lm4:xxxyyyy +`; jest.mock("electron", () => { return { @@ -47,13 +70,13 @@ describe("empty config", () => { clusterStore.addCluster( new Cluster({ id: "foo", - contextName: "minikube", + contextName: "foo", preferences: { terminalCWD: "/tmp", icon: "data:image/jpeg;base64, iVBORw0KGgoAAAANSUhEUgAAA1wAAAKoCAYAAABjkf5", clusterName: "minikube" }, - kubeConfigPath: ClusterStore.embedCustomKubeConfig("foo", "fancy foo config"), + kubeConfigPath: ClusterStore.embedCustomKubeConfig("foo", kubeconfig), workspace: workspaceStore.currentWorkspaceId }) ); @@ -91,20 +114,20 @@ describe("empty config", () => { clusterStore.addClusters( new Cluster({ id: "prod", - contextName: "prod", + contextName: "foo", preferences: { clusterName: "prod" }, - kubeConfigPath: ClusterStore.embedCustomKubeConfig("prod", "fancy config"), + kubeConfigPath: ClusterStore.embedCustomKubeConfig("prod", kubeconfig), workspace: "workstation" }), new Cluster({ id: "dev", - contextName: "dev", + contextName: "foo2", preferences: { clusterName: "dev" }, - kubeConfigPath: ClusterStore.embedCustomKubeConfig("dev", "fancy config"), + kubeConfigPath: ClusterStore.embedCustomKubeConfig("dev", kubeconfig), workspace: "workstation" }) ); @@ -177,20 +200,20 @@ describe("config with existing clusters", () => { clusters: [ { id: "cluster1", - kubeConfig: "foo", + kubeConfigPath: kubeconfig, contextName: "foo", preferences: { terminalCWD: "/foo" }, workspace: "default" }, { id: "cluster2", - kubeConfig: "foo2", + kubeConfigPath: kubeconfig, contextName: "foo2", preferences: { terminalCWD: "/foo2" } }, { id: "cluster3", - kubeConfig: "foo", + kubeConfigPath: kubeconfig, contextName: "foo", preferences: { terminalCWD: "/foo" }, workspace: "foo", diff --git a/src/common/cluster-store.ts b/src/common/cluster-store.ts index 01e6c0b7e536..9c2ad43b7829 100644 --- a/src/common/cluster-store.ts +++ b/src/common/cluster-store.ts @@ -321,16 +321,10 @@ export class ClusterStore extends BaseStore { if (cluster) { cluster.updateModel(clusterModel); } else { - try { - cluster = new Cluster(clusterModel); - - if (!cluster.isManaged) { - cluster.enabled = true; - } - } catch (err) { - logger.error(err); - logger.error(`[CLUSTER-STORE] Failed to load a cluster (context: ${clusterModel.contextName}, kubeconfig: ${clusterModel.kubeConfigPath})... Removing it from the app. `); - continue; + cluster = new Cluster(clusterModel); + + if (!cluster.isManaged && !cluster.isDead) { + cluster.enabled = true; } } newClusters.set(clusterModel.id, cluster); @@ -343,7 +337,7 @@ export class ClusterStore extends BaseStore { } }); - this.activeCluster = newClusters.has(activeCluster) ? activeCluster : null; + this.activeCluster = newClusters.has(activeCluster) && newClusters.get(activeCluster).enabled ? activeCluster : null; this.clusters.replace(newClusters); this.removedClusters.replace(removedClusters); } diff --git a/src/main/cluster.ts b/src/main/cluster.ts index b9ff62e8ac9b..b4037151faea 100644 --- a/src/main/cluster.ts +++ b/src/main/cluster.ts @@ -169,6 +169,12 @@ export class Cluster implements ClusterModel, ClusterState { * @observable */ @observable isAdmin = false; + /** + * Is cluster marked as dead, for example due the invalid kubeconfig + * + * @observable + */ + @observable isDead = false; /** * Preferences * @@ -242,10 +248,15 @@ export class Cluster implements ClusterModel, ClusterState { constructor(model: ClusterModel) { this.updateModel(model); - const kubeconfig = this.getKubeconfig(); - if (kubeconfig.getContextObject(this.contextName)) { + try { + const kubeconfig = this.getKubeconfig(); + this.apiUrl = kubeconfig.getCluster(kubeconfig.getContextObject(this.contextName).cluster).server; + } catch(err) { + logger.error(err); + logger.error(`[CLUSTER] Failed to load kubeconfig for the cluster (context: ${this.contextName}, kubeconfig: ${this.kubeConfigPath}).`); + this.isDead = true; } } From 6dd73748997aa2817552177ec81e476623ba5a84 Mon Sep 17 00:00:00 2001 From: Lauri Nevala Date: Thu, 14 Jan 2021 15:04:32 +0200 Subject: [PATCH 04/15] Fix tests Signed-off-by: Lauri Nevala --- src/common/__tests__/cluster-store.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/common/__tests__/cluster-store.test.ts b/src/common/__tests__/cluster-store.test.ts index f530030f68ed..4cdf5cd0b7d6 100644 --- a/src/common/__tests__/cluster-store.test.ts +++ b/src/common/__tests__/cluster-store.test.ts @@ -311,7 +311,7 @@ users: }, { id: "cluster2", - kubeConfig: "foo", + kubeConfigPath: kubeconfig, contextName: "foo", preferences: { terminalCWD: "/foo" }, workspace: "default" @@ -332,11 +332,13 @@ users: mockFs.restore(); }); - it("ignores clusters with invalid kubeconfig", () => { + it("does not enable clusters with invalid kubeconfig", () => { const storedClusters = clusterStore.clustersList; - expect(storedClusters.length).toBe(1); - expect(storedClusters[0].id).toBe("cluster2"); + expect(storedClusters.length).toBe(2); + expect(storedClusters[0].enabled).toBeFalsy; + expect(storedClusters[1].id).toBe("cluster2"); + expect(storedClusters[1].enabled).toBeTruthy; }); }); From 1cbac447b63f4aa080e3140eb5d6db0db6aaa1b6 Mon Sep 17 00:00:00 2001 From: Lauri Nevala Date: Thu, 14 Jan 2021 17:10:21 +0200 Subject: [PATCH 05/15] Validate cluster object in kubeconfig when constructing cluster Signed-off-by: Lauri Nevala --- src/common/kube-helpers.ts | 40 +++++++++++++++---- src/main/cluster.ts | 5 ++- .../components/+add-cluster/add-cluster.tsx | 2 +- 3 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/common/kube-helpers.ts b/src/common/kube-helpers.ts index bb0e6b86d2a5..eb0cd4ec8a2a 100644 --- a/src/common/kube-helpers.ts +++ b/src/common/kube-helpers.ts @@ -151,18 +151,42 @@ export function getNodeWarningConditions(node: V1Node) { } /** - * Validates kubeconfig supplied in the add clusters screen. At present this will just validate - * the User struct, specifically the command passed to the exec substructure. - */ -export function validateKubeConfig (config: KubeConfig) { + * Validates kubeconfig supplied in the add clusters screen. Additionally this will just validate + * the User struct, specifically the command passed to the exec substructure. + */ +export function validateKubeConfig (config: KubeConfig, contextName: string, validationOpts?: { validateCluster?: boolean, validateUser?: boolean, validateExec?: boolean}) { // we only receive a single context, cluster & user object here so lets validate them as this // will be called when we add a new cluster to Lens logger.debug(`validateKubeConfig: validating kubeconfig - ${JSON.stringify(config)}`); + const defaultOpts = { + validateContext: true, + validateUser: true, + validateCluster: true, + validateExec: true + }; + const opts = {...defaultOpts, ...validationOpts }; + + const contextObject = config.getContextObject(contextName); + + // Validate the Context Object + if (!contextObject) { + throw new Error(`No valid context object provided in kubeconfig for context '${contextName}'`); + } + + // Validate the Cluster Object + if (opts.validateCluster && !config.getCluster(contextObject.cluster)) { + throw new Error(`No valid cluster object provided in kubeconfig for context '${contextName}'`); + } + + const user = config.getUser(contextObject.user); // Validate the User Object - const user = config.getCurrentUser(); - - if (user.exec) { + if (opts.validateUser && !user) { + throw new Error(`No valid user object provided in kubeconfig for context '${contextName}'`); + } + + // Validate exec command if present + if (opts.validateExec && user.exec) { const execCommand = user.exec["command"]; // check if the command is absolute or not const isAbsolute = path.isAbsolute(execCommand); @@ -171,7 +195,7 @@ export function validateKubeConfig (config: KubeConfig) { logger.debug(`validateKubeConfig: validating user exec command - ${JSON.stringify(execCommand)}`); if (!commandExists.sync(execCommand)) { - logger.debug(`validateKubeConfig: exec command ${String(execCommand)} in kubeconfig ${config.currentContext} not found`); + logger.debug(`validateKubeConfig: exec command ${String(execCommand)} in kubeconfig ${contextName} not found`); throw new ExecValidationNotFoundError(execCommand, isAbsolute); } } diff --git a/src/main/cluster.ts b/src/main/cluster.ts index b4037151faea..125d2ce9592c 100644 --- a/src/main/cluster.ts +++ b/src/main/cluster.ts @@ -9,7 +9,7 @@ import { ContextHandler } from "./context-handler"; import { AuthorizationV1Api, CoreV1Api, KubeConfig, V1ResourceAttributes } from "@kubernetes/client-node"; import { Kubectl } from "./kubectl"; import { KubeconfigManager } from "./kubeconfig-manager"; -import { loadConfig } from "../common/kube-helpers"; +import { loadConfig, validateKubeConfig } from "../common/kube-helpers"; import request, { RequestPromiseOptions } from "request-promise-native"; import { apiResources, KubeApiResource } from "../common/rbac"; import logger from "./logger"; @@ -252,10 +252,11 @@ export class Cluster implements ClusterModel, ClusterState { try { const kubeconfig = this.getKubeconfig(); + validateKubeConfig(kubeconfig, this.contextName, { validateCluster: true, validateUser: false, validateExec: false}); this.apiUrl = kubeconfig.getCluster(kubeconfig.getContextObject(this.contextName).cluster).server; } catch(err) { logger.error(err); - logger.error(`[CLUSTER] Failed to load kubeconfig for the cluster (context: ${this.contextName}, kubeconfig: ${this.kubeConfigPath}).`); + logger.error(`[CLUSTER] Failed to load kubeconfig for the cluster '${this.name || this.contextName}' (context: ${this.contextName}, kubeconfig: ${this.kubeConfigPath}).`); this.isDead = true; } } diff --git a/src/renderer/components/+add-cluster/add-cluster.tsx b/src/renderer/components/+add-cluster/add-cluster.tsx index ae4a3e6acec9..38d03482e829 100644 --- a/src/renderer/components/+add-cluster/add-cluster.tsx +++ b/src/renderer/components/+add-cluster/add-cluster.tsx @@ -147,7 +147,7 @@ export class AddCluster extends React.Component { try { const kubeConfig = this.kubeContexts.get(context); - validateKubeConfig(kubeConfig); + validateKubeConfig(kubeConfig, context); return true; } catch (err) { From 7c8ca4085a6fff76aa9a91f5047cd0d2dc5a74cc Mon Sep 17 00:00:00 2001 From: Lauri Nevala Date: Fri, 15 Jan 2021 11:31:09 +0200 Subject: [PATCH 06/15] Add unit tests for validateKubeConfig Signed-off-by: Lauri Nevala --- src/common/__tests__/kube-helpers.test.ts | 120 ++++++++++++++++++++++ src/common/kube-helpers.ts | 22 ++-- 2 files changed, 127 insertions(+), 15 deletions(-) create mode 100644 src/common/__tests__/kube-helpers.test.ts diff --git a/src/common/__tests__/kube-helpers.test.ts b/src/common/__tests__/kube-helpers.test.ts new file mode 100644 index 000000000000..b4e6f8deddc1 --- /dev/null +++ b/src/common/__tests__/kube-helpers.test.ts @@ -0,0 +1,120 @@ +import { KubeConfig } from "@kubernetes/client-node"; +import { validateKubeConfig } from "../kube-helpers"; + +const validKubeconfig = ` +apiVersion: v1 +clusters: +- cluster: + server: https://localhost + name: test +contexts: +- context: + cluster: test + user: test + name: valid +- context: + cluster: test2 + user: test + name: invalidCluster +- context: + cluster: test + user: test2 + name: invalidUser +- context: + cluster: test + user: invalidExec + name: invalidExec +current-context: test +kind: Config +preferences: {} +users: +- name: test + user: + exec: + command: echo +- name: invalidExec + user: + exec: + command: foo +`; + +describe("validateKubeconfig", () => { + describe("with default validation options", () => { + describe("with valid kubeconfig", () => { + it("does not raise exceptions", () => { + const kc = new KubeConfig(); + + kc.loadFromString(validKubeconfig); + expect(() => { validateKubeConfig(kc, "valid");}).not.toThrow(); + }); + }); + describe("with invalid context object", () => { + it("it raises exception", () => { + const kc = new KubeConfig(); + + kc.loadFromString(validKubeconfig); + expect(() => { validateKubeConfig(kc, "invalid");}).toThrow("No valid context object provided in kubeconfig for context 'invalid'"); + }); + }); + + describe("with invalid cluster object", () => { + it("it raises exception", () => { + const kc = new KubeConfig(); + + kc.loadFromString(validKubeconfig); + expect(() => { validateKubeConfig(kc, "invalidCluster");}).toThrow("No valid cluster object provided in kubeconfig for context 'invalidCluster'"); + }); + }); + + describe("with invalid user object", () => { + it("it raises exception", () => { + const kc = new KubeConfig(); + + kc.loadFromString(validKubeconfig); + expect(() => { validateKubeConfig(kc, "invalidUser");}).toThrow("No valid user object provided in kubeconfig for context 'invalidUser'"); + }); + }); + + describe("with invalid exec command", () => { + it("it raises exception", () => { + const kc = new KubeConfig(); + + kc.loadFromString(validKubeconfig); + expect(() => { validateKubeConfig(kc, "invalidExec");}).toThrow("User Exec command \"foo\" not found on host. Please ensure binary is found in PATH or use absolute path to binary in Kubeconfig"); + }); + }); + }); + + describe("with validateCluster as false", () => { + describe("with invalid cluster object", () => { + it("does not raise exception", () => { + const kc = new KubeConfig(); + + kc.loadFromString(validKubeconfig); + expect(() => { validateKubeConfig(kc, "invalidCluster", { validateCluster: false });}).not.toThrow(); + }); + }); + }); + + describe("with validateUser as false", () => { + describe("with invalid user object", () => { + it("does not raise excpetions", () => { + const kc = new KubeConfig(); + + kc.loadFromString(validKubeconfig); + expect(() => { validateKubeConfig(kc, "invalidUser", { validateUser: false });}).not.toThrow(); + }); + }); + }); + + describe("with validateExec as false", () => { + describe("with invalid exec object", () => { + it("does not raise excpetions", () => { + const kc = new KubeConfig(); + + kc.loadFromString(validKubeconfig); + expect(() => { validateKubeConfig(kc, "invalidExec", { validateExec: false });}).not.toThrow(); + }); + }); + }); +}); \ No newline at end of file diff --git a/src/common/kube-helpers.ts b/src/common/kube-helpers.ts index eb0cd4ec8a2a..9442ef88913e 100644 --- a/src/common/kube-helpers.ts +++ b/src/common/kube-helpers.ts @@ -151,20 +151,14 @@ export function getNodeWarningConditions(node: V1Node) { } /** - * Validates kubeconfig supplied in the add clusters screen. Additionally this will just validate - * the User struct, specifically the command passed to the exec substructure. + * Validates Context, User and Cluster sructs in given kubeconfig. Additionally this will validate + * the command passed to the exec substructure. */ export function validateKubeConfig (config: KubeConfig, contextName: string, validationOpts?: { validateCluster?: boolean, validateUser?: boolean, validateExec?: boolean}) { // we only receive a single context, cluster & user object here so lets validate them as this // will be called when we add a new cluster to Lens - logger.debug(`validateKubeConfig: validating kubeconfig - ${JSON.stringify(config)}`); - const defaultOpts = { - validateContext: true, - validateUser: true, - validateCluster: true, - validateExec: true - }; - const opts = {...defaultOpts, ...validationOpts }; + const opts = validationOpts || {}; + const { validateUser = true, validateCluster = true, validateExec = true } = opts; const contextObject = config.getContextObject(contextName); @@ -174,26 +168,24 @@ export function validateKubeConfig (config: KubeConfig, contextName: string, val } // Validate the Cluster Object - if (opts.validateCluster && !config.getCluster(contextObject.cluster)) { + if (validateCluster && !config.getCluster(contextObject.cluster)) { throw new Error(`No valid cluster object provided in kubeconfig for context '${contextName}'`); } const user = config.getUser(contextObject.user); // Validate the User Object - if (opts.validateUser && !user) { + if (validateUser && !user) { throw new Error(`No valid user object provided in kubeconfig for context '${contextName}'`); } // Validate exec command if present - if (opts.validateExec && user.exec) { + if (validateExec && user?.exec) { const execCommand = user.exec["command"]; // check if the command is absolute or not const isAbsolute = path.isAbsolute(execCommand); // validate the exec struct in the user object, start with the command field - logger.debug(`validateKubeConfig: validating user exec command - ${JSON.stringify(execCommand)}`); - if (!commandExists.sync(execCommand)) { logger.debug(`validateKubeConfig: exec command ${String(execCommand)} in kubeconfig ${contextName} not found`); throw new ExecValidationNotFoundError(execCommand, isAbsolute); From 1b963b4473be795b6ace4300cacbdfc5eb185324 Mon Sep 17 00:00:00 2001 From: Lauri Nevala Date: Fri, 15 Jan 2021 11:42:56 +0200 Subject: [PATCH 07/15] Refactor validateKubeconfig unit tests Signed-off-by: Lauri Nevala --- src/common/__tests__/kube-helpers.test.ts | 31 +++++------------------ 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/src/common/__tests__/kube-helpers.test.ts b/src/common/__tests__/kube-helpers.test.ts index b4e6f8deddc1..94a2cae10c9b 100644 --- a/src/common/__tests__/kube-helpers.test.ts +++ b/src/common/__tests__/kube-helpers.test.ts @@ -1,7 +1,7 @@ import { KubeConfig } from "@kubernetes/client-node"; import { validateKubeConfig } from "../kube-helpers"; -const validKubeconfig = ` +const kubeconfig = ` apiVersion: v1 clusters: - cluster: @@ -38,48 +38,38 @@ users: command: foo `; +const kc = new KubeConfig(); + describe("validateKubeconfig", () => { + beforeAll(() => { + kc.loadFromString(kubeconfig); + }); describe("with default validation options", () => { describe("with valid kubeconfig", () => { it("does not raise exceptions", () => { - const kc = new KubeConfig(); - - kc.loadFromString(validKubeconfig); expect(() => { validateKubeConfig(kc, "valid");}).not.toThrow(); }); }); describe("with invalid context object", () => { it("it raises exception", () => { - const kc = new KubeConfig(); - - kc.loadFromString(validKubeconfig); expect(() => { validateKubeConfig(kc, "invalid");}).toThrow("No valid context object provided in kubeconfig for context 'invalid'"); }); }); describe("with invalid cluster object", () => { it("it raises exception", () => { - const kc = new KubeConfig(); - - kc.loadFromString(validKubeconfig); expect(() => { validateKubeConfig(kc, "invalidCluster");}).toThrow("No valid cluster object provided in kubeconfig for context 'invalidCluster'"); }); }); describe("with invalid user object", () => { it("it raises exception", () => { - const kc = new KubeConfig(); - - kc.loadFromString(validKubeconfig); expect(() => { validateKubeConfig(kc, "invalidUser");}).toThrow("No valid user object provided in kubeconfig for context 'invalidUser'"); }); }); describe("with invalid exec command", () => { it("it raises exception", () => { - const kc = new KubeConfig(); - - kc.loadFromString(validKubeconfig); expect(() => { validateKubeConfig(kc, "invalidExec");}).toThrow("User Exec command \"foo\" not found on host. Please ensure binary is found in PATH or use absolute path to binary in Kubeconfig"); }); }); @@ -88,9 +78,6 @@ describe("validateKubeconfig", () => { describe("with validateCluster as false", () => { describe("with invalid cluster object", () => { it("does not raise exception", () => { - const kc = new KubeConfig(); - - kc.loadFromString(validKubeconfig); expect(() => { validateKubeConfig(kc, "invalidCluster", { validateCluster: false });}).not.toThrow(); }); }); @@ -99,9 +86,6 @@ describe("validateKubeconfig", () => { describe("with validateUser as false", () => { describe("with invalid user object", () => { it("does not raise excpetions", () => { - const kc = new KubeConfig(); - - kc.loadFromString(validKubeconfig); expect(() => { validateKubeConfig(kc, "invalidUser", { validateUser: false });}).not.toThrow(); }); }); @@ -110,9 +94,6 @@ describe("validateKubeconfig", () => { describe("with validateExec as false", () => { describe("with invalid exec object", () => { it("does not raise excpetions", () => { - const kc = new KubeConfig(); - - kc.loadFromString(validKubeconfig); expect(() => { validateKubeConfig(kc, "invalidExec", { validateExec: false });}).not.toThrow(); }); }); From a88acc13c02fc9c53113603bcd07745272c99c39 Mon Sep 17 00:00:00 2001 From: Lauri Nevala Date: Fri, 22 Jan 2021 13:39:30 +0200 Subject: [PATCH 08/15] Extract ValidationOpts type Signed-off-by: Lauri Nevala --- src/common/kube-helpers.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/common/kube-helpers.ts b/src/common/kube-helpers.ts index 9442ef88913e..1387d1aa1b95 100644 --- a/src/common/kube-helpers.ts +++ b/src/common/kube-helpers.ts @@ -7,6 +7,12 @@ import logger from "../main/logger"; import commandExists from "command-exists"; import { ExecValidationNotFoundError } from "./custom-errors"; +export type KubeConfigValidationOpts = { + validateCluster?: boolean; + validateUser?: boolean; + validateExec?: boolean; +}; + export const kubeConfigDefaultPath = path.join(os.homedir(), ".kube", "config"); function resolveTilde(filePath: string) { @@ -154,11 +160,11 @@ export function getNodeWarningConditions(node: V1Node) { * Validates Context, User and Cluster sructs in given kubeconfig. Additionally this will validate * the command passed to the exec substructure. */ -export function validateKubeConfig (config: KubeConfig, contextName: string, validationOpts?: { validateCluster?: boolean, validateUser?: boolean, validateExec?: boolean}) { +export function validateKubeConfig (config: KubeConfig, contextName: string, validationOpts?: KubeConfigValidationOpts) { // we only receive a single context, cluster & user object here so lets validate them as this // will be called when we add a new cluster to Lens - const opts = validationOpts || {}; - const { validateUser = true, validateCluster = true, validateExec = true } = opts; + + const { validateUser = true, validateCluster = true, validateExec = true } = validationOpts || {}; const contextObject = config.getContextObject(contextName); From c4d029baa1bdd938c7b2728b0f0477ff2b62c69d Mon Sep 17 00:00:00 2001 From: Lauri Nevala Date: Fri, 22 Jan 2021 13:52:43 +0200 Subject: [PATCH 09/15] Add default value to validationOpts param Signed-off-by: Lauri Nevala --- src/common/kube-helpers.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/kube-helpers.ts b/src/common/kube-helpers.ts index 1387d1aa1b95..0826edf84389 100644 --- a/src/common/kube-helpers.ts +++ b/src/common/kube-helpers.ts @@ -160,11 +160,11 @@ export function getNodeWarningConditions(node: V1Node) { * Validates Context, User and Cluster sructs in given kubeconfig. Additionally this will validate * the command passed to the exec substructure. */ -export function validateKubeConfig (config: KubeConfig, contextName: string, validationOpts?: KubeConfigValidationOpts) { +export function validateKubeConfig (config: KubeConfig, contextName: string, validationOpts: KubeConfigValidationOpts = {}) { // we only receive a single context, cluster & user object here so lets validate them as this // will be called when we add a new cluster to Lens - const { validateUser = true, validateCluster = true, validateExec = true } = validationOpts || {}; + const { validateUser = true, validateCluster = true, validateExec = true } = validationOpts; const contextObject = config.getContextObject(contextName); From 082a45162e8a89a1acf9a123ebb110ac6d9a26b7 Mon Sep 17 00:00:00 2001 From: Lauri Nevala Date: Fri, 22 Jan 2021 13:58:00 +0200 Subject: [PATCH 10/15] Change isDead to property Signed-off-by: Lauri Nevala --- src/main/cluster.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/main/cluster.ts b/src/main/cluster.ts index 125d2ce9592c..9fc0fb7cc3fc 100644 --- a/src/main/cluster.ts +++ b/src/main/cluster.ts @@ -76,6 +76,7 @@ export class Cluster implements ClusterModel, ClusterState { * If extension sets this it needs to also mark cluster as enabled on activate (or when added to a store) */ public ownerRef: string; + public isDead = false; protected kubeconfigManager: KubeconfigManager; protected eventDisposers: Function[] = []; protected activated = false; @@ -169,12 +170,7 @@ export class Cluster implements ClusterModel, ClusterState { * @observable */ @observable isAdmin = false; - /** - * Is cluster marked as dead, for example due the invalid kubeconfig - * - * @observable - */ - @observable isDead = false; + /** * Preferences * From b5e2749b924e6937472c6d2ece0aa47de3715aed Mon Sep 17 00:00:00 2001 From: Lauri Nevala Date: Fri, 22 Jan 2021 14:08:24 +0200 Subject: [PATCH 11/15] Fix lint issues Signed-off-by: Lauri Nevala --- src/common/kube-helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/kube-helpers.ts b/src/common/kube-helpers.ts index 0826edf84389..3cf4fda6de7e 100644 --- a/src/common/kube-helpers.ts +++ b/src/common/kube-helpers.ts @@ -197,4 +197,4 @@ export function validateKubeConfig (config: KubeConfig, contextName: string, val throw new ExecValidationNotFoundError(execCommand, isAbsolute); } } -} \ No newline at end of file +} From 615627cc03a959628814a38b3993d71da907c1cb Mon Sep 17 00:00:00 2001 From: Lauri Nevala Date: Fri, 22 Jan 2021 14:59:31 +0200 Subject: [PATCH 12/15] Add missing new line Signed-off-by: Lauri Nevala --- src/common/__tests__/kube-helpers.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/__tests__/kube-helpers.test.ts b/src/common/__tests__/kube-helpers.test.ts index 94a2cae10c9b..a782772d3457 100644 --- a/src/common/__tests__/kube-helpers.test.ts +++ b/src/common/__tests__/kube-helpers.test.ts @@ -98,4 +98,4 @@ describe("validateKubeconfig", () => { }); }); }); -}); \ No newline at end of file +}); From f676429e3ef0ef86db6da00cda15e3e925eddf05 Mon Sep 17 00:00:00 2001 From: Lauri Nevala Date: Thu, 25 Feb 2021 16:21:25 +0200 Subject: [PATCH 13/15] Update validateKubeConfig in-code documentation Signed-off-by: Lauri Nevala --- src/common/kube-helpers.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/common/kube-helpers.ts b/src/common/kube-helpers.ts index 3cf4fda6de7e..c2a2a8df93fc 100644 --- a/src/common/kube-helpers.ts +++ b/src/common/kube-helpers.ts @@ -157,8 +157,7 @@ export function getNodeWarningConditions(node: V1Node) { } /** - * Validates Context, User and Cluster sructs in given kubeconfig. Additionally this will validate - * the command passed to the exec substructure. + * Checks if `config` has valid `Context`, `User`, `Cluster`, and `exec` fields (if present when required) */ export function validateKubeConfig (config: KubeConfig, contextName: string, validationOpts: KubeConfigValidationOpts = {}) { // we only receive a single context, cluster & user object here so lets validate them as this From ed6e00798dae83e9902ddae30a14ace5a6849b42 Mon Sep 17 00:00:00 2001 From: Lauri Nevala Date: Mon, 1 Mar 2021 11:56:30 +0200 Subject: [PATCH 14/15] Remove isDead property Signed-off-by: Lauri Nevala --- src/common/cluster-store.ts | 4 ++-- src/main/cluster.ts | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/common/cluster-store.ts b/src/common/cluster-store.ts index 2eae7da6a881..6bf932f0f4ef 100644 --- a/src/common/cluster-store.ts +++ b/src/common/cluster-store.ts @@ -323,7 +323,7 @@ export class ClusterStore extends BaseStore { } else { cluster = new Cluster(clusterModel); - if (!cluster.isManaged && !cluster.isDead) { + if (!cluster.isManaged && cluster.apiUrl) { cluster.enabled = true; } } @@ -337,7 +337,7 @@ export class ClusterStore extends BaseStore { } }); - this.activeCluster = newClusters.has(activeCluster) && newClusters.get(activeCluster).enabled ? activeCluster : null; + this.activeCluster = newClusters.get(activeCluster)?.enabled ? activeCluster : null; this.clusters.replace(newClusters); this.removedClusters.replace(removedClusters); } diff --git a/src/main/cluster.ts b/src/main/cluster.ts index 8e7bc507e24d..93083cf7cbc3 100644 --- a/src/main/cluster.ts +++ b/src/main/cluster.ts @@ -77,7 +77,6 @@ export class Cluster implements ClusterModel, ClusterState { * If extension sets this it needs to also mark cluster as enabled on activate (or when added to a store) */ public ownerRef: string; - public isDead = false; protected kubeconfigManager: KubeconfigManager; protected eventDisposers: Function[] = []; protected activated = false; @@ -267,7 +266,6 @@ export class Cluster implements ClusterModel, ClusterState { } catch(err) { logger.error(err); logger.error(`[CLUSTER] Failed to load kubeconfig for the cluster '${this.name || this.contextName}' (context: ${this.contextName}, kubeconfig: ${this.kubeConfigPath}).`); - this.isDead = true; } } From 20c1b061de4174e6f04d9bec4bd50f661b05f6a5 Mon Sep 17 00:00:00 2001 From: Lauri Nevala Date: Mon, 1 Mar 2021 13:15:32 +0200 Subject: [PATCH 15/15] Display warning notification if invalid kubeconfig detected (#2233) * Display warning notification if invalid kubeconfig detected Signed-off-by: Lauri Nevala --- src/common/ipc/index.ts | 1 + src/common/ipc/invalid-kubeconfig/index.ts | 3 ++ src/main/cluster.ts | 3 +- .../components/dock/create-resource.tsx | 2 +- .../notifications/notifications.tsx | 5 +- src/renderer/ipc/index.tsx | 2 + .../ipc/invalid-kubeconfig-handler.tsx | 46 +++++++++++++++++++ 7 files changed, 58 insertions(+), 4 deletions(-) create mode 100644 src/common/ipc/invalid-kubeconfig/index.ts create mode 100644 src/renderer/ipc/invalid-kubeconfig-handler.tsx diff --git a/src/common/ipc/index.ts b/src/common/ipc/index.ts index a34890472edc..c5e864dc7546 100644 --- a/src/common/ipc/index.ts +++ b/src/common/ipc/index.ts @@ -1,3 +1,4 @@ export * from "./ipc"; +export * from "./invalid-kubeconfig"; export * from "./update-available"; export * from "./type-enforced-ipc"; diff --git a/src/common/ipc/invalid-kubeconfig/index.ts b/src/common/ipc/invalid-kubeconfig/index.ts new file mode 100644 index 000000000000..9e8e7921d722 --- /dev/null +++ b/src/common/ipc/invalid-kubeconfig/index.ts @@ -0,0 +1,3 @@ +export const InvalidKubeconfigChannel = "invalid-kubeconfig"; + +export type InvalidKubeConfigArgs = [clusterId: string]; diff --git a/src/main/cluster.ts b/src/main/cluster.ts index 93083cf7cbc3..198d24c2f9f8 100644 --- a/src/main/cluster.ts +++ b/src/main/cluster.ts @@ -4,7 +4,7 @@ import type { IMetricsReqParams } from "../renderer/api/endpoints/metrics.api"; import type { WorkspaceId } from "../common/workspace-store"; import { action, comparer, computed, observable, reaction, toJS, when } from "mobx"; import { apiKubePrefix } from "../common/vars"; -import { broadcastMessage } from "../common/ipc"; +import { broadcastMessage, InvalidKubeconfigChannel } from "../common/ipc"; import { ContextHandler } from "./context-handler"; import { AuthorizationV1Api, CoreV1Api, KubeConfig, V1ResourceAttributes } from "@kubernetes/client-node"; import { Kubectl } from "./kubectl"; @@ -266,6 +266,7 @@ export class Cluster implements ClusterModel, ClusterState { } catch(err) { logger.error(err); logger.error(`[CLUSTER] Failed to load kubeconfig for the cluster '${this.name || this.contextName}' (context: ${this.contextName}, kubeconfig: ${this.kubeConfigPath}).`); + broadcastMessage(InvalidKubeconfigChannel, model.id); } } diff --git a/src/renderer/components/dock/create-resource.tsx b/src/renderer/components/dock/create-resource.tsx index 8ee859d2cf8f..01e60023096c 100644 --- a/src/renderer/components/dock/create-resource.tsx +++ b/src/renderer/components/dock/create-resource.tsx @@ -52,7 +52,7 @@ export class CreateResource extends React.Component { ); if (errors.length) { - errors.forEach(Notifications.error); + errors.forEach(error => Notifications.error(error)); if (!createdResources.length) throw errors[0]; } const successMessage = ( diff --git a/src/renderer/components/notifications/notifications.tsx b/src/renderer/components/notifications/notifications.tsx index 0c1ac692cfbc..206102b1a30c 100644 --- a/src/renderer/components/notifications/notifications.tsx +++ b/src/renderer/components/notifications/notifications.tsx @@ -21,11 +21,12 @@ export class Notifications extends React.Component { }); } - static error(message: NotificationMessage) { + static error(message: NotificationMessage, customOpts: Partial = {}) { notificationsStore.add({ message, timeout: 10000, - status: NotificationStatus.ERROR + status: NotificationStatus.ERROR, + ...customOpts }); } diff --git a/src/renderer/ipc/index.tsx b/src/renderer/ipc/index.tsx index b9644f74045f..544cefbf7847 100644 --- a/src/renderer/ipc/index.tsx +++ b/src/renderer/ipc/index.tsx @@ -5,6 +5,7 @@ import { Notifications, notificationsStore } from "../components/notifications"; import { Button } from "../components/button"; import { isMac } from "../../common/vars"; import * as uuid from "uuid"; +import { invalidKubeconfigHandler } from "./invalid-kubeconfig-handler"; function sendToBackchannel(backchannel: string, notificationId: string, data: BackchannelArg): void { notificationsStore.remove(notificationId); @@ -58,4 +59,5 @@ export function registerIpcHandlers() { listener: UpdateAvailableHandler, verifier: areArgsUpdateAvailableFromMain, }); + onCorrect(invalidKubeconfigHandler); } diff --git a/src/renderer/ipc/invalid-kubeconfig-handler.tsx b/src/renderer/ipc/invalid-kubeconfig-handler.tsx new file mode 100644 index 000000000000..cadf7e4e3fdb --- /dev/null +++ b/src/renderer/ipc/invalid-kubeconfig-handler.tsx @@ -0,0 +1,46 @@ +import React from "react"; +import { ipcRenderer, IpcRendererEvent, shell } from "electron"; +import { clusterStore } from "../../common/cluster-store"; +import { InvalidKubeConfigArgs, InvalidKubeconfigChannel } from "../../common/ipc/invalid-kubeconfig"; +import { Notifications, notificationsStore } from "../components/notifications"; +import { Button } from "../components/button"; + +export const invalidKubeconfigHandler = { + source: ipcRenderer, + channel: InvalidKubeconfigChannel, + listener: InvalidKubeconfigListener, + verifier: (args: [unknown]): args is InvalidKubeConfigArgs => { + return args.length === 1 && typeof args[0] === "string" && !!clusterStore.getById(args[0]); + }, +}; + +function InvalidKubeconfigListener(event: IpcRendererEvent, ...[clusterId]: InvalidKubeConfigArgs): void { + const notificationId = `invalid-kubeconfig:${clusterId}`; + const cluster = clusterStore.getById(clusterId); + const contextName = cluster.name !== cluster.contextName ? `(context: ${cluster.contextName})` : ""; + + Notifications.error( + ( +
+ Cluster with Invalid Kubeconfig Detected! +

Cluster {cluster.name} has invalid kubeconfig {contextName} and cannot be displayed. + Please fix the { e.preventDefault(); shell.showItemInFolder(cluster.kubeConfigPath); }}>kubeconfig manually and restart Lens + or remove the cluster.

+

Do you want to remove the cluster now?

+
+
+
+ ), + { + id: notificationId, + timeout: 0 + } + ); +} + +