Keep Proxmox agent links within provider scope

Change-source: pulse-maintainer
2026-09-10 02:25:56 +00:00 · 2026-09-01 13:04:46 +01:00
parent dc4639460a
commit 12b90e2e5e
3 changed files with 291 additions and 1 deletions
@@ -0,0 +1,123 @@
+package api
+
+import (
+	"bytes"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+
+	"github.com/rcourtman/pulse-go-rewrite/internal/config"
+	"github.com/rcourtman/pulse-go-rewrite/internal/models"
+	"github.com/rcourtman/pulse-go-rewrite/internal/monitoring"
+	"github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources"
+	agentshost "github.com/rcourtman/pulse-go-rewrite/pkg/agents/host"
+)
+
+// Issue #1753 crossed three runtime boundaries: report authentication, host to
+// provider-node linking, and canonical presentation. Keep those boundaries in
+// one regression test so two valid agents from independent sites cannot be
+// mistaken for an auth failure or collapsed merely because both PVE machines
+// report the same native short hostname.
+func TestIssue1753SameNameProxmoxAgentsAuthenticateAndStayDistinctEndToEnd(t *testing.T) {
+	const (
+		stagingToken    = "issue-1753-staging-agent-token.12345678"
+		productionToken = "issue-1753-production-agent-token.12345678"
+	)
+	stagingRecord := newTokenRecord(t, stagingToken, []string{config.ScopeAgentReport}, nil)
+	productionRecord := newTokenRecord(t, productionToken, []string{config.ScopeAgentReport}, nil)
+
+	dataPath := t.TempDir()
+	cfg := &config.Config{
+		DataPath:   dataPath,
+		ConfigPath: dataPath,
+		APITokens:  []config.APITokenRecord{stagingRecord, productionRecord},
+	}
+	monitor, err := monitoring.New(cfg)
+	if err != nil {
+		t.Fatalf("monitoring.New: %v", err)
+	}
+	t.Cleanup(monitor.Stop)
+
+	now := time.Date(2026, 9, 1, 10, 0, 0, 0, time.UTC)
+	monitorState(t, monitor).UpdateNodes([]models.Node{
+		{
+			ID: "staging-pve", NodeIdentity: "staging-pve", Name: "pve",
+			DisplayName: "Staging", Instance: "staging", Host: "https://pve.staging.example:8006",
+			Status: "online", LastSeen: now,
+			NetworkInterfaces: []models.HostNetworkInterface{{Name: "vmbr0", Addresses: []string{"192.0.2.11/24"}}},
+		},
+		{
+			ID: "production-pve", NodeIdentity: "production-pve", Name: "pve",
+			DisplayName: "Production", Instance: "production", Host: "https://pve.production.example:8006",
+			Status: "online", LastSeen: now,
+			NetworkInterfaces: []models.HostNetworkInterface{{Name: "vmbr0", Addresses: []string{"198.51.100.21/24"}}},
+		},
+	})
+
+	router := NewRouter(cfg, monitor, nil, nil, func() error { return nil }, "6.4.2")
+	t.Cleanup(router.shutdownBackgroundWorkers)
+
+	postReport := func(rawToken, agentID, machineID, address string, at time.Time) {
+		t.Helper()
+		report := agentshost.Report{
+			Agent: agentshost.AgentInfo{ID: agentID, Version: "6.4.2", Type: "unified"},
+			Host: agentshost.HostInfo{
+				ID: machineID, MachineID: machineID, Hostname: "pve",
+				Platform: "linux", OSName: "Proxmox VE",
+			},
+			Network:   []agentshost.NetworkInterface{{Name: "vmbr0", Addresses: []string{address}}},
+			Timestamp: at,
+		}
+		body, err := json.Marshal(report)
+		if err != nil {
+			t.Fatalf("marshal report: %v", err)
+		}
+		req := httptest.NewRequest(http.MethodPost, "/api/agents/agent/report", bytes.NewReader(body))
+		req.Header.Set("Authorization", "Bearer "+rawToken)
+		rec := httptest.NewRecorder()
+		router.Handler().ServeHTTP(rec, req)
+		if rec.Code != http.StatusOK {
+			t.Fatalf("agent %q report status = %d, want 200: %s", agentID, rec.Code, rec.Body.String())
+		}
+	}
+
+	postReport(stagingToken, "agent-staging", "machine-staging", "192.0.2.11/24", now.Add(time.Second))
+	postReport(productionToken, "agent-production", "machine-production", "198.51.100.21/24", now.Add(2*time.Second))
+
+	snapshot := monitor.GetLiveStateSnapshot()
+	if len(snapshot.Hosts) != 2 || len(snapshot.Nodes) != 2 {
+		t.Fatalf("live topology = %d hosts / %d nodes, want 2 / 2: %+v", len(snapshot.Hosts), len(snapshot.Nodes), snapshot)
+	}
+	linkedByInstance := make(map[string]string, len(snapshot.Nodes))
+	for _, node := range snapshot.Nodes {
+		linkedByInstance[node.Instance] = node.LinkedAgentID
+	}
+	if linkedByInstance["staging"] != "machine-staging" || linkedByInstance["production"] != "machine-production" {
+		t.Fatalf("provider links = %+v, want each site linked to its own authenticated agent", linkedByInstance)
+	}
+
+	resources, _ := monitor.UnifiedResourceSnapshot()
+	byInstance := make(map[string]unifiedresources.Resource)
+	for _, resource := range resources {
+		if resource.Proxmox != nil && resource.Proxmox.NodeName != "" {
+			byInstance[resource.Proxmox.Instance] = resource
+		}
+	}
+	if len(byInstance) != 2 {
+		t.Fatalf("presentation provider rows = %d, want 2: %+v", len(byInstance), resources)
+	}
+	for instance, want := range map[string]struct {
+		agentID string
+		name    string
+	}{
+		"staging":    {agentID: "machine-staging", name: "Staging"},
+		"production": {agentID: "machine-production", name: "Production"},
+	} {
+		resource := byInstance[instance]
+		if resource.Agent == nil || resource.Agent.AgentID != want.agentID || resource.Name != want.name {
+			t.Fatalf("%s presentation row = %+v, want agent %q and name %q", instance, resource, want.agentID, want.name)
+		}
+	}
+}
@@ -27,6 +27,8 @@ func inferLinkedHostsForProxmoxNodes(nodes []models.Node, hostByID map[string]*m
 	trustedHostIDs := make(map[string]struct{})
 	hostClusterByID := make(map[string]string)
 	hostClusterAmbiguous := make(map[string]struct{})
+	hostProviderNodeByID := make(map[string]models.Node)
+	hostProviderNodeAmbiguous := make(map[string]struct{})
 	recordHostCluster := func(hostID string, node models.Node) {
 		hostID = strings.TrimSpace(hostID)
 		cluster := strings.TrimSpace(strings.ToLower(node.ClusterName))
@@ -55,6 +57,38 @@ func inferLinkedHostsForProxmoxNodes(nodes []models.Node, hostByID map[string]*m
 		nodeCluster := strings.TrimSpace(strings.ToLower(node.ClusterName))
 		return nodeCluster != "" && nodeCluster != hostCluster
 	}
+	recordHostProviderNode := func(hostID string, node models.Node) {
+		hostID = strings.TrimSpace(hostID)
+		if hostID == "" {
+			return
+		}
+		if _, ambiguous := hostProviderNodeAmbiguous[hostID]; ambiguous {
+			return
+		}
+		if existing, ok := hostProviderNodeByID[hostID]; ok &&
+			!proxmoxProviderNodesProveSameMachine(existing, node, hostByID[hostID]) {
+			delete(hostProviderNodeByID, hostID)
+			hostProviderNodeAmbiguous[hostID] = struct{}{}
+			return
+		}
+		hostProviderNodeByID[hostID] = node
+	}
+	hostProviderConflicts := func(hostID string, node models.Node) bool {
+		hostID = strings.TrimSpace(hostID)
+		if _, ambiguous := hostProviderNodeAmbiguous[hostID]; ambiguous {
+			return true
+		}
+		existing, ok := hostProviderNodeByID[strings.TrimSpace(hostID)]
+		if !ok {
+			return false
+		}
+		existingInstance := strings.TrimSpace(strings.ToLower(existing.Instance))
+		candidateInstance := strings.TrimSpace(strings.ToLower(node.Instance))
+		if existingInstance == "" || candidateInstance == "" || existingInstance == candidateInstance {
+			return false
+		}
+		return !proxmoxProviderNodesProveSameMachine(existing, node, hostByID[hostID])
+	}
 	register := func(key, hostID string) {
 		key = strings.TrimSpace(key)
 		hostID = strings.TrimSpace(hostID)
@@ -99,6 +133,7 @@ func inferLinkedHostsForProxmoxNodes(nodes []models.Node, hostByID map[string]*m
 		}
 		trustedHostIDs[hostID] = struct{}{}
 		recordHostCluster(hostID, node)
+		recordHostProviderNode(hostID, node)
 		for _, key := range proxmoxNodeLinkKeys(node) {
 			register(key, hostID)
 		}
@@ -118,6 +153,7 @@ func inferLinkedHostsForProxmoxNodes(nodes []models.Node, hostByID map[string]*m
 		}
 		trustedHostIDs[hostID] = struct{}{}
 		recordHostCluster(hostID, *node)
+		recordHostProviderNode(hostID, *node)
 		registerNode(nodeID, hostID)
 		for _, key := range proxmoxNodeLinkKeys(*node) {
 			register(key, hostID)
@@ -153,7 +189,7 @@ func inferLinkedHostsForProxmoxNodes(nodes []models.Node, hostByID map[string]*m
 				continue
 			}
 			hostID := strings.TrimSpace(keyToHostID[key])
-			if hostID == "" || hostClusterConflicts(hostID, node) {
+			if hostID == "" || hostClusterConflicts(hostID, node) || hostProviderConflicts(hostID, node) {
 				continue
 			}
 			if inferredHostID != "" && inferredHostID != hostID {
@@ -173,6 +209,9 @@ func inferLinkedHostsForProxmoxNodes(nodes []models.Node, hostByID map[string]*m
 				if hostClusterConflicts(hostID, node) {
 					continue
 				}
+				if hostProviderConflicts(hostID, node) {
+					continue
+				}
 				if inferredHostID != "" && inferredHostID != hostID {
 					inferredHostID = ""
 					break
@@ -191,6 +230,77 @@ func inferLinkedHostsForProxmoxNodes(nodes []models.Node, hostByID map[string]*m
 	return out
 }

+// proxmoxProviderNodesProveSameMachine is the provider-scope boundary for
+// lending one trusted host-agent identity to another node view. Native PVE
+// 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
+// when both views independently match the trusted host's full endpoint/IP.
+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
+	}
+	if leftIdentity, rightIdentity := strings.TrimSpace(left.NodeIdentity), strings.TrimSpace(right.NodeIdentity); leftIdentity != "" && leftIdentity == rightIdentity {
+		return true
+	}
+	leftInstance := strings.TrimSpace(strings.ToLower(left.Instance))
+	rightInstance := strings.TrimSpace(strings.ToLower(right.Instance))
+	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 {
+		return true
+	}
+	return host != nil &&
+		proxmoxNodeStronglyCorroboratesHost(left, *host) &&
+		proxmoxNodeStronglyCorroboratesHost(right, *host)
+}
+
+func proxmoxNodeStronglyCorroboratesHost(node models.Node, host models.Host) bool {
+	hostIPs := make(map[string]struct{})
+	if ip := NormalizeIP(host.ReportIP); ip != "" {
+		hostIPs[ip] = struct{}{}
+	}
+	for _, network := range host.NetworkInterfaces {
+		for _, address := range network.Addresses {
+			if ip := NormalizeIP(address); ip != "" {
+				hostIPs[ip] = struct{}{}
+			}
+		}
+	}
+
+	endpoint := strings.TrimSpace(strings.ToLower(extractHostname(node.Host)))
+	if endpointIP := NormalizeIP(endpoint); endpointIP != "" {
+		_, ok := hostIPs[endpointIP]
+		return ok
+	}
+	hostname := NormalizeFullHostname(host.Hostname)
+	if strings.Contains(endpoint, ".") && endpoint == hostname {
+		return true
+	}
+	if nodeName := NormalizeFullHostname(node.Name); strings.Contains(nodeName, ".") && nodeName == hostname {
+		return true
+	}
+	for _, network := range node.NetworkInterfaces {
+		for _, address := range network.Addresses {
+			if ip := NormalizeIP(address); ip != "" {
+				if _, ok := hostIPs[ip]; ok {
+					return true
+				}
+			}
+		}
+	}
+	return false
+}
+
 func proxmoxNodeLinkKeys(node models.Node) []string {
 	name := NormalizeHostname(node.Name)
 	if name == "" {
@@ -56,3 +56,60 @@ func TestInferLinkedHostsForProxmoxNodesKeepsClustersApart(t *testing.T) {
 		}
 	})
 }
+
+func TestInferLinkedHostsForProxmoxNodesKeepsStandaloneProviderScopesApart(t *testing.T) {
+	staging := models.Node{
+		ID: "staging-pve", NodeIdentity: "staging-pve", Name: "pve",
+		Instance: "staging", Host: "https://pve.staging.example:8006",
+		LinkedAgentID: "host-staging",
+	}
+	production := models.Node{
+		ID: "production-pve", NodeIdentity: "production-pve", Name: "pve",
+		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("staging host identity leaked into production provider: %+v", got)
+	}
+}
+
+func TestProxmoxProviderNodesProveSameMachineAcrossDuplicateConnections(t *testing.T) {
+	base := models.Node{
+		ID: "site-a-pve", NodeIdentity: "node-pve", Name: "pve",
+		Instance: "site-a", Host: "https://pve.example:8006",
+	}
+
+	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) {
+			left := base
+			if name == "cluster" {
+				left.ClusterName = "prod"
+			}
+			if !proxmoxProviderNodesProveSameMachine(left, candidate, nil) {
+				t.Fatalf("expected provider evidence to prove duplicate views: left=%+v right=%+v", left, candidate)
+			}
+		})
+	}
+
+	distinct := models.Node{
+		ID: "site-b-pve", NodeIdentity: "other-pve", Name: "pve",
+		Instance: "site-b", Host: "https://pve.other.example:8006",
+	}
+	if proxmoxProviderNodesProveSameMachine(base, distinct, nil) {
+		t.Fatalf("shared native hostname proved distinct standalone providers equal: left=%+v right=%+v", base, distinct)
+	}
+}