From 34f73590bbb72c01a3e16e6085227849556a2b1d Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Fri, 2 Oct 2026 22:34:17 +0100 Subject: [PATCH] Keep pool disk facts on the owning storage host Repeated device paths and UnRAID group names must not attach foreign SMART, temperatures or topology counts to a pool. Preserve canonical children, require host or scoped native ownership for inferred membership, and compare exact device identifiers. Cover live target changes and real desktop/phone presentations. Change-source: pulse-maintainer --- .../subsystems/frontend-primitives.md | 17 ++ .../internal/subsystems/storage-recovery.md | 23 +++ .../browser-tests/pool-ownership.cjs | 177 ++++++++++++++++++ .../browser-tests/pool-ownership.html | 12 ++ .../browser-tests/pool-ownership.tsx | 123 ++++++++++++ frontend-modern/browser-verification.json | 52 ++--- .../StoragePoolDetail.ownership.test.tsx | 87 +++++++++ .../__tests__/StoragePoolDetail.test.tsx | 1 + .../useStoragePoolDetailModel.test.ts | 2 + ...gePoolDetailPresentation.branchcov.test.ts | 8 +- .../storagePoolDetailPresentation.test.ts | 5 + .../storagePoolDiskOwnership.test.ts | 170 +++++++++++++++++ .../storageBackups/storageAdapters.ts | 4 + .../storagePoolDetailPresentation.ts | 31 ++- 14 files changed, 674 insertions(+), 38 deletions(-) create mode 100644 frontend-modern/browser-tests/pool-ownership.cjs create mode 100644 frontend-modern/browser-tests/pool-ownership.html create mode 100644 frontend-modern/browser-tests/pool-ownership.tsx create mode 100644 frontend-modern/src/components/Storage/__tests__/StoragePoolDetail.ownership.test.tsx create mode 100644 frontend-modern/src/features/storageBackups/__tests__/storagePoolDiskOwnership.test.ts diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index e0d9578876..6a0e57516a 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -7752,3 +7752,20 @@ production disk/pool details and API transport in phone Chromium/WebKit, native vertical panning in Chromium, and desktop mouse/keyboard controls. Synthetic sample presentation is not native collector, physical device, assistive-device or installed whole-report acceptance. + +### Shared Storage presenters preserve disk ownership + +Pool-detail presenters under `features/storageBackups/` must not use repeated +device paths, UnRAID group labels or host display names as fleet-wide identity. +Direct pool children are retained; inferred membership requires the same +canonical host parent, or a complete native Proxmox instance/node pair when +parents are missing. Known parent conflicts win over native names. Full ZFS +device identifiers may normalize an optional `/dev/` prefix but must not match +arbitrary suffixes. Missing ownership produces no invented linked-disk facts. +The storage adapter preserves the instance needed for that presentation scope; +this does not rewrite canonical resources or History targets. Existing keyed +pool details must update linked disks and local health through target/snapshot +replacement without retaining peer-host rows. The mounted +`StoragePoolDetail.ownership.test.tsx` and production-component browser fixture +`browser-tests/pool-ownership.cjs` verify desktop/phone presentation and History +coexistence; they are not field collection or native recovery evidence. diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 20667521cd..97ffabafe7 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -6311,3 +6311,26 @@ units, collection, retention, access or recovery boundary. The production disk/pool fixture `frontend-modern/browser-tests/history-touch.cjs` and mounted `HistoryChart.test.tsx` regressions verify presentation/input behaviour only, not live appliance collection or recovery success. + +### Pool-to-physical-disk ownership in Storage details + +`storagePoolDetailPresentation.ts` must scope inferred ZFS device and UnRAID +array/cache-group membership to the owning host. An explicit disk parent equal +to the canonical pool ID or its retained resource reference remains a direct +association. Otherwise, matching canonical host parents establishes scope; +conflicting parents cannot be overridden by names, paths or group labels. +When canonical parents are unavailable, only a complete matching native +Proxmox instance/node pair establishes scope. `storageAdapters.ts` retains that +instance alongside the native node. Display names and unscoped hostnames alone +are insufficient. Unknown ownership must not present another host's SMART, +temperature, counters or topology counts as this pool's evidence. + +ZFS device matching compares full identifiers with only the optional `/dev/` +prefix normalized, not arbitrary suffixes or invented partition associations. +This is read-only presentation: no collector, canonical server identity, +metrics target, health threshold, persistence or recovery authority changes. +`storagePoolDiskOwnership.test.ts`, the mounted +`StoragePoolDetail.ownership.test.tsx` and `browser-tests/pool-ownership.cjs` +cover repeated paths/groups, direct children, scoped legacy snapshots, missing +ownership and live target replacement. Synthetic presentation proof does not +establish native appliance, installation or recovery acceptance. diff --git a/frontend-modern/browser-tests/pool-ownership.cjs b/frontend-modern/browser-tests/pool-ownership.cjs new file mode 100644 index 0000000000..7e9a879912 --- /dev/null +++ b/frontend-modern/browser-tests/pool-ownership.cjs @@ -0,0 +1,177 @@ +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); +const { chromium, webkit } = require('playwright'); +const parent = false; +(async () => { + const root = '/workspace/frontend-modern'; + process.chdir(root); + const artifacts = path.join(root, 'node_modules/pool-ownership-proof'); + fs.mkdirSync(artifacts, { recursive: true }); + const { createServer } = await import(path.join(root, 'node_modules/vite/dist/node/index.js')); + const server = await createServer({ + root, + configFile: path.join(root, 'vite.config.ts'), + cacheDir: fs.mkdtempSync(path.join(artifacts, 'vite-')), + server: { host: '127.0.0.1', port: 5234, strictPort: true }, + }); + const playwright = require('playwright/package.json').version; + const expected = JSON.parse( + fs.readFileSync('/workspace/tests/integration/package-lock.json', 'utf8'), + ).packages['node_modules/@playwright/test'].version; + assert.equal(playwright, expected); + const observations = []; + let browser; + try { + await server.listen(); + for (const [name, engine, phone] of [ + ['chromium-desktop', chromium, false], + ['webkit-phone', webkit, true], + ]) { + browser = await engine.launch( + engine === chromium + ? { headless: true, channel: 'chromium', args: ['--no-sandbox'] } + : { headless: true }, + ); + const page = await browser.newPage({ + viewport: phone ? { width: 390, height: 844 } : { width: 1365, height: 900 }, + hasTouch: phone, + isMobile: phone, + }); + const requests = [], + errors = []; + page.on('pageerror', (error) => errors.push(error.message)); + await page.route('**/*', (route) => { + const url = new URL(route.request().url()); + if (url.origin !== 'http://127.0.0.1:5234') return route.abort(); + if (!url.pathname.startsWith('/api/')) return route.continue(); + if (url.pathname === '/api/metrics-store/history') { + requests.push(Object.fromEntries(url.searchParams)); + return route.fulfill({ + json: { + points: [{ timestamp: 1790935200000, value: 25, min: 25, max: 25 }], + source: 'store', + }, + }); + } + return route.fulfill({ + json: + url.pathname === '/api/license/runtime-capabilities' + ? { + capabilities: [], + limits: [], + max_history_days: 7, + hosted_mode: false, + runtime: { build: 'community' }, + blocked_capabilities: [], + } + : { data: [], enabled: false }, + }); + }); + await page.goto('http://127.0.0.1:5234/browser-tests/pool-ownership.html'); + if (phone) await page.evaluate(() => document.documentElement.classList.add('dark')); + const detail = page.getByRole('region', { name: 'Pool detail' }); + const phases = parent + ? [ + ['ZFS host A', 4, null], + ['UnRAID host A', 3, null], + ] + : [ + ['ZFS host A', 1, 'Host A disk'], + ['ZFS host B', 1, 'Host B disk'], + ['Missing ownership', 0, null], + ['Direct child', 1, 'Host A disk'], + ['UnRAID host A', 1, 'Host A disk'], + ['UnRAID host B', 1, 'Host B disk'], + ]; + for (const [phase, count, model] of phases) { + await page.getByRole('button', { name: phase, exact: true }).click(); + const text = await detail.textContent(); + if (count) { + await detail.getByText(`Physical Disks (${count})`, { exact: true }).waitFor(); + if (model) { + assert.ok(text.includes(model)); + assert.ok(!text.includes(model === 'Host A disk' ? 'Host B disk' : 'Host A disk')); + assert.ok(!text.includes('Unowned disk') && !text.includes('Suffix collision disk')); + assert.equal(text.includes('99 errors'), model === 'Host B disk'); + assert.ok(text.includes(model === 'Host B disk' ? '82°C' : '34°C')); + } else + assert.ok( + text.includes('Host B disk') && text.includes('99 errors'), + 'parent must expose foreign health', + ); + } else assert.equal(await detail.getByText(/Physical Disks/).count(), 0); + const dimensions = await page.evaluate(() => ({ + scroll: document.documentElement.scrollWidth, + width: innerWidth, + })); + assert.ok(dimensions.scroll <= dimensions.width + 1, JSON.stringify(dimensions)); + const screenshot = `${parent ? 'parent-' : ''}${name}-${phase.toLowerCase().replaceAll(' ', '-')}.png`; + await page.screenshot({ path: path.join(artifacts, screenshot), fullPage: true }); + observations.push({ + name, + phase, + browser: browser.version(), + text, + dimensions, + screenshot, + }); + } + if (!parent) { + await page.getByRole('button', { name: 'ZFS host A', exact: true }).click(); + await detail.getByRole('tab', { name: 'History', exact: true }).click(); + await detail.getByRole('img', { name: 'Usage chart', exact: true }).waitFor(); + await page.waitForFunction(() => + [...document.querySelectorAll('canvas')].some((el) => + document + .getElementById(el.getAttribute('aria-describedby')) + ?.textContent.includes('1 data point'), + ), + ); + assert.ok( + requests.some( + (request) => + request.resourceId === 'history-a' && + request.metric === 'usage' && + request.resourceType === 'storage', + ), + ); + await page.getByRole('button', { name: 'ZFS host B', exact: true }).click(); + await page.waitForFunction(() => + [...document.querySelectorAll('canvas')].some((el) => + document + .getElementById(el.getAttribute('aria-describedby')) + ?.textContent.includes('1 data point'), + ), + ); + assert.ok( + requests.some( + (request) => + request.resourceId === 'history-b' && + request.metric === 'usage' && + request.resourceType === 'storage', + ), + ); + await detail.getByRole('tab', { name: 'Overview', exact: true }).click(); + await detail.getByText('Physical Disks (1)', { exact: true }).waitFor(); + assert.equal(await detail.getByText('Host A disk', { exact: true }).count(), 0); + assert.equal(await detail.getByText('Host B disk', { exact: true }).count(), 1); + } + assert.deepEqual(errors, []); + observations.push({ name, phase: 'history-requests', requests, errors }); + await browser.close(); + browser = null; + } + fs.writeFileSync( + path.join(artifacts, parent ? 'parent.json' : 'result.json'), + JSON.stringify({ playwright, expected, parent, observations }, null, 2), + ); + console.log(JSON.stringify({ result: 'passed', parent, observations: observations.length })); + } finally { + if (browser) await browser.close(); + await server.close(); + } +})().catch((error) => { + console.error(error); + process.exitCode = 1; +}); diff --git a/frontend-modern/browser-tests/pool-ownership.html b/frontend-modern/browser-tests/pool-ownership.html new file mode 100644 index 0000000000..a215a7dc44 --- /dev/null +++ b/frontend-modern/browser-tests/pool-ownership.html @@ -0,0 +1,12 @@ + + + + + + Pool ownership verification + + +
+ + + diff --git a/frontend-modern/browser-tests/pool-ownership.tsx b/frontend-modern/browser-tests/pool-ownership.tsx new file mode 100644 index 0000000000..9f26743e96 --- /dev/null +++ b/frontend-modern/browser-tests/pool-ownership.tsx @@ -0,0 +1,123 @@ +import { createMemo, createSignal, For } from 'solid-js'; +import { render } from 'solid-js/web'; +import { StoragePoolDetail } from '../src/components/Storage/StoragePoolDetail'; +import { buildStorageRecords } from '../src/features/storageBackups/storageAdapters'; +import type { State } from '../src/types/api'; +import type { Resource } from '../src/types/resource'; +import '../src/index.css'; + +const modes = [ + 'ZFS host A', + 'ZFS host B', + 'UnRAID host A', + 'UnRAID host B', + 'Missing ownership', + 'Direct child', +]; +const [mode, setMode] = createSignal(modes[0]); +const resource = createMemo(() => { + const unraid = mode().startsWith('UnRAID'); + const b = mode().endsWith('B'); + const unknown = mode() === 'Missing ownership'; + return { + id: b ? 'pool-b' : 'pool-a', + type: 'storage', + name: unraid ? 'Array' : 'tank', + displayName: 'tank', + platformType: unraid ? 'unraid' : 'truenas', + platformId: 'fixture', + sourceType: 'api', + parentId: unknown ? undefined : b ? 'host-b' : 'host-a', + parentName: 'Storage host', + status: 'online', + lastSeen: Date.now(), + metricsTarget: { resourceType: 'storage', resourceId: b ? 'history-b' : 'history-a' }, + disk: { current: 25, total: 1024, used: 256, free: 768 }, + storage: unraid + ? { type: 'unraid-array', platform: 'unraid' } + : { + type: 'zfs-pool', + isZfs: true, + zfsPool: { + name: 'tank', + state: 'ONLINE', + readErrors: 0, + writeErrors: 0, + checksumErrors: 0, + devices: [{ name: 'sda', type: 'disk', state: 'ONLINE' }], + }, + }, + } as Resource; +}); +const record = createMemo( + () => buildStorageRecords({ state: {} as State, resources: [resource()] })[0], +); +const disks = createMemo(() => + [ + ['a', 'host-a', 'Host A disk', '/dev/sda', 34, 0], + ['b', 'host-b', 'Host B disk', '/dev/sda', 82, 99], + ['unknown', undefined, 'Unowned disk', '/dev/sda', 70, 0], + ['suffix', 'host-a', 'Suffix collision disk', '/dev/not-sda', 66, 0], + ].map( + ([id, owner, model, devPath, temperature, errorCount]) => + ({ + id: `disk-${id}`, + type: 'physical_disk', + name: model, + displayName: model, + platformType: 'truenas', + platformId: 'fixture', + sourceType: 'api', + status: 'online', + lastSeen: Date.now(), + parentId: mode() === 'Direct child' && id === 'a' ? 'pool-a' : owner, + parentName: 'Storage host', + physicalDisk: { + devPath, + model, + temperature, + errorCount, + storageGroup: id === 'suffix' ? 'other-group' : 'unraid-array', + storageRole: 'data', + sizeBytes: 1024, + storageState: 'online', + }, + }) as Resource, + ), +); + +render( + () => ( +
+

Pool ownership verification

+

+ Repeated disk names and groups across synthetic hosts, using the production Storage detail. +

+ +
+

{mode()}

+ + + + +
+
+
+ ), + document.getElementById('root')!, +); diff --git a/frontend-modern/browser-verification.json b/frontend-modern/browser-verification.json index 395e434f7a..c4dc983bda 100644 --- a/frontend-modern/browser-verification.json +++ b/frontend-modern/browser-verification.json @@ -1,19 +1,17 @@ { "version": 1, - "base_sha": "e5d0843851a3837a90d71afaced3a009c0606bbf", - "verified_at": "2026-10-02T20:44:41.220410Z", + "base_sha": "689079622dfa8fb472349ad13c36ad2068f0c38e", + "verified_at": "2026-10-02T21:27:29.856838Z", "result": "passed", "changed_paths": [ - "frontend-modern/src/components/shared/HistoryChart.tsx", - "frontend-modern/src/components/shared/useHistoryChartState.ts" + "frontend-modern/src/features/storageBackups/storageAdapters.ts", + "frontend-modern/src/features/storageBackups/storagePoolDetailPresentation.ts" ], "content_sha256": { - "frontend-modern/src/components/shared/HistoryChart.tsx": "74fb7e6b466e1c2b2d2efb13e0a124d6c301896a7c9423432e903908cc9ee25b", - "frontend-modern/src/components/shared/useHistoryChartState.ts": "d6e8317019c8dc616bd9a4d3c6c71cb5329297ca0341e6ba28a6cc4c84db36b0" + "frontend-modern/src/features/storageBackups/storageAdapters.ts": "7933ab99a1cfdf35bcebf0f351ec2b6a8c486b1281a9a3daa36f828fa657c52e", + "frontend-modern/src/features/storageBackups/storagePoolDetailPresentation.ts": "dd97511242f9375a725f69b3f8f40b2aee19998bdcdd2e82ce996d27f9fc519b" }, - "routes": [ - "/browser-tests/history-touch.html" - ], + "routes": ["/browser-tests/pool-ownership.html"], "viewports": [ { "width": 1365, @@ -25,33 +23,19 @@ } ], "states": [ - "Production disk/pool details with stored multi-point, measured-zero singleton and empty history; 19 observed presentations across desktop Chromium and phone Chromium/WebKit", - "Actual touchscreen first/middle/last taps, shared I/O group focus transfer, keyboard navigation/exit and real mouse coexistence; phone WebKit dark-class presentation" + "Production StoragePoolDetail through the real storage adapter; desktop Chromium light and phone WebKit dark; ZFS/UnRAID host A and B, missing ownership and direct children", + "12 recorded presentation states, plus per-engine History target replacement and Overview return" ], "interactions": [ - "Native touchscreen taps select the actual reading at that position instead of the latest sample; touch compatibility focus/mouse movement/leave cannot overwrite it", - "Primary contact cancellation, non-tap gesture rejection and native Chromium vertical pan with room-to-scroll precondition; no prevented touch default, pointer capture or touch-action change", - "Desktop/phone keyboard Home/Right/End/Escape and focus exit, pointer announcements remain quiet, range replacement, zero/empty responses and no horizontal overflow", - "Predecessor native Chromium/WebKit first tap near 10% instead displays 30%; final first/middle/last taps display 10/20/30%. Inspected predecessor and final phone, zero/empty and desktop screenshots" + "Replace the selected host in the mounted detail: a remote 82\u00b0C/99-error disk appears only on its owning host; local temperature and errors remain visible", + "Missing ownership omits inferred Physical Disks without removing ZFS information; direct children remain", + "History range and canonical history-a/history-b requests stay isolated while selected hosts change; no horizontal page overflow" ], - "command": "pulse-worker-browser frontend-modern/browser-tests/history-touch.cjs", - "browser_versions": { + "observations": { "playwright": "1.56.1", - "chromium": "141.0.7390.37", - "webkit": "26.0" - }, - "artifacts": [ - "browser/result.json", - "browser/chromium-phone-first.png", - "browser/webkit-phone-first.png", - "browser/webkit-phone-middle.png", - "browser/webkit-phone-pool-first.png", - "browser/webkit-phone-pool-zero-singleton.png", - "browser/webkit-phone-pool-empty.png", - "browser/chromium-desktop-middle.png", - "browser/parent.json", - "browser/parent-chromium-phone-first.png", - "browser/parent-webkit-phone-first.png" - ], - "limitations": "Synthetic stored APIs, not native PBS/TrueNAS collection, physical phones or assistive-device speech. Native pan is Chromium-only; no physical/WebKit pan, pinch-zoom or whole-app theme-transition claim. Initial fixture and pan-driver failures are retained separately." + "expected_playwright": "1.56.1", + "result": "browser/result.json", + "parent": "browser/parent.json", + "driver": "frontend-modern/browser-tests/pool-ownership.cjs" + } } diff --git a/frontend-modern/src/components/Storage/__tests__/StoragePoolDetail.ownership.test.tsx b/frontend-modern/src/components/Storage/__tests__/StoragePoolDetail.ownership.test.tsx new file mode 100644 index 0000000000..b3a08bed5f --- /dev/null +++ b/frontend-modern/src/components/Storage/__tests__/StoragePoolDetail.ownership.test.tsx @@ -0,0 +1,87 @@ +import { createSignal } from 'solid-js'; +import { fireEvent, render, screen } from '@solidjs/testing-library'; +import { describe, expect, it, vi } from 'vitest'; +import { StoragePoolDetail } from '../StoragePoolDetail'; +import { buildStorageRecords } from '@/features/storageBackups/storageAdapters'; +import type { Resource } from '@/types/resource'; +import type { State } from '@/types/api'; + +vi.mock('@/components/shared/HistoryChart', () => ({ + HistoryChart: () =>
History fixture
, +})); + +const pool = (owner: string | undefined): Resource => + ({ + id: 'pool', + name: 'tank', + type: 'storage', + platformType: 'truenas', + parentId: owner, + parentName: 'Identical label', + storage: { + type: 'zfs-pool', + isZfs: true, + zfsPool: { + name: 'tank', + state: 'ONLINE', + readErrors: 0, + writeErrors: 0, + checksumErrors: 0, + devices: [{ name: 'sda', type: 'disk', state: 'ONLINE' }], + }, + }, + }) as Resource; + +const disk = (id: string, owner: string, model: string, errors = 0): Resource => + ({ + id, + name: model, + type: 'physical_disk', + platformType: 'truenas', + parentId: owner, + parentName: 'Identical label', + physicalDisk: { devPath: '/dev/sda', model, errorCount: errors, temperature: errors ? 82 : 34 }, + }) as Resource; + +describe('Storage pool ownership through live snapshots', () => { + it('updates mounted ownership, local warnings and removal without leaking peer disks', () => { + const [resource, setResource] = createSignal(pool('host-a')); + const [disks, setDisks] = createSignal([ + disk('a', 'host-a', 'Local disk'), + disk('b', 'host-b', 'Peer disk', 99), + ]); + const record = () => buildStorageRecords({ state: {} as State, resources: [resource()] })[0]; + render(() => ( + + + + +
+ )); + + expect(screen.getByText('Physical Disks (1)')).toBeVisible(); + expect(screen.getByText('Local disk')).toBeVisible(); + expect(screen.queryByText('Peer disk')).not.toBeInTheDocument(); + expect(screen.queryByText('99 errors')).not.toBeInTheDocument(); + + setResource(pool('host-b')); + expect(screen.queryByText('Local disk')).not.toBeInTheDocument(); + expect(screen.getByText('Peer disk')).toBeVisible(); + expect(screen.getByText('99 errors')).toBeVisible(); + expect(screen.getByText('82°C')).toBeVisible(); + fireEvent.click(screen.getByRole('tab', { name: 'History' })); + fireEvent.click(screen.getByRole('tab', { name: 'Overview' })); + expect(screen.getByText('Peer disk')).toBeVisible(); + + setResource(pool(undefined)); + expect(screen.queryByText(/Physical Disks/)).not.toBeInTheDocument(); + expect(screen.getByText('ZFS Pool')).toBeVisible(); + setResource(pool('host-a')); + setDisks([disk('a', 'host-a', 'Local disk', 4)]); + expect(screen.getByText('Local disk')).toBeVisible(); + expect(screen.getByText('4 errors')).toBeVisible(); + setDisks([]); + expect(screen.queryByText(/Physical Disks/)).not.toBeInTheDocument(); + expect(screen.queryByText('4 errors')).not.toBeInTheDocument(); + }); +}); diff --git a/frontend-modern/src/components/Storage/__tests__/StoragePoolDetail.test.tsx b/frontend-modern/src/components/Storage/__tests__/StoragePoolDetail.test.tsx index d0f472d70f..4652191e6c 100644 --- a/frontend-modern/src/components/Storage/__tests__/StoragePoolDetail.test.tsx +++ b/frontend-modern/src/components/Storage/__tests__/StoragePoolDetail.test.tsx @@ -238,6 +238,7 @@ describe('StoragePoolDetail', () => { [ { id: 'disk1', + parentId: 'storage-1', type: 'physical_disk', name: 'Disk 1', displayName: 'Disk 1', diff --git a/frontend-modern/src/components/Storage/__tests__/useStoragePoolDetailModel.test.ts b/frontend-modern/src/components/Storage/__tests__/useStoragePoolDetailModel.test.ts index 187507a5e6..9206ffef31 100644 --- a/frontend-modern/src/components/Storage/__tests__/useStoragePoolDetailModel.test.ts +++ b/frontend-modern/src/components/Storage/__tests__/useStoragePoolDetailModel.test.ts @@ -23,6 +23,7 @@ const buildRecord = (): StorageRecord => ({ metricsTarget: { resourceType: 'storage', resourceId: 'pool:tank' }, details: { node: 'truenas01', + parentId: 'host-truenas01', type: 'pool', zfsPool: { state: 'ONLINE', @@ -38,6 +39,7 @@ const buildRecord = (): StorageRecord => ({ const buildDisk = (): Resource => ({ id: 'disk-1', + parentId: 'host-truenas01', type: 'physical_disk', name: 'disk-1', displayName: 'disk-1', diff --git a/frontend-modern/src/features/storageBackups/__tests__/storagePoolDetailPresentation.branchcov.test.ts b/frontend-modern/src/features/storageBackups/__tests__/storagePoolDetailPresentation.branchcov.test.ts index ce66441060..5950ed1eef 100644 --- a/frontend-modern/src/features/storageBackups/__tests__/storagePoolDetailPresentation.branchcov.test.ts +++ b/frontend-modern/src/features/storageBackups/__tests__/storagePoolDetailPresentation.branchcov.test.ts @@ -453,16 +453,18 @@ describe('getStoragePoolLinkedDisks branch coverage', () => { id: 'cache-pool-1', name: ' My Cache ', source: unraidSource(), - details: { type: 'unraid-cache-pool', platform: 'unraid' }, + details: { type: 'unraid-cache-pool', platform: 'unraid', parentId: 'host-1' }, }); const disks = [ { id: 'in', + parentId: 'host-1', name: 'in', physicalDisk: { devPath: '/dev/sda', storageRole: 'cache', storageGroup: 'My Cache' }, }, { id: 'out', + parentId: 'host-1', name: 'out', physicalDisk: { devPath: '/dev/sdb', storageRole: 'cache', storageGroup: 'other' }, }, @@ -476,16 +478,18 @@ describe('getStoragePoolLinkedDisks branch coverage', () => { id: 'arr-1', name: 'Tower', source: unraidSource(), - details: { type: 'pool', platform: 'unraid', topology: 'array' }, + details: { type: 'pool', platform: 'unraid', topology: 'array', parentId: 'host-1' }, }); const disks = [ { id: 'in', + parentId: 'host-1', name: 'in', physicalDisk: { devPath: '/dev/sda', storageRole: 'data', storageGroup: 'unraid-array' }, }, { id: 'out', + parentId: 'host-1', name: 'out', physicalDisk: { devPath: '/dev/sdb', storageRole: 'data', storageGroup: 'nope' }, }, diff --git a/frontend-modern/src/features/storageBackups/__tests__/storagePoolDetailPresentation.test.ts b/frontend-modern/src/features/storageBackups/__tests__/storagePoolDetailPresentation.test.ts index 47f5645cfd..dc34f25a70 100644 --- a/frontend-modern/src/features/storageBackups/__tests__/storagePoolDetailPresentation.test.ts +++ b/frontend-modern/src/features/storageBackups/__tests__/storagePoolDetailPresentation.test.ts @@ -30,6 +30,7 @@ const buildRecord = (overrides: Partial = {}): StorageRecord => ( observedAt: Date.now(), details: { node: 'truenas01', + parentId: 'host-truenas01', type: 'pool', zfsPool: { state: 'ONLINE', @@ -66,6 +67,7 @@ const buildRecord = (overrides: Partial = {}): StorageRecord => ( const buildDisk = (): Resource => ({ id: 'disk-1', + parentId: 'host-truenas01', type: 'physical_disk', name: 'disk-1', displayName: 'disk-1', @@ -249,6 +251,7 @@ describe('storagePoolDetailPresentation', () => { }, details: { type: 'unraid-array', + parentId: 'host-tower', platform: 'unraid', topology: 'array', arrayState: 'STARTED', @@ -257,6 +260,7 @@ describe('storagePoolDetailPresentation', () => { const disks = [ { id: 'parity', + parentId: 'host-tower', type: 'physical_disk', name: 'Parity', displayName: 'Parity', @@ -277,6 +281,7 @@ describe('storagePoolDetailPresentation', () => { }, { id: 'disk1', + parentId: 'host-tower', type: 'physical_disk', name: 'Disk 1', displayName: 'Disk 1', diff --git a/frontend-modern/src/features/storageBackups/__tests__/storagePoolDiskOwnership.test.ts b/frontend-modern/src/features/storageBackups/__tests__/storagePoolDiskOwnership.test.ts new file mode 100644 index 0000000000..6ae92dafa8 --- /dev/null +++ b/frontend-modern/src/features/storageBackups/__tests__/storagePoolDiskOwnership.test.ts @@ -0,0 +1,170 @@ +import { describe, expect, it } from 'vitest'; +import type { State } from '@/types/api'; +import type { Resource } from '@/types/resource'; +import { buildStorageRecords } from '../storageAdapters'; +import { getStoragePoolLinkedDisks } from '../storagePoolDetailPresentation'; + +const storage = (overrides: Partial = {}): Resource => + ({ + id: 'pool-a', + type: 'storage', + name: 'tank', + displayName: 'tank', + platformId: 'provider-a', + platformType: 'proxmox-pve', + sourceType: 'api', + status: 'online', + lastSeen: 1, + parentId: 'host-a', + parentName: 'Same display name', + storage: { + type: 'zfspool', + isZfs: true, + zfsPool: { + name: 'tank', + state: 'ONLINE', + readErrors: 0, + writeErrors: 0, + checksumErrors: 0, + devices: [{ name: 'sda', type: 'disk', state: 'ONLINE' }], + }, + }, + proxmox: { nodeName: 'pve1', instance: 'cluster-a' }, + ...overrides, + }) as Resource; + +const disk = (id: string, overrides: Partial = {}): Resource => + ({ + id, + type: 'physical_disk', + name: id, + displayName: id, + platformId: 'provider-a', + platformType: 'proxmox-pve', + sourceType: 'api', + status: 'online', + lastSeen: 1, + parentId: 'host-a', + parentName: 'Same display name', + proxmox: { nodeName: 'pve1', instance: 'cluster-a' }, + physicalDisk: { devPath: '/dev/sda', model: id, temperature: 35 }, + ...overrides, + }) as Resource; + +const record = (resource = storage()) => + buildStorageRecords({ state: {} as State, resources: [resource] })[0]; +const linked = (resource: Resource, disks: Resource[]) => + getStoragePoolLinkedDisks(record(resource), disks).map((item) => item.id); + +describe('pool physical disk ownership', () => { + it('excludes other hosts and unknown owners despite identical paths and labels', () => { + expect( + linked(storage(), [ + disk('local'), + disk('remote', { parentId: 'host-b' }), + disk('unknown', { parentId: undefined, proxmox: undefined }), + ]), + ).toEqual(['local']); + }); + + it('never lets matching provider names override mismatched canonical parents', () => { + expect(linked(storage(), [disk('remote', { parentId: 'host-b' })])).toEqual([]); + }); + + it('preserves direct canonical pool and legacy resource-reference children', () => { + const target = record(); + target.refs = { resourceId: 'legacy-pool' }; + expect( + getStoragePoolLinkedDisks(target, [ + disk('direct', { parentId: 'pool-a', physicalDisk: { devPath: '/dev/other' } }), + disk('legacy', { parentId: 'legacy-pool', physicalDisk: { devPath: '/dev/legacy' } }), + ]).map((item) => item.id), + ).toEqual(['legacy', 'direct']); + }); + + it.each(['top-level', 'legacy'] as const)( + 'retains exact native instance/node ownership: %s', + (shape) => { + const pool = storage({ parentId: undefined, proxmox: undefined, platformData: undefined }); + const local = disk('local', { parentId: undefined, proxmox: undefined }); + const native = { nodeName: 'pve1', instance: 'cluster-a' }; + if (shape === 'top-level') { + pool.proxmox = native; + local.proxmox = native; + } else { + pool.platformData = { proxmox: native }; + local.platformData = { proxmox: native }; + } + expect(record(pool).details?.instance).toBe('cluster-a'); + expect( + linked(pool, [ + local, + disk('other-cluster', { + parentId: undefined, + proxmox: { ...native, instance: 'cluster-b' }, + }), + disk('other-node', { parentId: undefined, proxmox: { ...native, nodeName: 'pve2' } }), + disk('unscoped', { parentId: undefined, proxmox: { nodeName: 'pve1' } }), + ]), + ).toEqual(['local']); + }, + ); + + it('requires both native instance and node when canonical ownership is missing', () => { + expect( + linked(storage({ parentId: undefined, proxmox: { nodeName: 'pve1' } }), [ + disk('unscoped', { parentId: undefined, proxmox: { nodeName: 'pve1' } }), + ]), + ).toEqual([]); + }); + + it.each(['sda', '/dev/sda'])('matches whole device identifiers, not a suffix: %s', (name) => { + const pool = storage(); + pool.storage!.zfsPool!.devices = [ + { name, type: 'disk', state: 'ONLINE', readErrors: 0, writeErrors: 0, checksumErrors: 0 }, + ]; + expect( + linked(pool, [ + disk('local'), + disk('suffix-collision', { physicalDisk: { devPath: '/dev/not-sda' } }), + disk('partition', { physicalDisk: { devPath: '/dev/sda1' } }), + ]), + ).toEqual(['local']); + }); + + it('does not confuse nvme device numbers with a shorter vdev name', () => { + const pool = storage(); + pool.storage!.zfsPool!.devices = [ + { + name: 'n1', + type: 'disk', + state: 'ONLINE', + readErrors: 0, + writeErrors: 0, + checksumErrors: 0, + }, + ]; + expect(linked(pool, [disk('nvme', { physicalDisk: { devPath: '/dev/nvme0n1' } })])).toEqual([]); + }); + + it.each([ + ['unraid-array', 'Array', 'unraid-array'], + ['unraid-cache-pool', 'cache', 'cache'], + ])('keeps repeated UnRAID groups on their own host: %s', (type, name, group) => { + const pool = storage({ + name, + platformType: 'unraid', + proxmox: undefined, + storage: { type, platform: 'unraid' }, + }); + const member = { devPath: '/dev/sda', storageGroup: group, storageRole: 'data' }; + expect( + linked(pool, [ + disk('local', { physicalDisk: member }), + disk('remote', { parentId: 'host-b', physicalDisk: member }), + disk('unknown', { parentId: undefined, proxmox: undefined, physicalDisk: member }), + disk('different-group', { physicalDisk: { ...member, storageGroup: 'other' } }), + ]), + ).toEqual(['local']); + }); +}); diff --git a/frontend-modern/src/features/storageBackups/storageAdapters.ts b/frontend-modern/src/features/storageBackups/storageAdapters.ts index efa82a594d..19b8ac6598 100644 --- a/frontend-modern/src/features/storageBackups/storageAdapters.ts +++ b/frontend-modern/src/features/storageBackups/storageAdapters.ts @@ -224,6 +224,10 @@ const mapResourceStorageRecord = (resource: Resource, adapterId: string): Storag parentId: resource.parentId, parentName: resource.parentName, node: proxmoxNativeNode || (platformData.node as string | undefined) || storageNodes[0], + instance: + resource.proxmox?.instance || + (proxmoxPlatform?.instance as string | undefined) || + (platformData.instance as string | undefined), nodeHints, hostLabel, platformLabel, diff --git a/frontend-modern/src/features/storageBackups/storagePoolDetailPresentation.ts b/frontend-modern/src/features/storageBackups/storagePoolDetailPresentation.ts index 1fc455ff90..166571e68f 100644 --- a/frontend-modern/src/features/storageBackups/storagePoolDetailPresentation.ts +++ b/frontend-modern/src/features/storageBackups/storagePoolDetailPresentation.ts @@ -368,6 +368,28 @@ const diskRoleRank = (role: string): number => { } }; +const diskBelongsToStorageHost = (record: StorageRecord, disk: Resource): boolean => { + const hostId = readRecordDetailString(record, 'parentId'); + const diskParentId = disk.parentId?.trim(); + // Canonical ownership outranks labels, paths and source-native names. + // In particular, two hosts may both be called pve1 and both report /dev/sda. + if (hostId && diskParentId) return hostId === diskParentId; + + // Older Proxmox snapshots may lack canonical parents. Only the complete + // native instance/node pair is a scope; a hostname alone is not one. + const platformData = (disk.platformData || {}) as Record; + const proxmox = platformData.proxmox as Resource['proxmox'] | undefined; + const node = readRecordDetailString(record, 'node').toLowerCase(); + const instance = readRecordDetailString(record, 'instance'); + const diskNode = (disk.proxmox?.nodeName || disk.proxmox?.node || proxmox?.nodeName || '') + .trim() + .toLowerCase(); + const diskInstance = (disk.proxmox?.instance || proxmox?.instance || '').trim(); + return Boolean(node && instance && node === diskNode && instance === diskInstance); +}; + +const normalizeDevicePath = (value: string): string => value.trim().replace(/^\/dev\//, ''); + export function getStoragePoolLinkedDisks( record: StorageRecord, physicalDisks: Resource[], @@ -382,9 +404,14 @@ export function getStoragePoolLinkedDisks( .filter((disk) => { const devPath = readDiskDevPath(disk); const group = readDiskStorageGroup(disk).toLowerCase(); - if (directParentIds.has(disk.parentId || '')) return true; + if (directParentIds.has(disk.parentId?.trim() || '')) return true; + if (!diskBelongsToStorageHost(record, disk)) return false; if (pool?.devices?.length && devPath) { - return pool.devices.some((device) => devPath.endsWith(device.name)); + return pool.devices.some( + (device) => + Boolean(device.name.trim()) && + normalizeDevicePath(devPath) === normalizeDevicePath(device.name), + ); } return Boolean(unraidStorageGroup && group === unraidStorageGroup); })