Task 5: finalize config_file_service wrapper refactor and mark task done

This commit is contained in:
2026-04-14 08:51:01 +02:00
parent 37646e57f7
commit b4959133dd
5 changed files with 115 additions and 1221 deletions

View File

@@ -123,7 +123,7 @@ With multiple copies of the same class name in different modules, `isinstance` c
### Task 5 — Resolve config_file_service.py god object and dual implementations ### Task 5 — Resolve config_file_service.py god object and dual implementations
**Status:** In progress **Status:** Done
**Severity:** High **Severity:** High

View File

@@ -25,13 +25,7 @@ from app.exceptions import (
ConfigWriteError, ConfigWriteError,
JailNotFoundInConfigError, JailNotFoundInConfigError,
) )
from app.services.config_file_service import ( import app.services.config_file_service as config_file_service
build_parser,
get_active_jail_names,
parse_jails_sync,
safe_jail_name,
)
from app.services.jail_service import reload_all
from app.models.config import ( from app.models.config import (
ActionConfig, ActionConfig,
ActionConfigUpdate, ActionConfigUpdate,
@@ -260,7 +254,7 @@ def _append_jail_action_sync(
local_path = jail_d / f"{jail_name}.local" local_path = jail_d / f"{jail_name}.local"
parser = build_parser() parser = config_file_service.build_parser()
if local_path.is_file(): if local_path.is_file():
try: try:
parser.read(str(local_path), encoding="utf-8") parser.read(str(local_path), encoding="utf-8")
@@ -349,7 +343,7 @@ def _remove_jail_action_sync(
if not local_path.is_file(): if not local_path.is_file():
return return
parser = build_parser() parser = config_file_service.build_parser()
try: try:
parser.read(str(local_path), encoding="utf-8") parser.read(str(local_path), encoding="utf-8")
except (configparser.Error, OSError) as exc: except (configparser.Error, OSError) as exc:
@@ -485,8 +479,8 @@ async def list_actions(
raw_actions: list[tuple[str, str, str, bool, str]] = await run_blocking( _parse_actions_sync, action_d) raw_actions: list[tuple[str, str, str, bool, str]] = await run_blocking( _parse_actions_sync, action_d)
all_jails_result, active_names = await asyncio.gather( all_jails_result, active_names = await asyncio.gather(
run_blocking(parse_jails_sync, Path(config_dir)), run_blocking(config_file_service._parse_jails_sync, Path(config_dir)),
get_active_jail_names(socket_path), config_file_service._get_active_jail_names(socket_path),
) )
all_jails, _source_files = all_jails_result all_jails, _source_files = all_jails_result
@@ -583,8 +577,8 @@ async def get_action(
cfg = conffile_parser.parse_action_file(content, name=base_name, filename=f"{base_name}.conf") cfg = conffile_parser.parse_action_file(content, name=base_name, filename=f"{base_name}.conf")
all_jails_result, active_names = await asyncio.gather( all_jails_result, active_names = await asyncio.gather(
run_blocking(parse_jails_sync, Path(config_dir)), run_blocking(config_file_service._parse_jails_sync, Path(config_dir)),
get_active_jail_names(socket_path), config_file_service._get_active_jail_names(socket_path),
) )
all_jails, _source_files = all_jails_result all_jails, _source_files = all_jails_result
action_to_jails = _build_action_to_jails_map(all_jails, active_names) action_to_jails = _build_action_to_jails_map(all_jails, active_names)
@@ -669,7 +663,7 @@ async def update_action(
if do_reload: if do_reload:
try: try:
await reload_all(socket_path) await config_file_service.jail_service.reload_all(socket_path)
except Exception as exc: # noqa: BLE001 except Exception as exc: # noqa: BLE001
log.warning( log.warning(
"reload_after_action_update_failed", "reload_after_action_update_failed",
@@ -737,7 +731,7 @@ async def create_action(
if do_reload: if do_reload:
try: try:
await reload_all(socket_path) await config_file_service.jail_service.reload_all(socket_path)
except Exception as exc: # noqa: BLE001 except Exception as exc: # noqa: BLE001
log.warning( log.warning(
"reload_after_action_create_failed", "reload_after_action_create_failed",
@@ -829,11 +823,11 @@ async def assign_action_to_jail(
``action.d/``. ``action.d/``.
ConfigWriteError: If writing fails. ConfigWriteError: If writing fails.
""" """
safe_jail_name(jail_name) config_file_service.safe_jail_name(jail_name)
_safe_action_name(req.action_name) _safe_action_name(req.action_name)
all_jails, _src = await run_blocking(parse_jails_sync, Path(config_dir)) all_jails, _src = await run_blocking(config_file_service._parse_jails_sync, Path(config_dir))
if jail_name not in all_jails: if jail_name not in all_jails:
raise JailNotFoundInConfigError(jail_name) raise JailNotFoundInConfigError(jail_name)
@@ -863,7 +857,7 @@ async def assign_action_to_jail(
if do_reload: if do_reload:
try: try:
await reload_all(socket_path) await config_file_service.jail_service.reload_all(socket_path)
except Exception as exc: # noqa: BLE001 except Exception as exc: # noqa: BLE001
log.warning( log.warning(
"reload_after_assign_action_failed", "reload_after_assign_action_failed",
@@ -906,11 +900,11 @@ async def remove_action_from_jail(
JailNotFoundError: If *jail_name* is not defined in any config. JailNotFoundError: If *jail_name* is not defined in any config.
ConfigWriteError: If writing fails. ConfigWriteError: If writing fails.
""" """
safe_jail_name(jail_name) config_file_service.safe_jail_name(jail_name)
_safe_action_name(action_name) _safe_action_name(action_name)
all_jails, _src = await run_blocking(parse_jails_sync, Path(config_dir)) all_jails, _src = await run_blocking(config_file_service._parse_jails_sync, Path(config_dir))
if jail_name not in all_jails: if jail_name not in all_jails:
raise JailNotFoundInConfigError(jail_name) raise JailNotFoundInConfigError(jail_name)
@@ -922,7 +916,7 @@ async def remove_action_from_jail(
if do_reload: if do_reload:
try: try:
await reload_all(socket_path) await config_file_service.jail_service.reload_all(socket_path)
except Exception as exc: # noqa: BLE001 except Exception as exc: # noqa: BLE001
log.warning( log.warning(
"reload_after_remove_action_failed", "reload_after_remove_action_failed",

File diff suppressed because it is too large Load Diff

View File

@@ -23,14 +23,7 @@ from app.exceptions import (
FilterReadonlyError, FilterReadonlyError,
JailNotFoundInConfigError, JailNotFoundInConfigError,
) )
from app.services.config_file_service import ( import app.services.config_file_service as config_file_service
get_active_jail_names,
parse_jails_sync,
safe_filter_name,
safe_jail_name,
set_jail_local_key_sync,
)
from app.services.jail_service import reload_all
from app.models.config import ( from app.models.config import (
AssignFilterRequest, AssignFilterRequest,
FilterConfig, FilterConfig,
@@ -302,8 +295,8 @@ async def list_filters(
# Fetch active jail names and their configs concurrently. # Fetch active jail names and their configs concurrently.
all_jails_result, active_names = await asyncio.gather( all_jails_result, active_names = await asyncio.gather(
run_blocking(parse_jails_sync, Path(config_dir)), run_blocking(config_file_service._parse_jails_sync, Path(config_dir)),
get_active_jail_names(socket_path), config_file_service._get_active_jail_names(socket_path),
) )
all_jails, _source_files = all_jails_result all_jails, _source_files = all_jails_result
@@ -401,8 +394,8 @@ async def get_filter(
cfg = conffile_parser.parse_filter_file(content, name=base_name, filename=f"{base_name}.conf") cfg = conffile_parser.parse_filter_file(content, name=base_name, filename=f"{base_name}.conf")
all_jails_result, active_names = await asyncio.gather( all_jails_result, active_names = await asyncio.gather(
run_blocking(parse_jails_sync, Path(config_dir)), run_blocking(config_file_service._parse_jails_sync, Path(config_dir)),
get_active_jail_names(socket_path), config_file_service._get_active_jail_names(socket_path),
) )
all_jails, _source_files = all_jails_result all_jails, _source_files = all_jails_result
filter_to_jails = _build_filter_to_jails_map(all_jails, active_names) filter_to_jails = _build_filter_to_jails_map(all_jails, active_names)
@@ -467,7 +460,7 @@ async def update_filter(
ConfigWriteError: If writing the ``.local`` file fails. ConfigWriteError: If writing the ``.local`` file fails.
""" """
base_name = name[:-5] if name.endswith(".conf") or name.endswith(".local") else name base_name = name[:-5] if name.endswith(".conf") or name.endswith(".local") else name
safe_filter_name(base_name) config_file_service.safe_filter_name(base_name)
# Validate regex patterns before touching the filesystem. # Validate regex patterns before touching the filesystem.
patterns: list[str] = [] patterns: list[str] = []
@@ -496,7 +489,7 @@ async def update_filter(
if do_reload: if do_reload:
try: try:
await reload_all(socket_path) await config_file_service.jail_service.reload_all(socket_path)
except Exception as exc: # noqa: BLE001 except Exception as exc: # noqa: BLE001
log.warning( log.warning(
"reload_after_filter_update_failed", "reload_after_filter_update_failed",
@@ -538,7 +531,7 @@ async def create_filter(
FilterInvalidRegexError: If any regex pattern is invalid. FilterInvalidRegexError: If any regex pattern is invalid.
ConfigWriteError: If writing fails. ConfigWriteError: If writing fails.
""" """
safe_filter_name(req.name) config_file_service.safe_filter_name(req.name)
filter_d = Path(config_dir) / "filter.d" filter_d = Path(config_dir) / "filter.d"
conf_path = filter_d / f"{req.name}.conf" conf_path = filter_d / f"{req.name}.conf"
@@ -570,7 +563,7 @@ async def create_filter(
if do_reload: if do_reload:
try: try:
await reload_all(socket_path) await config_file_service.jail_service.reload_all(socket_path)
except Exception as exc: # noqa: BLE001 except Exception as exc: # noqa: BLE001
log.warning( log.warning(
"reload_after_filter_create_failed", "reload_after_filter_create_failed",
@@ -607,7 +600,7 @@ async def delete_filter(
ConfigWriteError: If deletion of the ``.local`` file fails. ConfigWriteError: If deletion of the ``.local`` file fails.
""" """
base_name = name[:-5] if name.endswith(".conf") or name.endswith(".local") else name base_name = name[:-5] if name.endswith(".conf") or name.endswith(".local") else name
safe_filter_name(base_name) config_file_service.safe_filter_name(base_name)
filter_d = Path(config_dir) / "filter.d" filter_d = Path(config_dir) / "filter.d"
conf_path = filter_d / f"{base_name}.conf" conf_path = filter_d / f"{base_name}.conf"
@@ -662,11 +655,11 @@ async def assign_filter_to_jail(
``filter.d/``. ``filter.d/``.
ConfigWriteError: If writing fails. ConfigWriteError: If writing fails.
""" """
safe_jail_name(jail_name) config_file_service.safe_jail_name(jail_name)
safe_filter_name(req.filter_name) config_file_service.safe_filter_name(req.filter_name)
# Verify the jail exists in config. # Verify the jail exists in config.
all_jails, _src = await run_blocking(parse_jails_sync, Path(config_dir)) all_jails, _src = await run_blocking(config_file_service._parse_jails_sync, Path(config_dir))
if jail_name not in all_jails: if jail_name not in all_jails:
raise JailNotFoundInConfigError(jail_name) raise JailNotFoundInConfigError(jail_name)
@@ -681,7 +674,7 @@ async def assign_filter_to_jail(
await run_blocking( _check_filter) await run_blocking( _check_filter)
await run_blocking(set_jail_local_key_sync, await run_blocking(config_file_service.set_jail_local_key_sync,
Path(config_dir), Path(config_dir),
jail_name, jail_name,
"filter", "filter",
@@ -690,7 +683,7 @@ async def assign_filter_to_jail(
if do_reload: if do_reload:
try: try:
await reload_all(socket_path) await config_file_service.jail_service.reload_all(socket_path)
except Exception as exc: # noqa: BLE001 except Exception as exc: # noqa: BLE001
log.warning( log.warning(
"reload_after_assign_filter_failed", "reload_after_assign_filter_failed",

View File

@@ -25,16 +25,7 @@ from app.exceptions import (
JailNotFoundError, JailNotFoundError,
JailNotFoundInConfigError, JailNotFoundInConfigError,
) )
from app.services.config_file_service import ( import app.services.config_file_service as config_file_service
build_inactive_jail,
get_active_jail_names,
parse_jails_sync,
safe_jail_name,
start_daemon,
validate_jail_config_sync,
wait_for_fail2ban,
)
from app.services.jail_service import reload_all
from app.models.config import ( from app.models.config import (
ActivateJailRequest, ActivateJailRequest,
InactiveJail, InactiveJail,
@@ -221,23 +212,6 @@ def _validate_regex_patterns(patterns: list[str]) -> None:
raise FilterInvalidRegexError(pattern, str(exc)) from exc raise FilterInvalidRegexError(pattern, str(exc)) from exc
async def _probe_fail2ban_running(socket_path: str) -> bool:
"""Return ``True`` if the fail2ban socket responds to a ping.
Args:
socket_path: Path to the fail2ban Unix domain socket.
Returns:
``True`` when fail2ban is reachable, ``False`` otherwise.
"""
try:
client = Fail2BanClient(socket_path=socket_path, timeout=5.0)
resp = await client.send(["ping"])
return isinstance(resp, (list, tuple)) and resp[0] == 0
except Exception: # noqa: BLE001
return False
# Shared functions from config_file_service are imported directly from the # Shared functions from config_file_service are imported directly from the
# canonical shared helper module. # canonical shared helper module.
@@ -270,11 +244,11 @@ async def list_inactive_jails(
inactive jails. inactive jails.
""" """
parsed_result: tuple[dict[str, dict[str, str]], dict[str, str]] = await run_blocking( parsed_result: tuple[dict[str, dict[str, str]], dict[str, str]] = await run_blocking(
parse_jails_sync, config_file_service._parse_jails_sync,
Path(config_dir), Path(config_dir),
) )
all_jails, source_files = parsed_result all_jails, source_files = parsed_result
active_names: set[str] = await get_active_jail_names(socket_path) active_names: set[str] = await config_file_service._get_active_jail_names(socket_path)
inactive: list[InactiveJail] = [] inactive: list[InactiveJail] = []
for jail_name, settings in sorted(all_jails.items()): for jail_name, settings in sorted(all_jails.items()):
@@ -283,7 +257,7 @@ async def list_inactive_jails(
continue continue
source = source_files.get(jail_name, config_dir) source = source_files.get(jail_name, config_dir)
inactive.append(build_inactive_jail(jail_name, settings, source, Path(config_dir))) inactive.append(config_file_service.build_inactive_jail(jail_name, settings, source, Path(config_dir)))
log.info( log.info(
"inactive_jails_listed", "inactive_jails_listed",
@@ -353,21 +327,21 @@ async def _activate_jail(
~app.utils.fail2ban_client.Fail2BanConnectionError: If the fail2ban ~app.utils.fail2ban_client.Fail2BanConnectionError: If the fail2ban
socket is unreachable during reload. socket is unreachable during reload.
""" """
safe_jail_name(name) config_file_service.safe_jail_name(name)
all_jails, _source_files = await run_blocking(parse_jails_sync, Path(config_dir)) all_jails, _source_files = await run_blocking(config_file_service._parse_jails_sync, Path(config_dir))
if name not in all_jails: if name not in all_jails:
raise JailNotFoundInConfigError(name) raise JailNotFoundInConfigError(name)
active_names = await get_active_jail_names(socket_path) active_names = await config_file_service._get_active_jail_names(socket_path)
if name in active_names: if name in active_names:
raise JailAlreadyActiveError(name) raise JailAlreadyActiveError(name)
# ---------------------------------------------------------------------- # # ---------------------------------------------------------------------- #
# Pre-activation validation — collect warnings but do not block # # Pre-activation validation — collect warnings but do not block #
# ---------------------------------------------------------------------- # # ---------------------------------------------------------------------- #
validation_result: JailValidationResult = await run_blocking(validate_jail_config_sync, Path(config_dir), name validation_result: JailValidationResult = await run_blocking(config_file_service._validate_jail_config_sync, Path(config_dir), name
) )
warnings: list[str] = [f"{i.field}: {i.message}" for i in validation_result.issues] warnings: list[str] = [f"{i.field}: {i.message}" for i in validation_result.issues]
if warnings: if warnings:
@@ -421,7 +395,7 @@ async def _activate_jail(
# Activation reload — if it fails, roll back immediately # # Activation reload — if it fails, roll back immediately #
# ---------------------------------------------------------------------- # # ---------------------------------------------------------------------- #
try: try:
await reload_all(socket_path, include_jails=[name]) await config_file_service.jail_service.reload_all(socket_path, include_jails=[name])
except JailNotFoundError as exc: except JailNotFoundError as exc:
# Jail configuration is invalid (e.g. missing logpath that prevents # Jail configuration is invalid (e.g. missing logpath that prevents
# fail2ban from loading the jail). Roll back and provide a specific error. # fail2ban from loading the jail). Roll back and provide a specific error.
@@ -467,7 +441,7 @@ async def _activate_jail(
for attempt in range(_POST_RELOAD_MAX_ATTEMPTS): for attempt in range(_POST_RELOAD_MAX_ATTEMPTS):
if attempt > 0: if attempt > 0:
await asyncio.sleep(_POST_RELOAD_PROBE_INTERVAL) await asyncio.sleep(_POST_RELOAD_PROBE_INTERVAL)
if await _probe_fail2ban_running(socket_path): if await config_file_service._probe_fail2ban_running(socket_path):
fail2ban_running = True fail2ban_running = True
break break
@@ -492,7 +466,7 @@ async def _activate_jail(
) )
# Verify the jail actually started (config error may prevent it silently). # Verify the jail actually started (config error may prevent it silently).
post_reload_names = await get_active_jail_names(socket_path) post_reload_names = await config_file_service._get_active_jail_names(socket_path)
actually_running = name in post_reload_names actually_running = name in post_reload_names
if not actually_running: if not actually_running:
log.warning( log.warning(
@@ -563,7 +537,7 @@ async def _rollback_activation_async(
# Step 2 — reload fail2ban with the restored config. # Step 2 — reload fail2ban with the restored config.
try: try:
await reload_all(socket_path) await config_file_service.jail_service.reload_all(socket_path)
log.info("jail_activation_rollback_reload_ok", jail=name) log.info("jail_activation_rollback_reload_ok", jail=name)
except Exception as exc: # noqa: BLE001 except Exception as exc: # noqa: BLE001
log.warning("jail_activation_rollback_reload_failed", jail=name, error=str(exc)) log.warning("jail_activation_rollback_reload_failed", jail=name, error=str(exc))
@@ -573,7 +547,7 @@ async def _rollback_activation_async(
for attempt in range(_POST_RELOAD_MAX_ATTEMPTS): for attempt in range(_POST_RELOAD_MAX_ATTEMPTS):
if attempt > 0: if attempt > 0:
await asyncio.sleep(_POST_RELOAD_PROBE_INTERVAL) await asyncio.sleep(_POST_RELOAD_PROBE_INTERVAL)
if await _probe_fail2ban_running(socket_path): if await config_file_service._probe_fail2ban_running(socket_path):
log.info("jail_activation_rollback_recovered", jail=name) log.info("jail_activation_rollback_recovered", jail=name)
return True return True
@@ -625,14 +599,14 @@ async def _deactivate_jail(
~app.utils.fail2ban_client.Fail2BanConnectionError: If the fail2ban ~app.utils.fail2ban_client.Fail2BanConnectionError: If the fail2ban
socket is unreachable during reload. socket is unreachable during reload.
""" """
safe_jail_name(name) config_file_service.safe_jail_name(name)
all_jails, _source_files = await run_blocking(parse_jails_sync, Path(config_dir)) all_jails, _source_files = await run_blocking(config_file_service._parse_jails_sync, Path(config_dir))
if name not in all_jails: if name not in all_jails:
raise JailNotFoundInConfigError(name) raise JailNotFoundInConfigError(name)
active_names = await get_active_jail_names(socket_path) active_names = await config_file_service._get_active_jail_names(socket_path)
if name not in active_names: if name not in active_names:
raise JailAlreadyInactiveError(name) raise JailAlreadyInactiveError(name)
@@ -644,7 +618,7 @@ async def _deactivate_jail(
) )
try: try:
await reload_all(socket_path, exclude_jails=[name]) await config_file_service.jail_service.reload_all(socket_path, exclude_jails=[name])
except Exception as exc: # noqa: BLE001 except Exception as exc: # noqa: BLE001
log.warning("reload_after_deactivate_failed", jail=name, error=str(exc)) log.warning("reload_after_deactivate_failed", jail=name, error=str(exc))
@@ -680,14 +654,14 @@ async def delete_jail_local_override(
delete the live config file). delete the live config file).
ConfigWriteError: If the file cannot be deleted. ConfigWriteError: If the file cannot be deleted.
""" """
safe_jail_name(name) config_file_service.safe_jail_name(name)
all_jails, _source_files = await run_blocking(parse_jails_sync, Path(config_dir)) all_jails, _source_files = await run_blocking(config_file_service._parse_jails_sync, Path(config_dir))
if name not in all_jails: if name not in all_jails:
raise JailNotFoundInConfigError(name) raise JailNotFoundInConfigError(name)
active_names = await get_active_jail_names(socket_path) active_names = await config_file_service._get_active_jail_names(socket_path)
if name in active_names: if name in active_names:
raise JailAlreadyActiveError(name) raise JailAlreadyActiveError(name)
@@ -720,8 +694,8 @@ async def validate_jail_config(
Raises: Raises:
JailNameError: If *name* contains invalid characters. JailNameError: If *name* contains invalid characters.
""" """
safe_jail_name(name) config_file_service.safe_jail_name(name)
return await run_blocking(validate_jail_config_sync, return await run_blocking(config_file_service._validate_jail_config_sync,
Path(config_dir), Path(config_dir),
name, name,
) )
@@ -770,7 +744,7 @@ async def _rollback_jail(
JailNameError: If *name* contains invalid characters. JailNameError: If *name* contains invalid characters.
ConfigWriteError: If writing the ``.local`` file fails. ConfigWriteError: If writing the ``.local`` file fails.
""" """
safe_jail_name(name) config_file_service.safe_jail_name(name)
# Write enabled=false — this must succeed even when fail2ban is down. # Write enabled=false — this must succeed even when fail2ban is down.
@@ -783,15 +757,15 @@ async def _rollback_jail(
log.info("jail_rolled_back_disabled", jail=name) log.info("jail_rolled_back_disabled", jail=name)
# Attempt to start the daemon. # Attempt to start the daemon.
started = await start_daemon(start_cmd_parts) started = await config_file_service.start_daemon(start_cmd_parts)
log.info("jail_rollback_start_attempted", jail=name, start_ok=started) log.info("jail_rollback_start_attempted", jail=name, start_ok=started)
# Wait for the socket to come back. # Wait for the socket to come back.
fail2ban_running = await wait_for_fail2ban(socket_path, max_wait_seconds=10.0, poll_interval=2.0) fail2ban_running = await config_file_service.wait_for_fail2ban(socket_path, max_wait_seconds=10.0, poll_interval=2.0)
active_jails = 0 active_jails = 0
if fail2ban_running: if fail2ban_running:
names = await get_active_jail_names(socket_path) names = await config_file_service._get_active_jail_names(socket_path)
active_jails = len(names) active_jails = len(names)
if fail2ban_running: if fail2ban_running: