From e6da0ff2991a9ed1206ddea89145146ae744a8ac Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 15 Oct 2026 10:39:55 -0400 Subject: [PATCH] feat(backup): a backup holds settings and keys only, for recovery (#5822) #5822 "Backup contents: settings and keys only". The operator's ruling (System 180, 2026-10-15) overturns 2026-10-12's "Config + audit + alerts": a backup is for system recovery and carries what would otherwise have to be set up again, with no logs or history. - contents.py: audit_events, alert_events, alert_states, metric_anomalies, metric_anomaly_spans and user_invites move to EXCLUDED. - Alert state is no longer restored, and evaluation skips a rule with no state row. ensure_rule_states (alerts/service.py) gives every single-resource rule lacking one an inactive row; alert_reconcile runs it each minute. - Docs, the Backups page and the CLI help say what a backup holds now. CI's image check also finds the audit log empty after a restore. - tests/integration/test_backup_restore_fixes.py is removed: its case (an alert event naming a left-out Ansible run) no longer arises. The SET NULL fix-up is covered by a unit test on synthetic tables. Co-Authored-By: Claude Opus 5.5 --- .forgejo/workflows/ci.yml | 6 +- docs/core/backups.md | 26 +++++--- steward/alerts/service.py | 19 ++++++ steward/app.py | 5 +- steward/backup/contents.py | 31 +++++---- steward/cli.py | 4 +- steward/core/alerts.py | 3 +- steward/templates/settings/backups.html | 5 +- tests/core/test_backup_contents.py | 27 ++++++-- tests/integration/test_alert_states.py | 57 +++++++++++++++++ .../integration/test_backup_restore_fixes.py | 63 ------------------- 11 files changed, 147 insertions(+), 99 deletions(-) delete mode 100644 tests/integration/test_backup_restore_fixes.py diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index bb2c553..4166a1e 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -228,11 +228,12 @@ jobs: fi # A backup made by the image restores into an empty database, and - # Steward starts on it (milestone 507). It carries configuration - # (the marker setting) and leaves metrics' rows out. + # Steward starts on it (milestone 507). It carries settings (the + # marker setting) and leaves metrics and the audit log out. sql() { docker exec "$SMOKE-pg" psql -U steward -v ON_ERROR_STOP=1 -tA "$@"; } sql -d steward -c "INSERT INTO app_settings (key, value_json, updated_at) VALUES ('smoke.backup', '\"kept\"', now())" sql -d steward -c "INSERT INTO plugin_metrics (id, source_module, resource_name, metric_name, value, recorded_at) VALUES ('smoke-metric', 'smoke', 'smoke', 'smoke', 1, now())" + sql -d steward -c "INSERT INTO audit_events (id, timestamp, username, action) VALUES ('smoke-audit', now(), 'smoke', 'smoke.backup')" docker exec -u app "$SMOKE-app" steward backup --to /tmp/backups sql -d steward -c "CREATE DATABASE restored" docker exec -u app \ @@ -240,6 +241,7 @@ jobs: "$SMOKE-app" sh -c 'steward restore /tmp/backups/steward-*.tar' test "$(sql -d restored -c "SELECT value_json FROM app_settings WHERE key = 'smoke.backup'")" = '"kept"' test "$(sql -d restored -c "SELECT count(*) FROM plugin_metrics")" = "0" + test "$(sql -d restored -c "SELECT count(*) FROM audit_events")" = "0" test "$(sql -d restored -c "SELECT count(*) FROM alembic_version")" -ge 1 docker run -d --name "$SMOKE-restored" --network "$SMOKE-net" \ -e "STEWARD_DATABASE_URL=postgresql+asyncpg://steward:steward@$SMOKE-pg:5432/restored" \ diff --git a/docs/core/backups.md b/docs/core/backups.md index 223f810..a94174a 100644 --- a/docs/core/backups.md +++ b/docs/core/backups.md @@ -14,18 +14,28 @@ Steward back from it into an empty database. | `secret.key` | The key Steward's stored passwords and tokens are sealed with | | `manifest.json` | Format, Steward version, migration heads, time, and the tables whose rows were left out | +A backup is for **system recovery**: it holds what you would otherwise have +to set up again, and no logs or history. + **Kept:** -- configuration: settings, users, API tokens, hosts, monitors, alert rules, maintenance windows, Ansible inventory and schedules, dashboards, SNMP devices, Prometheus targets, UniFi client names; -- the audit log; -- alert history. +- settings, with the passwords and keys they hold; +- users and API tokens; +- hosts and their agent enrolments, so agents don't need enrolling again; +- monitors, alert rules, maintenance windows and maintenance cadences; +- Ansible inventory and schedules; +- dashboards and share links; +- SNMP devices, Prometheus targets and widget templates, UniFi client names. **Left out** (the tables come back empty and fill again as Steward runs): -- metrics and their rollups; +- the audit log; +- alert history and alert state. Each rule starts inactive and is watched + again within a minute; +- metric anomalies and their reviews; +- metrics, their rollups and learned normals; - monitor check results; - Docker, Traefik and UniFi history; -- Ansible run records; -- the email queue; -- password-reset links. +- Ansible run records and maintenance job history; +- the email queue, pending invites and password-reset links. The list is `steward/backup/contents.py`. A test fails when a new table appears in neither list, so whoever adds a table decides. @@ -105,7 +115,7 @@ everything in the database's public schema first. then data, then constraints and indexes. Between the data and the constraints, a kept row that points at a left-out row is put right by its foreign key's ON DELETE: -- an alert event's link to an Ansible run is cleared; +- a SET NULL link is cleared and the row kept; - a row whose foreign key would cascade or block is removed. pg_restore 17 writes each section as a script for psql. It runs against diff --git a/steward/alerts/service.py b/steward/alerts/service.py index 3349912..33f476d 100644 --- a/steward/alerts/service.py +++ b/steward/alerts/service.py @@ -744,6 +744,25 @@ async def reconcile_covering_rules(app) -> None: ", ".join(added) or "none", ", ".join(removed) or "none") +async def ensure_rule_states(app) -> int: + """A state row, inactive, for every rule watching one resource that has + none. Evaluation skips a rule without one, and a backup leaves alert state + out (System 180, 2026-10-15), so a restored rule comes back without it. + Run each minute by the core scheduler (alert_reconcile): a restored rule is + watched again within a minute. Returns how many rows were added.""" + async with app.db_sessionmaker() as db: + async with db.begin(): + has_state = select(AlertState.id).where(AlertState.rule_id == AlertRule.id).exists() + missing = (await db.execute(select(AlertRule.id).where( + AlertRule.covers == "resource", ~has_state))).scalars().all() + now = datetime.now(timezone.utc) + db.add_all([AlertState(rule_id=rule_id, state=AlertStateEnum.inactive, + last_transition_at=now) for rule_id in missing]) + if missing: + logger.info("Alert rules without a state row: added %d", len(missing)) + return len(missing) + + def part_of(rule: AlertRule, rules) -> AlertRule | None: """The covering rule a child belongs to, from a list of rules; None for a rule that stands alone.""" diff --git a/steward/app.py b/steward/app.py index ed897b2..19dc84e 100644 --- a/steward/app.py +++ b/steward/app.py @@ -429,9 +429,10 @@ def _register_core_tasks(app: Quart) -> None: await record_cleanup_secs(app, done["secs"]) async def run_alert_reconcile(): - from .alerts.service import reconcile_covering_rules + from .alerts.service import ensure_rule_states, reconcile_covering_rules from .core.alerts import check_silence, notify_after_maintenance - for step in (reconcile_covering_rules, check_silence, notify_after_maintenance): + for step in (reconcile_covering_rules, ensure_rule_states, check_silence, + notify_after_maintenance): try: await step(app) except Exception: diff --git a/steward/backup/contents.py b/steward/backup/contents.py index 870ef2c..087a0f1 100644 --- a/steward/backup/contents.py +++ b/steward/backup/contents.py @@ -2,38 +2,43 @@ or left out, and tests/core/test_backup_contents.py fails on one that is neither, so a new table is a decision someone makes in this file. -The operator's ruling (System 180 "Backup & Restore", 2026-10-12): a backup -carries configuration, the audit log and alert history. Metrics, check -results and the Docker, Traefik and UniFi history are left out: their tables -come back with their structure and no rows, and fill again as Steward runs. +The operator's ruling (System 180 "Backup & Restore", 2026-10-15): a backup +is for system recovery. It carries settings and keys, what would otherwise +have to be set up again, and no logs or history: not the audit log, not +alert history, not metrics. Left-out tables come back with their structure +and no rows, and fill again as Steward runs. A table that is in neither list (an external plugin's) is backed up whole: a table nobody thought about is kept, never lost. """ from __future__ import annotations -# Configuration, and the history the operator chose to keep. +# What would otherwise have to be set up again. KEPT: frozenset[str] = frozenset({ - "app_settings", "users", "user_invites", "api_tokens", + "app_settings", "users", "api_tokens", "hosts", "host_agent_registrations", # A group's maintenance cadence (#5768): configuration. "host_maintenance_plans", "monitors", - "alert_rules", "alert_states", "maintenance_windows", + "alert_rules", "maintenance_windows", "ansible_targets", "ansible_groups", "ansible_target_groups", "ansible_schedules", "dashboards", "dashboard_widgets", "dashboard_share_tokens", "snmp_devices", "prometheus_targets", "prometheus_widget_templates", # Names given to network clients by hand (UniFi's alias column). "unifi_known_clients", - # History kept by the ruling. - "audit_events", "alert_events", - # Metric anomalies and their reviews (#5740): what someone marked normal - # or real is a decision, kept like alert history. - "metric_anomalies", "metric_anomaly_spans", }) -# Rows that are measurements, logs or in-flight work: left out. +# Rows that are measurements, logs, history or in-flight work: left out. EXCLUDED: frozenset[str] = frozenset({ + # Logs and history (the 2026-10-15 ruling): a recovery doesn't need them. + "audit_events", "alert_events", + # Live state: each rule starts inactive, and alert_reconcile gives a + # restored rule its state row within a minute (ensure_rule_states). + "alert_states", + # Metric anomalies and their reviews (#5740): history of the metrics. + "metric_anomalies", "metric_anomaly_spans", + # A pending invite is in flight, not setup; it can be sent again. + "user_invites", "plugin_metrics", "plugin_metric_latest", "plugin_metrics_hourly", # Learned from the metrics above, so relearned once they fill again. "metric_norms", diff --git a/steward/cli.py b/steward/cli.py index 92458db..9f261d6 100644 --- a/steward/cli.py +++ b/steward/cli.py @@ -65,8 +65,8 @@ def main(ctx: click.Context, config: str, host: str, port: int, debug: bool) -> help="The folder to write the backup file into.") @click.pass_context def backup_command(ctx: click.Context, out_dir: Path) -> None: - """Write a backup file of this Steward (configuration, audit log, alert - history and the secret key) into a folder, without sending it anywhere.""" + """Write a backup file of this Steward (settings and the secret key: what + recovery needs, no logs or history) into a folder, without sending it anywhere.""" from .backup.dump import BackupError, make_backup from .backup.job import migration_heads from .config import load_bootstrap diff --git a/steward/core/alerts.py b/steward/core/alerts.py index 078747e..716db40 100644 --- a/steward/core/alerts.py +++ b/steward/core/alerts.py @@ -352,7 +352,8 @@ async def record_metrics(session: AsyncSession, rows: list[PluginMetric], phases for rule in rules: alert_state = states.get(rule.id) if alert_state is None: - logger.warning("No alert_state row for rule %s, skipping", rule.id) + # A restored rule (backups leave state out); alert_reconcile adds it. + logger.warning("No alert_state row for rule %s yet, skipping", rule.id) continue samples = sorted(by_key[(rule.source_module, rule.resource_name, rule.metric_name)], key=lambda r: r.recorded_at) diff --git a/steward/templates/settings/backups.html b/steward/templates/settings/backups.html index 1a6720c..13f1176 100644 --- a/steward/templates/settings/backups.html +++ b/steward/templates/settings/backups.html @@ -30,8 +30,9 @@

Bucket

- Each backup is one file, steward-<time>.tar: Steward's configuration, its audit log - and alert history, and its secret key. Metrics and check results are left out. Anyone holding a + Each backup is one file, steward-<time>.tar, for getting Steward running again: + its settings, users, hosts, monitors, alert rules and dashboards, and its secret key. Logs and + history are left out: the audit log, alert history, metrics and check results. Anyone holding a copy can read every password and key Steward keeps, so keep the bucket private.

diff --git a/tests/core/test_backup_contents.py b/tests/core/test_backup_contents.py index 093e4ee..bbb056f 100644 --- a/tests/core/test_backup_contents.py +++ b/tests/core/test_backup_contents.py @@ -31,11 +31,25 @@ def test_no_table_is_both_and_none_is_named_for_nothing(): assert stale == set(), "contents.py names tables that no longer exist" -def test_an_alert_event_keeps_its_row_when_its_ansible_run_is_left_out(): - fixes = dangling_fixes(all_metadata()) - assert ('UPDATE "alert_events" SET "ansible_run_id" = NULL WHERE "ansible_run_id" IS NOT NULL ' - 'AND NOT EXISTS (SELECT 1 FROM "ansible_runs" p WHERE p."id" = "alert_events"."ansible_run_id")' - ) in fixes +def test_a_set_null_reference_is_cleared_and_the_row_kept(): + md = MetaData() + Table("ansible_runs", md, Column("id", Integer, primary_key=True)) + Table("plans", md, Column("id", Integer, primary_key=True), + Column("run_id", Integer, ForeignKey("ansible_runs.id", ondelete="SET NULL"))) + assert dangling_fixes(md) == [ + 'UPDATE "plans" SET "run_id" = NULL WHERE "run_id" IS NOT NULL AND NOT EXISTS ' + '(SELECT 1 FROM "ansible_runs" p WHERE p."id" = "plans"."run_id")' + ] + + +def test_logs_history_and_state_are_left_out(): + """The 2026-10-15 ruling: a backup is settings and keys, for recovery.""" + for table in ("audit_events", "alert_events", "alert_states", "metric_anomalies", + "metric_anomaly_spans", "user_invites"): + assert table in contents.EXCLUDED, table + for table in ("app_settings", "users", "api_tokens", "hosts", "host_agent_registrations", + "monitors", "alert_rules"): + assert table in contents.KEPT, table def test_a_cascading_reference_removes_the_row(): @@ -55,7 +69,8 @@ def test_dump_leaves_out_each_excluded_tables_rows(tmp_path): args = dump.dump_command(tmp_path / "x.dump") assert "--format=custom" in args assert "--exclude-table-data=plugin_metrics" in args - assert "--exclude-table-data=audit_events" not in args + assert "--exclude-table-data=audit_events" in args + assert "--exclude-table-data=app_settings" not in args def test_password_goes_in_the_environment_not_the_command_line(): diff --git a/tests/integration/test_alert_states.py b/tests/integration/test_alert_states.py index 85130e6..4f032b7 100644 --- a/tests/integration/test_alert_states.py +++ b/tests/integration/test_alert_states.py @@ -173,3 +173,60 @@ def test_record_metrics_marks_each_step(app): watched, unwatched = asyncio.run(_go()) assert watched == ["flush", "rules", "evaluate"] assert unwatched == ["flush", "rules"] # no rule watches it: nothing to evaluate + + +def test_a_rule_without_state_gets_one_and_is_watched_again(app): + """A backup leaves alert state out (System 180, 2026-10-15): a restored + rule has none, and evaluation skips it until alert_reconcile adds it.""" + from sqlalchemy import delete, func, select + from steward.alerts import service as alert_svc + from steward.core.alerts import record_metric + from steward.models.alerts import AlertOperator, AlertRule, AlertState, AlertStateEnum + from steward.models.users import User, UserRole + + async def _go(): + tag = uuid.uuid4().hex[:8] + res = f"restored-{tag}" + async with app.db_sessionmaker() as s: + async with s.begin(): + user = User(username=f"rs-{tag}", email=f"rs-{tag}@test", password_hash="x", + role=UserRole.admin) + s.add(user) + await s.flush() + rule = AlertRule(name=f"restored {tag}", source_module="host_agent", + resource_name=res, metric_name="cpu_pct", + operator=AlertOperator.gt, threshold=90.0, + consecutive_failures_required=1, enabled=True, + created_by=user.id) + cover = AlertRule(name=f"all hosts {tag}", source_module="host_agent", + resource_name="", metric_name="cpu_pct", covers="all_hosts", + operator=AlertOperator.gt, threshold=90.0, + consecutive_failures_required=1, enabled=True, + created_by=user.id) + s.add_all([rule, cover]) + await s.flush() + await s.execute(delete(AlertState).where(AlertState.rule_id == rule.id)) + ids = (rule.id, cover.id) + + async def states(rule_id): + async with app.db_sessionmaker() as s: + return (await s.execute(select(func.count()).select_from(AlertState).where( + AlertState.rule_id == rule_id))).scalar_one() + + added = await alert_svc.ensure_rule_states(app) + first = (await states(ids[0]), await states(ids[1])) + await alert_svc.ensure_rule_states(app) + second = await states(ids[0]) + async with app.db_sessionmaker() as s: + async with s.begin(): + await record_metric(s, "host_agent", res, "cpu_pct", 99.0) + async with app.db_sessionmaker() as s: + state = (await s.execute(select(AlertState.state).where( + AlertState.rule_id == ids[0]))).scalar_one() + return added, first, second, state + + added, first, second, state = asyncio.run(_go()) + assert added >= 1 + assert first == (1, 0) # the covering rule has no state of its own + assert second == 1 # a second pass adds nothing + assert state == AlertStateEnum.firing diff --git a/tests/integration/test_backup_restore_fixes.py b/tests/integration/test_backup_restore_fixes.py deleted file mode 100644 index 0e97402..0000000 --- a/tests/integration/test_backup_restore_fixes.py +++ /dev/null @@ -1,63 +0,0 @@ -"""Integration: between pg_restore's data and its constraints, an alert event -naming an Ansible run the backup left out keeps its row and loses the link -(steward/backup/restore.py, #5674). Requires STEWARD_DATABASE_URL.""" -from __future__ import annotations - -import asyncio -import os -import uuid - -import pytest - -pytestmark = pytest.mark.integration - - -@pytest.fixture -def app(): - if not os.environ.get("STEWARD_DATABASE_URL"): - pytest.skip("needs Postgres") - from steward.app import create_app - return create_app(testing=False) - - -def test_a_dangling_ansible_run_link_is_cleared_and_the_event_kept(app): - from sqlalchemy import select, text - - from steward.backup.restore import _execute, dangling_fixes - from steward.models.base import Base - from steward.models.alerts import AlertEvent, AlertOperator, AlertRule - from steward.models.users import User, UserRole - - uid, rule_id = str(uuid.uuid4()), str(uuid.uuid4()) - gone_run, dangling, plain = str(uuid.uuid4()), str(uuid.uuid4()), str(uuid.uuid4()) - - async def _go(): - async with app.db_sessionmaker() as s: - async with s.begin(): - s.add(User(id=uid, username=f"u{uid[:8]}", email=f"{uid[:8]}@e.test", - password_hash="x", role=UserRole.admin, is_active=True)) - async with s.begin(): - s.add(AlertRule(id=rule_id, name="backup fixes", source_module="host_agent", - resource_name="srv1", metric_name="cpu_pct", - operator=AlertOperator.gt, threshold=90.0, - consecutive_failures_required=1, enabled=True, created_by=uid)) - # The state pg_restore's data section leaves: the run's row was - # never restored. Replica mode lets the row in without its parent. - async with s.begin(): - await s.execute(text("SET LOCAL session_replication_role = replica")) - s.add(AlertEvent(id=dangling, rule_id=rule_id, from_state="pending", - to_state="firing", ansible_run_id=gone_run)) - s.add(AlertEvent(id=plain, rule_id=rule_id, from_state="firing", - to_state="inactive")) - - fixed = await _execute(os.environ["STEWARD_DATABASE_URL"], dangling_fixes(Base.metadata)) - - async with app.db_sessionmaker() as s: - rows = dict((await s.execute( - select(AlertEvent.id, AlertEvent.ansible_run_id) - .where(AlertEvent.rule_id == rule_id))).all()) - return fixed, rows - - fixed, rows = asyncio.run(_go()) - assert fixed >= 1 - assert rows == {dangling: None, plain: None}