9832760027
gates / gates (push) Successful in 11s
R-329. NotifyAppStartFailures emitted severity "warn". The hub accepts exactly
{info, warning, error, critical} and silently coerces anything else to "info",
which severityNotifies then drops BEFORE both legs. Banner shown, event stored,
POST 200, no mail sent. One word.
This is the second time: DiskAlertKind.Severity emitted "warn" until v0.215.0
and its own comment records that every warning-level disk alert went to nobody.
A comment recorded the lesson and nothing enforced it. The guard is now an AST
walk over the whole controller - grep cannot work here, since "warn" appears
legitimately nine times as a healthcheck status vocabulary.
The sweep found exactly one bad severity. Its limits are stated: the walk cannot
follow a variable, so all six dynamic call sites are registered by name with the
values each can take, and a new one fails the test. Two of the six were found by
the guard, not by the hand sweep before it.
Also pinned: fillwatch.Band.Severity() returns "" for BandOK, which would vanish
the same way. It is unreachable because Check() notifies only on escalation -
but that safety lives in a different function from the one that looks unsafe, so
the test asserts the consequence rather than the mapping.
app_start_failed gains a customer toggle, DEFAULT OFF, per operator ruling. The
operator is mailed either way: processOperator never consults customer prefs.
It is deliberately NOT in operatorOnlyEvents, which would make the toggle a lie.
R-386. classifyRunStates decided "the customer stopped this" from the STATE, so
every stopped stack was assumed deliberate. Measured on demo-hp: privatebin
stopped out of band, nine scans, zero events, zero banner - while the comment
beside it claimed an out-of-band stop still alerts.
DesiredState already records the answer and has exactly one writer. Stopped ->
no alarm; Running -> alarm; absent -> UNKNOWN, keep today's behaviour AND say
so. Absent stays silent deliberately: reading it as "nobody asked" would email
about every app anyone ever stopped, fleet-wide, on the first cycle after
upgrade. The gap is bounded not silent - IntentUnknown is set and the names are
logged at INFO on the heartbeat cadence. failedRestart still lifts a Stopped
intent, or F-CRIT-1 re-opens. No new DesiredState writer.
Two settings toggles each governed two alarms. "Lemez figyelmeztetes (90%+)"
also wrote disk_critical, the drive-is-FAILING alarm. Now four honest toggles;
12 became 15. A no-op save stores the existing slice verbatim, so byte identity
is by construction - without that guard the defaults case reorders, which the
red-proof caught.
Test count 1504 -> 1522. Five red-proofs, five seen failing; one passed first
time and is reported - that mutation was inert, not the test weak.
223 lines
8.4 KiB
Go
223 lines
8.4 KiB
Go
package web
|
|
|
|
import (
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"net/url"
|
|
"reflect"
|
|
"regexp"
|
|
"strings"
|
|
"testing"
|
|
|
|
"gitea.dooplex.hu/admin/felhom-controller/internal/settings"
|
|
)
|
|
|
|
// R-329 Part 4 — two settings checkboxes each governed TWO alarms, and said so in neither label.
|
|
//
|
|
// `event_disk_alerts`, labelled „Lemez figyelmeztetés (90%+)", also wrote `disk_critical` — the
|
|
// drive-is-FAILING alarm. A customer turning off a disk-nearly-full notice silently turned off "this
|
|
// drive is dying". `event_expected_missed` had the same shape across the file-backup and
|
|
// database-dump misses.
|
|
//
|
|
// THE RISK IS NOT THE SPLIT, IT IS THE MIGRATION. Every existing customer's stored list was written
|
|
// by the OLD form names. After the split they render through new ones, so a customer who merely opens
|
|
// the page and presses Save travels a different code path than the one that wrote their settings.
|
|
// **A settings page that quietly changes a setting while rendering it is worse than the defect being
|
|
// fixed**, so the round trip below is the real subject of this file.
|
|
//
|
|
// THE LAYER. This drives the REAL render (`settingsNotificationsPageHandler`) and the REAL save
|
|
// (`settingsNotificationsHandler`) over a REAL temp-file `Settings`, and compares the STORED slice.
|
|
// A test that only checked the handler's parsing would miss the half that matters: what the template
|
|
// actually ticks.
|
|
//
|
|
// RED-PROOF (observed, see REPORT.md): drop the `sameEventSet` no-op guard in the save handler and
|
|
// TestR329Part4_RoundTripIsByteIdentical/defaults fails, showing the stored order rewritten.
|
|
|
|
// checkedBoxes renders the settings page and returns the form names the template ticked — i.e.
|
|
// exactly what a browser would POST if the customer pressed Save without touching anything.
|
|
func checkedBoxes(t *testing.T, s *Server) url.Values {
|
|
t.Helper()
|
|
req := httptest.NewRequest(http.MethodGet, "/settings/notifications", nil)
|
|
rr := httptest.NewRecorder()
|
|
s.settingsNotificationsPageHandler(rr, req)
|
|
if rr.Code != http.StatusOK {
|
|
t.Fatalf("render: HTTP %d", rr.Code)
|
|
}
|
|
body := rr.Body.String()
|
|
if !strings.Contains(body, "event_backup_failed") {
|
|
t.Fatalf("the notifications form did not render — this test would then prove nothing")
|
|
}
|
|
|
|
// <input type="checkbox" name="event_x" ... checked> — `checked` before the closing angle.
|
|
re := regexp.MustCompile(`<input type="checkbox" name="(event_[a-z_]+)"([^>]*)>`)
|
|
out := url.Values{}
|
|
for _, m := range re.FindAllStringSubmatch(body, -1) {
|
|
if strings.Contains(m[2], "checked") {
|
|
out.Set(m[1], "on")
|
|
}
|
|
}
|
|
return out
|
|
}
|
|
|
|
func TestR329Part4_RoundTripIsByteIdentical(t *testing.T) {
|
|
cases := []struct {
|
|
name string
|
|
stored []string
|
|
}{
|
|
{
|
|
// SHAPE 1: the default list every provisioned customer starts with — it contains BOTH
|
|
// halves of BOTH compounds, and in an order that is NOT the save handler's order.
|
|
name: "defaults",
|
|
stored: append([]string(nil), settings.DefaultEnabledEvents...),
|
|
},
|
|
{
|
|
// SHAPE 2: a customer who switched the compounds OFF — neither key present.
|
|
name: "compounds off",
|
|
stored: []string{"backup_failed", "node_down", "health_critical"},
|
|
},
|
|
{
|
|
// SHAPE 3: written by the OLD handler, so the compound pairs sit in its exact order.
|
|
name: "as the old handler wrote it",
|
|
stored: []string{
|
|
"backup_failed", "db_dump_failed", "storage_disconnected", "node_down",
|
|
"health_critical", "storage_reconnected",
|
|
"disk_warning", "disk_critical", "expected_backup_missed", "expected_dbdump_missed",
|
|
},
|
|
},
|
|
}
|
|
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
s, sett := notifyGuardServer(t)
|
|
if err := sett.SetNotificationPrefs(&settings.NotificationPrefs{
|
|
Email: "seed@felhom.eu", EnabledEvents: tc.stored, CooldownHours: 6,
|
|
}); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
before := append([]string(nil), sett.GetNotificationPrefs().EnabledEvents...)
|
|
|
|
// Render, take exactly what the template ticked, post it back unchanged.
|
|
form := checkedBoxes(t, s)
|
|
if len(form) == 0 && len(tc.stored) > 0 {
|
|
t.Fatalf("the template ticked NOTHING for a customer with %d stored events — the "+
|
|
"round trip would trivially 'pass' while silently wiping every setting", len(tc.stored))
|
|
}
|
|
form.Set("notification_email", "seed@felhom.eu")
|
|
form.Set("cooldown_hours", "6")
|
|
|
|
rr := postNotifications(t, s, form)
|
|
if rr.Code >= 400 {
|
|
t.Fatalf("save: HTTP %d", rr.Code)
|
|
}
|
|
|
|
after := sett.GetNotificationPrefs().EnabledEvents
|
|
if !reflect.DeepEqual(before, after) {
|
|
t.Errorf("a no-op save CHANGED the stored settings.\n before: %v\n after: %v\n"+
|
|
"A settings page must not rewrite a setting while merely rendering it.", before, after)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// The split itself: each half is now independently switchable. The whole point is that turning off
|
|
// "disk nearly full" must NOT turn off "disk failing".
|
|
func TestR329Part4_TheTwoDiskAlarmsAreIndependent(t *testing.T) {
|
|
s, sett := notifyGuardServer(t)
|
|
|
|
// Only the FAILING alarm on — the mild one off.
|
|
rr := postNotifications(t, s, url.Values{
|
|
"notification_email": {"a@b.hu"},
|
|
"cooldown_hours": {"6"},
|
|
"event_disk_critical": {"on"},
|
|
})
|
|
if rr.Code >= 400 {
|
|
t.Fatalf("save: HTTP %d", rr.Code)
|
|
}
|
|
got := sett.GetNotificationPrefs().EnabledEvents
|
|
if !reflect.DeepEqual(got, []string{"disk_critical"}) {
|
|
t.Fatalf("enabled = %v, want exactly [disk_critical] — a customer must be able to keep the "+
|
|
"drive-is-failing alarm while silencing the 90%%-full notice", got)
|
|
}
|
|
|
|
// And the mirror: the mild one on, the failing one off.
|
|
rr = postNotifications(t, s, url.Values{
|
|
"notification_email": {"a@b.hu"},
|
|
"cooldown_hours": {"6"},
|
|
"event_disk_warning": {"on"},
|
|
})
|
|
if rr.Code >= 400 {
|
|
t.Fatalf("save: HTTP %d", rr.Code)
|
|
}
|
|
if got := sett.GetNotificationPrefs().EnabledEvents; !reflect.DeepEqual(got, []string{"disk_warning"}) {
|
|
t.Fatalf("enabled = %v, want exactly [disk_warning]", got)
|
|
}
|
|
}
|
|
|
|
// The legacy compound form names must still be honoured — a browser left open on the old page, or a
|
|
// bookmarked POST, must not silently drop a key.
|
|
func TestR329Part4_LegacyCompoundNamesStillWork(t *testing.T) {
|
|
s, sett := notifyGuardServer(t)
|
|
rr := postNotifications(t, s, url.Values{
|
|
"notification_email": {"a@b.hu"},
|
|
"cooldown_hours": {"6"},
|
|
"event_disk_alerts": {"on"},
|
|
"event_expected_missed": {"on"},
|
|
})
|
|
if rr.Code >= 400 {
|
|
t.Fatalf("save: HTTP %d", rr.Code)
|
|
}
|
|
got := sett.GetNotificationPrefs().EnabledEvents
|
|
want := []string{"disk_warning", "disk_critical", "expected_backup_missed", "expected_dbdump_missed"}
|
|
if !reflect.DeepEqual(got, want) {
|
|
t.Fatalf("legacy compound POST stored %v, want %v", got, want)
|
|
}
|
|
}
|
|
|
|
// Both the legacy compound AND its replacement in one POST must not double-write a key.
|
|
func TestR329Part4_LegacyAndNewTogetherDoNotDuplicate(t *testing.T) {
|
|
s, sett := notifyGuardServer(t)
|
|
rr := postNotifications(t, s, url.Values{
|
|
"notification_email": {"a@b.hu"},
|
|
"cooldown_hours": {"6"},
|
|
"event_disk_alerts": {"on"},
|
|
"event_disk_warning": {"on"},
|
|
"event_disk_critical": {"on"},
|
|
})
|
|
if rr.Code >= 400 {
|
|
t.Fatalf("save: HTTP %d", rr.Code)
|
|
}
|
|
got := sett.GetNotificationPrefs().EnabledEvents
|
|
if !reflect.DeepEqual(got, []string{"disk_warning", "disk_critical"}) {
|
|
t.Fatalf("stored %v — a duplicated key would be pushed to the hub and re-render oddly", got)
|
|
}
|
|
}
|
|
|
|
// R-329 Part 1.3: the app-down toggle exists, is switchable, and is OFF by default.
|
|
func TestR329_AppStartFailedToggleExistsAndDefaultsOff(t *testing.T) {
|
|
for _, e := range settings.DefaultEnabledEvents {
|
|
if e == "app_start_failed" {
|
|
t.Fatalf("app_start_failed is in DefaultEnabledEvents — the operator ruled it OFF by " +
|
|
"default; the OPERATOR is emailed regardless, via processOperator, which never " +
|
|
"consults customer preferences")
|
|
}
|
|
}
|
|
|
|
s, sett := notifyGuardServer(t)
|
|
// Default render must not tick it.
|
|
if _, ticked := checkedBoxes(t, s)["event_app_start_failed"]; ticked {
|
|
t.Errorf("the app-down toggle renders as ON for a fresh customer")
|
|
}
|
|
// And it must actually be switchable.
|
|
rr := postNotifications(t, s, url.Values{
|
|
"notification_email": {"a@b.hu"},
|
|
"cooldown_hours": {"6"},
|
|
"event_app_start_failed": {"on"},
|
|
})
|
|
if rr.Code >= 400 {
|
|
t.Fatalf("save: HTTP %d", rr.Code)
|
|
}
|
|
if got := sett.GetNotificationPrefs().EnabledEvents; !reflect.DeepEqual(got, []string{"app_start_failed"}) {
|
|
t.Fatalf("enabled = %v, want [app_start_failed] — a visible toggle that stores nothing is a lie", got)
|
|
}
|
|
}
|