From 384dc53608eebca505b77119fdfc1af891edd6be Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Thu, 3 Sep 2026 22:51:00 +0100 Subject: [PATCH] Keep repeated Proxmox cluster labels provider-scoped A cluster name is an operator-selected display label, not global machine identity. Two independently configured estates using the same label could share one linked agent and then collapse into one presentation row after delayed cluster detection. Require node identity, exact endpoint, or host corroboration across provider instances instead. Refs #1753 Change-source: pulse-maintainer --- .../internal/subsystems/unified-resources.md | 5 +++ .../unifiedresources/presentation_coalesce.go | 9 ++--- .../presentation_coalesce_test.go | 15 ++++++++ .../unifiedresources/proxmox_node_links.go | 9 ++--- .../proxmox_node_links_test.go | 37 ++++++++++++++++++- internal/unifiedresources/registry_test.go | 5 +++ 6 files changed, 68 insertions(+), 12 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/unified-resources.md b/docs/release-control/v6/internal/subsystems/unified-resources.md index b71946edf..ec63e8a36 100644 --- a/docs/release-control/v6/internal/subsystems/unified-resources.md +++ b/docs/release-control/v6/internal/subsystems/unified-resources.md @@ -86,6 +86,11 @@ 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. +That provider scope also governs node-to-agent link inference and host +presentation coalescing. A repeated cluster label is operator-authored +grouping metadata, not same-machine evidence; cross-instance equality requires +the same node identity, the exact configured endpoint, or independently +corroborated host evidence. 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 diff --git a/internal/unifiedresources/presentation_coalesce.go b/internal/unifiedresources/presentation_coalesce.go index ec1741410..fd76f1abb 100644 --- a/internal/unifiedresources/presentation_coalesce.go +++ b/internal/unifiedresources/presentation_coalesce.go @@ -231,8 +231,10 @@ func presentationHostIdentitiesDistinct(left, right Resource) bool { // two hand-added sites are commonly just "pve" (#1753), so a shared short // hostname must not fold one site's node row into the other's. The proof // mirrors the state-layer rule: the same connection instance, the same node -// identity, the same non-empty cluster, or the same endpoint host still -// merge; anything less keeps the rows apart. +// identity, or the same endpoint host still merge; anything less keeps the +// rows apart. Cluster names are operator-chosen display labels and are not +// globally unique, so an equal label across provider instances is not +// same-machine proof. func presentationProxmoxNodeScopesDistinct(left, right *ProxmoxData) bool { if left == nil || right == nil { return false @@ -246,9 +248,6 @@ func presentationProxmoxNodeScopesDistinct(left, right *ProxmoxData) bool { if presentationIdentityValuesEqual(left.NodeIdentity, right.NodeIdentity) { return false } - if presentationIdentityValuesEqual(left.ClusterName, right.ClusterName) { - return false - } if presentationIdentityValuesEqual(extractHostname(left.HostURL), extractHostname(right.HostURL)) { return false } diff --git a/internal/unifiedresources/presentation_coalesce_test.go b/internal/unifiedresources/presentation_coalesce_test.go index 9afdb9bdd..15c30f008 100644 --- a/internal/unifiedresources/presentation_coalesce_test.go +++ b/internal/unifiedresources/presentation_coalesce_test.go @@ -619,6 +619,21 @@ func TestCoalescePresentationHostResourcesKeepsStandaloneProviderScopesApart(t * } }) + t.Run("equal cluster display labels do not collapse provider scopes", func(t *testing.T) { + staging := nodeRow("staging-pve", "hema-staging", "https://pve.hemastaging.hot:8006", "Tripper Staging", "host-staging") + staging.Proxmox.ClusterName = "homelab" + staging.Sources = append(staging.Sources, SourceAgent) + staging.Agent = &AgentData{AgentID: "host-staging", Hostname: "pve", MachineID: "machine-staging"} + staging.Identity.MachineID = "machine-staging" + production := nodeRow("production-pve", "hema-production", "https://pve.hemaproduction.hot:8006", "VV Staging", "") + production.Proxmox.ClusterName = "homelab" + + got := CoalescePresentationHostResources([]Resource{staging, production}) + if len(got) != 2 { + t.Fatalf("equal cluster display labels collapsed distinct provider scopes: %#v", got) + } + }) + t.Run("single site still merges its node and agent rows", func(t *testing.T) { resources := []Resource{ nodeRow("home-pve", "home", "https://pve.home.lan:8006", "home", ""), diff --git a/internal/unifiedresources/proxmox_node_links.go b/internal/unifiedresources/proxmox_node_links.go index 4ba22606c..081c4a318 100644 --- a/internal/unifiedresources/proxmox_node_links.go +++ b/internal/unifiedresources/proxmox_node_links.go @@ -235,8 +235,10 @@ func inferLinkedHostsForProxmoxNodes(nodes []models.Node, hostByID map[string]*m // names are commonly short and repeat across independent standalone sites, so // a shared name is deliberately absent here. Distinct configured instances // may share a host only when provider-owned evidence says they are the same -// (the same node identity, named cluster, or exact configured endpoint), or +// (the same node identity or exact configured endpoint), or // when both views independently match the trusted host's full endpoint/IP. +// A cluster name is only an operator-chosen display label and may be reused +// by independent estates, so equality is not provider identity evidence. func proxmoxProviderNodesProveSameMachine(left, right models.Node, host *models.Host) bool { if leftID, rightID := strings.TrimSpace(left.ID), strings.TrimSpace(right.ID); leftID != "" && leftID == rightID { return true @@ -249,11 +251,6 @@ func proxmoxProviderNodesProveSameMachine(left, right models.Node, host *models. if leftInstance != "" && leftInstance == rightInstance { return true } - leftCluster := strings.TrimSpace(strings.ToLower(left.ClusterName)) - rightCluster := strings.TrimSpace(strings.ToLower(right.ClusterName)) - if leftCluster != "" && leftCluster == rightCluster { - return true - } leftEndpoint := strings.TrimSpace(strings.ToLower(extractHostname(left.Host))) rightEndpoint := strings.TrimSpace(strings.ToLower(extractHostname(right.Host))) if leftEndpoint != "" && leftEndpoint == rightEndpoint { diff --git a/internal/unifiedresources/proxmox_node_links_test.go b/internal/unifiedresources/proxmox_node_links_test.go index a296db932..b20f0ca77 100644 --- a/internal/unifiedresources/proxmox_node_links_test.go +++ b/internal/unifiedresources/proxmox_node_links_test.go @@ -83,6 +83,32 @@ func TestInferLinkedHostsForProxmoxNodesKeepsStandaloneProviderScopesApart(t *te } } +func TestInferLinkedHostsForProxmoxNodesDoesNotTrustRepeatedClusterLabel(t *testing.T) { + staging := models.Node{ + ID: "staging-pve", NodeIdentity: "staging-pve", Name: "pve", + ClusterName: "homelab", Instance: "staging", Host: "https://pve.staging.example:8006", + LinkedAgentID: "host-staging", + } + production := models.Node{ + ID: "production-pve", NodeIdentity: "production-pve", Name: "pve", + ClusterName: "homelab", Instance: "production", Host: "https://pve.production.example:8006", + } + host := &models.Host{ + ID: "host-staging", Hostname: "pve", LinkedNodeID: "staging-pve", + } + + got := inferLinkedHostsForProxmoxNodes( + []models.Node{staging, production}, + map[string]*models.Host{host.ID: host}, + ) + if got[staging.ID] == nil || got[staging.ID].ID != host.ID { + t.Fatalf("trusted staging link was lost: %+v", got) + } + if got[production.ID] != nil { + t.Fatalf("repeated display label leaked staging host into production provider: %+v", got) + } +} + func TestProxmoxProviderNodesProveSameMachineAcrossDuplicateConnections(t *testing.T) { base := models.Node{ ID: "site-a-pve", NodeIdentity: "node-pve", Name: "pve", @@ -91,7 +117,6 @@ func TestProxmoxProviderNodesProveSameMachineAcrossDuplicateConnections(t *testi for name, candidate := range map[string]models.Node{ "node identity": {ID: "site-b-pve", NodeIdentity: "node-pve", Name: "pve", Instance: "site-b", Host: "https://other.example:8006"}, - "cluster": {ID: "site-b-pve", Name: "pve", Instance: "site-b", ClusterName: "prod", Host: "https://other.example:8006"}, "endpoint": {ID: "site-b-pve", Name: "pve", Instance: "site-b", Host: "https://pve.example:8006"}, } { t.Run(name, func(t *testing.T) { @@ -112,4 +137,14 @@ func TestProxmoxProviderNodesProveSameMachineAcrossDuplicateConnections(t *testi if proxmoxProviderNodesProveSameMachine(base, distinct, nil) { t.Fatalf("shared native hostname proved distinct standalone providers equal: left=%+v right=%+v", base, distinct) } + + leftCluster := base + leftCluster.NodeIdentity = "" + leftCluster.ClusterName = "homelab" + rightCluster := distinct + rightCluster.NodeIdentity = "" + rightCluster.ClusterName = "homelab" + if proxmoxProviderNodesProveSameMachine(leftCluster, rightCluster, nil) { + t.Fatalf("equal cluster display labels proved distinct provider scopes equal: left=%+v right=%+v", leftCluster, rightCluster) + } } diff --git a/internal/unifiedresources/registry_test.go b/internal/unifiedresources/registry_test.go index fbf1b9c10..9b0083a80 100644 --- a/internal/unifiedresources/registry_test.go +++ b/internal/unifiedresources/registry_test.go @@ -298,6 +298,11 @@ func TestMonitorAdapterKeepsSameNamedProxmoxProvidersDistinct(t *testing.T) { if got := MonitoredSystemCount(adapter); got != 2 { t.Fatalf("same-name provider monitored-system count = %d, want 2", got) } + + presented := CoalescePresentationHostResources(resources) + if len(presented) != 2 { + t.Fatalf("repeated cluster label collapsed provider-scoped presentation rows: %+v", presented) + } } func TestResourceRegistryAvailabilityLinkedResourceResolvesSourceReference(t *testing.T) {