lukas.pupkalipinski/BanGUI
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor backend services and jail configuration

Browse Source 
- Refactor action_config_service, filter_config_service, jail_config_service, and jail_service
- Add jail_socket utility module for socket communication
- Update test_jail_service with new test cases
- Update architecture and task documentation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Lukas
2026-04-25 18:34:03 +02:00
parent c3410bd554
commit 83452ffc23
8 changed files with 168 additions and 142 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
Docs/Architekture.md
View File

@@ -252,6 +252,7 @@ Pure helper modules with no framework dependencies.
| Module | Purpose |
|---|---|
| `fail2ban_client.py` | Async client that communicates with fail2ban via its Unix domain socket — sends commands and parses responses using the fail2ban protocol. Modelled after [`./fail2ban-master/fail2ban/client/csocket.py`](../fail2ban-master/fail2ban/client/csocket.py) and [`./fail2ban-master/fail2ban/client/fail2banclient.py`](../fail2ban-master/fail2ban/client/fail2banclient.py). |
| `jail_socket.py` | Low-level jail reload operations (`reload_all`) extracted to break service dependencies. Used by `jail_service`, `jail_config_service`, `action_config_service`, and `filter_config_service` to avoid circular imports between sibling services. |
| `ip_utils.py` | Validates IPv4/IPv6 addresses and CIDR ranges using the `ipaddress` stdlib module, normalises formats |
| `jail_utils.py` | Jail helper functions for configuration and status inference |
| `jail_config.py` | Jail config parser and serializer for fail2ban config manipulation |
@@ -774,6 +775,7 @@ These principles govern all architectural decisions in BanGUI.
| Principle | Application |
|---|---|
| **Separation of Concerns** | Frontend and backend are independent. Backend layers (router → service → repository) never mix responsibilities. |
| **Service Independence** | Services must not import other services at the same layer (e.g., `jail_config_service` must not import `jail_service`). Shared logic belongs in the utils layer (`app/utils/`). This prevents circular dependencies, improves testability, and keeps each service focused on its domain. |
| **Single Responsibility** | Each module, service, and component has one well-defined job. |
| **Dependency Inversion** | Services depend on abstractions (protocols), not concrete implementations. FastAPI `Depends()` wires everything. |
| **Async Everything** | All I/O is non-blocking. No synchronous database, HTTP, or socket calls anywhere in the backend. |

23
Docs/Tasks.md
View File

