From afc419ddab57b28fa207937c06c7c4f1a7840930 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Fri, 24 Jul 2026 15:40:32 +0100 Subject: [PATCH] Add durable Proxmox node display names --- docs/CONFIGURATION.md | 25 ++ ...ga-proxmox-node-display-name-2026-07-24.md | 74 +++++ .../v6/internal/subsystems/agent-lifecycle.md | 7 + .../v6/internal/subsystems/alerts.md | 13 +- .../v6/internal/subsystems/api-contracts.md | 17 + .../subsystems/frontend-primitives.md | 16 + .../v6/internal/subsystems/monitoring.md | 18 +- .../internal/subsystems/security-privacy.md | 6 + .../internal/subsystems/storage-recovery.md | 8 + .../internal/subsystems/unified-resources.md | 12 + frontend-modern/public/docs/CONFIGURATION.md | 25 ++ .../src/api/__tests__/nodes.test.ts | 29 ++ frontend-modern/src/api/connections.ts | 2 + frontend-modern/src/api/nodes.ts | 7 + .../Settings/InfrastructureSourceManager.tsx | 11 +- .../NodeModalClusterMembersSection.tsx | 57 +++- .../InfrastructureOperationsModel.test.tsx | 14 +- .../InfrastructureWorkspace.test.tsx | 14 +- .../Settings/__tests__/nodeModalModel.test.ts | 34 ++ .../__tests__/useConnectionsLedger.test.ts | 63 ++++ .../Settings/connectionsTableModel.ts | 2 + .../src/components/Settings/nodeModalModel.ts | 38 ++- .../Settings/useConnectionsLedger.ts | 4 + .../useInfrastructureConfiguredNodesState.ts | 25 +- .../components/Settings/useNodeModalState.ts | 24 ++ .../__tests__/ProxmoxNodesTable.test.tsx | 29 ++ ...oxmoxBackupRecoveryModel.branchcov.test.ts | 21 ++ .../proxmox/proxmoxBackupRecoveryModel.ts | 26 +- .../__tests__/storageAdapters.test.ts | 29 ++ .../storageBackups/storageAdapters.ts | 28 +- frontend-modern/src/types/nodes.ts | 14 + frontend-modern/src/types/resource.ts | 5 + ...odeModalPresentation.branchcov0718.test.ts | 3 + .../__tests__/nodeModalPresentation.test.ts | 1 + .../__tests__/resourceSearchMatch.test.ts | 15 + .../src/utils/nodeModalPresentation.ts | 4 + .../src/utils/resourceSearchMatch.ts | 3 + internal/alerts/alerts_test.go | 20 ++ internal/alerts/node.go | 3 + internal/alerts/read_model.go | 72 ++-- internal/api/config_handlers.go | 50 ++- internal/api/config_node_display_name_test.go | 239 ++++++++++++++ internal/api/config_node_handlers.go | 111 ++++++- internal/api/connections_grouping.go | 27 +- internal/api/connections_grouping_test.go | 30 ++ internal/api/connections_types.go | 2 + internal/api/contract_test.go | 6 +- internal/config/config.go | 30 +- internal/config/config_load_test.go | 21 ++ internal/config/persistence.go | 6 +- internal/config/persistence_test.go | 8 + internal/config/pve_instances.go | 72 ++++ internal/config/pve_node_identity.go | 311 ++++++++++++++++++ internal/config/pve_node_identity_test.go | 153 +++++++++ internal/models/converters.go | 4 + internal/models/deepcopy.go | 1 + internal/models/models.go | 6 + internal/models/models_frontend.go | 6 +- internal/models/pve_node_identity_test.go | 43 +++ .../monitoring/canonical_guardrails_test.go | 6 +- internal/monitoring/helpers_test.go | 14 +- internal/monitoring/monitor.go | 30 +- internal/monitoring/monitor_cluster_dedupe.go | 12 +- ...nitor_host_agent_removal_lifecycle_test.go | 39 +++ .../monitoring/monitor_integration_test.go | 13 +- internal/monitoring/monitor_polling_node.go | 3 + .../monitor_polling_node_helpers.go | 21 +- internal/monitoring/monitor_pve.go | 10 +- internal/monitoring/monitor_pve_cluster.go | 106 ++++-- .../monitor_pve_cluster_refresh_test.go | 34 ++ .../monitoring/node_memory_sources_test.go | 21 +- internal/unifiedresources/adapters.go | 12 +- internal/unifiedresources/adapters_test.go | 37 ++- .../unifiedresources/canonical_identity.go | 9 + .../canonical_identity_test.go | 38 +++ internal/unifiedresources/clone.go | 1 + internal/unifiedresources/clone_test.go | 15 + internal/unifiedresources/registry.go | 18 + internal/unifiedresources/registry_test.go | 12 + internal/unifiedresources/types.go | 3 + .../91-proxmox-node-display-names.spec.ts | 223 +++++++++++++ 81 files changed, 2448 insertions(+), 173 deletions(-) create mode 100644 docs/release-control/v6/internal/records/known-rc-issue-closure-for-ga-proxmox-node-display-name-2026-07-24.md create mode 100644 internal/api/config_node_display_name_test.go create mode 100644 internal/config/pve_node_identity.go create mode 100644 internal/config/pve_node_identity_test.go create mode 100644 internal/models/pve_node_identity_test.go create mode 100644 tests/integration/tests/91-proxmox-node-display-names.spec.ts diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 782254ef66..bc89181c3e 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -272,6 +272,31 @@ PULSE_AGENT_CONNECT_URL=https://agents.example.com:7656 Agents then post telemetry to `https://agents.example.com:7656/api/agents/agent/report` and establish their command channel at `wss://agents.example.com:7656/api/agent/ws`, while the web UI and management API remain reachable only on the private `FRONTEND_PORT` listener. If command execution is enabled, both routes must traverse the same proxy/firewall path; a successful report does not prove that the WebSocket is admitted. +### Proxmox Cluster Node Display Names + +Nodes discovered through one Proxmox VE cluster connection can have an +optional Pulse display name. Open **Settings → Infrastructure**, edit the +Proxmox VE connection, and set **Display name** beside a cluster member. + +- Display names are presentation only. They do not change the Proxmox node + name, API address, credentials, TLS fingerprint, routing, or action target. +- An empty display name uses the current native Proxmox node name. +- Names are trimmed, may contain Unicode, are limited to 128 characters, and + cannot contain control characters. Duplicate display names are allowed + because they are cosmetic; Pulse still uses a separate immutable identity. +- The override is stored with that Proxmox connection and survives reloads, + restarts, member address changes, native node renames, and temporary cluster + membership loss. Confirmed removed members retain their identity metadata so + a later reappearance can recover the override. +- Pulse keeps the native Proxmox node name, prior native names, numeric node ID + when available, and immutable Pulse identity for diagnostics and search. + These fields are also available to API and mobile clients. + +For legacy configuration first seen before Pulse records a Proxmox numeric node +ID, changing both the native node name and every known member address at once +is intentionally treated as a new member. Pulse does not guess across +ambiguous evidence because that could transfer an override to the wrong node. + ### Iframe Embedding (system.json) Embedding is controlled by `system.json` and the UI (**Settings → System → Network**): diff --git a/docs/release-control/v6/internal/records/known-rc-issue-closure-for-ga-proxmox-node-display-name-2026-07-24.md b/docs/release-control/v6/internal/records/known-rc-issue-closure-for-ga-proxmox-node-display-name-2026-07-24.md new file mode 100644 index 0000000000..fa5f3c5728 --- /dev/null +++ b/docs/release-control/v6/internal/records/known-rc-issue-closure-for-ga-proxmox-node-display-name-2026-07-24.md @@ -0,0 +1,74 @@ +# Known RC Issue Closure For GA Proxmox Node Display Name Record + +- Date: `2026-07-24` +- Gate: `known-rc-issue-closure-for-ga` +- Issue: `#1517` +- Result: `fixed-main-proof` + +## Context + +A single configured Proxmox VE API endpoint discovers every member of its +cluster. Pulse previously presented those members by combining the connection +label and native node name, producing rows such as `pve1 (pve2)` and +`pve1 (pve3)`. The display string was assembled downstream and there was no +durable per-member configuration boundary where an operator could replace it. + +A display-only field could not safely be attached to the endpoint name alone. +Native names can change, equal node names can exist in separate clusters, +members can disappear and return, and API-backed nodes can merge with Pulse +agents. Reusing presentation as identity would risk moving metrics, alerts, +credentials, actions, or connection authority to the wrong machine. + +## Disposition + +Each PVE connection now owns a retained cluster-node identity ledger. Migration +assigns deterministic identities compatible with historical node state IDs; +once assigned, an identity is never regenerated. Membership correlation uses +the stored identity first, the Proxmox numeric node ID second, then +unambiguous native-name or address evidence. Native rename, re-IP, reload, +restart, temporary absence, confirmed removal, and later reappearance preserve +the identity and optional display override. Prior native names remain aliases. +Ambiguous name or address evidence fails closed. + +Settings writes an optional, trimmed display name against the immutable +identity. Unicode is supported up to 128 code points, control characters and +invalid UTF-8 are rejected, duplicate display values are allowed, and clearing +the value restores the current native Proxmox name. The override is +connection-scoped, deep-copied across tenant config views, and preserved by +config import/export. + +Monitoring and unified resources project the preferred name through Overview, +Infrastructure Settings, Storage, Backups, alerts and resolved history, +search/filter, REST/websocket payloads, and responsive/mobile resource rows. +Current and prior native names plus numeric and immutable IDs remain available +for diagnostics and search. Display values never participate in configuration +matching, provider scoping, polling, credentials, TLS fingerprints, URLs, +metrics, parent/guest identity, discovery targets, actions, restore matching, +or alert lifecycle keys. + +## Proof + +- configuration migration, persistence, import/export, same-name cluster, + Unicode/case collision, clear, stale-member, rename/re-IP/reappearance, + deep-copy tenant isolation, and immutable identity tests +- API validation, response diagnostics, full update/reload authority, manual + discovery merge, ambiguity fail-closed, connection grouping, contract, and + concurrent immutable-copy tests +- monitoring, model, unified-resource, agent-link, alert/history, + storage/backup, search/filter, settings payload, and responsive presentation + tests +- issue-owned Go race suites; frontend unit, type, lint, and production build + checks; desktop Chromium and mobile browser coverage +- v6 control-plane, status, registry, contract, completion, and readiness + audits + +## Outcome + +The change is on `main` for a future v6 release. It is not retroactively part +of `v6.1.1`, no v5 backport or publication date is claimed, and `#1517` +remains open for reporter confirmation after a release containing the change. + +The deliberate limitation is legacy identity with no recorded numeric Proxmox +node ID whose native name and every known address all change before a later +observation. Pulse treats that observation as a new member instead of guessing +and risking transfer of an override to the wrong node. diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index c674ece3a9..81d0fb3806 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -139,6 +139,13 @@ not change Unified Agent enrollment, token binding, report admission, tombstones, re-enrollment, or command authority. A linked agent remains supplemental evidence on the retained canonical Proxmox member; its presence does not decide provider membership or authorize removal. +That separation also applies to configured cluster-node display overrides. +Infrastructure lifecycle rows may present the connection-owned override and +the current native Proxmox name together, but row identity, linked-agent +attachment, update eligibility, uninstall targeting, enrollment continuity, +and removal tombstones remain keyed by immutable member and agent identities. +Clearing or changing a display override must not detach, re-enroll, remove, or +retarget an agent. The same shared model file may carry monitoring-owned native storage-health evidence such as structured ZFS scans, vdev members, and Ceph health checks. diff --git a/docs/release-control/v6/internal/subsystems/alerts.md b/docs/release-control/v6/internal/subsystems/alerts.md index 901f9a5642..e908f52336 100644 --- a/docs/release-control/v6/internal/subsystems/alerts.md +++ b/docs/release-control/v6/internal/subsystems/alerts.md @@ -376,9 +376,16 @@ active alerts, resolved history, or later UI projections. That same alerts runtime also owns instance-scoped node display-name resolution. Raw node names are not globally unique across configured infrastructure instances, so cached node display names must key on instance + -node identity whenever the alert carries instance context. Alert updates, -incident rebuilds, and guest-metric migrations may fall back to the legacy -name-only cache only for instance-less resources like standalone host agents. +immutable node identity whenever the alert carries instance context. Current +and prior native Proxmox node names may resolve that same entry so an override +or native rename updates active alerts, resolved alerts, and history +presentation without changing the stored diagnostic native resource name or +alert identity. Read-model presentation must snapshot resolved state before +acquiring the active-alert lock to preserve the established lock hierarchy. +Alert updates, incident rebuilds, and guest-metric migrations may fall back to +the legacy name-only cache only for instance-less resources like standalone +host agents. Display names never become acknowledgement, suppression, +rate-limit, flapping, escalation, or notification routing identity. That same host-alert boundary also owns vendor-managed NAS RAID suppression as an alert-lifecycle concern. Shared storagehealth rules decide which Synology or QNAP md arrays are vendor-managed system volumes rather than customer-facing diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 8ac129f9dc..9ee64723c8 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -7113,6 +7113,23 @@ same-token-identity secret refresh and full token promotion onto a credential-less cluster instance; it must not replace working cluster credentials with the member's per-node token, and discovered member `Host` and `IP` stay re-discovery-owned on this path too. +The node update payload also accepts a write-only +`clusterNodeDisplayNameOverrides` collection of +`{nodeIdentity, displayName}` entries. `nodeIdentity` must resolve exactly +inside the selected PVE connection's retained cluster-node identity ledger; +unknown or duplicate request identities fail with `400`. `displayName` is +trimmed, limited to 128 Unicode code points, rejects invalid UTF-8 and control +characters, and an empty value clears the override. Equal or case-equivalent +display values are allowed because presentation is never an identity or +routing key. `GET /api/config/nodes` exposes each endpoint's immutable +`nodeIdentity`, current `nativeName`, prior `nativeAliases`, optional numeric +`nativeNodeId`, and optional `displayName` override, while retaining provider status +`nodeId`, node name, host, address override, credentials, and fingerprint as +separate fields. Existing configuration is migrated deterministically on load +and import; export/import preserves the ledger and stale retained entries. +Tenant config copies must deep-copy the ledger and aliases. Clients may +project the display value but must keep native names and IDs for diagnostics +and must never send a display value as connection or action authority. The same frontend API contract now also governs Proxmox agent-install command transport in `frontend-modern/src/api/nodes.ts`: the canonical client request shape for `/api/agent-install-command` must support both `type:"pve"` and diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index d17cf96e1d..3fa656f3ba 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -1227,6 +1227,16 @@ Agent`), with the plain-language source phrase available through accessible backend-authored cluster members, the table primitive must render those nodes as child composition beneath the cluster row rather than flattening them back into peer top-level systems or hiding them entirely. + The governed connection editor may assign an optional display name to each + PVE cluster member only through its backend-authored immutable + `nodeIdentity`. It must show the current native Proxmox name alongside an + override, explain that clearing restores the native name, and keep the + connection address as a separate control. Optimistic settings projection + may update the display field only; it must not rewrite native name, host, + IP override, credential, fingerprint, node identity, or provider node ID. + Duplicate display values are valid presentation, so row keys, expansion, + search ownership, edit targeting, and grouped-member joins must continue to + use backend identity rather than label text. The same table shell must keep fulfilled rows visible across polling and manual reloads by using a retained-value query boundary, not app-level Suspense or a blank loading replacement, so configured infrastructure does @@ -1357,6 +1367,12 @@ Agent`), with the plain-language source phrase available through accessible than relying on an aria-hidden dot or opacity. Same-named cluster and member rows use Proxmox instance/platform scope for grouping, search ownership, and guest counts; a display label alone is not a cross-provider join key. + Shared resource search must match the preferred display label, current and + prior native Proxmox node names, and immutable node identity. Overview, + storage, backups, alerts/history, infrastructure settings, API/websocket, + and responsive/mobile rows must present the same preferred label while + retaining native diagnostics; page-local formatting such as + `cluster (node)` must not replace the backend-authored presentation. 8. Keep summary chart interaction identity on one shared helper. Summary surfaces that expose row-hover, group-hover, chart-hover, or route-focus-driven chart emphasis must derive page/group/entity scope through `frontend-modern/src/components/shared/summaryCardInteraction.ts` and pass that same resolved scope into card-state, sparkline, and density-map primitives, rather than letting cards read `hovered || focused` while charts listen to a different page-local ID source. Hovering one summary chart must promote that series into the shared active entity so sibling cards highlight the same object instead of keeping chart-local hover islands, and hovering or pinning a workload group header, infrastructure cluster header, or storage pool-group header must scope the matching summary cards through that same shared contract instead of forking a page-local summary filter path. Sibling cards should surface that synchronized hover as one compact header readout through the shared summary-card contract, while the chart under the pointer keeps the only floating tooltip. Recovery is explicitly outside this interaction dialect: its retired posture-card strip must not return with row/group/chart hover behavior without a separate governed product decision. 9. Keep page summaries page-scoped when table rows enter contextual focus. Route-backed row selection may add a focused label and shared series emphasis, but infrastructure, workloads, and storage summary cards must continue to render the page-level series set instead of collapsing the summary down to the selected row or replacing the global trend view with row-local empty states. 10. Keep contextual row focus on the shared summary primitive. Summary surfaces and same-route table drill-ins must reuse `frontend-modern/src/components/shared/contextualFocus.ts` for interactive-series filtering, focused-name lookup, active-series derivation, local scroll preservation, and deliberate inline-detail reveal instead of rebuilding page-local `Set` filters, focused-label scans, drawer-aware scroll math, or ad hoc scroll restoration in each surface. diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 6a86bb230d..8671f4b195 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -85,11 +85,19 @@ does not discard independent recovery facts. Removed host-agent reconnect blocks are identity-scoped: matching may use the canonical host ID or token-qualified machine/hostname continuity, but must never block a distinct live host by hostname alone. -Proxmox cluster node labels are monitoring-owned identity presentation: -configured cluster labels must project into node display names as -`cluster label (node name)` so duplicate node hostnames across clusters remain -distinguishable, while `models.Node.Name` and `ProxmoxData.NodeName` keep the -raw Proxmox node identity for linking, metrics, URLs, and actions. +Proxmox cluster node identity is connection scoped and immutable after first +assignment. Configuration owns one retained identity ledger per PVE instance, +correlating current endpoint membership by the stored identity first, numeric +Proxmox node ID second, then unambiguous native-name or address evidence. +Native rename, re-IP, temporary absence, confirmed removal and later +reappearance must not regenerate an established identity or lose its optional +display-name override. Ambiguous case-folded names or address matches fail +closed instead of transferring identity. Monitoring publishes the configured +override when present and otherwise the current native node name; it also +retains current and prior native names for diagnostics, history lookup, and +search. Presentation must never alter polling addresses, credentials, +fingerprints, external URLs, action routing, source-native guest IDs, or +same-name-cluster provider scoping. Node state aggregation is cluster-identity-scoped beyond presentation: a hostname match alone must never bind a node to a host agent, and neither a shared linked-agent identity nor an endpoint merge alias may fold two node diff --git a/docs/release-control/v6/internal/subsystems/security-privacy.md b/docs/release-control/v6/internal/subsystems/security-privacy.md index 57578a5032..3fdcdce938 100644 --- a/docs/release-control/v6/internal/subsystems/security-privacy.md +++ b/docs/release-control/v6/internal/subsystems/security-privacy.md @@ -23,6 +23,12 @@ and the security-facing settings surfaces that expose authentication posture, token-management visibility, and privacy controls to operators. Customer-facing privacy and Settings surfaces must not present maintainer commercial-event controls as normal product settings. +Configuration-level validation also keeps Proxmox cluster-node display +overrides presentation-only: valid Unicode is bounded to 128 code points, +control characters are rejected, and tenant deep copies must isolate the +retained identity ledger and aliases. Those labels never become authentication +principals, API-token ownership, credential lookup, TLS identity, routing +authority, audit actor identity, or organization scope. ## Canonical Files diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index c92ecf98fc..bbba81d8d9 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -41,6 +41,14 @@ recovery consumers may use the resulting provider-scoped node/instance identity to keep same-named cluster storage distinct, but member retention or confirmed retirement does not fabricate backup evidence, restore points, or storage protection state. +Storage and recovery presentation may consume the Proxmox connection's +cluster-node display override for host, location, parent, backup-task, and +filter labels. Matching remains on immutable node identity plus current or +prior native Proxmox names: a display value must never become a datastore, +archive, backup task, recovery-point, restore, PBS, routing, or credential key. +Search includes both presentation and native diagnostics, duplicate display +values are allowed, and clearing an override deterministically restores the +current native node name. ## Canonical Files diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index f778fadb15..696d8b26f5 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -75,6 +75,18 @@ then leaves the top-level monitored-system projection exactly once so usage and licensing do not count a decommissioned member. Proxmox platform consumers must scope equal cluster labels and node names by provider instance when grouping, searching, and assigning guests or storage. +The same boundary carries cluster-node presentation without weakening +canonical identity. `ProxmoxData` preserves the immutable connection-scoped +node identity, current native node name, prior native-name aliases, and +optional display override as distinct fields. The resource `Name`, +`ParentName`, and agent-linked cluster-node presentation may use the override, +but registry IDs, merge keys, parent links, metrics targets, source-native +guest IDs, external URLs, discovery targets, and actions must not. Children +such as VMs, containers, and storage inherit the parent's presentation and +native aliases so Overview, search, API/websocket/mobile, storage, backups, +and filters agree. A linked agent may enrich the canonical node but cannot +replace a configured override with its hostname; same-name cluster identity +remains provider scoped even after a native rename. Physical-disk resources own cross-source disk identity. When Proxmox inventory and host-agent SMART telemetry describe the same device, the merged resource must retain Proxmox node/instance source payloads while carrying SMART diff --git a/frontend-modern/public/docs/CONFIGURATION.md b/frontend-modern/public/docs/CONFIGURATION.md index 782254ef66..bc89181c3e 100644 --- a/frontend-modern/public/docs/CONFIGURATION.md +++ b/frontend-modern/public/docs/CONFIGURATION.md @@ -272,6 +272,31 @@ PULSE_AGENT_CONNECT_URL=https://agents.example.com:7656 Agents then post telemetry to `https://agents.example.com:7656/api/agents/agent/report` and establish their command channel at `wss://agents.example.com:7656/api/agent/ws`, while the web UI and management API remain reachable only on the private `FRONTEND_PORT` listener. If command execution is enabled, both routes must traverse the same proxy/firewall path; a successful report does not prove that the WebSocket is admitted. +### Proxmox Cluster Node Display Names + +Nodes discovered through one Proxmox VE cluster connection can have an +optional Pulse display name. Open **Settings → Infrastructure**, edit the +Proxmox VE connection, and set **Display name** beside a cluster member. + +- Display names are presentation only. They do not change the Proxmox node + name, API address, credentials, TLS fingerprint, routing, or action target. +- An empty display name uses the current native Proxmox node name. +- Names are trimmed, may contain Unicode, are limited to 128 characters, and + cannot contain control characters. Duplicate display names are allowed + because they are cosmetic; Pulse still uses a separate immutable identity. +- The override is stored with that Proxmox connection and survives reloads, + restarts, member address changes, native node renames, and temporary cluster + membership loss. Confirmed removed members retain their identity metadata so + a later reappearance can recover the override. +- Pulse keeps the native Proxmox node name, prior native names, numeric node ID + when available, and immutable Pulse identity for diagnostics and search. + These fields are also available to API and mobile clients. + +For legacy configuration first seen before Pulse records a Proxmox numeric node +ID, changing both the native node name and every known member address at once +is intentionally treated as a new member. Pulse does not guess across +ambiguous evidence because that could transfer an override to the wrong node. + ### Iframe Embedding (system.json) Embedding is controlled by `system.json` and the UI (**Settings → System → Network**): diff --git a/frontend-modern/src/api/__tests__/nodes.test.ts b/frontend-modern/src/api/__tests__/nodes.test.ts index 04476a9c38..6123f14bef 100644 --- a/frontend-modern/src/api/__tests__/nodes.test.ts +++ b/frontend-modern/src/api/__tests__/nodes.test.ts @@ -47,7 +47,12 @@ describe('NodesAPI', () => { clusterEndpoints: [ { nodeId: ' node-1 ', + nodeIdentity: ' production-pve-1 ', + nativeNodeId: 7, nodeName: ' pve-1 ', + nativeName: ' pve-1 ', + nativeAliases: [' pve-old ', ''], + displayName: ' Render East ', host: ' https://pve-1.local:8006 ', guestURL: ' https://guest.local ', ip: ' 10.0.0.1 ', @@ -71,7 +76,12 @@ describe('NodesAPI', () => { expect(endpoint).toMatchObject({ nodeId: 'node-1', + nodeIdentity: 'production-pve-1', + nativeNodeId: 7, nodeName: 'pve-1', + nativeName: 'pve-1', + nativeAliases: ['pve-old'], + displayName: 'Render East', host: 'https://pve-1.local:8006', guestURL: 'https://guest.local', ip: '10.0.0.1', @@ -201,6 +211,25 @@ describe('NodesAPI', () => { { nodeName: 'pve3', ipOverride: '' }, ]); }); + + it('sends presentation-only cluster node display names by immutable identity', async () => { + vi.mocked(apiFetchJSON).mockResolvedValueOnce({ success: true }); + await NodesAPI.updateNode( + 'pve-1', + makePveNode({ + clusterNodeDisplayNameOverrides: [ + { nodeIdentity: 'production-pve2', displayName: 'Render East' }, + { nodeIdentity: 'production-pve3', displayName: '' }, + ], + }) as NodeConfig, + ); + + const body = JSON.parse(vi.mocked(apiFetchJSON).mock.calls[0][1]!.body as string); + expect(body.clusterNodeDisplayNameOverrides).toEqual([ + { nodeIdentity: 'production-pve2', displayName: 'Render East' }, + { nodeIdentity: 'production-pve3', displayName: '' }, + ]); + }); }); describe('deleteNode', () => { diff --git a/frontend-modern/src/api/connections.ts b/frontend-modern/src/api/connections.ts index dec7aeda5f..d86557b24d 100644 --- a/frontend-modern/src/api/connections.ts +++ b/frontend-modern/src/api/connections.ts @@ -174,7 +174,9 @@ export interface ConnectionSystemComponent { export interface ConnectionSystemMember { id: string; + nodeIdentity?: string; name: string; + nativeName?: string; endpoint?: string; hostAliases?: string[]; state: ConnectionState; diff --git a/frontend-modern/src/api/nodes.ts b/frontend-modern/src/api/nodes.ts index f785bc88cf..404344ab1d 100644 --- a/frontend-modern/src/api/nodes.ts +++ b/frontend-modern/src/api/nodes.ts @@ -13,7 +13,14 @@ type RawClusterEndpoint = Partial; const normalizeClusterEndpoint = (endpoint: RawClusterEndpoint): ClusterEndpoint => ({ nodeId: trimmedString(endpoint.nodeId), + nodeIdentity: trimmedString(endpoint.nodeIdentity), + nativeNodeId: finiteNumberOrUndefined(endpoint.nativeNodeId), nodeName: trimmedString(endpoint.nodeName), + nativeName: optionalTrimmedString(endpoint.nativeName), + nativeAliases: arrayOrUndefined(endpoint.nativeAliases) + ?.map((alias) => trimmedString(alias)) + .filter(Boolean), + displayName: optionalTrimmedString(endpoint.displayName), host: trimmedString(endpoint.host), guestURL: optionalTrimmedString(endpoint.guestURL), ip: trimmedString(endpoint.ip), diff --git a/frontend-modern/src/components/Settings/InfrastructureSourceManager.tsx b/frontend-modern/src/components/Settings/InfrastructureSourceManager.tsx index 3a4171556c..1d7d51fd71 100644 --- a/frontend-modern/src/components/Settings/InfrastructureSourceManager.tsx +++ b/frontend-modern/src/components/Settings/InfrastructureSourceManager.tsx @@ -88,8 +88,15 @@ const rowSystemTitle = (row: InfrastructureSystemRow): string => { return detail ? `${row.name} · ${detail}` : row.name; }; -const memberSystemTitle = (member: InfrastructureSystemMemberRow): string => - member.subtitle ? `${member.name} · ${member.subtitle}` : member.name; +const memberSystemTitle = (member: InfrastructureSystemMemberRow): string => { + const details = [ + member.subtitle, + member.nativeName && member.nativeName !== member.name + ? `Proxmox node ${member.nativeName}` + : undefined, + ].filter(Boolean); + return details.length > 0 ? `${member.name} · ${details.join(' · ')}` : member.name; +}; // The connected-systems table sets `min-w-[820px]`; switch to the card // layout whenever the measured container can't fit that, otherwise the // table renders but overflows horizontally inside the settings panel. diff --git a/frontend-modern/src/components/Settings/NodeModalClusterMembersSection.tsx b/frontend-modern/src/components/Settings/NodeModalClusterMembersSection.tsx index 23423fc3dc..82963d65ca 100644 --- a/frontend-modern/src/components/Settings/NodeModalClusterMembersSection.tsx +++ b/frontend-modern/src/components/Settings/NodeModalClusterMembersSection.tsx @@ -18,10 +18,9 @@ const discoveredAddress = (endpoint: ClusterEndpoint): string => { return host || endpoint.ip || ''; }; -// Per-member connection addresses for an existing PVE cluster. The discovered -// host and IP are rebuilt from cluster status on every re-discovery, so the -// only durable user edit is the override (ClusterEndpoint.ipOverride), which -// wins at poll time via EffectiveIP. +// Per-member presentation and connection settings for an existing PVE +// cluster. Display names are keyed by Pulse's immutable node identity while +// connection overrides remain keyed by the provider's native node name. export const NodeModalClusterMembersSection: Component = ( props, ) => { @@ -43,17 +42,16 @@ export const NodeModalClusterMembersSection: Component

