diff --git a/CHANGELOG.md b/CHANGELOG.md index 7adb1af4..91c2e049 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ ### Changes - **Breaking: the output and mailbox integrations are now optional extras, so `pip install parsedmarc` installs the parsing core and a working core CLI instead of every SDK** ([#883](https://github.com/domainaware/parsedmarc/issues/883)). A 10.x install pulled in the Elasticsearch, OpenSearch, Kafka, AWS (boto3), Azure, Gmail, and Microsoft Graph client libraries whether or not a deployment used any of them — roughly 1 GB of site-packages against roughly 300 MB without — which is a lot to ask of the motivating case, running parsedmarc as a parsing library on a small mail host. The base install now covers the parsing library plus the CLI reading from files, IMAP, Maildir, and mbox, and writing CSV/JSON, Splunk HEC, webhook, and syslog output; those outputs need only `httpx` and the standard library, so they deliberately have no extra of their own. Everything else moves behind an extra: `elastic` (Elasticsearch), `opensearch` (OpenSearch, including the boto3 SigV4 signer), `kafka`, `s3`, `gelf`, `loganalytics` (Azure Monitor), `msgraph` (Microsoft 365 mailboxes), and `gmail` (Gmail API mailboxes), joining the `postgresql` extra that already existed. **CLI users upgrading from 10.x should switch their upgrade command to `pip install -U "parsedmarc[all]"`** — the new umbrella extra — to keep every integration available; add `postgresql` to the list (`parsedmarc[all,postgresql]`) if the PostgreSQL backend is in use. `all` deliberately excludes `postgresql` because `psycopg`'s prebuilt binary wheels do not exist for every platform, and `pip install parsedmarc[all]` must not fail on a platform they do not cover. Users of the prebuilt Docker image (`ghcr.io/domainaware/parsedmarc`) see no change at all: the image now installs `[all,postgresql]`, so it still bundles every integration. A configuration section whose extra is missing no longer fails with an `ImportError` traceback at startup; it fails fast with a `ConfigurationError` naming both the section and the command that fixes it, e.g. `The [elasticsearch] configuration section requires the elastic extra: pip install parsedmarc[elastic]`. Finally, the never-imported `dateparser` dependency is dropped in favor of declaring `python-dateutil`, which `parsedmarc.utils` actually imports and which used to arrive only transitively through `dateparser`. +- The CLI now accepts `--dns-timeout` as an alias of `--dns_timeout`, which is kept for backward compatibility (public since 6.0.0); `--dns-retries` already used the hyphenated form. + +### Bug fixes + +- **`[elasticsearch]`/`[opensearch]` `number_of_replicas` is no longer ignored when `number_of_shards` is not also set** — the parser only read `number_of_replicas` inside the `number_of_shards` branch (accidental nesting dating to the 6.4.0-era code), while the documentation lists the two options independently and the client code accepts them independently. ## 10.5.0 diff --git a/docs/source/usage.md b/docs/source/usage.md index b144f925..b43effd6 100644 --- a/docs/source/usage.md +++ b/docs/source/usage.md @@ -41,7 +41,7 @@ options: -n NAMESERVERS [NAMESERVERS ...], --nameservers NAMESERVERS [NAMESERVERS ...] nameservers to query: IP addresses, https:// URLs (DNS over HTTPS), and/or tls://ip[:port][#hostname] (DNS over TLS) - -t DNS_TIMEOUT, --dns_timeout DNS_TIMEOUT + -t DNS_TIMEOUT, --dns_timeout DNS_TIMEOUT, --dns-timeout DNS_TIMEOUT number of seconds to wait for an answer from DNS (default: 2.0) --dns-retries DNS_RETRIES number of times to retry DNS queries on timeout or other transient errors (default: 0) diff --git a/parsedmarc/cli.py b/parsedmarc/cli.py index b799be7c..f7f09978 100644 --- a/parsedmarc/cli.py +++ b/parsedmarc/cli.py @@ -1125,9 +1125,9 @@ def _parse_config(config: ConfigParser, opts): if "number_of_shards" in elasticsearch_config: number_of_shards = elasticsearch_config.getint("number_of_shards") opts.elasticsearch_number_of_shards = number_of_shards - if "number_of_replicas" in elasticsearch_config: - number_of_replicas = elasticsearch_config.getint("number_of_replicas") - opts.elasticsearch_number_of_replicas = number_of_replicas + if "number_of_replicas" in elasticsearch_config: + number_of_replicas = elasticsearch_config.getint("number_of_replicas") + opts.elasticsearch_number_of_replicas = number_of_replicas if "index_suffix" in elasticsearch_config: opts.elasticsearch_index_suffix = elasticsearch_config["index_suffix"] if "index_prefix" in elasticsearch_config: @@ -1174,9 +1174,9 @@ def _parse_config(config: ConfigParser, opts): if "number_of_shards" in opensearch_config: number_of_shards = opensearch_config.getint("number_of_shards") opts.opensearch_number_of_shards = number_of_shards - if "number_of_replicas" in opensearch_config: - number_of_replicas = opensearch_config.getint("number_of_replicas") - opts.opensearch_number_of_replicas = number_of_replicas + if "number_of_replicas" in opensearch_config: + number_of_replicas = opensearch_config.getint("number_of_replicas") + opts.opensearch_number_of_replicas = number_of_replicas if "index_suffix" in opensearch_config: opts.opensearch_index_suffix = opensearch_config["index_suffix"] if "index_prefix" in opensearch_config: @@ -2515,6 +2515,7 @@ def _main(): arg_parser.add_argument( "-t", "--dns_timeout", + "--dns-timeout", help="number of seconds to wait for an answer from DNS (default: 2.0)", type=float, default=2.0, diff --git a/tests/test_cli.py b/tests/test_cli.py index 85e7ae7d..37041edb 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1759,11 +1759,21 @@ class TestCliParserConfigWiring(unittest.TestCase): self._stdout_patch.stop() def _run_one_shot_mailbox( - self, dns_timeout: float | None = None, dns_retries: int | None = None + self, + dns_timeout: float | None = None, + dns_retries: int | None = None, + extra_argv: list[str] | None = None, ) -> parsedmarc.ParserConfig: """Runs a real one-shot _main() against a mocked IMAP connection and mocked get_dmarc_reports_from_mailbox, and returns the ParserConfig - the CLI passed as ``config=``.""" + the CLI passed as ``config=``. + + ``extra_argv``, when given, is appended to the CLI invocation after + ``-c `` -- e.g. to exercise a command-line flag such as + ``--dns-timeout`` without involving the config file's [general] + dns_timeout key at all. It defaults to None/no extra arguments, so + existing callers are unaffected. + """ config_lines = ["[general]", "silent = true"] if dns_timeout is not None: config_lines.append(f"dns_timeout = {dns_timeout}") @@ -1783,6 +1793,8 @@ class TestCliParserConfigWiring(unittest.TestCase): cfg_path = cfg.name self.addCleanup(lambda: os.path.exists(cfg_path) and os.remove(cfg_path)) + argv = ["parsedmarc", "-c", cfg_path] + list(extra_argv or []) + with ( patch("parsedmarc.cli.get_dmarc_reports_from_mailbox") as mock_get_reports, patch("parsedmarc.cli.IMAPConnection") as mock_imap, @@ -1793,7 +1805,7 @@ class TestCliParserConfigWiring(unittest.TestCase): "failure_reports": [], "smtp_tls_reports": [], } - with patch.object(sys, "argv", ["parsedmarc", "-c", cfg_path]): + with patch.object(sys, "argv", argv): parsedmarc.cli._main() return mock_get_reports.call_args.kwargs["config"] @@ -1813,6 +1825,25 @@ class TestCliParserConfigWiring(unittest.TestCase): self.assertEqual(cfg.dns_timeout, 11.5) self.assertEqual(cfg.dns_retries, 3) + def test_dns_timeout_hyphenated_alias_reaches_parser_config(self): + """``--dns-timeout`` (hyphenated) is an alias for ``--dns_timeout`` + (underscored), which has been the public spelling since 6.0.0. The + hyphenated alias exists because ``--dns-retries`` (added in 9.7.1) + uses a hyphen while ``--dns_timeout`` predates it and must keep + working unchanged, so the change adds ``--dns-timeout`` as an + additional option string rather than renaming anything. Both + spellings must resolve to the same argparse dest (``dns_timeout``) + and reach the same ParserConfig field passed to + get_dmarc_reports_from_mailbox — and both halves are exercised + here, because dropping the legacy option string would leave the + dest (derived from the surviving long option) and every + config-file test green while breaking the promised compatibility. + """ + cfg = self._run_one_shot_mailbox(extra_argv=["--dns-timeout", "7"]) + self.assertEqual(cfg.dns_timeout, 7.0) + cfg = self._run_one_shot_mailbox(extra_argv=["--dns_timeout", "9"]) + self.assertEqual(cfg.dns_timeout, 9.0) + def test_cli_config_binds_module_default_caches(self): """The ParserConfig built by the CLI must bind the process-wide default caches (parsedmarc.IP_ADDRESS_CACHE, @@ -5882,6 +5913,31 @@ class TestParseConfigElasticsearch(unittest.TestCase): self.assertEqual(opts.elasticsearch_password, "secret") self.assertEqual(opts.elasticsearch_api_key, "base64key") + def test_elasticsearch_number_of_replicas_without_shards(self): + """Regression test: a [elasticsearch] section that sets only + number_of_replicas (no number_of_shards) must still set + opts.elasticsearch_number_of_replicas. + + Before this fix, the number_of_replicas check was nested inside the + number_of_shards ``if`` block, so a replicas-only config was + silently ignored -- docs/source/usage.md lists number_of_shards and + number_of_replicas as independent options with no stated + dependency, and elastic.py's save functions accept them as + independent parameters with independent defaults, shards=1 and + replicas=0, but the accidental nesting made replicas silently + depend on shards also being set. + """ + from parsedmarc.cli import _parse_config + + cp = _config_with( + "elasticsearch", + {"hosts": "es:9200", "number_of_replicas": "2"}, + ) + opts = _opts() + _parse_config(cp, opts) + self.assertEqual(opts.elasticsearch_number_of_replicas, 2) + self.assertFalse(hasattr(opts, "elasticsearch_number_of_shards")) + def test_elasticsearch_apikey_camelcase_alias_pre_8_20(self): """`apiKey` (camelCase) is the legacy 8.20-and-earlier name.""" from parsedmarc.cli import _parse_config @@ -5999,6 +6055,31 @@ class TestParseConfigOpenSearch(unittest.TestCase): self.assertEqual(opts.opensearch_aws_region, "us-east-1") self.assertEqual(opts.opensearch_aws_service, "es") + def test_opensearch_number_of_replicas_without_shards(self): + """Regression test: a [opensearch] section that sets only + number_of_replicas (no number_of_shards) must still set + opts.opensearch_number_of_replicas. + + Before this fix, the number_of_replicas check was nested inside the + number_of_shards ``if`` block, so a replicas-only config was + silently ignored -- docs/source/usage.md lists number_of_shards and + number_of_replicas as independent options with no stated + dependency, and opensearch.py's save functions accept them as + independent parameters with independent defaults, shards=1 and + replicas=0, but the accidental nesting made replicas silently + depend on shards also being set. + """ + from parsedmarc.cli import _parse_config + + cp = _config_with( + "opensearch", + {"hosts": "os:9200", "number_of_replicas": "3"}, + ) + opts = _opts() + _parse_config(cp, opts) + self.assertEqual(opts.opensearch_number_of_replicas, 3) + self.assertFalse(hasattr(opts, "opensearch_number_of_shards")) + def test_opensearch_authentication_type_legacy_alias(self): """`authentication_type` is the legacy spelling of `auth_type`.""" from parsedmarc.cli import _parse_config