fix(backup): refuse another schema before reading the dump; tests read settings directly (#5815)
- restore_in_place checks the backup's migration heads before pg_restore writes anything, so a backup from another schema is refused at once. - The integration tests read their own setting rows directly: get_all_settings knows only Steward's own keys. - check.py: drop an import left unused by the move to pgdata. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -35,7 +35,7 @@ from sqlalchemy.ext.asyncio import create_async_engine
|
|||||||
|
|
||||||
from .contents import (ACTIVITY, ACTIVITY_EVERYWHERE, AREAS, EXCLUDED, HAND_SET_ONLY,
|
from .contents import (ACTIVITY, ACTIVITY_EVERYWHERE, AREAS, EXCLUDED, HAND_SET_ONLY,
|
||||||
STATE_SETTINGS)
|
STATE_SETTINGS)
|
||||||
from .dump import BackupError, read_backup
|
from .dump import read_backup
|
||||||
from .pgdata import async_url as _async_url
|
from .pgdata import async_url as _async_url
|
||||||
from .pgdata import copy_rows, restore_script, script_tables
|
from .pgdata import copy_rows, restore_script, script_tables
|
||||||
|
|
||||||
|
|||||||
@@ -321,17 +321,20 @@ async def restore_in_place(path: Path, database_url: str, *,
|
|||||||
from .contents import ACTIVITY, STATE_SETTINGS
|
from .contents import ACTIVITY, STATE_SETTINGS
|
||||||
from .pgdata import restore_script, script_blocks
|
from .pgdata import restore_script, script_blocks
|
||||||
|
|
||||||
with tempfile.TemporaryDirectory() as tmp:
|
|
||||||
info, dump, key = await asyncio.to_thread(read_backup, path, Path(tmp))
|
|
||||||
blocks = script_blocks(await restore_script(dump))
|
|
||||||
engine = create_async_engine(_async_url(database_url))
|
engine = create_async_engine(_async_url(database_url))
|
||||||
try:
|
try:
|
||||||
async with engine.connect() as conn:
|
# The schema first: a backup that can't go in place is refused before
|
||||||
heads = sorted((await conn.execute(text("SELECT version_num FROM alembic_version"))).scalars())
|
# anything else is read.
|
||||||
|
with tempfile.TemporaryDirectory() as tmp:
|
||||||
|
info, dump, key = await asyncio.to_thread(read_backup, path, Path(tmp))
|
||||||
|
async with engine.connect() as conn:
|
||||||
|
heads = sorted((await conn.execute(text("SELECT version_num FROM alembic_version"))).scalars())
|
||||||
backup_heads = sorted(info.get("migration_heads") or [])
|
backup_heads = sorted(info.get("migration_heads") or [])
|
||||||
schema = schema_of(backup_heads, heads, known_revisions(plugin_dirs))
|
schema = schema_of(backup_heads, heads, known_revisions(plugin_dirs))
|
||||||
if schema != "same":
|
if schema != "same":
|
||||||
raise NotSameSchema(schema, backup_heads, heads)
|
raise NotSameSchema(schema, backup_heads, heads)
|
||||||
|
blocks = script_blocks(await restore_script(dump))
|
||||||
|
async with engine.connect() as conn:
|
||||||
present = set((await conn.execute(text(
|
present = set((await conn.execute(text(
|
||||||
"SELECT tablename FROM pg_tables WHERE schemaname = 'public'"))).scalars())
|
"SELECT tablename FROM pg_tables WHERE schemaname = 'public'"))).scalars())
|
||||||
tables = sorted(t for t in blocks if t not in EXCLUDED and t in present)
|
tables = sorted(t for t in blocks if t not in EXCLUDED and t in present)
|
||||||
|
|||||||
@@ -47,6 +47,17 @@ async def _snapshot(app) -> bytes:
|
|||||||
return script_from(blocks)
|
return script_from(blocks)
|
||||||
|
|
||||||
|
|
||||||
|
async def _setting(app, key):
|
||||||
|
"""A setting's stored value (get_all_settings knows only Steward's own keys)."""
|
||||||
|
import json
|
||||||
|
|
||||||
|
from sqlalchemy import select
|
||||||
|
from steward.models.settings import AppSetting
|
||||||
|
async with app.db_sessionmaker() as s:
|
||||||
|
raw = (await s.execute(select(AppSetting.value_json).where(AppSetting.key == key))).scalar_one()
|
||||||
|
return json.loads(raw)
|
||||||
|
|
||||||
|
|
||||||
async def _backup_file(app, tmp_path, heads=None):
|
async def _backup_file(app, tmp_path, heads=None):
|
||||||
from steward.backup import dump, job
|
from steward.backup import dump, job
|
||||||
(tmp_path / "steward.dump").write_bytes(b"PGDMP")
|
(tmp_path / "steward.dump").write_bytes(b"PGDMP")
|
||||||
@@ -61,7 +72,7 @@ def test_a_backup_goes_back_in_place_and_history_stays(app, monkeypatch, tmp_pat
|
|||||||
from sqlalchemy import func, select
|
from sqlalchemy import func, select
|
||||||
from steward.backup import pgdata
|
from steward.backup import pgdata
|
||||||
from steward.backup.restore import restore_in_place
|
from steward.backup.restore import restore_in_place
|
||||||
from steward.core.settings import get_all_settings, set_setting
|
from steward.core.settings import set_setting
|
||||||
from steward.models.alerts import AlertEvent, AlertOperator, AlertRule
|
from steward.models.alerts import AlertEvent, AlertOperator, AlertRule
|
||||||
from steward.models.audit import AuditEvent
|
from steward.models.audit import AuditEvent
|
||||||
from steward.models.hosts import Host
|
from steward.models.hosts import Host
|
||||||
@@ -110,8 +121,8 @@ def test_a_backup_goes_back_in_place_and_history_stays(app, monkeypatch, tmp_pat
|
|||||||
|
|
||||||
done = await restore_in_place(path, app.config["DATABASE_URL"])
|
done = await restore_in_place(path, app.config["DATABASE_URL"])
|
||||||
|
|
||||||
|
settings = {k: await _setting(app, k) for k in (setting, "backup.last_attempt_at")}
|
||||||
async with app.db_sessionmaker() as s:
|
async with app.db_sessionmaker() as s:
|
||||||
settings = await get_all_settings(s)
|
|
||||||
hosts = set((await s.execute(select(Host.id).where(
|
hosts = set((await s.execute(select(Host.id).where(
|
||||||
Host.id.in_([ids["kept_host"], ids["new_host"]])))).scalars())
|
Host.id.in_([ids["kept_host"], ids["new_host"]])))).scalars())
|
||||||
events = dict((await s.execute(select(AlertEvent.rule_id, func.count()).where(
|
events = dict((await s.execute(select(AlertEvent.rule_id, func.count()).where(
|
||||||
@@ -135,7 +146,7 @@ def test_a_restore_that_fails_part_way_changes_nothing(app, monkeypatch, tmp_pat
|
|||||||
from steward.backup import pgdata
|
from steward.backup import pgdata
|
||||||
from steward.backup.dump import BackupError
|
from steward.backup.dump import BackupError
|
||||||
from steward.backup.restore import restore_in_place
|
from steward.backup.restore import restore_in_place
|
||||||
from steward.core.settings import get_all_settings, set_setting
|
from steward.core.settings import set_setting
|
||||||
|
|
||||||
tag = uuid.uuid4().hex[:8]
|
tag = uuid.uuid4().hex[:8]
|
||||||
setting = f"test.undone_{tag}"
|
setting = f"test.undone_{tag}"
|
||||||
@@ -158,8 +169,7 @@ def test_a_restore_that_fails_part_way_changes_nothing(app, monkeypatch, tmp_pat
|
|||||||
await set_setting(s, setting, "changed")
|
await set_setting(s, setting, "changed")
|
||||||
with pytest.raises(BackupError, match="undone, nothing changed"):
|
with pytest.raises(BackupError, match="undone, nothing changed"):
|
||||||
await restore_in_place(path, app.config["DATABASE_URL"])
|
await restore_in_place(path, app.config["DATABASE_URL"])
|
||||||
async with app.db_sessionmaker() as s:
|
return await _setting(app, setting)
|
||||||
return (await get_all_settings(s))[setting]
|
|
||||||
|
|
||||||
assert asyncio.run(_go()) == "changed"
|
assert asyncio.run(_go()) == "changed"
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user