Archived
Harden cleanup scheduler and audit cleanup deletions
This commit is contained in:
1 parent
24747866bc
commit
23ab466c52
4 files changed
+99
No files matched your search
@@ -104,7 +104,12 @@ def make_app(settings: DdnsSettings | None = None) -> FastAPI:
|
|||||||
async def _scheduler_loop(store: DdnsStore, settings: DdnsSettings) -> None:
|
async def _scheduler_loop(store: DdnsStore, settings: DdnsSettings) -> None:
|
||||||
poll_seconds = min(settings.cleanup_interval_seconds, 60)
|
poll_seconds = min(settings.cleanup_interval_seconds, 60)
|
||||||
while True:
|
while True:
|
||||||
|
try:
|
||||||
await _run_cleanup_if_due(store, settings)
|
await _run_cleanup_if_due(store, settings)
|
||||||
|
except asyncio.CancelledError:
|
||||||
|
raise
|
||||||
|
except Exception:
|
||||||
|
LOGGER.exception("Scheduled cleanup loop crashed; continuing")
|
||||||
await asyncio.sleep(poll_seconds)
|
await asyncio.sleep(poll_seconds)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -16,3 +16,6 @@ DASHBOARD_HOSTNAME_ACTIVATED = "dashboard_hostname_activated"
|
|||||||
ADMIN_HOSTNAME_DELETED = "admin_hostname_deleted"
|
ADMIN_HOSTNAME_DELETED = "admin_hostname_deleted"
|
||||||
ADMIN_UPDATE_URL_ROTATED = "admin_update_url_rotated"
|
ADMIN_UPDATE_URL_ROTATED = "admin_update_url_rotated"
|
||||||
ADMIN_MANAGEMENT_URL_ROTATED = "admin_management_url_rotated"
|
ADMIN_MANAGEMENT_URL_ROTATED = "admin_management_url_rotated"
|
||||||
|
|
||||||
|
CLEANUP_PENDING_HOSTNAME_DELETED = "cleanup_pending_hostname_deleted"
|
||||||
|
CLEANUP_UNUSED_HOSTNAME_DELETED = "cleanup_unused_hostname_deleted"
|
||||||
@@ -4,6 +4,7 @@ import threading
|
|||||||
import time
|
import time
|
||||||
from datetime import UTC, datetime, timedelta
|
from datetime import UTC, datetime, timedelta
|
||||||
|
|
||||||
|
from ..event_statuses import CLEANUP_PENDING_HOSTNAME_DELETED, CLEANUP_UNUSED_HOSTNAME_DELETED
|
||||||
from .base import StoreConnectionMixin
|
from .base import StoreConnectionMixin
|
||||||
from .types import CleanupRunRow, cleanup_run_row
|
from .types import CleanupRunRow, cleanup_run_row
|
||||||
|
|
||||||
@@ -134,6 +135,28 @@ class RateLimitCleanupMixin(StoreConnectionMixin):
|
|||||||
pending_account_cutoff = (datetime.now(UTC) - timedelta(hours=pending_account_hours)).isoformat()
|
pending_account_cutoff = (datetime.now(UTC) - timedelta(hours=pending_account_hours)).isoformat()
|
||||||
event_history_cutoff = (datetime.now(UTC) - timedelta(hours=event_history_hours)).isoformat()
|
event_history_cutoff = (datetime.now(UTC) - timedelta(hours=event_history_hours)).isoformat()
|
||||||
with self.connect() as conn:
|
with self.connect() as conn:
|
||||||
|
deleted_accounts = conn.execute(
|
||||||
|
"""
|
||||||
|
SELECT hostname,
|
||||||
|
CASE
|
||||||
|
WHEN accepted_at IS NULL THEN 'pending'
|
||||||
|
ELSE 'unused'
|
||||||
|
END AS cleanup_reason
|
||||||
|
FROM accounts
|
||||||
|
WHERE (
|
||||||
|
accepted_at IS NULL
|
||||||
|
AND created_at < ?
|
||||||
|
)
|
||||||
|
OR (
|
||||||
|
accepted_at IS NOT NULL
|
||||||
|
AND created_at < ?
|
||||||
|
AND NOT EXISTS (
|
||||||
|
SELECT 1 FROM records WHERE records.hostname = accounts.hostname
|
||||||
|
)
|
||||||
|
)
|
||||||
|
""",
|
||||||
|
(pending_account_cutoff, unused_account_cutoff),
|
||||||
|
).fetchall()
|
||||||
account_cursor = conn.execute(
|
account_cursor = conn.execute(
|
||||||
"""
|
"""
|
||||||
DELETE FROM accounts
|
DELETE FROM accounts
|
||||||
@@ -168,6 +191,24 @@ class RateLimitCleanupMixin(StoreConnectionMixin):
|
|||||||
""",
|
""",
|
||||||
(datetime.now(UTC).isoformat(), account_cursor.rowcount),
|
(datetime.now(UTC).isoformat(), account_cursor.rowcount),
|
||||||
)
|
)
|
||||||
|
conn.executemany(
|
||||||
|
"""
|
||||||
|
INSERT INTO update_events(hostname, ipv4, ipv6, status, detail, source_ip, created_at)
|
||||||
|
VALUES (?, NULL, NULL, ?, ?, ?, ?)
|
||||||
|
""",
|
||||||
|
[
|
||||||
|
(
|
||||||
|
row["hostname"],
|
||||||
|
CLEANUP_PENDING_HOSTNAME_DELETED
|
||||||
|
if row["cleanup_reason"] == "pending"
|
||||||
|
else CLEANUP_UNUSED_HOSTNAME_DELETED,
|
||||||
|
"cleanup deleted abandoned hostname",
|
||||||
|
"127.0.0.1",
|
||||||
|
datetime.now(UTC).isoformat(),
|
||||||
|
)
|
||||||
|
for row in deleted_accounts
|
||||||
|
],
|
||||||
|
)
|
||||||
return {
|
return {
|
||||||
"unused_accounts": account_cursor.rowcount,
|
"unused_accounts": account_cursor.rowcount,
|
||||||
"event_rollups": rollup_cursor.rowcount,
|
"event_rollups": rollup_cursor.rowcount,
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ from fastapi.testclient import TestClient
|
|||||||
|
|
||||||
import karldns.app as app_module
|
import karldns.app as app_module
|
||||||
from karldns import DdnsSettings, DdnsStore, make_app
|
from karldns import DdnsSettings, DdnsStore, make_app
|
||||||
|
from karldns.event_statuses import CLEANUP_PENDING_HOSTNAME_DELETED, CLEANUP_UNUSED_HOSTNAME_DELETED
|
||||||
from karldns.security import hash_lookup_token
|
from karldns.security import hash_lookup_token
|
||||||
|
|
||||||
|
|
||||||
@@ -174,6 +175,9 @@ def test_cleanup_removes_pending_generated_setups_after_cutoff(tmp_path: Path) -
|
|||||||
|
|
||||||
assert result["unused_accounts"] == 1
|
assert result["unused_accounts"] == 1
|
||||||
assert store.get_account_by_slug(account["update_slug"]) is None
|
assert store.get_account_by_slug(account["update_slug"]) is None
|
||||||
|
cleanup_events = store.list_update_events_for_hostname(account["hostname"])
|
||||||
|
assert cleanup_events[0]["status"] == CLEANUP_PENDING_HOSTNAME_DELETED
|
||||||
|
assert cleanup_events[0]["detail"] == "cleanup deleted abandoned hostname"
|
||||||
|
|
||||||
|
|
||||||
def test_cleanup_keeps_recent_pending_setup_until_pending_cutoff(tmp_path: Path) -> None:
|
def test_cleanup_keeps_recent_pending_setup_until_pending_cutoff(tmp_path: Path) -> None:
|
||||||
@@ -194,6 +198,26 @@ def test_cleanup_keeps_recent_pending_setup_until_pending_cutoff(tmp_path: Path)
|
|||||||
assert store.get_account_by_slug(account["update_slug"]) is not None
|
assert store.get_account_by_slug(account["update_slug"]) is not None
|
||||||
|
|
||||||
|
|
||||||
|
def test_cleanup_logs_event_for_accepted_unused_hostname(tmp_path: Path) -> None:
|
||||||
|
database_path = tmp_path / "ddns.sqlite3"
|
||||||
|
store = DdnsStore(database_path)
|
||||||
|
account = store.create_account("unused.ddns.example.net", "router", accepted=True)
|
||||||
|
|
||||||
|
with store.connect() as conn:
|
||||||
|
conn.execute(
|
||||||
|
"UPDATE accounts SET created_at = ? WHERE hostname = ?",
|
||||||
|
("2000-01-01T00:00:00+00:00", account["hostname"]),
|
||||||
|
)
|
||||||
|
|
||||||
|
result = store.cleanup(unused_account_hours=1)
|
||||||
|
|
||||||
|
assert result["unused_accounts"] == 1
|
||||||
|
assert store.get_account_by_slug(account["update_slug"]) is None
|
||||||
|
cleanup_events = store.list_update_events_for_hostname(account["hostname"])
|
||||||
|
assert cleanup_events[0]["status"] == CLEANUP_UNUSED_HOSTNAME_DELETED
|
||||||
|
assert cleanup_events[0]["detail"] == "cleanup deleted abandoned hostname"
|
||||||
|
|
||||||
|
|
||||||
def test_pending_generated_setup_stays_pending_after_restart(tmp_path: Path) -> None:
|
def test_pending_generated_setup_stays_pending_after_restart(tmp_path: Path) -> None:
|
||||||
database_path = tmp_path / "ddns.sqlite3"
|
database_path = tmp_path / "ddns.sqlite3"
|
||||||
store = DdnsStore(database_path)
|
store = DdnsStore(database_path)
|
||||||
@@ -389,3 +413,29 @@ def test_scheduled_cleanup_failure_is_logged(tmp_path: Path, monkeypatch, caplog
|
|||||||
assert scheduled_task["last_status"] == "error"
|
assert scheduled_task["last_status"] == "error"
|
||||||
assert "RuntimeError: cleanup exploded" in str(scheduled_task["last_error"])
|
assert "RuntimeError: cleanup exploded" in str(scheduled_task["last_error"])
|
||||||
assert "Scheduled cleanup failed" in caplog.text
|
assert "Scheduled cleanup failed" in caplog.text
|
||||||
|
|
||||||
|
|
||||||
|
def test_scheduler_loop_survives_iteration_exception(tmp_path: Path, monkeypatch, caplog) -> None:
|
||||||
|
settings = DdnsSettings(database_path=tmp_path / "ddns.sqlite3", admin_password="admin", cleanup_interval_seconds=60)
|
||||||
|
store = DdnsStore(settings.database_path)
|
||||||
|
calls = 0
|
||||||
|
|
||||||
|
async def fail_once_then_cancel(inner_store, inner_settings):
|
||||||
|
nonlocal calls
|
||||||
|
calls += 1
|
||||||
|
if calls == 1:
|
||||||
|
raise RuntimeError("post-cleanup crash")
|
||||||
|
raise asyncio.CancelledError
|
||||||
|
|
||||||
|
async def no_sleep(_seconds: float) -> None:
|
||||||
|
return None
|
||||||
|
|
||||||
|
caplog.set_level(logging.ERROR, logger="karldns.app")
|
||||||
|
monkeypatch.setattr(app_module, "_run_cleanup_if_due", fail_once_then_cancel)
|
||||||
|
monkeypatch.setattr(app_module.asyncio, "sleep", no_sleep)
|
||||||
|
|
||||||
|
with pytest.raises(asyncio.CancelledError):
|
||||||
|
asyncio.run(app_module._scheduler_loop(store, settings))
|
||||||
|
|
||||||
|
assert calls == 2
|
||||||
|
assert "Scheduled cleanup loop crashed; continuing" in caplog.text
|
||||||
Reference in new issue
Block a user