Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,23 @@ Users can interact with the webhook server through GitHub comments on pull reque
| `/<label>` | Add label | `/bug` |
| `/<label> cancel` | Remove label | `/bug cancel` |

### Review & Approval

| Command | Description | Example |
| ------------------- | ------------------------------------------------------------------------- | ------------------- |
| `/lgtm` | Approve changes (looks good to me) | `/lgtm` |
| `/approve` | Approve PR (approvers only) | `/approve` |
| `/automerge` | Enable automatic merging when all requirements are met (maintainers/approvers only) | `/automerge` |
| `/assign-reviewers` | Assign reviewers based on OWNERS file | `/assign-reviewers` |
| `/assign-reviewer` | Assign specific reviewer | `/assign-reviewer @username` |
| `/check-can-merge` | Checks if the pull request meets all merge requirements | `/check-can-merge` |

### Testing & Validation

| Command | Description | Example |
| ------------------- | ------------------------------------------------------------------------- | ------------------- |
| `/retest <test-name>` | Run specific tests like `tox` or `pre-commit` | `/retest <test-name>` |

## OWNERS File Format

The OWNERS file system provides fine-grained control over code review assignments.
Expand Down
34 changes: 30 additions & 4 deletions webhook_server/libs/check_run_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
from github.CheckRun import CheckRun
from github.PullRequest import PullRequest

from webhook_server.libs.labels_handler import LabelsHandler
from webhook_server.libs.owners_files_handler import OwnersFileHandler
from webhook_server.utils.constants import (
AUTOMERGE_LABEL_STR,
BUILD_CONTAINER_STR,
CAN_BE_MERGED_STR,
CHERRY_PICKED_LABEL_PREFIX,
Expand All @@ -24,14 +27,19 @@


class CheckRunHandler:
def __init__(self, github_webhook: "GithubWebhook"):
def __init__(self, github_webhook: "GithubWebhook", owners_file_handler: OwnersFileHandler | None = None):
self.github_webhook = github_webhook
self.owners_file_handler = owners_file_handler
self.hook_data = self.github_webhook.hook_data
self.logger = self.github_webhook.logger
self.log_prefix = self.github_webhook.log_prefix
self.repository = self.github_webhook.repository
if isinstance(self.owners_file_handler, OwnersFileHandler):
self.labels_handler = LabelsHandler(
github_webhook=self.github_webhook, owners_file_handler=self.owners_file_handler
)

async def process_pull_request_check_run_webhook_data(self) -> bool:
async def process_pull_request_check_run_webhook_data(self, pull_request: PullRequest | None = None) -> bool:
"""Return True if check_if_can_be_merged need to run"""

_check_run: dict[str, Any] = self.hook_data["check_run"]
Expand All @@ -50,8 +58,26 @@ async def process_pull_request_check_run_webhook_data(self) -> bool:
)

if check_run_name == CAN_BE_MERGED_STR:
self.logger.debug(f"{self.log_prefix} check run is {CAN_BE_MERGED_STR}, skipping")
return False
if getattr(self, "labels_handler", None) and pull_request and check_run_conclusion == SUCCESS_STR:
if await self.labels_handler.label_exists_in_pull_request(
label=AUTOMERGE_LABEL_STR, pull_request=pull_request
):
try:
await asyncio.to_thread(pull_request.merge, merge_method="SQUASH")
self.logger.info(
f"{self.log_prefix} Successfully auto-merged pull request #{pull_request.number}"
)
return False
except Exception as ex:
self.logger.error(
f"{self.log_prefix} Failed to auto-merge pull request #{pull_request.number}: {ex}"
)
# Continue processing to allow manual intervention
return True

else:
self.logger.debug(f"{self.log_prefix} check run is {CAN_BE_MERGED_STR}, skipping")
return False

return True

Expand Down
9 changes: 5 additions & 4 deletions webhook_server/libs/github_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,11 @@ async def process(self) -> Any:
)

elif self.github_event == "check_run":
if await CheckRunHandler(github_webhook=self).process_pull_request_check_run_webhook_data():
owners_file_handler = OwnersFileHandler(github_webhook=self)
owners_file_handler = await owners_file_handler.initialize(pull_request=pull_request)

