diff --git a/packages/main/src/plugin/index.ts b/packages/main/src/plugin/index.ts index f47524ed90dbb..091867b222c0c 100644 --- a/packages/main/src/plugin/index.ts +++ b/packages/main/src/plugin/index.ts @@ -52,7 +52,7 @@ import type { PullEvent } from './api/pull-event.js'; import type { ExtensionInfo } from './api/extension-info.js'; import type { ImageInspectInfo } from './api/image-inspect-info.js'; import type { TrayMenu } from '../tray-menu.js'; -import { getFreePort, getFreePortRange } from './util/port.js'; +import { getFreePort, getFreePortRange, isFreePort } from './util/port.js'; import { isLinux, isMac } from '../util.js'; import type { MessageBoxOptions, MessageBoxReturnValue } from './message-box.js'; import { MessageBox } from './message-box.js'; @@ -1326,6 +1326,10 @@ export class PluginSystem { return getFreePortRange(rangeSize); }); + this.ipcHandle('system:is-port-free', async (_, port: number): Promise => { + return isFreePort(port); + }); + this.ipcHandle( 'provider-registry:startReceiveLogs', async ( diff --git a/packages/preload/src/index.ts b/packages/preload/src/index.ts index c355754266cdb..aa7e1f0096b76 100644 --- a/packages/preload/src/index.ts +++ b/packages/preload/src/index.ts @@ -1228,6 +1228,10 @@ function initExposure(): void { return ipcInvoke('system:get-free-port-range', rangeSize); }); + contextBridge.exposeInMainWorld('isFreePort', async (port: number): Promise => { + return ipcInvoke('system:is-port-free', port); + }); + type LogFunction = (...data: unknown[]) => void; let onDataCallbacksStartReceiveLogsId = 0; diff --git a/packages/renderer/src/lib/image/RunImage.spec.ts b/packages/renderer/src/lib/image/RunImage.spec.ts index b345764902375..f564eb7fb85f1 100644 --- a/packages/renderer/src/lib/image/RunImage.spec.ts +++ b/packages/renderer/src/lib/image/RunImage.spec.ts @@ -19,7 +19,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import '@testing-library/jest-dom/vitest'; -import { test, vi, type Mock, beforeAll, describe, expect, beforeEach } from 'vitest'; +import { test, vi, type Mock, beforeAll, describe, expect, beforeEach, afterEach } from 'vitest'; import { fireEvent, render, screen } from '@testing-library/svelte'; import { runImageInfo } from '../../stores/run-image-store'; import RunImage from '/@/lib/image/RunImage.svelte'; @@ -28,6 +28,8 @@ import { mockBreadcrumb } from '../../stores/breadcrumb.spec'; import userEvent from '@testing-library/user-event'; import { router } from 'tinro'; +const originalConsoleDebug = console.debug; + // fake the window.events object beforeAll(() => { (window.events as unknown) = { @@ -40,19 +42,26 @@ beforeAll(() => { (window as any).listNetworks = vi.fn().mockResolvedValue([]); (window as any).listContainers = vi.fn().mockResolvedValue([]); (window as any).createAndStartContainer = vi.fn().mockResolvedValue({ id: '1234' }); + (window as any).getFreePort = vi.fn(); + (window as any).isFreePort = vi.fn(); mockBreadcrumb(); }); beforeEach(() => { + console.error = vi.fn(); vi.clearAllMocks(); }); +afterEach(() => { + console.error = originalConsoleDebug; +}); + async function waitRender() { const result = render(RunImage); //wait until dataReady is true - while (result.component.$$.ctx[31] !== true) { + while (result.component.$$.ctx[30] !== true) { await new Promise(resolve => setTimeout(resolve, 100)); } return result; @@ -447,4 +456,142 @@ describe('RunImage', () => { expect.objectContaining({ EnvFiles: [customEnvFile, 'foo3'] }), ); }); + + test('Expect "start container" button to be disabled if user adds a port which is invalid (lower than 0)', async () => { + router.goto('/basic'); + + await createRunImage(undefined, ['command1', 'command2']); + + const link1 = screen.getByRole('link', { name: 'Basic' }); + await fireEvent.click(link1); + + const customMappingButton = screen.getByRole('button', { name: 'Add custom port mapping' }); + await fireEvent.click(customMappingButton); + + const hostInput = screen.getByLabelText('host port'); + await userEvent.click(hostInput); + await userEvent.clear(hostInput); + // adds a negative port + await userEvent.keyboard('-1'); + + const containerInput = screen.getByLabelText('container port'); + await userEvent.click(containerInput); + await userEvent.clear(containerInput); + await userEvent.keyboard('80'); + + const button = screen.getByRole('button', { name: 'Start Container' }); + expect((button as HTMLButtonElement).disabled).toBeTruthy(); + }); + + test('Expect "start container" button to be disabled if user adds a port which is invalid (over upper limit > 65535)', async () => { + router.goto('/basic'); + + await createRunImage(undefined, ['command1', 'command2']); + + const link1 = screen.getByRole('link', { name: 'Basic' }); + await fireEvent.click(link1); + + const customMappingButton = screen.getByRole('button', { name: 'Add custom port mapping' }); + await fireEvent.click(customMappingButton); + + const hostInput = screen.getByLabelText('host port'); + await userEvent.click(hostInput); + await userEvent.clear(hostInput); + // adds a negative port + await userEvent.keyboard('71000'); + + const containerInput = screen.getByLabelText('container port'); + await userEvent.click(containerInput); + await userEvent.clear(containerInput); + await userEvent.keyboard('80'); + + const button = screen.getByRole('button', { name: 'Start Container' }); + expect((button as HTMLButtonElement).disabled).toBeTruthy(); + }); + + test('Expect "start container" button to be disabled if user adds a port which is invalid (isNaN)', async () => { + router.goto('/basic'); + + await createRunImage(undefined, ['command1', 'command2']); + + const link1 = screen.getByRole('link', { name: 'Basic' }); + await fireEvent.click(link1); + + const customMappingButton = screen.getByRole('button', { name: 'Add custom port mapping' }); + await fireEvent.click(customMappingButton); + + const hostInput = screen.getByLabelText('host port'); + await userEvent.click(hostInput); + await userEvent.clear(hostInput); + // adds a negative port + await userEvent.keyboard('test'); + + const containerInput = screen.getByLabelText('container port'); + await userEvent.click(containerInput); + await userEvent.clear(containerInput); + await userEvent.keyboard('80'); + + const button = screen.getByRole('button', { name: 'Start Container' }); + expect((button as HTMLButtonElement).disabled).toBeTruthy(); + }); + + test('Expect "start container" button to be disabled if user adds a port which is NOT free', async () => { + (window.isFreePort as Mock).mockResolvedValue(false); + router.goto('/basic'); + + await createRunImage(undefined, ['command1', 'command2']); + + const link1 = screen.getByRole('link', { name: 'Basic' }); + await fireEvent.click(link1); + + const customMappingButton = screen.getByRole('button', { name: 'Add custom port mapping' }); + await fireEvent.click(customMappingButton); + + const hostInput = screen.getByLabelText('host port'); + await userEvent.click(hostInput); + await userEvent.clear(hostInput); + // adds a negative port + await userEvent.keyboard('8080'); + + const containerInput = screen.getByLabelText('container port'); + await userEvent.click(containerInput); + await userEvent.clear(containerInput); + await userEvent.keyboard('80'); + + // wait onPortInputTimeout (500ms) triggers + await new Promise(resolve => setTimeout(resolve, 600)); + + const button = screen.getByRole('button', { name: 'Start Container' }); + expect((button as HTMLButtonElement).disabled).toBeTruthy(); + }); + + test('Expect "start container" button to be enabled if user adds a port which is valid and free', async () => { + (window.isFreePort as Mock).mockResolvedValue(true); + router.goto('/basic'); + + await createRunImage(undefined, ['command1', 'command2']); + + const link1 = screen.getByRole('link', { name: 'Basic' }); + await fireEvent.click(link1); + + const customMappingButton = screen.getByRole('button', { name: 'Add custom port mapping' }); + await fireEvent.click(customMappingButton); + + const hostInput = screen.getByLabelText('host port'); + await userEvent.click(hostInput); + await userEvent.clear(hostInput); + // adds a negative port + await userEvent.keyboard('8080'); + + const containerInput = screen.getByLabelText('container port'); + await userEvent.click(containerInput); + await userEvent.clear(containerInput); + await userEvent.keyboard('80'); + + // wait onPortInputTimeout (500ms) triggers + await new Promise(resolve => setTimeout(resolve, 600)); + + const button = screen.getByRole('button', { name: 'Start Container' }); + expect((button as HTMLButtonElement).disabled).toBeFalsy(); + }); }); diff --git a/packages/renderer/src/lib/image/RunImage.svelte b/packages/renderer/src/lib/image/RunImage.svelte index facc67d7baa91..1a7884d4559a0 100644 --- a/packages/renderer/src/lib/image/RunImage.svelte +++ b/packages/renderer/src/lib/image/RunImage.svelte @@ -19,6 +19,11 @@ import { array2String } from '/@/lib/string/string.js'; import Tab from '../ui/Tab.svelte'; import Button from '../ui/Button.svelte'; +interface PortInfo { + port: string; + error: string; +} + let image: ImageInfoUI; let imageInspectInfo: ImageInspectInfo; @@ -30,19 +35,22 @@ let command = ''; let entrypoint = ''; -let invalidFields = false; - -let containerPortMapping: string[]; +let containerPortMapping: PortInfo[]; let exposedPorts: string[] = []; let createError: string | undefined = undefined; let restartPolicyName = ''; let restartPolicyMaxRetryCount = 1; +let onPortInputTimeout: NodeJS.Timeout; // initialize with empty array let environmentVariables: { key: string; value: string }[] = [{ key: '', value: '' }]; let environmentFiles: string[] = ['']; let volumeMounts: { source: string; target: string }[] = [{ source: '', target: '' }]; -let hostContainerPortMappings: { hostPort: string; containerPort: string }[] = []; +let hostContainerPortMappings: { hostPort: PortInfo; containerPort: string }[] = []; + +let invalidName = false; +let invalidPorts = false; +$: invalidFields = invalidName || invalidPorts; // auto remove the container on exit let autoRemove = false; @@ -119,12 +127,12 @@ onMount(async () => { } // auto-assign ports from available free port - containerPortMapping = new Array(exposedPorts.length); + containerPortMapping = new Array(exposedPorts.length); await Promise.all( exposedPorts.map(async (port, index) => { const localPorts = await getPortsInfo(port); if (localPorts) { - containerPortMapping[index] = localPorts; + containerPortMapping[index] = { port: localPorts, error: '' }; } }), ); @@ -203,17 +211,23 @@ async function selectEnvironmentFile(index: number) { * undefined if the portDescriptor range is not valid * e.g 5000:5001 -> 9000:9001 */ -function getPortRange(portDescriptor: string): Promise { +async function getPortRange(portDescriptor: string): Promise { const rangeValues = getStartEndRange(portDescriptor); if (!rangeValues) { return Promise.resolve(undefined); } const rangeSize = rangeValues.endRange + 1 - rangeValues.startRange; - return window.getFreePortRange(rangeSize); + try { + // if free port range fails, return undefined + return await window.getFreePortRange(rangeSize); + } catch (e) { + console.error(e); + return undefined; + } } -function getPort(portDescriptor: string): Promise { +async function getPort(portDescriptor: string): Promise { let port: number; if (portDescriptor.endsWith('/tcp') || portDescriptor.endsWith('/udp')) { port = parseInt(portDescriptor.substring(0, portDescriptor.length - 4)); @@ -224,7 +238,13 @@ function getPort(portDescriptor: string): Promise { if (isNaN(port)) { return Promise.resolve(undefined); } - return window.getFreePort(port); + try { + // if getFreePort fails, it returns undefined + return await window.getFreePort(port); + } catch (e) { + console.error(e); + return undefined; + } } async function startContainer() { @@ -235,23 +255,23 @@ async function startContainer() { const PortBindings: any = {}; try { exposedPorts.forEach((port, index) => { - if (port.includes('-') || containerPortMapping[index]?.includes('-')) { - addPortsFromRange(ExposedPorts, PortBindings, port, containerPortMapping[index]); + if (port.includes('-') || containerPortMapping[index]?.port.includes('-')) { + addPortsFromRange(ExposedPorts, PortBindings, port, containerPortMapping[index].port); } else { - if (containerPortMapping[index]) { - PortBindings[port] = [{ HostPort: containerPortMapping[index] }]; + if (containerPortMapping[index]?.port) { + PortBindings[port] = [{ HostPort: containerPortMapping[index].port }]; } ExposedPorts[port] = {}; } }); hostContainerPortMappings - .filter(pair => pair.hostPort && pair.containerPort) + .filter(pair => pair.hostPort.port && pair.containerPort) .forEach(pair => { - if (pair.containerPort.includes('-') || pair.hostPort.includes('-')) { - addPortsFromRange(ExposedPorts, PortBindings, pair.containerPort, pair.hostPort); + if (pair.containerPort.includes('-') || pair.hostPort.port.includes('-')) { + addPortsFromRange(ExposedPorts, PortBindings, pair.containerPort, pair.hostPort.port); } else { - PortBindings[pair.containerPort] = [{ HostPort: pair.hostPort }]; + PortBindings[pair.containerPort] = [{ HostPort: pair.hostPort.port }]; ExposedPorts[pair.containerPort] = {}; } }); @@ -473,7 +493,16 @@ function handleCleanValueEnvFile( } function addHostContainerPorts() { - hostContainerPortMappings = [...hostContainerPortMappings, { hostPort: '', containerPort: '' }]; + hostContainerPortMappings = [ + ...hostContainerPortMappings, + { + hostPort: { + port: '', + error: '', + }, + containerPort: '', + }, + ]; } function deleteHostContainerPorts(index: number) { @@ -548,12 +577,65 @@ function checkContainerName(event: any) { ); if (containerAlreadyExists) { containerNameError = `The name ${containerValue} already exists. Please choose another name or leave blank to generate a name.`; - invalidFields = true; + invalidName = true; } else { containerNameError = ''; - invalidFields = false; + invalidName = false; } } + +function onContainerPortMappingInput(event: Event, index: number) { + onPortInput(event, containerPortMapping[index], () => { + containerPortMapping = containerPortMapping; + assertAllPortAreValid(); + }); +} + +function onHostContainerPortMappingInput(event: Event, index: number) { + onPortInput(event, hostContainerPortMappings[index].hostPort, () => { + hostContainerPortMappings = hostContainerPortMappings; + assertAllPortAreValid(); + }); +} + +function onPortInput(event: Event, portInfo: PortInfo, updateUI: () => void) { + // clear the timeout so if there was an old call to areAllPortsFree pending is deleted. We will create a new one soon + clearTimeout(onPortInputTimeout); + const target = event.currentTarget as HTMLInputElement; + // convert string to number + const _value: number = Number(target.value); + // if number is not valid (NaN or outside the value range), set the error + if (isNaN(_value) || _value < 0 || _value > 65535) { + portInfo.error = 'port should be >= 0 and < 65536'; + updateUI(); + return; + } + onPortInputTimeout = setTimeout(() => { + isPortFree(_value).then(isFree => { + portInfo.error = isFree; + updateUI(); + }); + }, 500); +} + +function isPortFree(port: number): Promise { + return window + .isFreePort(port) + .then(isFree => { + if (!isFree) { + return `Port ${port} is already in use`; + } else { + return ''; + } + }) + .catch((_: unknown) => `Port ${port} is already in use`); +} + +async function assertAllPortAreValid(): Promise { + const invalidHostPorts = hostContainerPortMappings.filter(pair => pair.hostPort.error); + const invalidContainerPortMapping = containerPortMapping?.filter(port => port.error) || []; + invalidPorts = invalidHostPorts.length > 0 || invalidContainerPortMapping.length > 0; +} @@ -654,9 +736,12 @@ function checkContainerName(event: any) { >Local port for {port}: + class="ml-2 w-full p-2 outline-none text-sm bg-charcoal-800 rounded-sm text-gray-700 placeholder-gray-700 border-b" + class:border-red-500="{containerPortMapping[index].error.length > 0}" + title="{containerPortMapping[index].error}" /> {/each} @@ -668,10 +753,13 @@ function checkContainerName(event: any) {
+ class="w-full p-2 outline-none text-sm bg-charcoal-800 rounded-sm text-gray-700 placeholder-gray-700 border-b" + class:border-red-500="{hostContainerPortMapping.hostPort.error.length > 0}" + title="{hostContainerPortMapping.hostPort.error}" />