From 6d63c39b117cd89fb51da535b821082b7d17f988 Mon Sep 17 00:00:00 2001 From: Jim Ehrismann Date: Mon, 4 Oct 2021 21:56:03 -0400 Subject: [PATCH] addressed some review comments Signed-off-by: Jim Ehrismann --- src/common/routes/port-forwards.ts | 3 +- src/main/routes/port-forward-route.ts | 36 ++++++------ .../service-port-component.scss | 1 + .../service-port-component.tsx | 31 +++++----- .../+workloads-pods/pod-container-port.scss | 1 + .../+workloads-pods/pod-container-port.tsx | 21 ++++--- .../port-forward/port-forward-dialog.tsx | 21 +++---- .../port-forward/port-forward-item.ts | 18 +++++- .../port-forward/port-forward.store.ts | 57 ++++--------------- 9 files changed, 80 insertions(+), 109 deletions(-) diff --git a/src/common/routes/port-forwards.ts b/src/common/routes/port-forwards.ts index bf644e5003..d05b8583ab 100644 --- a/src/common/routes/port-forwards.ts +++ b/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(portForwardsRoute.path); diff --git a/src/main/routes/port-forward-route.ts b/src/main/routes/port-forward-route.ts index 641e8af843..efc9d791bb 100644 --- a/src/main/routes/port-forward-route.ts +++ b/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); } } } diff --git a/src/renderer/components/+network-services/service-port-component.scss b/src/renderer/components/+network-services/service-port-component.scss index 579edc7c2c..c22f41d1e0 100644 --- a/src/renderer/components/+network-services/service-port-component.scss +++ b/src/renderer/components/+network-services/service-port-component.scss @@ -33,6 +33,7 @@ cursor: pointer; color: $primary; text-decoration: underline; + padding-right: 1em; } .portInput { diff --git a/src/renderer/components/+network-services/service-port-component.tsx b/src/renderer/components/+network-services/service-port-component.tsx index 0fffcd09f9..fb420acc4e 100644 --- a/src/renderer/components/+network-services/service-port-component.tsx +++ b/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 { } 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 { 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 { 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 { 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 { port: port.port.toString(), forwardPort: this.forwardPort.toString() }; - - PortForwardDialog.open(portForward, true); + + PortForwardDialog.open(portForward, { openInBrowser: true }); } }; return (
- this.portForward() }> + this.portForward()}> {port.toString()} - {this.waiting && ( - - )} - {" "} - + + {this.waiting && ( + + )}
); } diff --git a/src/renderer/components/+workloads-pods/pod-container-port.scss b/src/renderer/components/+workloads-pods/pod-container-port.scss index c34e4a51e3..efd5592065 100644 --- a/src/renderer/components/+workloads-pods/pod-container-port.scss +++ b/src/renderer/components/+workloads-pods/pod-container-port.scss @@ -34,6 +34,7 @@ color: $primary; text-decoration: underline; position: relative; + padding-right: 1em; } .portInput { diff --git a/src/renderer/components/+workloads-pods/pod-container-port.tsx b/src/renderer/components/+workloads-pods/pod-container-port.tsx index 2a0b98a68a..9543a27cfa 100644 --- a/src/renderer/components/+workloads-pods/pod-container-port.tsx +++ b/src/renderer/components/+workloads-pods/pod-container-port.tsx @@ -111,13 +111,13 @@ export class PodContainerPort extends React.Component { 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 { 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 { port: port.containerPort.toString(), forwardPort: this.forwardPort.toString() }; - - PortForwardDialog.open(portForward, true); + + PortForwardDialog.open(portForward, { openInBrowser: true }); } }; return (
- this.portForward() }> + this.portForward()}> {text} - {this.waiting && ( - - )} - {" "} - + + {this.waiting && ( + + )}
); } diff --git a/src/renderer/port-forward/port-forward-dialog.tsx b/src/renderer/port-forward/port-forward-dialog.tsx index 2a3949d5d6..7d0ff7524c 100644 --- a/src/renderer/port-forward/port-forward-dialog.tsx +++ b/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 { } +interface PortForwardDialogOpenOptions { + openInBrowser: boolean +} + const dialogState = observable.object({ isOpen: false, data: null as ForwardedPort, @@ -53,10 +57,10 @@ export class PortForwardDialog extends Component { 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 { PortForwardDialog.close(); }; - @computed get scaleMax() { - const { currentPort } = this; - const defaultMax = 50; - - return currentPort <= defaultMax - ? defaultMax * 2 - : currentPort * 2; - } - onOpen = async () => { const { portForward } = this; diff --git a/src/renderer/port-forward/port-forward-item.ts b/src/renderer/port-forward/port-forward-item.ts index 4d5390af75..0b79fff620 100644 --- a/src/renderer/port-forward/port-forward-item.ts +++ b/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); } diff --git a/src/renderer/port-forward/port-forward.store.ts b/src/renderer/port-forward/port-forward.store.ts index a6f43eaf14..96e1b6e34e 100644 --- a/src/renderer/port-forward/port-forward.store.ts +++ b/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 { private storage = createStorage("port_forwards", undefined); @@ -56,36 +47,18 @@ export class PortForwardStore extends ItemStore { 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 { 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; });