mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-09-10 02:25:56 +00:00
Clear temperature SSH failure backoff on system-settings save (#1638)
The reset added in b45bd66b9 only fired when the temperature SSH key
file on disk changed (mtime/size). An operator who repairs SSH access
any other way — fixing authorized_keys on the host, repairing
known_hosts, restoring network reachability — still waited out a
backoff window that may have compounded toward fifteen minutes.
A system-settings save is the natural operator touchpoint after such a
repair, so the settings handler now fans ResetSSHFailureBackoff out to
every live tenant monitor after a successful save, clearing the
per-host temperature SSH backoff and the knownhosts keyscan backoff.
The reset touches in-memory retry timing only; nothing is persisted
and no request field controls it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -2230,6 +2230,17 @@ fields. No agent-lifecycle behavior keyed off them — agent update targeting
|
||||
and command admission are unaffected — and the extension-point expectations
|
||||
on the system-settings boundary are otherwise unchanged.
|
||||
|
||||
### Shared system-settings boundary gained an SSH backoff reset side effect
|
||||
|
||||
The shared `internal/api` system-settings surface this subsystem consumes
|
||||
(`internal/api/system_settings.go`) now clears the temperature collector's
|
||||
SSH failure backoff on every live tenant monitor after a successful save
|
||||
(#1638). That backoff belongs to monitor-owned direct SSH temperature
|
||||
collection, not to the agent command transport: no agent registration, token,
|
||||
command admission, update, or fleet-lifecycle authority is touched, and the
|
||||
extension-point expectations on the system-settings boundary are otherwise
|
||||
unchanged.
|
||||
|
||||
### Monitor housekeeping prunes only live memory-evidence caches
|
||||
|
||||
The shared monitor housekeeping pass in `internal/monitoring/monitor_agents.go`
|
||||
|
||||
@@ -3738,6 +3738,21 @@ ignores them without a validation error and never writes them back into
|
||||
`internal/api/system_settings_telemetry_test.go` and the response snapshot in
|
||||
`internal/api/contract_test.go` pin that payload shape.
|
||||
|
||||
### System settings save clears the temperature SSH failure backoff
|
||||
|
||||
A successful `POST /api/system-settings` save now also calls
|
||||
`ResetSSHFailureBackoff` on every live tenant monitor (through the same
|
||||
`forEachTenantMonitor` fan-out the polling-cadence setters use), clearing the
|
||||
temperature collector's per-host SSH backoff and the knownhosts keyscan
|
||||
backoff (#1638). This is a save side effect, not payload surface: no request
|
||||
field controls it, it fires on any successful save regardless of which
|
||||
settings changed, and the request and response shapes are unchanged. The
|
||||
`SystemSettingsMonitor` interface gained the corresponding
|
||||
`ResetSSHFailureBackoff()` method.
|
||||
`TestIssue1638SettingsSaveResetsSSHFailureBackoff` in
|
||||
`internal/api/system_settings_telemetry_test.go` pins that a save touching no
|
||||
SSH-related field still triggers exactly one reset per monitor.
|
||||
|
||||
### Connections command-channel liveness tolerates stale host token IDs
|
||||
|
||||
The `/api/connections` ledger's `RemoteControl` and `CommandPolicy` signals
|
||||
|
||||
@@ -2506,10 +2506,13 @@ it at the floor when the collection deadline expired rather than compounding on
|
||||
evidence about Pulse's own budget rather than the host. Both backoffs decay: a
|
||||
failure whose retry deadline passed more than one window ago restarts at the
|
||||
floor instead of resuming the ceiling. Neither backoff may be a trap the
|
||||
operator cannot leave: replacing the temperature SSH key on disk clears both
|
||||
maps (`TemperatureCollector.ResetSSHFailures`, triggered from the per-cycle key
|
||||
change check), so repairing the key is retried on the next cycle rather than
|
||||
after a window that has compounded to fifteen minutes.
|
||||
operator cannot leave: `TemperatureCollector.ResetSSHFailures` clears both
|
||||
maps, and it is triggered both from the per-cycle key change check (replacing
|
||||
the temperature SSH key on disk) and from every system-settings save
|
||||
(`Monitor.ResetSSHFailureBackoff`, pushed into each live tenant monitor by the
|
||||
settings API), so repairing the key — or saving settings after repairing SSH
|
||||
access any other way — is retried on the next cycle rather than after a window
|
||||
that has compounded to fifteen minutes.
|
||||
`internal/monitoring/issue1638_dns_cache_test.go` is the registered proof that
|
||||
repeat polls stay on the DNS cache, that the link-local blocklist still rejects
|
||||
hostname endpoints resolving into it, and that the SSH backoffs suppress,
|
||||
|
||||
@@ -590,6 +590,23 @@ written before the removal still loads cleanly with the legacy keys ignored
|
||||
`TestSystemSettingsUpdate_LegacyAutoUpdateFieldsIgnored` in
|
||||
`internal/api/system_settings_telemetry_test.go`).
|
||||
|
||||
### Settings save resets only SSH retry timing, never SSH trust state
|
||||
|
||||
The governed system-settings write path in `internal/api/system_settings.go`
|
||||
now clears the temperature SSH failure backoff on every live tenant monitor
|
||||
after a successful save (#1638). The reset touches retry *timing* state only —
|
||||
in-memory per-host backoff windows and the knownhosts keyscan backoff — and
|
||||
never SSH trust state: it does not remove pinned host keys, does not relax
|
||||
known-hosts verification, and discloses nothing in the response. It runs
|
||||
behind the same authenticated write access as the rest of the settings
|
||||
mutation surface and adds no new accepted input — no request field enables,
|
||||
disables, or targets it, so there is no new attacker-controllable surface
|
||||
beyond the already-governed ability to save settings. The worst an authorized
|
||||
save can do is advance an SSH retry that the poll cycle would have run anyway
|
||||
once the window expired.
|
||||
`TestIssue1638SettingsSaveResetsSSHFailureBackoff` in
|
||||
`internal/api/system_settings_telemetry_test.go` pins the trigger.
|
||||
|
||||
### Canonical mutation-plane dependency
|
||||
|
||||
Raw command, file-write, arbitrary pod-exec, and legacy remediation authority
|
||||
|
||||
@@ -2013,6 +2013,16 @@ fields. Persisted `system.json` files that still carry the legacy keys load
|
||||
cleanly with the keys ignored, so tenant workspace preservation and recovery
|
||||
flows that copy `system.json` forward are unaffected.
|
||||
|
||||
### Shared system-settings boundary gained an SSH backoff reset side effect
|
||||
|
||||
The shared `internal/api` system-settings surface this subsystem consumes
|
||||
(`internal/api/system_settings.go`) now clears the temperature collector's
|
||||
in-memory SSH failure backoff on every live tenant monitor after a successful
|
||||
save (#1638). Nothing is persisted by the reset — `system.json` shape and the
|
||||
settings persistence path are unchanged — so tenant workspace preservation,
|
||||
recovery flows that copy `system.json` forward, and backup or restore
|
||||
attribution are unaffected.
|
||||
|
||||
### Agent exec token binding repairs are fail-closed durable writes
|
||||
|
||||
Command-token binding metadata (`bound_agent_id`, `bound_hostname`,
|
||||
|
||||
@@ -37,6 +37,7 @@ type SystemSettingsMonitor interface {
|
||||
SetBackupPollingInterval(interval time.Duration)
|
||||
SetPBSPollingInterval(interval time.Duration)
|
||||
SetPMGPollingInterval(interval time.Duration)
|
||||
ResetSSHFailureBackoff()
|
||||
}
|
||||
|
||||
// SystemSettingsHandler handles system settings
|
||||
@@ -1116,6 +1117,13 @@ func (h *SystemSettingsHandler) HandleUpdateSystemSettings(w http.ResponseWriter
|
||||
})
|
||||
}
|
||||
|
||||
// A settings save is an operator touchpoint that often follows repairing
|
||||
// SSH access; clear the temperature SSH backoff windows so the next poll
|
||||
// cycle retries instead of waiting one out (#1638).
|
||||
h.forEachTenantMonitor(r, func(m SystemSettingsMonitor) {
|
||||
m.ResetSSHFailureBackoff()
|
||||
})
|
||||
|
||||
// Reload cached system settings after successful save
|
||||
if h.reloadSystemSettingsFunc != nil {
|
||||
h.reloadSystemSettingsFunc()
|
||||
|
||||
@@ -22,10 +22,11 @@ import (
|
||||
|
||||
// MockMonitor implementation
|
||||
type mockMonitor struct {
|
||||
backupPollingEnabledCalls []bool
|
||||
backupPollingIntervalCalls []time.Duration
|
||||
pbsPollingIntervalCalls []time.Duration
|
||||
pmgPollingIntervalCalls []time.Duration
|
||||
backupPollingEnabledCalls []bool
|
||||
backupPollingIntervalCalls []time.Duration
|
||||
pbsPollingIntervalCalls []time.Duration
|
||||
pmgPollingIntervalCalls []time.Duration
|
||||
resetSSHFailureBackoffCalls int
|
||||
}
|
||||
|
||||
func (m *mockMonitor) GetDiscoveryService() *discovery.Service { return nil }
|
||||
@@ -47,6 +48,9 @@ func (m *mockMonitor) SetPBSPollingInterval(interval time.Duration) {
|
||||
func (m *mockMonitor) SetPMGPollingInterval(interval time.Duration) {
|
||||
m.pmgPollingIntervalCalls = append(m.pmgPollingIntervalCalls, interval)
|
||||
}
|
||||
func (m *mockMonitor) ResetSSHFailureBackoff() {
|
||||
m.resetSSHFailureBackoffCalls++
|
||||
}
|
||||
|
||||
type mockTenantMonitorProvider struct {
|
||||
orgID string
|
||||
|
||||
@@ -502,3 +502,49 @@ func TestSystemSettingsUpdate_LegacyAutoUpdateFieldsIgnored(t *testing.T) {
|
||||
t.Fatalf("real setting alongside legacy fields was dropped: connectionTimeout=%d", saved.ConnectionTimeout)
|
||||
}
|
||||
}
|
||||
|
||||
// TestIssue1638SettingsSaveResetsSSHFailureBackoff pins that saving system
|
||||
// settings clears the temperature SSH failure backoff on every live monitor.
|
||||
// A settings save is an operator touchpoint that often follows repairing SSH
|
||||
// access, and the on-disk key-change check only notices key file replacement,
|
||||
// so without this the operator waits out a backoff window that may have
|
||||
// compounded to fifteen minutes before Pulse retries (#1638).
|
||||
func TestIssue1638SettingsSaveResetsSSHFailureBackoff(t *testing.T) {
|
||||
tempDir := t.TempDir()
|
||||
cfg := &config.Config{
|
||||
DataPath: tempDir,
|
||||
ConfigPath: tempDir,
|
||||
EnvOverrides: make(map[string]bool),
|
||||
}
|
||||
persistence := config.NewConfigPersistence(cfg.DataPath)
|
||||
|
||||
tokenVal := "ssh-reset-test-token-123.12345678"
|
||||
tokenHash := internalauth.HashAPIToken(tokenVal)
|
||||
cfg.APITokens = []config.APITokenRecord{
|
||||
{ID: "tok1", Hash: tokenHash, Name: "Test Token"},
|
||||
}
|
||||
|
||||
monitor := &mockMonitor{}
|
||||
handler := newTestSystemSettingsHandler(cfg, persistence, monitor, func() {}, func() error { return nil })
|
||||
|
||||
initial := config.DefaultSystemSettings()
|
||||
if err := persistence.SaveSystemSettings(*initial); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
// A save that touches no SSH-related field must still clear the backoff:
|
||||
// the reset is tied to the save itself, not to any particular setting.
|
||||
body, _ := json.Marshal(map[string]interface{}{"connectionTimeout": 30})
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/system-settings", bytes.NewReader(body))
|
||||
req.Header.Set("X-API-Token", tokenVal)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
handler.HandleUpdateSystemSettings(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if monitor.resetSSHFailureBackoffCalls != 1 {
|
||||
t.Fatalf("settings save called ResetSSHFailureBackoff %d times, want 1", monitor.resetSSHFailureBackoffCalls)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -505,3 +505,43 @@ func TestIssue1638TemperatureDeadlineDoesNotCompound(t *testing.T) {
|
||||
t.Fatalf("deadline handling ran %d ssh commands, want 3 (one per cycle, no fallback, no compounding)", runner.runs)
|
||||
}
|
||||
}
|
||||
|
||||
// TestIssue1638MonitorResetSSHFailureBackoffDelegates pins the settings-save
|
||||
// escape hatch: Monitor.ResetSSHFailureBackoff must clear the temperature
|
||||
// collector's per-host SSH backoff (the settings API calls it on every live
|
||||
// tenant monitor after a save), and must be safe on a nil monitor or a monitor
|
||||
// without a temperature collector.
|
||||
func TestIssue1638MonitorResetSSHFailureBackoffDelegates(t *testing.T) {
|
||||
issue1638UseFakeClock(t)
|
||||
tc, runner := issue1638TemperatureCollector(t)
|
||||
|
||||
// Open a backoff window with a failing first cycle (sensors + fallback).
|
||||
if _, err := tc.CollectTemperature(context.Background(), "node1.local", "node1"); err != nil {
|
||||
t.Fatalf("CollectTemperature: %v", err)
|
||||
}
|
||||
if runner.runs != 2 {
|
||||
t.Fatalf("first cycle ran %d ssh commands, want 2", runner.runs)
|
||||
}
|
||||
if _, err := tc.CollectTemperature(context.Background(), "node1.local", "node1"); err != nil {
|
||||
t.Fatalf("CollectTemperature inside window: %v", err)
|
||||
}
|
||||
if runner.runs != 2 {
|
||||
t.Fatalf("backoff window still ran ssh, total %d commands, want 2", runner.runs)
|
||||
}
|
||||
|
||||
m := &Monitor{tempCollector: tc}
|
||||
m.ResetSSHFailureBackoff()
|
||||
|
||||
if _, err := tc.CollectTemperature(context.Background(), "node1.local", "node1"); err != nil {
|
||||
t.Fatalf("CollectTemperature after reset: %v", err)
|
||||
}
|
||||
if runner.runs != 4 {
|
||||
t.Fatalf("reset did not retry ssh, total %d commands, want 4", runner.runs)
|
||||
}
|
||||
|
||||
// Nil receiver and nil collector must both be no-ops, not panics: the
|
||||
// settings API calls this on whatever monitor the tenant lookup returns.
|
||||
var nilMonitor *Monitor
|
||||
nilMonitor.ResetSSHFailureBackoff()
|
||||
(&Monitor{}).ResetSSHFailureBackoff()
|
||||
}
|
||||
|
||||
@@ -153,6 +153,19 @@ func (m *Monitor) SetPMGPollingInterval(interval time.Duration) {
|
||||
m.runtimePollingMu.Unlock()
|
||||
}
|
||||
|
||||
// ResetSSHFailureBackoff clears the temperature collector's per-host SSH
|
||||
// backoff and the knownhosts keyscan backoff. A system-settings save is an
|
||||
// operator touchpoint that often follows repairing SSH access, and the
|
||||
// on-disk key-change check only notices key file replacement, so the save
|
||||
// itself also drops the backoff windows rather than making the operator wait
|
||||
// one out (#1638).
|
||||
func (m *Monitor) ResetSSHFailureBackoff() {
|
||||
if m == nil {
|
||||
return
|
||||
}
|
||||
m.tempCollector.ResetSSHFailures()
|
||||
}
|
||||
|
||||
// resetBackupPollTimestamps clears the per-instance backup poll timestamps
|
||||
// so the next cycle polls immediately.
|
||||
func (m *Monitor) resetBackupPollTimestamps() {
|
||||
|
||||
Reference in New Issue
Block a user