Skip to content

Commit

Permalink
Introduce email notification submitter allowlist
Browse files Browse the repository at this point in the history
Summary:
We want to onboard Meta-internal kernel developers to the email notification
workflow, by sending them emails about CI workflow run results of their own
submissions.
This change puts the necessary infrastructure in place. Specifically, we
introduce the 'submitter_allowlist' attribute to the configuration, which is
meant to contain a list of email addresses. If a CI result comes in and the
submitters email address is in this list an email will be sent to said submitter
(in addition to all the recipients that we send emails to unconditionally).

Reviewed By: chantra

Differential Revision: D49711217

fbshipit-source-id: be2cd0be34d703d4185ac164e080a90efc526507
  • Loading branch information
danielocfb-test authored and facebook-github-bot committed Sep 28, 2023
1 parent fcbda00 commit c50625e
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 6 deletions.
12 changes: 8 additions & 4 deletions kernel_patches_daemon/branch_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ def furnish_ci_email_body(
)


async def send_email(config: EmailConfig, subject: str, body: str):
async def send_email(config: EmailConfig, series: Series, subject: str, body: str):
"""Send an email."""
# We work with curl as it's the only thing that supports SMTP via an HTTP
# proxy, which is something we require for production environments.
Expand All @@ -220,7 +220,11 @@ async def send_email(config: EmailConfig, subject: str, body: str):
"-",
]

for to in config.smtp_to:
to_list = copy.copy(config.smtp_to)
if series.submitter_email in config.submitter_allowlist:
to_list += [series.submitter_email]

for to in to_list:
args += ["--mail-rcpt", to]

