mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-23 20:25:56 +00:00
Keep alert settings a config save leaves out
The Thresholds page builds its PUT /api/alerts/config body from the settings it edits. It has no control for flapping detection, alert TTL cleanup or custom rules, so it never sends flappingEnabled, maxAlertAgeDays, maxAcknowledgedAgeDays, autoAcknowledgeAfterHours or customRules. The handler decoded the body into a zero AlertConfig, so every thresholds save turned flapping detection and TTL cleanup off, reset the flapping tuning, dropped custom rules, and persisted that. The page's flapping, aggregation and ioNormalization objects are not AlertConfig fields and never enabled anything. The handler now hands the body to Manager.ApplyConfigUpdate, which reads the stored config, merges and applies it under the manager lock, so two concurrent partial saves cannot revert each other. The body is decoded on its own, so the keys it carries mean exactly what encoding/json makes of them; every top-level field it left out is copied from a JSON round trip of the stored config, sharing no maps with it. ApplyConfigUpdate returns a cloned snapshot, which the handler persists, because SaveAlertConfig normalizes the config it is handed in place and used to write into the live config's maps. Saves run one at a time so alerts.json ends with the last applied config. A body that is not a JSON object, null included, is a 400. Stored configs are not rewritten, so a value an earlier save zeroed stays as it is. On a private stack, a node CPU change saved from the Thresholds page zeroed all four fields in memory and in alerts.json on main; with this change the same save kept them and stored the new threshold.
This commit is contained in:
+3
-1
@@ -891,7 +891,9 @@ For an existing incident whose notifications should be paused without clearing
|
||||
it, use the snooze and unsnooze endpoints below.
|
||||
|
||||
- `GET /api/alerts/config`
|
||||
- `PUT /api/alerts/config`
|
||||
- `PUT /api/alerts/config` — replaces only the top-level keys the body
|
||||
carries; a key left out keeps its stored value, and the merged config is
|
||||
normalized as before. The body must be a JSON object.
|
||||
- `GET /api/alerts/deadman/config` — returns only whether an external watchdog
|
||||
is configured; `pingUrl` is `***REDACTED***` when present and never returns
|
||||
the credential-bearing URL
|
||||
|
||||
@@ -3678,6 +3678,14 @@ Agent` secondary handoff against the live setup wizard instead of relying
|
||||
|
||||
## Current State
|
||||
|
||||
### Alert settings saves do not touch agent lifecycle
|
||||
|
||||
`PUT /api/alerts/config` in `internal/api/alerting/alerts.go` now keeps stored
|
||||
values for the top-level keys a client leaves out. Agent alert defaults
|
||||
(`agentDefaults`) follow the same rule as every other key; enrollment,
|
||||
reporting leases, commands and agent removal are unchanged.
|
||||
`internal/api/alerting/alerts_test.go` pins the handler.
|
||||
|
||||
### VM guest execution admission (backup precaution)
|
||||
|
||||
`AgentRegisterPayload.guest_exec_guard_version` is optional; version 1 on a
|
||||
|
||||
@@ -1093,6 +1093,43 @@ history and affirmative recovery. This is source-fixture lifecycle proof, not
|
||||
an installed registry/destination result or a diagnosis of a reporter's daily
|
||||
resolve/reopen cycle.
|
||||
|
||||
### A settings save keeps the alert settings it did not send
|
||||
|
||||
`PUT /api/alerts/config` applies the body to the stored configuration through
|
||||
`Manager.ApplyConfigUpdate` and `ApplyAlertConfigUpdate` in
|
||||
`internal/alerts/config/update.go`: a top-level key the client sent replaces
|
||||
that setting whole, a key it left out keeps the stored value, and the result
|
||||
is normalized as any update is. The thresholds page has no control for
|
||||
flapping detection (`flappingEnabled` and its window, threshold and cooldown),
|
||||
alert TTL cleanup (`maxAlertAgeDays`, `maxAcknowledgedAgeDays`,
|
||||
`autoAcknowledgeAfterHours`) or `customRules`, so it never sends them. The
|
||||
handler used to decode the body into a zero `AlertConfig`, so every thresholds
|
||||
save turned flapping detection and TTL cleanup off, reset the flapping tuning
|
||||
to its defaults, dropped custom rules, and persisted that. The page's
|
||||
`flapping`, `aggregation` and `ioNormalization` payload objects are not
|
||||
`AlertConfig` fields and never enabled anything. A stored config keeps what it
|
||||
holds, including values an earlier save zeroed; nothing rewrites them.
|
||||
|
||||
The body is decoded on its own, so sent keys mean exactly what the decoder
|
||||
makes of them, duplicates and case-folded keys included. Unsent fields are
|
||||
copied from a JSON round trip of the stored config, so the result shares no
|
||||
maps or slices with it. `ApplyConfigUpdate` reads, merges and applies under
|
||||
the manager lock, so two concurrent partial saves cannot revert each other in
|
||||
memory, and returns a JSON-cloned snapshot so that persistence, which
|
||||
normalizes the config it is handed in place, never writes into the live
|
||||
config's maps.
|
||||
|
||||
`internal/alerts/config/update_test.go` covers unsent, explicit-off,
|
||||
case-folded, duplicate-key, malformed and full-body updates, a sent object
|
||||
replacing its stored value whole (a sent `truenasDiskDefaults: {}` does not
|
||||
inherit the stored temperature) and the no-sharing rule.
|
||||
`TestUpdateConfigKeepsSettingsAThresholdsSaveDidNotSend` and
|
||||
`TestApplyConfigUpdateKeepsConcurrentPartialUpdates` in
|
||||
`config_validation_test.go` run saves through the manager, and
|
||||
`TestUpdateAlertConfig_KeepsStoredValuesForUnsentKeys` in
|
||||
`internal/api/alerting/alerts_test.go` drives the handler against a real
|
||||
manager.
|
||||
|
||||
### Confirmed empty storage is recovery evidence
|
||||
|
||||
Static storage capacity evaluation must admit a zero usage observation when
|
||||
|
||||
@@ -10291,6 +10291,22 @@ boundaries. Alert evaluation after that save remains alerts-owned and must use
|
||||
the persisted trigger/recovery pair plus a valid derived critical threshold;
|
||||
the API must not invent a TrueNAS-specific threshold sidecar.
|
||||
|
||||
`PUT /api/alerts/config` replaces only the top-level keys the body carries
|
||||
and keeps the stored value of every key it leaves out, so a client that does
|
||||
not know a setting cannot turn it off by leaving it out: the thresholds page
|
||||
has no flapping-detection or alert-TTL controls, and an older browser bundle
|
||||
or API script predates later fields. A sent key replaces its stored value
|
||||
whole, so `"overrides": {}` removes every override rather than merging, and
|
||||
the merged config then goes through the same normalization as any update (for
|
||||
example, activation state is never cleared). The read, merge and apply run
|
||||
under the alert manager's lock, so two concurrent partial saves cannot revert
|
||||
each other in memory; the handler runs one save at a time and persists a
|
||||
snapshot of the applied config, so successful saves through this endpoint
|
||||
reach `alerts.json` in the order they were applied. A body that is not a JSON object, `null` included, is a 400 and changes
|
||||
nothing. `TestUpdateAlertConfig_KeepsStoredValuesForUnsentKeys`
|
||||
and `TestUpdateAlertConfig_RejectsNonObjectBody` in
|
||||
`internal/api/alerting/alerts_test.go` prove the transport.
|
||||
|
||||
### Alert intent and UDP availability transport
|
||||
|
||||
`GET /api/alerts/intent-policies` and
|
||||
|
||||
@@ -3006,6 +3006,15 @@ recovery evidence. Unremembered overflow deliveries remain auditable with
|
||||
`reason=capacity`; cache expiry, overflow, or restart is not a successful
|
||||
storage operation or a reason to delete historical events.
|
||||
|
||||
### Alert settings saves keep unsent keys without a new storage path
|
||||
|
||||
`PUT /api/alerts/config` now applies only the top-level keys the body carries
|
||||
and keeps stored values for the rest, so a thresholds save no longer turns off
|
||||
alert TTL cleanup. It still writes `alerts.json` through the existing
|
||||
`SaveAlertConfig` path, opens no storage or recovery route, and leaves backup
|
||||
and snapshot alert settings under the same replace-what-was-sent rule as every
|
||||
other key. `internal/api/alerting/alerts_test.go` pins the handler.
|
||||
|
||||
### Container diagnostics shares private bootstrap transport (1 October 2026)
|
||||
|
||||
Recovery-adjacent diagnostics now reuse the canonical complete installer
|
||||
|
||||
@@ -891,7 +891,9 @@ For an existing incident whose notifications should be paused without clearing
|
||||
it, use the snooze and unsnooze endpoints below.
|
||||
|
||||
- `GET /api/alerts/config`
|
||||
- `PUT /api/alerts/config`
|
||||
- `PUT /api/alerts/config` — replaces only the top-level keys the body
|
||||
carries; a key left out keeps its stored value, and the merged config is
|
||||
normalized as before. The body must be a JSON object.
|
||||
- `GET /api/alerts/deadman/config` — returns only whether an external watchdog
|
||||
is configured; `pingUrl` is `***REDACTED***` when present and never returns
|
||||
the credential-bearing URL
|
||||
|
||||
@@ -175,6 +175,17 @@ describe('docsLinks', () => {
|
||||
expect(handler).toContain('agentID != "" && len(*compatibility.ObservationLocationIDs) == 1');
|
||||
});
|
||||
|
||||
it('documents alert configuration saves as replacing only the keys they send', () => {
|
||||
const api = readFileSync(path.join(repoRoot, 'docs', 'API.md'), 'utf8');
|
||||
expect(api).toContain('`PUT /api/alerts/config` — replaces only the top-level keys the body');
|
||||
expect(api).toContain('a key left out keeps its stored value');
|
||||
const handler = readFileSync(
|
||||
path.join(repoRoot, 'internal', 'api', 'alerting', 'alerts.go'),
|
||||
'utf8',
|
||||
);
|
||||
expect(handler).toContain('GetAlertManager().ApplyConfigUpdate(body)');
|
||||
});
|
||||
|
||||
it('separates server removal from persistent-data erasure', () => {
|
||||
const installation = readFileSync(path.join(repoRoot, 'docs', 'INSTALL.md'), 'utf8');
|
||||
const removal = installation.split('## 🗑️ Uninstall')[1];
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"reflect"
|
||||
"strings"
|
||||
)
|
||||
|
||||
var errAlertConfigUpdateNotObject = errors.New("alert configuration update must be a JSON object")
|
||||
|
||||
// ApplyAlertConfigUpdate decodes a client's alert configuration update onto
|
||||
// the stored configuration. A top-level key the client sent replaces that
|
||||
// setting whole; a key it left out keeps the stored value.
|
||||
//
|
||||
// Clients only send what they know about. The settings page has no control
|
||||
// for flapping detection, alert TTL cleanup or custom rules, and an older
|
||||
// browser bundle or API script predates any field added since. Decoding the
|
||||
// body into a zero AlertConfig used to turn every one of those settings off
|
||||
// on each save.
|
||||
//
|
||||
// The body is decoded on its own, so the keys it carries mean exactly what
|
||||
// encoding/json makes of them. Each setting it left out is copied from a JSON
|
||||
// round trip of stored, so the result shares no maps or slices with it.
|
||||
func ApplyAlertConfigUpdate(stored AlertConfig, update []byte) (AlertConfig, error) {
|
||||
var sent map[string]json.RawMessage
|
||||
if err := json.Unmarshal(update, &sent); err != nil {
|
||||
return AlertConfig{}, err
|
||||
}
|
||||
if sent == nil {
|
||||
return AlertConfig{}, errAlertConfigUpdateNotObject
|
||||
}
|
||||
var result AlertConfig
|
||||
if err := json.Unmarshal(update, &result); err != nil {
|
||||
return AlertConfig{}, err
|
||||
}
|
||||
|
||||
kept, err := CloneAlertConfig(stored)
|
||||
if err != nil {
|
||||
return AlertConfig{}, err
|
||||
}
|
||||
|
||||
resultValue := reflect.ValueOf(&result).Elem()
|
||||
keptValue := reflect.ValueOf(kept)
|
||||
for i := 0; i < resultValue.NumField(); i++ {
|
||||
if !alertConfigKeySent(sent, alertConfigJSONKey(resultValue.Type().Field(i))) {
|
||||
resultValue.Field(i).Set(keptValue.Field(i))
|
||||
}
|
||||
}
|
||||
return result, nil
|
||||
}
|
||||
|
||||
// CloneAlertConfig returns a copy of config that shares no maps, slices or
|
||||
// pointers with it, by the same JSON round trip config takes through
|
||||
// alerts.json.
|
||||
func CloneAlertConfig(config AlertConfig) (AlertConfig, error) {
|
||||
data, err := json.Marshal(config)
|
||||
if err != nil {
|
||||
return AlertConfig{}, fmt.Errorf("encode alert config: %w", err)
|
||||
}
|
||||
var clone AlertConfig
|
||||
if err := json.Unmarshal(data, &clone); err != nil {
|
||||
return AlertConfig{}, fmt.Errorf("decode alert config: %w", err)
|
||||
}
|
||||
return clone, nil
|
||||
}
|
||||
|
||||
// alertConfigJSONKey returns the key encoding/json reads a field from.
|
||||
func alertConfigJSONKey(field reflect.StructField) string {
|
||||
name, _, _ := strings.Cut(field.Tag.Get("json"), ",")
|
||||
if name == "" {
|
||||
return field.Name
|
||||
}
|
||||
return name
|
||||
}
|
||||
|
||||
// alertConfigKeySent matches keys the way encoding/json matches them to
|
||||
// fields: exactly, or else case-insensitively.
|
||||
func alertConfigKeySent(sent map[string]json.RawMessage, key string) bool {
|
||||
if _, ok := sent[key]; ok {
|
||||
return true
|
||||
}
|
||||
for sentKey := range sent {
|
||||
if strings.EqualFold(sentKey, key) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
@@ -0,0 +1,257 @@
|
||||
package config
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func storedAlertConfigForUpdate() AlertConfig {
|
||||
activated := time.Date(2026, 10, 1, 9, 30, 0, 0, time.UTC)
|
||||
note := "keep"
|
||||
return AlertConfig{
|
||||
IdentitySchemaVersion: 2,
|
||||
Enabled: true,
|
||||
ActivationState: ActivationActive,
|
||||
ActivationTime: &activated,
|
||||
ObservationWindowHours: 24,
|
||||
GuestDefaults: ThresholdConfig{CPU: &HysteresisThreshold{Trigger: 80, Clear: 75}},
|
||||
StorageDefault: HysteresisThreshold{Trigger: 85, Clear: 80},
|
||||
DiskTempByType: map[string]HysteresisThreshold{"nvme": {Trigger: 70, Clear: 65}},
|
||||
IgnoredGuestPrefixes: []string{"test-"},
|
||||
Overrides: map[string]ThresholdConfig{"node:pve1": {Disabled: true, Note: ¬e}},
|
||||
CustomRules: []CustomAlertRule{{ID: "rule-1", Name: "Busy web", Enabled: true}},
|
||||
Schedule: ScheduleConfig{Cooldown: 5, MaxAlertsHour: 10, InitialNotify: "all"},
|
||||
MinimumDelta: 3,
|
||||
SuppressionWindow: 10,
|
||||
HysteresisMargin: 4,
|
||||
TimeThresholds: map[string]int{"guest": 5},
|
||||
MaxAlertAgeDays: 7,
|
||||
MaxAcknowledgedAgeDays: 1,
|
||||
AutoAcknowledgeAfterHours: 24,
|
||||
FlappingEnabled: true,
|
||||
FlappingWindowSeconds: 600,
|
||||
FlappingThreshold: 4,
|
||||
FlappingCooldownMinutes: 20,
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplyAlertConfigUpdateKeepsSettingsTheClientDidNotSend(t *testing.T) {
|
||||
stored := storedAlertConfigForUpdate()
|
||||
|
||||
// The settings page sends thresholds and schedule but has no control for
|
||||
// flapping detection, alert TTL cleanup or custom rules.
|
||||
got, err := ApplyAlertConfigUpdate(stored, []byte(`{
|
||||
"enabled": true,
|
||||
"guestDefaults": {"cpu": {"trigger": 90, "clear": 85}},
|
||||
"overrides": {},
|
||||
"flapping": {"enabled": true, "threshold": 5}
|
||||
}`))
|
||||
if err != nil {
|
||||
t.Fatalf("ApplyAlertConfigUpdate: %v", err)
|
||||
}
|
||||
|
||||
if got.GuestDefaults.CPU == nil || got.GuestDefaults.CPU.Trigger != 90 {
|
||||
t.Fatalf("guest CPU default = %+v, want the sent trigger 90", got.GuestDefaults.CPU)
|
||||
}
|
||||
if len(got.Overrides) != 0 {
|
||||
t.Fatalf("overrides = %v, want the sent empty map to replace the stored overrides", got.Overrides)
|
||||
}
|
||||
if !got.FlappingEnabled || got.FlappingWindowSeconds != 600 || got.FlappingThreshold != 4 || got.FlappingCooldownMinutes != 20 {
|
||||
t.Fatalf("flapping = %v/%d/%d/%d, want the stored true/600/4/20",
|
||||
got.FlappingEnabled, got.FlappingWindowSeconds, got.FlappingThreshold, got.FlappingCooldownMinutes)
|
||||
}
|
||||
if got.MaxAlertAgeDays != 7 || got.MaxAcknowledgedAgeDays != 1 || got.AutoAcknowledgeAfterHours != 24 {
|
||||
t.Fatalf("TTL cleanup = %d/%d/%d, want the stored 7/1/24",
|
||||
got.MaxAlertAgeDays, got.MaxAcknowledgedAgeDays, got.AutoAcknowledgeAfterHours)
|
||||
}
|
||||
if !reflect.DeepEqual(got.CustomRules, stored.CustomRules) {
|
||||
t.Fatalf("custom rules = %+v, want the stored rules", got.CustomRules)
|
||||
}
|
||||
if got.MinimumDelta != 3 || got.SuppressionWindow != 10 || got.HysteresisMargin != 4 {
|
||||
t.Fatalf("general settings = %v/%d/%v, want the stored 3/10/4",
|
||||
got.MinimumDelta, got.SuppressionWindow, got.HysteresisMargin)
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplyAlertConfigUpdateEmptyObjectChangesNothing(t *testing.T) {
|
||||
stored := storedAlertConfigForUpdate()
|
||||
|
||||
got, err := ApplyAlertConfigUpdate(stored, []byte(`{}`))
|
||||
if err != nil {
|
||||
t.Fatalf("ApplyAlertConfigUpdate({}): %v", err)
|
||||
}
|
||||
if !reflect.DeepEqual(got, stored) {
|
||||
t.Fatalf("ApplyAlertConfigUpdate({}) changed the config:\n got %+v\nwant %+v", got, stored)
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplyAlertConfigUpdateHonorsExplicitOffAndClear(t *testing.T) {
|
||||
stored := storedAlertConfigForUpdate()
|
||||
|
||||
got, err := ApplyAlertConfigUpdate(stored, []byte(`{
|
||||
"flappingEnabled": false,
|
||||
"maxAlertAgeDays": 0,
|
||||
"maxAcknowledgedAgeDays": 0,
|
||||
"autoAcknowledgeAfterHours": 0,
|
||||
"customRules": null,
|
||||
"ignoredGuestPrefixes": []
|
||||
}`))
|
||||
if err != nil {
|
||||
t.Fatalf("ApplyAlertConfigUpdate: %v", err)
|
||||
}
|
||||
|
||||
if got.FlappingEnabled || got.MaxAlertAgeDays != 0 || got.MaxAcknowledgedAgeDays != 0 || got.AutoAcknowledgeAfterHours != 0 {
|
||||
t.Fatalf("explicit off was not applied: flapping=%v ttl=%d/%d/%d",
|
||||
got.FlappingEnabled, got.MaxAlertAgeDays, got.MaxAcknowledgedAgeDays, got.AutoAcknowledgeAfterHours)
|
||||
}
|
||||
if got.CustomRules != nil {
|
||||
t.Fatalf("custom rules = %+v, want null to clear them", got.CustomRules)
|
||||
}
|
||||
if len(got.IgnoredGuestPrefixes) != 0 {
|
||||
t.Fatalf("ignored guest prefixes = %v, want the sent empty list", got.IgnoredGuestPrefixes)
|
||||
}
|
||||
if !got.Enabled || got.ActivationState != ActivationActive {
|
||||
t.Fatalf("unsent enabled/activation = %v/%q, want the stored true/active", got.Enabled, got.ActivationState)
|
||||
}
|
||||
}
|
||||
|
||||
// The merge is top-level only: a sent object replaces the stored one whole,
|
||||
// so a sent truenasDiskDefaults without a temperature must not inherit the
|
||||
// stored temperature.
|
||||
func TestApplyAlertConfigUpdateReplacesSentObjectsWhole(t *testing.T) {
|
||||
stored := storedAlertConfigForUpdate()
|
||||
stored.TrueNASDiskDefaults = ThresholdConfig{Temperature: &HysteresisThreshold{Trigger: 62, Clear: 57}}
|
||||
|
||||
got, err := ApplyAlertConfigUpdate(stored, []byte(`{"truenasDiskDefaults": {}}`))
|
||||
if err != nil {
|
||||
t.Fatalf("ApplyAlertConfigUpdate: %v", err)
|
||||
}
|
||||
if got.TrueNASDiskDefaults.Temperature != nil {
|
||||
t.Fatalf("TrueNAS disk temperature = %+v, want the sent empty object to clear it", got.TrueNASDiskDefaults.Temperature)
|
||||
}
|
||||
}
|
||||
|
||||
// encoding/json matches keys to fields case-insensitively, so a key in another
|
||||
// casing still names the field and must not leave the stored value in place.
|
||||
func TestApplyAlertConfigUpdateMatchesKeysLikeTheDecoder(t *testing.T) {
|
||||
stored := storedAlertConfigForUpdate()
|
||||
|
||||
got, err := ApplyAlertConfigUpdate(stored, []byte(`{"FlappingEnabled": false}`))
|
||||
if err != nil {
|
||||
t.Fatalf("ApplyAlertConfigUpdate: %v", err)
|
||||
}
|
||||
if got.FlappingEnabled {
|
||||
t.Fatal("FlappingEnabled sent as false stayed true")
|
||||
}
|
||||
}
|
||||
|
||||
// Sent keys mean what decoding the body alone makes of them, duplicates and
|
||||
// all.
|
||||
func TestApplyAlertConfigUpdateDecodesSentKeysLikeTheBodyAlone(t *testing.T) {
|
||||
stored := storedAlertConfigForUpdate()
|
||||
|
||||
for _, test := range []struct {
|
||||
body string
|
||||
field func(AlertConfig) any
|
||||
}{
|
||||
{`{"enabled": false, "enabled": null}`, func(c AlertConfig) any { return c.Enabled }},
|
||||
{`{"flappingEnabled": false, "FlappingEnabled": true}`, func(c AlertConfig) any { return c.FlappingEnabled }},
|
||||
{
|
||||
`{"guestDefaults": {"cpu": {"trigger": 90, "clear": 85}}, "guestDefaults": {"memory": {"trigger": 70, "clear": 65}}}`,
|
||||
func(c AlertConfig) any { return c.GuestDefaults },
|
||||
},
|
||||
} {
|
||||
var alone AlertConfig
|
||||
if err := json.Unmarshal([]byte(test.body), &alone); err != nil {
|
||||
t.Fatalf("decode %s alone: %v", test.body, err)
|
||||
}
|
||||
got, err := ApplyAlertConfigUpdate(stored, []byte(test.body))
|
||||
if err != nil {
|
||||
t.Fatalf("ApplyAlertConfigUpdate(%s): %v", test.body, err)
|
||||
}
|
||||
if !reflect.DeepEqual(test.field(got), test.field(alone)) {
|
||||
t.Fatalf("ApplyAlertConfigUpdate(%s) = %+v, decoding the body alone gives %+v",
|
||||
test.body, test.field(got), test.field(alone))
|
||||
}
|
||||
}
|
||||
|
||||
if _, err := ApplyAlertConfigUpdate(stored, []byte(`{"enabled": "yes", "enabled": true}`)); err == nil {
|
||||
t.Fatal("a type error in a duplicated key succeeded, want the decoder's error")
|
||||
}
|
||||
}
|
||||
|
||||
// The merge copies unsent settings field by field under their JSON keys, so
|
||||
// an embedded, unexported or untagged field would be skipped or matched
|
||||
// under the wrong key.
|
||||
func TestAlertConfigFieldsHaveExplicitJSONKeys(t *testing.T) {
|
||||
fields := reflect.TypeOf(AlertConfig{})
|
||||
keys := make([]string, fields.NumField())
|
||||
for i := 0; i < fields.NumField(); i++ {
|
||||
field := fields.Field(i)
|
||||
keys[i], _, _ = strings.Cut(field.Tag.Get("json"), ",")
|
||||
if field.Anonymous || !field.IsExported() || keys[i] == "" || keys[i] == "-" {
|
||||
t.Errorf("AlertConfig.%s needs to be an exported, named field with an explicit json key", field.Name)
|
||||
}
|
||||
// Keys that fold equal would both count as sent for either key.
|
||||
for j := 0; j < i; j++ {
|
||||
if strings.EqualFold(keys[i], keys[j]) {
|
||||
t.Errorf("AlertConfig.%s and AlertConfig.%s have json keys that fold equal",
|
||||
fields.Field(j).Name, field.Name)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplyAlertConfigUpdateSharesNoStateWithStored(t *testing.T) {
|
||||
stored := storedAlertConfigForUpdate()
|
||||
|
||||
got, err := ApplyAlertConfigUpdate(stored, []byte(`{"enabled": true}`))
|
||||
if err != nil {
|
||||
t.Fatalf("ApplyAlertConfigUpdate: %v", err)
|
||||
}
|
||||
got.TimeThresholds["guest"] = 99
|
||||
got.DiskTempByType["nvme"] = HysteresisThreshold{Trigger: 1, Clear: 0}
|
||||
got.IgnoredGuestPrefixes[0] = "changed-"
|
||||
*got.ActivationTime = time.Time{}
|
||||
|
||||
if stored.TimeThresholds["guest"] != 5 || stored.DiskTempByType["nvme"].Trigger != 70 ||
|
||||
stored.IgnoredGuestPrefixes[0] != "test-" || stored.ActivationTime.IsZero() {
|
||||
t.Fatal("editing the merged config changed the stored config it was merged from")
|
||||
}
|
||||
}
|
||||
|
||||
func TestApplyAlertConfigUpdateRejectsMalformedBodies(t *testing.T) {
|
||||
stored := storedAlertConfigForUpdate()
|
||||
|
||||
for _, body := range []string{`null`, `[]`, `"config"`, `{"flappingEnabled": "yes"}`, `{`} {
|
||||
if _, err := ApplyAlertConfigUpdate(stored, []byte(body)); err == nil {
|
||||
t.Fatalf("ApplyAlertConfigUpdate(%s) succeeded, want a decode error", body)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A config marshaled by a Go client still replaces every field it writes.
|
||||
// Empty omitempty fields are left out of that body, so they keep their
|
||||
// stored values like any other unsent key.
|
||||
func TestApplyAlertConfigUpdateFullBodyReplacesWrittenFields(t *testing.T) {
|
||||
stored := storedAlertConfigForUpdate()
|
||||
replacement := AlertConfig{Enabled: true, Overrides: map[string]ThresholdConfig{}, TimeThresholds: map[string]int{}}
|
||||
|
||||
body, err := json.Marshal(replacement)
|
||||
if err != nil {
|
||||
t.Fatalf("marshal replacement: %v", err)
|
||||
}
|
||||
got, err := ApplyAlertConfigUpdate(stored, body)
|
||||
if err != nil {
|
||||
t.Fatalf("ApplyAlertConfigUpdate: %v", err)
|
||||
}
|
||||
if got.FlappingEnabled || got.MaxAlertAgeDays != 0 || got.MinimumDelta != 0 || len(got.Overrides) != 0 {
|
||||
t.Fatalf("full body left written fields at their stored values: %+v", got)
|
||||
}
|
||||
if !reflect.DeepEqual(got.CustomRules, stored.CustomRules) {
|
||||
t.Fatalf("custom rules = %+v, want the stored rules for the omitted key", got.CustomRules)
|
||||
}
|
||||
}
|
||||
@@ -1,6 +1,8 @@
|
||||
package alerts
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -13,7 +15,36 @@ import (
|
||||
func (m *Manager) UpdateConfig(config AlertConfig) {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
m.updateConfigLocked(config)
|
||||
}
|
||||
|
||||
// ErrConfigSnapshot reports an update that was applied but could not be copied
|
||||
// for the caller.
|
||||
var ErrConfigSnapshot = errors.New("snapshot applied alert configuration")
|
||||
|
||||
// ApplyConfigUpdate applies a client's JSON update to the stored
|
||||
// configuration: keys the update carries replace their settings and the rest
|
||||
// keep their stored values (see alertconfig.ApplyAlertConfigUpdate). Reading,
|
||||
// merging and applying under one lock keeps two partial updates from
|
||||
// reverting each other's settings. It returns a snapshot of the applied
|
||||
// config that the caller owns, so persisting it cannot write into the live
|
||||
// config's maps.
|
||||
func (m *Manager) ApplyConfigUpdate(update []byte) (AlertConfig, error) {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
config, err := alertconfig.ApplyAlertConfigUpdate(m.config, update)
|
||||
if err != nil {
|
||||
return AlertConfig{}, err
|
||||
}
|
||||
m.updateConfigLocked(config)
|
||||
snapshot, err := alertconfig.CloneAlertConfig(m.config)
|
||||
if err != nil {
|
||||
return AlertConfig{}, fmt.Errorf("%w: %w", ErrConfigSnapshot, err)
|
||||
}
|
||||
return snapshot, nil
|
||||
}
|
||||
|
||||
func (m *Manager) updateConfigLocked(config AlertConfig) {
|
||||
// Clients and rollback-era config writers may not know about the additive
|
||||
// identity schema marker. Never lower a version already held by this
|
||||
// process; an older binary can still omit it on disk and the migration will
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
package alerts
|
||||
|
||||
import "testing"
|
||||
import (
|
||||
"sync"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestUpdateConfigNormalizesBackupIndicatorHours(t *testing.T) {
|
||||
t.Run("non-positive values default", func(t *testing.T) {
|
||||
@@ -83,6 +86,80 @@ func TestUpdateConfigNormalizesFlappingSettings(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// The thresholds page sends no flapping, alert TTL or custom-rule keys. A save
|
||||
// through ApplyConfigUpdate must leave those settings as stored, also after
|
||||
// the merged config is normalized in place.
|
||||
func TestUpdateConfigKeepsSettingsAThresholdsSaveDidNotSend(t *testing.T) {
|
||||
m := newTestManager(t)
|
||||
stored := m.GetConfig()
|
||||
stored.FlappingEnabled = true
|
||||
stored.FlappingWindowSeconds = 600
|
||||
stored.MaxAlertAgeDays = 7
|
||||
stored.MaxAcknowledgedAgeDays = 1
|
||||
stored.AutoAcknowledgeAfterHours = 24
|
||||
stored.CustomRules = []CustomAlertRule{{ID: "rule-1", Name: "Busy web", Enabled: true}}
|
||||
m.UpdateConfig(stored)
|
||||
|
||||
snapshot, err := m.ApplyConfigUpdate([]byte(`{
|
||||
"enabled": true,
|
||||
"nodeDefaults": {"cpu": {"trigger": 82, "clear": 77}},
|
||||
"overrides": {}
|
||||
}`))
|
||||
if err != nil {
|
||||
t.Fatalf("ApplyConfigUpdate: %v", err)
|
||||
}
|
||||
snapshot.TimeThresholds["guest"] = 999
|
||||
if m.GetConfig().TimeThresholds["guest"] == 999 {
|
||||
t.Fatal("the returned snapshot shares its maps with the live config")
|
||||
}
|
||||
|
||||
got := m.GetConfig()
|
||||
if got.NodeDefaults.CPU == nil || got.NodeDefaults.CPU.Trigger != 82 {
|
||||
t.Fatalf("node CPU default = %+v, want the saved trigger 82", got.NodeDefaults.CPU)
|
||||
}
|
||||
if !got.FlappingEnabled || got.FlappingWindowSeconds != 600 {
|
||||
t.Fatalf("flapping = %v/%d after a thresholds save, want true/600", got.FlappingEnabled, got.FlappingWindowSeconds)
|
||||
}
|
||||
if got.MaxAlertAgeDays != 7 || got.MaxAcknowledgedAgeDays != 1 || got.AutoAcknowledgeAfterHours != 24 {
|
||||
t.Fatalf("TTL cleanup = %d/%d/%d after a thresholds save, want 7/1/24",
|
||||
got.MaxAlertAgeDays, got.MaxAcknowledgedAgeDays, got.AutoAcknowledgeAfterHours)
|
||||
}
|
||||
if len(got.CustomRules) != 1 || got.CustomRules[0].ID != "rule-1" {
|
||||
t.Fatalf("custom rules = %+v after a thresholds save, want the stored rule", got.CustomRules)
|
||||
}
|
||||
}
|
||||
|
||||
// Each partial update reads, merges and applies under the manager lock, so
|
||||
// two saves that touch different settings cannot revert each other.
|
||||
func TestApplyConfigUpdateKeepsConcurrentPartialUpdates(t *testing.T) {
|
||||
m := newTestManager(t)
|
||||
|
||||
for round := 0; round < 50; round++ {
|
||||
base := m.GetConfig()
|
||||
base.FlappingEnabled = false
|
||||
base.MaxAlertAgeDays = 0
|
||||
m.UpdateConfig(base)
|
||||
|
||||
var wg sync.WaitGroup
|
||||
for _, body := range []string{`{"flappingEnabled": true}`, `{"maxAlertAgeDays": 9}`} {
|
||||
wg.Add(1)
|
||||
go func(body string) {
|
||||
defer wg.Done()
|
||||
if _, err := m.ApplyConfigUpdate([]byte(body)); err != nil {
|
||||
t.Errorf("ApplyConfigUpdate(%s): %v", body, err)
|
||||
}
|
||||
}(body)
|
||||
}
|
||||
wg.Wait()
|
||||
|
||||
got := m.GetConfig()
|
||||
if !got.FlappingEnabled || got.MaxAlertAgeDays != 9 {
|
||||
t.Fatalf("round %d: flapping=%v maxAlertAgeDays=%d, want both concurrent updates kept",
|
||||
round, got.FlappingEnabled, got.MaxAlertAgeDays)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdateConfigNormalizesExactEscalationRoutingAndRepeatPolicy(t *testing.T) {
|
||||
m := newTestManager(t)
|
||||
cfg := m.GetConfig()
|
||||
|
||||
@@ -32,6 +32,7 @@ import (
|
||||
type AlertManager interface {
|
||||
GetConfig() alerts.AlertConfig
|
||||
UpdateConfig(alerts.AlertConfig)
|
||||
ApplyConfigUpdate(update []byte) (alerts.AlertConfig, error)
|
||||
GetActiveAlerts() []alerts.Alert
|
||||
DiagnoseAlertDelivery(alertIdentifier string) (alerts.AlertDeliveryDiagnosis, bool)
|
||||
DiagnoseActiveAlertDeliveries() []alerts.AlertDeliveryDiagnosis
|
||||
@@ -80,6 +81,7 @@ type AlertMonitor interface {
|
||||
type AlertHandlers struct {
|
||||
stateMu sync.RWMutex
|
||||
intentPolicyMu sync.Mutex
|
||||
configSaveMu sync.Mutex
|
||||
mtMonitor *monitoring.MultiTenantMonitor
|
||||
defaultMonitor AlertMonitor
|
||||
wsHub *websocket.Hub
|
||||
@@ -292,13 +294,28 @@ func (h *AlertHandlers) UpdateAlertConfig(w http.ResponseWriter, r *http.Request
|
||||
// from instances with a few hundred disabled containers (#1601).
|
||||
r.Body = http.MaxBytesReader(w, r.Body, 1<<20)
|
||||
|
||||
var config alerts.AlertConfig
|
||||
if err := json.NewDecoder(r.Body).Decode(&config); err != nil {
|
||||
body, err := io.ReadAll(r.Body)
|
||||
if err != nil {
|
||||
http.Error(w, err.Error(), http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
// One save at a time, so a slower save cannot write an older config to
|
||||
// disk after a newer one was applied.
|
||||
h.configSaveMu.Lock()
|
||||
defer h.configSaveMu.Unlock()
|
||||
|
||||
// Keys the client left out keep their stored values, so a thresholds
|
||||
// save no longer turns off flapping detection or alert TTL cleanup.
|
||||
updatedConfig, err := h.getMonitor(r.Context()).GetAlertManager().ApplyConfigUpdate(body)
|
||||
if errors.Is(err, alerts.ErrConfigSnapshot) {
|
||||
log.Error().Err(err).Msg("Failed to snapshot applied alert configuration")
|
||||
http.Error(w, err.Error(), http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
if err != nil {
|
||||
http.Error(w, err.Error(), http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
h.getMonitor(r.Context()).GetAlertManager().UpdateConfig(config)
|
||||
updatedConfig := h.getMonitor(r.Context()).GetAlertManager().GetConfig()
|
||||
|
||||
// Update notification manager with schedule settings
|
||||
notificationMgr := h.getMonitor(r.Context()).GetNotificationManager()
|
||||
|
||||
@@ -74,6 +74,11 @@ func (m *MockAlertManager) UpdateConfig(cfg alerts.AlertConfig) {
|
||||
m.Called(cfg)
|
||||
}
|
||||
|
||||
func (m *MockAlertManager) ApplyConfigUpdate(update []byte) (alerts.AlertConfig, error) {
|
||||
args := m.Called(update)
|
||||
return args.Get(0).(alerts.AlertConfig), args.Error(1)
|
||||
}
|
||||
|
||||
func (m *MockAlertManager) GetActiveAlerts() []alerts.Alert {
|
||||
args := m.Called()
|
||||
return args.Get(0).([]alerts.Alert)
|
||||
@@ -238,7 +243,7 @@ func TestUpdateAlertConfig(t *testing.T) {
|
||||
h := NewAlertHandlers(nil, mockMonitor, nil)
|
||||
|
||||
cfg := alerts.AlertConfig{Enabled: true, ActivationState: alerts.ActivationPending}
|
||||
mockManager.On("UpdateConfig", testifymock.Anything).Return()
|
||||
mockManager.On("ApplyConfigUpdate", testifymock.Anything).Return(cfg, nil)
|
||||
mockManager.On("GetConfig").Return(cfg)
|
||||
mockPersist.On("SaveAlertConfig", testifymock.Anything).Return(nil)
|
||||
|
||||
@@ -252,6 +257,88 @@ func TestUpdateAlertConfig(t *testing.T) {
|
||||
assert.False(t, notificationMgr.IsEnabled())
|
||||
}
|
||||
|
||||
// The Thresholds page has no control for flapping detection or alert TTL
|
||||
// cleanup and never sends those keys; decoding its body into a zero config
|
||||
// turned both off on every save.
|
||||
func TestUpdateAlertConfig_KeepsStoredValuesForUnsentKeys(t *testing.T) {
|
||||
manager := alerts.NewManagerWithDataDir(t.TempDir())
|
||||
defer manager.Stop()
|
||||
stored := manager.GetConfig()
|
||||
stored.MaxAlertAgeDays = 7
|
||||
stored.MaxAcknowledgedAgeDays = 1
|
||||
stored.AutoAcknowledgeAfterHours = 24
|
||||
stored.FlappingEnabled = true
|
||||
manager.UpdateConfig(stored)
|
||||
|
||||
mockMonitor := new(MockAlertMonitor)
|
||||
mockPersist := new(MockConfigPersistence)
|
||||
notificationMgr := notifications.NewNotificationManagerWithDataDir("", t.TempDir())
|
||||
defer notificationMgr.Stop()
|
||||
mockMonitor.On("GetAlertManager").Return(manager)
|
||||
mockMonitor.On("GetConfigPersistence").Return(mockPersist)
|
||||
mockMonitor.On("GetNotificationManager").Return(notificationMgr)
|
||||
var persisted alerts.AlertConfig
|
||||
mockPersist.On("SaveAlertConfig", testifymock.Anything).Run(func(args testifymock.Arguments) {
|
||||
persisted = args.Get(0).(alerts.AlertConfig)
|
||||
}).Return(nil)
|
||||
|
||||
h := NewAlertHandlers(nil, mockMonitor, nil)
|
||||
|
||||
body := `{"enabled":true,"guestDefaults":{"cpu":{"trigger":90,"clear":85}},"overrides":{},` +
|
||||
`"flapping":{"enabled":true,"threshold":5,"window":10,"suppressionTime":30,"minStability":0.8}}`
|
||||
req := httptest.NewRequest(http.MethodPut, "/api/alerts/config", strings.NewReader(body))
|
||||
w := httptest.NewRecorder()
|
||||
|
||||
h.UpdateAlertConfig(w, req)
|
||||
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("status = %d, want 200: %s", w.Code, w.Body.String())
|
||||
}
|
||||
// Persistence normalizes the config it is handed in place, so it must get
|
||||
// a snapshot rather than the live config's maps.
|
||||
persisted.TimeThresholds["guest"] = 999
|
||||
if manager.GetConfig().TimeThresholds["guest"] == 999 {
|
||||
t.Fatal("the persisted config shares its maps with the live alert config")
|
||||
}
|
||||
persisted.TimeThresholds["guest"] = manager.GetConfig().TimeThresholds["guest"]
|
||||
for name, cfg := range map[string]alerts.AlertConfig{"live": manager.GetConfig(), "persisted": persisted} {
|
||||
if cfg.GuestDefaults.CPU == nil || cfg.GuestDefaults.CPU.Trigger != 90 {
|
||||
t.Fatalf("%s guest CPU default = %+v, want the sent trigger 90", name, cfg.GuestDefaults.CPU)
|
||||
}
|
||||
if !cfg.FlappingEnabled {
|
||||
t.Fatalf("thresholds save turned %s flapping detection off", name)
|
||||
}
|
||||
if cfg.MaxAlertAgeDays != 7 || cfg.MaxAcknowledgedAgeDays != 1 || cfg.AutoAcknowledgeAfterHours != 24 {
|
||||
t.Fatalf("%s TTL cleanup = %d/%d/%d after a thresholds save, want 7/1/24",
|
||||
name, cfg.MaxAlertAgeDays, cfg.MaxAcknowledgedAgeDays, cfg.AutoAcknowledgeAfterHours)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdateAlertConfig_RejectsNonObjectBody(t *testing.T) {
|
||||
manager := alerts.NewManagerWithDataDir(t.TempDir())
|
||||
defer manager.Stop()
|
||||
before := manager.GetConfig()
|
||||
|
||||
mockMonitor := new(MockAlertMonitor)
|
||||
mockMonitor.On("GetAlertManager").Return(manager)
|
||||
h := NewAlertHandlers(nil, mockMonitor, nil)
|
||||
|
||||
for _, body := range []string{`null`, `[]`, `"config"`} {
|
||||
req := httptest.NewRequest(http.MethodPut, "/api/alerts/config", strings.NewReader(body))
|
||||
w := httptest.NewRecorder()
|
||||
|
||||
h.UpdateAlertConfig(w, req)
|
||||
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("status = %d for body %s, want 400", w.Code, body)
|
||||
}
|
||||
}
|
||||
if after := manager.GetConfig(); after.Enabled != before.Enabled || after.ActivationState != before.ActivationState {
|
||||
t.Fatalf("a rejected body changed the config: %+v", after)
|
||||
}
|
||||
}
|
||||
|
||||
// Regression for #1341: persistence failures used to be silently swallowed
|
||||
// (logged but reported as "saved successfully"), so a user setting a custom
|
||||
// override would see the value stick in memory and revert on reload with
|
||||
@@ -271,7 +358,7 @@ func TestUpdateAlertConfig_PersistenceFailureSurfacesAsError(t *testing.T) {
|
||||
h := NewAlertHandlers(nil, mockMonitor, nil)
|
||||
|
||||
cfg := alerts.AlertConfig{Enabled: true, ActivationState: alerts.ActivationPending}
|
||||
mockManager.On("UpdateConfig", testifymock.Anything).Return()
|
||||
mockManager.On("ApplyConfigUpdate", testifymock.Anything).Return(cfg, nil)
|
||||
mockManager.On("GetConfig").Return(cfg)
|
||||
mockPersist.On("SaveAlertConfig", testifymock.Anything).Return(errors.New("permission denied"))
|
||||
|
||||
|
||||
Reference in New Issue
Block a user