From 1166576b2172f60fd45d6161dad87d458115ef54 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 12 Nov 2025 22:46:56 +0000 Subject: [PATCH] Ensure agent ID collisions respect token boundaries (Related to #658) --- internal/monitoring/monitor.go | 19 +++-- internal/monitoring/monitor_docker_test.go | 88 ++++++++++++++++++++-- 2 files changed, 94 insertions(+), 13 deletions(-) diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 2397890..fc3f1ee 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -1344,7 +1344,12 @@ func findMatchingDockerHost(hosts []models.DockerHost, report agentsdocker.Repor if agentID != "" { for _, host := range hosts { - if strings.TrimSpace(host.AgentID) == agentID { + if strings.TrimSpace(host.AgentID) != agentID { + continue + } + + existingToken := strings.TrimSpace(host.TokenID) + if tokenID == "" || existingToken == tokenID { return host, true } } @@ -8245,12 +8250,12 @@ func (m *Monitor) GetConfigPersistence() *config.ConfigPersistence { func (m *Monitor) pollStorageBackupsWithNodes(ctx context.Context, instanceName string, client PVEClientInterface, nodes []proxmox.Node, nodeEffectiveStatus map[string]string) { var allBackups []models.StorageBackup - seenVolids := make(map[string]bool) // Track seen volume IDs to avoid duplicates - hadSuccessfulNode := false // Track if at least one node responded successfully - storagesWithBackup := 0 // Number of storages that should contain backups - contentSuccess := 0 // Number of successful storage content fetches - contentFailures := 0 // Number of failed storage content fetches - storageQueryErrors := 0 // Number of nodes where storage list could not be queried + seenVolids := make(map[string]bool) // Track seen volume IDs to avoid duplicates + hadSuccessfulNode := false // Track if at least one node responded successfully + storagesWithBackup := 0 // Number of storages that should contain backups + contentSuccess := 0 // Number of successful storage content fetches + contentFailures := 0 // Number of failed storage content fetches + storageQueryErrors := 0 // Number of nodes where storage list could not be queried storagePreserveNeeded := map[string]struct{}{} storageSuccess := map[string]struct{}{} diff --git a/internal/monitoring/monitor_docker_test.go b/internal/monitoring/monitor_docker_test.go index 575c585..7aa000e 100644 --- a/internal/monitoring/monitor_docker_test.go +++ b/internal/monitoring/monitor_docker_test.go @@ -14,12 +14,12 @@ func newTestMonitor(t *testing.T) *Monitor { t.Helper() m := &Monitor{ - state: models.NewState(), - alertManager: alerts.NewManager(), - removedDockerHosts: make(map[string]time.Time), - rateTracker: NewRateTracker(), - dockerTokenBindings: make(map[string]string), - dockerMetadataStore: config.NewDockerMetadataStore(t.TempDir()), + state: models.NewState(), + alertManager: alerts.NewManager(), + removedDockerHosts: make(map[string]time.Time), + rateTracker: NewRateTracker(), + dockerTokenBindings: make(map[string]string), + dockerMetadataStore: config.NewDockerMetadataStore(t.TempDir()), } t.Cleanup(func() { m.alertManager.Stop() }) return m @@ -114,6 +114,82 @@ func TestApplyDockerReportGeneratesUniqueIDsForCollidingHosts(t *testing.T) { } } +func TestApplyDockerReportUsesTokenToDisambiguateAgentIDCollisions(t *testing.T) { + monitor := newTestMonitor(t) + + baseReport := agentsdocker.Report{ + Agent: agentsdocker.AgentInfo{ + ID: "duplicate-agent", + Version: "1.0.0", + IntervalSeconds: 30, + }, + Host: agentsdocker.HostInfo{ + Hostname: "docker-one", + Name: "Docker One", + MachineID: "machine-A", + DockerVersion: "26.0.0", + TotalCPU: 4, + TotalMemoryBytes: 16 << 30, + UptimeSeconds: 120, + }, + Containers: []agentsdocker.Container{ + {ID: "container-a", Name: "api"}, + }, + Timestamp: time.Now().UTC(), + } + + tokenOne := &config.APITokenRecord{ID: "token-one", Name: "Token One"} + hostOne, err := monitor.ApplyDockerReport(baseReport, tokenOne) + if err != nil { + t.Fatalf("ApplyDockerReport hostOne: %v", err) + } + if hostOne.ID == "" { + t.Fatal("expected hostOne to receive an identifier") + } + + secondReport := baseReport + secondReport.Host.Hostname = "docker-two" + secondReport.Host.Name = "Docker Two" + secondReport.Host.MachineID = "machine-B" + secondReport.Containers = []agentsdocker.Container{ + {ID: "container-b", Name: "db"}, + } + secondReport.Timestamp = baseReport.Timestamp.Add(30 * time.Second) + + tokenTwo := &config.APITokenRecord{ID: "token-two", Name: "Token Two"} + hostTwo, err := monitor.ApplyDockerReport(secondReport, tokenTwo) + if err != nil { + t.Fatalf("ApplyDockerReport hostTwo: %v", err) + } + + if hostTwo.ID == "" { + t.Fatal("expected hostTwo to receive an identifier") + } + if hostOne.ID == hostTwo.ID { + t.Fatalf("expected different identifiers for hosts sharing an agent ID, got %q", hostOne.ID) + } + + hosts := monitor.state.GetDockerHosts() + if len(hosts) != 2 { + t.Fatalf("expected 2 hosts after two reports, got %d", len(hosts)) + } + + updatedReport := baseReport + updatedReport.Timestamp = baseReport.Timestamp.Add(60 * time.Second) + updatedReport.Containers = append(updatedReport.Containers, agentsdocker.Container{ + ID: "container-c", + Name: "cache", + }) + + updatedHostOne, err := monitor.ApplyDockerReport(updatedReport, tokenOne) + if err != nil { + t.Fatalf("ApplyDockerReport hostOne update: %v", err) + } + if updatedHostOne.ID != hostOne.ID { + t.Fatalf("expected hostOne to retain identifier %q, got %q", hostOne.ID, updatedHostOne.ID) + } +} + func TestApplyDockerReportIncludesContainerDiskDetails(t *testing.T) { timestamp := time.Now().UTC() report := agentsdocker.Report{