Extract fail2ban restart orchestration into jail_service
This commit is contained in:
@@ -157,6 +157,8 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue.
|
||||
|
||||
**Why this is needed:** Multi-step orchestration workflows are business logic and do not belong in HTTP handler functions. Keeping them in the service layer makes the workflow testable in isolation and reusable if a second endpoint ever needs to restart the daemon.
|
||||
|
||||
**Status:** Completed ✅
|
||||
|
||||
---
|
||||
|
||||
### Task 9 — Move `sign_session_token` call out of `auth.py` router into `auth_service.login()`
|
||||
|
||||
@@ -33,7 +33,6 @@ from app.services import (
|
||||
jail_service,
|
||||
log_service,
|
||||
)
|
||||
from app.utils.config_file_utils import start_daemon, wait_for_fail2ban
|
||||
|
||||
log: structlog.stdlib.BoundLogger = structlog.get_logger()
|
||||
|
||||
@@ -187,9 +186,11 @@ async def restart_fail2ban(
|
||||
"""
|
||||
start_cmd_parts: list[str] = start_cmd.split()
|
||||
|
||||
# Step 1: stop the daemon via socket.
|
||||
try:
|
||||
await jail_service.restart(socket_path)
|
||||
restarted = await jail_service.restart_daemon(
|
||||
socket_path,
|
||||
start_cmd_parts,
|
||||
)
|
||||
except JailOperationError as exc:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_409_CONFLICT,
|
||||
@@ -198,16 +199,7 @@ async def restart_fail2ban(
|
||||
except Fail2BanConnectionError as exc:
|
||||
raise _bad_gateway(exc) from exc
|
||||
|
||||
# Step 2: start the daemon via subprocess.
|
||||
await start_daemon(start_cmd_parts)
|
||||
|
||||
# Step 3: probe the socket until fail2ban is responsive or the budget
|
||||
# expires.
|
||||
fail2ban_running: bool = await wait_for_fail2ban(
|
||||
socket_path,
|
||||
max_wait_seconds=10.0,
|
||||
)
|
||||
if not fail2ban_running:
|
||||
if not restarted:
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_503_SERVICE_UNAVAILABLE,
|
||||
detail=(
|
||||
|
||||
@@ -29,6 +29,7 @@ from app.models.jail import (
|
||||
JailStatus,
|
||||
JailSummary,
|
||||
)
|
||||
from app.utils.config_file_utils import start_daemon, wait_for_fail2ban
|
||||
from app.utils.fail2ban_client import (
|
||||
Fail2BanClient,
|
||||
Fail2BanCommand,
|
||||
@@ -759,6 +760,48 @@ async def restart(socket_path: str) -> None:
|
||||
raise JailOperationError(str(exc)) from exc
|
||||
|
||||
|
||||
async def restart_daemon(
|
||||
socket_path: str,
|
||||
start_cmd_parts: list[str],
|
||||
max_wait_seconds: float = _SOCKET_TIMEOUT,
|
||||
) -> bool:
|
||||
"""Restart the fail2ban daemon and verify it comes back online.
|
||||
|
||||
This function stops the daemon through the socket, starts it with the
|
||||
configured command, and probes the socket until fail2ban accepts status
|
||||
requests again.
|
||||
|
||||
Args:
|
||||
socket_path: Path to the fail2ban Unix domain socket.
|
||||
start_cmd_parts: The configured fail2ban start command split into
|
||||
executable and arguments.
|
||||
max_wait_seconds: The maximum number of seconds to wait for the daemon
|
||||
to become responsive after starting.
|
||||
|
||||
Returns:
|
||||
``True`` when the daemon is started and responsive.
|
||||
``False`` when the command failed or fail2ban never became responsive.
|
||||
|
||||
Raises:
|
||||
JailOperationError: If the stop command failed.
|
||||
~app.utils.fail2ban_client.Fail2BanConnectionError: If the socket
|
||||
cannot be reached while stopping fail2ban.
|
||||
"""
|
||||
await restart(socket_path)
|
||||
|
||||
if not await start_daemon(start_cmd_parts):
|
||||
log.warning(
|
||||
"fail2ban_start_command_failed",
|
||||
command=" ".join(start_cmd_parts),
|
||||
)
|
||||
return False
|
||||
|
||||
return await wait_for_fail2ban(
|
||||
socket_path,
|
||||
max_wait_seconds=max_wait_seconds,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Public API — Ban / Unban
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -406,19 +406,9 @@ class TestRestartFail2ban:
|
||||
|
||||
async def test_204_on_success(self, config_client: AsyncClient) -> None:
|
||||
"""POST /api/config/restart returns 204 when fail2ban restarts cleanly."""
|
||||
with (
|
||||
patch(
|
||||
"app.routers.config_misc.jail_service.restart",
|
||||
AsyncMock(return_value=None),
|
||||
),
|
||||
patch(
|
||||
"app.routers.config_misc.start_daemon",
|
||||
with patch(
|
||||
"app.routers.config_misc.jail_service.restart_daemon",
|
||||
AsyncMock(return_value=True),
|
||||
),
|
||||
patch(
|
||||
"app.routers.config_misc.wait_for_fail2ban",
|
||||
AsyncMock(return_value=True),
|
||||
),
|
||||
):
|
||||
resp = await config_client.post("/api/config/restart")
|
||||
|
||||
@@ -426,19 +416,9 @@ class TestRestartFail2ban:
|
||||
|
||||
async def test_503_when_fail2ban_does_not_come_back(self, config_client: AsyncClient) -> None:
|
||||
"""POST /api/config/restart returns 503 when fail2ban does not come back online."""
|
||||
with (
|
||||
patch(
|
||||
"app.routers.config_misc.jail_service.restart",
|
||||
AsyncMock(return_value=None),
|
||||
),
|
||||
patch(
|
||||
"app.routers.config_misc.start_daemon",
|
||||
AsyncMock(return_value=True),
|
||||
),
|
||||
patch(
|
||||
"app.routers.config_misc.wait_for_fail2ban",
|
||||
with patch(
|
||||
"app.routers.config_misc.jail_service.restart_daemon",
|
||||
AsyncMock(return_value=False),
|
||||
),
|
||||
):
|
||||
resp = await config_client.post("/api/config/restart")
|
||||
|
||||
@@ -449,7 +429,7 @@ class TestRestartFail2ban:
|
||||
from app.services.jail_service import JailOperationError
|
||||
|
||||
with patch(
|
||||
"app.routers.config_misc.jail_service.restart",
|
||||
"app.routers.config_misc.jail_service.restart_daemon",
|
||||
AsyncMock(side_effect=JailOperationError("stop failed")),
|
||||
):
|
||||
resp = await config_client.post("/api/config/restart")
|
||||
@@ -461,33 +441,24 @@ class TestRestartFail2ban:
|
||||
from app.exceptions import Fail2BanConnectionError
|
||||
|
||||
with patch(
|
||||
"app.routers.config_misc.jail_service.restart",
|
||||
"app.routers.config_misc.jail_service.restart_daemon",
|
||||
AsyncMock(side_effect=Fail2BanConnectionError("no socket", "/fake.sock")),
|
||||
):
|
||||
resp = await config_client.post("/api/config/restart")
|
||||
|
||||
assert resp.status_code == 502
|
||||
|
||||
async def test_start_daemon_called_after_stop(self, config_client: AsyncClient) -> None:
|
||||
"""start_daemon is called after a successful stop."""
|
||||
mock_start = AsyncMock(return_value=True)
|
||||
with (
|
||||
patch(
|
||||
"app.routers.config_misc.jail_service.restart",
|
||||
AsyncMock(return_value=None),
|
||||
),
|
||||
patch(
|
||||
"app.routers.config_misc.start_daemon",
|
||||
mock_start,
|
||||
),
|
||||
patch(
|
||||
"app.routers.config_misc.wait_for_fail2ban",
|
||||
AsyncMock(return_value=True),
|
||||
),
|
||||
async def test_service_restart_daemon_called(self, config_client: AsyncClient) -> None:
|
||||
"""The router delegates restart orchestration to jail_service.restart_daemon."""
|
||||
mock_restart = AsyncMock(return_value=True)
|
||||
with patch(
|
||||
"app.routers.config_misc.jail_service.restart_daemon",
|
||||
mock_restart,
|
||||
):
|
||||
await config_client.post("/api/config/restart")
|
||||
resp = await config_client.post("/api/config/restart")
|
||||
|
||||
mock_start.assert_awaited_once()
|
||||
assert resp.status_code == 204
|
||||
mock_restart.assert_awaited_once()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -8,12 +8,12 @@ from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from app.exceptions import Fail2BanConnectionError
|
||||
from app.models.ban import ActiveBanListResponse, JailBannedIpsResponse
|
||||
from app.models.geo import GeoDetail, GeoInfo
|
||||
from app.models.jail import JailDetailResponse, JailListResponse
|
||||
from app.services import jail_service
|
||||
from app.services.jail_service import JailNotFoundError, JailOperationError
|
||||
from app.exceptions import Fail2BanConnectionError
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Helpers
|
||||
@@ -492,6 +492,47 @@ class TestJailControls:
|
||||
):
|
||||
await jail_service.restart(_SOCKET)
|
||||
|
||||
async def test_restart_daemon_returns_true_on_success(self) -> None:
|
||||
"""restart_daemon returns True when stop, start, and probe all succeed."""
|
||||
with (
|
||||
patch("app.services.jail_service.restart", AsyncMock(return_value=None)),
|
||||
patch("app.services.jail_service.start_daemon", AsyncMock(return_value=True)),
|
||||
patch("app.services.jail_service.wait_for_fail2ban", AsyncMock(return_value=True)),
|
||||
):
|
||||
result = await jail_service.restart_daemon(
|
||||
_SOCKET,
|
||||
["fail2ban-client", "start"],
|
||||
)
|
||||
|
||||
assert result is True
|
||||
|
||||
async def test_restart_daemon_returns_false_when_start_fails(self) -> None:
|
||||
"""restart_daemon returns False when the configured start command fails."""
|
||||
with (
|
||||
patch("app.services.jail_service.restart", AsyncMock(return_value=None)),
|
||||
patch("app.services.jail_service.start_daemon", AsyncMock(return_value=False)),
|
||||
):
|
||||
result = await jail_service.restart_daemon(
|
||||
_SOCKET,
|
||||
["fail2ban-client", "start"],
|
||||
)
|
||||
|
||||
assert result is False
|
||||
|
||||
async def test_restart_daemon_returns_false_when_wait_fails(self) -> None:
|
||||
"""restart_daemon returns False when fail2ban does not become responsive."""
|
||||
with (
|
||||
patch("app.services.jail_service.restart", AsyncMock(return_value=None)),
|
||||
patch("app.services.jail_service.start_daemon", AsyncMock(return_value=True)),
|
||||
patch("app.services.jail_service.wait_for_fail2ban", AsyncMock(return_value=False)),
|
||||
):
|
||||
result = await jail_service.restart_daemon(
|
||||
_SOCKET,
|
||||
["fail2ban-client", "start"],
|
||||
)
|
||||
|
||||
assert result is False
|
||||
|
||||
async def test_start_not_found_raises(self) -> None:
|
||||
"""start_jail raises JailNotFoundError for unknown jail."""
|
||||
with _patch_client({"start|ghost": (1, Exception("Unknown jail: 'ghost'"))}), pytest.raises(JailNotFoundError):
|
||||
|
||||
Reference in New Issue
Block a user