addressed some review comments

Signed-off-by: Jim Ehrismann <jehrismann@mirantis.com>

src/common/routes/port-forwards.ts

@@ -23,11 +23,10 @@ import type { RouteProps } from "react-router";
 import { buildURL } from "../utils/buildUrl";
 
 export const portForwardsRoute: RouteProps = {
-  path: "/port-forwards/:forwardport?"
+  path: "/port-forwards"
 };
 
 export interface PortForwardsRouteParams {
-  forwardport?: string;
 }
 
 export const portForwardsURL = buildURL<PortForwardsRouteParams>(portForwardsRoute.path);

src/main/routes/port-forward-route.ts

@@ -76,7 +76,7 @@ class PortForward {
       "port-forward",
       "-n", this.namespace,
       `${this.kind}/${this.name}`,
-      `${this.forwardPort??""}:${this.port}`
+      `${this.forwardPort ?? ""}:${this.port}`
     ];
 
     this.process = spawn(kubectlBin, args, {
@@ -126,11 +126,11 @@ export class PortForwardRoute {
 
       let thePort: string; // undefined
       const portNum = Number(forwardPort);
-      
+
       if (portNum > 0 && portNum < 65536) {
         thePort = forwardPort;
       }
-      
+
       if (!portForward) {
         logger.info(`Creating a new port-forward ${namespace}/${resourceType}/${resourceName}:${port}`);
         portForward = new PortForward(await cluster.getProxyKubeconfigPath(), {
@@ -153,7 +153,7 @@ export class PortForwardRoute {
         }
       }
 
-      respondJson(response, {port: portForward.internalPort});
+      respondJson(response, { port: portForward.internalPort });
     } catch (error) {
       logger.error(`[PORT-FORWARD-ROUTE]: failed to open a port-forward: ${error}`, { namespace, port, resourceType, resourceName });
 
@@ -164,7 +164,7 @@ export class PortForwardRoute {
   }
 
   static async routeCurrentPortForward(request: LensApiRequest) {
-    const { params, response, cluster} = request;
+    const { params, response, cluster } = request;
     const { namespace, port, forwardPort, resourceType, resourceName } = params;
 
     const portForward = PortForward.getPortforward({
@@ -172,32 +172,28 @@ export class PortForwardRoute {
       namespace, port, forwardPort
     });
 
-    respondJson(response, {port: portForward?.internalPort?? null});
+    respondJson(response, { port: portForward?.internalPort ?? null });
   }
 
   static async routeAllPortForwards(request: LensApiRequest) {
     const { response } = request;
 
-    const portForwards: PortForwardArgs[] = [];
-
-    PortForward.portForwards.forEach(f => {
-      const pf: PortForwardArgs = {
+    const portForwards: PortForwardArgs[] = PortForward.portForwards.map(f => (
+      {
         clusterId: f.clusterId,
         kind: f.kind,
         namespace: f.namespace,
         name: f.name,
         port: f.port,
         forwardPort: f.forwardPort,
-      };
+      })
+    );
 
-      portForwards.push(pf);
-    });
-
-    respondJson(response, {portForwards});
+    respondJson(response, { portForwards });
   }
 
   static async routeCurrentPortForwardStop(request: LensApiRequest) {
-    const { params, response, cluster} = request;
+    const { params, response, cluster } = request;
     const { namespace, port, forwardPort, resourceType, resourceName } = params;
 
     const portForward = PortForward.getPortforward({
@@ -207,9 +203,13 @@ export class PortForwardRoute {
 
     try {
       await portForward.stop();
-      respondJson(response, {status: true});
+      respondJson(response, { status: true });
     } catch (error) {
-      logger.error(error);
+      logger.error("[PORT-FORWARD-ROUTE]: error stopping a port-forward", { namespace, port, forwardPort, resourceType, resourceName });
+
+      return respondJson(response, {
+        message: `error stopping a forward port ${port}`
+      }, 400);
     }
   }
 }

src/renderer/components/+network-services/service-port-component.scss

@@ -33,6 +33,7 @@
     cursor: pointer;
     color: $primary;
     text-decoration: underline;
+    padding-right: 1em;
   }
 
   .portInput {

src/renderer/components/+network-services/service-port-component.tsx

@@ -24,8 +24,8 @@ import "./service-port-component.scss";
 import React from "react";
 import { disposeOnUnmount, observer } from "mobx-react";
 import type { Service, ServicePort } from "../../../common/k8s-api/endpoints";
-import { observable, makeObservable, IReactionDisposer, reaction } from "mobx";
-import { cssNames } from "../../utils";
+import { observable, makeObservable, reaction } from "mobx";
+import { cssNames, disposer } from "../../utils";
 import { Notifications } from "../notifications";
 import { Button } from "../button";
 import { addPortForward, getPortForward, openPortForward, PortForwardDialog, portForwardStore, removePortForward } from "../../port-forward";
@@ -57,11 +57,9 @@ export class ServicePortComponent extends React.Component<Props> {
   }
 
   watch() {
-    const disposers: IReactionDisposer[] = [
+    return disposer(
       reaction(() => portForwardStore.portForwards, () => this.init()),
-    ];
-
-    return () => disposers.forEach((dispose) => dispose());
+    );
   }
 
   init() {
@@ -113,7 +111,7 @@ export class ServicePortComponent extends React.Component<Props> {
         openPortForward(portForward);
         this.isPortForwarded = true;
       }
-    } catch(error) {
+    } catch (error) {
       Notifications.error(error);
     } finally {
       this.waiting = false;
@@ -135,7 +133,7 @@ export class ServicePortComponent extends React.Component<Props> {
     try {
       await removePortForward(portForward);
       this.isPortForwarded = false;
-    } catch(error) {
+    } catch (error) {
       Notifications.error(error);
     } finally {
       this.waiting = false;
@@ -148,7 +146,7 @@ export class ServicePortComponent extends React.Component<Props> {
     const portForwardAction = async () => {
       if (this.isPortForwarded) {
         await this.stopPortForward();
-      }else {
+      } else {
         const portForward: ForwardedPort = {
           kind: "service",
           name: service.getName(),
@@ -156,21 +154,20 @@ export class ServicePortComponent extends React.Component<Props> {
           port: port.port.toString(),
           forwardPort: this.forwardPort.toString()
         };
-            
-        PortForwardDialog.open(portForward, true);
+
+        PortForwardDialog.open(portForward, { openInBrowser: true });
       }
     };
 
     return (
       <div className={cssNames("ServicePortComponent", { waiting: this.waiting })}>
-        <span title="Open in a browser" onClick={() => this.portForward() }>
+        <span title="Open in a browser" onClick={() => this.portForward()}>
           {port.toString()}
-          {this.waiting && (
-            <Spinner />
-          )}
         </span>
-        {" "}
-        <Button onClick={() => portForwardAction()}> {this.isPortForwarded ? "Stop":"Forward..."} </Button>
+        <Button onClick={() => portForwardAction()}> {this.isPortForwarded ? "Stop" : "Forward..."} </Button>
+        {this.waiting && (
+          <Spinner />
+        )}
       </div>
     );
   }

src/renderer/components/+workloads-pods/pod-container-port.scss

@@ -34,6 +34,7 @@
     color: $primary;
     text-decoration: underline;
     position: relative;
+    padding-right: 1em;
   }
 
   .portInput {

src/renderer/components/+workloads-pods/pod-container-port.tsx

@@ -111,13 +111,13 @@ export class PodContainerPort extends React.Component<Props> {
 
     try {
       this.forwardPort = await addPortForward(portForward);
-      
+
       if (this.forwardPort) {
         portForward.forwardPort = this.forwardPort.toString();
         openPortForward(portForward);
         this.isPortForwarded = true;
       }
-    } catch(error) {
+    } catch (error) {
       Notifications.error(error);
     } finally {
       this.waiting = false;
@@ -139,7 +139,7 @@ export class PodContainerPort extends React.Component<Props> {
     try {
       await removePortForward(portForward);
       this.isPortForwarded = false;
-    } catch(error) {
+    } catch (error) {
       Notifications.error(error);
     } finally {
       this.waiting = false;
@@ -162,21 +162,20 @@ export class PodContainerPort extends React.Component<Props> {
           port: port.containerPort.toString(),
           forwardPort: this.forwardPort.toString()
         };
-        
-        PortForwardDialog.open(portForward, true);
+
+        PortForwardDialog.open(portForward, { openInBrowser: true });
       }
     };
 
     return (
       <div className={cssNames("PodContainerPort", { waiting: this.waiting })}>
-        <span title="Open in a browser" onClick={() => this.portForward() }>
+        <span title="Open in a browser" onClick={() => this.portForward()}>
           {text}
-          {this.waiting && (
-            <Spinner />
-          )}
         </span>
-        {" "}
-        <Button onClick={() => portForwardAction()}> {this.isPortForwarded ? "Stop":"Forward..."} </Button>
+        <Button onClick={() => portForwardAction()}> {this.isPortForwarded ? "Stop" : "Forward..."} </Button>
+        {this.waiting && (
+          <Spinner />
+        )}
       </div>
     );
   }

src/renderer/port-forward/port-forward-dialog.tsx

@@ -22,7 +22,7 @@
 import "./port-forward-dialog.scss";
 
 import React, { Component } from "react";
-import { computed, observable, makeObservable } from "mobx";
+import { observable, makeObservable } from "mobx";
 import { observer } from "mobx-react";
 import { Dialog, DialogProps } from "../components/dialog";
 import { Wizard, WizardStep } from "../components/wizard";
@@ -30,12 +30,16 @@ import { Input } from "../components/input";
 import { Notifications } from "../components/notifications";
 import { cssNames } from "../utils";
 import { addPortForward, modifyPortForward } from "./port-forward.store";
-import type { ForwardedPort } from "./port-forward.store";
+import type { ForwardedPort } from "./port-forward-item";
 import { openPortForward } from ".";
 
 interface Props extends Partial<DialogProps> {
 }
 
+interface PortForwardDialogOpenOptions {
+  openInBrowser: boolean
+}
+
 const dialogState = observable.object({
   isOpen: false,
   data: null as ForwardedPort,
@@ -53,10 +57,10 @@ export class PortForwardDialog extends Component<Props> {
     makeObservable(this);
   }
 
-  static open(portForward: ForwardedPort, openInBrowser = false) {
+  static open(portForward: ForwardedPort, options : PortForwardDialogOpenOptions = { openInBrowser:  false }) {
     dialogState.isOpen = true;
     dialogState.data = portForward;
-    dialogState.openInBrowser = openInBrowser;
+    dialogState.openInBrowser = options.openInBrowser;
   }
 
   static close() {
@@ -71,15 +75,6 @@ export class PortForwardDialog extends Component<Props> {
     PortForwardDialog.close();
   };
 
-  @computed get scaleMax() {
-    const { currentPort } = this;
-    const defaultMax = 50;
-
-    return currentPort <= defaultMax
-      ? defaultMax * 2
-      : currentPort * 2;
-  }
-
   onOpen = async () => {
     const { portForward } = this;
 

src/renderer/port-forward/port-forward-item.ts

@@ -23,6 +23,15 @@
 import type { ItemObject } from "../../common/item.store";
 import { autoBind } from "../../common/utils";
 
+export interface ForwardedPort {
+  clusterId?: string;
+  kind: string;
+  namespace: string;
+  name: string;
+  port: string;
+  forwardPort: string;
+}
+  
 export class PortForwardItem implements ItemObject {
   clusterId: string;
   kind: string;
@@ -31,7 +40,14 @@ export class PortForwardItem implements ItemObject {
   port: string;
   forwardPort: string;
   
-  constructor() {
+  constructor(pf: ForwardedPort) {
+    this.clusterId = pf.clusterId;
+    this.kind = pf.kind;
+    this.namespace = pf.namespace;
+    this.name = pf.name;
+    this.port = pf.port;
+    this.forwardPort = pf.forwardPort;
+
     autoBind(this);
   }
   

src/renderer/port-forward/port-forward.store.ts

@@ -20,24 +20,15 @@
  */
 
 
-import { computed, IReactionDisposer, makeObservable, observable, reaction } from "mobx";
-import { ItemObject, ItemStore } from "../../common/item.store";
-import { autoBind, createStorage, getHostedClusterId, openExternal } from "../utils";
-import { PortForwardItem } from "./port-forward-item";
+import { makeObservable, observable, reaction } from "mobx";
+import { ItemStore } from "../../common/item.store";
+import { autoBind, createStorage, disposer, getHostedClusterId, openExternal } from "../utils";
+import { ForwardedPort, PortForwardItem } from "./port-forward-item";
 import { apiBase } from "../api";
 import { waitUntilFree } from "tcp-port-used";
 import { Notifications } from "../components/notifications";
 import logger from "../../common/logger";
 
-export interface ForwardedPort {
-  clusterId?: string;
-  kind: string;
-  namespace: string;
-  name: string;
-  port: string;
-  forwardPort: string;
-}
-
 export class PortForwardStore extends ItemStore<PortForwardItem> {
   private storage = createStorage<ForwardedPort[] | undefined>("port_forwards", undefined);
 
@@ -56,36 +47,18 @@ export class PortForwardStore extends ItemStore<PortForwardItem> {
 
     if (Array.isArray(savedPortForwards)) {
       logger.info("[PORT_FORWARD] starting saved port-forwards");
-      await Promise.all(savedPortForwards.map(pf => {
-        const port = new PortForwardItem;
+      await Promise.all(savedPortForwards.map(addPortForward));
 
-        port.clusterId = pf.clusterId;
-        port.kind = pf.kind;
-        port.namespace = pf.namespace;
-        port.name = pf.name;
-        port.port = pf.port;
-        port.forwardPort = pf.forwardPort;
-
-        return addPortForward(port);
-      }));
-
-      this.portForwards = [];
+      this.reset();
     }
   }
 
-  @observable selectedItemId?: string;
   @observable portForwards: PortForwardItem[];
 
-  @computed get selectedItem() {
-    return this.portForwards.find((e: ItemObject) => e.getId() === this.selectedItemId);
-  }
-
   watch() {
-    const disposers: IReactionDisposer[] = [
+    return disposer(
       reaction(() => this.portForwards, () => this.loadAll()),
-    ];
-
-    return () => disposers.forEach((dispose) => dispose());
+    );
   }
 
   loadAll() {
@@ -93,21 +66,11 @@ export class PortForwardStore extends ItemStore<PortForwardItem> {
       let portForwards = await getPortForwards();
 
       // filter out any not for this cluster
-      portForwards = portForwards?.filter(pf => pf.clusterId == getHostedClusterId());
+      portForwards = portForwards.filter(pf => pf.clusterId == getHostedClusterId());
       this.storage.set(portForwards);
 
       this.reset();
-      portForwards?.forEach(pf => {
-        const port = new PortForwardItem;
-
-        port.clusterId = pf.clusterId;
-        port.kind = pf.kind;
-        port.namespace = pf.namespace;
-        port.name = pf.name;
-        port.port = pf.port;
-        port.forwardPort = pf.forwardPort;
-        this.portForwards.push(port);
-      });
+      portForwards.map(pf => this.portForwards.push(new PortForwardItem(pf)));
 
       return this.portForwards;
     });