@@ -1,26 +1,3 @@
### T-06 · Eliminate `AppState` Protocol / `ApplicationContext` dataclass redundancy
**Where found:** `backend/app/dependencies.py` — `AppState` Protocol (lines ~40–60) and `ApplicationContext` dataclass (lines ~62–75) describe identical fields.
**Why this is needed:** Every new field must be added to both. The Protocol is only used for a single `cast()` call inside `_build_app_context`. Maintenance burden with no benefit.
**Goal:** One typed representation. Remove `AppState` Protocol; cast directly or use `ApplicationContext`.
**What to do:**
1. Delete the `AppState` Protocol class.
2. Replace `state = cast("AppState", request.app.state)` with `state = request.app.state` (type: ignore or use `ApplicationState` directly since it's the concrete type set in `create_app`).
3. Access fields from `state` directly using the `ApplicationState` / `RuntimeState` types.
**Possible traps and issues:**
- `mypy` / pyright may report type errors on `request.app.state` accesses — use `cast(ApplicationState, request.app.state)` once, then access typed.
- Ensure all fields accessed in `_build_app_context` are present on `ApplicationState`.
**Docs changes needed:** None.
**Doc references:** `backend/app/dependencies.py`, `backend/app/utils/runtime_state.py`
---
### T-07 · Break cross-service import: `jail_config_service` imports `jail_service`
**Where found:** `backend/app/services/jail_config_service.py` — `import app.services.jail_service as jail_service`

26
backend/app/services/action_config_service.py
View File

@@ -25,13 +25,6 @@ from app.exceptions import (
ConfigWriteError,
JailNotFoundInConfigError,
)
import app.services.jail_service as jail_service
from app.utils.config_file_utils import (
_get_active_jail_names as _config_file_get_active_jail_names,
_parse_jails_sync as _config_file_parse_jails_sync,
_safe_jail_name,
build_parser,
)
from app.models.config import (
ActionConfig,
ActionConfigUpdate,
@@ -42,6 +35,17 @@ from app.models.config import (
)
from app.utils import conffile_parser
from app.utils.async_utils import run_blocking
from app.utils.config_file_utils import (
_get_active_jail_names as _config_file_get_active_jail_names,
)
from app.utils.config_file_utils import (
_parse_jails_sync as _config_file_parse_jails_sync,
)
from app.utils.config_file_utils import (
_safe_jail_name,
build_parser,
)
from app.utils.jail_socket import reload_all
log: structlog.stdlib.BoundLogger = structlog.get_logger()
@@ -681,7 +685,7 @@ async def update_action(
if do_reload:
try:
await jail_service.reload_all(socket_path)
await reload_all(socket_path)
except Exception as exc: # noqa: BLE001
log.warning(
"reload_after_action_update_failed",
@@ -749,7 +753,7 @@ async def create_action(
if do_reload:
try:
await jail_service.reload_all(socket_path)
await reload_all(socket_path)
except Exception as exc: # noqa: BLE001
log.warning(
"reload_after_action_create_failed",
@@ -874,7 +878,7 @@ async def assign_action_to_jail(
if do_reload:
try:
await jail_service.reload_all(socket_path)
await reload_all(socket_path)
except Exception as exc: # noqa: BLE001
log.warning(
"reload_after_assign_action_failed",
@@ -932,7 +936,7 @@ async def remove_action_from_jail(
if do_reload:
try:
await jail_service.reload_all(socket_path)
await reload_all(socket_path)
except Exception as exc: # noqa: BLE001
log.warning(
"reload_after_remove_action_failed",

26
backend/app/services/filter_config_service.py
View File

@@ -23,14 +23,6 @@ from app.exceptions import (
FilterReadonlyError,
JailNotFoundInConfigError,
)
import app.services.jail_service as jail_service
from app.utils.config_file_utils import (
_get_active_jail_names as _config_file_get_active_jail_names,
_parse_jails_sync as _config_file_parse_jails_sync,
_safe_filter_name,
_safe_jail_name,
set_jail_local_key_sync,
)
from app.models.config import (
AssignFilterRequest,
FilterConfig,
@@ -41,6 +33,18 @@ from app.models.config import (
)
from app.utils import conffile_parser
from app.utils.async_utils import run_blocking
from app.utils.config_file_utils import (
_get_active_jail_names as _config_file_get_active_jail_names,
)
from app.utils.config_file_utils import (
_parse_jails_sync as _config_file_parse_jails_sync,
)
from app.utils.config_file_utils import (
_safe_filter_name,
_safe_jail_name,
set_jail_local_key_sync,
)
from app.utils.jail_socket import reload_all
log: structlog.stdlib.BoundLogger = structlog.get_logger()
@@ -508,7 +512,7 @@ async def update_filter(
if do_reload:
try:
await jail_service.reload_all(socket_path)
await reload_all(socket_path)
except Exception as exc: # noqa: BLE001
log.warning(
"reload_after_filter_update_failed",
@@ -582,7 +586,7 @@ async def create_filter(
if do_reload:
try:
await jail_service.reload_all(socket_path)
await reload_all(socket_path)
except Exception as exc: # noqa: BLE001
log.warning(
"reload_after_filter_create_failed",
@@ -704,7 +708,7 @@ async def assign_filter_to_jail(
if do_reload:
try:
await jail_service.reload_all(socket_path)
await reload_all(socket_path)
except Exception as exc: # noqa: BLE001
log.warning(
"reload_after_assign_filter_failed",

35
backend/app/services/jail_config_service.py
View File

@@ -25,17 +25,6 @@ from app.exceptions import (
JailNotFoundError,
JailNotFoundInConfigError,
)
import app.services.jail_service as jail_service
from app.utils.config_file_utils import (
_build_inactive_jail,
_parse_jails_sync as _config_file_parse_jails_sync,
_get_active_jail_names as _config_file_get_active_jail_names,
_probe_fail2ban_running,
_safe_jail_name,
_validate_jail_config_sync as _config_file_validate_jail_config_sync,
start_daemon,
wait_for_fail2ban,
)
from app.models.config import (
ActivateJailRequest,
InactiveJail,
@@ -46,7 +35,23 @@ from app.models.config import (
)
from app.services import health_service
from app.utils.async_utils import run_blocking
from app.utils.fail2ban_client import Fail2BanClient
from app.utils.config_file_utils import (
_build_inactive_jail,
_probe_fail2ban_running,
_safe_jail_name,
start_daemon,
wait_for_fail2ban,
)
from app.utils.config_file_utils import (
_get_active_jail_names as _config_file_get_active_jail_names,
)
from app.utils.config_file_utils import (
_parse_jails_sync as _config_file_parse_jails_sync,
)
from app.utils.config_file_utils import (
_validate_jail_config_sync as _config_file_validate_jail_config_sync,
)
from app.utils.jail_socket import reload_all
log: structlog.stdlib.BoundLogger = structlog.get_logger()
@@ -404,7 +409,7 @@ async def _activate_jail(
# Activation reload — if it fails, roll back immediately #
# ---------------------------------------------------------------------- #
try:
await jail_service.reload_all(socket_path, include_jails=[name])
await reload_all(socket_path, include_jails=[name])
except JailNotFoundError as exc:
# Jail configuration is invalid (e.g. missing logpath that prevents
# fail2ban from loading the jail). Roll back and provide a specific error.
@@ -546,7 +551,7 @@ async def _rollback_activation_async(
# Step 2 — reload fail2ban with the restored config.
try:
await jail_service.reload_all(socket_path)
await reload_all(socket_path)
log.info("jail_activation_rollback_reload_ok", jail=name)
except Exception as exc: # noqa: BLE001
log.warning("jail_activation_rollback_reload_failed", jail=name, error=str(exc))
@@ -626,7 +631,7 @@ async def _deactivate_jail(
)
try:
await jail_service.reload_all(socket_path, exclude_jails=[name])
await reload_all(socket_path, exclude_jails=[name])
except Exception as exc: # noqa: BLE001
log.warning("reload_after_deactivate_failed", jail=name, error=str(exc))

82
backend/app/services/jail_service.py
View File

@@ -36,7 +36,6 @@ from app.utils.fail2ban_client import (
Fail2BanCommand,
Fail2BanConnectionError,
Fail2BanResponse,
Fail2BanToken,
)
from app.utils.fail2ban_response import (
ensure_list,
@@ -44,6 +43,7 @@ from app.utils.fail2ban_response import (
ok,
to_dict,
)
from app.utils.jail_socket import reload_all
if TYPE_CHECKING:
from collections.abc import Awaitable
@@ -55,6 +55,8 @@ if TYPE_CHECKING:
log: structlog.stdlib.BoundLogger = structlog.get_logger()
__all__ = ["reload_all"]
class IpLookupResult(TypedDict):
"""Result returned by :func:`lookup_ip`.
@@ -73,12 +75,6 @@ class IpLookupResult(TypedDict):
_SOCKET_TIMEOUT: float = 10.0
# Guard against concurrent reload_all calls. Overlapping ``reload --all``
# commands sent to fail2ban's socket produce undefined behaviour and may cause
# jails to be permanently removed from the daemon. Serialising them here
# ensures only one reload stream is in-flight at a time.
_reload_all_lock: asyncio.Lock | None = None
# Capability detection for optional fail2ban transmitter commands (backend, idle).
# These commands are not supported in all fail2ban versions. Caching the result
# avoids sending unsupported commands every polling cycle and spamming the
@@ -87,19 +83,6 @@ _backend_cmd_supported: bool | None = None
_backend_cmd_lock: asyncio.Lock | None = None
def _get_reload_all_lock() -> asyncio.Lock:
"""Return the shared reload-all lock, initialising it lazily.
Asyncio primitives must be created inside an active event loop in test
environments that create new loops per test. Lazily initialising the lock
avoids binding it to the import-time loop.
"""
global _reload_all_lock
if _reload_all_lock is None:
_reload_all_lock = asyncio.Lock()
return _reload_all_lock
def _get_backend_cmd_lock() -> asyncio.Lock:
"""Return the shared backend capability probe lock, initialising it lazily.
@@ -605,65 +588,6 @@ async def reload_jail(socket_path: str, name: str) -> None:
raise JailOperationError(str(exc)) from exc
async def reload_all(
socket_path: str,
*,
include_jails: list[str] | None = None,
exclude_jails: list[str] | None = None,
) -> None:
"""Reload all fail2ban jails at once.
Fetches the current jail list first so that a ``['start', name]`` entry
can be included in the config stream for every active jail. Without a
non-empty stream the end-of-reload phase deletes every jail that received
no configuration commands.
*include_jails* are added to the stream (e.g. a newly activated jail that
is not yet running). *exclude_jails* are removed from the stream (e.g. a
jail that was just deactivated and should not be restarted).
Args:
socket_path: Path to the fail2ban Unix domain socket.
include_jails: Extra jail names to add to the start stream.
exclude_jails: Jail names to remove from the start stream.
Raises:
JailNotFoundError: If a jail in *include_jails* does not exist or
its configuration is invalid (e.g. missing logpath).
JailOperationError: If fail2ban reports the operation failed for
a different reason.
~app.utils.fail2ban_client.Fail2BanConnectionError: If the socket
cannot be reached.
"""
client = Fail2BanClient(socket_path=socket_path, timeout=_SOCKET_TIMEOUT)
async with _get_reload_all_lock():
try:
# Resolve jail names so we can build the minimal config stream.
status_raw = ok(await client.send(["status"]))
status_dict = to_dict(status_raw)
jail_list_raw: str = str(status_dict.get("Jail list", ""))
jail_names = [n.strip() for n in jail_list_raw.split(",") if n.strip()]
# Merge include/exclude sets so the stream matches the desired state.
names_set: set[str] = set(jail_names)
if include_jails:
names_set.update(include_jails)
if exclude_jails:
names_set -= set(exclude_jails)
stream: list[list[object]] = [["start", n] for n in sorted(names_set)]
ok(await client.send(["reload", "--all", [], cast("Fail2BanToken", stream)]))
log.info("all_jails_reloaded")
except ValueError as exc:
# Detect UnknownJailException (missing or invalid jail configuration)
# and re-raise as JailNotFoundError for better error specificity.
if is_not_found_error(exc):
# Extract the jail name from include_jails if available.
jail_name = include_jails[0] if include_jails else "unknown"
raise JailNotFoundError(jail_name) from exc
raise JailOperationError(str(exc)) from exc
async def restart(socket_path: str) -> None:
"""Stop the fail2ban daemon via the Unix socket.

108
backend/app/utils/jail_socket.py Normal file
View File

@@ -0,0 +1,108 @@
"""Low-level socket operations for jail management.
Provides shared socket utilities for reloading jails across all services.
These operations are extracted to a utility layer to avoid circular dependencies
between sibling services (jail_service, jail_config_service, action_config_service,
filter_config_service).
"""
from __future__ import annotations
import asyncio
from typing import cast
import structlog
from app.exceptions import JailNotFoundError, JailOperationError
from app.utils.fail2ban_client import (
Fail2BanClient,
Fail2BanToken,
)
from app.utils.fail2ban_response import (
is_not_found_error,
ok,
to_dict,
)
log: structlog.stdlib.BoundLogger = structlog.get_logger()
# Socket communication timeout in seconds.
SOCKET_TIMEOUT: float = 10.0
# Guard against concurrent reload_all calls. Overlapping ``reload --all``
# commands sent to fail2ban's socket produce undefined behaviour and may cause
# jails to be permanently removed from the daemon. Serialising them here
# ensures only one reload stream is in-flight at a time.
_reload_all_lock: asyncio.Lock | None = None
def _get_reload_all_lock() -> asyncio.Lock:
"""Return the shared reload-all lock, initialising it lazily.
Asyncio primitives must be created inside an active event loop in test
environments that create new loops per test. Lazily initialising the lock
avoids binding it to the import-time loop.
"""
global _reload_all_lock
if _reload_all_lock is None:
_reload_all_lock = asyncio.Lock()
return _reload_all_lock
async def reload_all(
socket_path: str,
*,
include_jails: list[str] | None = None,
exclude_jails: list[str] | None = None,
) -> None:
"""Reload all fail2ban jails at once.
Fetches the current jail list first so that a ``['start', name]`` entry
can be included in the config stream for every active jail. Without a
non-empty stream the end-of-reload phase deletes every jail that received
no configuration commands.
*include_jails* are added to the stream (e.g. a newly activated jail that
is not yet running). *exclude_jails* are removed from the stream (e.g. a
jail that was just deactivated and should not be restarted).
Args:
socket_path: Path to the fail2ban Unix domain socket.
include_jails: Extra jail names to add to the start stream.
exclude_jails: Jail names to remove from the start stream.
Raises:
JailNotFoundError: If a jail in *include_jails* does not exist or
its configuration is invalid (e.g. missing logpath).
JailOperationError: If fail2ban reports the operation failed for
a different reason.
~app.utils.fail2ban_client.Fail2BanConnectionError: If the socket
cannot be reached.
"""
client = Fail2BanClient(socket_path=socket_path, timeout=SOCKET_TIMEOUT)
async with _get_reload_all_lock():
try:
# Resolve jail names so we can build the minimal config stream.
status_raw = ok(await client.send(["status"]))
status_dict = to_dict(status_raw)
jail_list_raw: str = str(status_dict.get("Jail list", ""))
jail_names = [n.strip() for n in jail_list_raw.split(",") if n.strip()]
# Merge include/exclude sets so the stream matches the desired state.
names_set: set[str] = set(jail_names)
if include_jails:
names_set.update(include_jails)
if exclude_jails:
names_set -= set(exclude_jails)
stream: list[list[object]] = [["start", n] for n in sorted(names_set)]
ok(await client.send(["reload", "--all", [], cast("Fail2BanToken", stream)]))
log.info("all_jails_reloaded")
except ValueError as exc:
# Detect UnknownJailException (missing or invalid jail configuration)
# and re-raise as JailNotFoundError for better error specificity.
if is_not_found_error(exc):
# Extract the jail name from include_jails if available.
jail_name = include_jails[0] if include_jails else "unknown"
raise JailNotFoundError(jail_name) from exc
raise JailOperationError(str(exc)) from exc

8
backend/tests/test_services/test_jail_service.py
View File

@@ -15,6 +15,7 @@ from app.models.geo import GeoDetail, GeoInfo
from app.models.jail import JailDetailResponse, JailListResponse
from app.services import ban_service, jail_service
from app.services.jail_service import JailNotFoundError, JailOperationError
from app.utils import jail_socket
# ---------------------------------------------------------------------------
# Helpers
@@ -75,6 +76,7 @@ def _patch_client(responses: dict[str, Any]) -> Any:
stack = contextlib.ExitStack()
stack.enter_context(patch("app.services.jail_service.Fail2BanClient", _FakeClient))
stack.enter_context(patch("app.services.ban_service.Fail2BanClient", _FakeClient))
stack.enter_context(patch("app.utils.jail_socket.Fail2BanClient", _FakeClient))
return stack
@@ -281,12 +283,12 @@ class TestLockInitialization:
async def test_reload_all_lock_is_lazy_initialised(self) -> None:
"""The reload-all lock should be created lazily on first use."""
jail_service._reload_all_lock = None
jail_socket._reload_all_lock = None
lock = _ = jail_service._get_reload_all_lock()
lock = _ = jail_socket._get_reload_all_lock()
assert isinstance(lock, asyncio.Lock)
assert jail_service._reload_all_lock is lock
assert jail_socket._reload_all_lock is lock
async def test_backend_cmd_lock_is_lazy_initialised(self) -> None:
"""The backend capability probe lock should be created lazily on first use."""