refactor(kube-port): making kube port generic (podman-desktop#9680)

* refactor(kube-port): making kube port generic Signed-off-by: axel7083 <[email protected]> * fix: typecheck Signed-off-by: axel7083 <[email protected]> * fix: typecheck again Signed-off-by: axel7083 <[email protected]> * fix: naming issue Signed-off-by: axel7083 <[email protected]> --------- Signed-off-by: axel7083 <[email protected]>
SoniaSandler · Oct 30, 2024 · ed0f39a · ed0f39a
1 parent 4ea503b
commit ed0f39a
Show file tree

Hide file tree

Showing 6 changed files with 115 additions and 56 deletions.
diff --git a/packages/renderer/src/lib/kube/details/KubeContainerArtifact.svelte b/packages/renderer/src/lib/kube/details/KubeContainerArtifact.svelte
@@ -2,7 +2,9 @@
 import type { V1Container } from '@kubernetes/client-node';
 
 import Cell from '/@/lib/details/DetailsCell.svelte';
-import KubeContainerPorts from '/@/lib/kube/details/KubeContainerPorts.svelte';
+import { WorkloadKind } from '/@api/kubernetes-port-forward-model';
+
+import KubePortsList from './KubePortsList.svelte';
 
 interface Props {
   artifact?: V1Container;
@@ -25,7 +27,12 @@ let { artifact, podName, namespace }: Props = $props();
     <Cell>Image Pull Policy</Cell>
     <Cell>{artifact.imagePullPolicy}</Cell>
   </tr>
-  <KubeContainerPorts namespace={namespace} podName={podName}  ports={artifact.ports}/>
+  <KubePortsList namespace={namespace} resourceName={podName} kind={WorkloadKind.POD} ports={artifact.ports?.map((port) => ({
+    name: port.name,
+    value: port.containerPort,
+    protocol: port.protocol,
+    displayValue: `${port.name ? port.name + ':' : ''}${port.containerPort}/${port.protocol}`,
+  }))}/>
   {#if artifact.env}
     <tr>
       <Cell>Environment Variables</Cell>

diff --git a/...ib/kube/details/KubeContainerPort.spec.ts → ...rer/src/lib/kube/details/KubePort.spec.ts b/...ib/kube/details/KubeContainerPort.spec.ts → ...rer/src/lib/kube/details/KubePort.spec.ts
@@ -21,9 +21,10 @@ import '@testing-library/jest-dom/vitest';
 import { fireEvent, render } from '@testing-library/svelte';
 import { beforeEach, describe, expect, test, vi } from 'vitest';
 
-import KubeContainerPort from '/@/lib/kube/details/KubeContainerPort.svelte';
 import { type UserForwardConfig, WorkloadKind } from '/@api/kubernetes-port-forward-model';
 
+import KubePort from './KubePort.svelte';
+
 beforeEach(() => {
   vi.resetAllMocks();
 
@@ -48,32 +49,36 @@ const DUMMY_FORWARD_CONFIG: UserForwardConfig = {
 
 describe('port forwarding', () => {
   test('forward button should be visible and unique for each container port', async () => {
-    const { getByTitle } = render(KubeContainerPort, {
+    const { getByTitle } = render(KubePort, {
       namespace: 'dummy-ns',
       port: {
-        containerPort: 80,
+        displayValue: '80/TCP',
+        value: 80,
         protocol: 'TCP',
       },
       forwardConfig: undefined,
-      podName: 'dummy-pod-name',
+      resourceName: 'dummy-pod-name',
+      kind: WorkloadKind.POD,
     });
 
-    const port80 = getByTitle('Forward container port 80');
+    const port80 = getByTitle('Forward port 80');
     expect(port80).toBeDefined();
   });
 
   test('forward button should call ', async () => {
-    const { getByTitle } = render(KubeContainerPort, {
+    const { getByTitle } = render(KubePort, {
       namespace: 'dummy-ns',
       port: {
-        containerPort: 80,
+        displayValue: '80/TCP',
+        value: 80,
         protocol: 'TCP',
       },
       forwardConfig: undefined,
-      podName: 'dummy-pod-name',
+      resourceName: 'dummy-pod-name',
+      kind: WorkloadKind.POD,
     });
 
-    const forwardBtn = getByTitle('Forward container port 80');
+    const forwardBtn = getByTitle('Forward port 80');
     await fireEvent.click(forwardBtn);
 
     await vi.waitFor(() => {
@@ -92,14 +97,16 @@ describe('port forwarding', () => {
   });
 
   test('existing forward should display actions', async () => {
-    const { getByTitle } = render(KubeContainerPort, {
+    const { getByTitle } = render(KubePort, {
       namespace: 'dummy-ns',
       port: {
-        containerPort: 80,
+        displayValue: '80/TCP',
+        value: 80,
         protocol: 'TCP',
       },
       forwardConfig: DUMMY_FORWARD_CONFIG,
-      podName: 'dummy-pod-name',
+      resourceName: 'dummy-pod-name',
+      kind: WorkloadKind.POD,
     });
 
     const openBtn = getByTitle('Open in browser');
@@ -110,14 +117,16 @@ describe('port forwarding', () => {
   });
 
   test('open button should use window.openExternal with proper local port', async () => {
-    const { getByTitle } = render(KubeContainerPort, {
+    const { getByTitle } = render(KubePort, {
       namespace: 'dummy-ns',
       port: {
-        containerPort: 80,
+        displayValue: '80/TCP',
+        value: 80,
         protocol: 'TCP',
       },
       forwardConfig: DUMMY_FORWARD_CONFIG,
-      podName: 'dummy-pod-name',
+      resourceName: 'dummy-pod-name',
+      kind: WorkloadKind.POD,
     });
 
     const openBtn = getByTitle('Open in browser');
@@ -129,14 +138,16 @@ describe('port forwarding', () => {
   });
 
   test('remove button should use window.deleteKubernetesPortForward with proper local port', async () => {
-    const { getByTitle } = render(KubeContainerPort, {
+    const { getByTitle } = render(KubePort, {
       namespace: 'dummy-ns',
       port: {
-        containerPort: 80,
+        displayValue: '80/TCP',
+        value: 80,
         protocol: 'TCP',
       },
       forwardConfig: DUMMY_FORWARD_CONFIG,
-      podName: 'dummy-pod-name',
+      resourceName: 'dummy-pod-name',
+      kind: WorkloadKind.POD,
     });
 
     const removeBtn = getByTitle('Remove port forward');
@@ -153,17 +164,19 @@ describe('port forwarding', () => {
   test('error from createKubernetesPortForward should be displayed', async () => {
     vi.mocked(window.createKubernetesPortForward).mockRejectedValue('Dummy error');
 
-    const { getByTitle, getByRole } = render(KubeContainerPort, {
+    const { getByTitle, getByRole } = render(KubePort, {
       namespace: 'dummy-ns',
       port: {
-        containerPort: 80,
+        displayValue: '80/TCP',
+        value: 80,
         protocol: 'TCP',
       },
       forwardConfig: undefined,
-      podName: 'dummy-pod-name',
+      resourceName: 'dummy-pod-name',
+      kind: WorkloadKind.POD,
     });
 
-    const port80 = getByTitle('Forward container port 80');
+    const port80 = getByTitle('Forward port 80');
     await fireEvent.click(port80);
 
     await vi.waitFor(() => {

diff --git a/...lib/kube/details/KubeContainerPort.svelte → ...erer/src/lib/kube/details/KubePort.svelte b/...lib/kube/details/KubeContainerPort.svelte → ...erer/src/lib/kube/details/KubePort.svelte
@@ -1,44 +1,46 @@
 <script lang="ts">
 import { faSquareUpRight, faTrash } from '@fortawesome/free-solid-svg-icons';
-import type { V1ContainerPort } from '@kubernetes/client-node';
 import { Button, ErrorMessage } from '@podman-desktop/ui-svelte';
 
-import { type PortMapping, type UserForwardConfig, WorkloadKind } from '/@api/kubernetes-port-forward-model';
+import type { PortMapping, UserForwardConfig, WorkloadKind } from '/@api/kubernetes-port-forward-model';
+
+import type { KubePortInfo } from './kube-port';
 
 interface Props {
-  port: V1ContainerPort;
+  port: KubePortInfo;
   forwardConfig?: UserForwardConfig;
-  podName?: string;
+  resourceName?: string;
   namespace?: string;
+  kind: WorkloadKind;
 }
 
-let { port, forwardConfig, podName, namespace }: Props = $props();
+let { port, forwardConfig, resourceName, namespace, kind }: Props = $props();
 
 let mapping: PortMapping | undefined = $derived(
-  forwardConfig?.forwards.find(mapping => mapping.remotePort === port.containerPort),
+  forwardConfig?.forwards.find(mapping => mapping.remotePort === port.value),
 );
 let loading: boolean = $state(false);
 let error: string | undefined = $state(undefined);
 
-async function onForwardRequest(port: V1ContainerPort): Promise<void> {
-  if (!podName) throw new Error('pod name is undefined');
+async function onForwardRequest(port: KubePortInfo): Promise<void> {
+  if (!resourceName) throw new Error('pod name is undefined');
   loading = true;
   error = undefined;
 
   // get a free port starting from 50k
   const freePort = await window.getFreePort(50_000);
 
   // snapshot the object as Proxy cannot be serialized
-  const snapshot = $state.snapshot(port);
+  const snapshot: KubePortInfo = $state.snapshot(port);
   try {
     await window.createKubernetesPortForward({
-      displayName: `${podName}/${snapshot.name}`,
-      name: podName,
-      kind: WorkloadKind.POD,
+      displayName: `${resourceName}/${snapshot.name}`,
+      name: resourceName,
+      kind: kind,
       namespace: namespace ?? 'default',
       forward: {
         localPort: freePort,
-        remotePort: snapshot.containerPort,
+        remotePort: snapshot.value,
       },
     });
     error = undefined;
@@ -70,8 +72,8 @@ async function removePortForward(): Promise<void> {
 }
 </script>
 
-<span aria-label="container port {port.containerPort}" class="flex gap-x-2 items-center">
-  {port.containerPort}/{port.protocol}
+<span aria-label="port {port.value}" class="flex gap-x-2 items-center">
+  {port.displayValue}
   {#if mapping}
     <Button title="Open in browser" disabled={loading} icon={faSquareUpRight} on:click={openExternal.bind(undefined)} class="px-1 py-0.5" padding="0">
       Open
@@ -80,7 +82,7 @@ async function removePortForward(): Promise<void> {
       Remove
     </Button>
   {:else}
-    <Button title="Forward container port {port.containerPort}" disabled={loading} on:click={onForwardRequest.bind(undefined, port)} class="px-1 py-0.5" padding="0">
+    <Button title="Forward port {port.value}" disabled={loading} on:click={onForwardRequest.bind(undefined, port)} class="px-1 py-0.5" padding="0">
       Forward...
     </Button>
   {/if}

diff --git a/...b/kube/details/KubeContainerPorts.spec.ts → ...rc/lib/kube/details/KubePortsList.spec.ts b/...b/kube/details/KubeContainerPorts.spec.ts → ...rc/lib/kube/details/KubePortsList.spec.ts
@@ -22,8 +22,10 @@ import { render } from '@testing-library/svelte';
 import { readable } from 'svelte/store';
 import { beforeEach, expect, test, vi } from 'vitest';
 
-import KubeContainerPorts from '/@/lib/kube/details/KubeContainerPorts.svelte';
 import * as kubeContextStore from '/@/stores/kubernetes-contexts-state';
+import { WorkloadKind } from '/@api/kubernetes-port-forward-model';
+
+import KubePortsList from './KubePortsList.svelte';
 
 vi.mock('/@/stores/kubernetes-contexts-state', async () => ({}));
 
@@ -34,17 +36,22 @@ beforeEach(() => {
 });
 
 test('expect port title not to be visible when no ports provided', async () => {
-  const { queryByText } = render(KubeContainerPorts);
+  const { queryByText } = render(KubePortsList);
 
   const title = queryByText('Ports');
   expect(title).toBeNull();
 });
 
 test('expect port title to be visible when ports is defined', async () => {
-  const { getByText } = render(KubeContainerPorts, {
+  const { getByText } = render(KubePortsList, {
+    resourceName: 'dummy-resource-name',
+    namespace: 'dummy-ns',
+    kind: WorkloadKind.POD,
     ports: [
       {
-        containerPort: 80,
+        displayValue: '80/TCP',
+        value: 80,
+        protocol: 'TCP',
       },
     ],
   });
@@ -54,14 +61,19 @@ test('expect port title to be visible when ports is defined', async () => {
 });
 
 test('expect multiple ports to be visible', async () => {
-  const { getByText } = render(KubeContainerPorts, {
+  const { getByText } = render(KubePortsList, {
+    resourceName: 'dummy-resource-name',
+    namespace: 'dummy-ns',
+    kind: WorkloadKind.POD,
     ports: [
       {
-        containerPort: 80,
+        displayValue: '80/TCP',
+        value: 80,
         protocol: 'TCP',
       },
       {
-        containerPort: 100,
+        displayValue: '100/TCP',
+        value: 80,
         protocol: 'TCP',
       },
     ],

diff --git a/...ib/kube/details/KubeContainerPorts.svelte → ...src/lib/kube/details/KubePortsList.svelte b/...ib/kube/details/KubeContainerPorts.svelte → ...src/lib/kube/details/KubePortsList.svelte
@@ -1,22 +1,23 @@
 <script lang="ts">
-import type { V1ContainerPort } from '@kubernetes/client-node';
-
 import Cell from '/@/lib/details/DetailsCell.svelte';
-import KubeContainerPort from '/@/lib/kube/details/KubeContainerPort.svelte';
 import { kubernetesCurrentContextPortForwards } from '/@/stores/kubernetes-contexts-state';
-import { type UserForwardConfig, WorkloadKind } from '/@api/kubernetes-port-forward-model';
+import type { UserForwardConfig, WorkloadKind } from '/@api/kubernetes-port-forward-model';
+
+import type { KubePortInfo } from './kube-port';
+import KubePort from './KubePort.svelte';
 
 interface Props {
-  ports?: V1ContainerPort[];
-  podName?: string;
+  ports?: KubePortInfo[];
+  resourceName?: string;
   namespace?: string;
+  kind: WorkloadKind;
 }
 
-let { ports, podName, namespace }: Props = $props();
+let { ports, resourceName, namespace, kind }: Props = $props();
 
 let userForwardConfig: UserForwardConfig | undefined = $derived(
   $kubernetesCurrentContextPortForwards.find(
-    forward => forward.kind === WorkloadKind.POD && forward.name === podName && forward.namespace === namespace,
+    forward => forward.kind === kind && forward.name === resourceName && forward.namespace === namespace,
   ),
 );
 </script>
@@ -27,7 +28,7 @@ let userForwardConfig: UserForwardConfig | undefined = $derived(
     <Cell>
       <div class="flex gap-y-1 flex-col">
         {#each ports as port}
-          <KubeContainerPort namespace={namespace} podName={podName} port={port} forwardConfig={userForwardConfig}/>
+          <KubePort namespace={namespace} kind={kind} resourceName={resourceName} port={port} forwardConfig={userForwardConfig}/>
         {/each}
       </div>
     </Cell>

diff --git a/packages/renderer/src/lib/kube/details/kube-port.ts b/packages/renderer/src/lib/kube/details/kube-port.ts
@@ -0,0 +1,24 @@
+/**********************************************************************
+ * Copyright (C) 2024 Red Hat, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ ***********************************************************************/
+
+export interface KubePortInfo {
+  displayValue: string;
+  name?: string;
+  protocol?: string;
+  value: number;
+}