diff --git a/hub/CHANGELOG.md b/hub/CHANGELOG.md index 14601a6..f5306dc 100644 --- a/hub/CHANGELOG.md +++ b/hub/CHANGELOG.md @@ -1,5 +1,36 @@ # Felhom Hub — Changelog +## v0.78.0 — R-97a: the whole-guest backup tier gets a voice (operator-only) (2026-07-27) + +`internal/quiesce` had no route to the hub at all. On 2026-07-27 three failed whole-guest backups and +twelve app-stack stop/starts produced **zero** events. This is the hub half of the fix. + +**Two new operator-tier event types**, allowlisted with **no `customerMessages` entry**: +`whole_guest_backup_failed` (error) and `whole_guest_backup_recovered` (info). + +**Deliberately NOT `backup_failed`/`backup_completed`.** Both of those carry customer-facing Hungarian +templates AND sit in demo-felhom's live `enabled_events` — reusing them would have emailed the +**customer** „A biztonsági mentés sikertelen" while the backup was still retrying behind the R-88 +breaker. A customer can take no action on a failed whole-guest backup. Same pattern as R-85's +restore-test types: in the allowlist so the chain works, out of `customerMessages` so the dispatcher +*structurally cannot* route them to a customer. + +**The recovery rides the F11 pairing branch.** `whole_guest_backup_recovered` is severity `info`, and +`severityNotifies` drops `info` — routing it normally would store the event and never mail it, so the +operator would be told a tier broke and never told it healed. Adding it to `recoveredPairedDownTypes` +puts it on the recovery branch, which runs BEFORE the severity gate. Its customer leg needs no special +handling: that leg is pairing-gated on a customer-channel "sent" row for the down type, and the down +type can never produce one. + +**Per-tier operator cooldown** (`cooldownTierSuffix`). The operator cooldown was keyed +`customerID + ":" + eventType` — correct for an event describing ONE thing, wrong for one describing +ONE TIER when a box has two: `felhom-pbs` failing at 09:00 would swallow `local` failing at 09:20 for +the rest of the hour. The key now appends the `tier` from the event details **when present**, so no +existing event type's cooldown behaviour changes. Widening it for everything would turn one hourly +`app_start_failed` into one per app — a flood, not a fix. + +Tests +8. `build`/`vet`/`test` rc=0, 17 packages ok. + ## v0.77.0 — R-85 Part 2: a restore-test result becomes a SIGNAL (2026-07-26) Until now a failed restore-test was a `[WARN]` line in the ingest handler and nothing else — no diff --git a/hub/internal/api/handler.go b/hub/internal/api/handler.go index a649f5b..cf1f6bf 100644 --- a/hub/internal/api/handler.go +++ b/hub/internal/api/handler.go @@ -1555,6 +1555,19 @@ var allowedEventTypes = map[string]bool{ "restore_test_failed": true, "restore_test_stale": true, + // R-97a: the WHOLE-GUEST (vzdump) backup tier's outcome. Controller-pushed from + // `internal/quiesce`, which until now had no route to the hub at all — on 2026-07-27 three failed + // whole-guest backups and twelve app-stack stop/starts produced ZERO events. + // + // DELIBERATELY NOT `backup_failed`/`backup_completed`. Those two carry customerMessages entries + // AND sit in demo-felhom's live enabled_events, so reusing them would email the CUSTOMER, in + // Hungarian, that their backup failed — while it is still retrying behind the R-88 breaker. A + // customer can take no action on a failed whole-guest backup. These follow the R-85 pattern + // instead: allowlisted, with NO customerMessages entry, so the dispatcher structurally cannot + // route them to a customer. Do NOT add customerMessages entries without a copy review. + "whole_guest_backup_failed": true, + "whole_guest_backup_recovered": true, + // Controller-pushed events "controller_started": true, "claim_lockout": true, // v0.50.0 — claim/reset code brute-force lockout tripped diff --git a/hub/internal/api/whole_guest_event_test.go b/hub/internal/api/whole_guest_event_test.go new file mode 100644 index 0000000..acedda4 --- /dev/null +++ b/hub/internal/api/whole_guest_event_test.go @@ -0,0 +1,18 @@ +package api + +import "testing" + +// R-97a — the allowlist entry is the delivery chain's first link. +// +// The recorded gotcha: an event type missing from allowedEventTypes makes POST /event return 400 and +// the event vanishes. `internal/quiesce` had no route to the hub at all, so three failed whole-guest +// backups on 2026-07-27 produced zero events. This pins the entry so a cleanup cannot quietly +// re-break the chain. +func TestWholeGuestBackupEventTypesAreAllowlisted(t *testing.T) { + for _, et := range []string{"whole_guest_backup_failed", "whole_guest_backup_recovered"} { + if !allowedEventTypes[et] { + t.Fatalf("%s must be in allowedEventTypes, or POST /event 400s and the whole-guest "+ + "backup tier goes silent again", et) + } + } +} diff --git a/hub/internal/notify/cooldown_tier_test.go b/hub/internal/notify/cooldown_tier_test.go new file mode 100644 index 0000000..8897d57 --- /dev/null +++ b/hub/internal/notify/cooldown_tier_test.go @@ -0,0 +1,80 @@ +package notify + +import "testing" + +// R-97a — the operator cooldown must not let one backup tier mask another. +// +// The cooldown was keyed `customerID + ":" + eventType`, which is right for every event that +// describes ONE thing. A whole-guest backup failure describes ONE TIER, and a box has two: with the +// old key, `felhom-pbs` failing at 09:00 swallowed `local` failing at 09:20 for the rest of the hour. +// The operator would hear about the offsite tier and never about the local one — the exact masking +// the per-tier signal exists to prevent. + +func TestCooldownTierSuffix_SeparatesTiers(t *testing.T) { + pbs := cooldownTierSuffix(`{"tier":"felhom-pbs","error":"connection refused"}`) + local := cooldownTierSuffix(`{"tier":"local","error":"no space left"}`) + if pbs == local { + t.Fatalf("two tiers must produce DIFFERENT cooldown suffixes, both gave %q — one tier would mask the other", pbs) + } + if pbs != ":felhom-pbs" || local != ":local" { + t.Fatalf("suffix should be the tier: got %q and %q", pbs, local) + } +} + +// NARROWNESS IS THE POINT: every event type that does not send a tier must keep its old key exactly, +// or this becomes a notification flood instead of a fix. Widening the key for everything would turn +// one hourly app_start_failed into one per app. +func TestCooldownTierSuffix_EmptyForEverythingElse(t *testing.T) { + cases := []struct { + name string + details string + }{ + {"no details at all", ""}, + {"details without a tier", `{"error":"boom","drive_count":3}`}, + {"empty tier value", `{"tier":""}`}, + {"malformed json", `{{{not json`}, + {"tier mentioned in a STRING, not as a key", `{"error":"the tier: felhom-pbs is down"}`}, + {"null details", `null`}, + } + for _, c := range cases { + if got := cooldownTierSuffix(c.details); got != "" { + t.Errorf("%s: suffix must be EMPTY so the existing cooldown is unchanged, got %q", c.name, got) + } + } +} + +// The recovery must reach the operator despite being severity "info". +// +// severityNotifies drops "info", so routing whole_guest_backup_recovered normally would STORE the +// event and never mail it — the operator would be told the tier broke and never told it healed. +// Membership in recoveredPairedDownTypes puts it on the recovery branch, which runs BEFORE the +// severity gate. +func TestWholeGuestRecovery_IsOnTheRecoveryBranch(t *testing.T) { + if severityNotifies("info") { + t.Fatal("premise changed: info now notifies, so the pairing entry may be unnecessary — re-check") + } + paired, ok := recoveredPairedDownTypes["whole_guest_backup_recovered"] + if !ok { + t.Fatal("whole_guest_backup_recovered must be on the recovery branch, or its 'info' severity makes it silent") + } + found := false + for _, p := range paired { + if p == "whole_guest_backup_failed" { + found = true + } + } + if !found { + t.Fatalf("the recovery must pair with whole_guest_backup_failed; got %v", paired) + } +} + +// The customer must NOT hear either edge — a customer can take no action on a failed whole-guest +// backup, and being told it failed while it is retrying is alarming without being actionable. +func TestWholeGuestBackup_IsNeverCustomerRouted(t *testing.T) { + for _, et := range []string{"whole_guest_backup_failed", "whole_guest_backup_recovered"} { + if msg, ok := customerMessages[et]; ok { + t.Fatalf("%s must have NO customerMessages entry (operator-tier only); found %q — "+ + "adding one silently starts emailing customers about a backup they cannot act on", et, msg) + } + } +} diff --git a/hub/internal/notify/dispatcher.go b/hub/internal/notify/dispatcher.go index f1e9f79..ee3d40c 100644 --- a/hub/internal/notify/dispatcher.go +++ b/hub/internal/notify/dispatcher.go @@ -7,6 +7,7 @@ import ( "io" "log" "net/http" + "strings" "sync" "time" @@ -70,6 +71,16 @@ func priorityHeaders(severity string) map[string]string { var recoveredPairedDownTypes = map[string][]string{ "node_recovered": {"node_stale", "node_down"}, "host_recovered": {"host_stale", "host_down"}, + // R-97a. This branch runs BEFORE the severity gate, which is exactly why the recovery belongs + // here: `whole_guest_backup_recovered` is severity "info", and severityNotifies drops "info", so + // routing it normally would store the event and never mail it — the operator would be told the + // tier broke and never told it healed, which is the half of Scenario B that matters. + // + // The customer leg needs no special handling: it is PAIRING-gated on a customer-channel "sent" + // row for the down type, and `whole_guest_backup_failed` has no customerMessages entry and is in + // nobody's enabled_events — so no such row can exist, and the customer correctly hears neither + // edge. Operator hears both. + "whole_guest_backup_recovered": {"whole_guest_backup_failed"}, } // severityNotifies reports whether a severity triggers email notifications. warning / error / critical @@ -225,12 +236,36 @@ func (d *Dispatcher) processRecovery(customerID, eventType, severity, message, d d.store.LogNotification(customerID, eventType, severity, message, "sent", "", "customer") } +// cooldownTierSuffix returns ":"+tier when the event's details carry a non-empty `tier`, else "". +// +// R-97a. The operator cooldown was keyed `customerID + ":" + eventType` alone, which is correct for +// every event that describes ONE thing — but a whole-guest backup failure describes ONE TIER, and a +// box has two. `felhom-pbs` failing at 09:00 would swallow `local` failing at 09:20 for the whole +// hour, so the operator would be told about the offsite tier and never about the local one. That is +// precisely the masking the per-tier signal exists to prevent. +// +// NARROW ON PURPOSE: the suffix is empty unless the producer opts in by sending a `tier`, so no +// existing event type's cooldown behaviour changes. Widening the key for everything would, e.g., +// turn one hourly `app_start_failed` into one per app — a flood, not a fix. +func cooldownTierSuffix(detailsJSON string) string { + if detailsJSON == "" || !strings.Contains(detailsJSON, "\"tier\"") { + return "" + } + var d struct { + Tier string `json:"tier"` + } + if err := json.Unmarshal([]byte(detailsJSON), &d); err != nil || d.Tier == "" { + return "" + } + return ":" + d.Tier +} + func (d *Dispatcher) processOperator(customerID, eventType, severity, message, detailsJSON, source string) { if !d.operatorOn || d.operatorEmail == "" { return } - cooldownKey := customerID + ":" + eventType + cooldownKey := customerID + ":" + eventType + cooldownTierSuffix(detailsJSON) d.mu.Lock() if last, ok := d.opCooldowns[cooldownKey]; ok && time.Since(last) < 1*time.Hour { d.mu.Unlock() diff --git a/manifests/hub.yaml b/manifests/hub.yaml index 64d458f..10047ad 100644 --- a/manifests/hub.yaml +++ b/manifests/hub.yaml @@ -125,7 +125,7 @@ spec: spec: containers: - name: hub - image: gitea.dooplex.hu/admin/felhom-hub:0.77.0 + image: gitea.dooplex.hu/admin/felhom-hub:0.78.0 ports: - containerPort: 8080 name: http