Correctly handle promises and rejections around cluster connection (#4216)

src/main/__test__/kubeconfig-manager.test.ts

@@ -142,9 +142,9 @@ describe("kubeconfig manager tests", () => {
     const configPath = await kubeConfManager.getPath();
 
     expect(await fse.pathExists(configPath)).toBe(true);
-    await kubeConfManager.unlink();
+    await kubeConfManager.clear();
     expect(await fse.pathExists(configPath)).toBe(false);
-    await kubeConfManager.unlink(); // doesn't throw
+    await kubeConfManager.clear(); // doesn't throw
     expect(async () => {
       await kubeConfManager.getPath();
     }).rejects.toThrow("already unlinked");

src/main/cluster.ts

@@ -34,7 +34,7 @@ import { DetectorRegistry } from "./cluster-detectors/detector-registry";
 import plimit from "p-limit";
 import type { ClusterState, ClusterRefreshOptions, ClusterMetricsResourceType, ClusterId, ClusterMetadata, ClusterModel, ClusterPreferences, ClusterPrometheusPreferences, UpdateClusterModel, KubeAuthUpdate } from "../common/cluster-types";
 import { ClusterMetadataKey, initialNodeShellImage, ClusterStatus } from "../common/cluster-types";
-import { storedKubeConfigFolder, toJS } from "../common/utils";
+import { disposer, storedKubeConfigFolder, toJS } from "../common/utils";
 import type { Response } from "request";
 
 /**
@@ -52,8 +52,8 @@ export class Cluster implements ClusterModel, ClusterState {
    * @internal
    */
   public contextHandler: ContextHandler;
-  protected kubeconfigManager: KubeconfigManager;
-  protected eventDisposers: Function[] = [];
+  protected proxyKubeconfigManager: KubeconfigManager;
+  protected eventsDisposer = disposer();
   protected activated = false;
   private resourceAccessStatuses: Map<KubeApiResource, boolean> = new Map();
 
@@ -218,9 +218,7 @@ export class Cluster implements ClusterModel, ClusterState {
    * @internal
    */
   @computed get defaultNamespace(): string {
-    const { defaultNamespace } = this.preferences;
-
-    return defaultNamespace;
+    return this.preferences.defaultNamespace;
   }
 
   constructor(model: ClusterModel) {
@@ -240,7 +238,7 @@ export class Cluster implements ClusterModel, ClusterState {
     if (ipcMain) {
       // for the time being, until renderer gets its own cluster type
       this.contextHandler = new ContextHandler(this);
-      this.kubeconfigManager = new KubeconfigManager(this, this.contextHandler);
+      this.proxyKubeconfigManager = new KubeconfigManager(this, this.contextHandler);
 
       logger.debug(`[CLUSTER]: Cluster init success`, {
         id: this.id,
@@ -297,40 +295,31 @@ export class Cluster implements ClusterModel, ClusterState {
     const refreshTimer = setInterval(() => !this.disconnected && this.refresh(), 30000); // every 30s
     const refreshMetadataTimer = setInterval(() => !this.disconnected && this.refreshMetadata(), 900000); // every 15 minutes
 
-    if (ipcMain) {
-      this.eventDisposers.push(
-        reaction(() => this.getState(), () => this.pushState()),
-        reaction(() => this.prometheusPreferences, (prefs) => this.contextHandler.setupPrometheus(prefs), { equals: comparer.structural }),
-        () => {
-          clearInterval(refreshTimer);
-          clearInterval(refreshMetadataTimer);
-        },
-        reaction(() => this.defaultNamespace, () => this.recreateProxyKubeconfig()),
-      );
-    }
+    this.eventsDisposer.push(
+      reaction(() => this.getState(), state => this.pushState(state)),
+      reaction(
+        () => this.prometheusPreferences,
+        prefs => this.contextHandler.setupPrometheus(prefs),
+        { equals: comparer.structural },
+      ),
+      () => clearInterval(refreshTimer),
+      () => clearInterval(refreshMetadataTimer),
+      reaction(() => this.defaultNamespace, () => this.recreateProxyKubeconfig()),
+    );
   }
 
   /**
    * @internal
    */
-  async recreateProxyKubeconfig() {
-    logger.info("Recreate proxy kubeconfig");
+  protected async recreateProxyKubeconfig() {
+    logger.info("[CLUSTER]: Recreating proxy kubeconfig");
 
     try {
-      this.kubeconfigManager.clear();
-    } catch {
-      // do nothing
+      await this.proxyKubeconfigManager.clear();
+      await this.getProxyKubeconfig();
+    } catch (error) {
+      logger.error(`[CLUSTER]: failed to recreate proxy kubeconfig`, error);
     }
-    this.getProxyKubeconfig();
-  }
-
-  /**
-   * internal
-   */
-  protected unbindEvents() {
-    logger.info(`[CLUSTER]: unbind events`, this.getMeta());
-    this.eventDisposers.forEach(dispose => dispose());
-    this.eventDisposers.length = 0;
   }
 
   /**
@@ -345,7 +334,7 @@ export class Cluster implements ClusterModel, ClusterState {
 
     logger.info(`[CLUSTER]: activate`, this.getMeta());
 
-    if (!this.eventDisposers.length) {
+    if (!this.eventsDisposer.length) {
       this.bindEvents();
     }
 
@@ -395,7 +384,8 @@ export class Cluster implements ClusterModel, ClusterState {
    * @internal
    */
   @action disconnect() {
-    this.unbindEvents();
+    logger.info(`[CLUSTER]: disconnecting`, { id: this.id });
+    this.eventsDisposer();
     this.contextHandler?.stopServer();
     this.disconnected = true;
     this.online = false;
@@ -405,7 +395,7 @@ export class Cluster implements ClusterModel, ClusterState {
     this.allowedNamespaces = [];
     this.resourceAccessStatuses.clear();
     this.pushState();
-    logger.info(`[CLUSTER]: disconnect`, this.getMeta());
+    logger.info(`[CLUSTER]: disconnected`, { id: this.id });
   }
 
   /**
@@ -481,7 +471,7 @@ export class Cluster implements ClusterModel, ClusterState {
    * @internal
    */
   async getProxyKubeconfigPath(): Promise<string> {
-    return this.kubeconfigManager.getPath();
+    return this.proxyKubeconfigManager.getPath();
   }
 
   protected async getConnectionStatus(): Promise<ClusterStatus> {

src/main/kube-auth-proxy.ts

@@ -32,30 +32,21 @@ import { makeObservable, observable, when } from "mobx";
 const startingServeRegex = /^starting to serve on (?<address>.+)/i;
 
 export class KubeAuthProxy {
-  public readonly apiPrefix: string;
+  public readonly apiPrefix = `/${randomBytes(8).toString("hex")}`;
 
   public get port(): number {
     return this._port;
   }
 
-  protected _port?: number;
-  protected cluster: Cluster;
-  protected env: NodeJS.ProcessEnv = null;
-  protected proxyProcess: ChildProcess;
-  protected kubectl: Kubectl;
-  @observable protected ready: boolean;
+  protected _port: number;
+  protected proxyProcess?: ChildProcess;
+  protected readonly acceptHosts: string;
+  @observable protected ready = false;
 
-  constructor(cluster: Cluster, env: NodeJS.ProcessEnv) {
+  constructor(protected readonly cluster: Cluster, protected readonly env: NodeJS.ProcessEnv) {
     makeObservable(this);
-    this.ready = false;
-    this.env = env;
-    this.cluster = cluster;
-    this.kubectl = Kubectl.bundled();
-    this.apiPrefix = `/${randomBytes(8).toString("hex")}`;
-  }
 
-  get acceptHosts() {
-    return url.parse(this.cluster.apiUrl).hostname;
+    this.acceptHosts = url.parse(this.cluster.apiUrl).hostname;
   }
 
   get whenReady() {
@@ -67,7 +58,7 @@ export class KubeAuthProxy {
       return this.whenReady;
     }
 
-    const proxyBin = await this.kubectl.getPath();
+    const proxyBin = await Kubectl.bundled().getPath();
     const args = [
       "proxy",
       "-p", "0",

src/main/kubeconfig-manager.ts

@@ -30,57 +30,60 @@ import { LensProxy } from "./lens-proxy";
 import { AppPaths } from "../common/app-paths";
 
 export class KubeconfigManager {
-  protected configDir = AppPaths.get("temp");
-  protected tempFile: string = null;
+  /**
+   * The path to the temp config file
+   *
+   * - if `string` then path
+   * - if `null` then not yet created
+   * - if `undefined` then unlinked by calling `clear()`
+   */
+  protected tempFilePath: string | null | undefined = null;
 
   constructor(protected cluster: Cluster, protected contextHandler: ContextHandler) { }
 
+  /**
+   *
+   * @returns The path to the temporary kubeconfig
+   */
   async getPath(): Promise<string> {
-    if (this.tempFile === undefined) {
+    if (this.tempFilePath === undefined) {
       throw new Error("kubeconfig is already unlinked");
     }
 
-    if (!this.tempFile) {
-      await this.init();
+    if (this.tempFilePath === null || !(await fs.pathExists(this.tempFilePath))) {
+      await this.ensureFile();
     }
 
-    // create proxy kubeconfig if it is removed without unlink called
-    if (!(await fs.pathExists(this.tempFile))) {
-      try {
-        this.tempFile = await this.createProxyKubeconfig();
-      } catch (err) {
-        logger.error(`[KUBECONFIG-MANAGER]: Failed to created temp config for auth-proxy`, { err });
+    return this.tempFilePath;
+  }
+
+  /**
+   * Deletes the temporary kubeconfig file
+   */
+  async clear(): Promise<void> {
+    if (!this.tempFilePath) {
+      return;
+    }
+
+    logger.info(`[KUBECONFIG-MANAGER]: Deleting temporary kubeconfig: ${this.tempFilePath}`);
+
+    try {
+      await fs.unlink(this.tempFilePath);
+    } catch (error) {
+      if (error.code !== "ENOENT") {
+        throw error;
       }
+    } finally {
+      this.tempFilePath = undefined;
     }
-
-    return this.tempFile;
   }
 
-  async clear() {
-    if (!this.tempFile) {
-      return;
-    }
-
-    logger.info(`[KUBECONFIG-MANAGER]: Deleting temporary kubeconfig: ${this.tempFile}`);
-    await fs.unlink(this.tempFile);
-  }
-
-  async unlink() {
-    if (!this.tempFile) {
-      return;
-    }
-
-    logger.info(`[KUBECONFIG-MANAGER]: Deleting temporary kubeconfig: ${this.tempFile}`);
-    await fs.unlink(this.tempFile);
-    this.tempFile = undefined;
-  }
-
-  protected async init() {
+  protected async ensureFile() {
     try {
       await this.contextHandler.ensureServer();
-      this.tempFile = await this.createProxyKubeconfig();
-    } catch (err) {
-      logger.error(`[KUBECONFIG-MANAGER]: Failed to created temp config for auth-proxy`, err);
+      this.tempFilePath = await this.createProxyKubeconfig();
+    } catch (error) {
+      throw Object.assign(new Error("Failed to creat temp config for auth-proxy"), { cause: error });
     }
   }
 
@@ -93,9 +96,9 @@ export class KubeconfigManager {
    * This way any user of the config does not need to know anything about the auth etc. details.
    */
   protected async createProxyKubeconfig(): Promise<string> {
-    const { configDir, cluster } = this;
+    const { cluster } = this;
     const { contextName, id } = cluster;
-    const tempFile = path.join(configDir, `kubeconfig-${id}`);
+    const tempFile = path.join(AppPaths.get("temp"), `kubeconfig-${id}`);
     const kubeConfig = await cluster.getKubeconfig();
     const proxyConfig: Partial<KubeConfig> = {
       currentContext: contextName,