Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions src/app/constants/mac-address.constant.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { marker as T } from '@biesbjerg/ngx-translate-extract-marker';

/**
* Middleware validates custom MAC addresses on container and VM NIC devices as
* colon-separated only. Dash-separated (`10-66-6A-1F-F1-B1`), unseparated,
* mixed-separator and Cisco dotted (`1066.6a1f.f1b1`) forms are rejected: libvirt only
* ever parsed the colon form, so the permissive values used to save and then fail at start.
*/
export const macAddressRegex = /^([0-9A-F]{2}:){5}[0-9A-F]{2}$/i;

/**
* Shared by every field validated with {@link macAddressRegex}, so that a form which used to
* accept one of the rejected forms says what changed instead of falling back to the generic
* "Invalid format or character".
*/
export const macAddressInvalidMessage = T('MAC address must be colon-separated, for example 00:a0:98:1b:2c:3d');
2 changes: 2 additions & 0 deletions src/app/enums/container.enum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ export const containerTypeLabels = new Map<ContainerType, string>([
export enum ContainerStatus {
Running = 'RUNNING',
Stopped = 'STOPPED',
Suspended = 'SUSPENDED',
Unknown = 'UNKNOWN',
}

export const containerStatusLabels = new Map<ContainerStatus, string>([
[ContainerStatus.Running, T('Running')],
[ContainerStatus.Stopped, T('Stopped')],
[ContainerStatus.Suspended, T('Suspended')],
[ContainerStatus.Unknown, T('Unknown')],
]);

Expand Down
19 changes: 19 additions & 0 deletions src/app/helptext/containers/containers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,25 @@ export const containersHelptext = {
and to tweak behavior accordingly.'),
},

nameTooltip: T('Container name'),
renameRequiresStoppedTooltip: T('Container name. A container can only be renamed while it is stopped.'),

deviceRequiresStoppedTooltip: T('Cannot modify devices unless the container is stopped. Please stop the container first.'),

macTooltip: T('The address must be colon-separated, for example 00:a0:98:1b:2c:3d.'),
macEditTooltip: T('Leave empty to use the default MAC address. The address must be colon-separated, for example 00:a0:98:1b:2c:3d.'),

deleteDialog: {
forceLabel: T('Stop container before deleting'),
forceTooltip: T('A container that is running or suspended cannot be deleted. Check this to stop it first.'),
forceRequiredTooltip: T('This container is not stopped, so deleting it always stops it first.'),
forceRequiredWarning: T('This container is not stopped. Deleting it requires stopping it first.'),
recursiveLabel: T('Delete child datasets, snapshots and clones'),
recursiveTooltip: T('The container dataset is destroyed together with its child datasets and snapshots, any clones of those snapshots wherever they live in the pool, and any holds on them. Required when the container dataset has children or snapshots.'),
recursiveWarning: T('Nothing destroyed this way can be recovered. Releasing a snapshot hold can also break a replication task that depends on it.'),
confirmLabel: T('Confirm'),
},

validators: {
containerPathMustStartWithSlash: T('Container path must start with /'),
containerPathCannotEndWithSlash: T('Container path cannot end with /'),
Expand Down
3 changes: 2 additions & 1 deletion src/app/helptext/vm/devices/device-add-edit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ export const helptextDevice = {
mac_tooltip: T('By default, the VM receives an auto-generated random\
MAC address. Enter a custom address into the field to\
override the default. Click <b>Generate MAC Address</b>\
to add a new randomized address into this field.'),
to add a new randomized address into this field.\
The address must be colon-separated, for example 00:a0:98:1b:2c:3d.'),

nic_attach_tooltip: T('Select a physical interface to associate with the VM.'),

Expand Down
3 changes: 2 additions & 1 deletion src/app/helptext/vm/vm-wizard/vm-wizard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ export const helptextVmWizard = {
paravirtualized network drivers.'),

NIC_mac_tooltip: T('Enter the desired address into the field to\
override the randomized MAC address.'),
override the randomized MAC address.\
The address must be colon-separated, for example 00:a0:98:1b:2c:3d.'),
NIC_mac_value: '00:a0:98:FF:FF:FF',

nic_attach_tooltip: T('Select the physical interface to associate with\
Expand Down
1 change: 0 additions & 1 deletion src/app/interfaces/api/api-call-directory.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,6 @@ export interface ApiCallDirectory {
'container.device.usb_choices': { params: []; response: Record<string, AvailableUsb> };

// Container (actual available endpoints only)
'container.delete': { params: [containerId: number]; response: boolean };
'container.get_instance': { params: [containerId: number]; response: Container };
'container.image.query_registry': { params: []; response: ContainerImageRegistryResponse[] };
'container.pool_choices': { params: []; response: Choices };
Expand Down
2 changes: 2 additions & 0 deletions src/app/interfaces/api/api-job-directory.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { ConfigResetParams } from 'app/interfaces/config-reset-params.interface'
import { PullContainerImageParams, PullContainerImageResponse } from 'app/interfaces/container-image.interface';
import {
Container,
ContainerDeleteParams,
CreateContainer,
} from 'app/interfaces/container.interface';
import { CoreBulkQuery, CoreBulkResponse } from 'app/interfaces/core-bulk.interface';
Expand Down Expand Up @@ -196,6 +197,7 @@ export interface ApiJobDirectory {

// Container
'container.create': { params: [CreateContainer]; response: Container };
'container.delete': { params: ContainerDeleteParams; response: boolean };
'container.migrate': { params: [containerId: number]; response: boolean };
'container.stop': { params: [containerId: number, params?: { force?: boolean; force_after_timeout?: boolean }]; response: void };

Expand Down
17 changes: 17 additions & 0 deletions src/app/interfaces/container.interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,23 @@ export interface ContainerStopParams {
force_after_timeout?: boolean;
}

export interface ContainerDeleteOptions {
/**
* Stops the container first when it is not already stopped.
* Without it, deleting a running or suspended container is refused up front.
*/
force?: boolean;

/**
* Destroys the container dataset together with its child datasets, snapshots,
* clones of those snapshots and any holds on them. Not recoverable.
* Without it, deleting a container whose dataset has children or snapshots is refused.
*/
recursive?: boolean;
}

export type ContainerDeleteParams = [containerId: number, options?: ContainerDeleteOptions];

export interface ContainerGlobalConfig {
bridge: string | null;
v4_network: string | null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
<ix-device-actions-menu
[device]="disk"
[showEdit]="true"
[isDisabled]="isContainerRunning()"
[disabledTooltip]="isContainerRunning() ? ('Cannot modify devices while container is running. Please stop the container first.' | translate) : null"
[isDisabled]="isContainerActive()"
[disabledTooltip]="isContainerActive() ? (helptext.deviceRequiresStoppedTooltip | translate) : null"
></ix-device-actions-menu>
</div>
} @empty {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,45 @@ describe('ContainerFilesystemDevicesComponent', () => {
expect(actionsMenu[0].device).toBe(disks[0]);
});

// Middleware refuses device operations on any container that is not stopped, so Add is
// gated like the per-device Edit/Delete menu instead of failing at submit.
describe.each([ContainerStatus.Running, ContainerStatus.Suspended, ContainerStatus.Unknown])(
'when the container is %s',
(state) => {
const createActiveComponent = createComponentFactory({
component: ContainerFilesystemDevicesComponent,
imports: [
MockComponent(DeviceActionsMenuComponent),
],
providers: [
mockAuth(),
mockApi([]),
mockProvider(SnackbarService),
mockProvider(FilesystemService),
mockProvider(FormSidePanelService),
mockProvider(ContainersStore, {
selectedContainer: () => fakeContainer({
id: 1,
status: { state, pid: 0, domain_state: null },
}),
}),
mockProvider(ContainerDevicesStore, {
isLoading: () => false,
devices: () => disks,
}),
],
});

it('disables Add', async () => {
const activeSpectator = createActiveComponent({ props: { container: fakeContainer({ id: 1 }) } });

const addButton = await TestbedHarnessEnvironment.loader(activeSpectator.fixture)
.getHarness(TnButtonHarness.with({ label: 'Add' }));
expect(await addButton.isDisabled()).toBe(true);
});
},
);

describe('side panel', () => {
it('opens the form side panel to add a disk', async () => {
const addButton = await loader.getHarness(TnButtonHarness.with({ label: 'Add' }));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
import { TranslateModule, TranslateService } from '@ngx-translate/core';
import { TnCardAction, TnCardComponent } from '@truenas/ui-components';
import { NgxSkeletonLoaderModule } from 'ngx-skeleton-loader';
import { ContainerDeviceType, ContainerStatus } from 'app/enums/container.enum';
import { ContainerDeviceType } from 'app/enums/container.enum';
import { Role } from 'app/enums/role.enum';
import { containersHelptext } from 'app/helptext/containers/containers';
import { Container, ContainerDevice, ContainerFilesystemDevice } from 'app/interfaces/container.interface';
import { AuthService } from 'app/modules/auth/auth.service';
import { FormSidePanelService } from 'app/modules/slide-ins/form-side-panel/form-side-panel.service';
Expand All @@ -15,6 +16,7 @@
import { getDeviceDescription } from 'app/pages/containers/components/common/utils/get-device-description.utils';
import { ContainerDevicesStore } from 'app/pages/containers/stores/container-devices.store';
import { ContainersStore } from 'app/pages/containers/stores/containers.store';
import { isContainerActive } from 'app/pages/containers/utils/container-status.utils';

@Component({
selector: 'ix-container-filesystem-devices',
Expand Down Expand Up @@ -52,14 +54,20 @@
return {
label: this.translate.instant('Add'),
testId: 'add-disk',
// Gated like the per-device Edit/Delete menu: middleware refuses the create just the
// same, and failing at submit after the form is filled in is worse than not offering it.
disabled: this.isContainerActive(),

Check notice on line 59 in src/app/pages/containers/components/all-containers/container-details/container-filesystem-devices/container-filesystem-devices.component.ts

View workflow job for this annotation

GitHub Actions / review / Automatic PR review

LOW: The Filesystem Devices Add card action is disabled on an active container with no explanatory tooltip, unlike the three sibling add-*-menu buttons and the per-device actions menu.
Comment thread
AlexKarpov98 marked this conversation as resolved.
handler: () => this.addDisk(),
};
});

protected readonly isLoadingDevices = this.devicesStore.isLoading;
protected readonly isContainerRunning = computed(() => {
const container = this.containersStore.selectedContainer();
return container?.status.state === ContainerStatus.Running;
protected readonly helptext = containersHelptext;

// Middleware refuses device operations on any container that is not stopped, which since
// 26.0 includes SUSPENDED - not just RUNNING.
protected readonly isContainerActive = computed(() => {
Comment thread
AlexKarpov98 marked this conversation as resolved.
return isContainerActive(this.containersStore.selectedContainer());
});

protected readonly visibleDisks = computed(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,29 @@ import { TestbedHarnessEnvironment } from '@angular/cdk/testing/testbed';
import { KeyValuePipe } from '@angular/common';
import { Router } from '@angular/router';
import { createComponentFactory, mockProvider, Spectator } from '@ngneat/spectator/jest';
import { TnButtonHarness, TnCardComponent } from '@truenas/ui-components';
import { EMPTY, of } from 'rxjs';
import { mockCall, mockApi } from 'app/core/testing/utils/mock-api.utils';
import { TnButtonHarness, TnCardComponent, TnDialog } from '@truenas/ui-components';
import { of } from 'rxjs';
import { mockJob, mockApi } from 'app/core/testing/utils/mock-api.utils';
import { mockAuth } from 'app/core/testing/utils/mock-auth.utils';
import { ContainerCapabilitiesPolicy, ContainerIdmapType, ContainerStatus } from 'app/enums/container.enum';
import { ConfirmDeleteCallOptions } from 'app/interfaces/dialog.interface';
import { DialogService } from 'app/modules/dialog/dialog.service';
import { IxFormatterService } from 'app/modules/forms/ix-forms/services/ix-formatter.service';
import { MapValuePipe } from 'app/modules/pipes/map-value/map-value.pipe';
import { YesNoPipe } from 'app/modules/pipes/yes-no/yes-no.pipe';
import { FormSidePanelService } from 'app/modules/slide-ins/form-side-panel/form-side-panel.service';
import { SlideInResult } from 'app/modules/slide-ins/slide-in-result';
import { SnackbarService } from 'app/modules/snackbar/services/snackbar.service';
import { ApiService } from 'app/modules/websocket/api.service';
import {
ContainerGeneralInfoComponent,
} from 'app/pages/containers/components/all-containers/container-details/container-general-info/container-general-info.component';
import {
DeleteContainerDialog,
} from 'app/pages/containers/components/common/delete-container-dialog/delete-container-dialog.component';
import { ContainerFormComponent } from 'app/pages/containers/components/container-form/container-form.component';
import { ContainersStore } from 'app/pages/containers/stores/containers.store';
import { fakeContainer } from 'app/pages/containers/utils/fake-container.utils';
import { ErrorHandlerService } from 'app/services/errors/error-handler.service';

const container = fakeContainer({
id: 1,
Expand Down Expand Up @@ -54,11 +58,20 @@ describe('ContainerGeneralInfoComponent', () => {
reload: jest.fn(),
}),
mockApi([
mockCall('container.delete'),
mockJob('container.delete'),
]),
mockProvider(TnDialog, {
open: jest.fn(() => ({
closed: of({ force: false, recursive: false }),
})),
}),
mockProvider(DialogService, {
confirm: jest.fn(() => of(true)),
confirmDelete: jest.fn((options: ConfirmDeleteCallOptions) => options.call()),
jobDialog: jest.fn(() => ({ afterClosed: () => of({}) })),
}),
mockProvider(SnackbarService),
mockProvider(ErrorHandlerService, {
withErrorHandler: jest.fn(() => (source$: unknown) => source$),
}),
mockProvider(Router),
],
Expand Down Expand Up @@ -91,19 +104,36 @@ describe('ContainerGeneralInfoComponent', () => {
expect(cardContent).toContainText('CPU Set: All Host CPUs');
});

it('deletes container when "Delete" button is pressed and redirects to list root', async () => {
it('deletes container as a job with the options from the dialog and redirects to list root', async () => {
const deleteButton = await loader.getHarness(TnButtonHarness.with({ label: 'Delete' }));
await deleteButton.click();

expect(spectator.inject(DialogService).confirmDelete).toHaveBeenCalledWith({
message: 'Delete Demo?',
call: expect.any(Function),
});
expect(spectator.inject(TnDialog).open).toHaveBeenCalledWith(
DeleteContainerDialog,
expect.objectContaining({ data: container }),
);

expect(spectator.inject(ApiService).call).toHaveBeenCalledWith('container.delete', [1]);
expect(spectator.inject(ApiService).job).toHaveBeenCalledWith(
'container.delete',
[1, { force: false, recursive: false }],
);
expect(spectator.inject(SnackbarService).success).toHaveBeenCalledWith('Container deleted');
expect(spectator.inject(Router).navigate).toHaveBeenCalledWith(['/containers']);
});

it('passes force and recursive on to the delete job when the dialog asks for them', async () => {
const tnDialog = spectator.inject(TnDialog);
(tnDialog.open as jest.Mock).mockReturnValue({ closed: of({ force: true, recursive: true }) });

const deleteButton = await loader.getHarness(TnButtonHarness.with({ label: 'Delete' }));
await deleteButton.click();

expect(spectator.inject(ApiService).job).toHaveBeenCalledWith(
'container.delete',
[1, { force: true, recursive: true }],
);
});

it('opens edit container form in a side panel when Edit is pressed', async () => {
const editButton = await loader.getHarness(TnButtonHarness.with({ label: 'Edit' }));
await editButton.click();
Expand All @@ -115,14 +145,14 @@ describe('ContainerGeneralInfoComponent', () => {
expect(spectator.inject(ContainersStore).reload).toHaveBeenCalled();
});

it('does not delete container when confirmation is cancelled', async () => {
const dialogService = spectator.inject(DialogService);
(dialogService.confirmDelete as jest.Mock).mockReturnValue(EMPTY);
it('does not delete container when the delete dialog is cancelled', async () => {
const tnDialog = spectator.inject(TnDialog);
(tnDialog.open as jest.Mock).mockReturnValue({ closed: of(false) });

const deleteButton = await loader.getHarness(TnButtonHarness.with({ label: 'Delete' }));
await deleteButton.click();

expect(spectator.inject(ApiService).call).not.toHaveBeenCalledWith('container.delete', expect.anything());
expect(spectator.inject(ApiService).job).not.toHaveBeenCalled();
expect(spectator.inject(Router).navigate).not.toHaveBeenCalled();
});

Expand Down
Loading
Loading