From cedf1d261da3a11a47a5cc663c5308c238491569 Mon Sep 17 00:00:00 2001 From: byrsapty Date: Fri, 28 Aug 2026 17:20:18 +0300 Subject: [PATCH] =?UTF-8?q?=D0=95=D1=81=D0=BA=D0=B0=D0=BB=D0=B0=D1=86?= =?UTF-8?q?=D1=96=D1=97:=20=D0=B6=D1=83=D1=80=D0=BD=D0=B0=D0=BB=20=D0=B1?= =?UTF-8?q?=D1=96=D0=BB=D1=8C=D1=88=D0=B5=20=D0=BD=D0=B5=20=D0=B1=D1=80?= =?UTF-8?q?=D0=B5=D1=88=D0=B5,=20ack=20=D0=BD=D0=B5=20=D0=B2=D0=BE=D1=81?= =?UTF-8?q?=D0=BA=D1=80=D0=B5=D1=88=D0=B0=D1=94=20=D0=B4=D1=80=D0=B0=D0=B1?= =?UTF-8?q?=D0=B8=D0=BD=D1=83?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Три вади, знайдені рецензією, яких щасливий шлях показати не міг: * outcome='sent' писався до доставки; помилка читання каналів клала в кеш порожню мапу й з'їдала всі сходинки кабінету за тік — усі зі слідом «надіслано». Канали тепер читаються до просування стану, журнал пишеться після доставки, з правдою. * UPDATE не мав stopped_at IS NULL — підтвердження алерту посеред партії не рятувало людину від дзвінка. * час брався раз на партію. Плюс суміжне: UpdateRule не гасив алертів вимкненого правила, сервер домислював enabled на оновленні, channel_ids сходинок не звірялись із каналами кабінету (зокрема чужого). І scripts/dbtest.sh — тести проти бази перестали мовчки пропускатись. Co-Authored-By: Claude Opus 5 --- HISTORY.md | 131 +++++++++++ scripts/dbtest.sh | 70 ++++++ server/API.md | 12 + server/internal/alerting/escalation.go | 108 ++++++--- server/internal/httpapi/alerts.go | 45 +++- .../store/alerts_channel_refs_db_test.go | 168 ++++++++++++++ server/internal/store/alerts_channels.go | 145 ++++++++++++ server/internal/store/alerts_escalation.go | 130 ++++++++++- .../store/alerts_escalation_channels_test.go | 166 ++++++++++++++ .../store/alerts_escalation_db_test.go | 80 ++++++- server/internal/store/alerts_query.go | 70 +++++- .../store/alerts_rule_update_db_test.go | 210 ++++++++++++++++++ .../internal/store/alerts_rule_update_test.go | 92 ++++++++ web/src/api/client.ts | 7 + web/src/pages/ChannelsPage.tsx | 24 +- web/src/pages/EscalationsPage.tsx | 68 +++++- web/src/pages/RulesPage.tsx | 7 + web/src/test/escalationchannels.test.tsx | 133 +++++++++++ web/src/test/rules.test.tsx | 102 +++++++++ web/src/types.ts | 7 + 20 files changed, 1719 insertions(+), 56 deletions(-) create mode 100644 scripts/dbtest.sh create mode 100644 server/internal/store/alerts_channel_refs_db_test.go create mode 100644 server/internal/store/alerts_escalation_channels_test.go create mode 100644 server/internal/store/alerts_rule_update_db_test.go create mode 100644 server/internal/store/alerts_rule_update_test.go create mode 100644 web/src/test/escalationchannels.test.tsx create mode 100644 web/src/test/rules.test.tsx diff --git a/HISTORY.md b/HISTORY.md index d8c6cfd..ca1a582 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -7320,3 +7320,134 @@ FAIL modal > тло стає inert, поки вікно відкрите **Чого тести НЕ покривають:** саму подорож події від прогону відповідності до каналу — це перевірено вручну на стенді, не в CI. + +## 2026-08-28 — Вимкнення правила через форму: алерти висіли вічно, а правка вмикала назад + +Дві вади навколо одного прапорця, знайдені рецензією й підтверджені на коді. + +**Вада А: `PUT` вимикав правило, не гасячи його алертів.** `SetRuleEnabled` +(перемикач у списку) після вимкнення кличе `resolveRuleAlerts` — і має на це +причину: вимкнене правило випадає з `ActiveRules`, тобто `ResolveMissing` за +ним більше не біжить і закрити свої алерти воно вже не зможе. `UpdateRule` +(та сама дія, але з форми) цього не робила. Алерт лишався в `firing` +назавжди, а драбина ескалації продовжувала будити за ним людей — з повторами +це тижні. + +**Вада Б: відсутнє `enabled` сервер читав як згоду ввімкнути.** Форма правил +поля не надсилала взагалі (перемикача в ній немає — він у списку), а +`httpapi/alerts.go` ставив `req.Enabled == nil || *req.Enabled`. Тобто +відкрити НАВМИСНО вимкнене правило, поправити в ньому будь-що й зберегти — +означало мовчки його ввімкнути. У формі при цьому не змінювалось нічого. + +**Що зроблено.** + +* `store.RuleInput.Enabled` став `*bool`: «поля не було» і «поле = false» — + різні наміри, і `bool` їх не розрізняв. На створенні `nil` досі означає + «увімкнене», на правці — «не чіпати». +* `store.planRuleEnabled` — рішення окремою чистою функцією: яким стане + прапорець і чи гасити алерти. Гасіння прив'язане до ПЕРЕХОДУ + «увімкнене → вимкнене», а не до нового значення: повторне збереження вже + вимкненого правила не додає зайвих подій у `event_outbox`. +* `store.UpdateRule` читає стан до правки через `SELECT … FOR UPDATE` у тій + самій транзакції — інакше між читанням і записом уміщається перемикач зі + списку, і гасіння не спрацювало б у жодному з двох записів. +* Форма (`web/src/pages/RulesPage.tsx`) тепер надсилає `enabled` явно. + Обидва боки полагоджені навмисно: сервер — щоб не ламати чужі скрипти, + написані за цим API, форма — щоб не покладатись на здогад узагалі. +* Тести: `alerts_rule_update_test.go` (чотири випадки рішення), + `alerts_rule_update_db_test.go` (проти бази: гасіння, черга подій, + відсутність зайвого проходу), `web/src/test/rules.test.tsx` (форма + надсилає стан обома боками — вимкнений і увімкнений). + +**Чого тести НЕ покривають:** прогін проти бази вимагає `NETPULSE_TEST_DSN` +і на машині, де це писалось, не запускався — Postgres там немає. Тобто SQL +самої `UpdateRule` перевірено лише компілятором і читанням. Не покрито також +дальший ланцюг: що погашений алерт справді знімає взведену драбину (це +робить фон за подією `alert.resolved`, не сама `UpdateRule`) і що гонка +«форма проти перемикача» справді розв'язується замком — обидва потребують +живого стенду. + +**Помічено, але НЕ виправлено (окремий обсяг).** Зміна селектора чи умови +правила старі алерти не чіпає. Для опитуваних джерел це самолікується +наступним тіком: `ResolveMissing` закриває все, чого немає серед свіжих +кандидатів. Для ПОДІЄВИХ (`syslog`, `ncm`, `compliance`, `trap`) — ні: +`engine.go` їх у цьому циклі пропускає взагалі, тож алерт, який більше не +відповідає жодному селектору, висить до `auto_close_seconds`, а при нулі — +доки його не закриє людина. Те саме стосується зміни `source` з опитуваного +на подієвий. + +--- + +## 2026-08-28 — Ескалації: рецензія, три виправлення й перший справжній прогін тестів проти бази + +### Ескалації доведено до кінця на живих даних + +Наскрізно, без вигаданого правила: прогін відповідності → 6 критичних +порушень → 6 алертів → драбина. 18 доставок у Telegram (перше сповіщення ++ дві сходинки × 6 алертів), усі `sent`. Драбина зупинилась сама з +причиною «подієвий алерт не повторюється». + +### Рецензія знайшла три вади, яких щасливий шлях показати не міг + +**Журнал ескалацій брехав.** `outcome='sent'` писався ДО доставки. Сходинка, +чиї канали видалили, вимкнули або підняли їм поріг серйозності, лишала в +журналі «надіслано» — доказ, заради якого журнал існує, стверджував +протилежне. Гірше: помилка читання каналів клала в кеш **порожню мапу**, +і одна тимчасова невдача з'їдала всі належні сходинки кабінету за тік, +кожна з них — зі слідом «надіслано». + +Виправлено перестановкою порядку: канали читаються ДО просування стану +(не прочитались — сходинка лишається належною й повториться), стан +просувається ДО доставки (щоб не надіслати двічі), а журнал пишеться +ПІСЛЯ доставки — з тим, що сталося насправді (`no_channels`, а не +`sent`). `ApplyEscalation` розділено на просування стану й +`LogEscalationStep`. + +**Драбину можна було воскресити після «Прийняти».** `UPDATE` у +`ApplyEscalation` не мав умови `stopped_at IS NULL`. Партія обробляється +послідовно, кожна доставка з власним таймаутом, тож розрив між «взяли +сходинку в чергу» і «надіслали» вимірюється хвилинами. Людина підтверджує +алерт — а `UPDATE` знімає `stopped_at` і будить її знову. Тепер такий +`UPDATE` не влучає в рядок, сходинка скасовується з записом у лог. + +**Час брався один раз на партію** — рішення пізніх сходинок рахувались від +застарілого моменту. Тепер на кожну сходинку свій. + +### Суміжні вади, знайдені тією ж рецензією + +* `UpdateRule` з `enabled:false` не гасив алертів правила (на відміну від + `SetRuleEnabled`) — вони висіли `firing` вічно, а драбина будила людей + до ~21 доби. Рішення винесено в чисту `planRuleEnabled`, гасіння + прив'язане до переходу «увімкнене → вимкнене». +* Сервер домислював `enabled: true` за відсутнім полем і на ОНОВЛЕННІ, а + форма правил це поле не слала — редагування вимкненого правила мовчки + його вмикало. Полагоджено обидва боки. +* `channel_ids` сходинок драбини не звірялись із реальними каналами — + можна було зберегти драбину з мертвими або **чужими** id, і вона + виглядала налаштованою. Перевірка закрита у store, а не в HTTP. + Видалення каналу тепер прибирає його зі сходинок і попереджає, які + драбини зачепить. Сходинка лишається порожньою, а не викидається: + викинута мовчки зсунула б чергування. + +### scripts/dbtest.sh — і перший прогін тестів проти справжньої бази + +Найдорожча тиха відмова проєкту: тести проти бази мовчки пропускаються +без `NETPULSE_TEST_DSN`, тож `go test ./...` півтора року показував «ok», +а всередині кожного стояв `t.Skip`. Ізоляція кабінетів, стеля тарифу, +гасіння алертів, драбини, чистка пристрою, SLA, запис карт — усе +компілювалось і не виконувалось. + +Тепер є `scripts/dbtest.sh`: піднімає одноразовий Postgres, котить +міграції, ганяє `./internal/...`. Прогнано на стенді проти чистої бази — +**63 міграції, усі пакети зелені**, включно з новими перевірками +підтвердження посеред партії, оновлення правила й посилань на канали. + +`scripts/check.sh` лишається швидким (без Docker) — його ганяють на кожну +правку. Цей — перед розгортанням. + +**Що ще НЕ покрито:** доставка ескалації блокує весь такт движка (мертвий +вебхук одного кабінету затримує обчислення правил усім); алерт, народжений +під заглушенням або в тиху годину, не отримує ні першого сповіщення, ні +драбини — ніколи; втрата advisory-lock може дати подвійне сповіщення, бо +`ApplyEscalation` не звіряє оренду; та сама відсутність перевірки каналів +живе в `alr.rules.channel_ids` і `alr.routes.channel_ids`. diff --git a/scripts/dbtest.sh b/scripts/dbtest.sh new file mode 100644 index 0000000..7ce6423 --- /dev/null +++ b/scripts/dbtest.sh @@ -0,0 +1,70 @@ +#!/bin/sh +# Прогін тестів проти СПРАВЖНЬОЇ бази. +# +# ЧОМУ ЦЕ ОКРЕМИЙ СКРИПТ +# +# Тести проти бази мовчки пропускаються без NETPULSE_TEST_DSN — і саме +# тому вони півтора року нічого не перевіряли: `go test ./...` показував +# «ok», а всередині кожного з них стояв t.Skip. «Пропущено» в підсумку +# виглядає рівно як «пройдено», і це найдорожча тиха відмова в проєкті: +# перевірка ізоляції кабінетів, стелі тарифу, гасіння алертів і драбин +# ескалації існували, компілювались і не виконувались. +# +# scripts/check.sh лишається швидким (без Docker і без бази) — його +# ганяють на кожну правку. Цей скрипт повільніший і потребує Postgres, +# тож викликається окремо: перед розгортанням і в CI. +# +# БАЗА МУСИТЬ БУТИ ОДНОРАЗОВОЮ. Тести пишуть, видаляють і перемикають +# ролі; напрямляти їх на робочу базу не можна. Скрипт свою базу створює +# сам і дропає на початку кожного прогону. +# +# ВИКОРИСТАННЯ +# scripts/dbtest.sh # підніме свій Postgres у Docker +# NETPULSE_TEST_DSN=... scripts/dbtest.sh # проти готової бази +# +set -u + +SRC="$(cd "$(dirname "$0")/.." && pwd)" +OWN_DB=0 + +if [ -z "${NETPULSE_TEST_DSN:-}" ]; then + # Своя база на час прогону. Порт випадковий-таки ні: фіксований, але + # нетиповий, щоб не зіткнутись із локальним Postgres розробника. + PORT=${NETPULSE_TEST_PORT:-55433} + NAME=netpulse-dbtest + echo "== піднімаю одноразовий Postgres :$PORT" + docker rm -f "$NAME" >/dev/null 2>&1 + docker run -d --name "$NAME" -p "$PORT:5432" \ + -e POSTGRES_USER=netpulse -e POSTGRES_PASSWORD=probe \ + -e POSTGRES_DB=netpulse_probe \ + timescale/timescaledb:2.17.2-pg16 >/dev/null || exit 1 + OWN_DB=1 + NETPULSE_TEST_DSN="postgres://netpulse:probe@127.0.0.1:$PORT/netpulse_probe?sslmode=disable" + export NETPULSE_TEST_DSN + n=0 + until docker exec "$NAME" pg_isready -h 127.0.0.1 -U netpulse >/dev/null 2>&1; do + n=$((n+1)); [ "$n" -gt 60 ] && { echo "база не піднялась"; exit 1; } + sleep 1 + done +fi + +cleanup() { + [ "$OWN_DB" = 1 ] && docker rm -f netpulse-dbtest >/dev/null 2>&1 + return 0 +} +trap cleanup EXIT INT TERM + +echo "== накат міграцій" +( cd "$SRC/server" && NETPULSE_DSN="$NETPULSE_TEST_DSN" go run ./cmd/netpulse-migrate ) || exit 1 + +echo "== тести" +( cd "$SRC/server" && go test ./internal/... -count=1 ) +rc=$? + +echo +if [ "$rc" = 0 ]; then + echo "усе зелене проти бази" +else + echo "!!! тести проти бази не пройшли" +fi +exit $rc diff --git a/server/API.md b/server/API.md index a98f248..d8cdb99 100644 --- a/server/API.md +++ b/server/API.md @@ -107,6 +107,7 @@ JWT — ні. | `POST` | `/api/v1/mutes` | заглушити пристрій (`alerts:ack`) | | `GET` | `/api/v1/alert-rules` | правила з лічильником активних | | `POST` | `/api/v1/alert-rules` | створити правило (`alerts:write`) | +| `PUT` | `/api/v1/alert-rules/{id}` | замінити правило цілком (`alerts:write`) | | `PATCH` | `/api/v1/alert-rules/{id}` | увімкнути/вимкнути (`alerts:write`) | | `DELETE` | `/api/v1/alert-rules/{id}` | видалити правило (`alerts:write`) | | `GET` | `/api/v1/channels` | канали доставки (без секретів) | @@ -1172,6 +1173,17 @@ JSON у таблиці правил. `PUT /api/v1/alert-rules/{id}` замінює правило цілком. +**`enabled` на правці не домислюється.** Поля немає в тілі — стан +перемикача лишається таким, яким був. Раніше сервер підставляв `true` +кожному, хто поля не надіслав, і правка вимкненого правила мовчки його +вмикала. На СТВОРЕННІ відсутнє поле досі означає «увімкнене»: правило, +заведене вимкненим, не робить нічого й виглядає як забуте. + +**Вимкнення правила гасить його активні алерти** — і через `PATCH`, і +через `PUT`. Без цього вони висіли б у `firing` вічно: вимкнене правило +випадає з вибірки движка, тобто закрити їх немає кому, а драбина +ескалації справно будила б за ними людей тижнями. + ### Ескалація Сповіщення, надіслане один раз, нічого не гарантує: черговий може спати. diff --git a/server/internal/alerting/escalation.go b/server/internal/alerting/escalation.go index 9b9c1ea..7004545 100644 --- a/server/internal/alerting/escalation.go +++ b/server/internal/alerting/escalation.go @@ -54,46 +54,101 @@ func (e *Engine) escalate(ctx context.Context) { // може бути десяток. channels := map[string]map[string]store.Channel{} - now := time.Now() for _, snap := range due { - d := store.PlanEscalation(snap, now) + // Час береться на кожну сходинку, а не на партію: між першою і + // останньою може пройти скільки завгодно — кожна доставка має + // власний таймаут. Застарілий момент зсував би стелю життя й + // підлогу інтервалу рівно на цю затримку. + d := store.PlanEscalation(snap, time.Now()) - // Запис ДО надсилання — той самий порядок, що й у журналі - // доставки, і з тієї ж причини: якщо процес упаде між ними, - // краще не надіслати сходинку, ніж надіслати її вдруге. - if err := e.st.ApplyEscalation(ctx, snap, d); err != nil { + // Канали читаються ДО просування стану. Порядок не косметичний: + // якщо їх не вдалось прочитати, сходинка має лишитись належною й + // повторитись наступного тіку. У зворотному порядку тимчасова + // помилка бази списувала б сходинку назавжди — і в журналі + // стояло б «надіслано». + var byID map[string]store.Channel + if d.Action == store.EscFire { + var err error + byID, err = e.escalationChannels(ctx, snap.TenantID, channels) + if err != nil { + e.log.Error("читання каналів для ескалації — сходинку відкладено", + "tenant", snap.TenantID, "алерт", snap.AlertID, "помилка", err) + continue + } + } + + applied, err := e.st.ApplyEscalation(ctx, snap, d) + if err != nil { e.log.Error("запис рішення ескалації", "алерт", snap.AlertID, "помилка", err) continue } + if !applied { + // Драбину зупинили, поки сходинка чекала своєї черги — + // найчастіше людина натиснула «Прийняти». Це не помилка, це + // той випадок, заради якого кнопка й існує. + e.log.Info("сходинку скасовано: драбину вже зупинено", + "алерт", snap.AlertID, "сходинка", d.StepIdx+1) + continue + } + if d.Action != store.EscFire { + e.logStep(ctx, snap, d, d.Outcome, d.Detail) e.log.Debug("ескалацію не продовжено", "алерт", snap.AlertID, "причина", d.Outcome, "деталі", d.Detail) continue } - byID, ok := channels[snap.TenantID] - if !ok { - cs, err := e.st.LoadChannels(ctx, snap.TenantID, e.ring) - if err != nil { - e.log.Error("читання каналів для ескалації", - "tenant", snap.TenantID, "помилка", err) - channels[snap.TenantID] = map[string]store.Channel{} - continue - } - byID = make(map[string]store.Channel, len(cs)) - for _, c := range cs { - byID[c.ID] = c - } - channels[snap.TenantID] = byID + if sent := e.notifier.deliverEscalation(ctx, snap, d, byID); sent == 0 { + // Сходинка списана — інакше вона поверталася б щотіку. Але в + // журнал іде правда, а не намір: «надіслано» на сходинці, яка + // нікуди не пішла, — саме та мовчазна відмова, від якої + // ескалація рятує. + e.logStep(ctx, snap, d, "no_channels", + "жоден канал сходинки не прийняв повідомлення (видалено, вимкнено або поріг серйозності)") + continue } - - e.notifier.deliverEscalation(ctx, snap, d, byID) + e.logStep(ctx, snap, d, d.Outcome, d.Detail) } } -// deliverEscalation шле сходинку в її канали. +// escalationChannels читає канали кабінету, кешуючи лише успіх. +// +// Кешувати помилку не можна: порожня мапа в кеші означала б, що одна +// тимчасова невдача з'їдає всі належні сходинки кабінету за цей тік, і +// кожна з них виглядала б доставленою. +func (e *Engine) escalationChannels(ctx context.Context, tenantID string, + cache map[string]map[string]store.Channel) (map[string]store.Channel, error) { + + if byID, ok := cache[tenantID]; ok { + return byID, nil + } + cs, err := e.st.LoadChannels(ctx, tenantID, e.ring) + if err != nil { + return nil, err + } + byID := make(map[string]store.Channel, len(cs)) + for _, c := range cs { + byID[c.ID] = c + } + cache[tenantID] = byID + return byID, nil +} + +// logStep пише рядок журналу й не дає невдалому запису зупинити чергу. +func (e *Engine) logStep(ctx context.Context, snap store.EscalationSnapshot, + d store.EscalationDecision, outcome, detail string) { + + if err := e.st.LogEscalationStep(ctx, snap, d, outcome, detail); err != nil { + e.log.Error("журнал ескалації", "алерт", snap.AlertID, "помилка", err) + } +} + +// deliverEscalation шле сходинку в її канали й повертає, скільком дійшло. +// +// Кількість потрібна тому, хто пише журнал: сходинка без жодного каналу +// має лишити слід «нікуди не пішло», а не «надіслано». func (n *Notifier) deliverEscalation(ctx context.Context, snap store.EscalationSnapshot, - d store.EscalationDecision, byID map[string]store.Channel) { + d store.EscalationDecision, byID map[string]store.Channel) int { a := snap.Alert head := escalationHeader(snap, d) @@ -117,14 +172,11 @@ func (n *Notifier) deliverEscalation(ctx context.Context, snap store.EscalationS } if sent == 0 { - // Сходинка вже списана (рішення записано до надсилання), і це - // правильно: інакше вона поверталася б щотіку. Але мовчазна - // втрата сходинки — саме те, від чого ескалація рятує, тож слід - // лишається в журналі процесу. n.log.Warn("сходинка ескалації не мала куди піти", "алерт", snap.AlertID, "сходинка", d.StepIdx+1, "каналів у сходинці", len(d.ChannelIDs)) } + return sent } // escalationHeader пояснює людині, чому вона це читає. diff --git a/server/internal/httpapi/alerts.go b/server/internal/httpapi/alerts.go index 4313ffd..ba9160c 100644 --- a/server/internal/httpapi/alerts.go +++ b/server/internal/httpapi/alerts.go @@ -303,10 +303,24 @@ func (s *Server) handleCreateAlertRule(w http.ResponseWriter, r *http.Request, p Condition: string(req.Condition), ForSeconds: req.ForSeconds, DependsOnTopology: req.DependsOnTopology == nil || *req.DependsOnTopology, - Enabled: req.Enabled == nil || *req.Enabled, - ChannelIDs: req.ChannelIDs, - NotifySchedule: string(req.NotifySchedule), - NotifyOnResolve: req.NotifyOnResolve == nil || *req.NotifyOnResolve, + // `enabled` передається як є, разом з «поля не було». + // + // На створенні відсутність поля лишається згодою ввімкнути — це + // розумно й так було завжди (правило, заведене вимкненим, не + // робить нічого). На ОНОВЛЕННІ домислювати вже не можна: сервер + // ставив `true` кожному, хто поля не надіслав, тож правка опису + // вимкненого правила мовчки його вмикала — і людина дізнавалась + // про це зі сповіщення, а не з форми. + // + // Обрано саме «не було = не чіпати», а не «не було = 400»: PUT + // без `enabled` шле і власна форма, і будь-який чужий скрипт, + // написаний за цим API, тож вимога поля зламала б їх усі одним + // оновленням. Прапорець при цьому лишається керованим — його + // явно шлють і форма (тепер), і PATCH-перемикач. + Enabled: req.Enabled, + ChannelIDs: req.ChannelIDs, + NotifySchedule: string(req.NotifySchedule), + NotifyOnResolve: req.NotifyOnResolve == nil || *req.NotifyOnResolve, AutoCloseSeconds: autoClose, MinIntervalSeconds: minInterval, @@ -424,6 +438,20 @@ func (s *Server) handleListChannels(w http.ResponseWriter, r *http.Request, p *P if channels == nil { channels = []store.Channel{} } + // Драбини, що спираються на канал, — частина відповіді на питання + // «що зламається, якщо його видалити». Те саме питання вже має + // відповідь у переліку драбин (rule_count), і тут вона потрібна з + // тієї ж причини. + // + // Невдача не валить перелік: без назв драбин сторінка лишається + // робочою, а без каналів — ні. + if refs, err := s.store.ChannelEscalationRefs(r.Context(), p.TenantID); err != nil { + s.log.Error("драбини на каналах", "err", err) + } else { + for i := range channels { + channels[i].Escalations = refs[channels[i].ID] + } + } writeJSON(w, http.StatusOK, map[string]any{"channels": channels}) } @@ -685,6 +713,15 @@ func (s *Server) handleSaveEscalationPolicy(w http.ResponseWriter, r *http.Reque writeError(w, http.StatusNotFound, "not_found", "політику не знайдено") return } + // Відмова від store — це відмова людині, а не збій сервера: + // саме тут повертається «сходинка N посилається на канал, якого + // немає в цьому кабінеті». Без цієї гілки вона вийшла б назовні + // як «внутрішня помилка», тобто перевірка спрацювала б, а + // причини ніхто б не побачив. + if errors.Is(err, store.ErrInvalid) { + writeError(w, http.StatusBadRequest, "bad_steps", err.Error()) + return + } if isUniqueViolation(err) { writeError(w, http.StatusConflict, "duplicate", "політика з такою назвою вже є") return diff --git a/server/internal/store/alerts_channel_refs_db_test.go b/server/internal/store/alerts_channel_refs_db_test.go new file mode 100644 index 0000000..5cd67dd --- /dev/null +++ b/server/internal/store/alerts_channel_refs_db_test.go @@ -0,0 +1,168 @@ +package store + +import ( + "context" + "errors" + "os" + "strings" + "testing" + "time" +) + +// Канали в сходинках драбини ПРОТИ БАЗИ. +// +// Чиста ValidateStepChannels перевіряє рішення, і це половина. Друга +// половина існує тільки в базі й рішенням не перевіряється взагалі: +// +// - чи справді перелік каналів, який отримує перевірка, звужений до +// кабінету (тобто чи не проходить UUID сусіда); +// - чи прибирає видалення каналу посилання на нього зі сходинок — +// на JSONB зовнішнього ключа немає, і ON DELETE SET NULL, яким +// 0066 чистить драбину з правила, тут не спрацює. +// +// Мовчки пропускається без NETPULSE_TEST_DSN. Запускати треба на +// ОДНОРАЗОВІЙ базі — тест створює два кабінети й видаляє їх з усім +// вмістом: +// +// NETPULSE_TEST_DSN=postgres://postgres:x@localhost/np \ +// go test ./internal/store/ -run ChannelRefs -v +func TestChannelRefsAgainstDB(t *testing.T) { + dsn := os.Getenv("NETPULSE_TEST_DSN") + if dsn == "" { + t.Skip("NETPULSE_TEST_DSN не задано — перевірка проти бази пропускається") + } + ctx := context.Background() + + st, err := New(ctx, dsn) + if err != nil { + t.Fatalf("підключення: %v", err) + } + t.Cleanup(st.Close) + + stamp := strings.ReplaceAll(time.Now().Format("150405.000"), ".", "") + + newTenant := func(prefix string) string { + t.Helper() + var id string + if err := st.pool.QueryRow(ctx, ` + INSERT INTO core.tenants (slug, name) VALUES ($1, $2) RETURNING id::text + `, prefix+"-"+stamp, "Перевірка каналів драбини").Scan(&id); err != nil { + t.Fatalf("кабінет %s: %v", prefix, err) + } + t.Cleanup(func() { + _, _ = st.pool.Exec(context.Background(), `DELETE FROM core.tenants WHERE id = $1`, id) + }) + return id + } + + newChannel := func(tenantID, name string) string { + t.Helper() + var id string + if err := st.pool.QueryRow(ctx, ` + INSERT INTO alr.channels (tenant_id, kind, name, config, min_severity, enabled) + VALUES ($1, 'webhook', $2, '{}'::jsonb, 'warning', true) RETURNING id::text + `, tenantID, name).Scan(&id); err != nil { + t.Fatalf("канал %s: %v", name, err) + } + return id + } + + ours := newTenant("chref-a") + theirs := newTenant("chref-b") + + duty := newChannel(ours, "черговий") + lead := newChannel(ours, "старший зміни") + alien := newChannel(theirs, "чужий канал") + + // --- Канал чужого кабінету не зберігається ------------------------- + // + // Найдорожча з двох вад. Ззовні це не витік (доставити в чужий + // канал усе одно нікуди — движок читає канали лише свого кабінету), + // а гірше: драбина виглядає налаштованою й мовчить. Закривати це в + // HTTP-шарі марно — обійде будь-який інший шлях запису, тому тест + // б'є прямо в store. + _, err = st.SaveEscalationPolicy(ctx, ours, "", EscalationPolicy{ + Name: "Підсунутий сусід", + Steps: []EscalationStep{{AfterMin: 15, ChannelIDs: []string{alien}}}, + }) + if err == nil { + t.Fatal("драбина з каналом ЧУЖОГО кабінету збереглася") + } + if !errors.Is(err, ErrInvalid) { + t.Fatalf("відмова має бути ErrInvalid: %v", err) + } + + // --- Неіснуючий канал не зберігається ------------------------------ + _, err = st.SaveEscalationPolicy(ctx, ours, "", EscalationPolicy{ + Name: "Привид", + Steps: []EscalationStep{ + {AfterMin: 15, ChannelIDs: []string{duty}}, + {AfterMin: 45, ChannelIDs: []string{"00000000-0000-4000-8000-0000000000ff"}}, + }, + }) + if err == nil || !strings.Contains(err.Error(), "сходинка 2") { + t.Fatalf("драбина з неіснуючим каналом на другій сходинці: %v", err) + } + + // --- Справна драбина зберігається ---------------------------------- + policyID, err := st.SaveEscalationPolicy(ctx, ours, "", EscalationPolicy{ + Name: "Нічне чергування", + Steps: []EscalationStep{ + {AfterMin: 15, ChannelIDs: []string{duty}}, + {AfterMin: 45, ChannelIDs: []string{duty, lead}}, + }, + }) + if err != nil { + t.Fatalf("справна драбина не збереглася: %v", err) + } + + // --- Перелік «хто на кого спирається» ------------------------------ + // + // Це те, що бачить людина перед видаленням каналу. Порожній перелік + // означав би те саме мовчазне видалення, з якого все й почалось. + refs, err := st.ChannelEscalationRefs(ctx, ours) + if err != nil { + t.Fatalf("посилання драбин: %v", err) + } + if len(refs[duty]) != 1 || refs[duty][0] != "Нічне чергування" { + t.Fatalf("канал чергового не показано як зайнятий драбиною: %v", refs[duty]) + } + // Канал згадано у двох сходинках однієї драбини — назва має бути + // одна: людині цікаво, ЩО зламається, а не скільки разів. + if len(refs[lead]) != 1 { + t.Fatalf("драбина порахована двічі: %v", refs[lead]) + } + if len(refs[alien]) != 0 { + t.Fatal("у перелік кабінету потрапив чужий канал") + } + + // --- Видалення каналу чистить сходинки ----------------------------- + if err := st.DeleteChannel(ctx, ours, duty); err != nil { + t.Fatalf("видалення каналу: %v", err) + } + ps, err := st.ListEscalationPolicies(ctx, ours) + if err != nil { + t.Fatalf("перелік драбин: %v", err) + } + var got EscalationPolicy + for _, p := range ps { + if p.ID == policyID { + got = p + } + } + if len(got.Steps) != 2 { + t.Fatalf("видалення каналу зсунуло драбину: %d сходинок замість 2", len(got.Steps)) + } + // Перша сходинка лишається — порожня й видима. Саме порожня, а не + // викинута: зникла сходинка мовчки змінила б чергування, якого + // ніхто не міняв. + if len(got.Steps[0].ChannelIDs) != 0 { + t.Fatalf("посилання на видалений канал лишилось у сходинці 1: %v", got.Steps[0].ChannelIDs) + } + if len(got.Steps[1].ChannelIDs) != 1 || got.Steps[1].ChannelIDs[0] != lead { + t.Fatalf("сходинка 2 має лишитись зі старшим зміни: %v", got.Steps[1].ChannelIDs) + } + if got.Steps[0].AfterMin != 15 || got.Steps[1].AfterMin != 45 { + t.Fatalf("хвилини сходинок змінились: %v", got.Steps) + } +} diff --git a/server/internal/store/alerts_channels.go b/server/internal/store/alerts_channels.go index 6f516b0..22fb1d9 100644 --- a/server/internal/store/alerts_channels.go +++ b/server/internal/store/alerts_channels.go @@ -24,6 +24,12 @@ type Channel struct { Enabled bool `json:"enabled"` Secret string `json:"-"` HasSecret bool `json:"has_secret"` + // Назви драбин ескалації, сходинки яких посилаються на цей канал. + // + // Заповнюється лише для переліку в UI (ChannelEscalationRefs), а не + // в LoadChannels: движку доставки це не потрібно, а рахувати на + // кожному тіку — платити без причини. + Escalations []string `json:"escalations,omitempty"` } // Route — правило маршрутизації алерту в канали. @@ -320,6 +326,113 @@ func (s *Store) CreateChannel(ctx context.Context, tenantID string, in ChannelIn return id, err } +// escalationLadder — драбина в тому вигляді, в якому її читає чистка +// посилань: ідентифікатор, назва й розібрані сходинки. +type escalationLadder struct { + id string + name string + steps []EscalationStep +} + +// readLadders читає драбини кабінету з уже розібраними сходинками. +// +// Читання окремо від запису навмисно: pgx не дає слати новий запит, +// поки не дочитано попередній, а чистка посилань — це саме «прочитати +// всі, переписати деякі». +func readLadders(ctx context.Context, tx pgx.Tx, tenantID string) ([]escalationLadder, error) { + rows, err := tx.Query(ctx, ` + SELECT id::text, name, steps::text + FROM alr.escalation_policies WHERE tenant_id = $1 ORDER BY name + `, tenantID) + if err != nil { + return nil, err + } + defer rows.Close() + var out []escalationLadder + for rows.Next() { + var l escalationLadder + var steps string + if err := rows.Scan(&l.id, &l.name, &steps); err != nil { + return nil, err + } + if err := json.Unmarshal([]byte(steps), &l.steps); err != nil { + return nil, fmt.Errorf("політика %s: сходинки: %w", l.name, err) + } + out = append(out, l) + } + return out, rows.Err() +} + +// removeChannelFromSteps прибирає канал зі сходинок і каже, чи щось +// змінилось. +// +// Сходинка, яка лишилась без каналів, ЛИШАЄТЬСЯ порожньою, а не +// зникає. Викинута сходинка мовчки зсунула б усе чергування нижче, +// якого людина не міняла; порожню видно і в переліку драбин, і у формі +// (вона не збережеться, поки канал не оберуть), а движок пише про неї +// в журнал окремим рядком. +func removeChannelFromSteps(steps []EscalationStep, channelID string) ([]EscalationStep, bool) { + changed := false + out := make([]EscalationStep, len(steps)) + for i, s := range steps { + out[i] = s + kept := make([]string, 0, len(s.ChannelIDs)) + for _, id := range s.ChannelIDs { + if id == channelID { + changed = true + continue + } + kept = append(kept, id) + } + out[i].ChannelIDs = kept + } + return out, changed +} + +// ChannelEscalationRefs каже, які драбини посилаються на які канали: +// ідентифікатор каналу → назви драбин. +// +// Потрібне рівно для одного: щоб «Видалити канал» показало те саме, що +// вже показує «Видалити драбину» — скільки чужих налаштувань зараз +// перестане працювати. Мовчазне видалення каналу, на який спирається +// нічне чергування, коштує однієї пропущеної аварії, і дізнаються про +// це не в момент видалення. +func (s *Store) ChannelEscalationRefs(ctx context.Context, tenantID string) (map[string][]string, error) { + refs := map[string][]string{} + err := s.InTenantTx(ctx, tenantID, func(tx pgx.Tx) error { + ladders, err := readLadders(ctx, tx, tenantID) + if err != nil { + return err + } + for _, l := range ladders { + // Драбину називаємо один раз, скільки б сходинок у неї не + // вело в цей канал: людині перед видаленням цікаво, ЩО + // зламається, а не скільки разів воно згадане. + for _, id := range ladderChannelIDs(l.steps) { + refs[id] = append(refs[id], l.name) + } + } + return nil + }) + return refs, err +} + +// ladderChannelIDs — канали драбини без повторів, у порядку появи. +func ladderChannelIDs(steps []EscalationStep) []string { + var out []string + seen := map[string]bool{} + for _, s := range steps { + for _, id := range s.ChannelIDs { + if id == "" || seen[id] { + continue + } + seen[id] = true + out = append(out, id) + } + } + return out +} + func (s *Store) DeleteChannel(ctx context.Context, tenantID, channelID string) error { return s.InTenantTx(ctx, tenantID, func(tx pgx.Tx) error { // Секрет видаляється разом із каналом: залишений «на всякий @@ -348,6 +461,38 @@ func (s *Store) DeleteChannel(ctx context.Context, tenantID, channelID string) e if deleted == 0 { return ErrAlertNotFound } + + // Посилання зі сходинок драбин прибираємо руками, бо прибрати їх + // нікому: 0066 чистить драбину з правила через ON DELETE SET + // NULL, але на масив усередині JSONB зовнішнього ключа немає. + // Залишений UUID видаленого каналу — це сходинка, яка виглядає + // налаштованою й не йде нікуди, тобто рівно та мовчазна + // обіцянка, заради якої ескалацію й заводили. + // + // Переписуємо в Go, а не запитом по JSONB: рішення «що саме + // лишається в сходинці» перевіряється тоді без Postgres, а + // запити зводяться до читання й запису. + ladders, err := readLadders(ctx, tx, tenantID) + if err != nil { + return err + } + for _, l := range ladders { + steps, changed := removeChannelFromSteps(l.steps, channelID) + if !changed { + continue + } + raw, err := json.Marshal(steps) + if err != nil { + return err + } + if _, err := tx.Exec(ctx, ` + UPDATE alr.escalation_policies + SET steps = $3::jsonb, updated_at = now() + WHERE tenant_id = $1 AND id = $2 + `, tenantID, l.id, string(raw)); err != nil { + return err + } + } return nil }) } diff --git a/server/internal/store/alerts_escalation.go b/server/internal/store/alerts_escalation.go index 0bb227b..0188aa2 100644 --- a/server/internal/store/alerts_escalation.go +++ b/server/internal/store/alerts_escalation.go @@ -91,6 +91,65 @@ func ValidateEscalationSteps(steps []EscalationStep) error { return nil } +// ValidateStepChannels відмовляє в драбині, сходинка якої посилається на +// канал, якого в цьому кабінеті немає. +// +// Перевірка окремо від ValidateEscalationSteps, бо вона єдина потребує +// бази: решта драбини перевіряється як текст, а «чи є такий канал» — +// лише запитом. Розділення дозволяє тримати першу половину чистою й +// перевіреною без Postgres. +// +// Ціна відсутності цієї перевірки — рівно та сама мовчазна обіцянка, +// заради якої й написана вся 0066. Сходинка з неіснуючим UUID +// виглядає в переліку налаштованою, движок не знаходить для неї жодного +// каналу, списує її й пише в журнал «нікуди не пішло» — але читає той +// журнал уже той, хто прийшов розбиратися вранці, а не той, кого мали +// розбудити вночі. +// +// known — ідентифікатори каналів ЦЬОГО кабінету. Саме тому перевірка +// закриває й підстановку чужого UUID: перелік читається під RLS у тій +// самій транзакції, що й запис. +func ValidateStepChannels(steps []EscalationStep, known map[string]bool) error { + for i, s := range steps { + for _, id := range s.ChannelIDs { + if known[id] { + continue + } + // Ідентифікатор у тексті лишаємо навмисно: у формі канали + // обираються галочками, тож людина, яка це побачила, + // надсилає драбину не з форми — і їй потрібно знати, який + // саме рядок не прийнято. + return fmt.Errorf("%w: сходинка %d посилається на канал %s, якого немає в цьому "+ + "кабінеті — оберіть канал зі списку на сторінці «Канали»", ErrInvalid, i+1, id) + } + } + return nil +} + +// tenantChannelIDs — ідентифікатори каналів кабінету, як їх бачить ця +// транзакція. +// +// Читається саме в транзакції запису, а не окремим викликом до неї: +// інакше між перевіркою й записом лишалась би щілина, в якій канал +// встигає зникнути. +func tenantChannelIDs(ctx context.Context, tx pgx.Tx, tenantID string) (map[string]bool, error) { + rows, err := tx.Query(ctx, + `SELECT id::text FROM alr.channels WHERE tenant_id = $1`, tenantID) + if err != nil { + return nil, err + } + defer rows.Close() + known := map[string]bool{} + for rows.Next() { + var id string + if err := rows.Scan(&id); err != nil { + return nil, err + } + known[id] = true + } + return known, rows.Err() +} + // ListEscalationPolicies читає політики кабінету. func (s *Store) ListEscalationPolicies(ctx context.Context, tenantID string) ([]EscalationPolicy, error) { var out []EscalationPolicy @@ -142,6 +201,21 @@ func (s *Store) SaveEscalationPolicy(ctx context.Context, tenantID, id string, p } err = s.InTenantTx(ctx, tenantID, func(tx pgx.Tx) error { + // Обидві перевірки тут, а не лише в HTTP: драбина — це список + // людей, яких будять уночі, і єдине місце, де він може бути + // перевірений раз і назавжди, — це запис у базу. Перевірка, + // продубльована в кожному обробнику, розходиться на першому ж + // новому шляху запису (імпорт, шаблон, API-токен). + if err := ValidateEscalationSteps(p.Steps); err != nil { + return err + } + known, err := tenantChannelIDs(ctx, tx, tenantID) + if err != nil { + return err + } + if err := ValidateStepChannels(p.Steps, known); err != nil { + return err + } if id == "" { return tx.QueryRow(ctx, ` INSERT INTO alr.escalation_policies @@ -550,22 +624,38 @@ func (s *Store) TakeDueEscalations(ctx context.Context, limit int) ([]Escalation return out, rows.Err() } -// ApplyEscalation записує рішення й веде журнал. +// ApplyEscalation просуває стан драбини й повертає, чи вдалось. // -// Записується ДО надсилання. Порядок той самий, що в RecordNotification, -// і з тієї ж причини: падіння між записом і надсиланням лишає слід -// «сходинку пройдено» на недоставленому повідомленні, а зворотний -// порядок лишав би драбину на місці — і після підйому вона надіслала б -// те саме вдруге. -func (s *Store) ApplyEscalation(ctx context.Context, snap EscalationSnapshot, d EscalationDecision) error { +// Стан записується ДО надсилання — з тієї ж причини, що й у +// RecordNotification: падіння між записом і надсиланням лишає +// непройдену сходинку, а зворотний порядок лишав би драбину на місці, і +// після підйому вона надіслала б те саме вдруге. Краще не надіслати, ніж +// надіслати двічі о третій ночі. +// +// А от ЖУРНАЛ пишеться після доставки (LogEscalationStep), і це окреме +// рішення. Журнал існує рівно для відповіді на «чому мене розбудили» та +// «чому не розбудили», тож рядок «надіслано» на сходинці, якій не +// знайшлось жодного каналу, гірший за відсутній: він перетворює +// доказ на брехню. Ціна — рідкісний випадок падіння між просуванням і +// записом: сходинка лишиться без рядка. Це видно (step_idx більший за +// кількість рядків) і це чесно. +// +// stopped_at IS NULL — не оптимізація, а перевірка стану. Між тим, як +// сходинку взяли в чергу, і тим, як до неї дійшли руки, людина могла +// натиснути «Прийняти»: партія обробляється послідовно, кожна доставка +// має власний таймаут, і розрив вимірюється хвилинами. Без цієї умови +// UPDATE зняв би stopped_at і воскресив зупинену драбину — тобто +// розбудив би саме того, хто щойно сказав «я цим займаюсь». +func (s *Store) ApplyEscalation(ctx context.Context, snap EscalationSnapshot, d EscalationDecision) (bool, error) { var next any if d.NextAt != nil { next = *d.NextAt } fired := d.Action == EscFire - return s.InTenantTx(ctx, snap.TenantID, func(tx pgx.Tx) error { - if _, err := tx.Exec(ctx, ` + applied := false + err := s.InTenantTx(ctx, snap.TenantID, func(tx pgx.Tx) error { + tag, err := tx.Exec(ctx, ` UPDATE alr.alert_escalations SET step_idx = $2, repeat_idx = $3, @@ -576,16 +666,34 @@ func (s *Store) ApplyEscalation(ctx context.Context, snap EscalationSnapshot, d stopped_at = CASE WHEN $5::timestamptz IS NULL THEN now() ELSE NULL END, stop_reason = CASE WHEN $5::timestamptz IS NULL THEN $7 ELSE NULL END WHERE alert_id = $1 + AND stopped_at IS NULL `, snap.AlertID, d.NextStepIdx, d.NextRepeatIdx, d.NextPassStart, - next, fired, d.Outcome); err != nil { + next, fired, d.Outcome) + if err != nil { return err } + applied = tag.RowsAffected() > 0 + return nil + }) + return applied, err +} + +// LogEscalationStep записує в журнал те, що СПРАВДІ сталося зі сходинкою. +// +// Викликається після доставки, тому outcome тут може відрізнятись від +// того, що планувалось: сходинка, чиї канали видалили, вимкнули або +// підняли їм поріг серйозності, отримує 'no_channels', а не 'sent'. +// Саме заради цієї різниці журнал і винесено з ApplyEscalation. +func (s *Store) LogEscalationStep(ctx context.Context, snap EscalationSnapshot, + d EscalationDecision, outcome, detail string) error { + + return s.InTenantTx(ctx, snap.TenantID, func(tx pgx.Tx) error { _, err := tx.Exec(ctx, ` INSERT INTO alr.escalation_steps (tenant_id, alert_id, policy_id, step_idx, repeat_idx, outcome, detail) VALUES ($1, $2, $3, $4, $5, $6, $7) `, snap.TenantID, snap.AlertID, nullUUID(snap.PolicyID), - d.StepIdx, d.RepeatIdx, d.Outcome, nullString(d.Detail)) + d.StepIdx, d.RepeatIdx, outcome, nullString(detail)) return err }) } diff --git a/server/internal/store/alerts_escalation_channels_test.go b/server/internal/store/alerts_escalation_channels_test.go new file mode 100644 index 0000000..60db077 --- /dev/null +++ b/server/internal/store/alerts_escalation_channels_test.go @@ -0,0 +1,166 @@ +package store + +import ( + "errors" + "strings" + "testing" +) + +// Перевірки сходинки, яка посилається на канал. +// +// Ці тести описують одну ваду й одну її ціну. Драбина зберігалась із +// будь-яким рядком у channel_ids — хоч із UUID видаленого каналу, хоч +// із UUID каналу чужого кабінету. Форма показувала таку сходинку +// налаштованою; движок не знаходив для неї жодного каналу, списував її +// й ішов далі. Тобто драбина існувала, виглядала робочою і не будила +// нікого — рівно той стан, від якого ескалацію й заводили. +// +// ValidateEscalationSteps перевіряє форму драбини й не має доступу до +// бази; ValidateStepChannels перевіряє належність каналів і отримує +// перелік готовим. Розділені саме тому, що перша половина має лишитись +// перевіреною без Postgres — а без другої перша дає хибну впевненість. + +func knownChannels(ids ...string) map[string]bool { + m := map[string]bool{} + for _, id := range ids { + m[id] = true + } + return m +} + +// Головний випадок: канал, якого в кабінеті немає. +func TestStepChannelUnknownRefused(t *testing.T) { + steps := []EscalationStep{ + {AfterMin: 15, ChannelIDs: []string{"ch-duty"}}, + {AfterMin: 45, ChannelIDs: []string{"ch-lead", "ch-gone"}}, + } + err := ValidateStepChannels(steps, knownChannels("ch-duty", "ch-lead")) + if err == nil { + t.Fatal("драбина з неіснуючим каналом збереглася — вона нікого не розбудить") + } + if !errors.Is(err, ErrInvalid) { + t.Fatalf("відмова має бути ErrInvalid (інакше HTTP віддасть 500): %v", err) + } + // Відмова без номера сходинки марна: у драбині їх до десяти, і + // «щось не так із каналами» не каже людині, що саме виправляти. + if !strings.Contains(err.Error(), "сходинка 2") { + t.Fatalf("відмова не називає сходинку: %q", err) + } +} + +// Дзеркальний випадок: усе на місці — відмови бути не має. +func TestStepChannelsKnownAccepted(t *testing.T) { + steps := []EscalationStep{ + {AfterMin: 15, ChannelIDs: []string{"ch-duty"}}, + {AfterMin: 45, ChannelIDs: []string{"ch-duty", "ch-lead"}}, + } + if err := ValidateStepChannels(steps, knownChannels("ch-duty", "ch-lead", "ch-boss")); err != nil { + t.Fatalf("справна драбина не збереглася: %v", err) + } +} + +// Канал чужого кабінету — той самий випадок, і це головне в ньому. +// +// Перевірка не знає слова «чужий»: їй дають перелік каналів ЦЬОГО +// кабінету, прочитаний під RLS у транзакції запису. Тому підставлений +// UUID сусіда не проходить не як окремий випадок, а як частина +// загального правила — і його не можна забути закрити окремо. +func TestStepChannelFromOtherTenantRefused(t *testing.T) { + const foreign = "00000000-0000-4000-8000-000000000001" + steps := []EscalationStep{{AfterMin: 15, ChannelIDs: []string{foreign}}} + err := ValidateStepChannels(steps, knownChannels("ch-duty")) + if err == nil { + t.Fatal("канал чужого кабінету прийнято в сходинку") + } + if !strings.Contains(err.Error(), foreign) { + t.Fatalf("відмова не називає ідентифікатор: %q", err) + } +} + +// Вимкнений канал — не привід відмовляти. +// +// Перелік каналів кабінету не фільтрується за enabled навмисно: +// «вимкнув Telegram на час переїзду» не має ламати збереження драбини, +// у якій він стоїть. Про вимкнений канал говорить форма, і це інша +// розмова, ніж «такого каналу немає». +func TestStepChannelDisabledStillValid(t *testing.T) { + steps := []EscalationStep{{AfterMin: 15, ChannelIDs: []string{"ch-off"}}} + if err := ValidateStepChannels(steps, knownChannels("ch-off")); err != nil { + t.Fatalf("вимкнений канал відхилено: %v", err) + } +} + +// --- Чистка сходинок при видаленні каналу ---------------------------- +// +// Друга з підтверджених вад: видалення каналу не чіпало сходинок, що на +// нього посилались. Зовнішнього ключа на масив усередині JSONB немає, +// тож ON DELETE SET NULL, яким 0066 прибирає драбину з правила, тут не +// спрацьовує — і в сходинці лишався UUID каналу, якого вже немає. + +func TestRemoveChannelKeepsEmptyStep(t *testing.T) { + steps := []EscalationStep{ + {AfterMin: 15, ChannelIDs: []string{"ch-gone"}}, + {AfterMin: 45, ChannelIDs: []string{"ch-gone", "ch-lead"}}, + } + got, changed := removeChannelFromSteps(steps, "ch-gone") + if !changed { + t.Fatal("посилання на видалений канал лишилось у драбині") + } + // Сходинка лишається на місці порожньою. Викинута зникла б + // безслідно й мовчки зсунула б усе чергування нижче: «через 45» на + // другій сходинці стало б першим підйомом, якого ніхто не просив. + if len(got) != 2 { + t.Fatalf("драбину зсунуто: %d сходинок замість 2", len(got)) + } + if len(got[0].ChannelIDs) != 0 { + t.Fatalf("сходинка 1 мала лишитись порожньою: %v", got[0].ChannelIDs) + } + if got[0].AfterMin != 15 || got[1].AfterMin != 45 { + t.Fatalf("хвилини сходинок змінились: %+v", got) + } + if len(got[1].ChannelIDs) != 1 || got[1].ChannelIDs[0] != "ch-lead" { + t.Fatalf("сходинка 2 втратила чужий канал: %v", got[1].ChannelIDs) + } +} + +// Драбина, яка каналу не знає, не має переписуватись. +// +// Не заради швидкості: зайвий UPDATE зсунув би updated_at і в переліку +// правок виглядав би як зміна чергування, якої не було. +func TestRemoveChannelUntouchedLadder(t *testing.T) { + steps := []EscalationStep{{AfterMin: 15, ChannelIDs: []string{"ch-duty"}}} + got, changed := removeChannelFromSteps(steps, "ch-gone") + if changed { + t.Fatal("драбину без цього каналу оголошено зміненою") + } + if len(got[0].ChannelIDs) != 1 { + t.Fatalf("канали чужої драбини змінились: %v", got[0].ChannelIDs) + } +} + +// Драбина називається один раз, скільки б сходинок у неї не вело в цей +// канал: перед видаленням цікаво, ЩО зламається, а не скільки разів +// воно згадане. +func TestLadderChannelIDsDeduplicated(t *testing.T) { + ids := ladderChannelIDs([]EscalationStep{ + {AfterMin: 15, ChannelIDs: []string{"ch-duty"}}, + {AfterMin: 45, ChannelIDs: []string{"ch-duty", "ch-lead"}}, + }) + if len(ids) != 2 || ids[0] != "ch-duty" || ids[1] != "ch-lead" { + t.Fatalf("перелік каналів драбини: %v", ids) + } +} + +// Порожня сходинка лишається справою ValidateEscalationSteps. +// +// Тут вона проходить — і має проходити: дві перевірки не мають +// дублювати одна одну, інакше повідомлення розійдуться. +func TestStepChannelsEmptyStepIsOtherCheck(t *testing.T) { + steps := []EscalationStep{{AfterMin: 15, ChannelIDs: nil}} + if err := ValidateStepChannels(steps, knownChannels()); err != nil { + t.Fatalf("порожню сходинку має ловити ValidateEscalationSteps, а не ця перевірка: %v", err) + } + if err := ValidateEscalationSteps(steps); err == nil { + t.Fatal("порожню сходинку не спіймала жодна перевірка") + } +} diff --git a/server/internal/store/alerts_escalation_db_test.go b/server/internal/store/alerts_escalation_db_test.go index 11a6a0c..ab26dc0 100644 --- a/server/internal/store/alerts_escalation_db_test.go +++ b/server/internal/store/alerts_escalation_db_test.go @@ -59,9 +59,22 @@ func TestEscalationAgainstDB(t *testing.T) { t.Fatalf("хост: %v", err) } + // Канал справжній, а не рядок «ch»: збереження драбини тепер + // відмовляє в сходинці, що посилається на неіснуючий канал. + var channelID string + if err := st.pool.QueryRow(ctx, ` + INSERT INTO alr.channels (tenant_id, kind, name, config, min_severity, enabled) + VALUES ($1, 'webhook', $2, '{}'::jsonb, 'warning', true) RETURNING id::text + `, tenantID, slug+"-ch").Scan(&channelID); err != nil { + t.Fatalf("канал: %v", err) + } + policy := EscalationPolicy{ - Name: "Нічне чергування", - Steps: []EscalationStep{{AfterMin: 15, ChannelIDs: []string{"ch"}}, {AfterMin: 45, ChannelIDs: []string{"ch"}}}, + Name: "Нічне чергування", + Steps: []EscalationStep{ + {AfterMin: 15, ChannelIDs: []string{channelID}}, + {AfterMin: 45, ChannelIDs: []string{channelID}}, + }, } policyID, err := st.SaveEscalationPolicy(ctx, tenantID, "", policy) if err != nil { @@ -139,9 +152,16 @@ func TestEscalationAgainstDB(t *testing.T) { if d.Action != EscFire { t.Fatalf("сходинка мала спрацювати: %v/%s", d.Action, d.Outcome) } - if err := st.ApplyEscalation(ctx, due[0], d); err != nil { + applied, err := st.ApplyEscalation(ctx, due[0], d) + if err != nil { t.Fatalf("запис рішення: %v", err) } + if !applied { + t.Fatal("рішення не застосовано до живої драбини") + } + if err := st.LogEscalationStep(ctx, due[0], d, d.Outcome, d.Detail); err != nil { + t.Fatalf("журнал сходинки: %v", err) + } var stepIdx int var leased *time.Time @@ -201,6 +221,60 @@ func TestEscalationAgainstDB(t *testing.T) { } } + // --- Підтвердження ПОСЕРЕД партії не має воскрешати драбину -------- + // + // Найдовший розрив у механізмі: сходинку взяли в чергу, і поки до неї + // дійшли руки (кожна доставка попередніх — з власним таймаутом), + // людина натиснула «Прийняти». Тут відтворено саме цей порядок: + // знімок узято ДО ack, рішення застосовується ПІСЛЯ. + a3 := newAlert(ruleID + ":dev:" + deviceID + ":3") + if err := st.ArmEscalation(ctx, tenantID, a3, policyID, false, policy, started); err != nil { + t.Fatal(err) + } + if _, err := st.pool.Exec(ctx, + `UPDATE alr.alert_escalations SET next_at = now() - interval '1 minute' WHERE alert_id = $1`, + a3); err != nil { + t.Fatal(err) + } + dueRace, err := st.TakeDueEscalations(ctx, 10) + if err != nil { + t.Fatal(err) + } + var snap EscalationSnapshot + for _, x := range dueRace { + if x.AlertID == a3 { + snap = x + } + } + if snap.AlertID == "" { + t.Fatal("сходинка не потрапила в чергу") + } + dRace := PlanEscalation(snap, time.Now()) + if dRace.Action != EscFire { + t.Fatalf("сходинка мала спрацювати: %v/%s", dRace.Action, dRace.Outcome) + } + if _, err := st.AckAlert(ctx, tenantID, a3, "", "беру"); err != nil { + t.Fatalf("підтвердження: %v", err) + } + appliedRace, err := st.ApplyEscalation(ctx, snap, dRace) + if err != nil { + t.Fatal(err) + } + if appliedRace { + t.Error("рішення застосовано до вже зупиненої драбини — людину розбудять після ack") + } + var raceStop string + var raceNext *time.Time + if err := st.pool.QueryRow(ctx, ` + SELECT COALESCE(stop_reason,''), next_at + FROM alr.alert_escalations WHERE alert_id = $1 + `, a3).Scan(&raceStop, &raceNext); err != nil { + t.Fatal(err) + } + if raceStop != "acked" || raceNext != nil { + t.Errorf("зупинену драбину воскрешено: причина %q, наступна %v", raceStop, raceNext) + } + // --- Закритий алерт: сходинка не спрацьовує навіть якщо настала ---- a2 := newAlert(ruleID + ":dev:" + deviceID + ":2") if err := st.ArmEscalation(ctx, tenantID, a2, policyID, false, policy, started); err != nil { diff --git a/server/internal/store/alerts_query.go b/server/internal/store/alerts_query.go index 0cac4c1..39f1b21 100644 --- a/server/internal/store/alerts_query.go +++ b/server/internal/store/alerts_query.go @@ -350,7 +350,17 @@ type RuleInput struct { Condition string ForSeconds int DependsOnTopology bool - Enabled bool + // Enabled — вказівник, бо «поле не прийшло» і «поле прийшло зі + // значенням false» — це різні наміри, а bool їх не розрізняє. + // + // nil на СТВОРЕННІ означає «увімкнене»: правило, заведене вимкненим, + // не робить нічого й виглядає як забуте. + // + // nil на ОНОВЛЕННІ означає «не чіпати». Домислювати тут `true` — + // саме та вада, через яку правка опису мовчки вмикала вимкнене + // правило: форма поля не надсилала, а сервер читав його відсутність + // як згоду ввімкнути. + Enabled *bool // Куди слати. Порожньо — за загальними маршрутами тенанта. ChannelIDs []string NotifySchedule string @@ -440,7 +450,7 @@ func (s *Store) CreateRule(ctx context.Context, tenantID, userID string, in Rule RETURNING id::text `, tenantID, in.Name, nullString(in.Description), in.Source, in.Severity, in.Selector, in.Condition, in.ForSeconds, in.DependsOnTopology, - in.Enabled, nullUUID(userID), in.ChannelIDs, in.NotifySchedule, + in.Enabled == nil || *in.Enabled, nullUUID(userID), in.ChannelIDs, in.NotifySchedule, in.NotifyOnResolve, in.AutoCloseSeconds, in.MinIntervalSeconds, nullUUID(in.EscalationPolicyID)).Scan(&id) }) @@ -520,12 +530,57 @@ type jsonRaw string func (j jsonRaw) MarshalJSON() ([]byte, error) { return []byte(j), nil } +// planRuleEnabled рахує, яким стане прапорець «увімкнено» після правки +// правила й чи треба при цьому погасити його активні алерти. +// +// Винесене окремою функцією, бо це рішення, а не запит: перевіряти його +// на живій базі означало б стенд із правилом, алертом і драбиною на +// кожен із чотирьох випадків, а помилка в будь-якому з них не видна +// одразу — вона видна за тиждень, коли комусь дзвонять о третій ночі за +// алертом правила, вимкненого в понеділок. +// +// current — стан у базі ДО правки; want — те, що прийшло в запиті +// (nil = поля не було, стан не чіпаємо). +func planRuleEnabled(current bool, want *bool) (enabled, resolve bool) { + enabled = current + if want != nil { + enabled = *want + } + // Гасити треба рівно на ПЕРЕХОДІ «увімкнене → вимкнене». Не на + // кожному збереженні вимкненого правила: його алерти вже погашені + // тим переходом, який його вимкнув, і повторний прохід був би + // зайвим записом у event_outbox на кожну правку. + return enabled, current && !enabled +} + // UpdateRule замінює правило цілком. // // Цілком, а не полями: форма показує повний стан правила, і часткові // оновлення дали б спосіб отримати комбінацію, якої людина не бачила. func (s *Store) UpdateRule(ctx context.Context, tenantID, ruleID string, in RuleInput) error { return s.InTenantTx(ctx, tenantID, func(tx pgx.Tx) error { + // Стан ДО правки читається окремо й під замком. + // + // Окремо — бо після UPDATE його вже не відновити, а рішення про + // алерти приймається саме за переходом, а не за новим значенням. + // Під замком (FOR UPDATE) — бо між читанням і записом уміщається + // перемикач «Увімк.» зі списку правил: без замка два записи + // могли б лягти в порядку, у якому правило лишається вимкненим, + // а гасіння не спрацьовує в жодному з них. + var was bool + err := tx.QueryRow(ctx, ` + SELECT enabled FROM alr.rules + WHERE id = $1 AND tenant_id = $2 + FOR UPDATE + `, ruleID, tenantID).Scan(&was) + if isNoRows(err) { + return ErrNotFound + } + if err != nil { + return err + } + enabled, resolve := planRuleEnabled(was, in.Enabled) + ct, err := tx.Exec(ctx, ` UPDATE alr.rules SET name = $3, description = $4, source = $5::alr.rule_source, @@ -542,7 +597,7 @@ func (s *Store) UpdateRule(ctx context.Context, tenantID, ruleID string, in Rule WHERE id = $1 AND tenant_id = $2 `, ruleID, tenantID, in.Name, nullString(in.Description), in.Source, in.Severity, in.Selector, in.Condition, in.ForSeconds, - in.DependsOnTopology, in.Enabled, in.ChannelIDs, in.NotifySchedule, + in.DependsOnTopology, enabled, in.ChannelIDs, in.NotifySchedule, in.NotifyOnResolve, in.AutoCloseSeconds, in.MinIntervalSeconds, nullUUID(in.EscalationPolicyID)) if err != nil { @@ -551,6 +606,15 @@ func (s *Store) UpdateRule(ctx context.Context, tenantID, ruleID string, in Rule if ct.RowsAffected() == 0 { return ErrNotFound } + // Те саме, що робить SetRuleEnabled, і з тієї ж причини: + // вимкнене правило випадає з ActiveRules, тобто ResolveMissing + // за ним більше не біжить і закрити свої алерти воно вже не + // зможе. Без цього рядка алерти висіли б у firing вічно — а + // драбина ескалації справно будила б за ними людей, з повторами + // на тижні вперед. + if resolve { + return resolveRuleAlerts(ctx, tx, tenantID, ruleID, "правило вимкнено") + } return nil }) } diff --git a/server/internal/store/alerts_rule_update_db_test.go b/server/internal/store/alerts_rule_update_db_test.go new file mode 100644 index 0000000..5d3ec6e --- /dev/null +++ b/server/internal/store/alerts_rule_update_db_test.go @@ -0,0 +1,210 @@ +package store + +import ( + "context" + "os" + "strings" + "testing" + "time" +) + +// Правка правила ПРОТИ БАЗИ. +// +// planRuleEnabled поруч покриває рішення, і це головна половина. Друга +// половина рішенням не перевіряється взагалі: чи справді UpdateRule +// читає стан ДО запису, чи справді гасить алерти в тій самій +// транзакції й чи не вмикає правило, якого його не просили вмикати. Це +// властивість трьох запитів, а не функції. +// +// Мовчки пропускається без NETPULSE_TEST_DSN: `go test ./...` не має +// вимагати бази. Запускати треба на ОДНОРАЗОВІЙ базі — тест створює +// кабінет і видаляє його разом з усім вмістом: +// +// docker run --rm -d --name np-test -e POSTGRES_PASSWORD=x \ +// -e POSTGRES_DB=np timescale/timescaledb:2.17.2-pg16 +// NETPULSE_DSN=postgres://postgres:x@localhost/np go run ./cmd/netpulse-migrate +// NETPULSE_TEST_DSN=postgres://postgres:x@localhost/np \ +// go test ./internal/store/ -run UpdateRuleAgainstDB -v +func TestUpdateRuleAgainstDB(t *testing.T) { + dsn := os.Getenv("NETPULSE_TEST_DSN") + if dsn == "" { + t.Skip("NETPULSE_TEST_DSN не задано — перевірка проти бази пропускається") + } + ctx := context.Background() + + st, err := New(ctx, dsn) + if err != nil { + t.Fatalf("підключення: %v", err) + } + t.Cleanup(st.Close) + + slug := "upd-" + strings.ReplaceAll(time.Now().Format("150405.000"), ".", "") + var tenantID string + if err := st.pool.QueryRow(ctx, ` + INSERT INTO core.tenants (slug, name) VALUES ($1, $2) RETURNING id::text + `, slug, "Перевірка правки правил").Scan(&tenantID); err != nil { + t.Fatalf("кабінет: %v", err) + } + t.Cleanup(func() { + _, _ = st.pool.Exec(context.Background(), + `DELETE FROM core.tenants WHERE id = $1`, tenantID) + }) + + var deviceID string + if err := st.pool.QueryRow(ctx, ` + INSERT INTO inv.devices (tenant_id, name, address, kind) + VALUES ($1, $2, '10.78.0.1', 'switch') RETURNING id::text + `, tenantID, slug+"-sw").Scan(&deviceID); err != nil { + t.Fatalf("хост: %v", err) + } + + // Правило й алерт до нього — рівно те, що бачить людина на екрані + // перед тим, як відкрити форму й натиснути «Зберегти». + newRule := func(name string, enabled bool) string { + t.Helper() + var id string + if err := st.pool.QueryRow(ctx, ` + INSERT INTO alr.rules (tenant_id, name, source, severity, condition, enabled) + VALUES ($1, $2, 'icmp', 'high', + '{"metric":"loss_pct","op":">","value":20}'::jsonb, $3) + RETURNING id::text + `, tenantID, name, enabled).Scan(&id); err != nil { + t.Fatalf("правило %s: %v", name, err) + } + return id + } + newAlert := func(ruleID string) string { + t.Helper() + var id string + if err := st.pool.QueryRow(ctx, ` + INSERT INTO alr.alerts (tenant_id, rule_id, device_id, severity, state, + title, dedup_key) + VALUES ($1, $2, $3, 'high', 'firing', 'ядро не відповідає', $4) + RETURNING id::text + `, tenantID, ruleID, deviceID, ruleID+":dev:"+deviceID).Scan(&id); err != nil { + t.Fatalf("алерт: %v", err) + } + return id + } + state := func(alertID string) string { + t.Helper() + var s string + if err := st.pool.QueryRow(ctx, + `SELECT state::text FROM alr.alerts WHERE id = $1`, alertID).Scan(&s); err != nil { + t.Fatalf("стан алерту: %v", err) + } + return s + } + ruleEnabled := func(ruleID string) bool { + t.Helper() + var e bool + if err := st.pool.QueryRow(ctx, + `SELECT enabled FROM alr.rules WHERE id = $1`, ruleID).Scan(&e); err != nil { + t.Fatalf("стан правила: %v", err) + } + return e + } + + base := func(name string) RuleInput { + return RuleInput{ + Name: name, + Source: "icmp", + Severity: "high", + Selector: "{}", + Condition: `{"metric":"loss_pct","op":">","value":20}`, + ForSeconds: 60, + DependsOnTopology: true, + ChannelIDs: []string{}, + NotifyOnResolve: true, + } + } + off, on := false, true + + // --- ВАДА А: вимкнення правкою гасить алерти ---------------------- + // + // Вимкнене правило випадає з ActiveRules, тобто ResolveMissing за + // ним більше не біжить. Якщо алерт не погасити тут, він лишиться в + // firing назавжди — а драбина ескалації будитиме за ним людей + // повторами на тижні вперед. + r1 := newRule(slug+"-перше", true) + a1 := newAlert(r1) + + in := base(slug + "-перше") + in.Description = "правку опису самої по собі мало б бути видно лише в описі" + if err := st.UpdateRule(ctx, tenantID, r1, in); err != nil { + t.Fatalf("правка без зміни стану: %v", err) + } + if !ruleEnabled(r1) { + t.Fatal("правка без поля `enabled` вимкнула увімкнене правило") + } + if state(a1) != "firing" { + t.Fatalf("правка опису погасила алерт (стан %q) — проблема ж не зникла", state(a1)) + } + + in.Enabled = &off + if err := st.UpdateRule(ctx, tenantID, r1, in); err != nil { + t.Fatalf("вимкнення правкою: %v", err) + } + if ruleEnabled(r1) { + t.Fatal("правило не вимкнулось") + } + if got := state(a1); got != "resolved" { + t.Fatalf("алерт вимкненого правила лишився в %q: закрити його вже нікому, "+ + "а ескалація за ним працює далі", got) + } + var outbox int + if err := st.pool.QueryRow(ctx, ` + SELECT count(*)::int FROM core.event_outbox + WHERE tenant_id = $1 AND topic = 'alert.resolved' + AND payload->>'alert_id' = $2 + `, tenantID, a1).Scan(&outbox); err != nil { + t.Fatalf("черга подій: %v", err) + } + if outbox != 1 { + t.Fatalf("подій alert.resolved для алерту: %d, очікували 1 — "+ + "без неї екран показуватиме закритий алерт активним", outbox) + } + + // Повторне збереження вже вимкненого правила не має додавати ще + // одну подію: гасити нічого, а зайвий запис у чергу — це зайве + // сповіщення «відновлено» тим, хто підписаний на канал. + if err := st.UpdateRule(ctx, tenantID, r1, in); err != nil { + t.Fatalf("повторна правка вимкненого: %v", err) + } + if err := st.pool.QueryRow(ctx, ` + SELECT count(*)::int FROM core.event_outbox + WHERE tenant_id = $1 AND topic = 'alert.resolved' + AND payload->>'alert_id' = $2 + `, tenantID, a1).Scan(&outbox); err != nil { + t.Fatalf("черга подій: %v", err) + } + if outbox != 1 { + t.Fatalf("повторне збереження додало подій: %d", outbox) + } + + // --- ВАДА Б: відсутнє поле не вмикає правило ---------------------- + // + // Саме так виглядав кожен PUT із форми правил: поля `enabled` у + // тілі не було взагалі, а сервер читав його відсутність як згоду + // ввімкнути. + r2 := newRule(slug+"-друге", false) + + in2 := base(slug + "-друге") + in2.Description = "поправили опис вимкненого правила" + if err := st.UpdateRule(ctx, tenantID, r2, in2); err != nil { + t.Fatalf("правка вимкненого правила: %v", err) + } + if ruleEnabled(r2) { + t.Fatal("правка без поля `enabled` увімкнула вимкнене правило — " + + "моніторинг, який навмисно зупинили, тихо запрацював знову") + } + + // І другий бік тієї ж пари: явне `true` мусить вмикати. + in2.Enabled = &on + if err := st.UpdateRule(ctx, tenantID, r2, in2); err != nil { + t.Fatalf("увімкнення правкою: %v", err) + } + if !ruleEnabled(r2) { + t.Fatal("явне `enabled: true` не ввімкнуло правило") + } +} diff --git a/server/internal/store/alerts_rule_update_test.go b/server/internal/store/alerts_rule_update_test.go new file mode 100644 index 0000000..b0464c5 --- /dev/null +++ b/server/internal/store/alerts_rule_update_test.go @@ -0,0 +1,92 @@ +package store + +import "testing" + +// Рішення про прапорець «увімкнено» при правці правила. +// +// ЧОГО ЦЕЙ ФАЙЛ БОЇТЬСЯ. Обидві вади, які він закриває, тихі: система +// після них не падає й нічого не пише в журнал. Видно їх тільки з +// телефона о третій ночі — або, що гірше, не видно взагалі. +// +// 1. Правило вимкнули через PUT (форма надсилає правило цілком) — і +// його алерти лишились у firing НАЗАВЖДИ: вимкнене правило випадає +// з ActiveRules, тобто ResolveMissing за ним більше не біжить і +// погасити їх немає кому. Драбина ескалації при цьому працює далі — +// з повторами вона будить людей тижнями за проблемою, за якою вже +// ніхто не стежить. +// 2. Форма не надсилала `enabled` взагалі, а сервер читав відсутність +// поля як `true`. Тобто відкрити вимкнене правило, поправити в ньому +// будь-що й зберегти — означало мовчки його ввімкнути. +// +// Перевіряється чистою функцією, а не базою, з тієї ж причини, що й +// planRestore поруч: на живій базі кожен із чотирьох випадків — це +// окремий стенд із правилом, алертом і драбиною, а ціна помилки в +// будь-якому з них платиться не тут і не одразу. + +func TestPlanRuleEnabledResolvesOnDisable(t *testing.T) { + // Головний випадок: увімкнене правило вимикають правкою. Алерти + // треба гасити тут же — іншої нагоди в них не буде. + off := false + enabled, resolve := planRuleEnabled(true, &off) + if enabled { + t.Fatal("правило мало лишитись вимкненим") + } + if !resolve { + t.Fatal("алерти не гасяться: після вимкнення їх нікому закрити, " + + "і драбина ескалації будитиме людей за ними далі") + } +} + +func TestPlanRuleEnabledKeepsAlertsWhenStillDisabled(t *testing.T) { + // Правку вимкненого правила зберігають повторно. Гасити нема чого: + // його алерти закрив той перехід, який його вимкнув, а зайвий + // прохід — це ще один запис у event_outbox на кожне збереження. + off := false + enabled, resolve := planRuleEnabled(false, &off) + if enabled { + t.Fatal("правило мало лишитись вимкненим") + } + if resolve { + t.Fatal("гасити нічого: правило вже було вимкнене до правки") + } +} + +func TestPlanRuleEnabledKeepsAlertsWhenEnabling(t *testing.T) { + // Увімкнення — не привід чіпати алерти: рішення про них ухвалить + // найближчий тік движка, і саме він знає, чи проблема ще триває. + on := true + enabled, resolve := planRuleEnabled(false, &on) + if !enabled { + t.Fatal("правило мало ввімкнутись") + } + if resolve { + t.Fatal("увімкнення гасить алерти — цього не мало статись") + } +} + +func TestPlanRuleEnabledLeavesDisabledRuleAlone(t *testing.T) { + // ВАДА Б. Поля `enabled` у тілі немає — саме так виглядав кожен + // PUT із форми правил. Вимкнене правило мусить лишитись вимкненим: + // «поля не було» означає «не чіпайте», а не «вмикайте». + enabled, resolve := planRuleEnabled(false, nil) + if enabled { + t.Fatal("правка без поля `enabled` увімкнула вимкнене правило — " + + "людина дізнається про це зі сповіщення, а не з форми") + } + if resolve { + t.Fatal("гасити нічого: стан не змінився") + } +} + +func TestPlanRuleEnabledLeavesEnabledRuleAlone(t *testing.T) { + // Другий бік тієї ж пари, і без нього перший нічого не доводить: + // функція, яка завжди повертає false, пройшла б попередній тест. + enabled, resolve := planRuleEnabled(true, nil) + if !enabled { + t.Fatal("правка без поля `enabled` вимкнула увімкнене правило — " + + "це мовчазна зупинка моніторингу") + } + if resolve { + t.Fatal("гасити нічого: стан не змінився") + } +} diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 4bf7639..6176fda 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -949,6 +949,13 @@ export const api = { selector?: Record for_seconds: number depends_on_topology: boolean + /** + * Стан перемикача. Необов'язкове лише для СТВОРЕННЯ, де сервер + * бере `true`; на правці мовчання означає «не чіпати», і покластись + * на це замість явного значення — те саме, що не надіслати поле, + * яке форма насправді знає. + */ + enabled?: boolean channel_ids?: string[] notify_schedule?: Record | null notify_on_resolve?: boolean diff --git a/web/src/pages/ChannelsPage.tsx b/web/src/pages/ChannelsPage.tsx index 66260a3..dae4bc0 100644 --- a/web/src/pages/ChannelsPage.tsx +++ b/web/src/pages/ChannelsPage.tsx @@ -123,6 +123,25 @@ export function ChannelsPage() { — ), }, + { + // Той самий стовпчик, що «Тригерів» у переліку драбин, і + // з тієї ж причини: перед видаленням має бути видно, що + // на цьому каналі тримається чиєсь нічне чергування. + key: 'esc', + header: 'Драбини', + hideOnMobile: true, + cell: (c) => + c.escalations?.length ? ( + + {c.escalations.length} + + ) : ( + — + ), + }, { key: 'state', header: 'Стан', @@ -172,8 +191,9 @@ export function ChannelsPage() { Канал {c.name} буде видалено разом із його токеном. ), - detail: - 'Алерти, які йшли сюди, більше не доставлятимуться. Якщо це був єдиний канал, сповіщення припиняться взагалі.', + detail: c.escalations?.length + ? `Алерти, які йшли сюди, більше не доставлятимуться. На цей канал спираються драбини ескалації (${c.escalations.length}): ${c.escalations.join(', ')} — їхні сходинки лишаться без каналу й нікого не розбудять, поки в них не оберуть інший.` + : 'Алерти, які йшли сюди, більше не доставлятимуться. Якщо це був єдиний канал, сповіщення припиняться взагалі.', onConfirm: async () => { await api.deleteChannel(c.id) await reload() diff --git a/web/src/pages/EscalationsPage.tsx b/web/src/pages/EscalationsPage.tsx index 9b9cc9c..f63cbd2 100644 --- a/web/src/pages/EscalationsPage.tsx +++ b/web/src/pages/EscalationsPage.tsx @@ -56,7 +56,21 @@ export function EscalationsPage() { void reload() }, [reload]) - const channelName = (id: string) => channels.find((c) => c.id === id)?.name ?? '—' + // Канал, якого вже немає, показуємо словами, а не прочерком. + // + // Прочерк читається як «нічого не налаштовано», тобто як спокійний + // стан. А це протилежне: сходинка налаштована, виглядає робочою і не + // йде нікуди. Драбини, збережені до перевірки каналів, — єдине місце, + // де таке ще може лишитись. + const channelCell = (id: string) => { + const c = channels.find((x) => x.id === id) + if (c) return {c.name} + return ( + + канал видалено + + ) + } return ( <> @@ -117,7 +131,21 @@ export function EscalationsPage() {
+{s.after_min} хв {' → '} - {s.channel_ids.map(channelName).join(', ')} + {s.channel_ids.length === 0 ? ( + + жодного каналу + + ) : ( + s.channel_ids.map((id, k) => ( + + {k > 0 && ', '} + {channelCell(id)} + + )) + )}
))} @@ -225,9 +253,32 @@ function PolicyForm({ }) { const [name, setName] = useState(policy?.name ?? '') const [description, setDescription] = useState(policy?.description ?? '') - const [steps, setSteps] = useState( - policy?.steps?.length ? policy.steps : [{ after_min: 15, channel_ids: [] }], - ) + + // Ідентифікатори видалених каналів прибираємо ще при відкритті форми. + // + // Інакше вийшов би глухий кут: галочки для неіснуючого каналу в + // переліку немає, тобто прибрати його руками неможливо, а сервер + // драбину з ним уже не приймає — форма відмовлялась би зберігатись і + // не показувала б, через що. + // + // Тільки коли канали справді прочитались: порожній перелік буває й + // від невдалого запиту, і тоді «чистка» стерла б драбину цілком. + const [dropped] = useState(() => { + if (!policy?.steps?.length || channels.length === 0) return 0 + let n = 0 + for (const s of policy.steps) { + n += s.channel_ids.filter((id) => !channels.some((c) => c.id === id)).length + } + return n + }) + const [steps, setSteps] = useState(() => { + if (!policy?.steps?.length) return [{ after_min: 15, channel_ids: [] }] + if (channels.length === 0) return policy.steps + return policy.steps.map((s) => ({ + ...s, + channel_ids: s.channel_ids.filter((id) => channels.some((c) => c.id === id)), + })) + }) const [repeatAfter, setRepeatAfter] = useState(String(policy?.repeat_after_min ?? 0)) const [maxRepeats, setMaxRepeats] = useState(String(policy?.max_repeats ?? 0)) const [busy, setBusy] = useState(false) @@ -320,6 +371,13 @@ function PolicyForm({ сповіщенням і виглядала б як здвоєне повідомлення.

+ {dropped > 0 && ( +

+ У драбині лишались посилання на видалені канали ({dropped}) — їх прибрано з форми. + Перевірте сходинки нижче: та, що лишилась без каналу, нікого не розбудить. +

+ )} + {channels.length === 0 && (

Каналів ще немає. Заведіть їх на сторінці «Канали» — без жодного каналу сходинка diff --git a/web/src/pages/RulesPage.tsx b/web/src/pages/RulesPage.tsx index 2e7b6bd..113dee8 100644 --- a/web/src/pages/RulesPage.tsx +++ b/web/src/pages/RulesPage.tsx @@ -467,6 +467,13 @@ function RuleForm({ // найчастіший випадок, і вимагати вибору означало б змусити // людину відмічати всі групи по черзі. selector: groupIDs.length > 0 ? { group_ids: groupIDs } : {}, + // Прапорець «увімкнено» ця форма не показує — його перемикають + // у списку, — але надсилати його вона мусить: PUT замінює + // правило ЦІЛКОМ, і поле, про яке клієнт змовчав, сервер мусив + // би домислити. Раніше домислював `true`, і правка вимкненого + // правила мовчки його вмикала: моніторинг, який навмисно + // зупинили, знову починав будити людей. + enabled: rule?.enabled ?? true, for_seconds: Number(forSec) || 60, // Кореляція за топологією рахується з того, які хости зараз // лежать, — а подія не робить хост мертвим. Для подієвих правил diff --git a/web/src/test/escalationchannels.test.tsx b/web/src/test/escalationchannels.test.tsx new file mode 100644 index 0000000..8c37031 --- /dev/null +++ b/web/src/test/escalationchannels.test.tsx @@ -0,0 +1,133 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { EscalationsPage } from '../pages/EscalationsPage' +import { ChannelsPage } from '../pages/ChannelsPage' +import { session } from '../api/session' +import { fetchRouter, res, type Call } from './support' + +/** + * Канал, на який посилається сходинка драбини. + * + * ЧОГО ЦЕЙ ФАЙЛ БОЇТЬСЯ. Драбина зберігалась із будь-яким + * ідентифікатором каналу, а видалення каналу не чіпало сходинок, що на + * нього посилались. Ззовні це виглядало як робоче нічне чергування: + * сходинка стоїть, хвилини стоять, у стовпчику каналів — прочерк, який + * читається як «не заповнено» й нікого не турбує. Насправді ця + * сходинка не йде нікуди й списується мовчки. + * + * У формі це давало ще й глухий кут: галочки для видаленого каналу в + * переліку немає, тобто прибрати його руками неможливо, а сервер + * (після виправлення) такої драбини вже не приймає. + * + * Форма підставних відповідей звірена з + * `server/internal/httpapi/alerts.go` (`handleListEscalationPolicies`, + * `handleListChannels`) і зі `store.EscalationPolicy` / `store.Channel`. + */ + +const duty = { + id: 'ch-duty', + kind: 'telegram', + name: 'Черговий', + config: {}, + min_severity: 'warning', + enabled: true, + has_secret: true, +} + +/** Драбина, друга сходинка якої посилається на вже видалений канал. */ +const policy = { + id: 'p-1', + name: 'Нічне чергування', + description: '', + steps: [ + { after_min: 15, channel_ids: ['ch-duty'] }, + { after_min: 45, channel_ids: ['ch-gone'] }, + ], + repeat_after_min: 0, + max_repeats: 0, + rule_count: 2, +} + +beforeEach(() => { + vi.stubGlobal('WebSocket', class {}) + session.set('tok', { + userID: 'u-me', + username: 'me', + tenantID: 't-1', + permissions: ['alerts:read', 'alerts:write'], + }) +}) + +describe('сходинка, що посилається на видалений канал', () => { + it('у переліку її видно словами, а не прочерком', async () => { + fetchRouter({ + 'GET /api/v1/escalation-policies': { policies: [policy] }, + 'GET /api/v1/channels': { channels: [duty] }, + }) + + render() + + await screen.findByText('Черговий') + // Прочерк читався б як «нічого не налаштовано», тобто як спокійний + // стан. Тут стан протилежний. + expect(await screen.findByText('канал видалено')).toBeTruthy() + }) + + it('форма прибирає привида, говорить про це і не шле його на сервер', async () => { + const srv = fetchRouter({ + 'GET /api/v1/escalation-policies': { policies: [policy] }, + 'GET /api/v1/channels': { channels: [duty] }, + 'PUT /api/v1/escalation-policies/p-1': () => res(200, { id: 'p-1' }), + }) + + render() + fireEvent.click(await screen.findByRole('button', { name: 'Змінити' })) + await screen.findByText('Драбина: Нічне чергування') + + expect(screen.getByText(/лишались посилання на видалені канали/)).toBeTruthy() + + // Друга сходинка лишилась без каналу — і форма має сказати про це + // ДО збереження, а не віддати серверу відмову без пояснення. + expect(screen.getByText(/сходинка 2 не має жодного каналу/)).toBeTruthy() + + // Дали сходинці канал — тепер зберігається. + const boxes = screen.getAllByRole('checkbox') + fireEvent.click(boxes[boxes.length - 1]) + fireEvent.click(screen.getByRole('button', { name: 'Зберегти' })) + + let put: Call | undefined + await waitFor(() => { + put = srv.calls.find((c) => c.method === 'PUT') + expect(put).toBeTruthy() + }) + const body = put!.body as { steps: { channel_ids: string[] }[] } + expect(body.steps.map((s) => s.channel_ids)).toEqual([['ch-duty'], ['ch-duty']]) + }) +}) + +describe('видалення каналу, на який спирається драбина', () => { + it('називає драбини, які через це замовкнуть', async () => { + fetchRouter({ + 'GET /api/v1/channels': { + channels: [{ ...duty, escalations: ['Нічне чергування', 'Вихідні'] }], + }, + }) + + render() + fireEvent.click(await screen.findByRole('button', { name: 'Видалити' })) + + // Те саме, що вже показує видалення драбини: скільки чужих + // налаштувань зараз перестане працювати, і яких саме. + expect(await screen.findByText(/Нічне чергування, Вихідні/)).toBeTruthy() + }) + + it('без драбин попередження про них не вигадується', async () => { + fetchRouter({ 'GET /api/v1/channels': { channels: [duty] } }) + + render() + fireEvent.click(await screen.findByRole('button', { name: 'Видалити' })) + + await screen.findByText(/більше не доставлятимуться/) + expect(screen.queryByText(/драбини ескалації/)).toBeNull() + }) +}) diff --git a/web/src/test/rules.test.tsx b/web/src/test/rules.test.tsx new file mode 100644 index 0000000..76dd77d --- /dev/null +++ b/web/src/test/rules.test.tsx @@ -0,0 +1,102 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { RulesPage } from '../pages/RulesPage' +import { session } from '../api/session' +import { fetchRouter, res, type Call } from './support' + +/** + * Збереження тригера з форми. + * + * ЧОГО ЦЕЙ ФАЙЛ БОЇТЬСЯ. Форма показує тригер цілком і надсилає його + * цілком (`PUT /api/v1/alert-rules/{id}` замінює правило), але + * перемикача «увімкнено» в ній немає — його місце в списку. Через це + * поле `enabled` у тілі запиту не було ВЗАГАЛІ, а сервер читав його + * відсутність як згоду ввімкнути. + * + * Наслідок тихий і найгіршого можливого сорту: людина відкриває + * НАВМИСНО вимкнений тригер, правит у ньому будь-що, натискає + * «Зберегти» — і моніторинг, який зупинили свідомо, знову починає + * будити людей. У формі при цьому не змінюється нічого: вона про цей + * прапорець не знає й не показує його ні до, ні після. + * + * Перевірка йде парою — вимкнений і увімкнений тригер. Один бік + * окремо не доводить нічого: `enabled: false`, вписане намертво, + * пройшло б перший тест і зупинило б моніторинг усім. + */ + +const disabledRule = { + id: 'r-off', + name: 'Втрати пакетів на магістралі', + description: '', + source: 'icmp', + severity: 'high', + selector: {}, + condition: { metric: 'loss_pct', op: '>', value: 20 }, + for_seconds: 180, + depends_on_topology: true, + enabled: false, + active_alerts: 0, + channel_ids: [], + notify_on_resolve: true, +} + +const enabledRule = { ...disabledRule, id: 'r-on', name: 'Ядро не відповідає', enabled: true } + +/** Довідники форми: порожні — це робочий стан, а не збій. */ +const refs = { + 'GET /api/v1/device-groups': { groups: [] }, + 'GET /api/v1/channels': { channels: [] }, + 'GET /api/v1/escalation-policies': { policies: [] }, + 'GET /api/v1/traps/meta': { names: [] }, +} + +beforeEach(() => { + // liveEvents підписується на WebSocket одразу: без заглушки jsdom + // пішов би у справжню мережу, і тест залежав би від того, що зараз + // слухає localhost. + vi.stubGlobal('WebSocket', class {}) + session.set('tok', { + userID: 'u-me', + username: 'me', + tenantID: 't-1', + permissions: ['alerts:read', 'alerts:write'], + }) +}) + +/** Відкрити тригер на правку й одразу зберегти, нічого не змінивши. */ +async function saveWithoutChanges(rule: typeof disabledRule) { + const srv = fetchRouter({ + 'GET /api/v1/alert-rules': { rules: [rule] }, + ...refs, + [`PUT /api/v1/alert-rules/${rule.id}`]: () => res(200, { id: rule.id }), + }) + + render() + fireEvent.click(await screen.findByRole('button', { name: 'Змінити' })) + await screen.findByText(`Тригер: ${rule.name}`) + fireEvent.click(screen.getByRole('button', { name: 'Зберегти' })) + + let put: Call | undefined + await waitFor(() => { + put = srv.calls.find((c) => c.method === 'PUT') + expect(put).toBeDefined() + }) + return put!.body as Record +} + +describe('форма тригера', () => { + it('не вмикає вимкнений тригер збереженням', async () => { + const body = await saveWithoutChanges(disabledRule) + expect( + body.enabled, + 'форма змовчала про `enabled` — сервер домислить його сам, і вимкнений тригер увімкнеться', + ).toBe(false) + }) + + it('лишає увімкнений тригер увімкненим', async () => { + const body = await saveWithoutChanges(enabledRule) + expect(body.enabled, 'збереження вимкнуло робочий тригер — це тиха зупинка моніторингу').toBe( + true, + ) + }) +}) diff --git a/web/src/types.ts b/web/src/types.ts index 9dc7625..da65e2a 100644 --- a/web/src/types.ts +++ b/web/src/types.ts @@ -838,6 +838,13 @@ export interface Channel { enabled: boolean /** Чи збережено секрет. Сам секрет назад не віддається ніколи. */ has_secret: boolean + /** + * Назви драбин ескалації, сходинки яких шлють у цей канал. + * + * Потрібне лише перед видаленням: канал, на який спирається нічне + * чергування, не має зникати мовчки. Приходить лише в переліку. + */ + escalations?: string[] } /**