diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 636d0a2a2..4ff36c13e 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -2415,6 +2415,17 @@ verification. rather than verified results. Safe auto-fix and Autopilot remain visibly `not_assessed` until their governed canaries exist. +42. Frontend consumers identify a storage resource by + `storage.topology`, never by a ZFS vdev layout. TrueNAS pools arrive + as `type: 'storage'`, so `topology === 'pool'` is the only pool + discriminator available to the page model, and layout strings belong + in `storage.vdevLayout`. Regression coverage: the `pool identity + boundary` cases in + `frontend-modern/src/features/truenas/__tests__/truenasPageModel.test.ts` + and `shows the vdev layout as the storage kind while topology stays + the pool discriminator` in + `frontend-modern/src/components/Infrastructure/__tests__/resourceDetailDrawerTrueNASModel.test.ts`. + ## Current State ### System member rows are source-type aware diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 4f76172e6..0395042d4 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -712,6 +712,16 @@ remains. paths added to the adapter must keep that persistence step; ephemeral snapshot-bridge adapters stay read-only. +11. The TrueNAS provider projects pools with `Storage.Topology` fixed to + `pool` and the ZFS data vdev layout in `Storage.VDevLayout`. The + layout summary (`poolVDevLayout`) returns an empty string when the + native report carries no data vdevs so callers fall back instead of + inventing a layout. Publishing the layout as the topology drops every + pool out of the TrueNAS page, and package fixtures carry no vdevs, so + layout-bearing pools must be exercised explicitly. Regression + coverage: `TestPoolTopologyStaysDiscriminatorAcrossVDevLayouts` in + `internal/truenas/provider_pool_health_contract_test.go`. + ## Current State ### PBS health is one completed-poll outcome diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index 3fabcd12b..a9b4a31e5 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -1958,6 +1958,15 @@ pressure to assess context selection, but it must not read live backup records, invoke recovery APIs, mutate storage, or treat a synthetic pass as evidence that Safe auto-fix or Autopilot remediation is verified. +26. Storage row presentation resolves its topology label from + `storage.vdevLayout` first and falls back to `storage.topology`, so a + pool reads as its concrete layout ("Mirror", "Raidz2") without the + discriminator ever carrying presentation detail. Regression coverage: + `labels a TrueNAS pool by its vdev layout while keeping the pool + discriminator` and `falls back to the topology discriminator when no + vdev layout is reported` in + `frontend-modern/src/features/storageBackups/__tests__/storageAdapters.test.ts`. + ## Current State ### Proxmox runtime continuity is not protection evidence diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index 19ee51c8e..3684c49e8 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -1633,6 +1633,16 @@ AI-only summary payloads, or page-local heuristics. `TestIngestRecordsSkipRecordDeclaredSuccessionForLiveOldID` in `internal/unifiedresources/canonical_id_succession_test.go`. +28. `StorageMeta.Topology` is a closed discriminator vocabulary shared by + every storage provider (`pool`, `dataset`, `array`, `datastore`). + Consumers key resource identity off it, so a provider must never + publish presentation detail there. ZFS data vdev layout is + presentation and belongs in `StorageMeta.VDevLayout`, which is absent + rather than invented when the native report carries no data vdevs. + Regression coverage: + `TestStorageTopologyAndVDevLayoutAreDistinctIdentityFields` in + `internal/unifiedresources/canonical_identity_test.go`. + ## Current State ### Host views expose the integration-source discriminator diff --git a/frontend-modern/src/components/Infrastructure/__tests__/resourceDetailDrawerTrueNASModel.test.ts b/frontend-modern/src/components/Infrastructure/__tests__/resourceDetailDrawerTrueNASModel.test.ts index 5d45e6d40..eccd60937 100644 --- a/frontend-modern/src/components/Infrastructure/__tests__/resourceDetailDrawerTrueNASModel.test.ts +++ b/frontend-modern/src/components/Infrastructure/__tests__/resourceDetailDrawerTrueNASModel.test.ts @@ -206,7 +206,8 @@ describe('resourceDetailDrawerTrueNASModel', () => { platformScopes: ['truenas'], storage: { type: 'zfs-pool', - topology: 'mirror', + topology: 'pool', + vdevLayout: 'mirror', platform: 'truenas', zfsPoolState: 'DEGRADED', poolHealth: { @@ -313,4 +314,35 @@ describe('resourceDetailDrawerTrueNASModel', () => { 'SMART', ]); }); + + it('shows the vdev layout as the storage kind while topology stays the pool discriminator', () => { + // Regression guard for 599c8e634. `topology` is the cross-provider + // discriminator ('pool' | 'dataset' | ...); the ZFS layout belongs in + // `vdevLayout`. The drawer prefers the layout for display but must not + // depend on it being in `topology`. + const withLayout = baseResource({ + type: 'storage', + displayName: 'tank', + storage: { + type: 'zfs-pool', + topology: 'pool', + vdevLayout: 'raidz2', + platform: 'truenas', + }, + } as Partial); + + expect(buildTrueNASDetailsSummary(withLayout)).toContain('Raidz2'); + + const withoutLayout = baseResource({ + type: 'storage', + displayName: 'tank', + storage: { + type: 'zfs-pool', + topology: 'pool', + platform: 'truenas', + }, + } as Partial); + + expect(buildTrueNASDetailsSummary(withoutLayout)).toContain('Pool'); + }); }); diff --git a/frontend-modern/src/components/Infrastructure/resourceDetailDrawerTrueNASModel.ts b/frontend-modern/src/components/Infrastructure/resourceDetailDrawerTrueNASModel.ts index 874857026..2505d4dab 100644 --- a/frontend-modern/src/components/Infrastructure/resourceDetailDrawerTrueNASModel.ts +++ b/frontend-modern/src/components/Infrastructure/resourceDetailDrawerTrueNASModel.ts @@ -205,6 +205,8 @@ const buildTrueNASSystemSections = ( }; const storageKindLabel = (storage: ResourceStorageMeta): string | null => { + const layout = asString(storage.vdevLayout); + if (layout) return normalizeDelimitedLabel(layout); const topology = asString(storage.topology); if (topology) return normalizeDelimitedLabel(topology); return normalizeDelimitedLabel(storage.type); diff --git a/frontend-modern/src/features/storageBackups/__tests__/storageAdapters.test.ts b/frontend-modern/src/features/storageBackups/__tests__/storageAdapters.test.ts index 6ef682be9..274d7be12 100644 --- a/frontend-modern/src/features/storageBackups/__tests__/storageAdapters.test.ts +++ b/frontend-modern/src/features/storageBackups/__tests__/storageAdapters.test.ts @@ -662,4 +662,48 @@ describe('storageAdapters', () => { expect(records).toHaveLength(1); expect(records[0].details?.zfsPool).toEqual(zfsPool); }); + + it('labels a TrueNAS pool by its vdev layout while keeping the pool discriminator', () => { + // Regression guard for 599c8e634: the layout must ride in `vdevLayout`, not + // in `topology`. `topology` identifies the resource as a pool; publishing a + // layout there dropped every pool off the TrueNAS page on real hardware. + const state = baseState(); + const records = buildStorageRecords({ + state, + resources: [ + makeResourceStorage({ + storage: { + type: 'zfs-pool', + isZfs: true, + platform: 'truenas', + topology: 'pool', + vdevLayout: 'mirror', + } as Resource['storage'], + }), + ], + }); + + expect(records).toHaveLength(1); + expect(records[0].topologyLabel).toBe('Mirror'); + }); + + it('falls back to the topology discriminator when no vdev layout is reported', () => { + const state = baseState(); + const records = buildStorageRecords({ + state, + resources: [ + makeResourceStorage({ + storage: { + type: 'zfs-pool', + isZfs: true, + platform: 'truenas', + topology: 'pool', + } as Resource['storage'], + }), + ], + }); + + expect(records).toHaveLength(1); + expect(records[0].topologyLabel).toBe('Pool'); + }); }); diff --git a/frontend-modern/src/features/storageBackups/storageAdapters.ts b/frontend-modern/src/features/storageBackups/storageAdapters.ts index 0924119ac..17ff38d44 100644 --- a/frontend-modern/src/features/storageBackups/storageAdapters.ts +++ b/frontend-modern/src/features/storageBackups/storageAdapters.ts @@ -152,7 +152,9 @@ const mapResourceStorageRecord = (resource: Resource, adapterId: string): Storag const topologyLabel = getResourceStorageTopologyLabel( resource, storageType, - resource.storage?.topology, + // Prefer the concrete vdev layout ("Mirror", "Raidz2") over the generic + // discriminator ("Pool") when the provider reports one. + resource.storage?.vdevLayout || resource.storage?.topology, ); const metricsTarget = metricsTargetForStorageResource(resource); diff --git a/frontend-modern/src/features/truenas/__tests__/truenasPageModel.test.ts b/frontend-modern/src/features/truenas/__tests__/truenasPageModel.test.ts index 9f152f5df..3ef854256 100644 --- a/frontend-modern/src/features/truenas/__tests__/truenasPageModel.test.ts +++ b/frontend-modern/src/features/truenas/__tests__/truenasPageModel.test.ts @@ -990,3 +990,54 @@ describe('truenasPageModel', () => { ); }); }); + +describe('pool identity boundary', () => { + // Regression guard for 599c8e634: the producer briefly published the ZFS vdev + // layout ("mirror", "raidz2") as storage.topology. TrueNAS pools arrive as + // type 'storage', so `topology === 'pool'` is the only thing that identifies + // them here — publishing a layout there dropped every pool off this page on + // real hardware while every fixture (which omits vdevs) stayed green. + it('recognises a pool that also reports a vdev layout', () => { + const system = makeResource({ id: 'nas-a', type: 'agent', name: 'nas-a' }); + const pool = makeResource({ + id: 'pool-tank', + type: 'storage', + name: 'tank', + parentId: system.id, + storage: { topology: 'pool', vdevLayout: 'mirror', platform: 'truenas' }, + }); + const dataset = makeResource({ + id: 'dataset-media', + type: 'storage', + name: 'tank/media', + parentId: pool.id, + storage: { topology: 'dataset', platform: 'truenas' }, + }); + + const rows = buildTrueNASStorageTopologyRows([system, pool, dataset]); + const poolRow = rows.find((row) => row.resource.id === pool.id); + + expect(poolRow?.kind).toBe('pool'); + // The dataset must still hang off the pool rather than orphaning. + const datasetRow = rows.find((row) => row.resource.id === dataset.id); + expect(datasetRow?.parentRowId).toBe(poolRow?.id); + }); + + it('does not identify a pool by its vdev layout alone', () => { + const system = makeResource({ id: 'nas-a', type: 'agent', name: 'nas-a' }); + // A pool whose topology carries a layout instead of the discriminator is + // exactly the broken shape; assert it is NOT silently accepted, so the + // producer contract stays the single place this is fixed. + const malformed = makeResource({ + id: 'pool-tank', + type: 'storage', + name: 'tank', + parentId: system.id, + storage: { topology: 'mirror', platform: 'truenas' }, + }); + + const rows = buildTrueNASStorageTopologyRows([system, malformed]); + + expect(rows.find((row) => row.resource.id === malformed.id)).toBeUndefined(); + }); +}); diff --git a/frontend-modern/src/types/resource.ts b/frontend-modern/src/types/resource.ts index 7f07f7c76..bba034bc4 100644 --- a/frontend-modern/src/types/resource.ts +++ b/frontend-modern/src/types/resource.ts @@ -389,7 +389,14 @@ export interface ResourceStorageMeta { isCeph?: boolean; isZfs?: boolean; platform?: string; + /** + * Closed discriminator vocabulary shared by every storage provider + * ('pool' | 'dataset' | 'array' | 'datastore'). Resource identity keys off + * it, so it never carries presentation detail — see `vdevLayout`. + */ topology?: string; + /** ZFS data vdev layout for display ('mirror', 'raidz2', 'mirror+special'). */ + vdevLayout?: string; protection?: string; risk?: ResourceStorageRisk; riskSummary?: string; diff --git a/internal/truenas/provider.go b/internal/truenas/provider.go index f846570cc..157426b07 100644 --- a/internal/truenas/provider.go +++ b/internal/truenas/provider.go @@ -605,7 +605,8 @@ func truenasRecordsFromSnapshot(snapshot *FixtureSnapshot, connectionID string, Type: "zfs-pool", IsZFS: true, Platform: "truenas", - Topology: poolTopologyLabel(pool), + Topology: "pool", + VDevLayout: poolVDevLayout(pool), Protection: "zfs", Risk: risk, RiskSummary: unifiedresources.StorageRiskSummary(risk), @@ -1652,7 +1653,12 @@ func poolScanIsActive(state string) bool { } } -func poolTopologyLabel(pool Pool) string { +// poolVDevLayout summarizes the pool's data vdev layout ("mirror", "raidz2", +// "stripe", "mirror+special") for presentation. It returns an empty string when +// the native report carries no data vdevs, so callers fall back rather than +// inventing a layout. This is deliberately NOT the pool's topology +// discriminator: see StorageMeta.Topology. +func poolVDevLayout(pool Pool) string { var dataTypes []string for _, vdev := range pool.VDevs { if vdev.ParentID != "" || !strings.EqualFold(strings.TrimSpace(vdev.Role), "data") { @@ -1665,7 +1671,7 @@ func poolTopologyLabel(pool Pool) string { dataTypes = append(dataTypes, vdevType) } if len(dataTypes) == 0 { - return "pool" + return "" } if len(dataTypes) == 1 && dataTypes[0] == "disk" && len(pool.DiskMembers) > 1 { return "stripe" diff --git a/internal/truenas/provider_pool_health_contract_test.go b/internal/truenas/provider_pool_health_contract_test.go index 23ae966f3..5f515bebe 100644 --- a/internal/truenas/provider_pool_health_contract_test.go +++ b/internal/truenas/provider_pool_health_contract_test.go @@ -59,8 +59,14 @@ func TestProviderProjectsFullZFSHealthAndActionableDatasetAppIncidents(t *testin if pool.Storage == nil || pool.Storage.ZFSPool == nil || pool.Storage.PoolHealth == nil { t.Fatalf("full pool health contract missing: %+v", pool.Storage) } - if pool.Storage.Topology != "mirror" { - t.Fatalf("topology = %q", pool.Storage.Topology) + // Topology is the cross-provider discriminator every pool consumer keys on + // and must stay "pool" even for a pool with a rich vdev layout; the layout + // itself rides in VDevLayout. + if pool.Storage.Topology != "pool" { + t.Fatalf("topology = %q, want \"pool\"", pool.Storage.Topology) + } + if pool.Storage.VDevLayout != "mirror" { + t.Fatalf("vdev layout = %q, want \"mirror\"", pool.Storage.VDevLayout) } if pool.Storage.ZFSReadErrors != 1 || pool.Storage.ZFSChecksumErrors != 2 { t.Fatalf("flattened errors = %+v", pool.Storage) @@ -152,3 +158,80 @@ func requireRecordByNameAndType(t *testing.T, records []unifiedresources.IngestR t.Fatalf("missing %s %q in %+v", resourceType, name, records) return unifiedresources.IngestRecord{} } + +// TestPoolTopologyStaysDiscriminatorAcrossVDevLayouts pins the boundary that +// broke in 599c8e634: the frontend identifies a TrueNAS pool solely by +// storage.topology == "pool" (TrueNAS pools are transported as +// ResourceTypeStorage, so resource.type is never "pool"), and the producer +// briefly published the vdev layout there instead. Every fixture in this +// package omits VDevs, so the projection kept returning "pool" and both sides +// stayed green while disagreeing on real hardware. Drive real layouts here. +func TestPoolTopologyStaysDiscriminatorAcrossVDevLayouts(t *testing.T) { + observedAt := time.Date(2026, 7, 24, 10, 0, 0, 0, time.UTC) + + for _, tc := range []struct { + name string + vdevs []PoolVDev + members []PoolDiskMember + wantLayout string + }{ + {name: "no vdevs reported", wantLayout: ""}, + { + name: "mirror", + vdevs: []PoolVDev{{ID: "m0", Name: "mirror-0", Type: "MIRROR", Role: "data", Status: "ONLINE"}}, + wantLayout: "mirror", + }, + { + name: "raidz2", + vdevs: []PoolVDev{{ID: "z0", Name: "raidz2-0", Type: "RAIDZ2", Role: "data", Status: "ONLINE"}}, + wantLayout: "raidz2", + }, + { + name: "striped single disks", + vdevs: []PoolVDev{ + {ID: "d0", Name: "sda", Type: "DISK", Role: "data", Status: "ONLINE"}, + {ID: "d1", Name: "sdb", Type: "DISK", Role: "data", Status: "ONLINE"}, + }, + members: []PoolDiskMember{ + {Disk: "sda", Status: "ONLINE", Role: "data"}, + {Disk: "sdb", Status: "ONLINE", Role: "data"}, + }, + wantLayout: "stripe", + }, + { + name: "mixed data vdev types", + vdevs: []PoolVDev{ + {ID: "m0", Name: "mirror-0", Type: "MIRROR", Role: "data", Status: "ONLINE"}, + {ID: "s0", Name: "special-0", Type: "SPECIAL", Role: "data", Status: "ONLINE"}, + }, + wantLayout: "mirror+special", + }, + } { + t.Run(tc.name, func(t *testing.T) { + records := FixtureRecords(FixtureSnapshot{ + CollectedAt: observedAt, + System: SystemInfo{Hostname: "nas-a", Healthy: true}, + Pools: []Pool{{ + ID: "1", + GUID: "pool-guid", + Name: "tank", + Status: "ONLINE", + VDevs: tc.vdevs, + DiskMembers: tc.members, + }}, + }) + + pool := requirePoolRecord(t, records, "tank").Resource + if pool.Storage == nil { + t.Fatalf("pool record carries no storage meta: %+v", pool) + } + if pool.Storage.Topology != "pool" { + t.Fatalf("topology = %q, want %q: a non-%q value drops the pool out of the TrueNAS page entirely", + pool.Storage.Topology, "pool", "pool") + } + if pool.Storage.VDevLayout != tc.wantLayout { + t.Fatalf("vdev layout = %q, want %q", pool.Storage.VDevLayout, tc.wantLayout) + } + }) + } +} diff --git a/internal/unifiedresources/canonical_identity_test.go b/internal/unifiedresources/canonical_identity_test.go index 9824fadd5..f2b2164d3 100644 --- a/internal/unifiedresources/canonical_identity_test.go +++ b/internal/unifiedresources/canonical_identity_test.go @@ -1,6 +1,7 @@ package unifiedresources import ( + "encoding/json" "reflect" "testing" @@ -915,3 +916,51 @@ func TestRefreshCanonicalIdentityPreservesTrueNASVMIdentity(t *testing.T) { t.Fatalf("primaryId = %q, want vm:42", got) } } + +// TestStorageTopologyAndVDevLayoutAreDistinctIdentityFields pins the separation +// restored after 599c8e634. StorageMeta.Topology is the closed discriminator +// vocabulary consumers use to identify a resource ("pool", "dataset", "array", +// "datastore"); the ZFS vdev layout is presentation and lives in VDevLayout. +// Collapsing the two, or dropping either through a clone, silently changes +// resource identity for every storage consumer. +func TestStorageTopologyAndVDevLayoutAreDistinctIdentityFields(t *testing.T) { + meta := &StorageMeta{ + Type: "zfs-pool", + Platform: "truenas", + Topology: "pool", + VDevLayout: "mirror+special", + } + + cloned := cloneStorageMeta(meta) + if cloned == nil { + t.Fatal("cloneStorageMeta returned nil") + } + if cloned.Topology != "pool" { + t.Fatalf("cloned topology = %q, want \"pool\"", cloned.Topology) + } + if cloned.VDevLayout != "mirror+special" { + t.Fatalf("cloned vdev layout = %q, want \"mirror+special\"", cloned.VDevLayout) + } + + // A pool with no reported vdevs still carries the discriminator, and the + // layout stays absent rather than being backfilled from it. + bare := cloneStorageMeta(&StorageMeta{Topology: "pool"}) + if bare.Topology != "pool" || bare.VDevLayout != "" { + t.Fatalf("bare pool meta = %+v, want topology \"pool\" and empty layout", bare) + } + + encoded, err := json.Marshal(meta) + if err != nil { + t.Fatalf("marshal storage meta: %v", err) + } + var decoded map[string]any + if err := json.Unmarshal(encoded, &decoded); err != nil { + t.Fatalf("unmarshal storage meta: %v", err) + } + if decoded["topology"] != "pool" { + t.Fatalf("wire topology = %v, want \"pool\"", decoded["topology"]) + } + if decoded["vdevLayout"] != "mirror+special" { + t.Fatalf("wire vdevLayout = %v, want \"mirror+special\"", decoded["vdevLayout"]) + } +} diff --git a/internal/unifiedresources/types.go b/internal/unifiedresources/types.go index 2973056bb..d6efb2769 100644 --- a/internal/unifiedresources/types.go +++ b/internal/unifiedresources/types.go @@ -384,16 +384,21 @@ type ProxmoxData struct { // StorageMeta contains storage-specific metadata for storage resources. type StorageMeta struct { - Type string `json:"type,omitempty"` - Content string `json:"content,omitempty"` - ContentTypes []string `json:"contentTypes,omitempty"` - Shared bool `json:"shared"` - Enabled bool `json:"enabled"` - Active bool `json:"active"` - IsCeph bool `json:"isCeph"` - IsZFS bool `json:"isZfs"` - Platform string `json:"platform,omitempty"` + Type string `json:"type,omitempty"` + Content string `json:"content,omitempty"` + ContentTypes []string `json:"contentTypes,omitempty"` + Shared bool `json:"shared"` + Enabled bool `json:"enabled"` + Active bool `json:"active"` + IsCeph bool `json:"isCeph"` + IsZFS bool `json:"isZfs"` + Platform string `json:"platform,omitempty"` + // Topology is a closed discriminator vocabulary shared by every storage + // provider ("pool", "dataset", "array", "datastore"). Consumers key + // resource identity off it, so it must never carry presentation detail. + // Per-pool vdev layout belongs in VDevLayout. Topology string `json:"topology,omitempty"` + VDevLayout string `json:"vdevLayout,omitempty"` Protection string `json:"protection,omitempty"` Risk *StorageRisk `json:"risk,omitempty"` RiskSummary string `json:"riskSummary,omitempty"`