Move Cluster.apiUrl to seperate injectable (#7354)

* Refactor out Cluster.apiUrl to direct reading

- Should help prevent using stale data

- Removes some uses of synchronous FS operations

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

* Fix type errors

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

* Rename helper function to better communicate intent

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

* Improve prometheus handler tests to override less things

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

---------

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

packages/core/src/features/cluster/connections/main/api-url.injectable.ts

@@ -0,0 +1,48 @@
+/**
+ * Copyright (c) OpenLens Authors. All rights reserved.
+ * Licensed under MIT License. See LICENSE in root directory for more information.
+ */
+import { getInjectable, lifecycleEnum } from "@ogre-tools/injectable";
+import { URL } from "url";
+import type { Cluster } from "../../../../common/cluster/cluster";
+import statInjectable from "../../../../common/fs/stat.injectable";
+import loadValidatedClusterConfigInjectable from "../../../../common/kube-helpers/load-validated-config-from-file.injectable";
+
+interface ClusterApiUrlState {
+  url: URL;
+  lastReadMtimeMs: number;
+}
+
+const clusterApiUrlInjectable = getInjectable({
+  id: "cluster-api-url",
+  instantiate: (di, cluster): () => Promise<URL> => {
+    const loadValidatedClusterConfig = di.inject(loadValidatedClusterConfigInjectable);
+    const stat = di.inject(statInjectable);
+
+    let state: ClusterApiUrlState | undefined;
+
+    return async () => {
+      const stats = await stat(cluster.kubeConfigPath.get());
+
+      if (!state || state.lastReadMtimeMs >= stats.mtimeMs) {
+        const result = await loadValidatedClusterConfig(cluster);
+
+        if (result.error) {
+          throw result.error;
+        }
+
+        state = {
+          url: new URL(result.cluster.server),
+          lastReadMtimeMs: stats.mtimeMs,
+        };
+      }
+
+      return state.url;
+    };
+  },
+  lifecycle: lifecycleEnum.keyedSingleton({
+    getInstanceKey: (di, cluster: Cluster) => cluster.id,
+  }),
+});
+
+export default clusterApiUrlInjectable;

packages/core/src/features/cluster/delete-dialog/delete-cluster-dialog.test.tsx

@@ -109,8 +109,6 @@ describe("Deleting a cluster", () => {
           clusterName: "some-current-context-cluster",
         },
         kubeConfigPath: "./temp-kube-config",
-      }, {
-        clusterServerUrl: currentClusterServerUrl,
       });
       nonCurrentCluster = new Cluster({
         id: "some-non-current-context-cluster",
@@ -119,8 +117,6 @@ describe("Deleting a cluster", () => {
           clusterName: "some-non-current-context-cluster",
         },
         kubeConfigPath: "./temp-kube-config",
-      }, {
-        clusterServerUrl: currentClusterServerUrl,
       });
     });
 
@@ -197,8 +193,6 @@ describe("Deleting a cluster", () => {
           clusterName: "some-cluster",
         },
         kubeConfigPath: joinPaths(directoryForKubeConfigs, "some-cluster.json"),
-      }, {
-        clusterServerUrl: singleClusterServerUrl,
       });
     });
 
@@ -233,8 +227,6 @@ describe("Deleting a cluster", () => {
           clusterName: "some-cluster",
         },
         kubeConfigPath: "./temp-kube-config",
-      }, {
-        clusterServerUrl: singleClusterServerUrl,
       });
     });
 

packages/core/src/features/cluster/storage/cluster-storage.test.ts

@@ -23,7 +23,6 @@ import type { WriteFileSync } from "../../../common/fs/write-file-sync.injectabl
 import writeFileSyncInjectable from "../../../common/fs/write-file-sync.injectable";
 import type { WriteBufferSync } from "../../../common/fs/write-buffer-sync.injectable";
 import writeBufferSyncInjectable from "../../../common/fs/write-buffer-sync.injectable";
-import { Cluster } from "../../../common/cluster/cluster";
 import clustersPersistentStorageInjectable from "./common/storage.injectable";
 import type { PersistentStorage } from "../../../common/persistent-storage/create.injectable";
 import type { AddCluster } from "./common/add.injectable";
