-where failed_sessions > 0
-| stats sum(failed_sessions) as failed_sessions by organization_name, policy_domain, policy_type, failure_type, sending_mta_ip, receiving_mta_ip, receiving_mx_hostname
+| spath input=policy path=failure_details{} output=detail
+| mvexpand detail
+| spath input=detail
+| where failed_session_count > 0
+| rename result_type as failure_type
+| rename receiving_ip as receiving_mta_ip
+| fillnull value="none" failure_type sending_mta_ip receiving_mta_ip receiving_mx_hostname
+| stats sum(failed_session_count) as failed_sessions by organization_name, policy_domain, policy_type, failure_type, sending_mta_ip, receiving_mta_ip, receiving_mx_hostname
diff --git a/docs/source/elasticsearch.md b/docs/source/elasticsearch.md
index ffdb972e..2d0a1a43 100644
--- a/docs/source/elasticsearch.md
+++ b/docs/source/elasticsearch.md
@@ -226,6 +226,158 @@ Kibana index patterns with versions that match the upgraded indexes:
7. Import `export.ndjson` by clicking Import from the Kibana
Saved Objects page
+## Backfilling the combined DKIM/SPF result fields
+
+As of the version fixing [#169](https://github.com/domainaware/parsedmarc/issues/169),
+aggregate documents include `dkim_results_combined` and `spf_results_combined` —
+scalar string arrays that keep each auth result's selector/scope, domain, and
+result paired, which the dashboards' alignment-detail tables aggregate on.
+Reports saved by older versions lack these fields and will not appear in
+those tables.
+
+parsedmarc now backfills this automatically. On startup, it runs a cheap
+count query against each configured aggregate index pattern to check for
+documents that have DKIM or SPF results but are missing the corresponding
+combined field. If any are found, it submits the backfill as a background
+`_update_by_query` task (`wait_for_completion=false`), so startup is never
+blocked on it; progress is logged, including the task ID. The check itself
+is idempotent — once an index is fully backfilled, later startups see a
+count of 0 and log nothing further — and it works the same way on
+OpenSearch. Any error talking to the cluster (for example, no indexes yet
+on a fresh install) is logged as a warning and retried on the next startup,
+rather than aborting parsedmarc.
+
+If you upgrade the dashboards without pointing the new parsedmarc version
+at the cluster, or you'd rather control when the write load happens, you
+can still run the backfill manually. It is idempotent (documents that
+already have the fields are skipped), so it is safe to re-run. It works
+identically on OpenSearch; just adjust the URL and credentials. The query
+matches only documents that have at least one DKIM or SPF auth result and
+lack the corresponding combined field; documents with no auth results are
+skipped, because an `exists` query cannot see an empty array, and for
+search purposes an empty `dkim_results_combined` is identical to an
+absent one. Each result is matched on either its `domain` or its `result`
+subfield as defense in depth: an empty string indexes no text tokens and
+is invisible to `exists`, and the storage shape of every historical
+parsedmarc version can't be audited, so matching either subfield ensures
+no backfillable document is skipped.
+
+```bash
+curl -X POST "http://localhost:9200/dmarc_aggregate*/_update_by_query?conflicts=proceed&wait_for_completion=false" \
+ -H "Content-Type: application/json" -d '
+{
+ "query": {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {
+ "bool": {
+ "must": [
+ {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {"exists": {"field": "dkim_results.domain"}},
+ {"exists": {"field": "dkim_results.result"}}
+ ]
+ }
+ }
+ ],
+ "must_not": [{"exists": {"field": "dkim_results_combined"}}]
+ }
+ },
+ {
+ "bool": {
+ "must": [
+ {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {"exists": {"field": "spf_results.domain"}},
+ {"exists": {"field": "spf_results.result"}}
+ ]
+ }
+ }
+ ],
+ "must_not": [{"exists": {"field": "spf_results_combined"}}]
+ }
+ }
+ ]
+ }
+ },
+ "script": {
+ "lang": "painless",
+ "source": "List dk = new ArrayList(); def dr = ctx._source.dkim_results; if (dr != null) { if (!(dr instanceof List)) { dr = [dr]; } for (e in dr) { if (e == null) { continue; } def sel = e.selector != null ? e.selector : \"none\"; def dom = e.domain != null ? e.domain : \"none\"; def res = e.result != null ? e.result : \"none\"; dk.add(sel + \" / \" + dom + \" / \" + res); } } ctx._source.dkim_results_combined = dk; List sp = new ArrayList(); def sr = ctx._source.spf_results; if (sr != null) { if (!(sr instanceof List)) { sr = [sr]; } for (e in sr) { if (e == null) { continue; } def sc = e.scope != null ? e.scope : \"mfrom\"; def dom = e.domain != null ? e.domain : \"none\"; def res = e.result != null ? e.result : (e.results != null ? e.results : \"none\"); sp.add(sc + \" / \" + dom + \" / \" + res); } } ctx._source.spf_results_combined = sp;"
+ }
+}'
+```
+
+`wait_for_completion=false` returns a task ID — check progress with
+`GET _tasks/`. Adjust the index pattern if you use a custom
+`index_prefix`/`index_suffix`. After backfilling, re-import the updated
+dashboards ndjson (the index pattern saved object changed too) per the
+import instructions above.
+
+SMTP TLS documents have the same class of defect one level deeper:
+`policies` is an object array, and each policy's `failure_details` is an
+object array inside it. SMTP TLS documents now also carry
+`policies_combined` and `failure_details_combined`, backfilled
+automatically at startup the same way, and the equivalent manual command
+is:
+
+```bash
+curl -X POST "http://localhost:9200/smtp_tls*/_update_by_query?conflicts=proceed&wait_for_completion=false" \
+ -H "Content-Type: application/json" -d '
+{
+ "query": {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {
+ "bool": {
+ "must": [
+ {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {"exists": {"field": "policies.policy_domain"}},
+ {"exists": {"field": "policies.policy_type"}}
+ ]
+ }
+ }
+ ],
+ "must_not": [{"exists": {"field": "policies_combined"}}]
+ }
+ },
+ {
+ "bool": {
+ "must": [
+ {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {"exists": {"field": "policies.failure_details.result_type"}},
+ {"exists": {"field": "policies.failure_details.sending_mta_ip"}}
+ ]
+ }
+ }
+ ],
+ "must_not": [{"exists": {"field": "failure_details_combined"}}]
+ }
+ }
+ ]
+ }
+ },
+ "script": {
+ "lang": "painless",
+ "source": "List pols = new ArrayList(); List dets = new ArrayList(); def ps = ctx._source.policies; if (ps != null) { if (!(ps instanceof List)) { ps = [ps]; } for (p in ps) { if (p == null) { continue; } def dom = p.policy_domain != null ? p.policy_domain : \"none\"; def typ = p.policy_type != null ? p.policy_type : \"none\"; pols.add(dom + \" / \" + typ); def fds = p.failure_details; if (fds != null) { if (!(fds instanceof List)) { fds = [fds]; } for (f in fds) { if (f == null) { continue; } def rt = f.result_type != null ? f.result_type : \"none\"; def smi = f.sending_mta_ip != null ? f.sending_mta_ip : \"none\"; def ri = f.receiving_ip != null ? f.receiving_ip : \"none\"; def rmh = f.receiving_mx_hostname != null ? f.receiving_mx_hostname : \"none\"; dets.add(dom + \" / \" + typ + \" / \" + rt + \" / \" + smi + \" / \" + ri + \" / \" + rmh); } } } } ctx._source.policies_combined = pols; ctx._source.failure_details_combined = dets;"
+ }
+}'
+```
+
+It works identically on OpenSearch; just adjust the URL and credentials, same
+as the aggregate command above.
+
## Records retention
Starting in version 5.0.0, `parsedmarc` stores data in a separate
diff --git a/docs/source/kibana.md b/docs/source/kibana.md
index 33f4dd4d..8a2be4b7 100644
--- a/docs/source/kibana.md
+++ b/docs/source/kibana.md
@@ -82,7 +82,23 @@ Further down the dashboard, you can filter by source country or source IP
address.
Tables showing SPF and DKIM alignment details are located under the IP address
-table.
+table. Each row of the DKIM details table is one real DKIM signature, shown
+as a combined `selector / domain / result` value; the SPF details table
+shows `scope / domain / result` the same way. Combining the values into one
+column keeps each signature's selector, domain, and result paired together,
+rather than aggregating them as separate columns. Because a message that
+carries multiple DKIM signatures appears once per signature, summing the
+messages column across rows can exceed the total number of messages.
+
+The "Auth result filters" panel above the details tables
+provides dropdowns for the individual auth-result components — DKIM
+selector, DKIM domain, DKIM result, SPF scope, SPF domain, and SPF
+result — and filters the whole dashboard by them. Because components from
+different signatures of the same message are indexed together, combining
+two of these component filters matches documents where any signature
+satisfies each condition individually, not necessarily the same signature;
+the combined `selector / domain / result` (`scope / domain / result`)
+column remains the per-signature source of truth.
:::{note}
The alignment tables (SPF details, DKIM details) and the per-IP source
@@ -112,3 +128,15 @@ reporting organizations, the policy domains they report on, and the
specific failure types — certificate expiry, STARTTLS not supported,
STS policy fetch errors, validation failures, and similar — together with
the sending and receiving MTA addresses involved.
+
+Like the DKIM and SPF details tables above, the "SMTP TLS domains" and
+"SMTP TLS failure details" tables show one row per policy and one row per
+failure detail, respectively, using combined `policy (domain / type)` and
+`failure detail (domain / type / result / sending mta / receiving ip / mx)`
+columns so that each policy's or failure detail's fields stay paired
+together, rather than aggregating them as separate columns. The
+`successful_sessions` and `failed_sessions` columns are summed per report
+document, though, not per policy: when a single report carries multiple
+policies, a row's session sums include the sibling policies from that
+report as well as its own. Fully attributing session counts to a single
+policy would require restructuring the stored documents.
diff --git a/parsedmarc/cli.py b/parsedmarc/cli.py
index 73ee41ac..4d3521bd 100644
--- a/parsedmarc/cli.py
+++ b/parsedmarc/cli.py
@@ -1472,6 +1472,7 @@ def _init_output_clients(opts):
elastic.migrate_indexes(
aggregate_indexes=[es_aggregate_index],
failure_indexes=[es_failure_index],
+ smtp_tls_indexes=[es_smtp_tls_index],
)
clients["elasticsearch"] = _ElasticsearchHandle()
except Exception as e:
@@ -1518,6 +1519,7 @@ def _init_output_clients(opts):
opensearch.migrate_indexes(
aggregate_indexes=[os_aggregate_index],
failure_indexes=[os_failure_index],
+ smtp_tls_indexes=[os_smtp_tls_index],
)
clients["opensearch"] = _OpenSearchHandle()
except Exception as e:
diff --git a/parsedmarc/elastic.py b/parsedmarc/elastic.py
index 1af3c9a3..758c11fd 100644
--- a/parsedmarc/elastic.py
+++ b/parsedmarc/elastic.py
@@ -41,6 +41,181 @@ _SERVERLESS = False
# settings (e.g. ``refresh_interval``) are accepted and pass through.
_SERVERLESS_REJECTED_SETTINGS = frozenset({"number_of_shards", "number_of_replicas"})
+# Guard query for the dkim_results_combined/spf_results_combined backfill
+# (see ``migrate_indexes``). Matches only documents that have at least one
+# DKIM or SPF auth result and are missing the corresponding combined field.
+# Empty arrays are invisible to ``exists``, so documents with zero
+# DKIM/SPF results are correctly skipped (verified against real data;
+# this also makes the query idempotent — a backfilled document no longer
+# matches). Each result is matched on an OR of its ``domain``/``result``
+# subfields as defense in depth: the parsers we audited never store a
+# result without both, but an empty string indexes no text tokens and is
+# invisible to ``exists``, and the storage shape of every historical
+# parsedmarc version can't be audited — matching either subfield costs
+# nothing and cannot skip a document that has something to backfill.
+_COMBINED_BACKFILL_QUERY: dict[str, Any] = {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {
+ "bool": {
+ "must": [
+ {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {"exists": {"field": "dkim_results.domain"}},
+ {"exists": {"field": "dkim_results.result"}},
+ ],
+ }
+ }
+ ],
+ "must_not": [{"exists": {"field": "dkim_results_combined"}}],
+ }
+ },
+ {
+ "bool": {
+ "must": [
+ {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {"exists": {"field": "spf_results.domain"}},
+ {"exists": {"field": "spf_results.result"}},
+ ],
+ }
+ }
+ ],
+ "must_not": [{"exists": {"field": "spf_results_combined"}}],
+ }
+ },
+ ],
+ }
+}
+
+# Painless script that (re)derives dkim_results_combined/spf_results_combined
+# from dkim_results/spf_results, matching the format written by
+# save_aggregate_report_to_elasticsearch(): "{selector} / {domain} / {result}"
+# per DKIM result and "{scope} / {domain} / {result}" per SPF result.
+_COMBINED_BACKFILL_SCRIPT = (
+ "List dk = new ArrayList(); "
+ "def dr = ctx._source.dkim_results; "
+ "if (dr != null) { "
+ "if (!(dr instanceof List)) { dr = [dr]; } "
+ "for (e in dr) { "
+ "if (e == null) { continue; } "
+ 'def sel = e.selector != null ? e.selector : "none"; '
+ 'def dom = e.domain != null ? e.domain : "none"; '
+ 'def res = e.result != null ? e.result : "none"; '
+ 'dk.add(sel + " / " + dom + " / " + res); '
+ "} } "
+ "ctx._source.dkim_results_combined = dk; "
+ "List sp = new ArrayList(); "
+ "def sr = ctx._source.spf_results; "
+ "if (sr != null) { "
+ "if (!(sr instanceof List)) { sr = [sr]; } "
+ "for (e in sr) { "
+ "if (e == null) { continue; } "
+ 'def sc = e.scope != null ? e.scope : "mfrom"; '
+ 'def dom = e.domain != null ? e.domain : "none"; '
+ 'def res = e.result != null ? e.result : (e.results != null ? e.results : "none"); '
+ 'sp.add(sc + " / " + dom + " / " + res); '
+ "} } "
+ "ctx._source.spf_results_combined = sp;"
+)
+
+# Guard query for the policies_combined/failure_details_combined backfill
+# (see ``migrate_indexes``). Matches only SMTP TLS documents that have at
+# least one policy or failure detail and are missing the corresponding
+# combined field. Empty arrays are invisible to ``exists``, so documents
+# with zero policies/failure details are correctly skipped (this also
+# makes the query idempotent — a backfilled document no longer matches).
+# Each result is matched on an OR of its relevant subfields as defense in
+# depth: the parsers we audited never store a policy/failure detail
+# without these fields, but an empty string indexes no text tokens and is
+# invisible to ``exists``, and the storage shape of every historical
+# parsedmarc version can't be audited — matching either subfield costs
+# nothing and cannot skip a document that has something to backfill.
+_SMTP_TLS_COMBINED_BACKFILL_QUERY: dict[str, Any] = {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {
+ "bool": {
+ "must": [
+ {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {"exists": {"field": "policies.policy_domain"}},
+ {"exists": {"field": "policies.policy_type"}},
+ ],
+ }
+ }
+ ],
+ "must_not": [{"exists": {"field": "policies_combined"}}],
+ }
+ },
+ {
+ "bool": {
+ "must": [
+ {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {
+ "exists": {
+ "field": "policies.failure_details.result_type"
+ }
+ },
+ {
+ "exists": {
+ "field": "policies.failure_details.sending_mta_ip"
+ }
+ },
+ ],
+ }
+ }
+ ],
+ "must_not": [{"exists": {"field": "failure_details_combined"}}],
+ }
+ },
+ ],
+ }
+}
+
+# Painless script that (re)derives policies_combined/failure_details_combined
+# from policies/policies.failure_details, matching the format written by
+# save_smtp_tls_report_to_elasticsearch(): "{policy_domain} / {policy_type}"
+# per policy and "{policy_domain} / {policy_type} / {result_type} /
+# {sending_mta_ip} / {receiving_ip} / {receiving_mx_hostname}" per failure
+# detail.
+_SMTP_TLS_COMBINED_BACKFILL_SCRIPT = (
+ "List pols = new ArrayList(); "
+ "List dets = new ArrayList(); "
+ "def ps = ctx._source.policies; "
+ "if (ps != null) { "
+ "if (!(ps instanceof List)) { ps = [ps]; } "
+ "for (p in ps) { "
+ "if (p == null) { continue; } "
+ 'def dom = p.policy_domain != null ? p.policy_domain : "none"; '
+ 'def typ = p.policy_type != null ? p.policy_type : "none"; '
+ 'pols.add(dom + " / " + typ); '
+ "def fds = p.failure_details; "
+ "if (fds != null) { "
+ "if (!(fds instanceof List)) { fds = [fds]; } "
+ "for (f in fds) { "
+ "if (f == null) { continue; } "
+ 'def rt = f.result_type != null ? f.result_type : "none"; '
+ 'def smi = f.sending_mta_ip != null ? f.sending_mta_ip : "none"; '
+ 'def ri = f.receiving_ip != null ? f.receiving_ip : "none"; '
+ 'def rmh = f.receiving_mx_hostname != null ? f.receiving_mx_hostname : "none"; '
+ 'dets.add(dom + " / " + typ + " / " + rt + " / " + smi + " / " + ri + " / " + rmh); '
+ "} } } } "
+ "ctx._source.policies_combined = pols; "
+ "ctx._source.failure_details_combined = dets;"
+)
+
class _PolicyOverride(InnerDoc):
# The elasticsearch.dsl 8.x type stubs use dataclass_transform and only
@@ -99,7 +274,7 @@ class _SPFResult(InnerDoc):
domain = Text()
scope = Text()
- results = Text()
+ result = Text()
human_result = Text()
@@ -144,8 +319,25 @@ class _AggregateReportDoc(Document):
header_from = Text()
envelope_from = Text()
envelope_to = Text()
+ # Nested(...) on the two auth-result fields below is only the DSL's
+ # in-memory document shape; it is never installed as a mapping.
+ # create_indexes() deliberately skips Index.document() registration so
+ # these fields stay dynamic-mapped as plain `object` in the cluster
+ # (see the comment there and issue #169).
dkim_results = Nested(_DKIMResult)
spf_results = Nested(_SPFResult)
+ # One "{selector} / {domain} / {result}" (DKIM) or "{scope} / {domain} /
+ # {result}" (SPF) string per auth result. Kibana/Grafana tables cannot
+ # terms-aggregate the subfields of an object array without producing a
+ # cross-product of values (issue #169), so dashboards aggregate these
+ # composed keywords instead. Declared to match what dynamic mapping
+ # produces for a string array (text + .keyword).
+ dkim_results_combined = Text(
+ multi=True, fields={"keyword": Keyword(ignore_above=256)}
+ )
+ spf_results_combined = Text(
+ multi=True, fields={"keyword": Keyword(ignore_above=256)}
+ )
np = Keyword()
testing = Keyword()
discovery_method = Keyword()
@@ -158,7 +350,7 @@ class _AggregateReportDoc(Document):
self,
domain: str,
selector: str,
- result: _DKIMResult,
+ result: str,
human_result: str | None = None,
):
self.dkim_results.append(
@@ -169,12 +361,15 @@ class _AggregateReportDoc(Document):
human_result=human_result,
)
)
+ self.dkim_results_combined.append(
+ "{0} / {1} / {2}".format(selector, domain, result)
+ )
def add_spf_result(
self,
domain: str,
scope: str,
- result: _SPFResult,
+ result: str,
human_result: str | None = None,
):
self.spf_results.append(
@@ -185,6 +380,9 @@ class _AggregateReportDoc(Document):
human_result=human_result,
)
)
+ self.spf_results_combined.append(
+ "{0} / {1} / {2}".format(scope, domain, result)
+ )
def save(self, **kwargs): # pyright: ignore[reportIncompatibleMethodOverride]
self.passed_dmarc = False
@@ -301,6 +499,7 @@ class _SMTPTLSFailureDetailsDoc(InnerDoc):
result_type = Text()
sending_mta_ip = Ip()
receiving_mx_helo = Text()
+ receiving_mx_hostname = Text()
receiving_ip = Ip()
failed_session_count = Integer()
additional_information_uri = Text()
@@ -342,7 +541,7 @@ class _SMTPTLSPolicyDoc(InnerDoc):
receiving_mx_helo=receiving_mx_helo,
receiving_ip=receiving_ip,
failed_session_count=failed_session_count,
- additional_information=additional_information_uri,
+ additional_information_uri=additional_information_uri,
failure_reason_code=failure_reason_code,
)
self.failure_details.append(_details)
@@ -365,6 +564,19 @@ class _SMTPTLSReportDoc(Document):
contact_info = Text()
report_id = Text()
policies = Nested(_SMTPTLSPolicyDoc)
+ # One "{policy_domain} / {policy_type}" string per policy. Kibana/
+ # Grafana tables cannot terms-aggregate the subfields of an object
+ # array without producing a cross-product of values (issue #169), so
+ # dashboards aggregate these composed keywords instead. Declared to
+ # match what dynamic mapping produces for a string array (text +
+ # .keyword).
+ policies_combined = Text(multi=True, fields={"keyword": Keyword(ignore_above=256)})
+ # One "{policy_domain} / {policy_type} / {result_type} /
+ # {sending_mta_ip} / {receiving_ip} / {receiving_mx_hostname}" string
+ # per failure detail, across all policies.
+ failure_details_combined = Text(
+ multi=True, fields={"keyword": Keyword(ignore_above=256)}
+ )
class AlreadySaved(ValueError):
@@ -450,6 +662,18 @@ def create_indexes(names: list[str], settings: dict[str, Any] | None = None):
for name in names:
index = Index(name)
try:
+ # Deliberately no Index.document() registration: the shipped
+ # dashboards cannot rebuild their detail tables on a `nested`
+ # mapping — Kibana/OSD visual editors do not support nested
+ # fields, Vega can run nested aggregations but does not render
+ # tables, and Grafana's nested bucket aggregation (9.4+) lacks
+ # reverse_nested for parent-level metrics like message_count —
+ # so the dynamic `object` mapping produced by a bare create is
+ # load-bearing for the shipped dashboards. _AggregateReportDoc
+ # still declares dkim_results/spf_results with Nested(...), but
+ # that is only the DSL's in-memory shape for building documents
+ # — it is never installed as a mapping. See issue #169 and the
+ # *_combined fields on _AggregateReportDoc.
if not index.exists():
logger.debug("Creating Elasticsearch index: {0}".format(name))
if effective_settings:
@@ -462,25 +686,118 @@ def create_indexes(names: list[str], settings: dict[str, Any] | None = None):
def migrate_indexes(
aggregate_indexes: list[str] | None = None,
failure_indexes: list[str] | None = None,
+ smtp_tls_indexes: list[str] | None = None,
):
"""
- Updates index mappings
+ Backfills the ``dkim_results_combined``/``spf_results_combined`` fields
+ (added for issue #169) on aggregate report documents, and the
+ ``policies_combined``/``failure_details_combined`` fields on SMTP TLS
+ report documents, that were saved before those fields existed.
- This is a no-op kept for API compatibility (``cli.py`` calls it on
- startup). The only migration this function ever performed was
- re-typing ``published_policy.fo`` from ``long`` to ``text``, which
- applied exclusively to indices still carrying the legacy
- Elasticsearch 6-era ``"doc"`` mapping type. The 8.x client can only
- reach servers (Elasticsearch 8.x/9.x) whose indices were created on
- Elasticsearch 7.x or later and are therefore typeless, so that
- migration path is unreachable and has been removed.
+ For each name in ``aggregate_indexes``/``smtp_tls_indexes``, this
+ submits an ``update_by_query`` against the ``f"{name}*"`` index pattern
+ (the real indexes are date-suffixed) as a non-blocking background task
+ (``wait_for_completion=False``), so it never delays parsedmarc startup.
+ Submission is guarded by a cheap ``count`` query that only matches
+ documents with DKIM/SPF results (or policies/failure details) but no
+ combined field, so once an index is fully backfilled, later calls are a
+ fast no-op. Any error talking to the cluster (e.g. no indexes yet on a
+ fresh install, or a transient connection issue) is caught and logged as
+ a warning rather than raised; the backfill is simply retried on the
+ next startup, and the manual ``_update_by_query`` commands documented in
+ ``docs/source/elasticsearch.md`` remain available in the meantime.
Args:
aggregate_indexes (list): A list of aggregate index names
- (accepted for API compatibility; unused)
failure_indexes (list): A list of failure index names
(accepted for API compatibility; unused)
+ smtp_tls_indexes (list): A list of SMTP TLS index names
"""
+ if not aggregate_indexes and not smtp_tls_indexes:
+ return
+
+ try:
+ client = connections.get_connection()
+ except Exception as e:
+ logger.warning(
+ "Skipping the dkim_results_combined/spf_results_combined/"
+ "policies_combined/failure_details_combined backfill: could "
+ f"not get an Elasticsearch connection: {e}. This will be "
+ "retried at the next startup."
+ )
+ return
+ for name in aggregate_indexes or []:
+ pattern = f"{name}*"
+ try:
+ count_response = client.count(
+ index=pattern,
+ query=_COMBINED_BACKFILL_QUERY,
+ ignore_unavailable=True,
+ allow_no_indices=True,
+ )
+ count = count_response["count"]
+ if not count:
+ continue
+ update_response = client.update_by_query(
+ index=pattern,
+ query=_COMBINED_BACKFILL_QUERY,
+ script={"source": _COMBINED_BACKFILL_SCRIPT, "lang": "painless"},
+ conflicts="proceed",
+ wait_for_completion=False,
+ ignore_unavailable=True,
+ allow_no_indices=True,
+ )
+ task_id = update_response.get("task")
+ logger.info(
+ "Backfilling dkim_results_combined/spf_results_combined on "
+ f"{count} existing documents in {pattern} (task {task_id})"
+ )
+ except Exception as e:
+ logger.warning(
+ "Failed to check/submit the dkim_results_combined/"
+ f"spf_results_combined backfill for {pattern}: {e}. This "
+ "will be retried at the next startup; the manual "
+ "_update_by_query command in the documentation remains "
+ "available in the meantime."
+ )
+
+ for name in smtp_tls_indexes or []:
+ pattern = f"{name}*"
+ try:
+ count_response = client.count(
+ index=pattern,
+ query=_SMTP_TLS_COMBINED_BACKFILL_QUERY,
+ ignore_unavailable=True,
+ allow_no_indices=True,
+ )
+ count = count_response["count"]
+ if not count:
+ continue
+ update_response = client.update_by_query(
+ index=pattern,
+ query=_SMTP_TLS_COMBINED_BACKFILL_QUERY,
+ script={
+ "source": _SMTP_TLS_COMBINED_BACKFILL_SCRIPT,
+ "lang": "painless",
+ },
+ conflicts="proceed",
+ wait_for_completion=False,
+ ignore_unavailable=True,
+ allow_no_indices=True,
+ )
+ task_id = update_response.get("task")
+ logger.info(
+ "Backfilling policies_combined/failure_details_combined on "
+ f"{count} existing documents in {pattern} (task {task_id})"
+ )
+ except Exception as e:
+ logger.warning(
+ "Failed to check/submit the policies_combined/"
+ f"failure_details_combined backfill for {pattern}: {e}. "
+ "This will be retried at the next startup; the manual "
+ "_update_by_query command in the documentation remains "
+ "available in the meantime."
+ )
def save_aggregate_report_to_elasticsearch(
@@ -949,6 +1266,16 @@ def save_smtp_tls_report_to_elasticsearch(
policy_strings = policy["policy_strings"]
if "mx_host_patterns" in policy:
mx_host_patterns = policy["mx_host_patterns"]
+ # policies_combined/failure_details_combined: see the field
+ # declarations on _SMTPTLSReportDoc and issue #169. policies and
+ # their failure_details are object arrays with the same
+ # cross-product problem as dkim_results/spf_results, so dashboards
+ # aggregate these composed strings instead of the raw subfields.
+ policy_domain_combined = policy.get("policy_domain") or "none"
+ policy_type_combined = policy.get("policy_type") or "none"
+ smtp_tls_doc.policies_combined.append(
+ "{0} / {1}".format(policy_domain_combined, policy_type_combined)
+ )
policy_doc = _SMTPTLSPolicyDoc(
policy_domain=policy["policy_domain"],
policy_type=policy["policy_type"],
@@ -969,7 +1296,12 @@ def save_smtp_tls_report_to_elasticsearch(
if "receiving_mx_hostname" in failure_detail:
receiving_mx_hostname = failure_detail["receiving_mx_hostname"]
- if "additional_information_uri" in failure_detail:
+ # The parser's key is additional_info_uri (see
+ # SMTPTLSFailureDetailsOptional in types.py); accept the
+ # long-form key too for dicts built by other callers.
+ if "additional_info_uri" in failure_detail:
+ additional_information_uri = failure_detail["additional_info_uri"]
+ elif "additional_information_uri" in failure_detail:
additional_information_uri = failure_detail[
"additional_information_uri"
]
@@ -994,6 +1326,16 @@ def save_smtp_tls_report_to_elasticsearch(
additional_information_uri=additional_information_uri,
failure_reason_code=failure_reason_code,
)
+ smtp_tls_doc.failure_details_combined.append(
+ "{0} / {1} / {2} / {3} / {4} / {5}".format(
+ policy_domain_combined,
+ policy_type_combined,
+ failure_detail.get("result_type") or "none",
+ sending_mta_ip or "none",
+ receiving_ip or "none",
+ receiving_mx_hostname or "none",
+ )
+ )
smtp_tls_doc.policies.append(policy_doc)
create_indexes([index], index_settings)
diff --git a/parsedmarc/opensearch.py b/parsedmarc/opensearch.py
index fecefc83..adc3e182 100644
--- a/parsedmarc/opensearch.py
+++ b/parsedmarc/opensearch.py
@@ -34,6 +34,182 @@ class OpenSearchError(Exception):
"""Raised when an OpenSearch error occurs"""
+# Guard query for the dkim_results_combined/spf_results_combined backfill
+# (see ``migrate_indexes``). Matches only documents that have at least one
+# DKIM or SPF auth result and are missing the corresponding combined field.
+# Empty arrays are invisible to ``exists``, so documents with zero
+# DKIM/SPF results are correctly skipped (verified against real data;
+# this also makes the query idempotent — a backfilled document no longer
+# matches). Each result is matched on an OR of its ``domain``/``result``
+# subfields as defense in depth: the parsers we audited never store a
+# result without both, but an empty string indexes no text tokens and is
+# invisible to ``exists``, and the storage shape of every historical
+# parsedmarc version can't be audited — matching either subfield costs
+# nothing and cannot skip a document that has something to backfill.
+_COMBINED_BACKFILL_QUERY: dict[str, Any] = {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {
+ "bool": {
+ "must": [
+ {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {"exists": {"field": "dkim_results.domain"}},
+ {"exists": {"field": "dkim_results.result"}},
+ ],
+ }
+ }
+ ],
+ "must_not": [{"exists": {"field": "dkim_results_combined"}}],
+ }
+ },
+ {
+ "bool": {
+ "must": [
+ {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {"exists": {"field": "spf_results.domain"}},
+ {"exists": {"field": "spf_results.result"}},
+ ],
+ }
+ }
+ ],
+ "must_not": [{"exists": {"field": "spf_results_combined"}}],
+ }
+ },
+ ],
+ }
+}
+
+# Painless script that (re)derives dkim_results_combined/spf_results_combined
+# from dkim_results/spf_results, matching the format written by
+# save_aggregate_report_to_opensearch(): "{selector} / {domain} / {result}"
+# per DKIM result and "{scope} / {domain} / {result}" per SPF result.
+_COMBINED_BACKFILL_SCRIPT = (
+ "List dk = new ArrayList(); "
+ "def dr = ctx._source.dkim_results; "
+ "if (dr != null) { "
+ "if (!(dr instanceof List)) { dr = [dr]; } "
+ "for (e in dr) { "
+ "if (e == null) { continue; } "
+ 'def sel = e.selector != null ? e.selector : "none"; '
+ 'def dom = e.domain != null ? e.domain : "none"; '
+ 'def res = e.result != null ? e.result : "none"; '
+ 'dk.add(sel + " / " + dom + " / " + res); '
+ "} } "
+ "ctx._source.dkim_results_combined = dk; "
+ "List sp = new ArrayList(); "
+ "def sr = ctx._source.spf_results; "
+ "if (sr != null) { "
+ "if (!(sr instanceof List)) { sr = [sr]; } "
+ "for (e in sr) { "
+ "if (e == null) { continue; } "
+ 'def sc = e.scope != null ? e.scope : "mfrom"; '
+ 'def dom = e.domain != null ? e.domain : "none"; '
+ 'def res = e.result != null ? e.result : (e.results != null ? e.results : "none"); '
+ 'sp.add(sc + " / " + dom + " / " + res); '
+ "} } "
+ "ctx._source.spf_results_combined = sp;"
+)
+
+# Guard query for the policies_combined/failure_details_combined backfill
+# (see ``migrate_indexes``). Matches only SMTP TLS documents that have at
+# least one policy or failure detail and are missing the corresponding
+# combined field. Empty arrays are invisible to ``exists``, so documents
+# with zero policies/failure details are correctly skipped (this also
+# makes the query idempotent — a backfilled document no longer matches).
+# Each result is matched on an OR of its relevant subfields as defense in
+# depth: the parsers we audited never store a policy/failure detail
+# without these fields, but an empty string indexes no text tokens and is
+# invisible to ``exists``, and the storage shape of every historical
+# parsedmarc version can't be audited — matching either subfield costs
+# nothing and cannot skip a document that has something to backfill.
+_SMTP_TLS_COMBINED_BACKFILL_QUERY: dict[str, Any] = {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {
+ "bool": {
+ "must": [
+ {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {"exists": {"field": "policies.policy_domain"}},
+ {"exists": {"field": "policies.policy_type"}},
+ ],
+ }
+ }
+ ],
+ "must_not": [{"exists": {"field": "policies_combined"}}],
+ }
+ },
+ {
+ "bool": {
+ "must": [
+ {
+ "bool": {
+ "minimum_should_match": 1,
+ "should": [
+ {
+ "exists": {
+ "field": "policies.failure_details.result_type"
+ }
+ },
+ {
+ "exists": {
+ "field": "policies.failure_details.sending_mta_ip"
+ }
+ },
+ ],
+ }
+ }
+ ],
+ "must_not": [{"exists": {"field": "failure_details_combined"}}],
+ }
+ },
+ ],
+ }
+}
+
+# Painless script that (re)derives policies_combined/failure_details_combined
+# from policies/policies.failure_details, matching the format written by
+# save_smtp_tls_report_to_opensearch(): "{policy_domain} / {policy_type}"
+# per policy and "{policy_domain} / {policy_type} / {result_type} /
+# {sending_mta_ip} / {receiving_ip} / {receiving_mx_hostname}" per failure
+# detail.
+_SMTP_TLS_COMBINED_BACKFILL_SCRIPT = (
+ "List pols = new ArrayList(); "
+ "List dets = new ArrayList(); "
+ "def ps = ctx._source.policies; "
+ "if (ps != null) { "
+ "if (!(ps instanceof List)) { ps = [ps]; } "
+ "for (p in ps) { "
+ "if (p == null) { continue; } "
+ 'def dom = p.policy_domain != null ? p.policy_domain : "none"; '
+ 'def typ = p.policy_type != null ? p.policy_type : "none"; '
+ 'pols.add(dom + " / " + typ); '
+ "def fds = p.failure_details; "
+ "if (fds != null) { "
+ "if (!(fds instanceof List)) { fds = [fds]; } "
+ "for (f in fds) { "
+ "if (f == null) { continue; } "
+ 'def rt = f.result_type != null ? f.result_type : "none"; '
+ 'def smi = f.sending_mta_ip != null ? f.sending_mta_ip : "none"; '
+ 'def ri = f.receiving_ip != null ? f.receiving_ip : "none"; '
+ 'def rmh = f.receiving_mx_hostname != null ? f.receiving_mx_hostname : "none"; '
+ 'dets.add(dom + " / " + typ + " / " + rt + " / " + smi + " / " + ri + " / " + rmh); '
+ "} } } } "
+ "ctx._source.policies_combined = pols; "
+ "ctx._source.failure_details_combined = dets;"
+)
+
+
class _PolicyOverride(InnerDoc):
type = Text()
comment = Text()
@@ -62,7 +238,7 @@ class _DKIMResult(InnerDoc):
class _SPFResult(InnerDoc):
domain = Text()
scope = Text()
- results = Text()
+ result = Text()
human_result = Text()
@@ -101,8 +277,25 @@ class _AggregateReportDoc(Document):
header_from = Text()
envelope_from = Text()
envelope_to = Text()
+ # Nested(...) on the two auth-result fields below is only the DSL's
+ # in-memory document shape; it is never installed as a mapping.
+ # create_indexes() deliberately skips Index.document() registration so
+ # these fields stay dynamic-mapped as plain `object` in the cluster
+ # (see the comment there and issue #169).
dkim_results = Nested(_DKIMResult)
spf_results = Nested(_SPFResult)
+ # One "{selector} / {domain} / {result}" (DKIM) or "{scope} / {domain} /
+ # {result}" (SPF) string per auth result. Kibana/Grafana tables cannot
+ # terms-aggregate the subfields of an object array without producing a
+ # cross-product of values (issue #169), so dashboards aggregate these
+ # composed keywords instead. Declared to match what dynamic mapping
+ # produces for a string array (text + .keyword).
+ dkim_results_combined = Text(
+ multi=True, fields={"keyword": Keyword(ignore_above=256)}
+ )
+ spf_results_combined = Text(
+ multi=True, fields={"keyword": Keyword(ignore_above=256)}
+ )
np = Keyword()
testing = Keyword()
discovery_method = Keyword()
@@ -115,7 +308,7 @@ class _AggregateReportDoc(Document):
self,
domain: str,
selector: str,
- result: _DKIMResult,
+ result: str,
human_result: str | None = None,
):
self.dkim_results.append(
@@ -126,12 +319,15 @@ class _AggregateReportDoc(Document):
human_result=human_result,
)
)
+ self.dkim_results_combined.append(
+ "{0} / {1} / {2}".format(selector, domain, result)
+ )
def add_spf_result(
self,
domain: str,
scope: str,
- result: _SPFResult,
+ result: str,
human_result: str | None = None,
):
self.spf_results.append(
@@ -142,6 +338,9 @@ class _AggregateReportDoc(Document):
human_result=human_result,
)
)
+ self.spf_results_combined.append(
+ "{0} / {1} / {2}".format(scope, domain, result)
+ )
def save(self, **kwargs): # pyright: ignore[reportIncompatibleMethodOverride]
self.passed_dmarc = False
@@ -228,6 +427,7 @@ class _SMTPTLSFailureDetailsDoc(InnerDoc):
result_type = Text()
sending_mta_ip = Ip()
receiving_mx_helo = Text()
+ receiving_mx_hostname = Text()
receiving_ip = Ip()
failed_session_count = Integer()
additional_information_uri = Text()
@@ -263,7 +463,7 @@ class _SMTPTLSPolicyDoc(InnerDoc):
receiving_mx_helo=receiving_mx_helo,
receiving_ip=receiving_ip,
failed_session_count=failed_session_count,
- additional_information=additional_information_uri,
+ additional_information_uri=additional_information_uri,
failure_reason_code=failure_reason_code,
)
self.failure_details.append(_details)
@@ -280,6 +480,19 @@ class _SMTPTLSReportDoc(Document):
contact_info = Text()
report_id = Text()
policies = Nested(_SMTPTLSPolicyDoc)
+ # One "{policy_domain} / {policy_type}" string per policy. Kibana/
+ # Grafana tables cannot terms-aggregate the subfields of an object
+ # array without producing a cross-product of values (issue #169), so
+ # dashboards aggregate these composed keywords instead. Declared to
+ # match what dynamic mapping produces for a string array (text +
+ # .keyword).
+ policies_combined = Text(multi=True, fields={"keyword": Keyword(ignore_above=256)})
+ # One "{policy_domain} / {policy_type} / {result_type} /
+ # {sending_mta_ip} / {receiving_ip} / {receiving_mx_hostname}" string
+ # per failure detail, across all policies.
+ failure_details_combined = Text(
+ multi=True, fields={"keyword": Keyword(ignore_above=256)}
+ )
class AlreadySaved(ValueError):
@@ -367,6 +580,18 @@ def create_indexes(names: list[str], settings: dict[str, Any] | None = None):
for name in names:
index = Index(name)
try:
+ # Deliberately no Index.document() registration: the shipped
+ # dashboards cannot rebuild their detail tables on a `nested`
+ # mapping — Kibana/OSD visual editors do not support nested
+ # fields, Vega can run nested aggregations but does not render
+ # tables, and Grafana's nested bucket aggregation (9.4+) lacks
+ # reverse_nested for parent-level metrics like message_count —
+ # so the dynamic `object` mapping produced by a bare create is
+ # load-bearing for the shipped dashboards. _AggregateReportDoc
+ # still declares dkim_results/spf_results with Nested(...), but
+ # that is only the DSL's in-memory shape for building documents
+ # — it is never installed as a mapping. See issue #169 and the
+ # *_combined fields on _AggregateReportDoc.
if not index.exists():
logger.debug("Creating OpenSearch index: {0}".format(name))
if settings is None:
@@ -381,47 +606,175 @@ def create_indexes(names: list[str], settings: dict[str, Any] | None = None):
def migrate_indexes(
aggregate_indexes: list[str] | None = None,
failure_indexes: list[str] | None = None,
+ smtp_tls_indexes: list[str] | None = None,
):
"""
- Updates index mappings
+ Runs index migrations and backfills.
+
+ First, the legacy ``published_policy.fo`` migration: indexes where that
+ field was mapped as ``long`` (data indexed by very old parsedmarc
+ releases under the Elasticsearch 6-era ``doc`` mapping type) are rebuilt
+ as a ``-v2`` index with the text/keyword shape.
+
+ Second, the ``dkim_results_combined``/``spf_results_combined`` backfill
+ (added for issue #169) for aggregate report documents that were saved
+ before those fields existed. For each name in ``aggregate_indexes``,
+ this submits an ``update_by_query`` against the ``f"{name}*"`` index
+ pattern (the real indexes are date-suffixed) as a non-blocking
+ background task (``wait_for_completion=False``), so it never delays
+ parsedmarc startup. Submission is guarded by a cheap ``count`` query
+ that only matches documents with DKIM/SPF results but no combined
+ field, so once an index is fully backfilled, later calls are a fast
+ no-op. Any error talking to the cluster (e.g. no indexes yet on a
+ fresh install, or a transient connection issue) is caught and logged
+ as a warning rather than raised; the backfill is simply retried on the
+ next startup, and the manual ``_update_by_query`` command documented
+ in ``docs/source/elasticsearch.md`` remains available in the meantime.
+
+ Third, the same treatment for the ``policies_combined``/
+ ``failure_details_combined`` fields (same issue #169) on SMTP TLS
+ report documents, for each name in ``smtp_tls_indexes``.
Args:
aggregate_indexes (list): A list of aggregate index names
failure_indexes (list): A list of failure index names
(accepted for API compatibility; no migrations are
currently needed for failure indexes)
+ smtp_tls_indexes (list): A list of SMTP TLS index names
"""
- version = 2
- if aggregate_indexes is None:
- aggregate_indexes = []
- for aggregate_index_name in aggregate_indexes:
- if not Index(aggregate_index_name).exists():
- continue
- aggregate_index = Index(aggregate_index_name)
- doc = "doc"
- fo_field = "published_policy.fo"
- fo = "fo"
- fo_mapping = aggregate_index.get_field_mapping(fields=[fo_field])
- fo_mapping = fo_mapping[list(fo_mapping.keys())[0]]["mappings"]
- if doc not in fo_mapping:
- continue
+ if not aggregate_indexes and not smtp_tls_indexes:
+ return
- fo_mapping = fo_mapping[doc][fo_field]["mapping"][fo]
- fo_type = fo_mapping["type"]
- if fo_type == "long":
- new_index_name = "{0}-v{1}".format(aggregate_index_name, version)
- body = {
- "properties": {
- "published_policy.fo": {
- "type": "text",
- "fields": {"keyword": {"type": "keyword", "ignore_above": 256}},
+ version = 2
+ for aggregate_index_name in aggregate_indexes or []:
+ try:
+ if not Index(aggregate_index_name).exists():
+ continue
+ aggregate_index = Index(aggregate_index_name)
+ doc = "doc"
+ fo_field = "published_policy.fo"
+ fo = "fo"
+ fo_mapping = aggregate_index.get_field_mapping(fields=[fo_field])
+ fo_mapping = fo_mapping[list(fo_mapping.keys())[0]]["mappings"]
+ if doc not in fo_mapping:
+ continue
+
+ fo_mapping = fo_mapping[doc][fo_field]["mapping"][fo]
+ fo_type = fo_mapping["type"]
+ if fo_type == "long":
+ new_index_name = "{0}-v{1}".format(aggregate_index_name, version)
+ body = {
+ "properties": {
+ "published_policy.fo": {
+ "type": "text",
+ "fields": {
+ "keyword": {"type": "keyword", "ignore_above": 256}
+ },
+ }
}
}
- }
- Index(new_index_name).create()
- Index(new_index_name).put_mapping(doc_type=doc, body=body)
- reindex(connections.get_connection(), aggregate_index_name, new_index_name)
- Index(aggregate_index_name).delete()
+ Index(new_index_name).create()
+ Index(new_index_name).put_mapping(doc_type=doc, body=body)
+ reindex(
+ connections.get_connection(), aggregate_index_name, new_index_name
+ )
+ Index(aggregate_index_name).delete()
+ except Exception as e:
+ logger.warning(
+ "Failed the legacy published_policy.fo migration for "
+ f"{aggregate_index_name}: {e}. This will be retried at the "
+ "next startup."
+ )
+
+ try:
+ client = connections.get_connection()
+ except Exception as e:
+ logger.warning(
+ "Skipping the dkim_results_combined/spf_results_combined/"
+ "policies_combined/failure_details_combined backfill: could "
+ f"not get an OpenSearch connection: {e}. This will be retried "
+ "at the next startup."
+ )
+ return
+ for name in aggregate_indexes or []:
+ pattern = f"{name}*"
+ try:
+ count_response = client.count(
+ index=pattern,
+ body={"query": _COMBINED_BACKFILL_QUERY},
+ ignore_unavailable=True,
+ allow_no_indices=True,
+ )
+ count = count_response["count"]
+ if not count:
+ continue
+ update_response = client.update_by_query(
+ index=pattern,
+ body={
+ "query": _COMBINED_BACKFILL_QUERY,
+ "script": {
+ "source": _COMBINED_BACKFILL_SCRIPT,
+ "lang": "painless",
+ },
+ },
+ conflicts="proceed",
+ wait_for_completion=False,
+ ignore_unavailable=True,
+ allow_no_indices=True,
+ )
+ task_id = update_response.get("task")
+ logger.info(
+ "Backfilling dkim_results_combined/spf_results_combined on "
+ f"{count} existing documents in {pattern} (task {task_id})"
+ )
+ except Exception as e:
+ logger.warning(
+ "Failed to check/submit the dkim_results_combined/"
+ f"spf_results_combined backfill for {pattern}: {e}. This "
+ "will be retried at the next startup; the manual "
+ "_update_by_query command in the documentation remains "
+ "available in the meantime."
+ )
+
+ for name in smtp_tls_indexes or []:
+ pattern = f"{name}*"
+ try:
+ count_response = client.count(
+ index=pattern,
+ body={"query": _SMTP_TLS_COMBINED_BACKFILL_QUERY},
+ ignore_unavailable=True,
+ allow_no_indices=True,
+ )
+ count = count_response["count"]
+ if not count:
+ continue
+ update_response = client.update_by_query(
+ index=pattern,
+ body={
+ "query": _SMTP_TLS_COMBINED_BACKFILL_QUERY,
+ "script": {
+ "source": _SMTP_TLS_COMBINED_BACKFILL_SCRIPT,
+ "lang": "painless",
+ },
+ },
+ conflicts="proceed",
+ wait_for_completion=False,
+ ignore_unavailable=True,
+ allow_no_indices=True,
+ )
+ task_id = update_response.get("task")
+ logger.info(
+ "Backfilling policies_combined/failure_details_combined on "
+ f"{count} existing documents in {pattern} (task {task_id})"
+ )
+ except Exception as e:
+ logger.warning(
+ "Failed to check/submit the policies_combined/"
+ f"failure_details_combined backfill for {pattern}: {e}. "
+ "This will be retried at the next startup; the manual "
+ "_update_by_query command in the documentation remains "
+ "available in the meantime."
+ )
def save_aggregate_report_to_opensearch(
@@ -885,6 +1238,16 @@ def save_smtp_tls_report_to_opensearch(
policy_strings = policy["policy_strings"]
if "mx_host_patterns" in policy:
mx_host_patterns = policy["mx_host_patterns"]
+ # policies_combined/failure_details_combined: see the field
+ # declarations on _SMTPTLSReportDoc and issue #169. policies and
+ # their failure_details are object arrays with the same
+ # cross-product problem as dkim_results/spf_results, so dashboards
+ # aggregate these composed strings instead of the raw subfields.
+ policy_domain_combined = policy.get("policy_domain") or "none"
+ policy_type_combined = policy.get("policy_type") or "none"
+ smtp_tls_doc.policies_combined.append(
+ "{0} / {1}".format(policy_domain_combined, policy_type_combined)
+ )
policy_doc = _SMTPTLSPolicyDoc(
policy_domain=policy["policy_domain"],
policy_type=policy["policy_type"],
@@ -905,7 +1268,12 @@ def save_smtp_tls_report_to_opensearch(
if "receiving_mx_hostname" in failure_detail:
receiving_mx_hostname = failure_detail["receiving_mx_hostname"]
- if "additional_information_uri" in failure_detail:
+ # The parser's key is additional_info_uri (see
+ # SMTPTLSFailureDetailsOptional in types.py); accept the
+ # long-form key too for dicts built by other callers.
+ if "additional_info_uri" in failure_detail:
+ additional_information_uri = failure_detail["additional_info_uri"]
+ elif "additional_information_uri" in failure_detail:
additional_information_uri = failure_detail[
"additional_information_uri"
]
@@ -930,6 +1298,16 @@ def save_smtp_tls_report_to_opensearch(
additional_information_uri=additional_information_uri,
failure_reason_code=failure_reason_code,
)
+ smtp_tls_doc.failure_details_combined.append(
+ "{0} / {1} / {2} / {3} / {4} / {5}".format(
+ policy_domain_combined,
+ policy_type_combined,
+ failure_detail.get("result_type") or "none",
+ sending_mta_ip or "none",
+ receiving_ip or "none",
+ receiving_mx_hostname or "none",
+ )
+ )
smtp_tls_doc.policies.append(policy_doc)
create_indexes([index], index_settings)
diff --git a/tests/test_elastic.py b/tests/test_elastic.py
index be1c551a..e739b49c 100644
--- a/tests/test_elastic.py
+++ b/tests/test_elastic.py
@@ -16,6 +16,7 @@ from parsedmarc.elastic import (
AlreadySaved,
ElasticsearchError,
create_indexes,
+ migrate_indexes,
save_aggregate_report_to_elasticsearch,
save_failure_report_to_elasticsearch,
save_smtp_tls_report_to_elasticsearch,
@@ -395,6 +396,143 @@ class TestCreateIndexesServerless(unittest.TestCase):
mock_index.create.assert_called_once()
+# ---------------------------------------------------------------------------
+# migrate_indexes
+# ---------------------------------------------------------------------------
+
+
+class TestMigrateIndexes(unittest.TestCase):
+ """migrate_indexes backfills dkim_results_combined/spf_results_combined
+ (issue #169) on pre-existing aggregate documents as a non-blocking
+ background task. It is guarded by a cheap count() query so repeated
+ startups against an already-backfilled index are a no-op, and any SDK
+ error is caught and logged rather than raised, so it never blocks
+ parsedmarc startup."""
+
+ def test_backfill_submitted_when_old_docs_exist(self):
+ with patch("parsedmarc.elastic.connections.get_connection") as mock_get_conn:
+ mock_client = MagicMock()
+ mock_client.count.return_value = {"count": 42}
+ mock_get_conn.return_value = mock_client
+ migrate_indexes(aggregate_indexes=["dmarc_aggregate"])
+
+ mock_client.update_by_query.assert_called_once()
+ kwargs = mock_client.update_by_query.call_args.kwargs
+ self.assertEqual(kwargs["index"], "dmarc_aggregate*")
+ self.assertEqual(kwargs["conflicts"], "proceed")
+ self.assertFalse(kwargs["wait_for_completion"])
+ self.assertEqual(kwargs["query"], elastic_module._COMBINED_BACKFILL_QUERY)
+ script_source = kwargs["script"]["source"]
+ self.assertIn("ctx._source.dkim_results_combined", script_source)
+ self.assertIn("ctx._source.spf_results_combined", script_source)
+
+ # The count() guard query also targets the date-suffixed pattern.
+ count_kwargs = mock_client.count.call_args.kwargs
+ self.assertEqual(count_kwargs["index"], "dmarc_aggregate*")
+ self.assertEqual(count_kwargs["query"], elastic_module._COMBINED_BACKFILL_QUERY)
+
+ def test_backfill_skipped_when_no_old_docs(self):
+ with patch("parsedmarc.elastic.connections.get_connection") as mock_get_conn:
+ mock_client = MagicMock()
+ mock_client.count.return_value = {"count": 0}
+ mock_get_conn.return_value = mock_client
+ migrate_indexes(aggregate_indexes=["dmarc_aggregate"])
+
+ mock_client.update_by_query.assert_not_called()
+
+ def test_backfill_skipped_when_no_aggregate_indexes(self):
+ with patch("parsedmarc.elastic.connections.get_connection") as mock_get_conn:
+ migrate_indexes()
+ migrate_indexes(aggregate_indexes=None)
+
+ mock_get_conn.assert_not_called()
+
+ def test_backfill_failure_does_not_raise(self):
+ with patch("parsedmarc.elastic.connections.get_connection") as mock_get_conn:
+ mock_client = MagicMock()
+ mock_client.count.side_effect = RuntimeError("cluster unreachable")
+ mock_get_conn.return_value = mock_client
+ with self.assertLogs("parsedmarc.log", level="WARNING") as cm:
+ migrate_indexes(aggregate_indexes=["dmarc_aggregate"])
+
+ self.assertTrue(any("cluster unreachable" in msg for msg in cm.output))
+ mock_client.update_by_query.assert_not_called()
+
+ def test_get_connection_failure_does_not_raise(self):
+ """connections.get_connection() itself sits outside the per-index
+ try/except; if it raises (e.g. no Elasticsearch connection has been
+ configured yet), migrate_indexes must still not propagate the
+ exception, per its docstring's promise that any cluster error is
+ caught and logged."""
+ with patch("parsedmarc.elastic.connections.get_connection") as mock_get_conn:
+ mock_get_conn.side_effect = RuntimeError("no connection")
+ with self.assertLogs("parsedmarc.log", level="WARNING") as cm:
+ migrate_indexes(aggregate_indexes=["dmarc_aggregate"])
+
+ self.assertTrue(
+ any("Skipping the dkim_results_combined" in msg for msg in cm.output)
+ )
+ self.assertTrue(any("no connection" in msg for msg in cm.output))
+
+ def test_smtp_tls_backfill_submitted_when_old_docs_exist(self):
+ """SMTP TLS analogue of test_backfill_submitted_when_old_docs_exist:
+ policies_combined/failure_details_combined backfill (also issue
+ #169) is submitted with its own guard query and painless script."""
+ with patch("parsedmarc.elastic.connections.get_connection") as mock_get_conn:
+ mock_client = MagicMock()
+ mock_client.count.return_value = {"count": 7}
+ mock_get_conn.return_value = mock_client
+ migrate_indexes(smtp_tls_indexes=["smtp_tls"])
+
+ mock_client.update_by_query.assert_called_once()
+ kwargs = mock_client.update_by_query.call_args.kwargs
+ self.assertEqual(kwargs["index"], "smtp_tls*")
+ self.assertEqual(kwargs["conflicts"], "proceed")
+ self.assertFalse(kwargs["wait_for_completion"])
+ self.assertEqual(
+ kwargs["query"], elastic_module._SMTP_TLS_COMBINED_BACKFILL_QUERY
+ )
+ script_source = kwargs["script"]["source"]
+ self.assertIn("ctx._source.policies_combined", script_source)
+ self.assertIn("ctx._source.failure_details_combined", script_source)
+
+ count_kwargs = mock_client.count.call_args.kwargs
+ self.assertEqual(count_kwargs["index"], "smtp_tls*")
+ self.assertEqual(
+ count_kwargs["query"], elastic_module._SMTP_TLS_COMBINED_BACKFILL_QUERY
+ )
+
+ def test_smtp_tls_backfill_skipped_when_no_old_docs(self):
+ with patch("parsedmarc.elastic.connections.get_connection") as mock_get_conn:
+ mock_client = MagicMock()
+ mock_client.count.return_value = {"count": 0}
+ mock_get_conn.return_value = mock_client
+ migrate_indexes(smtp_tls_indexes=["smtp_tls"])
+
+ mock_client.update_by_query.assert_not_called()
+
+ def test_smtp_tls_backfill_skipped_when_no_smtp_tls_indexes_or_aggregate(self):
+ with patch("parsedmarc.elastic.connections.get_connection") as mock_get_conn:
+ migrate_indexes()
+ migrate_indexes(smtp_tls_indexes=None)
+
+ mock_get_conn.assert_not_called()
+
+ def test_smtp_tls_backfill_failure_does_not_raise(self):
+ """SMTP TLS analogue of test_backfill_failure_does_not_raise: an
+ error from the cluster during the smtp_tls_indexes loop is caught
+ and logged rather than raised."""
+ with patch("parsedmarc.elastic.connections.get_connection") as mock_get_conn:
+ mock_client = MagicMock()
+ mock_client.count.side_effect = RuntimeError("cluster unreachable")
+ mock_get_conn.return_value = mock_client
+ with self.assertLogs("parsedmarc.log", level="WARNING") as cm:
+ migrate_indexes(smtp_tls_indexes=["smtp_tls"])
+
+ self.assertTrue(any("cluster unreachable" in msg for msg in cm.output))
+ mock_client.update_by_query.assert_not_called()
+
+
# ---------------------------------------------------------------------------
# save_aggregate_report_to_elasticsearch
# ---------------------------------------------------------------------------
@@ -549,6 +687,54 @@ class TestSaveAggregateReport(unittest.TestCase):
mock_doc_cls.call_args.kwargs["date_begin"].timestamp(), 1705276800
)
+ def test_save_populates_combined_dkim_and_spf_fields(self):
+ """Regression guard for issue #169: two DKIM signatures on one
+ record must yield exactly two combined entries, not a 4-way
+ cross-product. autospec=True is required on the save patch so
+ mock_save.call_args captures the doc instance as ``self``."""
+ report = _aggregate_report()
+ report["records"][0]["auth_results"] = {
+ "dkim": [
+ {
+ "domain": "example.net",
+ "selector": "net1",
+ "result": "fail",
+ "human_result": None,
+ },
+ {
+ "domain": "example.org",
+ "selector": "org1",
+ "result": "pass",
+ "human_result": None,
+ },
+ ],
+ "spf": [
+ {
+ "domain": "example.org",
+ "scope": "mfrom",
+ "result": "pass",
+ "human_result": None,
+ },
+ ],
+ }
+ with (
+ patch("parsedmarc.elastic.Search", return_value=_empty_search()),
+ patch(
+ "parsedmarc.elastic.Index",
+ return_value=MagicMock(exists=MagicMock(return_value=True)),
+ ),
+ patch.object(
+ elastic_module._AggregateReportDoc, "save", autospec=True
+ ) as mock_save,
+ ):
+ save_aggregate_report_to_elasticsearch(report)
+ doc = mock_save.call_args[0][0]
+ self.assertEqual(
+ list(doc.dkim_results_combined),
+ ["net1 / example.net / fail", "org1 / example.org / pass"],
+ )
+ self.assertEqual(list(doc.spf_results_combined), ["mfrom / example.org / pass"])
+
class TestAggregateDocPassedDmarc(unittest.TestCase):
"""The _AggregateReportDoc.save() override derives passed_dmarc — the
@@ -576,6 +762,57 @@ class TestAggregateDocPassedDmarc(unittest.TestCase):
self.assertEqual(bool(doc.passed_dmarc), expected)
+class TestAggregateDocCombinedResults(unittest.TestCase):
+ """add_dkim_result/add_spf_result never touch the network, so these
+ construct _AggregateReportDoc directly rather than going through the
+ save_* entry point."""
+
+ def test_add_dkim_result_appends_combined_string(self):
+ """Regression guard for issue #169: dkim_results/spf_results are
+ arrays of objects that the engine dynamic-maps as plain ``object``
+ (not ``nested``) and flattens, so Kibana/Grafana tables cannot
+ terms-aggregate their subfields without producing a cross-product
+ of selector/domain/result values. The composed
+ "selector / domain / result" string preserves the per-signature
+ pairing that the flattened array loses."""
+ doc = elastic_module._AggregateReportDoc()
+ doc.add_dkim_result(
+ domain="example.net", selector="net1", result="fail", human_result=None
+ )
+ doc.add_dkim_result(
+ domain="example.org", selector="org1", result="pass", human_result=None
+ )
+ expected = ["net1 / example.net / fail", "org1 / example.org / pass"]
+ # dkim_results_combined is declared as Text(multi=True, ...); the SDK
+ # stub types the class attribute as Text (no Iterable protocol),
+ # even though the runtime value is an AttrList once multi=True is
+ # set. Same category of stub gap as the Q()/meta.index ignores in
+ # elastic.py.
+ self.assertEqual(list(doc.dkim_results_combined), expected) # pyright: ignore[reportArgumentType]
+ self.assertEqual(doc.to_dict()["dkim_results_combined"], expected)
+
+ def test_add_spf_result_appends_combined_string(self):
+ doc = elastic_module._AggregateReportDoc()
+ doc.add_spf_result(
+ domain="example.org", scope="mfrom", result="pass", human_result=None
+ )
+ expected = ["mfrom / example.org / pass"]
+ self.assertEqual(list(doc.spf_results_combined), expected) # pyright: ignore[reportArgumentType]
+ self.assertEqual(doc.to_dict()["spf_results_combined"], expected)
+
+ def test_spf_result_serializes_under_singular_result_key(self):
+ """The _SPFResult class previously declared a dead ``results``
+ (plural) field while the save path wrote ``result``; verify the
+ serialized inner doc actually uses the singular key."""
+ doc = elastic_module._AggregateReportDoc()
+ doc.add_spf_result(
+ domain="example.org", scope="mfrom", result="pass", human_result=None
+ )
+ d = doc.to_dict()["spf_results"][0]
+ self.assertEqual(d["result"], "pass")
+ self.assertNotIn("results", d)
+
+
# ---------------------------------------------------------------------------
# save_failure_report_to_elasticsearch
# ---------------------------------------------------------------------------
@@ -930,6 +1167,82 @@ class TestSaveSmtpTlsReport(unittest.TestCase):
save_smtp_tls_report_to_elasticsearch(report)
mock_save.assert_called_once()
+ def test_save_populates_combined_policy_and_failure_detail_fields(self):
+ """Regression guard for the SMTP TLS analogue of issue #169:
+ policies and their failure_details are object arrays, so stacked
+ terms aggregations on their subfields cross-product just like
+ dkim_results/spf_results did. Two policies (one with two failure
+ details, one with none) must yield exactly two policies_combined
+ entries and two failure_details_combined entries, not a
+ cross-product. autospec=True is required on the save patch so
+ mock_save.call_args captures the doc instance as ``self``."""
+ report = _smtp_tls_report(
+ policies=[
+ {
+ "policy_domain": "example.com",
+ "policy_type": "sts",
+ "successful_session_count": 100,
+ "failed_session_count": 2,
+ "failure_details": [
+ {
+ "result_type": "certificate-expired",
+ "failed_session_count": 1,
+ "sending_mta_ip": "192.0.2.1",
+ "receiving_ip": "203.0.113.1",
+ "receiving_mx_hostname": "mx1.example.com",
+ "additional_info_uri": (
+ "https://reports.example.com/tls-help"
+ ),
+ },
+ {
+ "result_type": "starttls-not-supported",
+ "failed_session_count": 1,
+ "sending_mta_ip": "192.0.2.2",
+ "receiving_ip": "203.0.113.2",
+ "receiving_mx_hostname": "mx2.example.com",
+ },
+ ],
+ },
+ {
+ "policy_domain": "example.net",
+ "policy_type": "tlsa",
+ "successful_session_count": 50,
+ "failed_session_count": 0,
+ },
+ ]
+ )
+ with (
+ patch("parsedmarc.elastic.Search", return_value=_empty_search()),
+ patch("parsedmarc.elastic.Index"),
+ patch.object(
+ elastic_module._SMTPTLSReportDoc, "save", autospec=True
+ ) as mock_save,
+ ):
+ save_smtp_tls_report_to_elasticsearch(report)
+ doc = mock_save.call_args[0][0]
+ self.assertEqual(
+ list(doc.policies_combined), ["example.com / sts", "example.net / tlsa"]
+ )
+ expected_detail_expired = (
+ "example.com / sts / certificate-expired / 192.0.2.1 / "
+ "203.0.113.1 / mx1.example.com"
+ )
+ expected_detail_starttls = (
+ "example.com / sts / starttls-not-supported / 192.0.2.2 / "
+ "203.0.113.2 / mx2.example.com"
+ )
+ self.assertEqual(
+ list(doc.failure_details_combined),
+ [expected_detail_expired, expected_detail_starttls],
+ )
+ # The parser emits additional_info_uri (SMTPTLSFailureDetailsOptional
+ # in types.py); the saver must persist it on the declared
+ # additional_information_uri field rather than dropping it.
+ self.assertEqual(
+ doc.policies[0].failure_details[0].additional_information_uri,
+ "https://reports.example.com/tls-help",
+ )
+
class TestBackwardCompatAlias(unittest.TestCase):
def test_save_forensic_alias_points_to_save_failure(self):
diff --git a/tests/test_opensearch.py b/tests/test_opensearch.py
index 130013b9..c1748466 100644
--- a/tests/test_opensearch.py
+++ b/tests/test_opensearch.py
@@ -376,16 +376,193 @@ class TestCreateIndexes(unittest.TestCase):
class TestMigrateIndexes(unittest.TestCase):
+ """migrate_indexes backfills dkim_results_combined/spf_results_combined
+ (issue #169) on pre-existing aggregate documents as a non-blocking
+ background task. It is guarded by a cheap count() query so repeated
+ startups against an already-backfilled index are a no-op, and any SDK
+ error is caught and logged rather than raised, so it never blocks
+ parsedmarc startup."""
+
+ def test_backfill_submitted_when_old_docs_exist(self):
+ with (
+ patch("parsedmarc.opensearch.Index") as mock_index_cls,
+ patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn,
+ ):
+ # The legacy fo migration that runs first sees no base index.
+ mock_index_cls.return_value.exists.return_value = False
+ mock_client = MagicMock()
+ mock_client.count.return_value = {"count": 42}
+ mock_get_conn.return_value = mock_client
+ migrate_indexes(aggregate_indexes=["dmarc_aggregate"])
+
+ mock_client.update_by_query.assert_called_once()
+ kwargs = mock_client.update_by_query.call_args.kwargs
+ self.assertEqual(kwargs["index"], "dmarc_aggregate*")
+ self.assertEqual(kwargs["conflicts"], "proceed")
+ self.assertFalse(kwargs["wait_for_completion"])
+ self.assertEqual(
+ kwargs["body"]["query"], opensearch_module._COMBINED_BACKFILL_QUERY
+ )
+ script_source = kwargs["body"]["script"]["source"]
+ self.assertIn("ctx._source.dkim_results_combined", script_source)
+ self.assertIn("ctx._source.spf_results_combined", script_source)
+
+ # The count() guard query also targets the date-suffixed pattern.
+ count_kwargs = mock_client.count.call_args.kwargs
+ self.assertEqual(count_kwargs["index"], "dmarc_aggregate*")
+ self.assertEqual(
+ count_kwargs["body"]["query"], opensearch_module._COMBINED_BACKFILL_QUERY
+ )
+
+ def test_backfill_skipped_when_no_old_docs(self):
+ with (
+ patch("parsedmarc.opensearch.Index") as mock_index_cls,
+ patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn,
+ ):
+ mock_index_cls.return_value.exists.return_value = False
+ mock_client = MagicMock()
+ mock_client.count.return_value = {"count": 0}
+ mock_get_conn.return_value = mock_client
+ migrate_indexes(aggregate_indexes=["dmarc_aggregate"])
+
+ mock_client.update_by_query.assert_not_called()
+
+ def test_backfill_skipped_when_no_aggregate_indexes(self):
+ with patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn:
+ migrate_indexes()
+ migrate_indexes(aggregate_indexes=None)
+
+ mock_get_conn.assert_not_called()
+
+ def test_backfill_failure_does_not_raise(self):
+ with (
+ patch("parsedmarc.opensearch.Index") as mock_index_cls,
+ patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn,
+ ):
+ mock_index_cls.return_value.exists.return_value = False
+ mock_client = MagicMock()
+ mock_client.count.side_effect = RuntimeError("cluster unreachable")
+ mock_get_conn.return_value = mock_client
+ with self.assertLogs("parsedmarc.log", level="WARNING") as cm:
+ migrate_indexes(aggregate_indexes=["dmarc_aggregate"])
+
+ self.assertTrue(any("cluster unreachable" in msg for msg in cm.output))
+ mock_client.update_by_query.assert_not_called()
+
+ def test_get_connection_failure_does_not_raise(self):
+ """connections.get_connection() itself sits outside the per-index
+ try/except for the combined-field backfill; if it raises (e.g. no
+ OpenSearch connection has been configured yet), migrate_indexes must
+ still not propagate the exception, per its docstring's promise that
+ any cluster error is caught and logged."""
+ with (
+ patch("parsedmarc.opensearch.Index") as mock_index_cls,
+ patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn,
+ ):
+ # The legacy fo migration that runs first sees no base index.
+ mock_index_cls.return_value.exists.return_value = False
+ mock_get_conn.side_effect = RuntimeError("no connection")
+ with self.assertLogs("parsedmarc.log", level="WARNING") as cm:
+ migrate_indexes(aggregate_indexes=["dmarc_aggregate"])
+
+ self.assertTrue(
+ any("Skipping the dkim_results_combined" in msg for msg in cm.output)
+ )
+ self.assertTrue(any("no connection" in msg for msg in cm.output))
+
+ def test_smtp_tls_backfill_submitted_when_old_docs_exist(self):
+ """SMTP TLS analogue of test_backfill_submitted_when_old_docs_exist:
+ policies_combined/failure_details_combined backfill (also issue
+ #169) is submitted with its own guard query and painless script."""
+ with (
+ patch("parsedmarc.opensearch.Index") as mock_index_cls,
+ patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn,
+ ):
+ # The legacy fo migration that runs first sees no base index.
+ mock_index_cls.return_value.exists.return_value = False
+ mock_client = MagicMock()
+ mock_client.count.return_value = {"count": 7}
+ mock_get_conn.return_value = mock_client
+ migrate_indexes(smtp_tls_indexes=["smtp_tls"])
+
+ mock_client.update_by_query.assert_called_once()
+ kwargs = mock_client.update_by_query.call_args.kwargs
+ self.assertEqual(kwargs["index"], "smtp_tls*")
+ self.assertEqual(kwargs["conflicts"], "proceed")
+ self.assertFalse(kwargs["wait_for_completion"])
+ self.assertEqual(
+ kwargs["body"]["query"], opensearch_module._SMTP_TLS_COMBINED_BACKFILL_QUERY
+ )
+ script_source = kwargs["body"]["script"]["source"]
+ self.assertIn("ctx._source.policies_combined", script_source)
+ self.assertIn("ctx._source.failure_details_combined", script_source)
+
+ count_kwargs = mock_client.count.call_args.kwargs
+ self.assertEqual(count_kwargs["index"], "smtp_tls*")
+ self.assertEqual(
+ count_kwargs["body"]["query"],
+ opensearch_module._SMTP_TLS_COMBINED_BACKFILL_QUERY,
+ )
+
+ def test_smtp_tls_backfill_skipped_when_no_old_docs(self):
+ with (
+ patch("parsedmarc.opensearch.Index") as mock_index_cls,
+ patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn,
+ ):
+ mock_index_cls.return_value.exists.return_value = False
+ mock_client = MagicMock()
+ mock_client.count.return_value = {"count": 0}
+ mock_get_conn.return_value = mock_client
+ migrate_indexes(smtp_tls_indexes=["smtp_tls"])
+
+ mock_client.update_by_query.assert_not_called()
+
+ def test_smtp_tls_backfill_skipped_when_no_smtp_tls_indexes_or_aggregate(self):
+ with patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn:
+ migrate_indexes()
+ migrate_indexes(smtp_tls_indexes=None)
+
+ mock_get_conn.assert_not_called()
+
+ def test_smtp_tls_backfill_failure_does_not_raise(self):
+ """SMTP TLS analogue of test_backfill_failure_does_not_raise: an
+ error from the cluster during the smtp_tls_indexes loop is caught
+ and logged rather than raised."""
+ with (
+ patch("parsedmarc.opensearch.Index") as mock_index_cls,
+ patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn,
+ ):
+ mock_index_cls.return_value.exists.return_value = False
+ mock_client = MagicMock()
+ mock_client.count.side_effect = RuntimeError("cluster unreachable")
+ mock_get_conn.return_value = mock_client
+ with self.assertLogs("parsedmarc.log", level="WARNING") as cm:
+ migrate_indexes(smtp_tls_indexes=["smtp_tls"])
+
+ self.assertTrue(any("cluster unreachable" in msg for msg in cm.output))
+ mock_client.update_by_query.assert_not_called()
+
+
+class TestMigrateIndexesFoMigration(unittest.TestCase):
"""The legacy `published_policy.fo` field was mapped as `long` in
older indexes. migrate_indexes detects that and rebuilds the index
with the text/keyword shape. The branch is gnarly; a regression
- would silently leave old data un-migrated."""
+ would silently leave old data un-migrated. Each test stubs the
+ combined-field backfill that now runs afterwards in the same call
+ (count 0 → no-op)."""
- def test_no_indexes_is_noop(self):
- migrate_indexes() # Should not raise
+ @staticmethod
+ def _noop_backfill_client():
+ client = MagicMock()
+ client.count.return_value = {"count": 0}
+ return client
def test_skips_non_existent_index(self):
- with patch("parsedmarc.opensearch.Index") as mock_index_cls:
+ with (
+ patch("parsedmarc.opensearch.Index") as mock_index_cls,
+ patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn,
+ ):
+ mock_get_conn.return_value = self._noop_backfill_client()
mock_index_cls.return_value.exists.return_value = False
migrate_indexes(aggregate_indexes=["missing"])
# exists() returned False — no field_mapping fetch.
@@ -394,12 +571,16 @@ class TestMigrateIndexes(unittest.TestCase):
def test_skips_when_doc_mapping_absent(self):
"""An index that has 'fo' but not under the 'doc' type
(e.g., empty index with default mapping) is left alone."""
- with patch("parsedmarc.opensearch.Index") as mock_index_cls:
+ with (
+ patch("parsedmarc.opensearch.Index") as mock_index_cls,
+ patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn,
+ patch("parsedmarc.opensearch.reindex") as mock_reindex,
+ ):
+ mock_get_conn.return_value = self._noop_backfill_client()
idx = mock_index_cls.return_value
idx.exists.return_value = True
idx.get_field_mapping.return_value = {"some_key": {"mappings": {}}}
- with patch("parsedmarc.opensearch.reindex") as mock_reindex:
- migrate_indexes(aggregate_indexes=["dmarc_aggregate-2023-01-01"])
+ migrate_indexes(aggregate_indexes=["dmarc_aggregate-2023-01-01"])
mock_reindex.assert_not_called()
def test_migrates_when_fo_is_long(self):
@@ -411,6 +592,8 @@ class TestMigrateIndexes(unittest.TestCase):
patch("parsedmarc.opensearch.reindex") as mock_reindex,
patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn,
):
+ mock_client = self._noop_backfill_client()
+ mock_get_conn.return_value = mock_client
idx = mock_index_cls.return_value
idx.exists.return_value = True
idx.get_field_mapping.return_value = {
@@ -423,16 +606,18 @@ class TestMigrateIndexes(unittest.TestCase):
}
}
migrate_indexes(aggregate_indexes=["dmarc_aggregate-2023-01-01"])
- # reindex called from old → new (v2) index.
+ # reindex called from old → new (v2) index, with the client from
+ # connections.get_connection().
mock_reindex.assert_called_once()
- # connections.get_connection consulted to get the ES client.
- mock_get_conn.assert_called_once()
+ self.assertIs(mock_reindex.call_args.args[0], mock_client)
def test_skips_when_fo_already_text(self):
with (
patch("parsedmarc.opensearch.Index") as mock_index_cls,
+ patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn,
patch("parsedmarc.opensearch.reindex") as mock_reindex,
):
+ mock_get_conn.return_value = self._noop_backfill_client()
idx = mock_index_cls.return_value
idx.exists.return_value = True
idx.get_field_mapping.return_value = {
@@ -447,6 +632,35 @@ class TestMigrateIndexes(unittest.TestCase):
migrate_indexes(aggregate_indexes=["dmarc_aggregate-2024-01-01"])
mock_reindex.assert_not_called()
+ def test_index_exists_failure_does_not_raise(self):
+ """A cluster error inside the per-index fo-migration loop (e.g.
+ Index(...).exists() raising because the cluster is unreachable)
+ must not abort startup: it is caught, logged, and the loop moves
+ on to the combined-field backfill, which is exercised here with
+ its own connection failure so both warnings are asserted."""
+ with (
+ patch("parsedmarc.opensearch.Index") as mock_index_cls,
+ patch("parsedmarc.opensearch.connections.get_connection") as mock_get_conn,
+ ):
+ mock_index_cls.return_value.exists.side_effect = ConnectionError(
+ "cluster unreachable"
+ )
+ mock_get_conn.side_effect = RuntimeError("no connection")
+ with self.assertLogs("parsedmarc.log", level="WARNING") as cm:
+ migrate_indexes(aggregate_indexes=["dmarc_aggregate"])
+
+ self.assertTrue(
+ any(
+ "legacy published_policy.fo migration" in msg
+ and "cluster unreachable" in msg
+ for msg in cm.output
+ )
+ )
+ self.assertTrue(
+ any("Skipping the dkim_results_combined" in msg for msg in cm.output)
+ )
+ self.assertTrue(any("no connection" in msg for msg in cm.output))
+
# ---------------------------------------------------------------------------
# save_aggregate_report_to_opensearch
@@ -602,6 +816,54 @@ class TestSaveAggregateReport(unittest.TestCase):
mock_doc_cls.call_args.kwargs["date_begin"].timestamp(), 1705276800
)
+ def test_save_populates_combined_dkim_and_spf_fields(self):
+ """Regression guard for issue #169: two DKIM signatures on one
+ record must yield exactly two combined entries, not a 4-way
+ cross-product. autospec=True is required on the save patch so
+ mock_save.call_args captures the doc instance as ``self``."""
+ report = _aggregate_report()
+ report["records"][0]["auth_results"] = {
+ "dkim": [
+ {
+ "domain": "example.net",
+ "selector": "net1",
+ "result": "fail",
+ "human_result": None,
+ },
+ {
+ "domain": "example.org",
+ "selector": "org1",
+ "result": "pass",
+ "human_result": None,
+ },
+ ],
+ "spf": [
+ {
+ "domain": "example.org",
+ "scope": "mfrom",
+ "result": "pass",
+ "human_result": None,
+ },
+ ],
+ }
+ with (
+ patch("parsedmarc.opensearch.Search", return_value=_empty_search()),
+ patch(
+ "parsedmarc.opensearch.Index",
+ return_value=MagicMock(exists=MagicMock(return_value=True)),
+ ),
+ patch.object(
+ opensearch_module._AggregateReportDoc, "save", autospec=True
+ ) as mock_save,
+ ):
+ save_aggregate_report_to_opensearch(report)
+ doc = mock_save.call_args[0][0]
+ self.assertEqual(
+ list(doc.dkim_results_combined),
+ ["net1 / example.net / fail", "org1 / example.org / pass"],
+ )
+ self.assertEqual(list(doc.spf_results_combined), ["mfrom / example.org / pass"])
+
class TestAggregateDocPassedDmarc(unittest.TestCase):
"""The _AggregateReportDoc.save() override derives passed_dmarc — the
@@ -629,6 +891,56 @@ class TestAggregateDocPassedDmarc(unittest.TestCase):
self.assertEqual(bool(doc.passed_dmarc), expected)
+class TestAggregateDocCombinedResults(unittest.TestCase):
+ """add_dkim_result/add_spf_result never touch the network, so these
+ construct _AggregateReportDoc directly rather than going through the
+ save_* entry point."""
+
+ def test_add_dkim_result_appends_combined_string(self):
+ """Regression guard for issue #169: dkim_results/spf_results are
+ arrays of objects that the engine dynamic-maps as plain ``object``
+ (not ``nested``) and flattens, so Kibana/Grafana tables cannot
+ terms-aggregate their subfields without producing a cross-product
+ of selector/domain/result values. The composed
+ "selector / domain / result" string preserves the per-signature
+ pairing that the flattened array loses."""
+ doc = opensearch_module._AggregateReportDoc()
+ doc.add_dkim_result(
+ domain="example.net", selector="net1", result="fail", human_result=None
+ )
+ doc.add_dkim_result(
+ domain="example.org", selector="org1", result="pass", human_result=None
+ )
+ expected = ["net1 / example.net / fail", "org1 / example.org / pass"]
+ # dkim_results_combined is declared as Text(multi=True, ...); the SDK
+ # stub types the class attribute as Text (no Iterable protocol),
+ # even though the runtime value is an AttrList once multi=True is
+ # set.
+ self.assertEqual(list(doc.dkim_results_combined), expected) # pyright: ignore[reportArgumentType]
+ self.assertEqual(doc.to_dict()["dkim_results_combined"], expected)
+
+ def test_add_spf_result_appends_combined_string(self):
+ doc = opensearch_module._AggregateReportDoc()
+ doc.add_spf_result(
+ domain="example.org", scope="mfrom", result="pass", human_result=None
+ )
+ expected = ["mfrom / example.org / pass"]
+ self.assertEqual(list(doc.spf_results_combined), expected) # pyright: ignore[reportArgumentType]
+ self.assertEqual(doc.to_dict()["spf_results_combined"], expected)
+
+ def test_spf_result_serializes_under_singular_result_key(self):
+ """The _SPFResult class previously declared a dead ``results``
+ (plural) field while the save path wrote ``result``; verify the
+ serialized inner doc actually uses the singular key."""
+ doc = opensearch_module._AggregateReportDoc()
+ doc.add_spf_result(
+ domain="example.org", scope="mfrom", result="pass", human_result=None
+ )
+ d = doc.to_dict()["spf_results"][0]
+ self.assertEqual(d["result"], "pass")
+ self.assertNotIn("results", d)
+
+
# ---------------------------------------------------------------------------
# save_failure_report_to_opensearch
# ---------------------------------------------------------------------------
@@ -979,6 +1291,82 @@ class TestSaveSmtpTlsReport(unittest.TestCase):
save_smtp_tls_report_to_opensearch(report)
mock_save.assert_called_once()
+ def test_save_populates_combined_policy_and_failure_detail_fields(self):
+ """Regression guard for the SMTP TLS analogue of issue #169:
+ policies and their failure_details are object arrays, so stacked
+ terms aggregations on their subfields cross-product just like
+ dkim_results/spf_results did. Two policies (one with two failure
+ details, one with none) must yield exactly two policies_combined
+ entries and two failure_details_combined entries, not a
+ cross-product. autospec=True is required on the save patch so
+ mock_save.call_args captures the doc instance as ``self``."""
+ report = _smtp_tls_report(
+ policies=[
+ {
+ "policy_domain": "example.com",
+ "policy_type": "sts",
+ "successful_session_count": 100,
+ "failed_session_count": 2,
+ "failure_details": [
+ {
+ "result_type": "certificate-expired",
+ "failed_session_count": 1,
+ "sending_mta_ip": "192.0.2.1",
+ "receiving_ip": "203.0.113.1",
+ "receiving_mx_hostname": "mx1.example.com",
+ "additional_info_uri": (
+ "https://reports.example.com/tls-help"
+ ),
+ },
+ {
+ "result_type": "starttls-not-supported",
+ "failed_session_count": 1,
+ "sending_mta_ip": "192.0.2.2",
+ "receiving_ip": "203.0.113.2",
+ "receiving_mx_hostname": "mx2.example.com",
+ },
+ ],
+ },
+ {
+ "policy_domain": "example.net",
+ "policy_type": "tlsa",
+ "successful_session_count": 50,
+ "failed_session_count": 0,
+ },
+ ]
+ )
+ with (
+ patch("parsedmarc.opensearch.Search", return_value=_empty_search()),
+ patch("parsedmarc.opensearch.Index"),
+ patch.object(
+ opensearch_module._SMTPTLSReportDoc, "save", autospec=True
+ ) as mock_save,
+ ):
+ save_smtp_tls_report_to_opensearch(report)
+ doc = mock_save.call_args[0][0]
+ self.assertEqual(
+ list(doc.policies_combined), ["example.com / sts", "example.net / tlsa"]
+ )
+ expected_detail_expired = (
+ "example.com / sts / certificate-expired / 192.0.2.1 / "
+ "203.0.113.1 / mx1.example.com"
+ )
+ expected_detail_starttls = (
+ "example.com / sts / starttls-not-supported / 192.0.2.2 / "
+ "203.0.113.2 / mx2.example.com"
+ )
+ self.assertEqual(
+ list(doc.failure_details_combined),
+ [expected_detail_expired, expected_detail_starttls],
+ )
+ # The parser emits additional_info_uri (SMTPTLSFailureDetailsOptional
+ # in types.py); the saver must persist it on the declared
+ # additional_information_uri field rather than dropping it.
+ self.assertEqual(
+ doc.policies[0].failure_details[0].additional_information_uri,
+ "https://reports.example.com/tls-help",
+ )
+
class TestBackwardCompatAlias(unittest.TestCase):
def test_save_forensic_alias_points_to_save_failure(self):