diff --git a/frigate/api/config_util.py b/frigate/api/config_util.py index 5d963ad725..7c27ab5d78 100644 --- a/frigate/api/config_util.py +++ b/frigate/api/config_util.py @@ -16,6 +16,10 @@ def swap_runtime_config(app: FastAPI, config: FrigateConfig) -> None: camera the user turned off would silently come back on. """ app.frigate_config = config + + if app.config_holder is not None: + app.config_holder.set(config) + app.genai_manager.update_config(config) if app.profile_manager is not None: diff --git a/frigate/api/fastapi_app.py b/frigate/api/fastapi_app.py index 5e8323b58f..8a383d41ce 100644 --- a/frigate/api/fastapi_app.py +++ b/frigate/api/fastapi_app.py @@ -35,6 +35,7 @@ from frigate.comms.event_metadata_updater import ( ) from frigate.config import FrigateConfig from frigate.config.camera.updater import CameraConfigUpdatePublisher +from frigate.config.holder import ConfigHolder from frigate.config.profile_manager import ProfileManager from frigate.debug_replay import DebugReplayManager, debug_replay_auto_stop_watchdog from frigate.embeddings import EmbeddingsContext @@ -74,6 +75,7 @@ def create_fastapi_app( dispatcher: Dispatcher | None = None, profile_manager: ProfileManager | None = None, enforce_default_admin: bool = True, + config_holder: ConfigHolder | None = None, ): logger.info("Starting FastAPI app") app = FastAPI( @@ -162,6 +164,7 @@ def create_fastapi_app( app.replay_manager = replay_manager app.dispatcher = dispatcher app.profile_manager = profile_manager + app.config_holder = config_holder if frigate_config.auth.enabled: secret = get_jwt_secret() diff --git a/frigate/app.py b/frigate/app.py index 5a39fc8a5b..f0c0b7f9b4 100644 --- a/frigate/app.py +++ b/frigate/app.py @@ -30,6 +30,7 @@ from frigate.comms.ws import WebSocketClient from frigate.comms.zmq_proxy import ZmqProxy from frigate.config.camera.updater import CameraConfigUpdatePublisher from frigate.config.config import FrigateConfig +from frigate.config.holder import ConfigHolder from frigate.config.profile_manager import ProfileManager from frigate.const import ( CACHE_DIR, @@ -122,7 +123,18 @@ class FrigateApp: self.processes: dict[str, int] = {} self.embeddings: EmbeddingsContext | None = None self.profile_manager: ProfileManager | None = None - self.config = config + self.config_holder = ConfigHolder(config) + + @property + def config(self) -> FrigateConfig: + """The current config, not the one Frigate booted with. + + Read through the holder so the deferred watchdog factories below build + a replacement process from the config as it is now. There is no setter + on purpose: a plain attribute would let a caller pin this back to a + single object and reintroduce the staleness. + """ + return self.config_holder.config def ensure_dirs(self) -> None: dirs = [ @@ -645,6 +657,7 @@ class FrigateApp: self.replay_manager, self.dispatcher, self.profile_manager, + config_holder=self.config_holder, ), host="127.0.0.1", port=5001, diff --git a/frigate/config/holder.py b/frigate/config/holder.py new file mode 100644 index 0000000000..e5f3e7bd8f --- /dev/null +++ b/frigate/config/holder.py @@ -0,0 +1,34 @@ +"""Shared handle on the config object that is current for this instance.""" + +from .config import FrigateConfig + +__all__ = ["ConfigHolder"] + + +class ConfigHolder: + """Indirection for the most recently parsed config. + + /api/config/set re-parses yaml into a brand new FrigateConfig instead of + mutating the old one, so any reference captured during startup goes stale + the first time a user saves. Anything that has to build something after + startup, most importantly the watchdog factories that rebuild a crashed + process, must read through a holder rather than close over a config + object, or the rebuilt process comes back with the config as it was at + boot and silently discards every change made since. + + There is deliberately no setter on the read side: the swap runs in exactly + one place (frigate.api.config_util.swap_runtime_config) and everyone else + only reads. + """ + + def __init__(self, config: FrigateConfig) -> None: + self._config = config + + @property + def config(self) -> FrigateConfig: + """The config as of the most recent successful save.""" + return self._config + + def set(self, config: FrigateConfig) -> None: + """Install a freshly parsed config as the current one.""" + self._config = config diff --git a/frigate/test/http_api/test_http_config_set.py b/frigate/test/http_api/test_http_config_set.py index 0540a50818..68d4537165 100644 --- a/frigate/test/http_api/test_http_config_set.py +++ b/frigate/test/http_api/test_http_config_set.py @@ -13,6 +13,7 @@ from frigate.config.camera.updater import ( CameraConfigUpdatePublisher, CameraConfigUpdateTopic, ) +from frigate.config.holder import ConfigHolder from frigate.models import Event, Recordings, ReviewSegment from frigate.test.http_api.base_http_test import AuthTestClient, BaseTestHttp @@ -373,6 +374,72 @@ class TestConfigSetWildcardPropagation(BaseTestHttp): finally: os.unlink(config_path) + @patch("frigate.api.app.find_config_file") + def test_save_updates_the_config_holder(self, mock_find_config): + """A save must move the holder onto the freshly parsed config. + + FrigateApp reads the holder when the watchdog rebuilds a crashed + process; if the save leaves it on the boot config, that process comes + back having lost every change made since Frigate started. + """ + from fastapi import Request + + from frigate.api.auth import get_allowed_cameras_for_filter, get_current_user + from frigate.api.fastapi_app import create_fastapi_app + + config_path = self._write_config_file() + mock_find_config.return_value = config_path + + mock_publisher = Mock(spec=CameraConfigUpdatePublisher) + mock_publisher.publisher = MagicMock() + boot_config = FrigateConfig(**self.minimal_config) + holder = ConfigHolder(boot_config) + + try: + app = create_fastapi_app( + boot_config, + self.db, + None, + None, + None, + None, + None, + None, + mock_publisher, + None, + enforce_default_admin=False, + config_holder=holder, + ) + + async def mock_get_current_user(request: Request): + return {"username": "admin", "role": "admin"} + + async def mock_get_allowed_cameras_for_filter(request: Request): + return list(self.minimal_config.get("cameras", {}).keys()) + + app.dependency_overrides[get_current_user] = mock_get_current_user + app.dependency_overrides[get_allowed_cameras_for_filter] = ( + mock_get_allowed_cameras_for_filter + ) + + with AuthTestClient(app) as client: + resp = client.put( + "/config/set", + json={ + "config_data": {"birdseye": {"inactivity_threshold": 5}}, + "update_topic": "config/birdseye", + "requires_restart": 0, + }, + ) + + self.assertEqual(resp.status_code, 200) + + self.assertIsNot(holder.config, boot_config) + self.assertIs(holder.config, app.frigate_config) + self.assertEqual(holder.config.birdseye.inactivity_threshold, 5) + finally: + os.unlink(config_path) + if __name__ == "__main__": unittest.main() diff --git a/frigate/test/test_config_util.py b/frigate/test/test_config_util.py index 300b0b0c51..5888f26188 100644 --- a/frigate/test/test_config_util.py +++ b/frigate/test/test_config_util.py @@ -4,6 +4,7 @@ import unittest from unittest.mock import MagicMock from frigate.api.config_util import swap_runtime_config +from frigate.config.holder import ConfigHolder class TestSwapRuntimeConfig(unittest.TestCase): @@ -12,6 +13,7 @@ class TestSwapRuntimeConfig(unittest.TestCase): def _make_app(self) -> MagicMock: app = MagicMock() app.dispatcher.comms = [MagicMock(), MagicMock()] + app.config_holder = ConfigHolder(MagicMock(name="boot_config")) return app def test_rebinds_all_references(self) -> None: @@ -37,11 +39,40 @@ class TestSwapRuntimeConfig(unittest.TestCase): # the swap rebuilds cameras from yaml, so overrides must be re-layered app.dispatcher.reapply_runtime_state_to_config.assert_called_once_with() + def test_updates_the_config_holder(self) -> None: + app = self._make_app() + holder = app.config_holder + config = MagicMock(name="new_config") + + swap_runtime_config(app, config) + + self.assertIs(holder.config, config) + + def test_deferred_factory_builds_from_the_swapped_config(self) -> None: + """A watchdog-style factory must not rebuild from the boot config. + + The factories in FrigateApp are lambdas evaluated when a process is + restarted, long after a user may have saved. Reading through the + holder is what keeps a rebuilt process from reverting every change + made since Frigate started. + """ + app = self._make_app() + holder = app.config_holder + boot_config = holder.config + factory = lambda: holder.config # noqa: E731 + self.assertIs(factory(), boot_config) + + config = MagicMock(name="new_config") + swap_runtime_config(app, config) + + self.assertIs(factory(), config) + def test_tolerates_missing_optional_collaborators(self) -> None: app = MagicMock() app.profile_manager = None app.stats_emitter = None app.dispatcher = None + app.config_holder = None config = MagicMock(name="new_config") # must not raise when the optional collaborators are absent