diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 54b4d34e51..c409067f3e 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -23,6 +23,9 @@ Monitoring also owns the distinction between Proxmox VM power state and QEMU guest-agent reachability: fresh or never-healthy VMs with an enabled but unavailable guest agent stay `not-running`, while only VMs with recent healthy guest-agent evidence may become `expected-unreachable`. +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. ## Canonical Files @@ -1193,10 +1196,13 @@ before snapshots are returned to diagnostics consumers, not only when new snapshots are first recorded. The same canonical identity rule now applies when removed host agents are blocked from re-reporting. `ApplyHostReport` must resolve the final canonical -host identifier for the `(token, machine-id, hostname)` tuple before it checks -`removedHostAgents` or emits the reconnect-blocking error, so removing one -token-bound host cannot poison a different host that shared the same raw -machine identifier. +host identifier before it checks `removedHostAgents` or emits the +reconnect-blocking error, and removed-host records must carry machine and token +identity so the block is scoped to the retired host. Hostname equivalence may +only participate when it is qualified by the same token and compatible machine +identity; removing one stale duplicate must not poison a different live host +that shares the same hostname or raw machine identifier through a different +token. Docker host re-identification now shares the same hostname-equivalence rule: monitoring may treat `qnap` and `qnap.local` as the same host when the token or machine identity already points at one canonical runtime, but it must not diff --git a/internal/models/models.go b/internal/models/models.go index 7106c61980..7efc743998 100644 --- a/internal/models/models.go +++ b/internal/models/models.go @@ -824,6 +824,8 @@ type RemovedHostAgent struct { ID string `json:"id"` Hostname string `json:"hostname,omitempty"` DisplayName string `json:"displayName,omitempty"` + MachineID string `json:"machineId,omitempty"` + TokenID string `json:"tokenId,omitempty"` LinkedVMID string `json:"linkedVmId,omitempty"` LinkedContainerID string `json:"linkedContainerId,omitempty"` RemovedAt time.Time `json:"removedAt"` diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index 615f81e57f..fbfc0b0ee9 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -2048,7 +2048,9 @@ func TestHostAgentRemovalGuardUsesResolvedIdentifier(t *testing.T) { } source := string(data) requiredSnippets := []string{ - "removedAt, wasRemoved := m.lookupRemovedHostAgent(identifier, hostname)", + "removedAt, wasRemoved := m.lookupRemovedHostAgent(identifier, hostname, report.Host.MachineID, tokenID)", + "func removedHostAgentMatchesReport(entry models.RemovedHostAgent, identifier, hostname, machineID, tokenID string) bool {", + "entryTokenID == \"\" || tokenID == \"\" || entryTokenID != tokenID", `Str("hostID", identifier)`, `fmt.Errorf("host agent %q had monitoring stopped at %v and cannot report again.`, } @@ -2060,6 +2062,9 @@ func TestHostAgentRemovalGuardUsesResolvedIdentifier(t *testing.T) { if strings.Contains(source, "m.lookupRemovedHostAgent(baseIdentifier, hostname)") { t.Fatal("monitor_agents.go must not check removed host-agent state against pre-resolution baseIdentifier") } + if strings.Contains(source, "if hostAgentHostnamesMatch(entry.Hostname, hostname) {") { + t.Fatal("monitor_agents.go must not block host-agent re-enroll solely by removed hostname") + } } func TestAlertLifecycleCanonicalChangesRemainWritable(t *testing.T) { diff --git a/internal/monitoring/monitor_agents.go b/internal/monitoring/monitor_agents.go index 0c45478e0f..001c3f185b 100644 --- a/internal/monitoring/monitor_agents.go +++ b/internal/monitoring/monitor_agents.go @@ -219,6 +219,8 @@ func (m *Monitor) RemoveHostAgent(hostID string) (models.Host, error) { ID: hostID, Hostname: host.Hostname, DisplayName: host.DisplayName, + MachineID: host.MachineID, + TokenID: host.TokenID, LinkedVMID: host.LinkedVMID, LinkedContainerID: host.LinkedContainerID, RemovedAt: removedAt, @@ -282,8 +284,10 @@ func (m *Monitor) AllowHostAgentReenroll(hostID string) error { return nil } -func (m *Monitor) lookupRemovedHostAgent(identifier, hostname string) (time.Time, bool) { +func (m *Monitor) lookupRemovedHostAgent(identifier, hostname, machineID, tokenID string) (time.Time, bool) { identifier = strings.TrimSpace(identifier) + machineID = sanitizeDockerHostSuffix(machineID) + tokenID = strings.TrimSpace(tokenID) m.mu.RLock() removedAt, wasRemoved := m.removedHostAgents[identifier] @@ -293,10 +297,7 @@ func (m *Monitor) lookupRemovedHostAgent(identifier, hostname string) (time.Time } for _, entry := range m.state.GetRemovedHostAgents() { - if strings.TrimSpace(entry.ID) == identifier { - return entry.RemovedAt, true - } - if hostAgentHostnamesMatch(entry.Hostname, hostname) { + if removedHostAgentMatchesReport(entry, identifier, hostname, machineID, tokenID) { return entry.RemovedAt, true } } @@ -304,6 +305,32 @@ func (m *Monitor) lookupRemovedHostAgent(identifier, hostname string) (time.Time return time.Time{}, false } +func removedHostAgentMatchesReport(entry models.RemovedHostAgent, identifier, hostname, machineID, tokenID string) bool { + if strings.TrimSpace(entry.ID) == strings.TrimSpace(identifier) { + return true + } + + entryMachineID := sanitizeDockerHostSuffix(entry.MachineID) + entryTokenID := strings.TrimSpace(entry.TokenID) + if entryTokenID != "" || tokenID != "" { + if entryTokenID == "" || tokenID == "" || entryTokenID != tokenID { + return false + } + if entryMachineID != "" && machineID != "" && entryMachineID == machineID { + return true + } + if !hostAgentHostnamesMatch(entry.Hostname, hostname) { + return false + } + return entryMachineID == "" || machineID == "" || entryMachineID == machineID + } + + return entryMachineID != "" && + machineID != "" && + entryMachineID == machineID && + hostAgentHostnamesMatch(entry.Hostname, hostname) +} + // LinkHostAgent manually links a host agent to a specific PVE node. // This is used when auto-linking can't disambiguate (e.g., multiple nodes with hostname "pve"). // After linking, the host agent's temperature/sensor data will appear on the correct node. @@ -1726,7 +1753,11 @@ func (m *Monitor) ApplyHostReport(report agentshost.Report, tokenRecord *config. } } - removedAt, wasRemoved := m.lookupRemovedHostAgent(identifier, hostname) + tokenID := "" + if tokenRecord != nil { + tokenID = strings.TrimSpace(tokenRecord.ID) + } + removedAt, wasRemoved := m.lookupRemovedHostAgent(identifier, hostname, report.Host.MachineID, tokenID) if wasRemoved { log.Info(). Str("hostID", identifier). diff --git a/internal/monitoring/monitor_host_agents_test.go b/internal/monitoring/monitor_host_agents_test.go index e7f346ac6e..771f7e87ee 100644 --- a/internal/monitoring/monitor_host_agents_test.go +++ b/internal/monitoring/monitor_host_agents_test.go @@ -971,6 +971,121 @@ func TestApplyHostReportDisambiguatesCollidingIdentifiersAcrossTokens(t *testing } } +func TestRemoveHostAgent_DoesNotBlockDistinctLiveHostWithSameHostname(t *testing.T) { + t.Helper() + + monitor := &Monitor{ + state: models.NewState(), + alertManager: alerts.NewManager(), + hostTokenBindings: make(map[string]string), + config: &config.Config{}, + rateTracker: NewRateTracker(), + } + t.Cleanup(func() { monitor.alertManager.Stop() }) + + now := time.Now().UTC() + staleReport := agentshost.Report{ + Agent: agentshost.AgentInfo{ + ID: "old-agent", + Version: "1.0.0", + IntervalSeconds: 30, + }, + Host: agentshost.HostInfo{ + ID: "shared-machine", + MachineID: "shared-machine", + Hostname: "pve-node", + Platform: "linux", + }, + Timestamp: now, + } + liveReport := agentshost.Report{ + Agent: agentshost.AgentInfo{ + ID: "new-agent", + Version: "1.0.0", + IntervalSeconds: 30, + }, + Host: agentshost.HostInfo{ + ID: "shared-machine", + MachineID: "shared-machine", + Hostname: "pve-node", + Platform: "linux", + }, + Timestamp: now.Add(30 * time.Second), + } + + staleHost, err := monitor.ApplyHostReport(staleReport, &config.APITokenRecord{ID: "old-token"}) + if err != nil { + t.Fatalf("ApplyHostReport stale host: %v", err) + } + liveHost, err := monitor.ApplyHostReport(liveReport, &config.APITokenRecord{ID: "new-token"}) + if err != nil { + t.Fatalf("ApplyHostReport live host: %v", err) + } + if liveHost.ID == staleHost.ID { + t.Fatalf("expected distinct duplicate host IDs, got %q", liveHost.ID) + } + + if _, err := monitor.RemoveHostAgent(staleHost.ID); err != nil { + t.Fatalf("RemoveHostAgent stale host: %v", err) + } + + liveReport.Timestamp = now.Add(time.Minute) + liveHostAfterRemoval, err := monitor.ApplyHostReport(liveReport, &config.APITokenRecord{ID: "new-token"}) + if err != nil { + t.Fatalf("live host with same hostname should not be blocked by stale removal: %v", err) + } + if liveHostAfterRemoval.ID != liveHost.ID { + t.Fatalf("expected live host ID %q to remain stable, got %q", liveHost.ID, liveHostAfterRemoval.ID) + } +} + +func TestRemoveHostAgent_BlocksRemovedTokenMachineAfterIdentifierChurn(t *testing.T) { + t.Helper() + + monitor := &Monitor{ + state: models.NewState(), + alertManager: alerts.NewManager(), + hostTokenBindings: make(map[string]string), + config: &config.Config{}, + rateTracker: NewRateTracker(), + } + t.Cleanup(func() { monitor.alertManager.Stop() }) + + now := time.Now().UTC() + hostID := "agent-id-before-reinstall" + monitor.state.UpsertHost(models.Host{ + ID: hostID, + Hostname: "blocked-node", + MachineID: "machine-stable", + TokenID: "host-token", + Status: "offline", + LastSeen: now.Add(-time.Minute), + }) + + if _, err := monitor.RemoveHostAgent(hostID); err != nil { + t.Fatalf("RemoveHostAgent: %v", err) + } + + report := agentshost.Report{ + Agent: agentshost.AgentInfo{ + ID: "agent-id-after-reinstall", + Version: "1.0.0", + IntervalSeconds: 30, + }, + Host: agentshost.HostInfo{ + ID: "machine-stable", + MachineID: "machine-stable", + Hostname: "blocked-node", + Platform: "linux", + }, + Timestamp: now, + } + + if _, err := monitor.ApplyHostReport(report, &config.APITokenRecord{ID: "host-token"}); err == nil { + t.Fatal("expected stable token and machine identity to remain blocked after host ID changed") + } +} + func TestRemoveHostAgentUnbindsToken(t *testing.T) { t.Helper()