Harden against navigating to the cluster frame route

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

src/common/cluster-frames.ts

@@ -19,6 +19,7 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
+import AwaitLock from "await-lock";
 import { action, observable } from "mobx";
 import type { ClusterId } from "./cluster-store";
 import { iter, Singleton } from "./utils";
@@ -30,14 +31,26 @@ export interface ClusterFrameInfo {
 }
 
 export class ClusterFrames extends Singleton {
-  private mapping = observable.map<ClusterId, ClusterFrameInfo>();
+  /**
+   * The current set of frame info for each cluster
+   */
+  private frames = observable.map<ClusterId, ClusterFrameInfo>();
+
+  /**
+   * The current mapping of clusters to the window that hope to create an iframe
+   *
+   * Used to make sure that if two windows try and open the same cluster, the one
+   * locks and submits a claim first is the only one.
+   */
+  private claims = observable.map<ClusterId, number>();
+  private claimsLock = new AwaitLock();
 
   public getAllFrameInfo(): ClusterFrameInfo[] {
-    return [...this.mapping.values()];
+    return [...this.frames.values()];
   }
 
   public getClusterIdFromFrameInfo(query: ClusterFrameInfo): ClusterId | undefined {
-    for (const [clusterId, info] of this.mapping) {
+    for (const [clusterId, info] of this.frames) {
       if (
         info.frameId === query.frameId
         && info.processId === query.processId
@@ -50,27 +63,74 @@ export class ClusterFrames extends Singleton {
     return undefined;
   }
 
+  @action
   public set(clusterId: ClusterId, info: ClusterFrameInfo): void {
-    this.mapping.set(clusterId, info);
+    if (!this.claims.has(clusterId)) {
+      throw new Error("Cannot set a cluster's FrameInfo if no claim exists");
+    }
+
+    if (this.claims.get(clusterId) !== info.windowId) {
+      throw new Error("Cannot set a cluster's FrameInfo for a window that didn't previously claim the cluster");
+    }
+
+    this.frames.set(clusterId, info);
+    this.claims.delete(clusterId);
+  }
+
+  /**
+   * Attempts to claim cluster for window. Will succeed if previously claimed by the same window
+   * @param clusterId The cluster to claim for a particular window
+   * @param windowId The ID of the window trying to claim it
+   * @returns `true` if that window now has a claim, otherwise `false`
+   */
+  public async claimCluster(clusterId: ClusterId, windowId: number): Promise<boolean> {
+    try {
+      await this.claimsLock.acquireAsync();
+
+      if (this.claims.get(clusterId) === windowId) {
+        return true;
+      }
+
+      if (this.frames.get(clusterId)?.windowId === windowId) {
+        return true;
+      }
+
+      if (this.claims.has(clusterId) || this.frames.has(clusterId)) {
+        return false;
+      }
+
+      this.claims.set(clusterId, windowId);
+
+      return true;
+    } finally {
+      this.claimsLock.release();
+    }
   }
 
   public getFrameInfoByClusterId(clusterId: ClusterId): ClusterFrameInfo | undefined {
-    return this.mapping.get(clusterId);
+    return this.frames.get(clusterId);
   }
 
   public getFrameInfoByFrameId(frameId: number): ClusterFrameInfo | undefined {
-    return iter.find(this.mapping.values(), frameInfo => frameInfo.frameId === frameId);
+    return iter.find(this.frames.values(), frameInfo => frameInfo.frameId === frameId);
   }
 
   public clearInfoForCluster(clusterId: ClusterId): void {
-    this.mapping.delete(clusterId);
+    this.frames.delete(clusterId);
+    this.claims.delete(clusterId);
   }
 
   @action
   public clearInfoForWindow(windowId: number): void {
-    for (const [clusterId, frameInfo] of this.mapping) {
+    for (const [clusterId, frameInfo] of this.frames) {
       if (frameInfo.windowId === windowId) {
-        this.mapping.delete(clusterId);
+        this.frames.delete(clusterId);
+      }
+    }
+
+    for (const [clusterId, windowIdClaim] of this.claims) {
+      if (windowIdClaim === windowId) {
+        this.claims.delete(clusterId);
       }
     }
   }

src/common/cluster-ipc.ts

@@ -22,6 +22,7 @@
 export const navigateToClusterHandler = "navigate:to-cluster";
 export const clusterActivateHandler = "cluster:activate";
 export const clusterSetFrameIdHandler = "cluster:set-frame-id";
+export const claimClusterFrameHandler = "cluster:claim-frame";
 export const clusterVisibilityHandler = "cluster:visibility";
 export const clusterRefreshHandler = "cluster:refresh";
 export const clusterDisconnectHandler = "cluster:disconnect";

src/main/initializers/ipc.ts

@@ -24,7 +24,7 @@ import type { IpcMainInvokeEvent } from "electron";
 import { when } from "mobx";
 import { KubernetesCluster } from "../../common/catalog-entities";
 import { ClusterFrames } from "../../common/cluster-frames";
-import { clusterActivateHandler, clusterSetFrameIdHandler, clusterVisibilityHandler, clusterRefreshHandler, clusterDisconnectHandler, clusterKubectlApplyAllHandler, clusterKubectlDeleteAllHandler, clusterDeleteHandler, navigateToClusterHandler } from "../../common/cluster-ipc";
+import { clusterActivateHandler, clusterSetFrameIdHandler, clusterVisibilityHandler, clusterRefreshHandler, clusterDisconnectHandler, clusterKubectlApplyAllHandler, clusterKubectlDeleteAllHandler, clusterDeleteHandler, navigateToClusterHandler, claimClusterFrameHandler } from "../../common/cluster-ipc";
 import { ClusterId, ClusterStore } from "../../common/cluster-store";
 import { appEventBus } from "../../common/event-bus";
 import { ipcMainHandle } from "../../common/ipc";
@@ -146,6 +146,10 @@ export function initIpcMainHandlers() {
     }
   });
 
+  ipcMainHandle(claimClusterFrameHandler, async (event, clusterId: ClusterId) => {
+    return ClusterFrames.getInstance().claimCluster(clusterId, event.sender.getProcessId());
+  });
+
   const navigateLocks = new Map<ClusterId, AwaitLock>();
 
   ipcMainHandle(navigateToClusterHandler, async (event, clusterId: ClusterId, newWindow?: boolean) => {
@@ -159,7 +163,9 @@ export function initIpcMainHandlers() {
     const lock = getOrInsert(navigateLocks, clusterId, new AwaitLock());
 
     try {
+      console.log("trying to acquire lock for", clusterId, "in", navigateToClusterHandler);
       await lock.acquireAsync();
+      console.log("acquired lock for", clusterId, "in", navigateToClusterHandler);
       const specifics: NavigateFrameInfoSpecifier[] = [{ clusterId }];
 
       if (newWindow) {

src/renderer/components/cluster-manager/cluster-view.tsx

@@ -29,7 +29,7 @@ import { hasLoadedView, initView, refreshViews } from "./lens-views";
 import type { Cluster } from "../../../main/cluster";
 import { ClusterStore } from "../../../common/cluster-store";
 import { requestMain } from "../../../common/ipc";
-import { clusterActivateHandler } from "../../../common/cluster-ipc";
+import { clusterActivateHandler, navigateToClusterHandler } from "../../../common/cluster-ipc";
 import { catalogEntityRegistry } from "../../api/catalog-entity-registry";
 import { navigate } from "../../navigation";
 import { catalogURL, ClusterViewRouteParams } from "../../../common/routes";
@@ -65,10 +65,27 @@ export class ClusterView extends React.Component<Props> {
   componentDidMount() {
     disposeOnUnmount(this, [
       reaction(() => this.clusterId, async (clusterId) => {
-        refreshViews(clusterId); // refresh visibility of active cluster
+        // refresh visibility of active cluster
-        initView(clusterId); // init cluster-view (iframe), requires parent container #lens-views to be in DOM
+        refreshViews(clusterId);
-        requestMain(clusterActivateHandler, clusterId, false); // activate and fetch cluster's state from main
+
-        catalogEntityRegistry.activeEntity = catalogEntityRegistry.getById(clusterId);
+        // activate and fetch cluster's state from main, don't force. Do this before starting to init
+        requestMain(clusterActivateHandler, clusterId, false)
+          .catch(error => console.warn("[CLUSTER-VIEW]: failed to activate cluster", error));
+
+        console.log("start initView", { clusterId });
+
+        if (await initView(clusterId)) {
+          console.log("success initView", { clusterId });
+          // init cluster-view (iframe), requires parent container #lens-views to be in DOM
+          catalogEntityRegistry.activeEntity = catalogEntityRegistry.getById(clusterId);
+        } else {
+          console.log("initView: need to navigate", { clusterId });
+          // if it fails then navigate to the new window
+          await requestMain(
+            navigateToClusterHandler,
+            clusterId,
+          );
+        }
       }, {
         fireImmediately: true,
       }),

src/renderer/components/cluster-manager/lens-views.ts

@@ -23,7 +23,7 @@ import { observable, observe, when } from "mobx";
 import { ClusterId, ClusterStore, getClusterFrameUrl } from "../../../common/cluster-store";
 import logger from "../../../main/logger";
 import { requestMain } from "../../../common/ipc";
-import { clusterVisibilityHandler } from "../../../common/cluster-ipc";
+import { claimClusterFrameHandler, clusterVisibilityHandler } from "../../../common/cluster-ipc";
 import { toJS } from "../../utils";
 
 export interface LensView {
@@ -32,8 +32,15 @@ export interface LensView {
   view: HTMLIFrameElement
 }
 
-export const lensViews = observable.map<ClusterId, LensView>();
+/**
-export const visibleCluster = observable.box<ClusterId | undefined>();
+ * These shouldn't be exported so that this file can ensure consistency
+ */
+const lensViews = observable.map<ClusterId, LensView>();
+const visibleCluster = observable.box<ClusterId | undefined>();
+
+export function getVisibleCluster() {
+  return visibleCluster.get();
+}
 
 observe(lensViews, change => {
   console.info(`lensViews change: type=${change.type} name=${change.name}`, toJS((change as any).newValue));
@@ -43,15 +50,34 @@ export function hasLoadedView(clusterId: ClusterId): boolean {
   return !!lensViews.get(clusterId)?.isLoaded;
 }
 
-export async function initView(clusterId: ClusterId) {
+/**
+ *
+ * @param clusterId The cluster to initialize the frame of
+ * @resolves to `true` if the frame has been initialized (or the ID is invalid) or `false` if
+ * a different window has submitted a claim for the cluster
+ */
+export async function initView(clusterId: ClusterId): Promise<boolean> {
   const cluster = ClusterStore.getInstance().getById(clusterId);
 
+  // If the cluster is unknown or this window already has an iframe active
+  // then do nothing as the cluster has already been initialized
   if (!cluster || lensViews.has(clusterId)) {
-    return;
+    return true;
+  }
+
+  // If we have not successfull claimed this cluster that means that a different
+  // window has, return false so that the called can navigate to it
+  if (!await requestMain(claimClusterFrameHandler, clusterId)) {
+    return false;
   }
 
   logger.info(`[LENS-VIEW]: init dashboard, clusterId=${clusterId}`);
   const parentElem = document.getElementById("lens-views");
+
+  if (!parentElem) {
+    throw new Error(`Failed to initialize view for clusterId=${clusterId}: DOM missing #lens-views`);
+  }
+
   const iframe = document.createElement("iframe");
 
   iframe.name = cluster.contextName;
@@ -69,26 +95,32 @@ export async function initView(clusterId: ClusterId) {
     await when(() => cluster.ready, { timeout: 5_000 }); // we cannot wait forever because cleanup would be blocked for broken cluster connections
     logger.info(`[LENS-VIEW]: cluster is ready, clusterId=${clusterId}`);
   } finally {
-    await autoCleanOnRemove(clusterId, iframe);
+    autoCleanOnRemove(clusterId, iframe);
   }
+
+  return true;
 }
 
-export async function autoCleanOnRemove(clusterId: ClusterId, iframe: HTMLIFrameElement) {
+function autoCleanOnRemove(clusterId: ClusterId, iframe: HTMLIFrameElement) {
-  await when(() => {
+  return when(
-    const cluster = ClusterStore.getInstance().getById(clusterId);
+    () => {
+      const cluster = ClusterStore.getInstance().getById(clusterId);
 
-    return !cluster || (cluster.disconnected && lensViews.get(clusterId)?.isLoaded);
+      return !cluster || (cluster.disconnected && lensViews.get(clusterId)?.isLoaded);
-  });
+    },
-  logger.info(`[LENS-VIEW]: remove dashboard, clusterId=${clusterId}`);
+    () => {
-  lensViews.delete(clusterId);
+      logger.info(`[LENS-VIEW]: remove dashboard, clusterId=${clusterId}`);
+      lensViews.delete(clusterId);
 
-  // Keep frame in DOM to avoid possible bugs when same cluster re-created after being removed.
+      // Keep frame in DOM to avoid possible bugs when same cluster re-created after being removed.
-  // In that case for some reasons `webFrame.routingId` returns some previous frameId (usage in app.tsx)
+      // In that case for some reasons `webFrame.routingId` returns some previous frameId (usage in app.tsx)
-  // Issue: https://github.com/lensapp/lens/issues/811
+      // Issue: https://github.com/lensapp/lens/issues/811
-  iframe.style.display = "none";
+      iframe.style.display = "none";
-  iframe.dataset.meta = `${iframe.name} was removed at ${new Date().toLocaleString()}`;
+      iframe.dataset.meta = `${iframe.name} was removed at ${new Date().toLocaleString()}`;
-  iframe.removeAttribute("name");
+      iframe.removeAttribute("name");
-  iframe.contentWindow.postMessage("teardown", "*");
+      iframe.contentWindow.postMessage("teardown", "*");
+    }
+  );
 }
 
 export function refreshViews(visibleClusterId?: string) {

src/renderer/components/hotbar/hotbar-entity-icon.tsx

@@ -31,7 +31,7 @@ import { cssNames, IClassName } from "../../utils";
 import { Icon } from "../icon";
 import { HotbarIcon } from "./hotbar-icon";
 import { HotbarStore } from "../../../common/hotbar-store";
-import { visibleCluster } from "../cluster-manager/lens-views";
+import { getVisibleCluster } from "../cluster-manager/lens-views";
 
 interface Props extends DOMAttributes<HTMLElement> {
   entity: CatalogEntity;
@@ -88,7 +88,7 @@ export class HotbarEntityIcon extends React.Component<Props> {
   }
 
   isActive(item: CatalogEntity) {
-    return visibleCluster.get() === item.getId();
+    return getVisibleCluster() === item.getId();
   }
 
   isPersisted(entity: CatalogEntity) {

src/renderer/ipc/index.tsx

@@ -28,7 +28,7 @@ import { isMac } from "../../common/vars";
 import { ClusterStore } from "../../common/cluster-store";
 import { navigate } from "../navigation";
 import { entitySettingsURL } from "../../common/routes";
-import { visibleCluster } from "../components/cluster-manager/lens-views";
+import { getVisibleCluster } from "../components/cluster-manager/lens-views";
 
 function sendToBackchannel(backchannel: string, notificationId: string, data: BackchannelArg): void {
   notificationsStore.remove(notificationId);
@@ -87,7 +87,7 @@ function ListNamespacesForbiddenHandler(event: IpcRendererEvent, ...[clusterId]:
     return void console.warn("[IPC]: ListNamespacesForbiddenHandler was called with unknown clusterId", { clusterId });
   }
 
-  const visibileClusterId = visibleCluster.get();
+  const visibileClusterId = getVisibleCluster();
 
   if (visibileClusterId && visibileClusterId !== clusterId) {
     return void console.debug("[IPC]: ListNamespacesForbiddenHandler not displaying notification that is not about the currently active cluster");