- Pulse found these nodes in the cluster and connects to each one directly. If a node's - discovered address isn't reachable from Pulse (for example when the cluster reports an - internal network IP), enter the address Pulse should use instead. + Give each node an optional display name for Pulse. This never changes its Proxmox name, + identity, credentials, or connection address.

{(endpoint) => ( -
-
+
+
- {endpoint.nodeName} + {endpoint.displayName || endpoint.nodeName}
+ +
+ Proxmox node: {endpoint.nodeName} +
+
Discovered: {discoveredAddress(endpoint)}
-
+
+ +
)}

- Optional. IP or hostname, with a port if it isn't 8006. Leave blank to use the discovered - address. + Leave a display name blank to use the native Proxmox name. Connection addresses are also + optional; enter an IP or hostname, with a port if it isn't 8006, only when Pulse cannot + reach the discovered address.

diff --git a/frontend-modern/src/components/Settings/__tests__/InfrastructureOperationsModel.test.tsx b/frontend-modern/src/components/Settings/__tests__/InfrastructureOperationsModel.test.tsx index c2c82245e2..e07c9fc2e4 100644 --- a/frontend-modern/src/components/Settings/__tests__/InfrastructureOperationsModel.test.tsx +++ b/frontend-modern/src/components/Settings/__tests__/InfrastructureOperationsModel.test.tsx @@ -337,15 +337,21 @@ describe('infrastructure operations model', () => { }); it('mirrors saved cluster member overrides onto cached node config', () => { - // clusterEndpointOverrides is a write-only PUT payload field. The nodes - // cache must apply it to clusterEndpoints (matching the optimistic host - // patch) instead of spreading the raw payload key onto config state. + // Both cluster override collections are write-only PUT fields. The nodes + // cache must apply them to clusterEndpoints instead of spreading either + // raw payload key onto config state. expect(useInfrastructureConfiguredNodesStateSource).toContain( - 'const { clusterEndpointOverrides, ...nodePatch }', + 'clusterNodeDisplayNameOverrides,', ); expect(useInfrastructureConfiguredNodesStateSource).toContain( 'applyClusterEndpointOverridesLocally(', ); + expect(useInfrastructureConfiguredNodesStateSource).toContain( + 'applyClusterNodeDisplayNamesLocally(', + ); + expect(useInfrastructureConfiguredNodesStateSource).toContain( + 'clusterNodeDisplayNameOverrides?: ClusterNodeDisplayNameOverridePayload[]', + ); expect(useInfrastructureConfiguredNodesStateSource).not.toContain('...nodeData,'); }); diff --git a/frontend-modern/src/components/Settings/__tests__/InfrastructureWorkspace.test.tsx b/frontend-modern/src/components/Settings/__tests__/InfrastructureWorkspace.test.tsx index af59d510c7..f2c19e609d 100644 --- a/frontend-modern/src/components/Settings/__tests__/InfrastructureWorkspace.test.tsx +++ b/frontend-modern/src/components/Settings/__tests__/InfrastructureWorkspace.test.tsx @@ -1194,7 +1194,9 @@ describe('InfrastructureWorkspace', () => { members: [ { id: 'node-delly', - name: 'delly', + nodeIdentity: 'homelab-delly', + name: 'Render East', + nativeName: 'delly', subtitle: 'Primary node', source: 'agent', host: 'https://delly:8006', @@ -1240,18 +1242,20 @@ describe('InfrastructureWorkspace', () => { // keep table rows single-line. expect(screen.getByTitle('homelab · Cluster · 2 nodes')).toBeInTheDocument(); expect(screen.queryByText('Fleet OK')).toBeNull(); - expect(screen.queryByTitle('delly · Primary node')).toBeNull(); + expect(screen.queryByTitle('Render East · Primary node · Proxmox node delly')).toBeNull(); fireEvent.click(screen.getByRole('button', { name: 'Show 2 nodes for homelab' })); - expect(screen.getByTitle('delly · Primary node')).toBeInTheDocument(); + expect( + screen.getByTitle('Render East · Primary node · Proxmox node delly'), + ).toBeInTheDocument(); expect(screen.getAllByText('Agent').length).toBeGreaterThan(0); - expect(screen.getByText('delly')).toBeInTheDocument(); + expect(screen.getByText('Render East')).toBeInTheDocument(); expect(screen.getByText('minipc')).toBeInTheDocument(); expect(screen.queryByText('Remote control disabled')).toBeNull(); expect(screen.queryByText('Config pending')).toBeNull(); expect(screen.queryByText('Rollout pending')).toBeNull(); expect(screen.getAllByText('Host telemetry').length).toBeGreaterThan(0); expect(screen.queryByText('Pulse Agent hosts')).toBeNull(); - expect(screen.getAllByText('delly')).toHaveLength(1); + expect(screen.getAllByText('Render East')).toHaveLength(1); expect(screen.getAllByText('minipc')).toHaveLength(1); }); diff --git a/frontend-modern/src/components/Settings/__tests__/nodeModalModel.test.ts b/frontend-modern/src/components/Settings/__tests__/nodeModalModel.test.ts index fb56279a11..bb07c721e8 100644 --- a/frontend-modern/src/components/Settings/__tests__/nodeModalModel.test.ts +++ b/frontend-modern/src/components/Settings/__tests__/nodeModalModel.test.ts @@ -2,12 +2,15 @@ import { describe, expect, it } from 'vitest'; import type { ClusterEndpoint } from '@/types/nodes'; import { applyClusterEndpointOverridesLocally, + applyClusterNodeDisplayNamesLocally, buildClusterEndpointOverridesPayload, + buildClusterNodeDisplayNameOverridesPayload, PVE_MANUAL_PERMISSION_COMMAND, } from '../nodeModalModel'; const endpoint = (nodeName: string, ipOverride?: string): ClusterEndpoint => ({ nodeId: `node/${nodeName}`, + nodeIdentity: `cluster-${nodeName}`, nodeName, host: `https://${nodeName}.local:8006`, ip: '10.0.0.1', @@ -16,6 +19,37 @@ const endpoint = (nodeName: string, ipOverride?: string): ClusterEndpoint => ({ lastSeen: '', }); +describe('cluster node display-name overrides', () => { + it('writes immutable identities, permits duplicate labels, and sends an empty clear', () => { + const endpoints = [ + { ...endpoint('pve1'), displayName: 'Compute' }, + endpoint('pve2'), + ]; + expect( + buildClusterNodeDisplayNameOverridesPayload(endpoints, { + 'cluster-pve1': '', + 'cluster-pve2': 'Compute', + }), + ).toEqual([ + { nodeIdentity: 'cluster-pve1', displayName: '' }, + { nodeIdentity: 'cluster-pve2', displayName: 'Compute' }, + ]); + }); + + it('patches the cached presentation without changing native names or addresses', () => { + const endpoints = [endpoint('pve1')]; + const result = applyClusterNodeDisplayNamesLocally(endpoints, [ + { nodeIdentity: 'cluster-pve1', displayName: 'Render East' }, + ]); + expect(result?.[0]).toMatchObject({ + nodeIdentity: 'cluster-pve1', + nodeName: 'pve1', + displayName: 'Render East', + ip: '10.0.0.1', + }); + }); +}); + describe('nodeModalModel', () => { it('prefers PVE 9 guest-agent privileges before legacy VM.Monitor', () => { expect(PVE_MANUAL_PERMISSION_COMMAND).toContain('VM.GuestAgent.Audit'); diff --git a/frontend-modern/src/components/Settings/__tests__/useConnectionsLedger.test.ts b/frontend-modern/src/components/Settings/__tests__/useConnectionsLedger.test.ts index 7b54a7523f..b72056ef39 100644 --- a/frontend-modern/src/components/Settings/__tests__/useConnectionsLedger.test.ts +++ b/frontend-modern/src/components/Settings/__tests__/useConnectionsLedger.test.ts @@ -199,6 +199,69 @@ describe('useConnectionsLedger', () => { expect(result.rows()[119]).toBe(firstRows[119]); }); + it('invalidates a cached cluster row when immutable or native member identity changes', async () => { + const connection: Connection = { + id: 'pve:cluster', + type: 'pve', + name: 'cluster', + address: 'https://pve1:8006', + state: 'active', + stateReason: '', + enabled: true, + surfaces: ['nodes'], + scope: { nodes: true }, + lastSeen: null, + lastError: null, + source: 'manual', + capabilities: { supportsPause: true, supportsScope: true, supportsTest: true }, + }; + const system = (nodeIdentity: string, nativeName: string): ConnectionSystem => ({ + id: connection.id, + type: 'pve', + clusterName: 'production', + components: [{ connectionId: connection.id, type: 'pve', role: 'primary' }], + members: [ + { + id: 'node', + nodeIdentity, + name: 'Render East', + nativeName, + state: 'active', + }, + ], + }); + const listSpy = vi + .spyOn(ConnectionsAPI, 'list') + .mockResolvedValueOnce({ + connections: [connection], + systems: [system('production-pve-old', 'pve-old')], + }) + .mockResolvedValueOnce({ + connections: [connection], + systems: [system('production-pve-new', 'pve-new')], + }); + + const { result } = renderHook(() => useConnectionsLedger()); + await waitFor(() => expect(result.rows()).toHaveLength(1)); + const firstRow = result.rows()[0]; + expect(firstRow.members[0]).toMatchObject({ + name: 'Render East', + nodeIdentity: 'production-pve-old', + nativeName: 'pve-old', + }); + + result.reload(); + await waitFor(() => expect(listSpy).toHaveBeenCalledTimes(2)); + await waitFor(() => + expect(result.rows()[0].members[0]).toMatchObject({ + name: 'Render East', + nodeIdentity: 'production-pve-new', + nativeName: 'pve-new', + }), + ); + expect(result.rows()[0]).not.toBe(firstRow); + }); + it('retains the fulfilled ledger while a reload is in flight', async () => { const firstConnection: Connection = { id: 'agent:tower', diff --git a/frontend-modern/src/components/Settings/connectionsTableModel.ts b/frontend-modern/src/components/Settings/connectionsTableModel.ts index 526e186f31..bf2348d3a6 100644 --- a/frontend-modern/src/components/Settings/connectionsTableModel.ts +++ b/frontend-modern/src/components/Settings/connectionsTableModel.ts @@ -976,7 +976,9 @@ export interface InfrastructureRowProblem { export interface InfrastructureSystemMemberRow { id: string; + nodeIdentity?: string; name: string; + nativeName?: string; subtitle: string; source: InfrastructureSourceKind; host?: string; diff --git a/frontend-modern/src/components/Settings/nodeModalModel.ts b/frontend-modern/src/components/Settings/nodeModalModel.ts index d84a945a19..bacb559215 100644 --- a/frontend-modern/src/components/Settings/nodeModalModel.ts +++ b/frontend-modern/src/components/Settings/nodeModalModel.ts @@ -1,4 +1,9 @@ -import type { ClusterEndpoint, ClusterEndpointOverridePayload, NodeConfig } from '@/types/nodes'; +import type { + ClusterEndpoint, + ClusterEndpointOverridePayload, + ClusterNodeDisplayNameOverridePayload, + NodeConfig, +} from '@/types/nodes'; import type { SecurityStatus } from '@/types/config'; import type { NodeModalNodeType } from '@/utils/nodeModalPresentation'; @@ -58,6 +63,37 @@ export const applyClusterEndpointOverridesLocally = ( }); }; +export const buildClusterNodeDisplayNameOverridesPayload = ( + endpoints: ClusterEndpoint[] | undefined, + displayNames: Record, +): ClusterNodeDisplayNameOverridePayload[] | undefined => { + if (!endpoints?.length) return undefined; + const changed = endpoints.flatMap((endpoint) => { + const nodeIdentity = endpoint.nodeIdentity; + if (!nodeIdentity) return []; + const value = displayNames[nodeIdentity]; + if (value === undefined || value.trim() === (endpoint.displayName ?? '')) return []; + return [{ nodeIdentity, displayName: value.trim() }]; + }); + return changed.length > 0 ? changed : undefined; +}; + +export const applyClusterNodeDisplayNamesLocally = ( + endpoints: ClusterEndpoint[] | undefined, + overrides: ClusterNodeDisplayNameOverridePayload[] | undefined, +): ClusterEndpoint[] | undefined => { + if (!endpoints?.length || !overrides?.length) return endpoints; + const byIdentity = new Map( + overrides.map((override) => [override.nodeIdentity, override.displayName]), + ); + return endpoints.map((endpoint) => { + if (!endpoint.nodeIdentity) return endpoint; + const value = byIdentity.get(endpoint.nodeIdentity); + if (value === undefined) return endpoint; + return { ...endpoint, displayName: value || undefined }; + }); +}; + export const deriveNameFromHost = (host: string): string => { let value = host.trim(); if (!value) { diff --git a/frontend-modern/src/components/Settings/useConnectionsLedger.ts b/frontend-modern/src/components/Settings/useConnectionsLedger.ts index c3f8c2e45e..19c1888a44 100644 --- a/frontend-modern/src/components/Settings/useConnectionsLedger.ts +++ b/frontend-modern/src/components/Settings/useConnectionsLedger.ts @@ -251,7 +251,9 @@ const systemRowSignature = ( components: system.components, members: (system.members ?? []).map((member) => ({ id: member.id, + nodeIdentity: member.nodeIdentity, name: member.name, + nativeName: member.nativeName, endpoint: member.endpoint, hostAliases: member.hostAliases, state: member.state, @@ -308,7 +310,9 @@ const buildMemberRow = ( primaryRowProblem(fleetHighlights); return { id: member.id, + nodeIdentity: member.nodeIdentity?.trim() || undefined, name, + nativeName: member.nativeName?.trim() || undefined, subtitle: memberSubtitleFor(member, systemType), source, host: member.endpoint?.trim() || undefined, diff --git a/frontend-modern/src/components/Settings/useInfrastructureConfiguredNodesState.ts b/frontend-modern/src/components/Settings/useInfrastructureConfiguredNodesState.ts index 9205d11d3a..9cb1fcde1a 100644 --- a/frontend-modern/src/components/Settings/useInfrastructureConfiguredNodesState.ts +++ b/frontend-modern/src/components/Settings/useInfrastructureConfiguredNodesState.ts @@ -7,11 +7,15 @@ import { getPreferredConfiguredNodeLabel } from '@/utils/resourceIdentity'; import type { Temperature } from '@/types/api'; import type { ClusterEndpointOverridePayload, + ClusterNodeDisplayNameOverridePayload, NodeConfig, NodeConfigWithStatus, } from '@/types/nodes'; import { settingsAgentNodeLabel } from './settingsRouting'; -import { applyClusterEndpointOverridesLocally } from './nodeModalModel'; +import { + applyClusterEndpointOverridesLocally, + applyClusterNodeDisplayNamesLocally, +} from './nodeModalModel'; import { matchConfiguredNodeToResource, type NodeType } from './infrastructureSettingsModel'; import { getNodeDeleteErrorMessage, @@ -291,18 +295,27 @@ export const useInfrastructureConfiguredNodesState = ({ await NodesAPI.updateNode(existingNode.id, nodeData as NodeConfig); // clusterEndpointOverrides is a write-only payload field: mirror it // onto the cached endpoints instead of spreading it onto the node. - const { clusterEndpointOverrides, ...nodePatch } = nodeData as Partial & { + const { + clusterEndpointOverrides, + clusterNodeDisplayNameOverrides, + ...nodePatch + } = nodeData as Partial & { clusterEndpointOverrides?: ClusterEndpointOverridePayload[]; + clusterNodeDisplayNameOverrides?: ClusterNodeDisplayNameOverridePayload[]; }; setNodes( nodes().map((node) => { if (node.id !== existingNode.id) return node; const endpointPatch = - clusterEndpointOverrides?.length && 'clusterEndpoints' in node + (clusterEndpointOverrides?.length || clusterNodeDisplayNameOverrides?.length) && + 'clusterEndpoints' in node ? { - clusterEndpoints: applyClusterEndpointOverridesLocally( - node.clusterEndpoints, - clusterEndpointOverrides, + clusterEndpoints: applyClusterNodeDisplayNamesLocally( + applyClusterEndpointOverridesLocally( + node.clusterEndpoints, + clusterEndpointOverrides, + ), + clusterNodeDisplayNameOverrides, ), } : undefined; diff --git a/frontend-modern/src/components/Settings/useNodeModalState.ts b/frontend-modern/src/components/Settings/useNodeModalState.ts index 552ab3e295..9c0879368d 100644 --- a/frontend-modern/src/components/Settings/useNodeModalState.ts +++ b/frontend-modern/src/components/Settings/useNodeModalState.ts @@ -15,6 +15,7 @@ import { import { buildClusterEndpointOverridesPayload, + buildClusterNodeDisplayNameOverridesPayload, deriveNameFromHost, type NodeModalProps, } from './nodeModalModel'; @@ -323,6 +324,14 @@ export const useNodeModalState = (props: NodeModalProps) => { ]), ) : {}, + clusterNodeDisplayNames: + node.type === 'pve' && 'clusterEndpoints' in node + ? Object.fromEntries( + (node.clusterEndpoints ?? []) + .filter((endpoint) => endpoint.nodeIdentity) + .map((endpoint) => [endpoint.nodeIdentity, endpoint.displayName ?? '']), + ) + : {}, }; const formSourceSignature = JSON.stringify(formSource); @@ -396,6 +405,13 @@ export const useNodeModalState = (props: NodeModalProps) => { if (overrides) { Object.assign(nodeData, { clusterEndpointOverrides: overrides }); } + const displayNameOverrides = buildClusterNodeDisplayNameOverridesPayload( + endpoints, + data.clusterNodeDisplayNames, + ); + if (displayNameOverrides) { + Object.assign(nodeData, { clusterNodeDisplayNameOverrides: displayNameOverrides }); + } } props.onSave(nodeData); @@ -436,6 +452,13 @@ export const useNodeModalState = (props: NodeModalProps) => { })); }; + const updateClusterNodeDisplayName = (nodeIdentity: string, value: string) => { + setFormData((prev) => ({ + ...prev, + clusterNodeDisplayNames: { ...prev.clusterNodeDisplayNames, [nodeIdentity]: value }, + })); + }; + const handleTestConnection = async () => { const data = formData(); const normalizedName = data.name.trim() || deriveNameFromHost(data.host); @@ -555,6 +578,7 @@ export const useNodeModalState = (props: NodeModalProps) => { testResult, testResultPresentation, updateClusterEndpointOverride, + updateClusterNodeDisplayName, updateField, }; }; diff --git a/frontend-modern/src/features/proxmox/__tests__/ProxmoxNodesTable.test.tsx b/frontend-modern/src/features/proxmox/__tests__/ProxmoxNodesTable.test.tsx index 7b5302cec9..d81c7fae19 100644 --- a/frontend-modern/src/features/proxmox/__tests__/ProxmoxNodesTable.test.tsx +++ b/frontend-modern/src/features/proxmox/__tests__/ProxmoxNodesTable.test.tsx @@ -146,6 +146,35 @@ describe('ProxmoxNodesTable', () => { ); }); + it('shows the configured display name without using it as the native web target', () => { + render(() => ( + } + emptyTitle="No Proxmox VE nodes" + emptyDescription="No nodes" + /> + )); + + const link = screen.getByRole('link', { name: 'Open web interface for Render East' }); + expect(screen.getByText('Render East')).toBeInTheDocument(); + expect(link).toHaveAttribute('href', 'https://pve1:8006'); + }); + it('passes alert-backed temperature thresholds into the node temperature gauge', () => { render(() => ( { // --------------------------------------------------------------------------- describe('matchWorkloadByHints branches (via vzdump tasks)', () => { + it('matches prior native node aliases while presenting the current display name', () => { + const model = buildModel({ + workloads: [ + workload({ + id: 'vm-419a', + proxmox: { + vmid: 419, + nodeName: 'pve-new', + nodeAliases: ['pve-old'], + nodeDisplayName: 'Render East', + }, + }), + workload({ id: 'vm-419b', proxmox: { vmid: 419, nodeName: 'pve-other' } }), + ], + tasks: [task({ id: 't-419', type: 'vzdump', vmid: 419, node: 'pve-old', instance: '' })], + }); + const renamed = model.coverageRows.find((row) => row.workload.node === 'Render East'); + expect(renamed?.latestTask?.label).toBe('OK'); + expect(renamed?.workload.nativeNode).toBe('pve-new'); + }); + it('returns the hint-matched candidate among several same-vmid workloads', () => { const model = buildModel({ workloads: [ diff --git a/frontend-modern/src/features/proxmox/proxmoxBackupRecoveryModel.ts b/frontend-modern/src/features/proxmox/proxmoxBackupRecoveryModel.ts index c315aed5b2..dee38ede44 100644 --- a/frontend-modern/src/features/proxmox/proxmoxBackupRecoveryModel.ts +++ b/frontend-modern/src/features/proxmox/proxmoxBackupRecoveryModel.ts @@ -20,7 +20,11 @@ export interface WorkloadReference { vmid: string; label: string; name?: string; + /** Preferred presentation label for the hosting node. */ node?: string; + /** Provider-native node name retained for matching and diagnostics. */ + nativeNode?: string; + nativeNodeAliases?: string[]; instance?: string; } @@ -209,6 +213,10 @@ function resourceNode(resource: Resource): string | undefined { return resource.proxmox?.nodeName || resource.proxmox?.node || resource.parentName || undefined; } +function resourceNodeDisplayName(resource: Resource): string | undefined { + return resource.proxmox?.nodeDisplayName || resource.parentName || resourceNode(resource); +} + function resourceInstance(resource: Resource): string | undefined { const fromMeta = resource.proxmox?.instance; if (fromMeta) return fromMeta; @@ -236,7 +244,9 @@ function buildCandidateFromResource(resource: Resource): WorkloadCandidate | nul const type = resourceBackupType(resource); if (!vmid || isZeroWorkloadId(vmid) || type === 'unknown') return null; const name = resource.displayName || resource.name || undefined; - const node = resourceNode(resource); + const nativeNode = resourceNode(resource); + const node = resourceNodeDisplayName(resource); + const nativeNodeAliases = resource.proxmox?.nodeAliases ?? []; const instance = resourceInstance(resource); return { key: `resource:${resource.id}`, @@ -247,8 +257,10 @@ function buildCandidateFromResource(resource: Resource): WorkloadCandidate | nul label: buildWorkloadLabel(name, type, vmid), name, node, + nativeNode, + nativeNodeAliases, instance, - nodeKey: normalizeKey(node), + nodeKey: normalizeKey(nativeNode), instanceKey: normalizeKey(instance), }; } @@ -268,6 +280,7 @@ function fallbackWorkload( vmid: id, label: workloadFallbackLabel(type, id), node: type === 'host' ? undefined : hints.find((hint) => !!hint?.trim()), + nativeNode: type === 'host' ? undefined : hints.find((hint) => !!hint?.trim()), }; } @@ -295,6 +308,8 @@ function resolveWorkload( hint === candidate.nodeKey || hint === candidate.instanceKey || normalizeKey(candidate.node).includes(hint) || + normalizeKey(candidate.nativeNode).includes(hint) || + candidate.nativeNodeAliases?.some((alias) => normalizeKey(alias).includes(hint)) || normalizeKey(candidate.instance).includes(hint), ), ); @@ -314,8 +329,11 @@ function matchWorkloadByHints( normalizedHints.some( (hint) => hint === normalizeKey(workload.node) || + hint === normalizeKey(workload.nativeNode) || hint === normalizeKey(workload.instance) || normalizeKey(workload.node).includes(hint) || + normalizeKey(workload.nativeNode).includes(hint) || + workload.nativeNodeAliases?.some((alias) => normalizeKey(alias).includes(hint)) || normalizeKey(workload.instance).includes(hint), ), ); @@ -596,6 +614,8 @@ export function coverageRowMatchesSearch(row: WorkloadCoverageRow, term: string) row.workload.typeLabel, row.workload.vmid, row.workload.node, + row.workload.nativeNode, + ...(row.workload.nativeNodeAliases ?? []), row.workload.instance, getWorkloadRecoveryPostureLabel(row.posture), row.latestTask?.label, @@ -622,6 +642,8 @@ export function recoverableArtifactMatchesSearch( artifact.workload.typeLabel, artifact.workload.vmid, artifact.workload.node, + artifact.workload.nativeNode, + ...(artifact.workload.nativeNodeAliases ?? []), artifact.workload.instance, artifact.sourceLabel, artifact.sourceTitle, diff --git a/frontend-modern/src/features/storageBackups/__tests__/storageAdapters.test.ts b/frontend-modern/src/features/storageBackups/__tests__/storageAdapters.test.ts index 3d39bf3d44..6ef682be92 100644 --- a/frontend-modern/src/features/storageBackups/__tests__/storageAdapters.test.ts +++ b/frontend-modern/src/features/storageBackups/__tests__/storageAdapters.test.ts @@ -118,6 +118,35 @@ describe('storageAdapters', () => { ); }); + it('presents a node override while retaining native storage routing hints', () => { + const records = buildStorageRecords({ + state: baseState(), + resources: [ + makeResourceStorage({ + id: 'resource-storage-custom-node', + parentId: 'production-pve1', + parentName: 'Render East', + platformData: { + type: 'dir', + proxmox: { + nodeIdentity: 'production-pve1', + nodeName: 'pve1', + nodeAliases: ['pve-old'], + nodeDisplayName: 'Render East', + }, + }, + }), + ], + }); + + expect(records).toHaveLength(1); + expect(records[0].location.label).toBe('Render East'); + expect(records[0].details?.node).toBe('pve1'); + expect(records[0].details?.nodeHints).toEqual( + expect.arrayContaining(['Render East', 'pve1', 'pve-old', 'production-pve1']), + ); + }); + it('collapses duplicate resource records by canonical identity and merges capabilities/details', () => { const resources: Resource[] = [ makeResourceStorage({ diff --git a/frontend-modern/src/features/storageBackups/storageAdapters.ts b/frontend-modern/src/features/storageBackups/storageAdapters.ts index 773ce5658a..0924119aca 100644 --- a/frontend-modern/src/features/storageBackups/storageAdapters.ts +++ b/frontend-modern/src/features/storageBackups/storageAdapters.ts @@ -76,16 +76,32 @@ const mapResourceStorageRecord = (resource: Resource, adapterId: string): Storag platformData, isBackupRepository ? 'backup' : '', ); - const proxmoxNode = + const proxmoxPlatform = platformData.proxmox as Record | undefined; + const proxmoxNativeNode = + resource.proxmox?.nodeName || resource.proxmox?.node || - ((platformData.proxmox as Record | undefined)?.nodeName as string | undefined); + (proxmoxPlatform?.nodeName as string | undefined); + const proxmoxNodeDisplay = + resource.proxmox?.nodeDisplayName || + (proxmoxPlatform?.nodeDisplayName as string | undefined) || + resource.parentName || + proxmoxNativeNode; + const proxmoxNodeAliases = [ + ...(resource.proxmox?.nodeAliases ?? []), + ...getStringArray(proxmoxPlatform?.nodeAliases), + ]; + const proxmoxNodeIdentity = + resource.proxmox?.nodeIdentity || (proxmoxPlatform?.nodeIdentity as string | undefined); const storageNodes = getStringArray( (resource.storage as Record | undefined)?.nodes, ); const nodeHints = dedupe( [ resource.parentName, - proxmoxNode, + proxmoxNodeDisplay, + proxmoxNativeNode, + ...proxmoxNodeAliases, + proxmoxNodeIdentity, platformData.node as string | undefined, ...storageNodes, resource.parentId, @@ -95,14 +111,14 @@ const mapResourceStorageRecord = (resource: Resource, adapterId: string): Storag const locationLabel = isBackupRepository ? resource.parentName || (platformData.pbsInstanceName as string | undefined) || - proxmoxNode || + proxmoxNodeDisplay || (platformData.node as string | undefined) || storageNodes[0] || resource.parentId || resource.platformId || 'Unknown' : resource.parentName || - proxmoxNode || + proxmoxNodeDisplay || (platformData.node as string | undefined) || storageNodes[0] || resource.parentId || @@ -192,7 +208,7 @@ const mapResourceStorageRecord = (resource: Resource, adapterId: string): Storag status: statusLabel, parentId: resource.parentId, parentName: resource.parentName, - node: proxmoxNode || (platformData.node as string | undefined) || storageNodes[0], + node: proxmoxNativeNode || (platformData.node as string | undefined) || storageNodes[0], nodeHints, hostLabel, platformLabel, diff --git a/frontend-modern/src/types/nodes.ts b/frontend-modern/src/types/nodes.ts index 6ad449da01..527bb772ff 100644 --- a/frontend-modern/src/types/nodes.ts +++ b/frontend-modern/src/types/nodes.ts @@ -7,7 +7,12 @@ export type TemperatureTransport = export interface ClusterEndpoint { nodeId: string; + nodeIdentity?: string; + nativeNodeId?: number; nodeName: string; + nativeName?: string; + nativeAliases?: string[]; + displayName?: string; host: string; guestURL?: string; ip: string; @@ -29,6 +34,13 @@ export interface ClusterEndpointOverridePayload { ipOverride: string; } +// Write-only PUT payload entry. The empty displayName clears the override and +// restores the provider's native node name. +export interface ClusterNodeDisplayNameOverridePayload { + nodeIdentity: string; + displayName: string; +} + export interface PVENodeConfig { id: string; name: string; @@ -55,6 +67,8 @@ export interface PVENodeConfig { // Write-only: per-member connection address overrides included in PUT // payloads; never returned by the API. clusterEndpointOverrides?: ClusterEndpointOverridePayload[]; + // Write-only presentation overrides keyed by immutable node identity. + clusterNodeDisplayNameOverrides?: ClusterNodeDisplayNameOverridePayload[]; } export interface PBSNodeConfig { diff --git a/frontend-modern/src/types/resource.ts b/frontend-modern/src/types/resource.ts index 026fea5fd3..7f07f7c76c 100644 --- a/frontend-modern/src/types/resource.ts +++ b/frontend-modern/src/types/resource.ts @@ -673,9 +673,14 @@ export interface ResourceProxmoxMeta { sourceId?: string; /** Proxmox-authored VM/LXC power state, kept separate from collection freshness. */ runtimeStatus?: string; + /** Pulse-owned immutable identity for the node within one PVE connection. */ + nodeIdentity?: string; vmid?: number; node?: string; nodeName?: string; + nodeAliases?: string[]; + /** Preferred node label; nodeName remains the provider-native diagnostic key. */ + nodeDisplayName?: string; pool?: string; instance?: string; clusterName?: string; diff --git a/frontend-modern/src/utils/__tests__/nodeModalPresentation.branchcov0718.test.ts b/frontend-modern/src/utils/__tests__/nodeModalPresentation.branchcov0718.test.ts index 973f5b3d29..a14ad7a284 100644 --- a/frontend-modern/src/utils/__tests__/nodeModalPresentation.branchcov0718.test.ts +++ b/frontend-modern/src/utils/__tests__/nodeModalPresentation.branchcov0718.test.ts @@ -245,6 +245,7 @@ describe('getNodeModalDefaultFormData branch coverage', () => { monitorQuarantine: true, monitorDomainStats: false, clusterEndpointOverrides: {}, + clusterNodeDisplayNames: {}, }); }); @@ -280,6 +281,7 @@ describe('getNodeModalDefaultFormData branch coverage', () => { monitorQuarantine: true, monitorDomainStats: false, clusterEndpointOverrides: {}, + clusterNodeDisplayNames: {}, }); }); @@ -315,6 +317,7 @@ describe('getNodeModalDefaultFormData branch coverage', () => { monitorQuarantine: true, monitorDomainStats: false, clusterEndpointOverrides: {}, + clusterNodeDisplayNames: {}, }); }); }); diff --git a/frontend-modern/src/utils/__tests__/nodeModalPresentation.test.ts b/frontend-modern/src/utils/__tests__/nodeModalPresentation.test.ts index f3d09518fa..930517ad5d 100644 --- a/frontend-modern/src/utils/__tests__/nodeModalPresentation.test.ts +++ b/frontend-modern/src/utils/__tests__/nodeModalPresentation.test.ts @@ -21,6 +21,7 @@ describe('nodeModalPresentation', () => { verifySSL: true, monitorPhysicalDisks: false, clusterEndpointOverrides: {}, + clusterNodeDisplayNames: {}, }); expect(getNodeModalDefaultFormData('pbs')).toMatchObject({ authType: 'token', diff --git a/frontend-modern/src/utils/__tests__/resourceSearchMatch.test.ts b/frontend-modern/src/utils/__tests__/resourceSearchMatch.test.ts index d4e231caf8..c2fd650338 100644 --- a/frontend-modern/src/utils/__tests__/resourceSearchMatch.test.ts +++ b/frontend-modern/src/utils/__tests__/resourceSearchMatch.test.ts @@ -59,6 +59,21 @@ describe('resourceMatchesSearch', () => { }); }); + it('matches both custom and provider-native Proxmox node names', () => { + const resource = makeResource({ + proxmox: { + nodeIdentity: 'cluster-pve1', + nodeName: 'pve1', + nodeAliases: ['pve-old'], + nodeDisplayName: 'Render East', + }, + }); + expect(resourceMatchesSearch(resource, 'render east')).toBe(true); + expect(resourceMatchesSearch(resource, 'pve1')).toBe(true); + expect(resourceMatchesSearch(resource, 'cluster-pve1')).toBe(true); + expect(resourceMatchesSearch(resource, 'pve-old')).toBe(true); + }); + describe('canonicalIdentity fields', () => { it.each([ ['displayName', { canonicalIdentity: { displayName: 'Canonical Box' } }, 'canonical'], diff --git a/frontend-modern/src/utils/nodeModalPresentation.ts b/frontend-modern/src/utils/nodeModalPresentation.ts index 0759e37031..8dab96cc64 100644 --- a/frontend-modern/src/utils/nodeModalPresentation.ts +++ b/frontend-modern/src/utils/nodeModalPresentation.ts @@ -36,6 +36,9 @@ export interface NodeModalFormData { // PVE clusters: per-member connection address overrides keyed by node name. // Empty string means "use the discovered address". clusterEndpointOverrides: Record; + // PVE clusters: optional labels keyed by immutable node identity. + // Empty string means "use the native Proxmox node name". + clusterNodeDisplayNames: Record; } export interface NodeModalTestResultPresentation { @@ -75,6 +78,7 @@ export function getNodeModalDefaultFormData(nodeType: NodeModalNodeType): NodeMo monitorQuarantine: true, monitorDomainStats: false, clusterEndpointOverrides: {}, + clusterNodeDisplayNames: {}, }; } diff --git a/frontend-modern/src/utils/resourceSearchMatch.ts b/frontend-modern/src/utils/resourceSearchMatch.ts index 8397146362..0d96aeb430 100644 --- a/frontend-modern/src/utils/resourceSearchMatch.ts +++ b/frontend-modern/src/utils/resourceSearchMatch.ts @@ -44,6 +44,9 @@ const collectSearchCandidates = (resource: Resource): string[] => { resource.vmware?.datacenterName, resource.proxmox?.node, resource.proxmox?.nodeName, + resource.proxmox?.nodeDisplayName, + resource.proxmox?.nodeIdentity, + ...(resource.proxmox?.nodeAliases ?? []), resource.proxmox?.clusterName, ...(resource.docker?.repoTags ?? []), ...(resource.docker?.repoDigests ?? []), diff --git a/internal/alerts/alerts_test.go b/internal/alerts/alerts_test.go index 3f427bdd09..a2ab356ed5 100644 --- a/internal/alerts/alerts_test.go +++ b/internal/alerts/alerts_test.go @@ -327,6 +327,26 @@ func TestNodeDisplayNameCacheKeyTrimsParts(t *testing.T) { } } +func TestAlertHistoryUsesDisplayNameForPriorNativeAlias(t *testing.T) { + m := newTestManager(t) + node := models.Node{ + ID: "production-pve-old", NodeIdentity: "production-pve-old", + Name: "pve-new", NativeNameAliases: []string{"pve-old"}, + DisplayName: "Render East", Instance: "cluster-api", + } + m.CheckNode(node) + + history := m.applyCurrentNodeDisplayNames([]Alert{{ + Node: "pve-old", Instance: "cluster-api", NodeDisplayName: "Old label", + }}) + if history[0].NodeDisplayName != "Render East" { + t.Fatalf("historical alias display name = %q, want Render East", history[0].NodeDisplayName) + } + if history[0].Node != "pve-old" { + t.Fatalf("history diagnostic native name changed: %+v", history[0]) + } +} + func TestHostResourceIDTrimsID(t *testing.T) { if got := hostResourceID(" host-1 "); got != "agent:host-1" { t.Fatalf("hostResourceID() = %q, want %q", got, "agent:host-1") diff --git a/internal/alerts/node.go b/internal/alerts/node.go index 4ee9934415..24d2c664ff 100644 --- a/internal/alerts/node.go +++ b/internal/alerts/node.go @@ -15,6 +15,9 @@ import ( func (m *Manager) CheckNode(node models.Node) { // Cache display name so all alerts (including guest alerts on this node) can resolve it. m.UpdateNodeDisplayName(node.Instance, node.Name, node.DisplayName) + for _, alias := range node.NativeNameAliases { + m.UpdateNodeDisplayName(node.Instance, alias, node.DisplayName) + } m.mu.RLock() if !m.config.Enabled { diff --git a/internal/alerts/read_model.go b/internal/alerts/read_model.go index 9816fc7e19..c168c54d04 100644 --- a/internal/alerts/read_model.go +++ b/internal/alerts/read_model.go @@ -264,25 +264,39 @@ func (m *Manager) GetRecentlyResolved() []models.ResolvedAlert { } m.resolvedMutex.RLock() - defer m.resolvedMutex.RUnlock() - - resolved := make([]models.ResolvedAlert, 0, len(m.recentlyResolved)) + resolvedSnapshot := make([]*ResolvedAlert, 0, len(m.recentlyResolved)) for _, alert := range m.recentlyResolved { + resolvedSnapshot = append(resolvedSnapshot, &ResolvedAlert{ + Alert: cloneAlertForOutput(alert.Alert), + ResolvedTime: alert.ResolvedTime, + }) + } + m.resolvedMutex.RUnlock() + + resolved := make([]models.ResolvedAlert, 0, len(resolvedSnapshot)) + m.mu.RLock() + defer m.mu.RUnlock() + for _, alert := range resolvedSnapshot { exported := cloneAlertForOutput(alert.Alert) + nodeDisplayName := exported.NodeDisplayName + if current := m.resolveNodeDisplayName(exported.Instance, exported.Node); current != "" { + nodeDisplayName = current + } resolved = append(resolved, models.ResolvedAlert{ Alert: models.Alert{ - ID: exported.ID, - Type: exported.Type, - Level: string(exported.Level), - ResourceID: exported.ResourceID, - ResourceName: exported.ResourceName, - Node: exported.Node, - Instance: exported.Instance, - Message: exported.Message, - Value: exported.Value, - Threshold: exported.Threshold, - StartTime: exported.StartTime, - Acknowledged: exported.Acknowledged, + ID: exported.ID, + Type: exported.Type, + Level: string(exported.Level), + ResourceID: exported.ResourceID, + ResourceName: exported.ResourceName, + Node: exported.Node, + NodeDisplayName: nodeDisplayName, + Instance: exported.Instance, + Message: exported.Message, + Value: exported.Value, + Threshold: exported.Threshold, + StartTime: exported.StartTime, + Acknowledged: exported.Acknowledged, // exported is a deep clone, so the map is already private. Metadata: exported.Metadata, }, @@ -298,22 +312,27 @@ func (m *Manager) GetResolvedAlert(alertID string) *ResolvedAlert { // canonical-identity miss, so concurrent read-locked callers would race // each other on that write. m.resolvedMutex.Lock() - defer m.resolvedMutex.Unlock() - resolved, ok := m.getResolvedAlertNoLock(alertID) if !ok || resolved == nil || resolved.Alert == nil { + m.resolvedMutex.Unlock() return nil } - - return &ResolvedAlert{ + result := &ResolvedAlert{ Alert: cloneAlertForOutput(resolved.Alert), ResolvedTime: resolved.ResolvedTime, } + m.resolvedMutex.Unlock() + m.mu.RLock() + if current := m.resolveNodeDisplayName(result.Alert.Instance, result.Alert.Node); current != "" { + result.Alert.NodeDisplayName = current + } + m.mu.RUnlock() + return result } // GetAlertHistory returns alert history func (m *Manager) GetAlertHistory(limit int) []Alert { - return canonicalizeAlertHistoryForOutput(m.historyManager.GetAllHistory(limit)) + return m.applyCurrentNodeDisplayNames(canonicalizeAlertHistoryForOutput(m.historyManager.GetAllHistory(limit))) } // GetAlertHistorySince returns alert history entries created after the provided time. @@ -322,7 +341,18 @@ func (m *Manager) GetAlertHistorySince(since time.Time, limit int) []Alert { return m.GetAlertHistory(limit) } - return canonicalizeAlertHistoryForOutput(m.historyManager.GetHistory(since, limit)) + return m.applyCurrentNodeDisplayNames(canonicalizeAlertHistoryForOutput(m.historyManager.GetHistory(since, limit))) +} + +func (m *Manager) applyCurrentNodeDisplayNames(alerts []Alert) []Alert { + m.mu.RLock() + defer m.mu.RUnlock() + for idx := range alerts { + if displayName := m.resolveNodeDisplayName(alerts[idx].Instance, alerts[idx].Node); displayName != "" { + alerts[idx].NodeDisplayName = displayName + } + } + return alerts } // ClearAlertHistory clears all alert history diff --git a/internal/api/config_handlers.go b/internal/api/config_handlers.go index 55b1816df8..6776733b67 100644 --- a/internal/api/config_handlers.go +++ b/internal/api/config_handlers.go @@ -408,6 +408,7 @@ func (h *ConfigHandlers) normalizePVEConfigState(ctx context.Context) { if normalized, changed := config.ConsolidatePVEInstances(cfg.PVEInstances); changed { cfg.PVEInstances = normalized } + config.EnsurePVEClusterNodeIdentities(cfg.PVEInstances) } // cleanupExpiredSetupTokens removes expired or used setup tokens periodically. @@ -730,9 +731,10 @@ type NodeConfigRequest struct { Enabled *bool `json:"enabled,omitempty"` // Lifecycle toggle; nil on update preserves current // PVE only: per-cluster-member connection address overrides. Only the // members named here are touched; an empty ipOverride clears the override. - ClusterEndpointOverrides []ClusterEndpointOverrideRequest `json:"clusterEndpointOverrides,omitempty"` - guestURLSet bool `json:"-"` - fingerprintSet bool `json:"-"` + ClusterEndpointOverrides []ClusterEndpointOverrideRequest `json:"clusterEndpointOverrides,omitempty"` + ClusterNodeDisplayNameOverrides []ClusterNodeDisplayNameOverrideRequest `json:"clusterNodeDisplayNameOverrides,omitempty"` + guestURLSet bool `json:"-"` + fingerprintSet bool `json:"-"` } // ClusterEndpointOverrideRequest sets or clears the connection address Pulse @@ -744,6 +746,13 @@ type ClusterEndpointOverrideRequest struct { IPOverride string `json:"ipOverride"` } +// ClusterNodeDisplayNameOverrideRequest sets or clears a presentation-only +// name using the immutable, connection-scoped node identity. +type ClusterNodeDisplayNameOverrideRequest struct { + NodeIdentity string `json:"nodeIdentity"` + DisplayName string `json:"displayName"` +} + func (r *NodeConfigRequest) UnmarshalJSON(data []byte) error { type nodeConfigRequestJSON NodeConfigRequest @@ -864,7 +873,12 @@ func (r NodeResponse) NormalizeCollections() NodeResponse { // It keeps config storage structs internal while returning canonical JSON keys. type ClusterEndpointResponse struct { NodeID string `json:"nodeId,omitempty"` + NodeIdentity string `json:"nodeIdentity"` + NativeNodeID int `json:"nativeNodeId,omitempty"` NodeName string `json:"nodeName,omitempty"` + NativeName string `json:"nativeName,omitempty"` + NativeAliases []string `json:"nativeAliases,omitempty"` + DisplayName string `json:"displayName,omitempty"` Host string `json:"host,omitempty"` GuestURL string `json:"guestURL,omitempty"` IP string `json:"ip,omitempty"` @@ -877,10 +891,31 @@ type ClusterEndpointResponse struct { PulseError string `json:"pulseError,omitempty"` } -func toClusterEndpointResponse(endpoint config.ClusterEndpoint) ClusterEndpointResponse { +func toClusterEndpointResponse(endpoint config.ClusterEndpoint, identitySets ...[]config.PVEClusterNodeIdentity) ClusterEndpointResponse { + displayName := "" + nativeName := endpoint.NodeName + var nativeAliases []string + for _, identities := range identitySets { + for _, identity := range identities { + if identity.ID != endpoint.NodeIdentity { + continue + } + displayName = identity.DisplayName + nativeAliases = append([]string(nil), identity.NativeAliases...) + if identity.NativeName != "" { + nativeName = identity.NativeName + } + break + } + } return ClusterEndpointResponse{ NodeID: endpoint.NodeID, + NodeIdentity: endpoint.NodeIdentity, + NativeNodeID: endpoint.NativeNodeID, NodeName: endpoint.NodeName, + NativeName: nativeName, + NativeAliases: nativeAliases, + DisplayName: displayName, Host: endpoint.Host, GuestURL: endpoint.GuestURL, IP: endpoint.IP, @@ -894,14 +929,14 @@ func toClusterEndpointResponse(endpoint config.ClusterEndpoint) ClusterEndpointR } } -func toClusterEndpointResponses(endpoints []config.ClusterEndpoint) []ClusterEndpointResponse { +func toClusterEndpointResponses(endpoints []config.ClusterEndpoint, identitySets ...[]config.PVEClusterNodeIdentity) []ClusterEndpointResponse { if len(endpoints) == 0 { return []ClusterEndpointResponse{} } out := make([]ClusterEndpointResponse, 0, len(endpoints)) for _, endpoint := range endpoints { - out = append(out, toClusterEndpointResponse(endpoint)) + out = append(out, toClusterEndpointResponse(endpoint, identitySets...)) } return out } @@ -1340,6 +1375,7 @@ func defaultDetectPVECluster(clientConfig proxmox.ClientConfig, nodeName string, pulseReachable := isValid endpoint := config.ClusterEndpoint{ NodeID: clusterNode.ID, + NativeNodeID: clusterNode.Nodeid, NodeName: clusterNode.Name, GuestURL: findExistingGuestURL(clusterNode.Name, existingEndpoints), IPOverride: findExistingIPOverride(clusterNode.Name, existingEndpoints), // Preserve user override @@ -1452,7 +1488,7 @@ func (h *ConfigHandlers) GetAllNodesForAPI(ctx context.Context) []NodeResponse { Status: h.getNodeStatus(ctx, "pve", pve.Name), IsCluster: pve.IsCluster, ClusterName: pve.ClusterName, - ClusterEndpoints: toClusterEndpointResponses(pve.ClusterEndpoints), + ClusterEndpoints: toClusterEndpointResponses(pve.ClusterEndpoints, pve.ClusterNodeIdentities), Source: pve.Source, }.NormalizeCollections() nodes = append(nodes, node) diff --git a/internal/api/config_node_display_name_test.go b/internal/api/config_node_display_name_test.go new file mode 100644 index 0000000000..98124b54b7 --- /dev/null +++ b/internal/api/config_node_display_name_test.go @@ -0,0 +1,239 @@ +package api + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" +) + +func displayNameTestInstance() config.PVEInstance { + return config.PVEInstance{ + Name: "cluster", + IsCluster: true, + ClusterEndpoints: []config.ClusterEndpoint{ + {NodeName: "pve1", NodeIdentity: "cluster-pve1"}, + {NodeName: "pve2", NodeIdentity: "cluster-pve2"}, + }, + ClusterNodeIdentities: []config.PVEClusterNodeIdentity{ + {ID: "cluster-pve1", NativeName: "pve1"}, + {ID: "cluster-pve2", NativeName: "pve2"}, + }, + } +} + +func TestApplyClusterNodeDisplayNameOverridesSupportsUnicodeDuplicatesAndClearing(t *testing.T) { + instance := displayNameTestInstance() + updated, err := applyClusterNodeDisplayNameOverrides(instance, []ClusterNodeDisplayNameOverrideRequest{ + {NodeIdentity: "cluster-pve1", DisplayName: " Rendu \u00c9tage "}, + {NodeIdentity: "cluster-pve2", DisplayName: "Rendu \u00c9tage"}, + }) + if err != nil { + t.Fatalf("apply display names: %v", err) + } + if updated.ClusterNodeIdentities[0].DisplayName != "Rendu \u00c9tage" || + updated.ClusterNodeIdentities[1].DisplayName != "Rendu \u00c9tage" { + t.Fatalf("cosmetic duplicate Unicode labels should be allowed: %+v", updated.ClusterNodeIdentities) + } + if instance.ClusterNodeIdentities[0].DisplayName != "" { + t.Fatal("display-name update mutated the caller's config slice") + } + + cleared, err := applyClusterNodeDisplayNameOverrides(updated, []ClusterNodeDisplayNameOverrideRequest{{ + NodeIdentity: "cluster-pve1", + DisplayName: "", + }}) + if err != nil || cleared.ClusterNodeIdentities[0].DisplayName != "" { + t.Fatalf("clear override failed: identities=%+v err=%v", cleared.ClusterNodeIdentities, err) + } +} + +func TestApplyClusterNodeDisplayNameOverridesRejectsStaleAndAmbiguousRequests(t *testing.T) { + instance := displayNameTestInstance() + cases := []struct { + name string + overrides []ClusterNodeDisplayNameOverrideRequest + errorText string + }{ + { + name: "unknown identity", + overrides: []ClusterNodeDisplayNameOverrideRequest{{NodeIdentity: "missing", DisplayName: "Name"}}, + errorText: "unknown cluster node identity", + }, + { + name: "duplicate request", + overrides: []ClusterNodeDisplayNameOverrideRequest{ + {NodeIdentity: "cluster-pve1", DisplayName: "A"}, + {NodeIdentity: "cluster-pve1", DisplayName: "B"}, + }, + errorText: "provided more than once", + }, + { + name: "control character", + overrides: []ClusterNodeDisplayNameOverrideRequest{{NodeIdentity: "cluster-pve1", DisplayName: "bad\nname"}}, + errorText: "control characters", + }, + { + name: "too long", + overrides: []ClusterNodeDisplayNameOverrideRequest{{ + NodeIdentity: "cluster-pve1", + DisplayName: strings.Repeat("\u754c", 129), + }}, + errorText: "at most 128", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := applyClusterNodeDisplayNameOverrides(instance, tc.overrides) + if err == nil || !strings.Contains(err.Error(), tc.errorText) { + t.Fatalf("error = %v, want containing %q", err, tc.errorText) + } + }) + } +} + +func TestClusterEndpointResponseSeparatesNativeAndDisplayIdentity(t *testing.T) { + instance := displayNameTestInstance() + instance.ClusterNodeIdentities[0].NativeNodeID = 11 + instance.ClusterNodeIdentities[0].NativeAliases = []string{"pve-old"} + instance.ClusterNodeIdentities[0].DisplayName = "Compute A" + instance.ClusterEndpoints[0].NativeNodeID = 11 + + response := toClusterEndpointResponse(instance.ClusterEndpoints[0], instance.ClusterNodeIdentities) + if response.NodeIdentity != "cluster-pve1" || response.NativeNodeID != 11 || + response.NativeName != "pve1" || response.DisplayName != "Compute A" { + t.Fatalf("unexpected endpoint response: %+v", response) + } + if len(response.NativeAliases) != 1 || response.NativeAliases[0] != "pve-old" { + t.Fatalf("native diagnostic aliases missing from endpoint response: %+v", response) + } +} + +func TestHandleUpdateNodePersistsDisplayNameWithoutChangingConnectionAuthority(t *testing.T) { + cfg := &config.Config{ + DataPath: t.TempDir(), + PVEInstances: []config.PVEInstance{{ + Name: "cluster", Host: "https://configured.example.test:8006", + TokenName: "root@pam!pulse", TokenValue: "secret", + IsCluster: true, ClusterName: "production", + ClusterEndpoints: []config.ClusterEndpoint{{ + NodeID: "node/pve1", NodeIdentity: "production-pve1", NativeNodeID: 1, + NodeName: "pve1", Host: "https://pve1:8006", IP: "10.0.0.1", IPOverride: "10.20.0.1", + }}, + ClusterNodeIdentities: []config.PVEClusterNodeIdentity{{ + ID: "production-pve1", NativeNodeID: 1, NativeName: "pve1", + }}, + }}, + } + handler := newTestConfigHandlers(t, cfg) + body, _ := json.Marshal(map[string]any{ + "clusterNodeDisplayNameOverrides": []map[string]string{{ + "nodeIdentity": "production-pve1", + "displayName": "Render East", + }}, + }) + req := httptest.NewRequest(http.MethodPut, "/api/config/nodes/pve-0", bytes.NewReader(body)) + rec := httptest.NewRecorder() + handler.HandleUpdateNode(rec, req) + if rec.Code != http.StatusOK { + t.Fatalf("update status = %d: %s", rec.Code, rec.Body.String()) + } + + instance := cfg.PVEInstances[0] + if instance.ClusterNodeIdentities[0].DisplayName != "Render East" { + t.Fatalf("display name was not persisted in config: %+v", instance.ClusterNodeIdentities) + } + if instance.Host != "https://configured.example.test:8006" || + instance.TokenValue != "secret" || + instance.ClusterEndpoints[0].Host != "https://pve1:8006" || + instance.ClusterEndpoints[0].IPOverride != "10.20.0.1" { + t.Fatalf("presentation update changed routing or credentials: %+v", instance) + } + + reloaded, err := handler.defaultPersistence.LoadNodesConfig() + if err != nil { + t.Fatalf("reload persisted nodes: %v", err) + } + if got := reloaded.PVEInstances[0].ClusterNodeIdentities[0].DisplayName; got != "Render East" { + t.Fatalf("reloaded display name = %q, want Render East", got) + } +} + +func TestHandleUpdateNodeRejectDoesNotMutateLegacyIdentityState(t *testing.T) { + cfg := &config.Config{ + DataPath: t.TempDir(), + PVEInstances: []config.PVEInstance{{ + Name: "cluster", Host: "https://configured.example.test:8006", + TokenName: "root@pam!pulse", TokenValue: "secret", + IsCluster: true, ClusterName: "production", + ClusterEndpoints: []config.ClusterEndpoint{{NodeName: "pve1"}}, + }}, + } + handler := newTestConfigHandlers(t, cfg) + body, _ := json.Marshal(map[string]any{ + "clusterNodeDisplayNameOverrides": []map[string]string{{ + "nodeIdentity": "stale-browser-identity", + "displayName": "Wrong node", + }}, + }) + req := httptest.NewRequest(http.MethodPut, "/api/config/nodes/pve-0", bytes.NewReader(body)) + rec := httptest.NewRecorder() + handler.HandleUpdateNode(rec, req) + if rec.Code != http.StatusBadRequest { + t.Fatalf("update status = %d, want 400: %s", rec.Code, rec.Body.String()) + } + if cfg.PVEInstances[0].ClusterEndpoints[0].NodeIdentity != "" || + len(cfg.PVEInstances[0].ClusterNodeIdentities) != 0 { + t.Fatalf("rejected update mutated live legacy config: %+v", cfg.PVEInstances[0]) + } +} + +func TestMergeDiscoveredClusterEndpointConfigurationUsesStrongIdentityOnly(t *testing.T) { + existing := []config.ClusterEndpoint{ + {NodeIdentity: "identity-a", NativeNodeID: 1, NodeName: "old-a", IP: "10.0.0.1", IPOverride: "route-a"}, + {NodeIdentity: "identity-b", NativeNodeID: 2, NodeName: "old-b", IP: "10.0.0.2", IPOverride: "route-b"}, + } + renamed := mergeDiscoveredClusterEndpointConfiguration(existing, []config.ClusterEndpoint{{ + NativeNodeID: 1, NodeName: "new-a", IP: "10.20.0.1", + }}) + if renamed[0].NodeIdentity != "identity-a" || renamed[0].IPOverride != "route-a" { + t.Fatalf("numeric identity did not preserve user metadata through rename/re-IP: %+v", renamed[0]) + } + + ambiguousLegacy := mergeDiscoveredClusterEndpointConfiguration([]config.ClusterEndpoint{ + {NodeIdentity: "identity-a", NodeName: "old-a", IP: "10.0.0.1"}, + {NodeIdentity: "identity-b", NodeName: "old-b", IP: "10.0.0.1"}, + }, []config.ClusterEndpoint{{ + NodeName: "new-name", IP: "10.0.0.1", + }}) + if ambiguousLegacy[0].NodeIdentity != "" { + t.Fatalf("ambiguous legacy address evidence must not corrupt canonical identity: %+v", ambiguousLegacy[0]) + } +} + +func TestApplyClusterNodeDisplayNameOverridesConcurrentCopies(t *testing.T) { + instance := displayNameTestInstance() + var wait sync.WaitGroup + for worker := 0; worker < 32; worker++ { + wait.Add(1) + go func() { + defer wait.Done() + updated, err := applyClusterNodeDisplayNameOverrides(instance, []ClusterNodeDisplayNameOverrideRequest{{ + NodeIdentity: "cluster-pve1", + DisplayName: "Render East", + }}) + if err != nil || updated.ClusterNodeIdentities[0].DisplayName != "Render East" { + t.Errorf("concurrent immutable update failed: identities=%+v err=%v", updated.ClusterNodeIdentities, err) + } + }() + } + wait.Wait() + if instance.ClusterNodeIdentities[0].DisplayName != "" { + t.Fatal("concurrent presentation updates mutated shared tenant config input") + } +} diff --git a/internal/api/config_node_handlers.go b/internal/api/config_node_handlers.go index a492ad18e8..1f879408e1 100644 --- a/internal/api/config_node_handlers.go +++ b/internal/api/config_node_handlers.go @@ -1302,6 +1302,41 @@ func applyClusterEndpointOverrides(endpoints []config.ClusterEndpoint, overrides return result, nil } +func applyClusterNodeDisplayNameOverrides(instance config.PVEInstance, overrides []ClusterNodeDisplayNameOverrideRequest) (config.PVEInstance, error) { + instance.ClusterEndpoints = append([]config.ClusterEndpoint(nil), instance.ClusterEndpoints...) + instance.ClusterNodeIdentities = append([]config.PVEClusterNodeIdentity(nil), instance.ClusterNodeIdentities...) + seen := make(map[string]struct{}, len(overrides)) + for _, override := range overrides { + identityID := strings.TrimSpace(override.NodeIdentity) + if identityID == "" { + return instance, fmt.Errorf("cluster node identity is required") + } + if _, duplicate := seen[identityID]; duplicate { + return instance, fmt.Errorf("cluster node identity %q was provided more than once", identityID) + } + seen[identityID] = struct{}{} + + displayName, err := config.NormalizePVEClusterNodeDisplayName(override.DisplayName) + if err != nil { + return instance, fmt.Errorf("display name for cluster node %q %w", identityID, err) + } + + found := false + for idx := range instance.ClusterNodeIdentities { + if instance.ClusterNodeIdentities[idx].ID != identityID { + continue + } + instance.ClusterNodeIdentities[idx].DisplayName = displayName + found = true + break + } + if !found { + return instance, fmt.Errorf("unknown cluster node identity %q", identityID) + } + } + return instance, nil +} + func (h *ConfigHandlers) handleUpdateNode(w http.ResponseWriter, r *http.Request) { // Prevent node modifications in mock mode if mock.IsMockEnabled() { @@ -1344,7 +1379,10 @@ func (h *ConfigHandlers) handleUpdateNode(w http.ResponseWriter, r *http.Request // Update the node if nodeType == "pve" && index < len(h.getConfig(r.Context()).PVEInstances) { pve := &h.getConfig(r.Context()).PVEInstances[index] - current := *pve + current := pve.DeepCopy() + normalizedCurrent := []config.PVEInstance{current} + config.EnsurePVEClusterNodeIdentities(normalizedCurrent) + current = normalizedCurrent[0] updated := current // Only update name if provided @@ -1373,6 +1411,13 @@ func (h *ConfigHandlers) handleUpdateNode(w http.ResponseWriter, r *http.Request } updated.ClusterEndpoints = endpoints } + if len(req.ClusterNodeDisplayNameOverrides) > 0 { + updated, err = applyClusterNodeDisplayNameOverrides(updated, req.ClusterNodeDisplayNameOverrides) + if err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + return + } + } if err := applyPVEAuthUpdate(&updated, req); err != nil { http.Error(w, err.Error(), http.StatusBadRequest) @@ -1620,6 +1665,66 @@ func (h *ConfigHandlers) handleUpdateNode(w http.ResponseWriter, r *http.Request json.NewEncoder(w).Encode(map[string]string{"status": "success"}) } +func mergeDiscoveredClusterEndpointConfiguration(existing, discovered []config.ClusterEndpoint) []config.ClusterEndpoint { + merged := append([]config.ClusterEndpoint(nil), discovered...) + used := make(map[int]struct{}, len(existing)) + for idx := range merged { + matchUnique := func(predicate func(config.ClusterEndpoint) bool) int { + match := -1 + for existingIdx, candidate := range existing { + if _, alreadyUsed := used[existingIdx]; alreadyUsed || !predicate(candidate) { + continue + } + if match >= 0 { + return -1 + } + match = existingIdx + } + return match + } + match := -1 + if merged[idx].NativeNodeID != 0 { + match = matchUnique(func(candidate config.ClusterEndpoint) bool { + return candidate.NativeNodeID != 0 && candidate.NativeNodeID == merged[idx].NativeNodeID + }) + } + if match < 0 && merged[idx].NodeName != "" { + match = matchUnique(func(candidate config.ClusterEndpoint) bool { + return strings.EqualFold(strings.TrimSpace(candidate.NodeName), strings.TrimSpace(merged[idx].NodeName)) + }) + } + if match < 0 && merged[idx].IP != "" { + match = matchUnique(func(candidate config.ClusterEndpoint) bool { + return strings.TrimSpace(candidate.IP) == strings.TrimSpace(merged[idx].IP) + }) + } + if match < 0 && merged[idx].Host != "" { + match = matchUnique(func(candidate config.ClusterEndpoint) bool { + return strings.EqualFold(strings.TrimSpace(candidate.Host), strings.TrimSpace(merged[idx].Host)) + }) + } + if match < 0 { + continue + } + used[match] = struct{}{} + old := existing[match] + merged[idx].NodeIdentity = old.NodeIdentity + if merged[idx].NativeNodeID == 0 { + merged[idx].NativeNodeID = old.NativeNodeID + } + if merged[idx].IPOverride == "" { + merged[idx].IPOverride = old.IPOverride + } + if merged[idx].GuestURL == "" { + merged[idx].GuestURL = old.GuestURL + } + if merged[idx].Fingerprint == "" { + merged[idx].Fingerprint = old.Fingerprint + } + } + return merged +} + // HandleDeleteNode deletes a node func (h *ConfigHandlers) handleDeleteNode(w http.ResponseWriter, r *http.Request) { @@ -1809,7 +1914,7 @@ func (h *ConfigHandlers) handleRefreshClusterNodes(w http.ResponseWriter, r *htt if clusterName != "" && !strings.EqualFold(clusterName, "unknown cluster") { pve.ClusterName = clusterName } - pve.ClusterEndpoints = clusterEndpoints + pve.ClusterEndpoints = mergeDiscoveredClusterEndpointConfiguration(pve.ClusterEndpoints, clusterEndpoints) // Save configuration h.normalizePVEConfigState(r.Context()) @@ -1853,7 +1958,7 @@ func (h *ConfigHandlers) handleRefreshClusterNodes(w http.ResponseWriter, r *htt "oldNodeCount": oldEndpointCount, "newNodeCount": newEndpointCount, "nodesAdded": newEndpointCount - oldEndpointCount, - "clusterNodes": toClusterEndpointResponses(clusterEndpoints), + "clusterNodes": toClusterEndpointResponses(pve.ClusterEndpoints, pve.ClusterNodeIdentities), }) } diff --git a/internal/api/connections_grouping.go b/internal/api/connections_grouping.go index 291cd506ad..6a271e33cd 100644 --- a/internal/api/connections_grouping.go +++ b/internal/api/connections_grouping.go @@ -231,7 +231,9 @@ func connectionSystemMemberFromNode( return ConnectionSystemMember{ ID: strings.TrimSpace(node.ID), + NodeIdentity: firstNonEmptyTrimmed(node.NodeIdentity, node.ID), Name: firstNonEmptyTrimmed(node.DisplayName, node.Name), + NativeName: strings.TrimSpace(node.Name), Endpoint: strings.TrimSpace(node.Host), HostAliases: connectionSystemMemberHostAliasesFromNode(node), State: connectionSystemMemberStateFromNode(node), @@ -275,7 +277,9 @@ func connectionSystemMemberFromResource( return ConnectionSystemMember{ ID: strings.TrimSpace(resource.ID), - Name: firstNonEmptyTrimmed(resource.Name, nodeName), + NodeIdentity: firstNonEmptyTrimmed(resource.Proxmox.NodeIdentity, resource.Proxmox.SourceID), + Name: firstNonEmptyTrimmed(resource.Proxmox.NodeDisplayName, resource.Name, nodeName), + NativeName: nodeName, Endpoint: strings.TrimSpace(resource.Proxmox.HostURL), HostAliases: connectionSystemMemberHostAliasesFromResource(resource), State: connectionSystemMemberStateFromResource(resource), @@ -332,6 +336,12 @@ func mergeConnectionSystemMembers( if strings.TrimSpace(existing.Name) == "" { existing.Name = candidate.Name } + if strings.TrimSpace(existing.NodeIdentity) == "" { + existing.NodeIdentity = candidate.NodeIdentity + } + if strings.TrimSpace(existing.NativeName) == "" { + existing.NativeName = candidate.NativeName + } if strings.TrimSpace(existing.Endpoint) == "" { existing.Endpoint = candidate.Endpoint } @@ -380,12 +390,13 @@ func connectionSystemMemberStateSeverity(state ConnectionState) int { } func connectionSystemMemberHostAliasesFromNode(node models.Node) []string { - return appendNormalizedHosts( - nil, + values := []string{ node.Name, node.DisplayName, node.Host, - ) + } + values = append(values, node.NativeNameAliases...) + return appendNormalizedHosts(nil, values...) } func connectionSystemMemberHostAliasesFromResource(resource unified.Resource) []string { @@ -394,7 +405,8 @@ func connectionSystemMemberHostAliasesFromResource(resource unified.Resource) [] canonicalResourceHostname(resource), } if resource.Proxmox != nil { - values = append(values, resource.Proxmox.NodeName, resource.Proxmox.HostURL) + values = append(values, resource.Proxmox.NodeName, resource.Proxmox.NodeDisplayName, resource.Proxmox.HostURL) + values = append(values, resource.Proxmox.NodeAliases...) } if resource.Agent != nil { values = append(values, resource.Agent.Hostname) @@ -406,9 +418,10 @@ func connectionSystemMemberHostAliasesFromResource(resource unified.Resource) [] func connectionSystemMemberKey(member ConnectionSystemMember) string { for _, candidate := range []string{ - strings.TrimSpace(member.Name), - strings.TrimSpace(member.Endpoint), + strings.TrimSpace(member.NodeIdentity), strings.TrimSpace(member.ID), + strings.TrimSpace(member.NativeName), + strings.TrimSpace(member.Endpoint), } { if normalized := normalizeHost(candidate); normalized != "" { return normalized diff --git a/internal/api/connections_grouping_test.go b/internal/api/connections_grouping_test.go index 39c930c3d0..c746a2e3f5 100644 --- a/internal/api/connections_grouping_test.go +++ b/internal/api/connections_grouping_test.go @@ -337,6 +337,36 @@ func TestMergeConnectionSystemMembersKeepsMostSevereState(t *testing.T) { } } +func TestConnectionSystemMemberUsesDisplayNameWithoutChangingMemberKey(t *testing.T) { + resource := unified.Resource{ + ID: "resource-node", + Type: unified.ResourceTypeAgent, + Name: "Render East", + Status: unified.StatusOnline, + Proxmox: &unified.ProxmoxData{ + SourceID: "production-pve1", NodeIdentity: "production-pve1", + NodeName: "pve1", NodeDisplayName: "Render East", + Instance: "cluster", IsClusterMember: true, + }, + } + connections := map[string]Connection{ + "pve:cluster": {ID: "pve:cluster", Type: ConnectionTypePVE}, + } + member, primaryID, ok := connectionSystemMemberFromResource(resource, connections, nil) + if !ok || primaryID != "pve:cluster" { + t.Fatalf("member projection failed: primary=%q member=%+v", primaryID, member) + } + if member.Name != "Render East" || member.NativeName != "pve1" || + member.NodeIdentity != "production-pve1" { + t.Fatalf("member presentation/native fields were not separated: %+v", member) + } + beforeKey := connectionSystemMemberKey(member) + member.Name = "Another label" + if afterKey := connectionSystemMemberKey(member); afterKey != beforeKey { + t.Fatalf("cosmetic rename changed member key: before=%q after=%q", beforeKey, afterKey) + } +} + func TestBuildConnectionSystems_GuestAgentStaysStandaloneWhenOnlyClusterInstanceMatches(t *testing.T) { cfg := &config.Config{DataPath: t.TempDir()} monitor, err := monitoring.New(cfg) diff --git a/internal/api/connections_types.go b/internal/api/connections_types.go index 54ac0fdcfb..83fe502d11 100644 --- a/internal/api/connections_types.go +++ b/internal/api/connections_types.go @@ -208,7 +208,9 @@ type ConnectionSystemComponent struct { // owning cluster source instead of reopening standalone host rows. type ConnectionSystemMember struct { ID string `json:"id"` + NodeIdentity string `json:"nodeIdentity,omitempty"` Name string `json:"name"` + NativeName string `json:"nativeName,omitempty"` Endpoint string `json:"endpoint,omitempty"` HostAliases []string `json:"hostAliases,omitempty"` State ConnectionState `json:"state"` diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 79cc23f917..c174d5eab5 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -15604,7 +15604,9 @@ func TestContract_ConnectionSystemMembersPayloadShapeStaysCanonical(t *testing.T Members: []ConnectionSystemMember{ { ID: "node-lab", - Name: "lab", + NodeIdentity: "homelab-lab", + Name: "Render East", + NativeName: "lab", Endpoint: "https://lab:8006", HostAliases: []string{"lab", "192.168.0.2"}, State: ConnectionStateActive, @@ -15628,7 +15630,7 @@ func TestContract_ConnectionSystemMembersPayloadShapeStaysCanonical(t *testing.T t.Fatalf("marshal ConnectionSystem with members: %v", err) } - want := `{"id":"pve-lab","type":"pve","clusterName":"homelab","components":[{"connectionId":"pve-lab","type":"pve","role":"primary"},{"connectionId":"agent:agent-lab","type":"agent","role":"attachment"}],"members":[{"id":"node-lab","name":"lab","endpoint":"https://lab:8006","hostAliases":["lab","192.168.0.2"],"state":"active","lastSeen":"2026-04-23T12:00:00Z","primary":true,"agentConnectionId":"agent:agent-lab"},{"id":"node-minipc","name":"minipc","endpoint":"https://minipc:8006","hostAliases":["minipc"],"state":"stale","lastSeen":"2026-04-23T11:55:00Z"}]}` + want := `{"id":"pve-lab","type":"pve","clusterName":"homelab","components":[{"connectionId":"pve-lab","type":"pve","role":"primary"},{"connectionId":"agent:agent-lab","type":"agent","role":"attachment"}],"members":[{"id":"node-lab","nodeIdentity":"homelab-lab","name":"Render East","nativeName":"lab","endpoint":"https://lab:8006","hostAliases":["lab","192.168.0.2"],"state":"active","lastSeen":"2026-04-23T12:00:00Z","primary":true,"agentConnectionId":"agent:agent-lab"},{"id":"node-minipc","name":"minipc","endpoint":"https://minipc:8006","hostAliases":["minipc"],"state":"stale","lastSeen":"2026-04-23T11:55:00Z"}]}` assertJSONSnapshot(t, body, want) } diff --git a/internal/config/config.go b/internal/config/config.go index e6492047a0..c844d5ee4e 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -299,8 +299,7 @@ func (c *Config) DeepCopy() *Config { // Deep copy PVEInstances slice if len(c.PVEInstances) > 0 { - clone.PVEInstances = make([]PVEInstance, len(c.PVEInstances)) - copy(clone.PVEInstances, c.PVEInstances) + clone.PVEInstances = clonePVEInstances(c.PVEInstances) } // Deep copy PBSInstances slice @@ -571,9 +570,10 @@ type PVEInstance struct { SSHPort int // SSH port for temperature monitoring (0 = use global default) // Cluster support - IsCluster bool // True if this is a cluster - ClusterName string // Cluster name if applicable - ClusterEndpoints []ClusterEndpoint // All discovered cluster nodes + IsCluster bool // True if this is a cluster + ClusterName string // Cluster name if applicable + ClusterEndpoints []ClusterEndpoint // All discovered cluster nodes + ClusterNodeIdentities []PVEClusterNodeIdentity // Durable presentation identities for cluster members // Agent tracking Source string // "agent" or "script" - how this node was registered (empty = legacy/manual) @@ -587,7 +587,9 @@ type PVEInstance struct { // ClusterEndpoint represents a single node in a cluster type ClusterEndpoint struct { - NodeID string // Node ID in cluster + NodeID string // Provider-reported cluster status ID (diagnostic only) + NodeIdentity string // Pulse-owned immutable identity scoped to this connection + NativeNodeID int // Provider-reported numeric node ID when available NodeName string // Node name Host string // Full URL (e.g., https://node1.lan:8006) GuestURL string // Optional guest-accessible URL (for navigation) @@ -601,6 +603,17 @@ type ClusterEndpoint struct { PulseError string // Last error Pulse encountered connecting to this endpoint } +// PVEClusterNodeIdentity keeps presentation metadata separate from endpoint +// routing. ID is created once per connection and remains stable across native +// node renames, address changes, and temporary cluster-membership removal. +type PVEClusterNodeIdentity struct { + ID string `json:"id"` + NativeNodeID int `json:"nativeNodeId,omitempty"` + NativeName string `json:"nativeName"` + NativeAliases []string `json:"nativeAliases,omitempty"` + DisplayName string `json:"displayName,omitempty"` +} + // EffectiveIP returns the IP to use for this endpoint, preferring IPOverride if set func (e ClusterEndpoint) EffectiveIP() string { if e.IPOverride != "" { @@ -1771,6 +1784,11 @@ func (c *Config) Validate() error { if pve.Password == "" && (pve.TokenName == "" || pve.TokenValue == "") { return fmt.Errorf("PVE instance %d: either password or token authentication is required", i+1) } + for _, identity := range pve.ClusterNodeIdentities { + if _, err := NormalizePVEClusterNodeDisplayName(identity.DisplayName); err != nil { + return fmt.Errorf("PVE instance %d cluster node %q display name %s", i+1, identity.ID, err) + } + } } // Validate and auto-fix PBS instances diff --git a/internal/config/config_load_test.go b/internal/config/config_load_test.go index a7ff7dd0d0..56acb85ef9 100644 --- a/internal/config/config_load_test.go +++ b/internal/config/config_load_test.go @@ -44,6 +44,27 @@ func TestLoad_DefaultHostedCommercialBaseURLAvoidsRetiredTrialPath(t *testing.T) } } +func TestConfigValidateRejectsUnsafePVEClusterDisplayName(t *testing.T) { + t.Setenv("PULSE_DATA_DIR", t.TempDir()) + cfg, err := LoadWithoutLoggingInit() + require.NoError(t, err) + cfg.PVEInstances = []PVEInstance{{ + Host: "https://pve.example.test:8006", + TokenName: "root@pam!pulse", + TokenValue: "secret", + IsCluster: true, + ClusterNodeIdentities: []PVEClusterNodeIdentity{{ + ID: "production-pve1", + NativeName: "pve1", + DisplayName: "unsafe\nlabel", + }}, + }} + + err = cfg.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "control characters") +} + func TestResolveRuntimeDataDir(t *testing.T) { tmpDefault := t.TempDir() prevDefault := defaultDataDir diff --git a/internal/config/persistence.go b/internal/config/persistence.go index 26282bb3ba..67b9083f4a 100644 --- a/internal/config/persistence.go +++ b/internal/config/persistence.go @@ -1849,6 +1849,9 @@ func (c *ConfigPersistence) LoadNodesConfig() (*NodesConfig, error) { config.PVEInstances = consolidated migrationApplied = true } + if EnsurePVEClusterNodeIdentities(config.PVEInstances) { + migrationApplied = true + } // Fix for bug where TokenName was incorrectly set when using password auth // If a PBS instance has both Password and TokenName, clear the TokenName @@ -1928,8 +1931,7 @@ func (c *ConfigPersistence) LoadNodesConfig() (*NodesConfig, error) { // between unlock and relock, causing migrated data to be lost if migrationApplied { // Make copies while still holding read lock to ensure consistency - pveCopy := make([]PVEInstance, len(config.PVEInstances)) - copy(pveCopy, config.PVEInstances) + pveCopy := clonePVEInstances(config.PVEInstances) pbsCopy := make([]PBSInstance, len(config.PBSInstances)) copy(pbsCopy, config.PBSInstances) pmgCopy := make([]PMGInstance, len(config.PMGInstances)) diff --git a/internal/config/persistence_test.go b/internal/config/persistence_test.go index a7dbcde79c..41285959ed 100644 --- a/internal/config/persistence_test.go +++ b/internal/config/persistence_test.go @@ -619,6 +619,14 @@ func TestImportConfigTransactionalSuccess(t *testing.T) { User: "root@pam", MonitorVMs: true, MonitorStorage: true, + IsCluster: true, + ClusterName: "production", + ClusterEndpoints: []config.ClusterEndpoint{{ + NodeID: "node/pve-new", NodeIdentity: "production-pve-new", NativeNodeID: 4, NodeName: "pve-new", + }}, + ClusterNodeIdentities: []config.PVEClusterNodeIdentity{{ + ID: "production-pve-new", NativeNodeID: 4, NativeName: "pve-new", DisplayName: "Render East", + }}, }, } newPBS := []config.PBSInstance{ diff --git a/internal/config/pve_instances.go b/internal/config/pve_instances.go index 3cb862084a..d8906afb9d 100644 --- a/internal/config/pve_instances.go +++ b/internal/config/pve_instances.go @@ -43,10 +43,25 @@ func clonePVEInstances(instances []PVEInstance) []PVEInstance { out[i].ClusterEndpoints = make([]ClusterEndpoint, len(instances[i].ClusterEndpoints)) copy(out[i].ClusterEndpoints, instances[i].ClusterEndpoints) } + if len(instances[i].ClusterNodeIdentities) > 0 { + out[i].ClusterNodeIdentities = append([]PVEClusterNodeIdentity(nil), instances[i].ClusterNodeIdentities...) + for identityIdx := range out[i].ClusterNodeIdentities { + out[i].ClusterNodeIdentities[identityIdx].NativeAliases = append( + []string(nil), + instances[i].ClusterNodeIdentities[identityIdx].NativeAliases..., + ) + } + } } return out } +// DeepCopy returns an independently mutable copy of one PVE instance, +// including endpoint and retained cluster-node identity metadata. +func (instance PVEInstance) DeepCopy() PVEInstance { + return clonePVEInstances([]PVEInstance{instance})[0] +} + func mergeDuplicateClusterInstances(instances []PVEInstance) ([]PVEInstance, bool) { removed := make(map[int]struct{}) mergedAny := false @@ -87,6 +102,7 @@ func mergeDuplicateClusterInstances(instances []PVEInstance) ([]PVEInstance, boo Msg("Merging duplicate cluster instance") mergePVEInstanceData(primary, duplicate) + mergePVEClusterNodeIdentities(primary, duplicate) for _, ep := range duplicate.ClusterEndpoints { nodeKey := strings.TrimSpace(strings.ToLower(ep.NodeName)) @@ -125,6 +141,12 @@ func mergeClusterEndpointData(dst *ClusterEndpoint, src ClusterEndpoint) { if dst.Host == "" && strings.TrimSpace(src.Host) != "" { dst.Host = strings.TrimSpace(src.Host) } + if dst.NodeIdentity == "" && strings.TrimSpace(src.NodeIdentity) != "" { + dst.NodeIdentity = strings.TrimSpace(src.NodeIdentity) + } + if dst.NativeNodeID == 0 && src.NativeNodeID != 0 { + dst.NativeNodeID = src.NativeNodeID + } if dst.GuestURL == "" && strings.TrimSpace(src.GuestURL) != "" { dst.GuestURL = strings.TrimSpace(src.GuestURL) } @@ -156,6 +178,56 @@ func mergeClusterEndpointData(dst *ClusterEndpoint, src ClusterEndpoint) { } } +func mergePVEClusterNodeIdentities(dst *PVEInstance, src PVEInstance) { + if dst == nil { + return + } + for _, identity := range src.ClusterNodeIdentities { + found := false + for idx := range dst.ClusterNodeIdentities { + current := &dst.ClusterNodeIdentities[idx] + if current.ID != identity.ID { + continue + } + if current.NativeNodeID == 0 { + current.NativeNodeID = identity.NativeNodeID + } + if current.NativeName == "" { + current.NativeName = identity.NativeName + } + if current.DisplayName == "" { + current.DisplayName = identity.DisplayName + } + current.NativeAliases = appendUniquePVEIdentityAliases(current.NativeAliases, identity.NativeAliases...) + found = true + break + } + if !found { + dst.ClusterNodeIdentities = append(dst.ClusterNodeIdentities, identity) + } + } +} + +func appendUniquePVEIdentityAliases(existing []string, values ...string) []string { + for _, value := range values { + value = strings.TrimSpace(value) + if value == "" { + continue + } + found := false + for _, current := range existing { + if current == value { + found = true + break + } + } + if !found { + existing = append(existing, value) + } + } + return existing +} + func mergePVEInstanceData(dst *PVEInstance, src PVEInstance) { if dst == nil { return diff --git a/internal/config/pve_node_identity.go b/internal/config/pve_node_identity.go new file mode 100644 index 0000000000..91470302c8 --- /dev/null +++ b/internal/config/pve_node_identity.go @@ -0,0 +1,311 @@ +package config + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "reflect" + "strconv" + "strings" + "unicode" + "unicode/utf8" +) + +// EnsurePVEClusterNodeIdentities migrates and repairs persisted cluster-node +// identities in place. Existing IDs are never derived again once recorded. +func EnsurePVEClusterNodeIdentities(instances []PVEInstance) bool { + before := clonePVEInstances(instances) + clusterNameCounts := make(map[string]int) + for i := range instances { + if !instances[i].IsCluster { + continue + } + if name := strings.ToLower(strings.TrimSpace(instances[i].ClusterName)); name != "" { + clusterNameCounts[name]++ + } + } + + for i := range instances { + ensurePVEClusterNodeIdentitiesForInstance(&instances[i], clusterNameCounts) + } + return !reflect.DeepEqual(before, instances) +} + +func ensurePVEClusterNodeIdentitiesForInstance(instance *PVEInstance, clusterNameCounts map[string]int) { + if instance == nil || !instance.IsCluster { + return + } + + scope := strings.TrimSpace(instance.Name) + clusterName := strings.TrimSpace(instance.ClusterName) + if clusterName != "" && clusterNameCounts[strings.ToLower(clusterName)] == 1 { + scope = clusterName + } + if scope == "" { + scope = "proxmox" + } + + normalized := make([]PVEClusterNodeIdentity, 0, len(instance.ClusterNodeIdentities)+len(instance.ClusterEndpoints)) + byID := make(map[string]int) + for _, identity := range instance.ClusterNodeIdentities { + identity.ID = strings.TrimSpace(identity.ID) + identity.NativeName = strings.TrimSpace(identity.NativeName) + identity.NativeAliases = appendUniquePVEIdentityAliases(nil, identity.NativeAliases...) + identity.DisplayName = strings.TrimSpace(identity.DisplayName) + if identity.ID == "" { + continue + } + if existingIdx, exists := byID[identity.ID]; exists { + existing := &normalized[existingIdx] + if existing.NativeNodeID == 0 { + existing.NativeNodeID = identity.NativeNodeID + } + if existing.NativeName == "" { + existing.NativeName = identity.NativeName + } + if existing.DisplayName == "" { + existing.DisplayName = identity.DisplayName + } + existing.NativeAliases = appendUniquePVEIdentityAliases(existing.NativeAliases, identity.NativeAliases...) + continue + } + byID[identity.ID] = len(normalized) + normalized = append(normalized, identity) + } + instance.ClusterNodeIdentities = normalized + + usedIdentity := make(map[string]int) + for endpointIdx := range instance.ClusterEndpoints { + endpoint := &instance.ClusterEndpoints[endpointIdx] + endpoint.NodeIdentity = strings.TrimSpace(endpoint.NodeIdentity) + endpoint.NodeName = strings.TrimSpace(endpoint.NodeName) + + identityIdx := findPVEClusterNodeIdentityIndex(*instance, *endpoint) + if identityIdx < 0 && endpoint.NodeIdentity != "" { + identityIdx = len(instance.ClusterNodeIdentities) + instance.ClusterNodeIdentities = append(instance.ClusterNodeIdentities, PVEClusterNodeIdentity{ + ID: endpoint.NodeIdentity, + NativeNodeID: endpoint.NativeNodeID, + NativeName: endpoint.NodeName, + }) + } + if identityIdx < 0 { + seed := endpoint.NodeName + if seed == "" { + seed = endpoint.NodeID + } + candidate := scope + "-" + seed + if seed == "" || identityIDConflicts(instance.ClusterNodeIdentities, candidate, endpoint.NativeNodeID, endpoint.NodeName) { + candidate = deterministicPVEClusterNodeIdentity(scope, *endpoint, endpointIdx) + for suffix := 1; identityIDExists(instance.ClusterNodeIdentities, candidate); suffix++ { + candidate = deterministicPVEClusterNodeIdentity(scope, *endpoint, endpointIdx+suffix) + } + } + identityIdx = len(instance.ClusterNodeIdentities) + instance.ClusterNodeIdentities = append(instance.ClusterNodeIdentities, PVEClusterNodeIdentity{ + ID: candidate, + NativeNodeID: endpoint.NativeNodeID, + NativeName: endpoint.NodeName, + }) + } + + identity := &instance.ClusterNodeIdentities[identityIdx] + if previousEndpointIdx, duplicate := usedIdentity[identity.ID]; duplicate && + !samePVEClusterNode(instance.ClusterEndpoints[previousEndpointIdx], *endpoint) { + candidate := deterministicPVEClusterNodeIdentity(scope, *endpoint, endpointIdx) + for suffix := 1; identityIDExists(instance.ClusterNodeIdentities, candidate); suffix++ { + candidate = deterministicPVEClusterNodeIdentity(scope, *endpoint, endpointIdx+suffix) + } + identityIdx = len(instance.ClusterNodeIdentities) + instance.ClusterNodeIdentities = append(instance.ClusterNodeIdentities, PVEClusterNodeIdentity{ + ID: candidate, + NativeNodeID: endpoint.NativeNodeID, + NativeName: endpoint.NodeName, + }) + identity = &instance.ClusterNodeIdentities[identityIdx] + } + + endpoint.NodeIdentity = identity.ID + if endpoint.NativeNodeID != 0 { + identity.NativeNodeID = endpoint.NativeNodeID + } else if identity.NativeNodeID != 0 { + endpoint.NativeNodeID = identity.NativeNodeID + } + if endpoint.NodeName != "" { + if identity.NativeName != "" && identity.NativeName != endpoint.NodeName { + identity.NativeAliases = appendUniquePVEIdentityAliases(identity.NativeAliases, identity.NativeName) + } + identity.NativeName = endpoint.NodeName + } + usedIdentity[identity.ID] = endpointIdx + } +} + +func findPVEClusterNodeIdentityIndex(instance PVEInstance, endpoint ClusterEndpoint) int { + if endpoint.NodeIdentity != "" { + for i := range instance.ClusterNodeIdentities { + if instance.ClusterNodeIdentities[i].ID == endpoint.NodeIdentity { + return i + } + } + } + if endpoint.NativeNodeID != 0 { + match := -1 + for i := range instance.ClusterNodeIdentities { + if instance.ClusterNodeIdentities[i].NativeNodeID != endpoint.NativeNodeID { + continue + } + if match >= 0 { + return -1 + } + match = i + } + return match + } + if endpoint.NodeName != "" { + match := -1 + for i := range instance.ClusterNodeIdentities { + if !strings.EqualFold(instance.ClusterNodeIdentities[i].NativeName, endpoint.NodeName) { + continue + } + if match >= 0 { + return -1 + } + match = i + } + return match + } + return -1 +} + +func identityIDConflicts(identities []PVEClusterNodeIdentity, id string, nativeID int, nativeName string) bool { + for _, identity := range identities { + if identity.ID != id { + continue + } + if nativeID != 0 && identity.NativeNodeID != 0 { + return nativeID != identity.NativeNodeID + } + return !strings.EqualFold(identity.NativeName, nativeName) + } + return false +} + +// NormalizePVEClusterNodeDisplayName validates and normalizes one +// presentation-only node label. Empty is valid and clears the override. +func NormalizePVEClusterNodeDisplayName(value string) (string, error) { + value = strings.TrimSpace(value) + if !utf8.ValidString(value) || utf8.RuneCountInString(value) > 128 { + return "", fmt.Errorf("must be valid Unicode and at most 128 characters") + } + for _, r := range value { + if unicode.IsControl(r) { + return "", fmt.Errorf("must not contain control characters") + } + } + return value, nil +} + +func identityIDExists(identities []PVEClusterNodeIdentity, id string) bool { + for _, identity := range identities { + if identity.ID == id { + return true + } + } + return false +} + +func deterministicPVEClusterNodeIdentity(scope string, endpoint ClusterEndpoint, ordinal int) string { + seed := strings.Join([]string{ + scope, + endpoint.NodeID, + endpoint.NodeName, + endpoint.Host, + endpoint.IP, + strconv.Itoa(endpoint.NativeNodeID), + strconv.Itoa(ordinal), + }, "\x00") + sum := sha256.Sum256([]byte(seed)) + return scope + "-node-" + hex.EncodeToString(sum[:6]) +} + +func samePVEClusterNode(left, right ClusterEndpoint) bool { + if left.NativeNodeID != 0 && right.NativeNodeID != 0 { + return left.NativeNodeID == right.NativeNodeID + } + return strings.EqualFold(strings.TrimSpace(left.NodeName), strings.TrimSpace(right.NodeName)) +} + +// PVEClusterNodeIdentityForName returns the durable node identity associated +// with the provider's current native node name. +func PVEClusterNodeIdentityForName(instance *PVEInstance, nativeName string) string { + if instance == nil { + return "" + } + nativeName = strings.TrimSpace(nativeName) + for _, endpoint := range instance.ClusterEndpoints { + if strings.TrimSpace(endpoint.NodeName) == nativeName { + return endpoint.NodeIdentity + } + } + match := "" + for _, endpoint := range instance.ClusterEndpoints { + if !strings.EqualFold(strings.TrimSpace(endpoint.NodeName), nativeName) { + continue + } + if match != "" { + return "" + } + match = endpoint.NodeIdentity + } + return match +} + +// PVEClusterNodePresentation returns the immutable identity and preferred +// display label for a native node name. The native name is the deterministic +// fallback when no override exists. +func PVEClusterNodePresentation(instance *PVEInstance, nativeName string) (string, string) { + nativeName = strings.TrimSpace(nativeName) + if instance == nil { + return "", nativeName + } + identityID := PVEClusterNodeIdentityForName(instance, nativeName) + for _, identity := range instance.ClusterNodeIdentities { + if identity.ID != identityID { + continue + } + if identity.DisplayName != "" { + return identity.ID, identity.DisplayName + } + if identity.NativeName != "" { + return identity.ID, identity.NativeName + } + return identity.ID, nativeName + } + return identityID, nativeName +} + +// PVEClusterNodeIdentityByID returns a copy of one persisted identity. +func PVEClusterNodeIdentityByID(instance *PVEInstance, identityID string) (PVEClusterNodeIdentity, bool) { + if instance == nil { + return PVEClusterNodeIdentity{}, false + } + for _, identity := range instance.ClusterNodeIdentities { + if identity.ID == identityID { + identity.NativeAliases = append([]string(nil), identity.NativeAliases...) + return identity, true + } + } + return PVEClusterNodeIdentity{}, false +} + +// PVEClusterNodeNativeAliases returns prior provider-native names retained for +// diagnostics, history relabeling, and search after a native rename. +func PVEClusterNodeNativeAliases(instance *PVEInstance, nativeName string) []string { + identityID := PVEClusterNodeIdentityForName(instance, nativeName) + identity, ok := PVEClusterNodeIdentityByID(instance, identityID) + if !ok { + return nil + } + return identity.NativeAliases +} diff --git a/internal/config/pve_node_identity_test.go b/internal/config/pve_node_identity_test.go new file mode 100644 index 0000000000..0f331cb97d --- /dev/null +++ b/internal/config/pve_node_identity_test.go @@ -0,0 +1,153 @@ +package config + +import ( + "strings" + "testing" +) + +func TestEnsurePVEClusterNodeIdentitiesMigratesLegacyIDsWithoutChurn(t *testing.T) { + instances := []PVEInstance{{ + Name: "api-entry", + IsCluster: true, + ClusterName: "production", + ClusterEndpoints: []ClusterEndpoint{{ + NodeID: "node/pve1", + NativeNodeID: 1, + NodeName: "pve1", + }}, + }} + + if !EnsurePVEClusterNodeIdentities(instances) { + t.Fatal("expected legacy configuration to be migrated") + } + endpoint := instances[0].ClusterEndpoints[0] + if endpoint.NodeIdentity != "production-pve1" { + t.Fatalf("identity = %q, want historical ID %q", endpoint.NodeIdentity, "production-pve1") + } + if EnsurePVEClusterNodeIdentities(instances) { + t.Fatal("second normalization should be stable") + } +} + +func TestEnsurePVEClusterNodeIdentitiesScopesSameNamedClustersByConnection(t *testing.T) { + instances := []PVEInstance{ + { + Name: "site-a", IsCluster: true, ClusterName: "production", + ClusterEndpoints: []ClusterEndpoint{{NodeName: "pve1", NativeNodeID: 1}}, + }, + { + Name: "site-b", IsCluster: true, ClusterName: "production", + ClusterEndpoints: []ClusterEndpoint{{NodeName: "pve1", NativeNodeID: 1}}, + }, + } + + EnsurePVEClusterNodeIdentities(instances) + left := instances[0].ClusterEndpoints[0].NodeIdentity + right := instances[1].ClusterEndpoints[0].NodeIdentity + if left != "site-a-pve1" || right != "site-b-pve1" || left == right { + t.Fatalf("same-name cluster identities were not connection scoped: %q, %q", left, right) + } +} + +func TestEnsurePVEClusterNodeIdentitiesSurvivesRemovalRenameAndReIP(t *testing.T) { + instances := []PVEInstance{{ + Name: "edge", IsCluster: true, ClusterName: "edge-cluster", + ClusterEndpoints: []ClusterEndpoint{{ + NodeName: "pve-old", NativeNodeID: 7, IP: "10.0.0.7", + }}, + }} + EnsurePVEClusterNodeIdentities(instances) + identityID := instances[0].ClusterEndpoints[0].NodeIdentity + instances[0].ClusterNodeIdentities[0].DisplayName = "Render host" + + instances[0].ClusterEndpoints = nil + EnsurePVEClusterNodeIdentities(instances) + if len(instances[0].ClusterNodeIdentities) != 1 { + t.Fatal("confirmed membership removal must retain presentation identity metadata") + } + + instances[0].ClusterEndpoints = []ClusterEndpoint{{ + NodeName: "pve-new", NativeNodeID: 7, IP: "10.20.0.7", + }} + EnsurePVEClusterNodeIdentities(instances) + endpoint := instances[0].ClusterEndpoints[0] + if endpoint.NodeIdentity != identityID { + t.Fatalf("reappearing renamed member identity = %q, want %q", endpoint.NodeIdentity, identityID) + } + _, displayName := PVEClusterNodePresentation(&instances[0], "pve-new") + if displayName != "Render host" { + t.Fatalf("display override = %q, want retained override", displayName) + } + if instances[0].ClusterNodeIdentities[0].NativeName != "pve-new" { + t.Fatalf("native diagnostic name was not refreshed: %+v", instances[0].ClusterNodeIdentities[0]) + } + if len(instances[0].ClusterNodeIdentities[0].NativeAliases) != 1 || + instances[0].ClusterNodeIdentities[0].NativeAliases[0] != "pve-old" { + t.Fatalf("previous native name was not retained as a diagnostic/history alias: %+v", instances[0].ClusterNodeIdentities[0]) + } +} + +func TestEnsurePVEClusterNodeIdentitiesRepairsConflictingCaseNames(t *testing.T) { + instances := []PVEInstance{{ + Name: "case-cluster", IsCluster: true, + ClusterEndpoints: []ClusterEndpoint{ + {NodeName: "PVE", NativeNodeID: 1}, + {NodeName: "pve", NativeNodeID: 2}, + }, + }} + EnsurePVEClusterNodeIdentities(instances) + left := instances[0].ClusterEndpoints[0].NodeIdentity + right := instances[0].ClusterEndpoints[1].NodeIdentity + if left == "" || right == "" || left == right { + t.Fatalf("case-conflicting native names need distinct deterministic identities: %q, %q", left, right) + } +} + +func TestEnsurePVEClusterNodeIdentitiesFailsClosedOnConflictingNumericLedger(t *testing.T) { + instances := []PVEInstance{{ + Name: "conflict", IsCluster: true, + ClusterEndpoints: []ClusterEndpoint{{ + NodeName: "pve-new", NativeNodeID: 7, + }}, + ClusterNodeIdentities: []PVEClusterNodeIdentity{ + {ID: "conflict-a", NativeNodeID: 7, NativeName: "pve-a", DisplayName: "A"}, + {ID: "conflict-b", NativeNodeID: 7, NativeName: "pve-b", DisplayName: "B"}, + }, + }} + + EnsurePVEClusterNodeIdentities(instances) + got := instances[0].ClusterEndpoints[0].NodeIdentity + if got == "conflict-a" || got == "conflict-b" || got == "" { + t.Fatalf("ambiguous numeric ledger must mint an isolated identity, got %q", got) + } +} + +func TestNormalizePVEClusterNodeDisplayName(t *testing.T) { + if got, err := NormalizePVEClusterNodeDisplayName(" Rendu Étage "); err != nil || got != "Rendu Étage" { + t.Fatalf("Unicode display name = %q, %v", got, err) + } + for _, invalid := range []string{"bad\nname", string([]byte{0xff}), strings.Repeat("界", 129)} { + if _, err := NormalizePVEClusterNodeDisplayName(invalid); err == nil { + t.Fatalf("expected invalid display name %q to fail", invalid) + } + } +} + +func TestConfigDeepCopyIsolatesPVEClusterIdentityMetadata(t *testing.T) { + original := &Config{PVEInstances: []PVEInstance{{ + Name: "cluster", IsCluster: true, + ClusterEndpoints: []ClusterEndpoint{{NodeName: "pve1", NodeIdentity: "cluster-pve1"}}, + ClusterNodeIdentities: []PVEClusterNodeIdentity{{ID: "cluster-pve1", NativeName: "pve1", DisplayName: "Compute"}}, + }}} + original.PVEInstances[0].ClusterNodeIdentities[0].NativeAliases = []string{"pve-old"} + clone := original.DeepCopy() + clone.PVEInstances[0].ClusterEndpoints[0].NodeName = "changed" + clone.PVEInstances[0].ClusterNodeIdentities[0].DisplayName = "Changed" + clone.PVEInstances[0].ClusterNodeIdentities[0].NativeAliases[0] = "changed" + + if original.PVEInstances[0].ClusterEndpoints[0].NodeName != "pve1" || + original.PVEInstances[0].ClusterNodeIdentities[0].DisplayName != "Compute" || + original.PVEInstances[0].ClusterNodeIdentities[0].NativeAliases[0] != "pve-old" { + t.Fatal("tenant-scoped deep copy leaked nested Proxmox cluster metadata") + } +} diff --git a/internal/models/converters.go b/internal/models/converters.go index e0b86eb67f..7c809f9066 100644 --- a/internal/models/converters.go +++ b/internal/models/converters.go @@ -1123,6 +1123,8 @@ type ResourceConvertInput struct { SourceType string Sources []string ParentID string + ParentName string + ChildCount int ClusterID string Status string CPU *ResourceMetricInput @@ -1215,6 +1217,8 @@ func ConvertResourceToFrontend(input ResourceConvertInput) ResourceFrontend { SourceType: input.SourceType, Sources: append([]string(nil), input.Sources...), ParentID: input.ParentID, + ParentName: input.ParentName, + ChildCount: input.ChildCount, ClusterID: input.ClusterID, Status: input.Status, Temperature: input.Temperature, diff --git a/internal/models/deepcopy.go b/internal/models/deepcopy.go index 3b38647342..88eab56a7f 100644 --- a/internal/models/deepcopy.go +++ b/internal/models/deepcopy.go @@ -143,6 +143,7 @@ func cloneGuestNetworkInterfaces(src []GuestNetworkInterface) []GuestNetworkInte func cloneNode(src Node) Node { dest := src dest.LoadAverage = append([]float64(nil), src.LoadAverage...) + dest.NativeNameAliases = append([]string(nil), src.NativeNameAliases...) dest.Temperature = cloneTemperature(src.Temperature) dest.TemperatureMonitoringEnabled = cloneBoolPtr(src.TemperatureMonitoringEnabled) return dest.NormalizeCollections() diff --git a/internal/models/models.go b/internal/models/models.go index b2f3e71ddc..e0eb06fb2e 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -94,7 +94,9 @@ type ResolvedAlert struct { // Node represents a Proxmox VE node type Node struct { ID string `json:"id"` + NodeIdentity string `json:"nodeIdentity,omitempty"` Name string `json:"name"` + NativeNameAliases []string `json:"nativeNameAliases,omitempty"` DisplayName string `json:"displayName,omitempty"` Instance string `json:"instance"` Host string `json:"host"` // Full host URL from config @@ -115,6 +117,7 @@ type Node struct { ConnectionHealth string `json:"connectionHealth"` IsClusterMember bool `json:"isClusterMember"` // True if part of a cluster ClusterName string `json:"clusterName"` // Name of cluster (empty if standalone) + ProviderScopedIdentity bool `json:"providerScopedIdentity,omitempty"` // Package updates - polled less frequently (every 30 mins) PendingUpdates int `json:"pendingUpdates"` // Number of pending apt updates @@ -3368,6 +3371,9 @@ func nodeEndpointMergeAliases(node Node) []string { } func nodeLogicalKey(node Node) string { + if identity := normalizeNodeIdentityPart(node.NodeIdentity); identity != "" { + return "node-identity:" + identity + } name := normalizeNodeIdentityPart(node.Name) if name == "" { return "" diff --git a/internal/models/models_frontend.go b/internal/models/models_frontend.go index 54f724e381..885a4acc64 100644 --- a/internal/models/models_frontend.go +++ b/internal/models/models_frontend.go @@ -1006,8 +1006,10 @@ type ResourceFrontend struct { Sources []string `json:"sources,omitempty"` // Hierarchy - ParentID string `json:"parentId,omitempty"` - ClusterID string `json:"clusterId,omitempty"` + ParentID string `json:"parentId,omitempty"` + ParentName string `json:"parentName,omitempty"` + ChildCount int `json:"childCount,omitempty"` + ClusterID string `json:"clusterId,omitempty"` // Universal Metrics Status string `json:"status"` diff --git a/internal/models/pve_node_identity_test.go b/internal/models/pve_node_identity_test.go new file mode 100644 index 0000000000..7064d011f8 --- /dev/null +++ b/internal/models/pve_node_identity_test.go @@ -0,0 +1,43 @@ +package models + +import "testing" + +func TestUpdateNodesForInstanceKeepsAgentLinkAcrossNativeRename(t *testing.T) { + state := NewState() + state.Nodes = []Node{{ + ID: "production-pve-old", NodeIdentity: "production-pve-old", + Name: "pve-old", Instance: "cluster-api", ClusterName: "production", + IsClusterMember: true, LinkedAgentID: "agent-1", Status: "online", + }} + state.Hosts = []Host{{ID: "agent-1", Hostname: "pve-new", Status: "online"}} + + state.UpdateNodesForInstance("cluster-api", []Node{{ + ID: "production-pve-old", NodeIdentity: "production-pve-old", + Name: "pve-new", Instance: "cluster-api", ClusterName: "production", + IsClusterMember: true, Status: "online", + }}) + + if len(state.Nodes) != 1 { + t.Fatalf("native rename duplicated node state: %+v", state.Nodes) + } + if state.Nodes[0].Name != "pve-new" || state.Nodes[0].LinkedAgentID != "agent-1" { + t.Fatalf("native rename lost immutable identity/link: %+v", state.Nodes[0]) + } +} + +func TestCloneNodeIsolatesNativeNameAliases(t *testing.T) { + original := Node{ + ID: "production-pve1", + NodeIdentity: "production-pve1", + Name: "pve1", + NativeNameAliases: []string{"pve-old"}, + DisplayName: "Render East", + } + + cloned := cloneNode(original) + cloned.NativeNameAliases[0] = "changed" + + if original.NativeNameAliases[0] != "pve-old" { + t.Fatalf("node snapshot clone shared native alias storage: %+v", original) + } +} diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index 6e8e823e23..5372696a05 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -1295,7 +1295,7 @@ func TestStoragePollingKeepsSharedStorageClusterStatusCanonical(t *testing.T) { } } -func TestMonitoringProxmoxClusterNodeDisplayNamesStayClusterQualified(t *testing.T) { +func TestMonitoringProxmoxClusterNodeDisplayNamesUsePersistedPresentationIdentity(t *testing.T) { data, err := os.ReadFile("monitor.go") if err != nil { t.Fatalf("failed to read monitor.go: %v", err) @@ -1304,9 +1304,7 @@ func TestMonitoringProxmoxClusterNodeDisplayNamesStayClusterQualified(t *testing for _, snippet := range []string{ "func getNodeDisplayName(instance *config.PVEInstance, nodeName string) string {", - "clusterLabel := friendly", - "clusterLabel = strings.TrimSpace(instance.ClusterName)", - `return fmt.Sprintf("%s (%s)", clusterLabel, baseName)`, + "config.PVEClusterNodePresentation(instance, baseName)", "return baseName", } { if !strings.Contains(source, snippet) { diff --git a/internal/monitoring/helpers_test.go b/internal/monitoring/helpers_test.go index 444e5497c5..e465140c0f 100644 --- a/internal/monitoring/helpers_test.go +++ b/internal/monitoring/helpers_test.go @@ -136,23 +136,23 @@ func TestGetNodeDisplayName(t *testing.T) { // non-cluster: returns unknown-node when host is IP address {"host IP fallback to unknown-node", &config.PVEInstance{Name: "", Host: "https://192.168.1.100:8006"}, "", "unknown-node"}, - // cluster: configured instance label disambiguates the Proxmox node name - {"cluster configured label", clusterInstance, "node1", "cluster (node1)"}, + // cluster: native node name is the deterministic fallback + {"cluster configured label", clusterInstance, "node1", "node1"}, // cluster: still uses configured label even when no endpoint label is present - {"cluster base fallback", clusterInstance, "node3", "cluster (node3)"}, + {"cluster base fallback", clusterInstance, "node3", "node3"}, // cluster: keeps the node identity instead of substituting endpoint IP labels - {"cluster ip fallback", clusterInstance, "node2", "cluster (node2)"}, + {"cluster ip fallback", clusterInstance, "node2", "node2"}, // cluster: ClusterName backs the display label when the user-facing name is empty - {"cluster name fallback", &config.PVEInstance{IsCluster: true, ClusterName: "detected-cluster"}, "node4", "detected-cluster (node4)"}, + {"cluster name fallback", &config.PVEInstance{IsCluster: true, ClusterName: "detected-cluster"}, "node4", "node4"}, // cluster: do not duplicate the label when it already equals the node name {"cluster duplicate label", &config.PVEInstance{IsCluster: true, Name: "node5", ClusterName: "detected-cluster"}, "node5", "node5"}, - // cluster: falls back to friendly name when baseName is "unknown-node" - {"cluster friendly fallback", &config.PVEInstance{IsCluster: true, Name: "Cluster Name", ClusterEndpoints: []config.ClusterEndpoint{}}, "", "Cluster Name"}, + // cluster: missing native identity remains explicitly unknown + {"cluster friendly fallback", &config.PVEInstance{IsCluster: true, Name: "Cluster Name", ClusterEndpoints: []config.ClusterEndpoint{}}, "", "unknown-node"}, // cluster: returns unknown-node when no fallbacks available {"cluster no fallbacks", &config.PVEInstance{IsCluster: true, Name: "", Host: "pve.example.com", ClusterEndpoints: []config.ClusterEndpoint{}}, "", "unknown-node"}, diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 5cfe87bde5..f209288c8c 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -185,35 +185,15 @@ func getNodeDisplayName(instance *config.PVEInstance, nodeName string) string { return baseName } - friendly := strings.TrimSpace(instance.Name) - if instance.IsCluster { - clusterLabel := friendly - if clusterLabel == "" { - clusterLabel = strings.TrimSpace(instance.ClusterName) + _, displayName := config.PVEClusterNodePresentation(instance, baseName) + if displayName != "" { + return displayName } - - if baseName != "" && baseName != "unknown-node" { - if clusterLabel != "" && !strings.EqualFold(clusterLabel, baseName) { - return fmt.Sprintf("%s (%s)", clusterLabel, baseName) - } - return baseName - } - - if endpointLabel := lookupClusterEndpointLabel(instance, nodeName); endpointLabel != "" { - if clusterLabel != "" && !strings.EqualFold(clusterLabel, endpointLabel) { - return fmt.Sprintf("%s (%s)", clusterLabel, endpointLabel) - } - return endpointLabel - } - - if clusterLabel != "" { - return clusterLabel - } - return baseName } + friendly := strings.TrimSpace(instance.Name) if friendly != "" { return friendly } @@ -5743,6 +5723,8 @@ func monitorResourceToConvertInput(resource unifiedresources.Resource) models.Re SourceType: monitorSourceType(resource.Sources), Sources: monitorSourceKeys(resource.Sources), ParentID: monitorStringValue(resource.ParentID), + ParentName: resource.ParentName, + ChildCount: resource.ChildCount, ClusterID: monitorClusterID(resource), Status: monitorFrontendStatus(resource, resourceType), CPU: monitorMetricInput(monitorMetricValue(resource.Metrics, func(metrics *unifiedresources.ResourceMetrics) *unifiedresources.MetricValue { return metrics.CPU })), diff --git a/internal/monitoring/monitor_cluster_dedupe.go b/internal/monitoring/monitor_cluster_dedupe.go index 122e37e161..149ed626cc 100644 --- a/internal/monitoring/monitor_cluster_dedupe.go +++ b/internal/monitoring/monitor_cluster_dedupe.go @@ -33,7 +33,7 @@ func (m *Monitor) consolidateDuplicateClusters() { } func (m *Monitor) normalizePVEConfigState() bool { - if m == nil || m.config == nil || len(m.config.PVEInstances) < 2 { + if m == nil || m.config == nil { return false } @@ -44,11 +44,15 @@ func (m *Monitor) normalizePVEConfigState() bool { } } - instances, changed := config.ConsolidatePVEInstances(m.config.PVEInstances) - if !changed { + instances, consolidated := config.ConsolidatePVEInstances(m.config.PVEInstances) + if consolidated { + m.config.PVEInstances = instances + } + identityChanged := config.EnsurePVEClusterNodeIdentities(m.config.PVEInstances) + if !consolidated && !identityChanged { return false } - m.config.PVEInstances = instances + instances = m.config.PVEInstances remainingNames := make(map[string]struct{}, len(instances)) for _, instance := range instances { diff --git a/internal/monitoring/monitor_host_agent_removal_lifecycle_test.go b/internal/monitoring/monitor_host_agent_removal_lifecycle_test.go index c3fa53d718..53e10a8605 100644 --- a/internal/monitoring/monitor_host_agent_removal_lifecycle_test.go +++ b/internal/monitoring/monitor_host_agent_removal_lifecycle_test.go @@ -296,6 +296,45 @@ func TestHostAgentRemovalLifecycleDoesNotPoisonDuplicateActiveIdentity(t *testin } } +func TestHostAgentRemovalLifecycleDoesNotUseProxmoxDisplayNameAsIdentity(t *testing.T) { + monitor := newHostRemovalLifecycleMonitor(t, t.TempDir()) + monitor.state.UpdateNodesForInstance("production-api", []models.Node{{ + ID: "production-pve1", + NodeIdentity: "production-pve1", + Name: "pve1", + DisplayName: "Render East", + Instance: "production-api", + IsClusterMember: true, + LinkedAgentID: "agent-machine", + }}) + + now := time.Now().UTC() + report := hostRemovalLifecycleReport( + "agent-machine", + "agent-machine", + "agent-state", + "pve1", + "linux", + now, + ) + token := &config.APITokenRecord{ID: "agent-token", CreatedAt: now.Add(-time.Hour)} + host, err := monitor.ApplyHostReport(report, token) + if err != nil { + t.Fatalf("ApplyHostReport: %v", err) + } + if _, err := monitor.RemoveHostAgent(host.ID); err != nil { + t.Fatalf("RemoveHostAgent: %v", err) + } + + nodes := monitor.state.GetSnapshot().Nodes + if len(nodes) != 1 || + nodes[0].NodeIdentity != "production-pve1" || + nodes[0].Name != "pve1" || + nodes[0].DisplayName != "Render East" { + t.Fatalf("agent removal changed provider-owned node presentation identity: %+v", nodes) + } +} + func TestHostAgentRemovalLifecycleFailsClosedWhenTombstoneCannotPersist(t *testing.T) { dataPath := filepath.Join(t.TempDir(), "not-a-directory") if err := os.WriteFile(dataPath, []byte("fixture"), 0o600); err != nil { diff --git a/internal/monitoring/monitor_integration_test.go b/internal/monitoring/monitor_integration_test.go index 6a12bce904..db2dd0d5ad 100644 --- a/internal/monitoring/monitor_integration_test.go +++ b/internal/monitoring/monitor_integration_test.go @@ -454,8 +454,8 @@ func TestGetNodeDisplayName_FriendlyName(t *testing.T) { func TestGetNodeDisplayName_ClusterUsesNodeName(t *testing.T) { inst := &config.PVEInstance{Name: "cluster1", IsCluster: true} got := getNodeDisplayName(inst, "node1") - if got != "cluster1 (node1)" { - t.Errorf("cluster mode should include configured cluster label, got %q", got) + if got != "node1" { + t.Errorf("cluster mode should fall back to the native node name, got %q", got) } } @@ -464,11 +464,14 @@ func TestGetNodeDisplayName_ClusterWithEndpointLabel(t *testing.T) { Name: "cluster1", IsCluster: true, ClusterEndpoints: []config.ClusterEndpoint{ - {NodeName: "node1", NodeID: "node/node1"}, + {NodeName: "node1", NodeID: "node/node1", NodeIdentity: "cluster1-node1"}, }, + ClusterNodeIdentities: []config.PVEClusterNodeIdentity{{ + ID: "cluster1-node1", NativeName: "node1", DisplayName: "Compute one", + }}, } got := getNodeDisplayName(inst, "node1") - if got != "cluster1 (node1)" { - t.Errorf("expected cluster-qualified node display name, got %q", got) + if got != "Compute one" { + t.Errorf("expected persisted cluster-node display name, got %q", got) } } diff --git a/internal/monitoring/monitor_polling_node.go b/internal/monitoring/monitor_polling_node.go index 98b80eacda..85d5efec9f 100644 --- a/internal/monitoring/monitor_polling_node.go +++ b/internal/monitoring/monitor_polling_node.go @@ -57,7 +57,9 @@ func (m *Monitor) pollPVENode( modelNode := models.Node{ ID: nodeID, + NodeIdentity: nodeID, Name: node.Node, + NativeNameAliases: config.PVEClusterNodeNativeAliases(instanceCfg, node.Node), DisplayName: displayName, Instance: instanceName, Host: connectionHost, @@ -72,6 +74,7 @@ func (m *Monitor) pollPVENode( ConnectionHealth: connectionHealthStr, // Use the determined health status IsClusterMember: instanceCfg.IsCluster, ClusterName: instanceCfg.ClusterName, + ProviderScopedIdentity: m.pveNodeUsesProviderScopedIdentity(instanceName, instanceCfg), TemperatureMonitoringEnabled: instanceCfg.TemperatureMonitoringEnabled, } var nodeDiskSource string diff --git a/internal/monitoring/monitor_polling_node_helpers.go b/internal/monitoring/monitor_polling_node_helpers.go index 3b32de71dd..711a8f11c4 100644 --- a/internal/monitoring/monitor_polling_node_helpers.go +++ b/internal/monitoring/monitor_polling_node_helpers.go @@ -41,7 +41,7 @@ func (m *Monitor) determineNodeIDAndStatus(instanceName string, instanceCfg *con // but two unrelated clusters are allowed to share that display name. In // that ambiguous case the instance name prevents cross-cluster resource // collisions without churning IDs for established unambiguous clusters. - nodeID := m.pveNodeIdentityScope(instanceName, instanceCfg) + "-" + node.Node + nodeID := m.pveNodeID(instanceName, instanceCfg, node.Node) effectiveStatus := node.Status now := time.Now() gracePeriod := m.pveNodeOfflineGracePeriod() @@ -78,6 +78,25 @@ func (m *Monitor) determineNodeIDAndStatus(instanceName string, instanceCfg *con return nodeID, effectiveStatus } +func (m *Monitor) pveNodeID(instanceName string, instanceCfg *config.PVEInstance, nativeName string) string { + if instanceCfg != nil && instanceCfg.IsCluster { + if identityID := config.PVEClusterNodeIdentityForName(instanceCfg, nativeName); identityID != "" { + return identityID + } + } + return m.pveNodeIdentityScope(instanceName, instanceCfg) + "-" + nativeName +} + +func (m *Monitor) pveNodeUsesProviderScopedIdentity(instanceName string, instanceCfg *config.PVEInstance) bool { + if instanceCfg == nil || !instanceCfg.IsCluster || strings.TrimSpace(instanceCfg.ClusterName) == "" { + return false + } + return !strings.EqualFold( + strings.TrimSpace(m.pveNodeIdentityScope(instanceName, instanceCfg)), + strings.TrimSpace(instanceCfg.ClusterName), + ) +} + func (m *Monitor) collectNodeTemperatureData( ctx context.Context, instanceName string, diff --git a/internal/monitoring/monitor_pve.go b/internal/monitoring/monitor_pve.go index 2aeefae1a1..5f4ced3161 100644 --- a/internal/monitoring/monitor_pve.go +++ b/internal/monitoring/monitor_pve.go @@ -656,11 +656,13 @@ func (m *Monitor) markPVEInstanceNodesUnreachable(instanceName string) { } func (m *Monitor) placeholderNodeForInstance(instanceName string, instanceCfg *config.PVEInstance, nodeName string) models.Node { - nodeID := m.pveNodeIdentityScope(instanceName, instanceCfg) + "-" + nodeName + nodeID := m.pveNodeID(instanceName, instanceCfg, nodeName) connectionHost, guestURL := resolveNodeConnectionInfo(instanceCfg, monitorDiscoveryConfig(m), nodeName) return models.Node{ ID: nodeID, + NodeIdentity: nodeID, Name: nodeName, + NativeNameAliases: config.PVEClusterNodeNativeAliases(instanceCfg, nodeName), DisplayName: getNodeDisplayName(instanceCfg, nodeName), Instance: instanceName, Host: connectionHost, @@ -671,6 +673,7 @@ func (m *Monitor) placeholderNodeForInstance(instanceName string, instanceCfg *c LoadAverage: []float64{}, IsClusterMember: instanceCfg.IsCluster, ClusterName: instanceCfg.ClusterName, + ProviderScopedIdentity: m.pveNodeUsesProviderScopedIdentity(instanceName, instanceCfg), TemperatureMonitoringEnabled: instanceCfg.TemperatureMonitoringEnabled, } } @@ -759,7 +762,10 @@ func (m *Monitor) preserveOrExpireNodes(prevInstanceNodes []models.Node) []model func (m *Monitor) seedNodeDisplayNames(modelNodes []models.Node) { for i := range modelNodes { if modelNodes[i].DisplayName != "" { - m.alertManager.UpdateNodeDisplayName(modelNodes[i].Name, modelNodes[i].DisplayName) + m.alertManager.UpdateNodeDisplayName(modelNodes[i].Instance, modelNodes[i].Name, modelNodes[i].DisplayName) + for _, alias := range modelNodes[i].NativeNameAliases { + m.alertManager.UpdateNodeDisplayName(modelNodes[i].Instance, alias, modelNodes[i].DisplayName) + } } } } diff --git a/internal/monitoring/monitor_pve_cluster.go b/internal/monitoring/monitor_pve_cluster.go index 76ec65699b..083948619f 100644 --- a/internal/monitoring/monitor_pve_cluster.go +++ b/internal/monitoring/monitor_pve_cluster.go @@ -96,7 +96,8 @@ func clusterEndpointsFromStatus( for _, clusterNode := range members { lastSeen := time.Time{} for _, endpoint := range existing { - if strings.EqualFold(strings.TrimSpace(endpoint.NodeName), strings.TrimSpace(clusterNode.Name)) { + if (clusterNode.Nodeid != 0 && endpoint.NativeNodeID == clusterNode.Nodeid) || + strings.EqualFold(strings.TrimSpace(endpoint.NodeName), strings.TrimSpace(clusterNode.Name)) { lastSeen = endpoint.LastSeen break } @@ -105,15 +106,16 @@ func clusterEndpointsFromStatus( lastSeen = time.Now() } endpoints = append(endpoints, config.ClusterEndpoint{ - NodeID: clusterNode.ID, - NodeName: clusterNode.Name, - Host: monitorBuildClusterEndpointHost(scheme, clusterNode.Name, port), - IP: strings.TrimSpace(clusterNode.IP), - GuestURL: monitorExistingClusterGuestURL(clusterNode.Name, existing), - IPOverride: monitorExistingClusterIPOverride(clusterNode.Name, existing), - Fingerprint: monitorExistingClusterFingerprint(clusterNode.Name, existing), - Online: clusterNode.Online == 1, - LastSeen: lastSeen, + NodeID: clusterNode.ID, + NativeNodeID: clusterNode.Nodeid, + NodeName: clusterNode.Name, + Host: monitorBuildClusterEndpointHost(scheme, clusterNode.Name, port), + IP: strings.TrimSpace(clusterNode.IP), + GuestURL: monitorExistingClusterGuestURL(clusterNode.Name, existing), + IPOverride: monitorExistingClusterIPOverride(clusterNode.Name, existing), + Fingerprint: monitorExistingClusterFingerprint(clusterNode.Name, existing), + Online: clusterNode.Online == 1, + LastSeen: lastSeen, }) } sort.Slice(endpoints, func(i, j int) bool { @@ -557,11 +559,11 @@ func (m *Monitor) refreshClusterEndpoints(instanceName string, instanceCfg *conf // would present stale reachability evidence after a cluster network move. func mergeRefreshedClusterEndpoints(existing, discovered []config.ClusterEndpoint, verifySSL bool) []config.ClusterEndpoint { merged := make([]config.ClusterEndpoint, 0, len(discovered)) + used := make(map[int]struct{}, len(existing)) for _, ep := range discovered { - for _, old := range existing { - if !strings.EqualFold(strings.TrimSpace(old.NodeName), strings.TrimSpace(ep.NodeName)) { - continue - } + if oldIdx := refreshedClusterEndpointMatch(existing, ep, used); oldIdx >= 0 { + old := existing[oldIdx] + used[oldIdx] = struct{}{} oldEffectiveURL := clusterEndpointEffectiveURL(old, verifySSL, false) if strings.TrimSpace(ep.IP) == "" { ep.IP = old.IP @@ -572,18 +574,74 @@ func mergeRefreshedClusterEndpoints(existing, discovered []config.ClusterEndpoin if strings.TrimSpace(ep.NodeID) == "" { ep.NodeID = old.NodeID } + if ep.NativeNodeID == 0 { + ep.NativeNodeID = old.NativeNodeID + } + ep.NodeIdentity = old.NodeIdentity + if strings.TrimSpace(ep.GuestURL) == "" { + ep.GuestURL = old.GuestURL + } + if strings.TrimSpace(ep.IPOverride) == "" { + ep.IPOverride = old.IPOverride + } + if strings.TrimSpace(ep.Fingerprint) == "" { + ep.Fingerprint = old.Fingerprint + } if clusterEndpointEffectiveURL(ep, verifySSL, false) == oldEffectiveURL { ep.PulseReachable = old.PulseReachable ep.LastPulseCheck = old.LastPulseCheck ep.PulseError = old.PulseError } - break } merged = append(merged, ep) } return merged } +func refreshedClusterEndpointMatch(existing []config.ClusterEndpoint, discovered config.ClusterEndpoint, used map[int]struct{}) int { + matchUnique := func(predicate func(config.ClusterEndpoint) bool) int { + match := -1 + for idx, candidate := range existing { + if _, alreadyUsed := used[idx]; alreadyUsed || !predicate(candidate) { + continue + } + if match >= 0 { + return -1 + } + match = idx + } + return match + } + + if discovered.NodeIdentity != "" { + if match := matchUnique(func(candidate config.ClusterEndpoint) bool { + return candidate.NodeIdentity == discovered.NodeIdentity + }); match >= 0 { + return match + } + } + if discovered.NativeNodeID != 0 { + if match := matchUnique(func(candidate config.ClusterEndpoint) bool { + return candidate.NativeNodeID != 0 && candidate.NativeNodeID == discovered.NativeNodeID + }); match >= 0 { + return match + } + } + if discovered.NodeName != "" { + if match := matchUnique(func(candidate config.ClusterEndpoint) bool { + return strings.EqualFold(strings.TrimSpace(candidate.NodeName), strings.TrimSpace(discovered.NodeName)) + }); match >= 0 { + return match + } + } + return matchUnique(func(candidate config.ClusterEndpoint) bool { + if discovered.IP != "" && strings.TrimSpace(candidate.IP) == strings.TrimSpace(discovered.IP) { + return true + } + return discovered.Host != "" && strings.EqualFold(strings.TrimSpace(candidate.Host), strings.TrimSpace(discovered.Host)) + }) +} + // clusterEndpointIdentityChanged reports whether the set of nodes or any // node's address changed. Volatile fields (Online, LastSeen, Pulse // reachability) are deliberately ignored - they change every poll and are not @@ -592,18 +650,28 @@ func clusterEndpointIdentityChanged(existing, refreshed []config.ClusterEndpoint if len(existing) != len(refreshed) { return true } - byName := make(map[string]config.ClusterEndpoint, len(existing)) + byIdentity := make(map[string]config.ClusterEndpoint, len(existing)) for _, ep := range existing { - byName[strings.ToLower(strings.TrimSpace(ep.NodeName))] = ep + key := strings.TrimSpace(ep.NodeIdentity) + if key == "" { + key = strings.ToLower(strings.TrimSpace(ep.NodeName)) + } + byIdentity[key] = ep } for _, ep := range refreshed { - old, ok := byName[strings.ToLower(strings.TrimSpace(ep.NodeName))] + key := strings.TrimSpace(ep.NodeIdentity) + if key == "" { + key = strings.ToLower(strings.TrimSpace(ep.NodeName)) + } + old, ok := byIdentity[key] if !ok { return true } if strings.TrimSpace(old.IP) != strings.TrimSpace(ep.IP) || strings.TrimSpace(old.Host) != strings.TrimSpace(ep.Host) || - strings.TrimSpace(old.NodeID) != strings.TrimSpace(ep.NodeID) { + strings.TrimSpace(old.NodeID) != strings.TrimSpace(ep.NodeID) || + old.NativeNodeID != ep.NativeNodeID || + !strings.EqualFold(strings.TrimSpace(old.NodeName), strings.TrimSpace(ep.NodeName)) { return true } } diff --git a/internal/monitoring/monitor_pve_cluster_refresh_test.go b/internal/monitoring/monitor_pve_cluster_refresh_test.go index 87d4737596..430e806bc1 100644 --- a/internal/monitoring/monitor_pve_cluster_refresh_test.go +++ b/internal/monitoring/monitor_pve_cluster_refresh_test.go @@ -289,6 +289,40 @@ func TestMergeRefreshedClusterEndpoints_ReachabilityFollowsEffectiveTarget(t *te }) } +func TestMergeRefreshedClusterEndpoints_RenameAndReIPKeepImmutableIdentity(t *testing.T) { + existing := []config.ClusterEndpoint{{ + NodeID: "node/pve-old", + NodeIdentity: "cluster-pve-old", + NativeNodeID: 42, + NodeName: "pve-old", + Host: "https://pve-old:8006", + IP: "10.0.0.42", + IPOverride: "pulse-route.example.test", + GuestURL: "https://console.example.test", + Fingerprint: "AA:BB", + }} + discovered := []config.ClusterEndpoint{{ + NodeID: "node/pve-new", + NativeNodeID: 42, + NodeName: "pve-new", + Host: "https://pve-new:8006", + IP: "10.20.0.42", + }} + + merged := mergeRefreshedClusterEndpoints(existing, discovered, false) + if len(merged) != 1 || merged[0].NodeIdentity != "cluster-pve-old" { + t.Fatalf("immutable identity did not follow native numeric identity: %+v", merged) + } + if merged[0].IPOverride != "pulse-route.example.test" || + merged[0].GuestURL != "https://console.example.test" || + merged[0].Fingerprint != "AA:BB" { + t.Fatalf("user-managed member fields did not survive rename/re-IP: %+v", merged[0]) + } + if !clusterEndpointIdentityChanged(existing, merged) { + t.Fatal("native rename and re-IP must trigger endpoint persistence without changing NodeIdentity") + } +} + func TestBuildClusterEndpoints_PreservesConfiguredAuthority(t *testing.T) { monitor := &Monitor{config: &config.Config{}} instance := config.PVEInstance{ diff --git a/internal/monitoring/node_memory_sources_test.go b/internal/monitoring/node_memory_sources_test.go index e19ab03ffb..9a4b9267ac 100644 --- a/internal/monitoring/node_memory_sources_test.go +++ b/internal/monitoring/node_memory_sources_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/rcourtman/pulse-go-rewrite/internal/config" "github.com/rcourtman/pulse-go-rewrite/internal/models" "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox" ) @@ -137,6 +138,17 @@ func TestPollPVENodePrefersLinkedHostDiskOverRootFS(t *testing.T) { mon := newTestPVEMonitor("test") defer mon.alertManager.Stop() defer mon.notificationMgr.Stop() + mon.config.PVEInstances[0].IsCluster = true + mon.config.PVEInstances[0].ClusterName = "production" + mon.config.PVEInstances[0].ClusterEndpoints = []config.ClusterEndpoint{{ + NodeIdentity: "production-ebringa", + NodeName: "ebringa", + }} + mon.config.PVEInstances[0].ClusterNodeIdentities = []config.PVEClusterNodeIdentity{{ + ID: "production-ebringa", + NativeName: "ebringa", + DisplayName: "Render East", + }} mon.state.UpsertHost(models.Host{ ID: "host-1", @@ -148,8 +160,10 @@ func TestPollPVENodePrefersLinkedHostDiskOverRootFS(t *testing.T) { }) mon.state.UpdateNodesForInstance("test", []models.Node{ { - ID: "test-ebringa", + ID: "production-ebringa", + NodeIdentity: "production-ebringa", Name: "ebringa", + DisplayName: "Render East", Instance: "test", LinkedAgentID: "host-1", }, @@ -190,6 +204,11 @@ func TestPollPVENodePrefersLinkedHostDiskOverRootFS(t *testing.T) { if modelNode.Disk.Usage < 42 || modelNode.Disk.Usage > 42.1 { t.Fatalf("node disk usage = %.2f, want linked host usage around 42.05", modelNode.Disk.Usage) } + if modelNode.NodeIdentity != "production-ebringa" || + modelNode.Name != "ebringa" || + modelNode.DisplayName != "Render East" { + t.Fatalf("cluster presentation identity corrupted linked-agent lifecycle fields: %+v", modelNode) + } } func TestResolveNodeDiskFallsBackToRootFSWithoutLinkedHost(t *testing.T) { diff --git a/internal/unifiedresources/adapters.go b/internal/unifiedresources/adapters.go index d880ea8854..c535201fb2 100644 --- a/internal/unifiedresources/adapters.go +++ b/internal/unifiedresources/adapters.go @@ -23,7 +23,7 @@ func resourceFromProxmoxNode(node models.Node, linkedHost *models.Host) (Resourc endpointHost := extractHostname(node.Host) identity := ResourceIdentity{ - Hostnames: uniqueStrings([]string{node.Name}), + Hostnames: uniqueStrings(append([]string{node.Name}, node.NativeNameAliases...)), ClusterName: proxmoxNodeIdentityClusterName(node), } if endpointHost != "" { @@ -45,10 +45,17 @@ func resourceFromProxmoxNode(node models.Node, linkedHost *models.Host) (Resourc if linkedAgentID == "" && linkedHost != nil { linkedAgentID = strings.TrimSpace(linkedHost.ID) } + nodeDisplayName := "" + if node.IsClusterMember && !strings.EqualFold(strings.TrimSpace(node.DisplayName), strings.TrimSpace(node.Name)) { + nodeDisplayName = strings.TrimSpace(node.DisplayName) + } proxmox := &ProxmoxData{ SourceID: node.ID, + NodeIdentity: node.NodeIdentity, NodeName: node.Name, + NodeAliases: append([]string(nil), node.NativeNameAliases...), + NodeDisplayName: nodeDisplayName, ClusterName: node.ClusterName, IsClusterMember: node.IsClusterMember, Instance: node.Instance, @@ -99,6 +106,9 @@ func resourceFromProxmoxNode(node models.Node, linkedHost *models.Host) (Resourc // contributes stronger machine identity. func proxmoxNodeIdentityClusterName(node models.Node) string { clusterName := strings.TrimSpace(node.ClusterName) + if node.ProviderScopedIdentity { + return "" + } instance := strings.TrimSpace(node.Instance) nodeName := strings.TrimSpace(node.Name) if instance == "" || nodeName == "" || strings.EqualFold(instance, clusterName) { diff --git a/internal/unifiedresources/adapters_test.go b/internal/unifiedresources/adapters_test.go index 828b5c8f43..87e813497c 100644 --- a/internal/unifiedresources/adapters_test.go +++ b/internal/unifiedresources/adapters_test.go @@ -43,18 +43,22 @@ func TestResourceFromProxmoxNodeIncludesTemperature(t *testing.T) { func TestResourceFromProxmoxNodeUsesDisplayNameWithoutChangingRawNodeIdentity(t *testing.T) { node := models.Node{ - ID: "pve-cluster-node01", - Name: "node01", - DisplayName: "PVE Cluster (node01)", - ClusterName: "PVE Cluster", - Status: "online", + ID: "pve-cluster-node01", + NodeIdentity: "pve-cluster-node01", + Name: "node01", + DisplayName: "Compute East", + ClusterName: "PVE Cluster", + IsClusterMember: true, + Status: "online", } resource, identity := resourceFromProxmoxNode(node, nil) - if resource.Name != "PVE Cluster (node01)" { - t.Fatalf("resource.Name = %q, want cluster-qualified display label", resource.Name) + if resource.Name != "Compute East" { + t.Fatalf("resource.Name = %q, want custom display label", resource.Name) } - if resource.Proxmox == nil || resource.Proxmox.NodeName != "node01" { + if resource.Proxmox == nil || resource.Proxmox.NodeName != "node01" || + resource.Proxmox.NodeIdentity != "pve-cluster-node01" || + resource.Proxmox.NodeDisplayName != "Compute East" { t.Fatalf("Proxmox.NodeName = %+v, want raw node identity node01", resource.Proxmox) } foundClusterIdentity := false @@ -69,6 +73,23 @@ func TestResourceFromProxmoxNodeUsesDisplayNameWithoutChangingRawNodeIdentity(t } } +func TestResourceFromRenamedNodeKeepsSameNameClustersProviderScoped(t *testing.T) { + node := models.Node{ + ID: "site-a-pve-old", NodeIdentity: "site-a-pve-old", + Name: "pve-new", DisplayName: "Render East", + Instance: "site-a", ClusterName: "production", IsClusterMember: true, + ProviderScopedIdentity: true, Status: "online", + } + resource, identity := resourceFromProxmoxNode(node, nil) + if identity.ClusterName != "" { + t.Fatalf("same-name cluster identity became globally cluster scoped after native rename: %+v", identity) + } + if resource.Proxmox == nil || resource.Proxmox.SourceID != "site-a-pve-old" || + resource.Proxmox.NodeName != "pve-new" { + t.Fatalf("stable source ID/native name separation failed: %+v", resource.Proxmox) + } +} + func TestResourceFromHostAdvertisesElevatedTypedPackageUpdateCapability(t *testing.T) { now := time.Now().UTC() host := models.Host{ diff --git a/internal/unifiedresources/canonical_identity.go b/internal/unifiedresources/canonical_identity.go index ada14f278c..e5e96b8d2d 100644 --- a/internal/unifiedresources/canonical_identity.go +++ b/internal/unifiedresources/canonical_identity.go @@ -102,6 +102,15 @@ func canonicalAliases(resource Resource, primaryID, platformID, hostname string) strings.TrimSpace(resource.Identity.MachineID), strings.TrimSpace(resource.ID), ) + values = append(values, resource.Identity.Hostnames...) + if resource.Proxmox != nil { + values = append( + values, + strings.TrimSpace(resource.Proxmox.NodeIdentity), + strings.TrimSpace(resource.Proxmox.NodeName), + ) + values = append(values, resource.Proxmox.NodeAliases...) + } return uniqueTrimmed(values...) } diff --git a/internal/unifiedresources/canonical_identity_test.go b/internal/unifiedresources/canonical_identity_test.go index 90e002cf70..9824fadd59 100644 --- a/internal/unifiedresources/canonical_identity_test.go +++ b/internal/unifiedresources/canonical_identity_test.go @@ -75,6 +75,44 @@ func TestRefreshCanonicalIdentityPrefersTargetsAndCanonicalHostData(t *testing.T } } +func TestRefreshCanonicalIdentityKeepsProxmoxPresentationSeparateFromNativeAliases(t *testing.T) { + resource := Resource{ + ID: "production-pve1", + Type: ResourceTypeAgent, + Name: "Render East", + Identity: ResourceIdentity{ + Hostnames: []string{"pve1", "pve-old"}, + }, + Proxmox: &ProxmoxData{ + Instance: "production-api", + NodeIdentity: "production-pve1", + NodeName: "pve1", + NodeAliases: []string{"pve-old"}, + NodeDisplayName: "Render East", + }, + } + + RefreshCanonicalIdentity(&resource) + + if resource.Canonical == nil || + resource.Canonical.DisplayName != "Render East" || + resource.Canonical.PrimaryID == "Render East" { + t.Fatalf("display override became canonical identity: %+v", resource.Canonical) + } + for _, want := range []string{"production-pve1", "pve1", "pve-old"} { + found := false + for _, alias := range resource.Canonical.Aliases { + if alias == want { + found = true + break + } + } + if !found { + t.Fatalf("canonical aliases %v missing native identity %q", resource.Canonical.Aliases, want) + } + } +} + func TestUnavailableMemoryFacetsDoNotChangeCanonicalIdentity(t *testing.T) { total := int64(8 << 30) resource := Resource{ diff --git a/internal/unifiedresources/clone.go b/internal/unifiedresources/clone.go index 74607cb315..7ed599e71c 100644 --- a/internal/unifiedresources/clone.go +++ b/internal/unifiedresources/clone.go @@ -152,6 +152,7 @@ func cloneProxmoxData(in *ProxmoxData) *ProxmoxData { out.TemperatureDetails = cloneTemperature(in.TemperatureDetails) out.CPUInfo = cloneCPUInfo(in.CPUInfo) out.LoadAverage = cloneFloat64Slice(in.LoadAverage) + out.NodeAliases = cloneStringSlice(in.NodeAliases) out.NetworkInterfaces = cloneNetworkInterfaces(in.NetworkInterfaces) out.DockerCheckedAt = cloneTimePtr(in.DockerCheckedAt) out.TemperatureMonitoringEnabled = cloneBoolPtr(in.TemperatureMonitoringEnabled) diff --git a/internal/unifiedresources/clone_test.go b/internal/unifiedresources/clone_test.go index ded86776c1..bd0ad3c127 100644 --- a/internal/unifiedresources/clone_test.go +++ b/internal/unifiedresources/clone_test.go @@ -343,6 +343,21 @@ func TestCloneProxmoxData_Nil(t *testing.T) { } } +func TestCloneProxmoxData_NodeAliasesIsolation(t *testing.T) { + original := &ProxmoxData{ + NodeIdentity: "production-pve1", + NodeName: "pve1", + NodeAliases: []string{"pve-old"}, + } + + cloned := cloneProxmoxData(original) + cloned.NodeAliases[0] = "changed" + + if original.NodeAliases[0] != "pve-old" { + t.Fatalf("Proxmox resource clone shared node aliases: %+v", original) + } +} + func TestCloneProxmoxData_TemperatureIsolation(t *testing.T) { temp := 45.5 original := &ProxmoxData{ diff --git a/internal/unifiedresources/registry.go b/internal/unifiedresources/registry.go index 7ad0bcda54..5f75383c6d 100644 --- a/internal/unifiedresources/registry.go +++ b/internal/unifiedresources/registry.go @@ -3357,9 +3357,18 @@ func mergeProxmoxData(existing *ProxmoxData, incoming *ProxmoxData) *ProxmoxData if incoming.SourceID != "" { merged.SourceID = incoming.SourceID } + if incoming.NodeIdentity != "" { + merged.NodeIdentity = incoming.NodeIdentity + } if incoming.NodeName != "" { merged.NodeName = incoming.NodeName } + if len(incoming.NodeAliases) > 0 { + merged.NodeAliases = uniqueStrings(append(merged.NodeAliases, incoming.NodeAliases...)) + } + if incoming.NodeDisplayName != "" { + merged.NodeDisplayName = incoming.NodeDisplayName + } if incoming.ClusterName != "" { merged.ClusterName = incoming.ClusterName } @@ -4248,6 +4257,10 @@ func (rr *ResourceRegistry) buildChildCounts() { // ChildCount and ParentName are derived fields. Clear prior values before // recomputing to prevent stale state after re-parenting or parent removal. for _, r := range rr.resources { + if CanonicalResourceType(r.Type) == ResourceTypeAgent && r.Proxmox != nil && + strings.TrimSpace(r.Proxmox.NodeDisplayName) != "" { + r.Name = r.Proxmox.NodeDisplayName + } r.ParentID = rr.resolveCanonicalParentID(r) rr.refreshLinkedAgentIDFromParentLocked(r) r.ChildCount = 0 @@ -4270,6 +4283,11 @@ func (rr *ResourceRegistry) buildChildCounts() { if r.ParentID != nil { if parent, ok := rr.resources[*r.ParentID]; ok { r.ParentName = parent.Name + if r.Proxmox != nil && parent.Proxmox != nil { + r.Proxmox.NodeIdentity = parent.Proxmox.NodeIdentity + r.Proxmox.NodeDisplayName = parent.Proxmox.NodeDisplayName + r.Proxmox.NodeAliases = append([]string(nil), parent.Proxmox.NodeAliases...) + } } } } diff --git a/internal/unifiedresources/registry_test.go b/internal/unifiedresources/registry_test.go index 83dfc7618c..219602e930 100644 --- a/internal/unifiedresources/registry_test.go +++ b/internal/unifiedresources/registry_test.go @@ -1491,7 +1491,9 @@ func TestResourceRegistry_IngestSnapshotUnifiesLinkedProxmoxNodeViewsByHostIdent Nodes: []models.Node{ { ID: "homelab-minipc", + NodeIdentity: "homelab-minipc", Name: "minipc", + DisplayName: "Render East", Instance: "homelab-entry", ClusterName: "homelab", IsClusterMember: true, @@ -1539,6 +1541,10 @@ func TestResourceRegistry_IngestSnapshotUnifiesLinkedProxmoxNodeViewsByHostIdent if resource.Proxmox == nil || resource.Proxmox.NodeName != "minipc" { t.Fatalf("expected proxmox node metadata for minipc, got %+v", resource.Proxmox) } + if resource.Name != "Render East" || resource.Proxmox.NodeIdentity != "homelab-minipc" || + resource.Proxmox.NodeDisplayName != "Render East" { + t.Fatalf("linked agent did not retain Proxmox presentation identity: %+v", resource) + } } func TestResourceRegistry_IngestSnapshotUnifiesAsymmetricLinkedProxmoxNodeViewsByHostIdentity(t *testing.T) { @@ -1998,6 +2004,7 @@ func TestResourceRegistry_IngestSnapshotDerivesProxmoxWorkloadParentWhenNodeSour Nodes: []models.Node{ { ID: "mock-cluster-pve1", + NodeIdentity: "mock-cluster-pve1", Name: "pve1", DisplayName: "West Production A", Instance: "Core Fabric", @@ -2047,6 +2054,11 @@ func TestResourceRegistry_IngestSnapshotDerivesProxmoxWorkloadParentWhenNodeSour if vms[0].ParentName != "West Production A" { t.Fatalf("expected vm parent name West Production A, got %q", vms[0].ParentName) } + if vms[0].Proxmox == nil || vms[0].Proxmox.NodeIdentity != "mock-cluster-pve1" || + vms[0].Proxmox.NodeDisplayName != "West Production A" || + vms[0].Proxmox.NodeName != "pve1" { + t.Fatalf("expected VM presentation metadata without changing native name, got %+v", vms[0].Proxmox) + } containers := rr.ListByType(ResourceTypeSystemContainer) if len(containers) != 1 { diff --git a/internal/unifiedresources/types.go b/internal/unifiedresources/types.go index e795fdb0cf..b84c5ff2b6 100644 --- a/internal/unifiedresources/types.go +++ b/internal/unifiedresources/types.go @@ -333,7 +333,10 @@ type MetricValue struct { type ProxmoxData struct { SourceID string `json:"sourceId,omitempty"` // raw model ID from source snapshot RuntimeStatus string `json:"runtimeStatus,omitempty"` // source-authored VM/LXC power state, independent of collection freshness + NodeIdentity string `json:"nodeIdentity,omitempty"` NodeName string `json:"nodeName,omitempty"` + NodeAliases []string `json:"nodeAliases,omitempty"` + NodeDisplayName string `json:"nodeDisplayName,omitempty"` Pool string `json:"pool,omitempty"` ClusterName string `json:"clusterName,omitempty"` IsClusterMember bool `json:"isClusterMember,omitempty"` diff --git a/tests/integration/tests/91-proxmox-node-display-names.spec.ts b/tests/integration/tests/91-proxmox-node-display-names.spec.ts new file mode 100644 index 0000000000..bd359eb522 --- /dev/null +++ b/tests/integration/tests/91-proxmox-node-display-names.spec.ts @@ -0,0 +1,223 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { expect, test as base, type Page } from '@playwright/test'; + +import { createAuthenticatedStorageState } from './helpers'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); + +type WorkerFixtures = { + authStorageStatePath: string; +}; + +const connectionsFixture = { + connections: [ + { + id: 'pve:Production Cluster', + type: 'pve', + name: 'Production Cluster', + address: 'https://pve1.example.test:8006', + hostAliases: ['pve1.example.test'], + state: 'active', + stateReason: '', + enabled: true, + surfaces: ['nodes', 'vms', 'containers', 'storage', 'backups'], + scope: { nodes: true, vms: true, containers: true, storage: true, backups: true }, + lastSeen: '2026-07-24T14:00:00Z', + lastError: null, + source: 'manual', + capabilities: { supportsPause: true, supportsScope: true, supportsTest: true }, + }, + ], + systems: [ + { + id: 'pve:Production Cluster', + type: 'pve', + clusterName: 'production', + components: [ + { connectionId: 'pve:Production Cluster', type: 'pve', role: 'primary' }, + ], + members: [ + { + id: 'production-pve1', + nodeIdentity: 'production-pve1', + name: 'Render East', + nativeName: 'pve1', + endpoint: 'https://pve1.example.test:8006', + hostAliases: ['pve1', 'pve-old'], + state: 'active', + primary: true, + }, + { + id: 'production-pve2', + nodeIdentity: 'production-pve2', + name: 'pve2', + nativeName: 'pve2', + endpoint: 'https://pve2.example.test:8006', + hostAliases: ['pve2'], + state: 'active', + }, + ], + }, + ], +}; + +const nodesFixture = [ + { + id: 'pve-0', + type: 'pve', + name: 'Production Cluster', + host: 'https://pve1.example.test:8006', + user: 'root@pam', + hasPassword: false, + tokenName: 'pulse', + hasToken: true, + verifySSL: false, + monitorVMs: true, + monitorContainers: true, + monitorStorage: true, + monitorBackups: true, + excludeDatastores: [], + enabled: true, + status: 'connected', + isCluster: true, + clusterName: 'production', + clusterEndpoints: [ + { + nodeId: 'node/pve1', + nodeIdentity: 'production-pve1', + nativeNodeId: 1, + nodeName: 'pve1', + nativeName: 'pve1', + nativeAliases: ['pve-old'], + displayName: 'Render East', + host: 'https://pve1.example.test:8006', + ip: '10.0.0.1', + online: true, + lastSeen: '2026-07-24T14:00:00Z', + pulseReachable: true, + }, + { + nodeId: 'node/pve2', + nodeIdentity: 'production-pve2', + nativeNodeId: 2, + nodeName: 'pve2', + nativeName: 'pve2', + host: 'https://pve2.example.test:8006', + ip: '10.0.0.2', + online: true, + lastSeen: '2026-07-24T14:00:00Z', + pulseReachable: true, + }, + ], + }, +]; + +async function routeClusterNodeDisplayNames( + page: Page, + onUpdate: (payload: Record) => void, +) { + await page.routeWebSocket('**/ws*', () => {}); + await page.route('**/api/connections*', async (route) => { + if (new URL(route.request().url()).pathname !== '/api/connections') { + await route.continue(); + return; + } + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify(connectionsFixture), + }); + }); + await page.route('**/api/config/nodes**', async (route) => { + const request = route.request(); + const url = new URL(request.url()); + if (request.method() === 'PUT' && url.pathname === '/api/config/nodes/pve-0') { + onUpdate(request.postDataJSON() as Record); + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ status: 'success' }), + }); + return; + } + if (request.method() === 'GET' && url.pathname === '/api/config/nodes') { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify(nodesFixture), + }); + return; + } + await route.continue(); + }); +} + +const test = base.extend<{}, WorkerFixtures>({ + storageState: async ({ authStorageStatePath }, use) => { + await use(authStorageStatePath); + }, + authStorageStatePath: [ + async ({ browser }, use, workerInfo) => { + const storageStatePath = path.resolve( + __dirname, + '..', + '..', + 'tmp', + 'playwright-auth', + `proxmox-node-display-names-${workerInfo.project.name}.json`, + ); + fs.mkdirSync(path.dirname(storageStatePath), { recursive: true }); + await createAuthenticatedStorageState(browser, storageStatePath); + try { + await use(storageStatePath); + } finally { + fs.rmSync(storageStatePath, { force: true }); + } + }, + { scope: 'worker' }, + ], +}); + +test.describe('Proxmox cluster node display names', () => { + test.setTimeout(180_000); + + test('keeps native diagnostics while saving by immutable identity', async ({ page }) => { + let updatePayload: Record | undefined; + await routeClusterNodeDisplayNames(page, (payload) => { + updatePayload = payload; + }); + + await page.goto('/settings/infrastructure', { waitUntil: 'domcontentloaded' }); + const memberToggle = page.getByRole('button', { name: 'Show 2 nodes for production' }); + await expect(memberToggle).toBeVisible(); + await memberToggle.click(); + + await expect(page.getByText('Render East', { exact: true })).toBeVisible(); + await expect(page.getByText('Production Cluster (pve1)', { exact: true })).toHaveCount(0); + + await page.getByRole('button', { name: 'Manage', exact: true }).click(); + await expect( + page.getByText( + 'Give each node an optional display name for Pulse. This never changes its Proxmox name, identity, credentials, or connection address.', + { exact: true }, + ), + ).toBeVisible(); + await expect(page.getByText('Proxmox node: pve1', { exact: true })).toBeVisible(); + await expect(page.getByLabel('Display name for pve1')).toHaveValue('Render East'); + + // Equal display labels are intentionally valid presentation. The write + // target remains the second member's immutable identity. + await page.getByLabel('Display name for pve2').fill('Render East'); + await page.getByRole('button', { name: 'Save changes', exact: true }).click(); + + await expect.poll(() => updatePayload).toBeDefined(); + expect(updatePayload?.clusterNodeDisplayNameOverrides).toEqual([ + { nodeIdentity: 'production-pve2', displayName: 'Render East' }, + ]); + expect(updatePayload?.host).toBe('https://pve1.example.test:8006'); + expect(updatePayload).not.toHaveProperty('clusterEndpoints'); + expect(updatePayload).not.toHaveProperty('tokenValue'); + }); +});