Simplify add cluster view (#2716)

- Only shows editor, and will save all text to file

- Reports some validation errors, but doesn't block the add cluster
  button

- Cleanup kube helpers to facilitate clearer FS interactions

Signed-off-by: Sebastian Malton <sebastian@malton.name>

src/common/__tests__/cluster-store.test.ts

@@ -22,8 +22,10 @@
 import fs from "fs";
 import mockFs from "mock-fs";
 import yaml from "js-yaml";
+import path from "path";
+import fse from "fs-extra";
 import { Cluster } from "../../main/cluster";
-import { ClusterStore, getClusterIdFromHost } from "../cluster-store";
+import { ClusterId, ClusterStore, getClusterIdFromHost } from "../cluster-store";
 import { Console } from "console";
 import { stdout, stderr } from "process";
 
@@ -54,6 +56,15 @@ users:
     token: kubeconfig-user-q4lm4:xxxyyyy
 `;
 
+function embed(clusterId: ClusterId, contents: any): string {
+  const absPath = ClusterStore.getCustomKubeConfigPath(clusterId);
+
+  fse.ensureDirSync(path.dirname(absPath));
+  fse.writeFileSync(absPath, contents, { encoding: "utf-8", mode: 0o600 });
+
+  return absPath;
+}
+
 jest.mock("electron", () => {
   return {
     app: {
@@ -102,7 +113,7 @@ describe("empty config", () => {
             icon: "data:image/jpeg;base64, iVBORw0KGgoAAAANSUhEUgAAA1wAAAKoCAYAAABjkf5",
             clusterName: "minikube"
           },
-          kubeConfigPath: ClusterStore.embedCustomKubeConfig("foo", kubeconfig)
+          kubeConfigPath: embed("foo", kubeconfig)
         })
       );
     });
@@ -130,7 +141,7 @@ describe("empty config", () => {
           preferences: {
             clusterName: "prod"
           },
-          kubeConfigPath: ClusterStore.embedCustomKubeConfig("prod", kubeconfig)
+          kubeConfigPath: embed("prod", kubeconfig)
         }),
         new Cluster({
           id: "dev",
@@ -138,7 +149,7 @@ describe("empty config", () => {
           preferences: {
             clusterName: "dev"
           },
-          kubeConfigPath: ClusterStore.embedCustomKubeConfig("dev", kubeconfig)
+          kubeConfigPath: embed("dev", kubeconfig)
         })
       );
     });
@@ -149,7 +160,7 @@ describe("empty config", () => {
     });
 
     it("check if cluster's kubeconfig file saved", () => {
-      const file = ClusterStore.embedCustomKubeConfig("boo", "kubeconfig");
+      const file = embed("boo", "kubeconfig");
 
       expect(fs.readFileSync(file, "utf8")).toBe("kubeconfig");
     });
@@ -160,6 +171,7 @@ describe("config with existing clusters", () => {
   beforeEach(() => {
     ClusterStore.resetInstance();
     const mockOpts = {
+      "temp-kube-config": kubeconfig,
       "tmp": {
         "lens-cluster-store.json": JSON.stringify({
           __internal__: {
@@ -170,20 +182,20 @@ describe("config with existing clusters", () => {
           clusters: [
             {
               id: "cluster1",
-              kubeConfigPath: kubeconfig,
+              kubeConfigPath: "./temp-kube-config",
               contextName: "foo",
               preferences: { terminalCWD: "/foo" },
               workspace: "default"
             },
             {
               id: "cluster2",
-              kubeConfigPath: kubeconfig,
+              kubeConfigPath: "./temp-kube-config",
               contextName: "foo2",
               preferences: { terminalCWD: "/foo2" }
             },
             {
               id: "cluster3",
-              kubeConfigPath: kubeconfig,
+              kubeConfigPath: "./temp-kube-config",
               contextName: "foo",
               preferences: { terminalCWD: "/foo" },
               workspace: "foo",
@@ -256,6 +268,8 @@ users:
 
     ClusterStore.resetInstance();
     const mockOpts = {
+      "invalid-kube-config": invalidKubeconfig,
+      "valid-kube-config": kubeconfig,
       "tmp": {
         "lens-cluster-store.json": JSON.stringify({
           __internal__: {
@@ -266,14 +280,14 @@ users:
           clusters: [
             {
               id: "cluster1",
-              kubeConfigPath: invalidKubeconfig,
+              kubeConfigPath: "./invalid-kube-config",
               contextName: "test",
               preferences: { terminalCWD: "/foo" },
               workspace: "foo",
             },
             {
               id: "cluster2",
-              kubeConfigPath: kubeconfig,
+              kubeConfigPath: "./valid-kube-config",
               contextName: "foo",
               preferences: { terminalCWD: "/foo" },
               workspace: "default"

src/common/__tests__/kube-helpers.test.ts

@@ -20,7 +20,7 @@
  */
 
 import { KubeConfig } from "@kubernetes/client-node";
-import { validateKubeConfig, loadConfig, getNodeWarningConditions } from "../kube-helpers";
+import { validateKubeConfig, loadConfigFromString, getNodeWarningConditions } from "../kube-helpers";
 
 const kubeconfig = `
 apiVersion: v1
@@ -59,8 +59,6 @@ users:
       command: foo
 `;
 
-const kc = new KubeConfig();
-
 interface kubeconfig {
   apiVersion: string,
   clusters: [{
@@ -88,6 +86,8 @@ let mockKubeConfig: kubeconfig;
 
 describe("kube helpers", () => {
   describe("validateKubeconfig", () => {
+    const kc = new KubeConfig();
+
     beforeAll(() => {
       kc.loadFromString(kubeconfig);
     });
@@ -164,12 +164,12 @@ describe("kube helpers", () => {
       it("invalid yaml string", () => {
         const invalidYAMLString = "fancy foo config";
 
-        expect(() => loadConfig(invalidYAMLString)).toThrowError("must be an object");
+        expect(loadConfigFromString(invalidYAMLString).error).toBeInstanceOf(Error);
       });
       it("empty contexts", () => {
-        const emptyContexts = `apiVersion: v1\ncontexts:`;
+        const emptyContexts = `apiVersion: v1\ncontexts: []`;
 
-        expect(() => loadConfig(emptyContexts)).not.toThrow();
+        expect(loadConfigFromString(emptyContexts).error).toBeUndefined();
       });
     });
 
@@ -200,17 +200,17 @@ describe("kube helpers", () => {
       });
 
       it("single context is ok", async () => {
-        const kc:KubeConfig = loadConfig(JSON.stringify(mockKubeConfig));
+        const { config } = loadConfigFromString(JSON.stringify(mockKubeConfig));
 
-        expect(kc.getCurrentContext()).toBe("minikube");
+        expect(config.getCurrentContext()).toBe("minikube");
       });
 
       it("multiple context is ok", async () => {
         mockKubeConfig.contexts.push({context: {cluster: "cluster-2", user: "cluster-2"}, name: "cluster-2"});
-        const kc:KubeConfig = loadConfig(JSON.stringify(mockKubeConfig));
+        const { config } = loadConfigFromString(JSON.stringify(mockKubeConfig));
 
-        expect(kc.getCurrentContext()).toBe("minikube");
-        expect(kc.contexts.length).toBe(2);
+        expect(config.getCurrentContext()).toBe("minikube");
+        expect(config.contexts.length).toBe(2);
       });
     });
 
@@ -243,40 +243,40 @@ describe("kube helpers", () => {
       it("empty name in context causes it to be removed", async () => {
         mockKubeConfig.contexts.push({context: {cluster: "cluster-2", user: "cluster-2"}, name: ""});
         expect(mockKubeConfig.contexts.length).toBe(2);
-        const kc:KubeConfig = loadConfig(JSON.stringify(mockKubeConfig));
+        const { config } = loadConfigFromString(JSON.stringify(mockKubeConfig));
 
-        expect(kc.getCurrentContext()).toBe("minikube");
-        expect(kc.contexts.length).toBe(1);
+        expect(config.getCurrentContext()).toBe("minikube");
+        expect(config.contexts.length).toBe(1);
       });
 
       it("empty cluster in context causes it to be removed", async () => {
         mockKubeConfig.contexts.push({context: {cluster: "", user: "cluster-2"}, name: "cluster-2"});
         expect(mockKubeConfig.contexts.length).toBe(2);
-        const kc:KubeConfig = loadConfig(JSON.stringify(mockKubeConfig));
+        const { config } = loadConfigFromString(JSON.stringify(mockKubeConfig));
 
-        expect(kc.getCurrentContext()).toBe("minikube");
-        expect(kc.contexts.length).toBe(1);
+        expect(config.getCurrentContext()).toBe("minikube");
+        expect(config.contexts.length).toBe(1);
       });
 
       it("empty user in context causes it to be removed", async () => {
         mockKubeConfig.contexts.push({context: {cluster: "cluster-2", user: ""}, name: "cluster-2"});
         expect(mockKubeConfig.contexts.length).toBe(2);
-        const kc:KubeConfig = loadConfig(JSON.stringify(mockKubeConfig));
+        const { config } = loadConfigFromString(JSON.stringify(mockKubeConfig));
 
-        expect(kc.getCurrentContext()).toBe("minikube");
-        expect(kc.contexts.length).toBe(1);
+        expect(config.getCurrentContext()).toBe("minikube");
+        expect(config.contexts.length).toBe(1);
       });
 
       it("invalid context in between valid contexts is removed", async () => {
         mockKubeConfig.contexts.push({context: {cluster: "cluster-2", user: ""}, name: "cluster-2"});
         mockKubeConfig.contexts.push({context: {cluster: "cluster-3", user: "cluster-3"}, name: "cluster-3"});
         expect(mockKubeConfig.contexts.length).toBe(3);
-        const kc:KubeConfig = loadConfig(JSON.stringify(mockKubeConfig));
+        const { config } = loadConfigFromString(JSON.stringify(mockKubeConfig));
 
-        expect(kc.getCurrentContext()).toBe("minikube");
-        expect(kc.contexts.length).toBe(2);
-        expect(kc.contexts[0].name).toBe("minikube");
-        expect(kc.contexts[1].name).toBe("cluster-3");
+        expect(config.getCurrentContext()).toBe("minikube");
+        expect(config.contexts.length).toBe(2);
+        expect(config.contexts[0].name).toBe("minikube");
+        expect(config.contexts[1].name).toBe("cluster-3");
       });
     });
   });

src/common/__tests__/user-store.test.ts

@@ -66,19 +66,6 @@ describe("user store tests", () => {
       expect(us.lastSeenAppVersion).toBe("1.2.3");
     });
 
-    it("allows adding and listing seen contexts", () => {
-      const us = UserStore.getInstance();
-
-      us.seenContexts.add("foo");
-      expect(us.seenContexts.size).toBe(1);
-
-      us.seenContexts.add("foo");
-      us.seenContexts.add("bar");
-      expect(us.seenContexts.size).toBe(2); // check 'foo' isn't added twice
-      expect(us.seenContexts.has("foo")).toBe(true);
-      expect(us.seenContexts.has("bar")).toBe(true);
-    });
-
     it("allows setting and getting preferences", () => {
       const us = UserStore.getInstance();