feat(email): the report, invites, resets and test sends go through the outbox; only it talks SMTP (#5508)
#5508 "Every other email through the outbox: weekly report, invites, password resets, and the test buttons" (milestone 501). - outbox.send_each queues one email per reader (claimed, so the sender doesn't take it too) and tries them at once, for a page that answers with what happened; outbox.said words the result. attempt takes a cfg for a test of the form's unsaved settings and returns the settled rows. - Weekly report (scheduled and Send now) and invites: queued, tried at once; one the server couldn't take yet is retried for an hour. An invite that didn't go shows its link with the reason, as before. - Password reset: queued in the reset link's own transaction; the sender delivers it on its next tick. - Settings -> Notifications test email and a rule's Send test: queued and recorded with retry=False, tried once (System 156 ruling). - Removed mail.send, notifications._send_email and _smtp_send_sync; dispatch_notifications is webhook only. A guard asserts only the outbox (and smtp_open itself) opens an SMTP connection, with a self-test. Also fixes run 8941 (#5507's integration test): its Also-send-to addresses were "@test", which is_real_address rightly drops; they are example.com now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
+13
-6
@@ -6,7 +6,6 @@ from datetime import datetime, timezone
|
||||
from zoneinfo import ZoneInfo
|
||||
|
||||
from steward.core import mail
|
||||
from steward.core import notifications as n
|
||||
from steward.core import reports
|
||||
|
||||
ALERT = {"rule_name": 'Disk <b>"root"</b> on web01', "state": "FIRING", "metric": "disk_used_pct",
|
||||
@@ -46,9 +45,8 @@ def test_every_colour_is_a_theme_value():
|
||||
|
||||
|
||||
async def _sent(monkeypatch, cfg, alert, to=(EXTRA_NY,)):
|
||||
out = []
|
||||
monkeypatch.setattr(n, "_smtp_send_sync", lambda c, msgs: out.extend(msgs))
|
||||
assert (await n._send_email(cfg, alert, list(to)))["sent"]
|
||||
"""The message each of `to` is handed for `alert`, as the outbox writes it."""
|
||||
out = [mail.message(cfg, r.address, *mail.alert_email(cfg, alert, r)) for r in to]
|
||||
return out[0] if len(to) == 1 else out
|
||||
|
||||
|
||||
@@ -140,12 +138,21 @@ async def test_the_report_is_sent_from_the_configured_address(monkeypatch):
|
||||
assert kind == "report"
|
||||
return [PERSON_NY, EXTRA_NY]
|
||||
|
||||
from types import SimpleNamespace
|
||||
from steward.core import outbox
|
||||
out = []
|
||||
|
||||
async def _send_each(app, kind, to, words, **kw):
|
||||
# Queued then tried at once (#5508); here every one is accepted.
|
||||
assert kind == "report" and kw.get("retry", True)
|
||||
out.extend(mail.message(app.config["SMTP"], r.address, *words(r)) for r in to)
|
||||
return [SimpleNamespace(recipient=r.address, state="sent", last_error=None) for r in to]
|
||||
|
||||
monkeypatch.setattr(reports, "build_report_data", _data)
|
||||
monkeypatch.setattr(reports, "recipients", _recipients)
|
||||
monkeypatch.setattr(n, "_smtp_send_sync", lambda c, msgs: out.extend(msgs))
|
||||
monkeypatch.setattr(outbox, "send_each", _send_each)
|
||||
ok, said = await reports.send_report(_App())
|
||||
assert ok and said == "Report sent to ann@example.com, ops@example.com."
|
||||
assert ok and said == "Sent to ann@example.com, ops@example.com."
|
||||
assert [(m["From"], m["To"]) for m in out] == [("alerts@example.com", "ann@example.com"),
|
||||
("alerts@example.com", "ops@example.com")]
|
||||
assert all(m.get_content_type() == "multipart/alternative" for m in out)
|
||||
|
||||
@@ -49,7 +49,7 @@ def test_a_bad_template_raises_valueerror_saying_why(tpl, says):
|
||||
n.render_webhook(tpl, ALERT)
|
||||
|
||||
|
||||
# ── smtp_mode / _smtp_send_sync ───────────────────────────────────────────────
|
||||
# ── smtp_mode / smtp_open (via outbox.deliver_sync) ───────────────────────────────────────────────
|
||||
|
||||
@pytest.mark.parametrize("port, tls, mode", [
|
||||
(465, True, "ssl"), (465, False, "ssl"), ("465", False, "ssl"),
|
||||
@@ -98,27 +98,30 @@ class SMTP_SSL(_FakeSMTP):
|
||||
(25, False, [("connect", "SMTP", "mx", 25, n.SMTP_TIMEOUT_S)]),
|
||||
])
|
||||
def test_smtp_send_connects_by_mode_with_a_timeout(monkeypatch, port, tls, expected):
|
||||
from email.message import EmailMessage
|
||||
from steward.core import outbox
|
||||
monkeypatch.setattr(n.smtplib, "SMTP", SMTP)
|
||||
monkeypatch.setattr(n.smtplib, "SMTP_SSL", SMTP_SSL)
|
||||
_FakeSMTP.made = []
|
||||
msgs = [n.EmailMessage(), n.EmailMessage()]
|
||||
msgs = [EmailMessage(), EmailMessage()]
|
||||
msgs[0]["From"], msgs[1]["From"] = "steward@example.com", "other@example.com"
|
||||
n._smtp_send_sync({"host": "mx", "port": port, "tls": tls, "username": "u", "password": "p"}, msgs)
|
||||
cfg = {"host": "mx", "port": port, "tls": tls, "username": "u", "password": "p"}
|
||||
assert outbox.deliver_sync(cfg, msgs) == [None, None]
|
||||
(smtp,) = _FakeSMTP.made # every message over one connection
|
||||
assert smtp.calls == expected + [("login", "u", "p"), ("send", "steward@example.com"),
|
||||
("send", "other@example.com")]
|
||||
|
||||
|
||||
async def test_email_from_falls_back_and_a_line_break_in_the_name_still_sends(monkeypatch):
|
||||
def test_email_from_falls_back_and_a_line_break_in_the_name_still_sends():
|
||||
from steward.core.mail import alert_email, message
|
||||
sent = []
|
||||
monkeypatch.setattr(n, "_smtp_send_sync",
|
||||
lambda cfg, msgs: sent.extend((m["From"], m["Subject"]) for m in msgs))
|
||||
to = [Recipient("a@example.com", timezone.utc, False)]
|
||||
to = Recipient("a@example.com", timezone.utc, False)
|
||||
base = {"host": "mx", "port": 587}
|
||||
for cfg in ({**base, "from": "alerts@example.com", "username": "u@example.com"},
|
||||
{**base, "from": "", "username": "u@example.com"},
|
||||
{**base, "from": "", "username": ""}):
|
||||
assert (await n._send_email(cfg, ALERT, to))["sent"]
|
||||
m = message(cfg, to.address, *alert_email(cfg, ALERT, to))
|
||||
sent.append((m["From"], m["Subject"]))
|
||||
assert [f for f, _ in sent] == ["alerts@example.com", "u@example.com", "steward@localhost"]
|
||||
# The rule name has a line break; the subject folds it to a space rather than failing.
|
||||
assert sent[0][1] == '[Steward] Firing: Disk "root" on web01 \\ backslash — web01 94%'
|
||||
|
||||
@@ -186,3 +186,40 @@ def test_several_alerts_are_one_email_listing_each_oldest_first():
|
||||
assert f"Agent offline: {host}" in text and f"Agent offline: {host}" in html
|
||||
assert text.index("web01") < text.index("web02") < text.index("db01")
|
||||
assert "Fri 9 Oct 2026 08:00 EDT" in text # the reader's zone
|
||||
|
||||
|
||||
def test_said_names_who_got_it_who_refused_and_who_is_still_being_tried():
|
||||
rows = [_row(recipient="a@x", state="sent"),
|
||||
_row(recipient="b@x", state="failed", last_error="550 no such user"),
|
||||
_row(recipient="c@x", state="pending", last_error="451 greylisted")]
|
||||
ok, said = outbox.said(rows)
|
||||
assert not ok
|
||||
assert said == ("Sent to a@x. Not sent to b@x: 550 no such user. Not sent yet to c@x "
|
||||
"(451 greylisted); Steward will try again for up to an hour.")
|
||||
assert outbox.said(rows[:1]) == (True, "Sent to a@x.")
|
||||
assert outbox.said([])[0] is False
|
||||
|
||||
|
||||
# The guard: every email goes through the queue (System 156 ruling), so the
|
||||
# only code that opens a connection to the mail server is the outbox's.
|
||||
def _smtp_openers(sources: dict[str, str]) -> set[str]:
|
||||
import re
|
||||
opens = re.compile(r"\bsmtp_open\(|\bsmtplib\.SMTP(_SSL)?\(")
|
||||
return {path for path, src in sources.items()
|
||||
if any(opens.search(line) and not line.lstrip().startswith(("#", "def "))
|
||||
for line in src.splitlines())}
|
||||
|
||||
|
||||
def test_only_the_outbox_talks_to_the_mail_server():
|
||||
from pathlib import Path
|
||||
root = Path(__file__).resolve().parents[2] # the checkout
|
||||
sources = {str(p.relative_to(root)): p.read_text() for p in (root / "steward").rglob("*.py")}
|
||||
sources.update({str(p.relative_to(root)): p.read_text() for p in (root / "plugins").rglob("*.py")}
|
||||
if (root / "plugins").is_dir() else {})
|
||||
assert _smtp_openers(sources) == {"steward/core/outbox.py", "steward/core/notifications.py"}
|
||||
|
||||
|
||||
def test_the_smtp_guard_can_fail():
|
||||
assert _smtp_openers({"x.py": " with smtplib.SMTP(host) as s:"}) == {"x.py"}
|
||||
assert _smtp_openers({"y.py": " smtp = notifications.smtp_open(cfg)"}) == {"y.py"}
|
||||
assert _smtp_openers({"z.py": "# smtp_open(cfg) is the outbox's\ndef smtp_open(cfg):"}) == set()
|
||||
|
||||
Reference in New Issue
Block a user