Skip to content

Commit 772944e

Browse files
committed
branch_worker: implement PR comment forwarding
Implement an optional generic PR comment forwarding feature. Whenever a relevant patchwork subject is processed in sync_relevant_subject(), inspect pull request comments and send selected comment body via email according to KPD configuration. This by design only affects pull request with corresponding up-to-date patch series on patchwork. To be forwarded the comment must pass the following filters: - the comment hasn't been forwarded yet - KPD is stateless, and so we determine this by checking for other comments on the same PR that report previous notifications - comment author's GitHub login is in configured allowlist - comment body contains an "In-Reply-To-Subject" tag with the subject string of a patch message to reply to - rationale for such tag is the necessity for a mechanism to determine the exact message in a thread to reply to; commit hash or index can change between series version, and a subject string uniquely identifies a patch message within a series When a comment is selected for forwarding KPD: - posts a PR comment reporting the forward action - sends an email to all original recipients of the patch message, filtered according to the configured allow/denylist An example of PR comments forwarding configuration: "email": { ... "pr_comments_forwarding": { "enabled": true, "commenter_allowlist": ["kpd-bot[bot]"], "always_cc": ["[email protected]"], "recipient_denylist": [".*@foobar.org"] } } Signed-off-by: Ihor Solodrai <[email protected]>
1 parent 0bef2b7 commit 772944e

File tree

7 files changed

+337
-3
lines changed

7 files changed

+337
-3
lines changed

kernel_patches_daemon/branch_worker.py

