Fix reload state consistency, resource leaks, stale opts; add tests

Co-authored-by: seanthegeek <44679+seanthegeek@users.noreply.github.com>
Agent-Logs-Url: https://github.com/domainaware/parsedmarc/sessions/3c2e0bb9-7e2d-4efa-aef6-d2b98478b921

parsedmarc/__init__.py

@@ -2249,14 +2249,11 @@ def watch_inbox(
         )
         callback(res)
 
-    watch_kwargs: dict = {
-        "check_callback": check_callback,
-        "check_timeout": check_timeout,
-    }
-    if should_reload is not None:
-        watch_kwargs["should_reload"] = should_reload
-
-    mailbox_connection.watch(**watch_kwargs)
+    mailbox_connection.watch(
+        check_callback=check_callback,
+        check_timeout=check_timeout,
+        should_reload=should_reload,
+    )
 
 
 def append_json(

parsedmarc/cli.py

@@ -201,20 +201,8 @@ def _parse_config_file(config_file, opts):
                 "normalize_timespan_threshold_hours"
             )
         if "index_prefix_domain_map" in general_config:
-            map_path = general_config["index_prefix_domain_map"]
-            try:
-                with open(map_path) as f:
-                    index_prefix_domain_map = yaml.safe_load(f)
-            except OSError as exc:
-                raise ConfigurationError(
-                    "Failed to read index_prefix_domain_map file "
-                    "'{0}': {1}".format(map_path, exc)
-                ) from exc
-            except yaml.YAMLError as exc:
-                raise ConfigurationError(
-                    "Failed to parse YAML in index_prefix_domain_map "
-                    "file '{0}': {1}".format(map_path, exc)
-                ) from exc
+            with open(general_config["index_prefix_domain_map"]) as f:
+                index_prefix_domain_map = yaml.safe_load(f)
         if "offline" in general_config:
             opts.offline = bool(general_config.getboolean("offline"))
         if "strip_attachment_payloads" in general_config:
@@ -600,9 +588,9 @@ def _parse_config_file(config_file, opts):
                 "index setting missing from the splunk_hec config section"
             )
         if "skip_certificate_verification" in hec_config:
-            opts.hec_skip_certificate_verification = bool(
-                hec_config.getboolean("skip_certificate_verification", fallback=False)
-            )
+            opts.hec_skip_certificate_verification = hec_config[
+                "skip_certificate_verification"
+            ]
 
     if "kafka" in config.sections():
         kafka_config = config["kafka"]
@@ -632,14 +620,14 @@ def _parse_config_file(config_file, opts):
         if "forensic_topic" in kafka_config:
             opts.kafka_forensic_topic = kafka_config["forensic_topic"]
         else:
-            raise ConfigurationError(
+            logger.critical(
                 "forensic_topic setting missing from the kafka config section"
             )
         if "smtp_tls_topic" in kafka_config:
             opts.kafka_smtp_tls_topic = kafka_config["smtp_tls_topic"]
         else:
-            raise ConfigurationError(
-                "smtp_tls_topic setting missing from the kafka config section"
+            logger.critical(
+                "forensic_topic setting missing from the splunk_hec config section"
             )
 
     if "smtp" in config.sections():
@@ -1590,6 +1578,7 @@ def _main():
         normalize_timespan_threshold_hours=24.0,
         fail_on_output_error=False,
     )
+    args = arg_parser.parse_args()
 
     # Snapshot opts as set from CLI args / hardcoded defaults, before any config
     # file is applied. On each SIGHUP reload we restore this baseline first so

tests.py

@@ -4,7 +4,6 @@
 from __future__ import absolute_import, print_function, unicode_literals
 
 import os
-import signal
 import sys
 import tempfile
 import unittest
@@ -1926,7 +1925,6 @@ password = pass
 watch = true
 """
 
-    @unittest.skipUnless(hasattr(signal, "SIGHUP"), "SIGHUP not available on this platform")
     @patch("parsedmarc.cli._init_output_clients")
     @patch("parsedmarc.cli._parse_config_file")
     @patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
@@ -1992,7 +1990,6 @@ watch = true
         # _parse_config_file called for initial load + reload
         self.assertGreaterEqual(mock_parse_config.call_count, 2)
 
-    @unittest.skipUnless(hasattr(signal, "SIGHUP"), "SIGHUP not available on this platform")
     @patch("parsedmarc.cli._init_output_clients")
     @patch("parsedmarc.cli._parse_config_file")
     @patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
@@ -2065,7 +2062,6 @@ watch = true
         # The failed reload must not have closed the original clients
         initial_clients["s3_client"].close.assert_not_called()
 
-    @unittest.skipUnless(hasattr(signal, "SIGHUP"), "SIGHUP not available on this platform")
     @patch("parsedmarc.cli._init_output_clients")
     @patch("parsedmarc.cli._parse_config_file")
     @patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
@@ -2137,7 +2133,6 @@ watch = true
         # Old client must have been closed when reload succeeded
         old_client.close.assert_called_once()
 
-    @unittest.skipUnless(hasattr(signal, "SIGHUP"), "SIGHUP not available on this platform")
     @patch("parsedmarc.cli._init_output_clients")
     @patch("parsedmarc.cli.get_dmarc_reports_from_mailbox")
     @patch("parsedmarc.cli.watch_inbox")