Skip to content

Commit 792bbbc

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 f32c3e0 commit 792bbbc

File tree

7 files changed

+324
-3
lines changed

7 files changed

+324
-3
lines changed

kernel_patches_daemon/branch_worker.py

Lines changed: 132 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,61 @@ 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: Sequence[re.Pattern] = None,
376+
denylist: Sequence[re.Pattern] = None,
377+
) -> Tuple[List[str], List[str]]:
378+
"""
379+
`msg` is an EmailMessage we are replying to
380+
"""
381+
tos = msg.get_all("To", [])
382+
ccs = msg.get_all("Cc", [])
383+
cc_list = [a for (_, a) in email.utils.getaddresses(tos + ccs)]
384+
385+
(_, sender_address) = email.utils.parseaddr(msg.get("From"))
386+
to_list = [sender_address]
387+
388+
if allowlist:
389+
cc_list = [a for a in cc_list if email_matches_any(a, allowlist)]
390+
if not email_matches_any(sender_address, allowlist):
391+
to_list = []
392+
393+
if denylist:
394+
cc_list = [a for a in cc_list if not email_matches_any(a, denylist)]
395+
if email_matches_any(sender_address, denylist):
396+
to_list = []
397+
398+
return (to_list, cc_list)
399+
400+
401+
async def send_pr_comment_email(
402+
config: EmailConfig, msg: EmailMessage, body: str
403+
) -> Optional[str]:
404+
"""
405+
`msg` is an EmailMessage we are replying to
406+
"""
407+
if config is None or not config.is_pr_comment_forwarding_enabled():
408+
return
409+
410+
cfg = config.pr_comments_forwarding
411+
(to_list, cc_list) = reply_email_recipients(
412+
msg, allowlist=cfg.recipient_allowlist, denylist=cfg.recipient_denylist
413+
)
414+
if not to_list:
415+
to_list = cfg.always_cc
416+
else:
417+
cc_list += cfg.always_cc
418+
419+
if not to_list:
420+
return
421+
422+
subject = "Re: " + msg.get("Subject")
423+
in_reply_to = msg.get("Message-Id")
424+
425+
return await send_email(config, to_list, cc_list, subject, body, in_reply_to)
426+
427+
371428
def pr_has_label(pr: PullRequest, label: str) -> bool:
372429
for pr_label in pr.get_labels():
373430
if pr_label.name == label:
@@ -1307,3 +1364,77 @@ async def submit_pr_summary(
13071364
description="PR summary",
13081365
)
13091366
pr_summary_report.add(1)
1367+
1368+
async def forward_pr_comments(self, pr: PullRequest, series: Series):
1369+
if (
1370+
self.email_config is None
1371+
or not self.email_config.is_pr_comment_forwarding_enabled()
1372+
):
1373+
return
1374+
cfg = self.email_config.pr_comments_forwarding
1375+
1376+
comments = pr.get_issue_comments()
1377+
1378+
# Look for comments indicating what has already been forwarded
1379+
# to filter them out
1380+
forwarded_set = set()
1381+
for comment in comments:
1382+
match = re.search(
1383+
r"Forwarding comment \[([0-9]+)\].* via email",
1384+
comment.body,
1385+
re.MULTILINE,
1386+
)
1387+
if not match:
1388+
continue
1389+
forwarded_id = int(match.group(1))
1390+
forwarded_set.add(forwarded_id)
1391+
1392+
for comment in comments:
1393+
if (
1394+
comment.user.login not in cfg.commenter_allowlist
1395+
or comment.id in forwarded_set
1396+
):
1397+
continue
1398+
# Determine the message to reply to
1399+
# Check for In-Reply-To-Subject tag in the comment body
1400+
match = re.search(
1401+
r"In-Reply-To-Subject: \`(.*)\`", comment.body, re.MULTILINE
1402+
)
1403+
if not match:
1404+
logger.info(
1405+
f"Ignoring PR comment {comment.html_url} without In-Reply-To-Subject tag"
1406+
)
1407+
continue
1408+
subject = match.group(1)
1409+
patch = series.patch_by_subject(subject)
1410+
if not patch:
1411+
logger.warn(
1412+
f"Ignoring PR comment {comment.html_url}, could not find relevant patch on patchwork"
1413+
)
1414+
continue
1415+
1416+
# first, post a comment that the message is being forwarded
1417+
msg_id = patch["msgid"]
1418+
patch_url = patch["web_url"]
1419+
message = f"Forwarding comment [{comment.id}]({comment.html_url}) via email"
1420+
message += f"\nIn-Reply-To: {msg_id}"
1421+
message += f"\nPatch: {patch_url}"
1422+
self._add_pull_request_comment(pr, message)
1423+
1424+
# then, load the message we are replying to
1425+
mbox = await series.pw_client.get_blob(patch["mbox"])
1426+
parser = email.parser.BytesParser(policy=email.policy.default)
1427+
patch_msg = parser.parsebytes(mbox, headersonly=True)
1428+
1429+
# and forward the target comment via email
1430+
sent_msg_id = await send_pr_comment_email(
1431+
self.email_config, patch_msg, comment.body
1432+
)
1433+
if sent_msg_id is not None:
1434+
logger.info(
1435+
f"Forwarded PR comment {comment.html_url} via email, Message-Id: {sent_msg_id}"
1436+
)
1437+
else:
1438+
logger.warn(
1439+
f"Failed to forward PR comment in reply to {msg_id}, to_list is empty"
1440+
)

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)