From 6405c470648d503619939bc925896be76cc2f119 Mon Sep 17 00:00:00 2001 From: Pulse Test Date: Sun, 30 Aug 2026 00:07:53 +0100 Subject: [PATCH] Clear Docker host removal block on fresh-token re-enroll Removing a Docker host blocks its reports, but the report path only ever consulted the in-memory block and offered no way for a legitimate re-enroll to clear it: reinstalling the agent with a newly generated token kept being rejected until a server restart happened to wipe the in-memory map, which read as the Docker tab staying missing until the Pulse server was rebooted. The restart also silently lifted deliberate blocks, because the persisted entry was never consulted on report. Docker reports now follow the host-agent rule from #1581: the block check consults both the in-memory map and the persisted store, and a token minted after the removal is explicit re-enroll intent that clears the block from both. A still-running old agent presenting its pre-removal token stays blocked, including across restarts. Refs #1728 --- .../monitoring/docker_reenroll_block_test.go | 94 +++++++++++++++++++ internal/monitoring/monitor_agents.go | 73 +++++++++++--- 2 files changed, 153 insertions(+), 14 deletions(-) create mode 100644 internal/monitoring/docker_reenroll_block_test.go diff --git a/internal/monitoring/docker_reenroll_block_test.go b/internal/monitoring/docker_reenroll_block_test.go new file mode 100644 index 000000000..e7522a0c2 --- /dev/null +++ b/internal/monitoring/docker_reenroll_block_test.go @@ -0,0 +1,94 @@ +package monitoring + +import ( + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/config" + agentsdocker "github.com/rcourtman/pulse-go-rewrite/pkg/agents/docker" +) + +func reenrollBlockTestReport(agentID string, at time.Time) agentsdocker.Report { + return agentsdocker.Report{ + Agent: agentsdocker.AgentInfo{ + ID: agentID, + Version: "6.4.1", + Type: "unified", + IntervalSeconds: 30, + }, + Host: agentsdocker.HostInfo{ + Hostname: "alpine-docker", + MachineID: "machine-alpine-docker", + TotalCPU: 4, + }, + Containers: []agentsdocker.Container{ + {ID: "container-" + agentID, Name: "portainer"}, + }, + Timestamp: at, + } +} + +// Removing a Docker host blocks its reports, but a token minted after the +// removal is explicit re-enroll intent and must clear the block — the same +// rule host agents follow (#1581). Before this rule the re-enrolled host was +// rejected until a server restart happened to wipe the in-memory block, which +// read as "Docker tab missing until reboot" (#1728). The block must also +// survive a restart via the persisted store, so the restart itself never +// becomes the thing that lifts it. +func TestDockerRemovalBlockClearsOnFreshTokenReenroll(t *testing.T) { + monitor := newTestMonitor(t) + base := time.Now().UTC().Add(-2 * time.Hour) + + oldToken := &config.APITokenRecord{ID: "token-old", CreatedAt: base} + host, err := monitor.ApplyDockerReport(reenrollBlockTestReport("agent-alpine", base.Add(time.Minute)), oldToken) + if err != nil { + t.Fatalf("initial report: %v", err) + } + + if _, err := monitor.RemoveDockerHost(host.ID); err != nil { + t.Fatalf("remove docker host: %v", err) + } + + // The still-running old agent keeps presenting its pre-removal token and + // stays blocked. + if _, err := monitor.ApplyDockerReport(reenrollBlockTestReport("agent-alpine", base.Add(2*time.Minute)), oldToken); err == nil { + t.Fatal("expected pre-removal token report to stay blocked") + } + + // Simulate a server restart: the in-memory map resets while the persisted + // entry remains. The block must hold. + monitor.mu.Lock() + monitor.removedDockerHosts = make(map[string]time.Time) + monitor.mu.Unlock() + if _, err := monitor.ApplyDockerReport(reenrollBlockTestReport("agent-alpine", base.Add(3*time.Minute)), oldToken); err == nil { + t.Fatal("expected persisted removal block to survive restart") + } + + // A token minted after removal clears the block and the report lands. + freshToken := &config.APITokenRecord{ID: "token-new", CreatedAt: time.Now().UTC()} + reenrolled, err := monitor.ApplyDockerReport(reenrollBlockTestReport("agent-alpine", base.Add(4*time.Minute)), freshToken) + if err != nil { + t.Fatalf("fresh-token re-enroll rejected: %v", err) + } + + hosts := monitor.state.GetDockerHosts() + if len(hosts) != 1 || hosts[0].ID != reenrolled.ID { + ids := make([]string, 0, len(hosts)) + for _, h := range hosts { + ids = append(ids, h.ID) + } + t.Fatalf("expected exactly the re-enrolled host in state, got %v", ids) + } + + monitor.mu.RLock() + _, stillBlockedInMemory := monitor.removedDockerHosts[host.ID] + monitor.mu.RUnlock() + if stillBlockedInMemory { + t.Fatal("expected in-memory removal block to be cleared by fresh-token re-enroll") + } + for _, entry := range monitor.state.GetRemovedDockerHosts() { + if entry.ID == host.ID { + t.Fatal("expected persisted removal block to be cleared by fresh-token re-enroll") + } + } +} diff --git a/internal/monitoring/monitor_agents.go b/internal/monitoring/monitor_agents.go index a77646603..c6e65a5e3 100644 --- a/internal/monitoring/monitor_agents.go +++ b/internal/monitoring/monitor_agents.go @@ -1087,6 +1087,39 @@ func (m *Monitor) SetDockerHostCustomDisplayName(hostID string, customName strin return host, nil } +// lookupRemovedDockerHost reports whether any of the presented Docker host +// identities is blocked by a deliberate removal, returning the blocked entry's +// ID and removal time. It consults the in-memory map first and the persisted +// store second: the map resets on restart while the persisted entry keeps +// blocking (#1581), so memory absence alone must not admit a blocked host. +func (m *Monitor) lookupRemovedDockerHost(ids []string) (string, time.Time, bool) { + m.mu.RLock() + for _, id := range ids { + id = strings.TrimSpace(id) + if id == "" { + continue + } + if ts, ok := m.removedDockerHosts[id]; ok { + m.mu.RUnlock() + return id, ts, true + } + } + m.mu.RUnlock() + + for _, entry := range m.state.GetRemovedDockerHosts() { + entryID := strings.TrimSpace(entry.ID) + if entryID == "" { + continue + } + for _, id := range ids { + if strings.TrimSpace(id) == entryID { + return entryID, entry.RemovedAt, true + } + } + } + return "", time.Time{}, false +} + // AllowDockerHostReenroll removes a host ID from the removal blocklist so it can report again. func (m *Monitor) AllowDockerHostReenroll(hostID string) error { hostID = strings.TrimSpace(hostID) @@ -1759,22 +1792,34 @@ func (m *Monitor) ApplyDockerReport(report agentsdocker.Report, tokenRecord *con return models.DockerHost{}, fmt.Errorf("docker report missing agent identifier") } - // Check if this host was deliberately removed - reject report to prevent resurrection - m.mu.RLock() - removedAt, wasRemoved := m.removedDockerHosts[identifier] - if !wasRemoved { - for _, legacyID := range legacyIDs { - if legacyID == "" || legacyID == identifier { - continue - } - if ts, ok := m.removedDockerHosts[legacyID]; ok { - removedAt = ts - wasRemoved = true - break - } + // Check if this host was deliberately removed - reject report to prevent + // resurrection. Both stores must be consulted: the in-memory map resets on + // restart while the persisted entry keeps blocking (#1581), so a restart + // must not silently lift a deliberate block. + blockedID, removedAt, wasRemoved := m.lookupRemovedDockerHost(append([]string{identifier}, legacyIDs...)) + + if wasRemoved && tokenRecord != nil && + !tokenRecord.CreatedAt.IsZero() && tokenRecord.CreatedAt.After(removedAt) { + // A token minted after the host was removed means the user generated a + // fresh install command for this machine: that is explicit re-enroll + // intent, so clear the block instead of rejecting until the TTL + // expires or Allow reconnect is clicked. A still-running old agent + // keeps presenting its pre-removal token and stays blocked. This + // mirrors the host-agent rule from #1581. + if err := m.AllowDockerHostReenroll(blockedID); err != nil { + log.Warn(). + Err(err). + Str("dockerHostID", blockedID). + Msg("Failed to clear Docker host removal block for fresh re-enroll token; report remains blocked") + } else { + log.Info(). + Str("dockerHostID", blockedID). + Time("removedAt", removedAt). + Time("tokenCreatedAt", tokenRecord.CreatedAt). + Msg("Cleared Docker host removal block: report presented a token created after removal") + wasRemoved = false } } - m.mu.RUnlock() if wasRemoved { log.Info().