Refactor config router to use explicit dependency injection

This commit is contained in:
2026-04-06 21:11:02 +02:00
parent ca4b0ed324
commit 0a70e40d8b
2 changed files with 71 additions and 54 deletions

View File

@@ -10,6 +10,8 @@ Reference: `Docs/Refactoring.md` for full analysis of each issue.
### Backend Architecture Review Findings
- Status: **done**`backend/app/routers/config.py` now uses explicit dependency injection for fail2ban settings and no longer reads `request.app.state.settings` directly.
- `backend/app/routers/*` often reads config directly from `request.app.state.settings` instead of using dependency injection. This bypasses the dependency layer and creates hidden coupling between routers and application state.
- Fix: replace direct `request.app.state.settings` access with `SettingsDep` or other explicit dependencies such as `ServerStatusDep` and `PendingRecoveryDep` in router function signatures.
- Expected outcome: routers become easier to unit test, composition is more explicit, and shared state access is only available through documented FastAPI dependencies.

View File

@@ -43,7 +43,15 @@ from typing import Annotated
import structlog
from fastapi import APIRouter, HTTPException, Path, Query, Request, status
from app.dependencies import AuthDep, DbDep
from app.dependencies import (
AuthDep,
DbDep,
Fail2BanConfigDirDep,
Fail2BanSocketDep,
Fail2BanStartCommandDep,
PendingRecoveryDep,
)
from app.exceptions import ConfigOperationError, ConfigValidationError, JailNotFoundError, JailOperationError
from app.models.config import (
ActionConfig,
ActionCreateRequest,
@@ -76,12 +84,14 @@ from app.models.config import (
RollbackResponse,
ServiceStatusResponse,
)
from app.services import config_service, jail_service, log_service
from app.services import (
action_config_service,
config_file_service,
config_service,
filter_config_service,
jail_config_service,
jail_service,
log_service,
)
from app.services.action_config_service import (
ActionAlreadyExistsError,
@@ -103,7 +113,6 @@ from app.services.jail_config_service import (
JailNameError,
JailNotFoundInConfigError,
)
from app.exceptions import ConfigOperationError, ConfigValidationError, JailNotFoundError, JailOperationError
from app.tasks.health_check import _run_probe
from app.utils.fail2ban_client import Fail2BanConnectionError
@@ -159,6 +168,7 @@ def _bad_request(message: str) -> HTTPException:
async def get_jail_configs(
request: Request,
_auth: AuthDep,
socket_path: Fail2BanSocketDep,
) -> JailConfigListResponse:
"""Return editable configuration for every active fail2ban jail.
@@ -172,7 +182,6 @@ async def get_jail_configs(
Returns:
:class:`~app.models.config.JailConfigListResponse`.
"""
socket_path: str = request.app.state.settings.fail2ban_socket
try:
return await config_service.list_jail_configs(socket_path)
except Fail2BanConnectionError as exc:
@@ -187,6 +196,8 @@ async def get_jail_configs(
async def get_inactive_jails(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
socket_path: Fail2BanSocketDep,
) -> InactiveJailListResponse:
"""Return all jails defined in fail2ban config files that are not running.
@@ -201,8 +212,6 @@ async def get_inactive_jails(
Returns:
:class:`~app.models.config.InactiveJailListResponse`.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
socket_path: str = request.app.state.settings.fail2ban_socket
return await jail_config_service.list_inactive_jails(config_dir, socket_path)
@@ -214,6 +223,7 @@ async def get_inactive_jails(
async def get_jail_config(
request: Request,
_auth: AuthDep,
socket_path: Fail2BanSocketDep,
name: _NamePath,
) -> JailConfigResponse:
"""Return the full editable configuration for one fail2ban jail.
@@ -230,7 +240,6 @@ async def get_jail_config(
HTTPException: 404 when the jail does not exist.
HTTPException: 502 when fail2ban is unreachable.
"""
socket_path: str = request.app.state.settings.fail2ban_socket
try:
return await config_service.get_jail_config(socket_path, name)
except JailNotFoundError:
@@ -247,6 +256,7 @@ async def get_jail_config(
async def update_jail_config(
request: Request,
_auth: AuthDep,
socket_path: Fail2BanSocketDep,
name: _NamePath,
body: JailConfigUpdate,
) -> None:
@@ -267,7 +277,6 @@ async def update_jail_config(
HTTPException: 400 when a set command is rejected.
HTTPException: 502 when fail2ban is unreachable.
"""
socket_path: str = request.app.state.settings.fail2ban_socket
try:
await config_service.update_jail_config(socket_path, name, body)
except JailNotFoundError:
@@ -293,6 +302,7 @@ async def update_jail_config(
async def get_global_config(
request: Request,
_auth: AuthDep,
socket_path: Fail2BanSocketDep,
) -> GlobalConfigResponse:
"""Return global fail2ban settings (log level, log target, database config).
@@ -306,7 +316,6 @@ async def get_global_config(
Raises:
HTTPException: 502 when fail2ban is unreachable.
"""
socket_path: str = request.app.state.settings.fail2ban_socket
try:
return await config_service.get_global_config(socket_path)
except Fail2BanConnectionError as exc:
@@ -321,6 +330,7 @@ async def get_global_config(
async def update_global_config(
request: Request,
_auth: AuthDep,
socket_path: Fail2BanSocketDep,
body: GlobalConfigUpdate,
) -> None:
"""Update global fail2ban settings.
@@ -334,7 +344,6 @@ async def update_global_config(
HTTPException: 400 when a set command is rejected.
HTTPException: 502 when fail2ban is unreachable.
"""
socket_path: str = request.app.state.settings.fail2ban_socket
try:
await config_service.update_global_config(socket_path, body)
except ConfigOperationError as exc:
@@ -356,6 +365,7 @@ async def update_global_config(
async def reload_fail2ban(
request: Request,
_auth: AuthDep,
socket_path: Fail2BanSocketDep,
) -> None:
"""Trigger a full fail2ban reload.
@@ -369,7 +379,6 @@ async def reload_fail2ban(
HTTPException: 409 when fail2ban reports the reload failed.
HTTPException: 502 when fail2ban is unreachable.
"""
socket_path: str = request.app.state.settings.fail2ban_socket
try:
await jail_service.reload_all(socket_path)
except JailOperationError as exc:
@@ -393,6 +402,8 @@ async def reload_fail2ban(
async def restart_fail2ban(
request: Request,
_auth: AuthDep,
socket_path: Fail2BanSocketDep,
start_cmd: Fail2BanStartCommandDep,
) -> None:
"""Trigger a full fail2ban service restart.
@@ -414,8 +425,6 @@ async def restart_fail2ban(
``POST /api/config/jails/{name}/rollback`` if a specific jail
is suspect.
"""
socket_path: str = request.app.state.settings.fail2ban_socket
start_cmd: str = request.app.state.settings.fail2ban_start_command
start_cmd_parts: list[str] = start_cmd.split()
# Step 1: stop the daemon via socket.
@@ -488,6 +497,7 @@ async def regex_test(
async def add_log_path(
request: Request,
_auth: AuthDep,
socket_path: Fail2BanSocketDep,
name: _NamePath,
body: AddLogPathRequest,
) -> None:
@@ -507,7 +517,6 @@ async def add_log_path(
HTTPException: 400 when the command is rejected.
HTTPException: 502 when fail2ban is unreachable.
"""
socket_path: str = request.app.state.settings.fail2ban_socket
try:
await config_service.add_log_path(socket_path, name, body)
except JailNotFoundError:
@@ -526,6 +535,7 @@ async def add_log_path(
async def delete_log_path(
request: Request,
_auth: AuthDep,
socket_path: Fail2BanSocketDep,
name: _NamePath,
log_path: str = Query(..., description="Absolute path of the log file to stop monitoring."),
) -> None:
@@ -545,7 +555,6 @@ async def delete_log_path(
HTTPException: 400 when the command is rejected.
HTTPException: 502 when fail2ban is unreachable.
"""
socket_path: str = request.app.state.settings.fail2ban_socket
try:
await config_service.delete_log_path(socket_path, name, log_path)
except JailNotFoundError:
@@ -669,6 +678,8 @@ async def update_map_color_thresholds(
async def activate_jail(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
socket_path: Fail2BanSocketDep,
name: _NamePath,
body: ActivateJailRequest | None = None,
) -> JailActivationResponse:
@@ -694,8 +705,6 @@ async def activate_jail(
HTTPException: 409 if the jail is already active.
HTTPException: 502 if fail2ban is unreachable.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
socket_path: str = request.app.state.settings.fail2ban_socket
req = body if body is not None else ActivateJailRequest()
try:
@@ -748,6 +757,8 @@ async def activate_jail(
async def deactivate_jail(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
socket_path: Fail2BanSocketDep,
name: _NamePath,
) -> JailActivationResponse:
"""Disable an active jail and reload fail2ban.
@@ -769,8 +780,6 @@ async def deactivate_jail(
HTTPException: 409 if the jail is already inactive.
HTTPException: 502 if fail2ban is unreachable.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
socket_path: str = request.app.state.settings.fail2ban_socket
try:
result = await jail_config_service.deactivate_jail(config_dir, socket_path, name)
@@ -807,6 +816,8 @@ async def deactivate_jail(
async def delete_jail_local_override(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
socket_path: Fail2BanSocketDep,
name: _NamePath,
) -> None:
"""Remove the ``jail.d/{name}.local`` override file for an inactive jail.
@@ -828,8 +839,6 @@ async def delete_jail_local_override(
HTTPException: 500 if the file cannot be deleted.
HTTPException: 502 if fail2ban is unreachable.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
socket_path: str = request.app.state.settings.fail2ban_socket
try:
await jail_config_service.delete_jail_local_override(config_dir, socket_path, name)
@@ -864,6 +873,7 @@ async def delete_jail_local_override(
async def validate_jail(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
name: _NamePath,
) -> JailValidationResult:
"""Run pre-activation validation checks on a jail configuration.
@@ -883,7 +893,6 @@ async def validate_jail(
HTTPException: 400 if *name* contains invalid characters.
HTTPException: 404 if *name* is not found in any config file.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
try:
return await jail_config_service.validate_jail_config(config_dir, name)
except JailNameError as exc:
@@ -896,8 +905,8 @@ async def validate_jail(
summary="Return active crash-recovery record if one exists",
)
async def get_pending_recovery(
request: Request,
_auth: AuthDep,
pending_recovery: PendingRecoveryDep,
) -> PendingRecovery | None:
"""Return the current :class:`~app.models.config.PendingRecovery` record.
@@ -912,7 +921,7 @@ async def get_pending_recovery(
Returns:
:class:`~app.models.config.PendingRecovery` or ``None``.
"""
return getattr(request.app.state, "pending_recovery", None)
return pending_recovery
@router.post(
@@ -923,6 +932,9 @@ async def get_pending_recovery(
async def rollback_jail(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
socket_path: Fail2BanSocketDep,
start_cmd: Fail2BanStartCommandDep,
name: _NamePath,
) -> RollbackResponse:
"""Disable the specified jail and attempt to restart fail2ban.
@@ -945,9 +957,6 @@ async def rollback_jail(
HTTPException: 400 if *name* contains invalid characters.
HTTPException: 500 if writing the .local override file fails.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
socket_path: str = request.app.state.settings.fail2ban_socket
start_cmd: str = request.app.state.settings.fail2ban_start_command
start_cmd_parts: list[str] = start_cmd.split()
try:
@@ -981,6 +990,8 @@ async def rollback_jail(
async def list_filters(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
socket_path: Fail2BanSocketDep,
) -> FilterListResponse:
"""Return all filters discovered in ``filter.d/`` with active/inactive status.
@@ -1001,8 +1012,6 @@ async def list_filters(
:class:`~app.models.config.FilterListResponse` with all discovered
filters.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
socket_path: str = request.app.state.settings.fail2ban_socket
result = await filter_config_service.list_filters(config_dir, socket_path)
# Sort: active first (by name), then inactive (by name).
result.filters.sort(key=lambda f: (not f.active, f.name.lower()))
@@ -1017,6 +1026,8 @@ async def list_filters(
async def get_filter(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
socket_path: Fail2BanSocketDep,
name: Annotated[str, Path(description="Filter base name, e.g. ``sshd`` or ``sshd.conf``.")],
) -> FilterConfig:
"""Return the full parsed configuration and active/inactive status for one filter.
@@ -1037,8 +1048,6 @@ async def get_filter(
HTTPException: 404 if the filter is not found in ``filter.d/``.
HTTPException: 502 if fail2ban is unreachable.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
socket_path: str = request.app.state.settings.fail2ban_socket
try:
return await filter_config_service.get_filter(config_dir, socket_path, name)
except FilterNotFoundError:
@@ -1074,6 +1083,8 @@ def _filter_not_found(name: str) -> HTTPException:
async def update_filter(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
socket_path: Fail2BanSocketDep,
name: _FilterNamePath,
body: FilterUpdateRequest,
reload: bool = Query(default=False, description="Reload fail2ban after writing."),
@@ -1101,8 +1112,6 @@ async def update_filter(
HTTPException: 422 if any regex pattern fails to compile.
HTTPException: 500 if writing the ``.local`` file fails.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
socket_path: str = request.app.state.settings.fail2ban_socket
try:
return await filter_config_service.update_filter(config_dir, socket_path, name, body, do_reload=reload)
except FilterNameError as exc:
@@ -1127,6 +1136,8 @@ async def update_filter(
async def create_filter(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
socket_path: Fail2BanSocketDep,
body: FilterCreateRequest,
reload: bool = Query(default=False, description="Reload fail2ban after creating."),
) -> FilterConfig:
@@ -1151,8 +1162,6 @@ async def create_filter(
HTTPException: 422 if any regex pattern is invalid.
HTTPException: 500 if writing fails.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
socket_path: str = request.app.state.settings.fail2ban_socket
try:
return await filter_config_service.create_filter(config_dir, socket_path, body, do_reload=reload)
except FilterNameError as exc:
@@ -1179,6 +1188,7 @@ async def create_filter(
async def delete_filter(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
name: _FilterNamePath,
) -> None:
"""Delete a user-created filter's ``.local`` override file.
@@ -1199,7 +1209,6 @@ async def delete_filter(
HTTPException: 409 if the filter is a shipped default (conf-only).
HTTPException: 500 if deletion fails.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
try:
await filter_config_service.delete_filter(config_dir, name)
except FilterNameError as exc:
@@ -1226,6 +1235,8 @@ async def delete_filter(
async def assign_filter_to_jail(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
socket_path: Fail2BanSocketDep,
name: _NamePath,
body: AssignFilterRequest,
reload: bool = Query(default=False, description="Reload fail2ban after assigning."),
@@ -1247,8 +1258,6 @@ async def assign_filter_to_jail(
HTTPException: 404 if the jail or filter does not exist.
HTTPException: 500 if writing fails.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
socket_path: str = request.app.state.settings.fail2ban_socket
try:
await filter_config_service.assign_filter_to_jail(config_dir, socket_path, name, body, do_reload=reload)
except (JailNameError, FilterNameError) as exc:
@@ -1292,6 +1301,8 @@ def _action_not_found(name: str) -> HTTPException:
async def list_actions(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
socket_path: Fail2BanSocketDep,
) -> ActionListResponse:
"""Return all actions discovered in ``action.d/`` with active/inactive status.
@@ -1312,8 +1323,6 @@ async def list_actions(
:class:`~app.models.config.ActionListResponse` with all discovered
actions.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
socket_path: str = request.app.state.settings.fail2ban_socket
result = await action_config_service.list_actions(config_dir, socket_path)
result.actions.sort(key=lambda a: (not a.active, a.name.lower()))
return result
@@ -1327,6 +1336,8 @@ async def list_actions(
async def get_action(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
socket_path: Fail2BanSocketDep,
name: _ActionNamePath,
) -> ActionConfig:
"""Return the full parsed configuration and active/inactive status for one action.
@@ -1346,8 +1357,6 @@ async def get_action(
Raises:
HTTPException: 404 if the action is not found in ``action.d/``.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
socket_path: str = request.app.state.settings.fail2ban_socket
try:
return await action_config_service.get_action(config_dir, socket_path, name)
except ActionNotFoundError:
@@ -1367,6 +1376,8 @@ async def get_action(
async def update_action(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
socket_path: Fail2BanSocketDep,
name: _ActionNamePath,
body: ActionUpdateRequest,
reload: bool = Query(default=False, description="Reload fail2ban after writing."),
@@ -1391,8 +1402,6 @@ async def update_action(
HTTPException: 404 if the action does not exist.
HTTPException: 500 if writing the ``.local`` file fails.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
socket_path: str = request.app.state.settings.fail2ban_socket
try:
return await action_config_service.update_action(config_dir, socket_path, name, body, do_reload=reload)
except ActionNameError as exc:
@@ -1415,6 +1424,8 @@ async def update_action(
async def create_action(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
socket_path: Fail2BanSocketDep,
body: ActionCreateRequest,
reload: bool = Query(default=False, description="Reload fail2ban after creating."),
) -> ActionConfig:
@@ -1437,8 +1448,6 @@ async def create_action(
HTTPException: 409 if the action already exists.
HTTPException: 500 if writing fails.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
socket_path: str = request.app.state.settings.fail2ban_socket
try:
return await action_config_service.create_action(config_dir, socket_path, body, do_reload=reload)
except ActionNameError as exc:
@@ -1463,6 +1472,7 @@ async def create_action(
async def delete_action(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
name: _ActionNamePath,
) -> None:
"""Delete a user-created action's ``.local`` override file.
@@ -1481,7 +1491,6 @@ async def delete_action(
HTTPException: 409 if the action is a shipped default (conf-only).
HTTPException: 500 if deletion fails.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
try:
await action_config_service.delete_action(config_dir, name)
except ActionNameError as exc:
@@ -1508,6 +1517,8 @@ async def delete_action(
async def assign_action_to_jail(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
socket_path: Fail2BanSocketDep,
name: _NamePath,
body: AssignActionRequest,
reload: bool = Query(default=False, description="Reload fail2ban after assigning."),
@@ -1530,8 +1541,6 @@ async def assign_action_to_jail(
HTTPException: 404 if the jail or action does not exist.
HTTPException: 500 if writing fails.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
socket_path: str = request.app.state.settings.fail2ban_socket
try:
await action_config_service.assign_action_to_jail(config_dir, socket_path, name, body, do_reload=reload)
except (JailNameError, ActionNameError) as exc:
@@ -1558,6 +1567,8 @@ async def assign_action_to_jail(
async def remove_action_from_jail(
request: Request,
_auth: AuthDep,
config_dir: Fail2BanConfigDirDep,
socket_path: Fail2BanSocketDep,
name: _NamePath,
action_name: Annotated[str, Path(description="Action base name to remove.")],
reload: bool = Query(default=False, description="Reload fail2ban after removing."),
@@ -1579,10 +1590,14 @@ async def remove_action_from_jail(
HTTPException: 404 if the jail is not found in config files.
HTTPException: 500 if writing fails.
"""
config_dir: str = request.app.state.settings.fail2ban_config_dir
socket_path: str = request.app.state.settings.fail2ban_socket
try:
await action_config_service.remove_action_from_jail(config_dir, socket_path, name, action_name, do_reload=reload)
await action_config_service.remove_action_from_jail(
config_dir,
socket_path,
name,
action_name,
do_reload=reload,
)
except (JailNameError, ActionNameError) as exc:
raise _bad_request(str(exc)) from exc
except JailNotFoundInConfigError:
@@ -1607,6 +1622,7 @@ async def remove_action_from_jail(
async def get_fail2ban_log(
request: Request,
_auth: AuthDep,
socket_path: Fail2BanSocketDep,
lines: Annotated[int, Query(ge=1, le=2000, description="Number of lines to return from the tail.")] = 200,
filter: Annotated[ # noqa: A002
str | None,
@@ -1633,7 +1649,6 @@ async def get_fail2ban_log(
the allowed directory.
HTTPException: 502 when fail2ban is unreachable.
"""
socket_path: str = request.app.state.settings.fail2ban_socket
try:
return await config_service.read_fail2ban_log(socket_path, lines, filter)
except config_service.ConfigOperationError as exc:
@@ -1650,6 +1665,7 @@ async def get_fail2ban_log(
async def get_service_status(
request: Request,
_auth: AuthDep,
socket_path: Fail2BanSocketDep,
) -> ServiceStatusResponse:
"""Return fail2ban service health and current log configuration.
@@ -1667,7 +1683,6 @@ async def get_service_status(
HTTPException: 502 when fail2ban is unreachable (the service itself
handles this gracefully and returns ``online=False``).
"""
socket_path: str = request.app.state.settings.fail2ban_socket
from app.services import health_service
try: