diff --git a/kernel_patches_daemon/branch_worker.py b/kernel_patches_daemon/branch_worker.py index 2d6743b..6073046 100644 --- a/kernel_patches_daemon/branch_worker.py +++ b/kernel_patches_daemon/branch_worker.py @@ -11,6 +11,7 @@ import email import email.parser import email.policy +import email.utils import hashlib import logging import os @@ -378,6 +379,7 @@ def reply_email_recipients( msg: EmailMessage, allowlist: Optional[Sequence[re.Pattern]] = None, denylist: Optional[Sequence[re.Pattern]] = None, + always_reply_to_author: bool = False, ) -> Tuple[List[str], List[str]]: """ Extracts response recipients from the `msg`, applying allowlist/denylist. @@ -386,18 +388,27 @@ def reply_email_recipients( msg: the EmailMessage we will be replying to allowlist: list of email address regexes to allow, ignored if empty denylist: list of email address regexes to deny, ignored if empty + always_reply_to_author: if True, the patch author (message sender) + will be added to the to_list even if filtered out by allowlist/denylist Returns: (to_list, cc_list) - recipients of the reply email """ + + sender_address = None + to_list = [] + + sender = msg.get("From") + if sender is not None: + (_, sender_address) = email.utils.parseaddr(sender) + if sender_address: # parseaddr might return an emptystring + to_list.append(sender_address) + tos = msg.get_all("To", []) - ccs = msg.get_all("Cc", []) - # pyrefly: ignore # implicit-import - cc_list = [a for (_, a) in email.utils.getaddresses(tos + ccs)] + to_list += [a for (_, a) in email.utils.getaddresses(tos)] - # pyrefly: ignore # implicit-import, bad-argument-type - (_, sender_address) = email.utils.parseaddr(msg.get("From")) - to_list = [sender_address] + ccs = msg.get_all("Cc", []) + cc_list = [a for (_, a) in email.utils.getaddresses(ccs)] if allowlist: cc_list = [a for a in cc_list if email_matches_any(a, allowlist)] @@ -407,6 +418,9 @@ def reply_email_recipients( cc_list = [a for a in cc_list if not email_matches_any(a, denylist)] to_list = [a for a in to_list if not email_matches_any(a, denylist)] + if always_reply_to_author and sender_address and sender_address not in to_list: + to_list.append(sender_address) + return (to_list, cc_list) @@ -433,7 +447,10 @@ async def send_pr_comment_email( return (to_list, cc_list) = reply_email_recipients( - msg, allowlist=cfg.recipient_allowlist, denylist=cfg.recipient_denylist + msg, + allowlist=cfg.recipient_allowlist, + denylist=cfg.recipient_denylist, + always_reply_to_author=cfg.always_reply_to_author, ) cc_list += cfg.always_cc diff --git a/kernel_patches_daemon/config.py b/kernel_patches_daemon/config.py index 142a9ac..2a70ad1 100644 --- a/kernel_patches_daemon/config.py +++ b/kernel_patches_daemon/config.py @@ -108,6 +108,7 @@ def from_json(cls, json: Dict) -> "BranchConfig": class PRCommentsForwardingConfig: enabled: bool always_cc: List[str] + always_reply_to_author: bool commenter_allowlist: List[str] recipient_allowlist: List[re.Pattern] recipient_denylist: List[re.Pattern] @@ -117,6 +118,7 @@ def from_json(cls, json: Dict) -> "PRCommentsForwardingConfig": return cls( enabled=json.get("enabled", False), always_cc=json.get("always_cc", []), + always_reply_to_author=json.get("always_reply_to_author", False), recipient_allowlist=[ re.compile(pattern) for pattern in json.get("recipient_allowlist", []) ], diff --git a/tests/test_branch_worker.py b/tests/test_branch_worker.py index bf1c55d..2c8886e 100644 --- a/tests/test_branch_worker.py +++ b/tests/test_branch_worker.py @@ -7,6 +7,8 @@ # pyre-unsafe import email +import email.parser +import email.policy import json import os import random @@ -42,7 +44,6 @@ prs_for_the_same_series, reply_email_recipients, same_series_different_target, - send_pr_comment_email, temporary_patch_file, UPSTREAM_REMOTE_NAME, ) @@ -1480,26 +1481,21 @@ def test_email_submitter_not_in_allowlist_and_allowlist_disabled(self): self.assertEqual(expected_email, email) def test_reply_email_recipients(self): - kpd_config_json = json.loads(read_fixture("kpd_config.json")) - kpd_config = KPDConfig.from_json(kpd_config_json) - self.assertIsNotNone(kpd_config) - mbox = read_test_data_file( "test_sync_patches_pr_summary_success/series-970926.mbox" ) - # pyrefly: ignore # implicit-import parser = email.parser.BytesParser(policy=email.policy.default) msg = parser.parsebytes(mbox.encode("utf-8"), headersonly=True) self.assertIsNotNone(mbox) - # pyrefly: ignore # missing-attribute - denylist = kpd_config.email.pr_comments_forwarding.recipient_denylist + denylist = [re.compile(".*@vger.kernel.org")] (to_list, cc_list) = reply_email_recipients(msg, denylist=denylist) - self.assertEqual(to_list, ["chen.dylane@linux.dev"]) - self.assertEqual(len(cc_list), 17) + self.assertIn("chen.dylane@linux.dev", to_list) + self.assertEqual(len(to_list), 17) + self.assertEqual(len(cc_list), 1) - # test allowlist by using the same denylist - (to_list, cc_list) = reply_email_recipients(msg, allowlist=denylist) + allowlist = [re.compile(".*@vger.kernel.org")] + (to_list, cc_list) = reply_email_recipients(msg, allowlist=allowlist) self.assertEqual(to_list, []) self.assertEqual(len(cc_list), 3) @@ -1509,8 +1505,23 @@ def test_reply_email_recipients(self): (to_list, cc_list) = reply_email_recipients( msg, allowlist=allowlist, denylist=denylist ) + self.assertIn("chen.dylane@linux.dev", to_list) + self.assertEqual(len(to_list), 3) + self.assertEqual(len(cc_list), 1) + + # test denylist all + denylist = [re.compile(".*")] + (to_list, cc_list) = reply_email_recipients(msg, denylist=denylist) + self.assertEqual(to_list, []) + self.assertEqual(cc_list, []) + + # test denylist all, but the sender + denylist = [re.compile(".*")] + (to_list, cc_list) = reply_email_recipients( + msg, denylist=denylist, always_reply_to_author=True + ) self.assertEqual(to_list, ["chen.dylane@linux.dev"]) - self.assertEqual(len(cc_list), 3) + self.assertEqual(cc_list, []) class TestParsePrRef(unittest.TestCase): @@ -1655,6 +1666,7 @@ async def test_forward_pr_comments(self, m: aioresponses) -> None: self._bw.email_config = MagicMock( pr_comments_forwarding=PRCommentsForwardingConfig( enabled=True, + always_reply_to_author=False, always_cc=["bpf-ci-test@example.com"], commenter_allowlist=["test_user"], recipient_denylist=[re.compile(".*@vger.kernel.org")], @@ -1702,9 +1714,10 @@ async def test_forward_pr_comments(self, m: aioresponses) -> None: mock_send_email.call_args[0] ) - self.assertEqual(to_list, ["chen.dylane@linux.dev"]) - self.assertEqual(len(cc_list), 18) + self.assertIn("chen.dylane@linux.dev", to_list) + self.assertEqual(len(to_list), 17) self.assertIn("bpf-ci-test@example.com", cc_list) + self.assertEqual(len(cc_list), 2) self.assertEqual( "Re: [PATCH bpf-next] bpf: clear user buf when bpf_d_path failed", subject, diff --git a/tests/test_config.py b/tests/test_config.py index f2c0d53..7b28ae3 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -175,6 +175,7 @@ def test_valid(self) -> None: pr_comments_forwarding=PRCommentsForwardingConfig( enabled=True, always_cc=["bpf-ci-test@example.com"], + always_reply_to_author=False, commenter_allowlist=["kpd-bot[bot]"], recipient_denylist=[re.compile(".*@vger.kernel.org")], recipient_allowlist=[],