@@ -32,6 +31,7 @@ import type { GetClusterById } from "./common/get-by-id.injectable";
 import getClusterByIdInjectable from "./common/get-by-id.injectable";
 import type { IComputedValue } from "mobx";
 import clustersInjectable from "./common/clusters.injectable";
+import type { Cluster } from "../../../common/cluster/cluster";
 
 // NOTE: this is intended to read the actual file system
 const testDataIcon = readFileSync("test-data/cluster-store-migration-icon.png");
@@ -102,7 +102,7 @@ describe("cluster storage technical tests", () => {
 
     describe("with foo cluster added", () => {
       beforeEach(() => {
-        const cluster = new Cluster({
+        addCluster({
           id: "foo",
           contextName: "foo",
           preferences: {
@@ -114,11 +114,7 @@ describe("cluster storage technical tests", () => {
             getCustomKubeConfigFilePath("foo"),
             kubeconfig,
           ),
-        }, {
-          clusterServerUrl,
         });
-
-        addCluster(cluster);
       });
 
       it("adds new cluster to store", async () => {
@@ -232,47 +228,6 @@ describe("cluster storage technical tests", () => {
     });
   });
 
-  describe("config with invalid cluster kubeconfig", () => {
-    beforeEach(() => {
-      writeFileSync("/invalid-kube-config", invalidKubeconfig);
-      writeFileSync("/valid-kube-config", kubeconfig);
-      writeJsonSync("/some-directory-for-user-data/lens-cluster-store.json", {
-        __internal__: {
-          migrations: {
-            version: "99.99.99",
-          },
-        },
-        clusters: [
-          {
-            id: "cluster1",
-            kubeConfigPath: "/invalid-kube-config",
-            contextName: "test",
-            preferences: { terminalCWD: "/foo" },
-            workspace: "foo",
-          },
-          {
-            id: "cluster2",
-            kubeConfigPath: "/valid-kube-config",
-            contextName: "foo",
-            preferences: { terminalCWD: "/foo" },
-            workspace: "default",
-          },
-        ],
-      });
-
-      getCustomKubeConfigFilePath = di.inject(getCustomKubeConfigFilePathInjectable);
-
-      clustersPersistentStorage = di.inject(clustersPersistentStorageInjectable);
-      clustersPersistentStorage.loadAndStartSyncing();
-    });
-
-    it("does not enable clusters with invalid kubeconfig", () => {
-      const storedClusters = clusters.get();
-
-      expect(storedClusters.length).toBe(1);
-    });
-  });
-
   describe("pre 3.6.0-beta.1 config with an existing cluster", () => {
     beforeEach(() => {
       di.override(storeMigrationVersionInjectable, () => "3.6.0");
@@ -315,32 +270,6 @@ describe("cluster storage technical tests", () => {
   });
 });
 
-const invalidKubeconfig = JSON.stringify({
-  apiVersion: "v1",
-  clusters: [{
-    cluster: {
-      server: "https://localhost",
-    },
-    name: "test2",
-  }],
-  contexts: [{
-    context: {
-      cluster: "test",
-      user: "test",
-    },
-    name: "test",
-  }],
-  "current-context": "test",
-  kind: "Config",
-  preferences: {},
-  users: [{
-    user: {
-      token: "kubeconfig-user-q4lm4:xxxyyyy",
-    },
-    name: "test",
-  }],
-});
-
 const minimalValidKubeConfig = JSON.stringify({
   apiVersion: "v1",
   clusters: [

packages/core/src/features/cluster/storage/common/add.injectable.ts

@@ -5,33 +5,23 @@
 import { getInjectable } from "@ogre-tools/injectable";
 import { action } from "mobx";
 import emitAppEventInjectable from "../../../../common/app-event-bus/emit-event.injectable";
-import readClusterConfigSyncInjectable from "./read-cluster-config.injectable";
 import type { ClusterModel } from "../../../../common/cluster-types";
 import { Cluster } from "../../../../common/cluster/cluster";
 import clustersStateInjectable from "./state.injectable";
+import { setAndGet } from "@k8slens/utilities";
 
-export type AddCluster = (clusterOrModel: ClusterModel | Cluster) => Cluster;
+export type AddCluster = (clusterModel: ClusterModel) => Cluster;
 
 const addClusterInjectable = getInjectable({
   id: "add-cluster",
   instantiate: (di): AddCluster => {
     const clustersState = di.inject(clustersStateInjectable);
     const emitAppEvent = di.inject(emitAppEventInjectable);
-    const readClusterConfigSync = di.inject(readClusterConfigSyncInjectable);
 
-    return action((clusterOrModel) => {
+    return action((clusterModel) => {
       emitAppEvent({ name: "cluster", action: "add" });
 
-      const cluster = clusterOrModel instanceof Cluster
-        ? clusterOrModel
-        : new Cluster(
-          clusterOrModel,
-          readClusterConfigSync(clusterOrModel),
-        );
-
-      clustersState.set(cluster.id, cluster);
-
-      return cluster;
+      return setAndGet(clustersState, clusterModel.id, new Cluster(clusterModel));
     });
   },
 });

packages/core/src/features/cluster/storage/common/read-cluster-config.injectable.ts

@@ -1,31 +0,0 @@
-/**
- * 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 { ClusterConfigData, ClusterModel } from "../../../../common/cluster-types";
-import readFileSyncInjectable from "../../../../common/fs/read-file-sync.injectable";
-import { loadConfigFromString, validateKubeConfig } from "../../../../common/kube-helpers";
-
-export type ReadClusterConfigSync = (model: ClusterModel) => ClusterConfigData;
-
-const readClusterConfigSyncInjectable = getInjectable({
-  id: "read-cluster-config-sync",
-  instantiate: (di): ReadClusterConfigSync => {
-    const readFileSync = di.inject(readFileSyncInjectable);
-
-    return ({ kubeConfigPath, contextName }) => {
-      const kubeConfigData = readFileSync(kubeConfigPath);
-      const { config } = loadConfigFromString(kubeConfigData);
-      const result = validateKubeConfig(config, contextName);
-
-      if (result.error) {
-        throw result.error;
-      }
-
-      return { clusterServerUrl: result.cluster.server };
-    };
-  },
-});
-
-export default readClusterConfigSyncInjectable;

packages/core/src/features/cluster/storage/common/storage.injectable.ts

@@ -6,7 +6,6 @@ import { iter } from "@k8slens/utilities";
 import { getInjectable } from "@ogre-tools/injectable";
 import { comparer, action } from "mobx";
 import { clusterStoreMigrationInjectionToken } from "./migration-token";
-import readClusterConfigSyncInjectable from "./read-cluster-config.injectable";
 import type { ClusterId, ClusterModel } from "../../../../common/cluster-types";
 import { Cluster } from "../../../../common/cluster/cluster";
 import loggerInjectable from "../../../../common/logger.injectable";
@@ -23,7 +22,6 @@ const clustersPersistentStorageInjectable = getInjectable({
   id: "clusters-persistent-storage",
   instantiate: (di) => {
     const createPersistentStorage = di.inject(createPersistentStorageInjectable);
-    const readClusterConfigSync = di.inject(readClusterConfigSyncInjectable);
     const clustersState = di.inject(clustersStateInjectable);
     const logger = di.inject(loggerInjectable);
 
@@ -39,7 +37,6 @@ const clustersPersistentStorageInjectable = getInjectable({
         const currentClusters = new Map(clustersState);
         const newClusters = new Map<ClusterId, Cluster>();
 
-        // update new clusters
         for (const clusterModel of clusters) {
           try {
             let cluster = currentClusters.get(clusterModel.id);
@@ -47,11 +44,9 @@ const clustersPersistentStorageInjectable = getInjectable({
             if (cluster) {
               cluster.updateModel(clusterModel);
             } else {
-              cluster = new Cluster(
-                clusterModel,
-                readClusterConfigSync(clusterModel),
-              );
+              cluster = new Cluster(clusterModel);
             }
+
             newClusters.set(clusterModel.id, cluster);
           } catch (error) {
             logger.warn(`[CLUSTER-STORE]: Failed to update/create a cluster: ${error}`);