From 8fad031d95d9e8463e2979f1c7af1cfa6890742e Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Tue, 15 Sep 2026 09:37:01 +0100 Subject: [PATCH] Keep agent credentials consistent across monitor reloads Reloading configuration replaced the monitor's config pointer while API and authentication handlers retained the original object. Tokens created afterwards could authenticate successfully while Agent Doctor reported an unlisted token. Refresh the canonical runtime configuration in place before constructing the replacement manager. Cover repeated reloads, live token creation and revocation. --- .../v6/internal/subsystems/monitoring.md | 10 ++++ internal/monitoring/reload.go | 14 +++++- internal/monitoring/reload_test.go | 46 +++++++++++++++++++ 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/monitoring.md b/docs/release-control/v6/internal/subsystems/monitoring.md index 628240b6dd..e3897ec5e2 100644 --- a/docs/release-control/v6/internal/subsystems/monitoring.md +++ b/docs/release-control/v6/internal/subsystems/monitoring.md @@ -3488,6 +3488,16 @@ of that current inventory immediately and cannot produce a false `agent_credential_missing` diagnosis. `MultiTenantMonitor` deep-copies only non-default tenant configuration; those tenant copies remain isolated from the primary runtime's mutable token state. + +That ownership survives a full monitor reload. `ReloadableMonitor` loads and +validates replacement settings, stops the old monitors, and refreshes the +original configuration under `config.Mu` before creating the replacement +manager. It must not split the retained API/authentication configuration from +the default monitor. `TestReloadPreservesLiveAgentCredentialInventory` exercises +two reloads followed by token creation and revocation, including the diagnostic +for a recently used credential. It preserves the registry-stale warning for a +credential that is actually absent, while preventing it for a live token. + `internal/fleethealth/agent_test.go` and `internal/monitoring/agent_fleet_doctor_test.go` are the focused runtime proofs. diff --git a/internal/monitoring/reload.go b/internal/monitoring/reload.go index 5e36b97dc7..fe1f408336 100644 --- a/internal/monitoring/reload.go +++ b/internal/monitoring/reload.go @@ -179,13 +179,23 @@ func (rm *ReloadableMonitor) doReload() error { // Wait a moment for cleanup time.Sleep(1 * time.Second) + // Authentication middleware and API handlers retain the original runtime + // configuration. Refresh that object so the replacement default monitor also + // sees later token creation, rotation and revocation through the same owner. + config.Mu.Lock() + if rm.config == nil { + rm.config = cfg + } else { + *rm.config = *cfg + } + config.Mu.Unlock() + // Create new multi-tenant monitor // Note: We lose existing instances state here, which is expected on full reload. - newMTMonitor := NewMultiTenantMonitor(cfg, rm.persistence, rm.wsHub) + newMTMonitor := NewMultiTenantMonitor(rm.config, rm.persistence, rm.wsHub) // Replace monitor rm.mtMonitor = newMTMonitor - rm.config = cfg // Start new monitor context (individual monitors are lazy loaded/started) rm.ctx, rm.cancel = context.WithCancel(rm.parentCtx) diff --git a/internal/monitoring/reload_test.go b/internal/monitoring/reload_test.go index adedc82487..41b9fb811c 100644 --- a/internal/monitoring/reload_test.go +++ b/internal/monitoring/reload_test.go @@ -82,6 +82,52 @@ func TestReloadableMonitor_Lifecycle_Coverage(t *testing.T) { rm.Stop() } +func TestReloadPreservesLiveAgentCredentialInventory(t *testing.T) { + t.Setenv("PULSE_DATA_DIR", t.TempDir()) + runtimeConfig, err := config.LoadWithoutLoggingInit() + require.NoError(t, err) + rm, err := NewReloadableMonitor(runtimeConfig, config.NewMultiTenantPersistence(runtimeConfig.DataPath), nil) + require.NoError(t, err) + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + rm.Start(ctx) + t.Cleanup(rm.Stop) + + // The server and authentication handlers retain runtimeConfig across reloads. + // Token creation and revocation replace its slice after the monitor restarts. + for generation := 0; generation < 2; generation++ { + require.NoError(t, rm.Reload()) + monitor := rm.GetMonitor() + require.NotNil(t, monitor) + now := time.Now().UTC() + config.Mu.Lock() + runtimeConfig.APITokens = []config.APITokenRecord{{ + ID: "reload-agent-token", CreatedAt: now, LastUsedAt: &now, + Scopes: []string{config.ScopeAgentExec}, + }} + config.Mu.Unlock() + monitor.state.UpsertHost(models.Host{ + ID: "reload-agent", Hostname: "reload-host", Status: "online", + LastSeen: now, AgentVersion: "6.4.1", TokenID: "reload-agent-token", + TokenLastUsedAt: &now, CommandsEnabled: true, + }) + agent := requireAgentDiagnostic(t, monitor.GetAgentFleetDiagnostics("6.4.1", now), "agent-reload-agent") + require.False(t, diagnosticHasAnyReason(agent.Reasons, + AgentFleetReasonCredentialMissing, AgentFleetReasonCredentialUnlisted, AgentFleetReasonExecScopeMissing), + "live credential is absent from diagnostics after reload: %+v", agent.Reasons) + require.Same(t, runtimeConfig, rm.GetConfig()) + require.Same(t, runtimeConfig, monitor.GetConfig()) + + // Revocation must also reach diagnostics. Recent usage still produces the + // existing contradiction warning, rather than hiding a missing credential. + config.Mu.Lock() + runtimeConfig.APITokens = nil + config.Mu.Unlock() + agent = requireAgentDiagnostic(t, monitor.GetAgentFleetDiagnostics("6.4.1", now), "agent-reload-agent") + requireReasonCode(t, agent, AgentFleetReasonCredentialUnlisted) + } +} + func TestReloadableMonitorAggregateInstallSnapshotCountsIncludesProvisionedTenants(t *testing.T) { baseDir := t.TempDir() cfg := &config.Config{DataPath: baseDir}