if config.smtp_http_proxy is not None:
Expand All @@ -229,7 +233,7 @@ async def send_email(config: EmailConfig, subject: str, body: str):
msg = MIMEMultipart()
msg["Subject"] = subject
msg["From"] = config.smtp_from
msg["To"] = ",".join(config.smtp_to)
msg["To"] = ",".join(to_list)
msg.attach(MIMEText(body, "plain"))
proc = await asyncio.create_subprocess_exec(
*args, stdin=PIPE, stdout=PIPE, stderr=PIPE
Expand Down Expand Up @@ -1015,7 +1019,7 @@ async def evaluate_ci_result(self, series: Series, pr: PullRequest, vmtests=None
# way, send an email notifying the submitter.
logger.info(f"PR {pr.number} {pr.title} is now {new_label}; adding label")
pr.add_to_labels(new_label)
await send_email(email, subject, body)
await send_email(email, series, subject, body)

def expire_branches(self) -> None:
for branch in self.branches:
Expand Down
9 changes: 8 additions & 1 deletion kernel_patches_daemon/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import logging
import os
from dataclasses import dataclass
from typing import Dict, List, Optional
from typing import Dict, List, Optional, Set


logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -107,6 +107,12 @@ class EmailConfig:
smtp_pass: str
smtp_to: List[str]
smtp_http_proxy: Optional[str]
# List of email addresses of patch submitters to whom we send email
# notifications only for *their* very submission. This attribute is meant to
# be temporary while the email notification feature is being rolled out.
# Once we send email notifications to all patch submitters it can be
# removed.
submitter_allowlist: Set[str]

@classmethod
def from_json(cls, json: Dict) -> "EmailConfig":
Expand All @@ -118,6 +124,7 @@ def from_json(cls, json: Dict) -> "EmailConfig":
smtp_pass=json["pass"],
smtp_to=json.get("to", []),
smtp_http_proxy=json.get("http_proxy", None),
submitter_allowlist=json.get("submitter_allowlist", set()),
)


Expand Down
9 changes: 9 additions & 0 deletions kernel_patches_daemon/patchwork.py
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ def __init__(self, pw_client: "Patchwork", data: Dict) -> None:
self.url = data["url"]
self.web_url = data["web_url"]
self.version = data["version"]
self._submitter_email = data["submitter"]["email"]
self.mbox = data["mbox"]
self.patches = data.get("patches", [])
self.cover_letter = data.get("cover_letter")
Expand Down Expand Up @@ -474,6 +475,11 @@ def to_json(self) -> str:
def __repr__(self) -> str:
return f"Series({self.to_json()})"

@property
def submitter_email(self):
"""Retrieve the email address of the patch series submitter."""
return self._submitter_email


class Patchwork:
def __init__(
Expand Down Expand Up @@ -709,6 +715,9 @@ async def get_relevant_subjects(self) -> Sequence[Subject]:
patch_series = patch["series"]
for series_data in patch_series:
if series_data["name"]:
series_data["submitter"] = {
"email": patch["submitter"]["email"],
}
series = Series(self, series_data)
logger.debug(f"Adding {series.id} into list of known series.")
self.known_series[series.id] = series
Expand Down
16 changes: 16 additions & 0 deletions tests/common/patchwork_mock.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ def get_dict_key(d: Dict[Any, Any], idx: int = 0) -> Any:
"version": 0,
"url": "https://example.com",
"web_url": "https://example.com",
"submitter": {"email": "a-user@example.com"},
"mbox": "https://example.com",
},
# Matches and has relevant diff and is neither the oldest, nor the newest serie. Appears before FOO_SERIES_FIRST to ensure sorting is performed.
Expand All @@ -102,6 +103,7 @@ def get_dict_key(d: Dict[Any, Any], idx: int = 0) -> Any:
"version": 0,
"url": "https://example.com",
"web_url": "https://example.com",
"submitter": {"email": "a-user@example.com"},
"mbox": "https://example.com",
},
# Matches and has relevant diff
Expand All @@ -113,6 +115,7 @@ def get_dict_key(d: Dict[Any, Any], idx: int = 0) -> Any:
"version": 0,
"url": "https://example.com",
"web_url": "https://example.com",
"submitter": {"email": "a-user@example.com"},
"mbox": "https://example.com",
},
# Matches and has only non-relevant diffs
Expand All @@ -124,6 +127,7 @@ def get_dict_key(d: Dict[Any, Any], idx: int = 0) -> Any:
"version": 0,
"url": "https://example.com",
"web_url": "https://example.com",
"submitter": {"email": "a-user@example.com"},
"mbox": "https://example.com",
},
# Matches and has one relevant diffs
Expand All @@ -135,6 +139,7 @@ def get_dict_key(d: Dict[Any, Any], idx: int = 0) -> Any:
"version": 0,
"url": "https://example.com",
"web_url": "https://example.com",
"submitter": {"email": "a-user@example.com"},
"mbox": "https://example.com",
},
# Matches and has one relevant diffs, not the most recent series, appears after FOO_SERIES_LAST to ensure sorting is performed.
Expand All @@ -146,6 +151,7 @@ def get_dict_key(d: Dict[Any, Any], idx: int = 0) -> Any:
"version": 0,
"url": "https://example.com",
"web_url": "https://example.com",
"submitter": {"email": "a-user@example.com"},
"mbox": "https://example.com",
},
# Matches and has only non-relevant diffs
Expand All @@ -157,6 +163,7 @@ def get_dict_key(d: Dict[Any, Any], idx: int = 0) -> Any:
"version": 0,
"url": "https://example.com",
"web_url": "https://example.com",
"submitter": {"email": "a-user@example.com"},
"mbox": "https://example.com",
},
],
Expand All @@ -171,6 +178,7 @@ def get_dict_key(d: Dict[Any, Any], idx: int = 0) -> Any:
"version": 1,
"url": "https://example.com",
"web_url": "https://example.com",
"submitter": {"email": "a-user@example.com"},
"mbox": "https://example.com",
},
# Matches, has one relevant diffs, and is the most recent series.
Expand All @@ -182,6 +190,7 @@ def get_dict_key(d: Dict[Any, Any], idx: int = 0) -> Any:
"version": 2,
"url": "https://example.com",
"web_url": "https://example.com",
"submitter": {"email": "a-user@example.com"},
"mbox": "https://example.com",
},
],
Expand All @@ -196,6 +205,8 @@ def get_dict_key(d: Dict[Any, Any], idx: int = 0) -> Any:
"version": 2,
"url": "https://example.com",
"web_url": "https://example.com",
"submitter": {"email": "a-user@example.com"},
"submitter": {"email": "a-user@example.com"},
"mbox": "https://example.com",
},
],
Expand Down Expand Up @@ -254,6 +265,7 @@ def get_dict_key(d: Dict[Any, Any], idx: int = 0) -> Any:
"version": 4,
"url": "https://example.com",
"web_url": "https://example.com",
"submitter": {"email": "a-user@example.com"},
"mbox": "https://example.com",
},
# Patch in an relevant state.
Expand Down Expand Up @@ -297,6 +309,7 @@ def get_dict_key(d: Dict[Any, Any], idx: int = 0) -> Any:
"version": 0,
"url": "https://example.com",
"web_url": "https://example.com",
"submitter": {"email": "a-user@example.com"},
"mbox": "https://example.com",
},
# Patch in an irrelevant state.
Expand Down Expand Up @@ -325,6 +338,7 @@ def get_dict_key(d: Dict[Any, Any], idx: int = 0) -> Any:
"version": 1,
"url": "https://example.com",
"web_url": "https://example.com",
"submitter": {"email": "a-user@example.com"},
"mbox": "https://example.com",
},
# Expiration test cases
Expand All @@ -338,6 +352,7 @@ def get_dict_key(d: Dict[Any, Any], idx: int = 0) -> Any:
"version": 1,
"url": "https://example.com",
"web_url": "https://example.com",
"submitter": {"email": "a-user@example.com"},
"mbox": "https://example.com",
},
# Patch in a non-expirable state.
Expand Down Expand Up @@ -375,6 +390,7 @@ def get_dict_key(d: Dict[Any, Any], idx: int = 0) -> Any:
"version": 1,
"url": "https://example.com",
"web_url": "https://example.com",
"submitter": {"email": "a-user@example.com"},
"mbox": "https://example.com",
},
# Patch in a non-expirable state.
Expand Down
3 changes: 2 additions & 1 deletion tests/fixtures/kpd_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
"from": "bot+bpf-ci@example.com",
"to": ["email1@example.com", "email2@example.com"],
"pass": "super-secret-is-king",
"http_proxy": "http://example.com:8080"
"http_proxy": "http://example.com:8080",
"submitter_allowlist": ["email3@example.com", "email4@example.com"]
},
"tag_to_branch_mapping": {
"tag": [
Expand Down
1 change: 1 addition & 0 deletions tests/test_branch_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
"version": 0,
"url": "https://example.com",
"web_url": "https://example.com",
"submitter": {"email": "a-user@example.com"},
"mbox": "https://example.com",
}

Expand Down
2 changes: 2 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ def test_valid(self) -> None:
smtp_to=["email1@example.com", "email2@example.com"],
smtp_pass="super-secret-is-king",
smtp_http_proxy="http://example.com:8080",
# pyre-ignore
submitter_allowlist=["email3@example.com", "email4@example.com"],
),
tag_to_branch_mapping={"tag": ["branch"]},
branches={
Expand Down

0 comments on commit c50625e

Please sign in to comment.