Skip to content

Commit

Permalink
fix: disable create/start container if any port is busy (podman-deskt…
Browse files Browse the repository at this point in the history
…op#4637)

* fix: disable create/start container if any port is busy

Signed-off-by: lstocchi <[email protected]>

* fix: add tests

Signed-off-by: lstocchi <[email protected]>

---------

Signed-off-by: lstocchi <[email protected]>
  • Loading branch information
lstocchi authored Nov 14, 2023
1 parent 57006e5 commit 670d3ef
Show file tree
Hide file tree
Showing 4 changed files with 271 additions and 28 deletions.
6 changes: 5 additions & 1 deletion packages/main/src/plugin/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -1326,6 +1326,10 @@ export class PluginSystem {
return getFreePortRange(rangeSize);
});

this.ipcHandle('system:is-port-free', async (_, port: number): Promise<boolean> => {
return isFreePort(port);
});

this.ipcHandle(
'provider-registry:startReceiveLogs',
async (
Expand Down
4 changes: 4 additions & 0 deletions packages/preload/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1228,6 +1228,10 @@ function initExposure(): void {
return ipcInvoke('system:get-free-port-range', rangeSize);
});

contextBridge.exposeInMainWorld('isFreePort', async (port: number): Promise<boolean> => {
return ipcInvoke('system:is-port-free', port);
});

type LogFunction = (...data: unknown[]) => void;

let onDataCallbacksStartReceiveLogsId = 0;
Expand Down
151 changes: 149 additions & 2 deletions packages/renderer/src/lib/image/RunImage.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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) = {
Expand All @@ -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;
Expand Down Expand Up @@ -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();
});
});
Loading

0 comments on commit 670d3ef

Please sign in to comment.