From 9e31873520d84c31fbad9ef597b346c0f24aa291 Mon Sep 17 00:00:00 2001 From: Nicolas Mowen Date: Tue, 8 Nov 2022 18:47:45 -0700 Subject: [PATCH] More config checks (#4310) * Move existing checks to own functions * Add config check for zone objects that are not tracked * Add tests for config error * Formatting * Catch case where field is defined multiple times and add test * Add warning for rtmp --- frigate/config.py | 108 +++++++++++++++++++++++------------- frigate/test/test_config.py | 47 ++++++++++++++++ frigate/util.py | 30 ++++++++++ 3 files changed, 146 insertions(+), 39 deletions(-) diff --git a/frigate/config.py b/frigate/config.py index 9ca561f1f8..cf3ae63988 100644 --- a/frigate/config.py +++ b/frigate/config.py @@ -22,6 +22,7 @@ create_mask, deep_merge, escape_special_characters, + load_config_with_no_duplicates, load_labels, ) @@ -786,6 +787,66 @@ class LoggerConfig(FrigateBaseModel): ) +def verify_config_roles(camera_config: CameraConfig) -> None: + """Verify that roles are setup in the config correctly.""" + assigned_roles = list( + set([r for i in camera_config.ffmpeg.inputs for r in i.roles]) + ) + + if camera_config.record.enabled and not "record" in assigned_roles: + raise ValueError( + f"Camera {camera_config.name} has record enabled, but record is not assigned to an input." + ) + + if camera_config.rtmp.enabled and not "rtmp" in assigned_roles: + raise ValueError( + f"Camera {camera_config.name} has rtmp enabled, but rtmp is not assigned to an input." + ) + + if camera_config.restream.enabled and not "restream" in assigned_roles: + raise ValueError( + f"Camera {camera_config.name} has restream enabled, but restream is not assigned to an input." + ) + + +def verify_old_retain_config(camera_config: CameraConfig) -> None: + """Leave log if old retain_days is used.""" + if not camera_config.record.retain_days is None: + logger.warning( + "The 'retain_days' config option has been DEPRECATED and will be removed in a future version. Please use the 'days' setting under 'retain'" + ) + if camera_config.record.retain.days == 0: + camera_config.record.retain.days = camera_config.record.retain_days + + +def verify_recording_retention(camera_config: CameraConfig) -> None: + """Verify that recording retention modes are ranked correctly.""" + rank_map = { + RetainModeEnum.all: 0, + RetainModeEnum.motion: 1, + RetainModeEnum.active_objects: 2, + } + + if ( + camera_config.record.retain.days != 0 + and rank_map[camera_config.record.retain.mode] + > rank_map[camera_config.record.events.retain.mode] + ): + logger.warning( + f"{camera_config.name}: Recording retention is configured for {camera_config.record.retain.mode} and event retention is configured for {camera_config.record.events.retain.mode}. The more restrictive retention policy will be applied." + ) + + +def verify_zone_objects_are_tracked(camera_config: CameraConfig) -> None: + """Verify that user has not entered zone objects that are not in the tracking config.""" + for zone_name, zone in camera_config.zones.items(): + for obj in zone.objects: + if obj not in camera_config.objects.track: + raise ValueError( + f"Zone {zone_name} is configured to track {obj} but that object type is not added to objects -> track." + ) + + class FrigateConfig(FrigateBaseModel): mqtt: MqttConfig = Field(title="MQTT Configuration.") database: DatabaseConfig = Field( @@ -927,47 +988,16 @@ def runtime_config(self) -> FrigateConfig: **camera_config.motion.dict(exclude_unset=True), ) - # check runtime config - assigned_roles = list( - set([r for i in camera_config.ffmpeg.inputs for r in i.roles]) - ) - if camera_config.record.enabled and not "record" in assigned_roles: - raise ValueError( - f"Camera {name} has record enabled, but record is not assigned to an input." - ) - - if camera_config.rtmp.enabled and not "rtmp" in assigned_roles: - raise ValueError( - f"Camera {name} has rtmp enabled, but rtmp is not assigned to an input." - ) + verify_config_roles(camera_config) + verify_old_retain_config(camera_config) + verify_recording_retention(camera_config) + verify_zone_objects_are_tracked(camera_config) - if camera_config.restream.enabled and not "restream" in assigned_roles: - raise ValueError( - f"Camera {name} has restream enabled, but restream is not assigned to an input." - ) - - # backwards compatibility for retain_days - if not camera_config.record.retain_days is None: + if camera_config.rtmp.enabled: logger.warning( - "The 'retain_days' config option has been DEPRECATED and will be removed in a future version. Please use the 'days' setting under 'retain'" - ) - if camera_config.record.retain.days == 0: - camera_config.record.retain.days = camera_config.record.retain_days - - # warning if the higher level record mode is potentially more restrictive than the events - rank_map = { - RetainModeEnum.all: 0, - RetainModeEnum.motion: 1, - RetainModeEnum.active_objects: 2, - } - if ( - camera_config.record.retain.days != 0 - and rank_map[camera_config.record.retain.mode] - > rank_map[camera_config.record.events.retain.mode] - ): - logger.warning( - f"{name}: Recording retention is configured for {camera_config.record.retain.mode} and event retention is configured for {camera_config.record.events.retain.mode}. The more restrictive retention policy will be applied." + "RTMP restream is deprecated in favor of the restream role, recommend disabling RTMP." ) + # generate the ffmpeg commands camera_config.create_ffmpeg_cmds() config.cameras[name] = camera_config @@ -987,7 +1017,7 @@ def parse_file(cls, config_file): raw_config = f.read() if config_file.endswith(YAML_EXT): - config = yaml.safe_load(raw_config) + config = load_config_with_no_duplicates(raw_config) elif config_file.endswith(".json"): config = json.loads(raw_config) diff --git a/frigate/test/test_config.py b/frigate/test/test_config.py index 70b6ba03f5..7fceb2e6ad 100644 --- a/frigate/test/test_config.py +++ b/frigate/test/test_config.py @@ -1,11 +1,13 @@ import unittest import numpy as np from pydantic import ValidationError + from frigate.config import ( BirdseyeModeEnum, FrigateConfig, DetectorTypeEnum, ) +from frigate.util import load_config_with_no_duplicates class TestConfig(unittest.TestCase): @@ -1424,6 +1426,51 @@ def test_fails_on_bad_camera_name(self): ValidationError, lambda: frigate_config.runtime_config.cameras ) + def test_fails_zone_defines_untracked_object(self): + config = { + "mqtt": {"host": "mqtt"}, + "objects": {"track": ["person"]}, + "cameras": { + "back": { + "ffmpeg": { + "inputs": [ + { + "path": "rtsp://10.0.0.1:554/video", + "roles": ["detect"], + }, + ] + }, + "zones": { + "steps": { + "coordinates": "0,0,0,0", + "objects": ["car", "person"], + }, + }, + } + }, + } + + frigate_config = FrigateConfig(**config) + + self.assertRaises(ValueError, lambda: frigate_config.runtime_config.cameras) + + def test_fails_duplicate_keys(self): + raw_config = """ + cameras: + test: + ffmpeg: + inputs: + - one + - two + inputs: + - three + - four + """ + + self.assertRaises( + ValueError, lambda: load_config_with_no_duplicates(raw_config) + ) + def test_object_filter_ratios_work(self): config = { "mqtt": {"host": "mqtt"}, diff --git a/frigate/util.py b/frigate/util.py index 7ce973a94f..8b37038915 100755 --- a/frigate/util.py +++ b/frigate/util.py @@ -5,7 +5,10 @@ import signal import traceback import urllib.parse +import yaml + from abc import ABC, abstractmethod +from collections import Counter from collections.abc import Mapping from multiprocessing import shared_memory from typing import AnyStr @@ -44,6 +47,33 @@ def deep_merge(dct1: dict, dct2: dict, override=False, merge_lists=False) -> dic return merged +def load_config_with_no_duplicates(raw_config) -> dict: + """Get config ensuring duplicate keys are not allowed.""" + + # https://stackoverflow.com/a/71751051 + class PreserveDuplicatesLoader(yaml.loader.Loader): + pass + + def map_constructor(loader, node, deep=False): + keys = [loader.construct_object(node, deep=deep) for node, _ in node.value] + vals = [loader.construct_object(node, deep=deep) for _, node in node.value] + key_count = Counter(keys) + data = {} + for key, val in zip(keys, vals): + if key_count[key] > 1: + raise ValueError( + f"Config input {key} is defined multiple times for the same field, this is not allowed." + ) + else: + data[key] = val + return data + + PreserveDuplicatesLoader.add_constructor( + yaml.resolver.BaseResolver.DEFAULT_MAPPING_TAG, map_constructor + ) + return yaml.load(raw_config, PreserveDuplicatesLoader) + + def draw_timestamp( frame, timestamp,