owners_file_handler = OwnersFileHandler(github_webhook=self)
owners_file_handler = await owners_file_handler.initialize(pull_request=pull_request)
if await CheckRunHandler(
github_webhook=self, owners_file_handler=owners_file_handler
).process_pull_request_check_run_webhook_data(pull_request=pull_request):
return await PullRequestHandler(
github_webhook=self, owners_file_handler=owners_file_handler
).check_if_can_be_merged(pull_request=pull_request)
Expand Down
17 changes: 15 additions & 2 deletions webhook_server/libs/issue_comment_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from webhook_server.libs.pull_request_handler import PullRequestHandler
from webhook_server.libs.runner_handler import RunnerHandler
from webhook_server.utils.constants import (
AUTOMERGE_LABEL_STR,
BUILD_AND_PUSH_CONTAINER_STR,
BUILD_CONTAINER_STR,
CHERRY_PICK_LABEL_PREFIX,
Expand Down Expand Up @@ -127,6 +128,18 @@ async def user_commands(
await asyncio.to_thread(pull_request.create_issue_comment, body=missing_command_arg_comment_msg)
return

if _command == AUTOMERGE_LABEL_STR:
if reviewed_user not in (
await self.owners_file_handler.get_all_repository_maintainers()
+ self.owners_file_handler.all_repository_approvers
):
msg = "Only maintainers or approvers can set pull request to auto-merge"
self.logger.debug(f"{self.log_prefix} {msg}")
await asyncio.to_thread(pull_request.create_issue_comment, body=msg)
return

await self.labels_handler._add_label(pull_request=pull_request, label=AUTOMERGE_LABEL_STR)

await self.create_comment_reaction(
pull_request=pull_request, issue_comment_id=issue_comment_id, reaction=REACTIONS.ok
)
Expand Down Expand Up @@ -203,7 +216,7 @@ async def user_commands(
await self.labels_handler._add_label(pull_request=pull_request, label=VERIFIED_LABEL_STR)
await self.check_run_handler.set_verify_check_success()

else:
elif _command != AUTOMERGE_LABEL_STR:
await self.labels_handler.label_by_user_comment(
pull_request=pull_request,
user_requested_label=_command,
Expand Down Expand Up @@ -342,4 +355,4 @@ async def process_retest_command(
self.logger.error(f"{self.log_prefix} Async task failed: {result}")

if automerge:
await self.labels_handler._add_label(pull_request=pull_request, label="automerge")
await self.labels_handler._add_label(pull_request=pull_request, label=AUTOMERGE_LABEL_STR)
8 changes: 6 additions & 2 deletions webhook_server/libs/pull_request_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from webhook_server.libs.runner_handler import RunnerHandler
from webhook_server.utils.constants import (
APPROVED_BY_LABEL_PREFIX,
AUTOMERGE_LABEL_STR,
BRANCH_LABEL_PREFIX,
BUILD_CONTAINER_STR,
CAN_BE_MERGED_STR,
Expand Down Expand Up @@ -48,7 +49,9 @@ def __init__(self, github_webhook: "GithubWebhook", owners_file_handler: OwnersF
self.labels_handler = LabelsHandler(
github_webhook=self.github_webhook, owners_file_handler=self.owners_file_handler
)
self.check_run_handler = CheckRunHandler(github_webhook=self.github_webhook)
self.check_run_handler = CheckRunHandler(
github_webhook=self.github_webhook, owners_file_handler=self.owners_file_handler
)
self.runner_handler = RunnerHandler(
github_webhook=self.github_webhook, owners_file_handler=self.owners_file_handler
)
Expand Down Expand Up @@ -160,7 +163,7 @@ async def process_pull_request_webhook_data(self, pull_request: PullRequest) ->
else:
await self.check_run_handler.set_verify_check_queued()

if labeled_lower in (WIP_STR, HOLD_LABEL_STR):
if labeled_lower in (WIP_STR, HOLD_LABEL_STR, AUTOMERGE_LABEL_STR):
_check_for_merge = True
self.logger.debug(f"PR has {labeled_lower} label, will check for merge.")

Expand Down Expand Up @@ -225,6 +228,7 @@ def _prepare_welcome_comment(self) -> str:
#### Review & Approval
* `/lgtm` - Approve changes (looks good to me)
* `/approve` - Approve PR (approvers only)
* `/automerge` - Enable automatic merging when all requirements are met (maintainers and approvers only)
* `/assign-reviewers` - Assign reviewers based on OWNERS file
* `/assign-reviewer @username` - Assign specific reviewer
* `/check-can-merge` - Check if PR meets merge requirements
Expand Down
14 changes: 7 additions & 7 deletions webhook_server/libs/runner_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@
class RunnerHandler:
def __init__(self, github_webhook: "GithubWebhook", owners_file_handler: OwnersFileHandler | None = None):
self.github_webhook = github_webhook
self.owners_file_handler = owners_file_handler
self.owners_file_handler = owners_file_handler or OwnersFileHandler(github_webhook=self.github_webhook)
self.hook_data = self.github_webhook.hook_data
self.logger = self.github_webhook.logger
self.log_prefix = self.github_webhook.log_prefix
self.repository = self.github_webhook.repository

self.check_run_handler = CheckRunHandler(github_webhook=self.github_webhook)
self.check_run_handler = CheckRunHandler(
github_webhook=self.github_webhook, owners_file_handler=self.owners_file_handler
)

@contextlib.asynccontextmanager
async def _prepare_cloned_repo_dir(
Expand Down Expand Up @@ -185,12 +187,10 @@ async def run_tox(self, pull_request: PullRequest) -> None:
)
cmd = f"uvx {python_ver} {TOX_STR} --workdir {clone_repo_dir} --root {clone_repo_dir} -c {clone_repo_dir}"
_tox_tests = self.github_webhook.tox.get(pull_request.base.ref, "")
if not _tox_tests:
tests = ""
else:
tests = f" -e {_tox_tests}"

cmd += tests
if _tox_tests and _tox_tests != "all":
tests = _tox_tests.replace(" ", "")
cmd += f" -e {tests}"

await self.check_run_handler.set_run_tox_check_in_progress()
self.logger.debug(f"{self.log_prefix} Tox command to run: {cmd}")
Expand Down
32 changes: 1 addition & 31 deletions webhook_server/tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from fastapi.testclient import TestClient
import ipaddress

from webhook_server.app import FASTAPI_APP, verify_signature, gate_by_allowlist_ips
from webhook_server.app import FASTAPI_APP, gate_by_allowlist_ips
from webhook_server.libs.exceptions import RepositoryNotFoundError


Expand Down Expand Up @@ -141,36 +141,6 @@ def test_process_webhook_repository_not_found(
assert response.status_code == 404
assert "Repository not found in configuration" in response.json()["detail"]

def test_verify_signature_success(self) -> None:
"""Test successful signature verification."""
payload = "test payload"
secret = "test-secret" # pragma: allowlist secret
signature = self.create_github_signature(payload, secret)

# Should not raise any exception
verify_signature(payload.encode(), secret, signature)

def test_verify_signature_missing_header(self) -> None:
"""Test signature verification with missing header."""
payload = "test payload"
secret = "test-secret" # pragma: allowlist secret

with pytest.raises(Exception) as exc_info:
verify_signature(payload.encode(), secret, None)

assert "x-hub-signature-256 header is missing" in str(exc_info.value)

def test_verify_signature_invalid_signature(self) -> None:
"""Test signature verification with invalid signature."""
payload = "test payload"
secret = "test-secret" # pragma: allowlist secret
invalid_signature = "sha256=invalid"

with pytest.raises(Exception) as exc_info:
verify_signature(payload.encode(), secret, invalid_signature)

assert "Request signatures didn't match" in str(exc_info.value)

@patch.dict(os.environ, {"WEBHOOK_SERVER_DATA_DIR": "webhook_server/tests/manifests"})
def test_process_webhook_signature_verification_failure(
self, client: TestClient, valid_webhook_payload: dict[str, Any]
Expand Down
15 changes: 9 additions & 6 deletions webhook_server/utils/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@
HAS_CONFLICTS_LABEL_STR: str = "has-conflicts"
HOLD_LABEL_STR: str = "hold"
SIZE_LABEL_PREFIX: str = "size/"
COMMAND_RETEST_STR = "retest"
COMMAND_CHERRY_PICK_STR = "cherry-pick"
COMMAND_ASSIGN_REVIEWERS_STR = "assign-reviewers"
COMMAND_CHECK_CAN_MERGE_STR = "check-can-merge"
COMMAND_ASSIGN_REVIEWER_STR = "assign-reviewer"
COMMAND_ADD_ALLOWED_USER_STR = "add-allowed-user"
COMMAND_RETEST_STR: str = "retest"
COMMAND_CHERRY_PICK_STR: str = "cherry-pick"
COMMAND_ASSIGN_REVIEWERS_STR: str = "assign-reviewers"
COMMAND_CHECK_CAN_MERGE_STR: str = "check-can-merge"
COMMAND_ASSIGN_REVIEWER_STR: str = "assign-reviewer"
COMMAND_ADD_ALLOWED_USER_STR: str = "add-allowed-user"
COMMAND_AUTOMERGE_STR: str = "automerge"
AUTOMERGE_LABEL_STR: str = "automerge"

# Gitlab colors require a '#' prefix; e.g: #
USER_LABELS_DICT: dict[str, str] = {
Expand All @@ -42,6 +44,7 @@
WIP_STR: "B60205",
LGTM_STR: "0E8A16",
APPROVE_STR: "0E8A16",
AUTOMERGE_LABEL_STR: "0E8A16",
}

STATIC_LABELS_DICT: dict[str, str] = {
Expand Down