Lines changed: 145 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ async def send_email(
328328
subject: str,
329329
body: str,
330330
in_reply_to: Optional[str] = None,
331-
):
331+
) -> str:
332332
"""Send an email."""
333333
msg_id = generate_msg_id(config.smtp_host)
334334
curl_args, msg = build_email(
@@ -343,6 +343,8 @@ async def send_email(
343343
logger.error(f"failed to send email: {stdout.decode()} {stderr.decode()}")
344344
email_send_fail_counter.add(1)
345345

346+
return msg_id
347+
346348

347349
def ci_results_email_recipients(
348350
config: EmailConfig, series: Series
@@ -368,6 +370,74 @@ async def send_ci_results_email(
368370
await send_email(config, to_list, cc_list, subject, body, in_reply_to)
369371

370372

373+
def reply_email_recipients(
374+
msg: EmailMessage,
375+
allowlist: Optional[Sequence[re.Pattern]] = None,
376+
denylist: Optional[Sequence[re.Pattern]] = None,
377+
) -> Tuple[List[str], List[str]]:
378+
"""
379+
Extracts response recipients from the `msg`, applying allowlist/denylist.
380+
381+
Args:
382+
msg: the EmailMessage we will be replying to
383+
allowlist: list of email address regexes to allow, ignored if empty
384+
denylist: list of email address regexes to deny, ignored if empty
385+
386+
Returns:
387+
(to_list, cc_list) - recipients of the reply email
388+
"""
389+
tos = msg.get_all("To", [])
390+
ccs = msg.get_all("Cc", [])
391+
cc_list = [a for (_, a) in email.utils.getaddresses(tos + ccs)]
392+
393+
(_, sender_address) = email.utils.parseaddr(msg.get("From"))
394+
to_list = [sender_address]
395+
396+
if allowlist:
397+
cc_list = [a for a in cc_list if email_matches_any(a, allowlist)]
398+
to_list = [a for a in to_list if email_matches_any(a, allowlist)]
399+
400+
if denylist:
401+
cc_list = [a for a in cc_list if not email_matches_any(a, denylist)]
402+
to_list = [a for a in to_list if not email_matches_any(a, denylist)]
403+
404+
return (to_list, cc_list)
405+
406+
407+
async def send_pr_comment_email(
408+
config: EmailConfig, msg: EmailMessage, body: str
409+
) -> Optional[str]:
410+
"""
411+
This function forwards a pull request comment (`body`) as an email reply to the original
412+
patch email message `msg`. It extracts recipients from the `msg`, applies allowlist, denylist,
413+
and always_cc as configured, and sets Subject and In-Reply-To based on the `msg`
414+
415+
Args:
416+
config: EmailConfig with PRCommentsForwardingConfig
417+
msg: the original EmailMessage we are replying to (the patch submission)
418+
body: the content of the reply we are sending
419+
420+
Returns:
421+
Message-Id of the sent email, or None if it wasn't sent
422+
"""
423+
if config is None or not config.is_pr_comment_forwarding_enabled():
424+
return
425+
426+
cfg = config.pr_comments_forwarding
427+
(to_list, cc_list) = reply_email_recipients(
428+
msg, allowlist=cfg.recipient_allowlist, denylist=cfg.recipient_denylist
429+
)
430+
cc_list += cfg.always_cc
431+
432+
if not to_list and not cc_list:
433+
return
434+
435+
subject = "Re: " + msg.get("Subject")
436+
in_reply_to = msg.get("Message-Id")
437+
438+
return await send_email(config, to_list, cc_list, subject, body, in_reply_to)
439+
440+
371441
def pr_has_label(pr: PullRequest, label: str) -> bool:
372442
for pr_label in pr.get_labels():
373443
if pr_label.name == label:
@@ -1307,3 +1377,77 @@ async def submit_pr_summary(
13071377
description="PR summary",
13081378
)
13091379
pr_summary_report.add(1)
1380+
1381+
async def forward_pr_comments(self, pr: PullRequest, series: Series):
1382+
if (
1383+
self.email_config is None
1384+
or not self.email_config.is_pr_comment_forwarding_enabled()
1385+
):
1386+
return
1387+
cfg = self.email_config.pr_comments_forwarding
1388+
1389+
comments = pr.get_issue_comments()
1390+
1391+
# Look for comments indicating what has already been forwarded
1392+
# to filter them out
1393+
forwarded_set = set()
1394+
for comment in comments:
1395+
match = re.search(
1396+
r"Forwarding comment \[([0-9]+)\].* via email",
1397+
comment.body,
1398+
re.MULTILINE,
1399+
)
1400+
if not match:
1401+
continue
1402+
forwarded_id = int(match.group(1))
1403+
forwarded_set.add(forwarded_id)
1404+
1405+
for comment in comments:
1406+
if (
1407+
comment.user.login not in cfg.commenter_allowlist
1408+
or comment.id in forwarded_set
1409+
):
1410+
continue
1411+
# Determine the message to reply to
1412+
# Check for In-Reply-To-Subject tag in the comment body
1413+
match = re.search(
1414+
r"In-Reply-To-Subject: \`(.*)\`", comment.body, re.MULTILINE
1415+
)
1416+
if not match:
1417+
logger.info(
1418+
f"Ignoring PR comment {comment.html_url} without In-Reply-To-Subject tag"
1419+
)
1420+
continue
1421+
subject = match.group(1)
1422+
patch = series.patch_by_subject(subject)
1423+
if not patch:
1424+
logger.warn(
1425+
f"Ignoring PR comment {comment.html_url}, could not find relevant patch on patchwork"
1426+
)
1427+
continue
1428+
1429+
# first, post a comment that the message is being forwarded
1430+
msg_id = patch["msgid"]
1431+
patch_url = patch["web_url"]
1432+
message = f"Forwarding comment [{comment.id}]({comment.html_url}) via email"
1433+
message += f"\nIn-Reply-To: {msg_id}"
1434+
message += f"\nPatch: {patch_url}"
1435+
self._add_pull_request_comment(pr, message)
1436+
1437+
# then, load the message we are replying to
1438+
mbox = await series.pw_client.get_blob(patch["mbox"])
1439+
parser = email.parser.BytesParser(policy=email.policy.default)
1440+
patch_msg = parser.parsebytes(mbox, headersonly=True)
1441+
1442+
# and forward the target comment via email
1443+
sent_msg_id = await send_pr_comment_email(
1444+
self.email_config, patch_msg, comment.body
1445+
)
1446+
if sent_msg_id is not None:
1447+
logger.info(
1448+
f"Forwarded PR comment {comment.html_url} via email, Message-Id: {sent_msg_id}"
1449+
)
1450+
else:
1451+
logger.warn(
1452+
f"Failed to forward PR comment in reply to {msg_id}, no recipients"
1453+
)

kernel_patches_daemon/config.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,29 @@ def from_json(cls, json: Dict) -> "BranchConfig":
104104
)
105105

106106

107+
@dataclass
108+
class PRCommentsForwardingConfig:
109+
enabled: bool
110+
always_cc: List[str]
111+
commenter_allowlist: List[str]
112+
recipient_allowlist: List[re.Pattern]
113+
recipient_denylist: List[re.Pattern]
114+
115+
@classmethod
116+
def from_json(cls, json: Dict) -> "PRCommentsForwardingConfig":
117+
return cls(
118+
enabled=json.get("enabled", False),
119+
always_cc=json.get("always_cc", []),
120+
recipient_allowlist=[
121+
re.compile(pattern) for pattern in json.get("recipient_allowlist", [])
122+
],
123+
recipient_denylist=[
124+
re.compile(pattern) for pattern in json.get("recipient_denylist", [])
125+
],
126+
commenter_allowlist=json.get("commenter_allowlist", []),
127+
)
128+
129+
107130
@dataclass
108131
class EmailConfig:
109132
smtp_host: str
@@ -123,6 +146,7 @@ class EmailConfig:
123146
# Ignore the `submitter_allowlist` entries and send emails to all patch
124147
# submitters, unconditionally.
125148
ignore_allowlist: bool
149+
pr_comments_forwarding: Optional[PRCommentsForwardingConfig]
126150

127151
@classmethod
128152
def from_json(cls, json: Dict) -> "EmailConfig":
@@ -139,8 +163,17 @@ def from_json(cls, json: Dict) -> "EmailConfig":
139163
re.compile(pattern) for pattern in json.get("submitter_allowlist", [])
140164
],
141165
ignore_allowlist=json.get("ignore_allowlist", False),
166+
pr_comments_forwarding=PRCommentsForwardingConfig.from_json(
167+
json.get("pr_comments_forwarding", {})
168+
),
142169
)
143170

171+
def is_pr_comment_forwarding_enabled(self) -> bool:
172+
if self.pr_comments_forwarding is not None:
173+
return self.pr_comments_forwarding.enabled
174+
else:
175+
return False
176+
144177

145178
@dataclass
146179
class PatchworksConfig:

kernel_patches_daemon/github_sync.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,7 @@ async def sync_relevant_subject(self, subject: Subject) -> None:
269269
f"Created/updated {pr} ({pr.head.ref}): {pr.url} for series {series.id}"
270270
)
271271
await worker.sync_checks(pr, series)
272+
await worker.forward_pr_comments(pr, series)
272273
# Close out other PRs if exists
273274
self.close_existing_prs_for_series(list(self.workers.values()), pr)
274275

kernel_patches_daemon/patchwork.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,12 @@ def submitter_email(self):
497497
"""Retrieve the email address of the patch series submitter."""
498498
return self._submitter_email
499499

500+
def patch_by_subject(self, subject_str: str) -> Optional[Dict]:
501+
for patch in self.patches:
502+
if subject_str in patch["name"]:
503+
return patch
504+
return None
505+
500506

501507
class Patchwork:
502508
def __init__(

tests/data/fixtures/kpd_config.json

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,13 @@
2222
"pass": "super-secret-is-king",
2323
"http_proxy": "http://example.com:8080",
2424
"submitter_allowlist": ["[email protected]", "[email protected]"],
25-
"ignore_allowlist": true
25+
"ignore_allowlist": true,
26+
"pr_comments_forwarding": {
27+
"enabled": true,
28+
"always_cc": ["[email protected]"],
29+
"commenter_allowlist": ["kpd-bot[bot]"],
30+
"recipient_denylist": [".*@vger.kernel.org"]
31+
}
2632
},
2733
"tag_to_branch_mapping": {
2834
"tag": [

0 commit comments

Comments
 (0)