diff --git a/.gitignore b/.gitignore index 516556d7..60abbcef 100644 --- a/.gitignore +++ b/.gitignore @@ -148,3 +148,4 @@ local-run.sh .scannerwork/ webhook-server.private-key.pem log-colors.json +webhook_server/tests/manifests/logs diff --git a/Dockerfile b/Dockerfile index cdc2d65f..91604196 100644 --- a/Dockerfile +++ b/Dockerfile @@ -34,9 +34,8 @@ RUN mkdir -p $BIN_DIR \ && mkdir -p $DATA_DIR \ && mkdir -p $DATA_DIR/logs -COPY entrypoint.sh entrypoint.py pyproject.toml uv.lock README.md $APP_DIR/ +COPY entrypoint.py pyproject.toml uv.lock README.md $APP_DIR/ COPY webhook_server $APP_DIR/webhook_server/ -RUN chmod +x $APP_DIR/entrypoint.sh RUN usermod --add-subuids 100000-165535 --add-subgids 100000-165535 $USERNAME \ && chown -R $USERNAME:$USERNAME $HOME_DIR @@ -67,4 +66,4 @@ RUN uv sync HEALTHCHECK CMD curl --fail http://127.0.0.1:5000/webhook_server/healthcheck || exit 1 -ENTRYPOINT ["./entrypoint.sh"] +ENTRYPOINT ["uv", "run", "entrypoint.py"] diff --git a/entrypoint.py b/entrypoint.py index 8e080206..8bb3350e 100644 --- a/entrypoint.py +++ b/entrypoint.py @@ -1,3 +1,7 @@ +import asyncio + +import uvicorn + from webhook_server.libs.config import Config from webhook_server.utils.github_repository_and_webhook_settings import repository_and_webhook_settings @@ -8,6 +12,13 @@ _max_workers = _root_config.get("max-workers", 10) _webhook_secret = _root_config.get("webhook-secret") + if __name__ == "__main__": - repository_and_webhook_settings(webhook_secret=_webhook_secret) - print(f"uv run uvicorn webhook_server.app:FASTAPI_APP --host {_ip_bind} --port {_port} --workers {_max_workers}") + result = asyncio.run(repository_and_webhook_settings(webhook_secret=_webhook_secret)) + uvicorn.run( + "webhook_server.app:FASTAPI_APP", + host=_ip_bind, + port=int(_port), + workers=int(_max_workers), + reload=False, + ) diff --git a/entrypoint.sh b/entrypoint.sh deleted file mode 100755 index 76081a23..00000000 --- a/entrypoint.sh +++ /dev/null @@ -1,9 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -# Generate the uvicorn command from Python -cmd=$(uv run ./entrypoint.py) -echo "${cmd}" - -# Replace the shell with the server process (PID 1 inside container) -exec ${cmd} diff --git a/pyproject.toml b/pyproject.toml index a4e07c7e..7702ef09 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -66,6 +66,7 @@ dependencies = [ "timeout-sampler>=0.0.46", "uvicorn>=0.31.0", "httpx>=0.28.1", + "asyncstdlib>=3.13.1", ] [[project.authors]] @@ -82,6 +83,11 @@ dependencies = [ Download = "https://quay.io/repository/myakove/github-webhook-server" "Bug Tracker" = "https://github.com/myakove/github-webhook-server/issues" +[project.optional-dependencies] +tests = [ + "pytest-asyncio>=0.26.0", +] + [build-system] requires = [ "hatchling" ] build-backend = "hatchling.build" diff --git a/tox.toml b/tox.toml index 4905ed01..e04e8152 100644 --- a/tox.toml +++ b/tox.toml @@ -14,4 +14,4 @@ commands = [ [env.unittests] deps = ["uv"] -commands = [["uv", "run", "pytest", "webhook_server/tests"]] +commands = [["uv", "run", "--extra", "tests", "pytest", "webhook_server/tests"]] diff --git a/uv.lock b/uv.lock index dc8abfef..b0507ee3 100644 --- a/uv.lock +++ b/uv.lock @@ -34,6 +34,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/25/8a/c46dcc25341b5bce5472c718902eb3d38600a903b14fa6aeecef3f21a46f/asttokens-3.0.0-py3-none-any.whl", hash = "sha256:e3078351a059199dd5138cb1c706e6430c05eff2ff136af5eb4790f9d28932e2", size = 26918, upload-time = "2024-11-30T04:30:10.946Z" }, ] +[[package]] +name = "asyncstdlib" +version = "3.13.1" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/50/e1/72e388631c85233a2fd890d024fc20a8a9961dbba8614d78266636218f1f/asyncstdlib-3.13.1.tar.gz", hash = "sha256:f47564b9a3566f8f9172631d88c75fe074b0ce2127963b7265d310df9aeed03a", size = 49752, upload-time = "2025-03-09T07:52:51.587Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/b9/4a/c86c045bc7bb0244044935ba80c83998f1fdee4f4cef64c6b078e043b0e6/asyncstdlib-3.13.1-py3-none-any.whl", hash = "sha256:a64da68176af1da8c699026cad98f70b184f82b4cb39739e0b9701a2a7541cf9", size = 43993, upload-time = "2025-03-09T07:52:50.108Z" }, +] + [[package]] name = "bcrypt" version = "4.3.0" @@ -344,6 +353,7 @@ name = "github-webhook-server" version = "3.0.0" source = { editable = "." } dependencies = [ + { name = "asyncstdlib" }, { name = "build" }, { name = "colorama" }, { name = "colorlog" }, @@ -364,6 +374,11 @@ dependencies = [ { name = "uvicorn" }, ] +[package.optional-dependencies] +tests = [ + { name = "pytest-asyncio" }, +] + [package.dev-dependencies] dev = [ { name = "ipdb" }, @@ -372,6 +387,7 @@ dev = [ [package.metadata] requires-dist = [ + { name = "asyncstdlib", specifier = ">=3.13.1" }, { name = "build", specifier = ">=1.2.2.post1" }, { name = "colorama", specifier = ">=0.4.6" }, { name = "colorlog", specifier = ">=6.8.2" }, @@ -380,6 +396,7 @@ requires-dist = [ { name = "pygithub", specifier = ">=2.4.0" }, { name = "pyhelper-utils", specifier = ">=0.0.42" }, { name = "pytest", specifier = ">=8.3.3" }, + { name = "pytest-asyncio", marker = "extra == 'tests'", specifier = ">=0.26.0" }, { name = "pytest-cov", specifier = ">=6.0.0" }, { name = "pytest-mock", specifier = ">=3.14.0" }, { name = "python-simple-logger", specifier = ">=1.0.40" }, @@ -391,6 +408,7 @@ requires-dist = [ { name = "timeout-sampler", specifier = ">=0.0.46" }, { name = "uvicorn", specifier = ">=0.31.0" }, ] +provides-extras = ["tests"] [package.metadata.requires-dev] dev = [ @@ -799,6 +817,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/30/3d/64ad57c803f1fa1e963a7946b6e0fea4a70df53c1a7fed304586539c2bac/pytest-8.3.5-py3-none-any.whl", hash = "sha256:c69214aa47deac29fad6c2a4f590b9c4a9fdb16a403176fe154b79c0b4d4d820", size = 343634, upload-time = "2025-03-02T12:54:52.069Z" }, ] +[[package]] +name = "pytest-asyncio" +version = "0.26.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pytest" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/8e/c4/453c52c659521066969523e87d85d54139bbd17b78f09532fb8eb8cdb58e/pytest_asyncio-0.26.0.tar.gz", hash = "sha256:c4df2a697648241ff39e7f0e4a73050b03f123f760673956cf0d72a4990e312f", size = 54156, upload-time = "2025-03-25T06:22:28.883Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/20/7f/338843f449ace853647ace35870874f69a764d251872ed1b4de9f234822c/pytest_asyncio-0.26.0-py3-none-any.whl", hash = "sha256:7b51ed894f4fbea1340262bdae5135797ebbe21d8638978e35d31c6d19f72fb0", size = 19694, upload-time = "2025-03-25T06:22:27.807Z" }, +] + [[package]] name = "pytest-cov" version = "6.1.1" diff --git a/webhook_server/app.py b/webhook_server/app.py index 70a876ec..7a707a1c 100644 --- a/webhook_server/app.py +++ b/webhook_server/app.py @@ -1,12 +1,14 @@ import hashlib import hmac import ipaddress +import json import logging import os import sys from contextlib import asynccontextmanager from typing import Any, AsyncGenerator +import httpx import requests import urllib3 from fastapi import ( @@ -20,15 +22,55 @@ from starlette.datastructures import Headers from webhook_server.libs.config import Config -from webhook_server.libs.exceptions import NoPullRequestError, RepositoryNotFoundError +from webhook_server.libs.exceptions import RepositoryNotFoundError from webhook_server.libs.github_api import GithubWebhook from webhook_server.utils.helpers import get_logger_with_params ALLOWED_IPS: tuple[ipaddress._BaseNetwork, ...] = () +LOGGER = get_logger_with_params(name="main") + APP_URL_ROOT_PATH: str = "/webhook_server" urllib3.disable_warnings() +_lifespan_http_client: httpx.AsyncClient | None = None + + +async def get_github_allowlist() -> list[str]: + """Fetch and cache GitHub IP allowlist asynchronously.""" + try: + assert _lifespan_http_client is not None + response = await _lifespan_http_client.get("https://api.github.com/meta") + response.raise_for_status() # Check for HTTP errors + data = response.json() + return data.get("hooks", []) + + except httpx.RequestError as e: + LOGGER.error(f"Error fetching GitHub allowlist: {e}") + raise + + except Exception as e: + LOGGER.error(f"Unexpected error fetching GitHub allowlist: {e}") + raise + + +async def get_cloudflare_allowlist() -> list[str]: + """Fetch and cache Cloudflare IP allowlist asynchronously.""" + try: + assert _lifespan_http_client is not None + response = await _lifespan_http_client.get("https://api.cloudflare.com/client/v4/ips") + response.raise_for_status() + result = response.json().get("result", {}) + return result.get("ipv4_cidrs", []) + result.get("ipv6_cidrs", []) + + except httpx.RequestError as e: + LOGGER.error(f"Error fetching Cloudflare allowlist: {e}") + raise + + except Exception as e: + LOGGER.error(f"Unexpected error fetching Cloudflare allowlist: {e}") + raise + def verify_signature(payload_body: bytes, secret_token: str, signature_header: Headers | None = None) -> None: """Verify that the payload was sent from GitHub by validating SHA256. @@ -50,22 +92,6 @@ def verify_signature(payload_body: bytes, secret_token: str, signature_header: H raise HTTPException(status_code=403, detail="Request signatures didn't match!") -def get_github_allowlist() -> list[str]: - """Fetch and cache GitHub IP allowlist""" - response = requests.get("https://api.github.com/meta", timeout=5) - response.raise_for_status() - data = response.json() - return data.get("hooks", []) - - -def get_cloudflare_allowlist() -> list[str]: - """Fetch and cache Cloudflare IP allowlist""" - response = requests.get("https://api.cloudflare.com/client/v4/ips", timeout=5) - response.raise_for_status() - result = response.json()["result"] - return result.get("ipv4_cidrs", []) + result.get("ipv6_cidrs", []) - - async def gate_by_allowlist_ips(request: Request) -> None: if ALLOWED_IPS: try: @@ -85,36 +111,54 @@ async def gate_by_allowlist_ips(request: Request) -> None: @asynccontextmanager async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: - logger = get_logger_with_params(name="startup") + global _lifespan_http_client + _lifespan_http_client = httpx.AsyncClient(timeout=10.0) try: - logger.info("Application starting up...") - config = Config(logger=logger) + LOGGER.info("Application starting up...") + config = Config(logger=LOGGER) root_config = config.root_data verify_github_ips = root_config.get("verify-github-ips") verify_cloudflare_ips = root_config.get("verify-cloudflare-ips") - logger.info("Repository and webhook settings initialized successfully.") + LOGGER.debug(f"verify_github_ips: {verify_github_ips}, verify_cloudflare_ips: {verify_cloudflare_ips}") global ALLOWED_IPS + networks: set[ipaddress._BaseNetwork] = set() - if verify_github_ips or verify_cloudflare_ips: - networks: list[ipaddress._BaseNetwork] = [] + if verify_cloudflare_ips: + cf_ips = await get_cloudflare_allowlist() - if verify_cloudflare_ips: - networks += [ipaddress.ip_network(cidr) for cidr in get_cloudflare_allowlist()] + for cidr in cf_ips: + try: + networks.add(ipaddress.ip_network(cidr)) + except ValueError: + LOGGER.warning(f"Skipping invalid CIDR from Cloudflare: {cidr}") - if verify_github_ips: - networks += [ipaddress.ip_network(cidr) for cidr in get_github_allowlist()] + if verify_github_ips: + gh_ips = await get_github_allowlist() - ALLOWED_IPS = tuple(networks) # immutable & de-duplicated + for cidr in gh_ips: + try: + networks.add(ipaddress.ip_network(cidr)) + except ValueError: + LOGGER.warning(f"Skipping invalid CIDR from Github: {cidr}") - logger.info(f"IP allowlist initialized successfully. {ALLOWED_IPS}") + if networks: + ALLOWED_IPS = tuple(networks) + LOGGER.info(f"IP allowlist initialized successfully with {len(ALLOWED_IPS)} networks.") yield + except Exception as ex: - logger.error(f"Application failed to start up: {ex}") + LOGGER.error(f"Application failed during lifespan management: {ex}") raise + finally: + if _lifespan_http_client: + await _lifespan_http_client.aclose() + + LOGGER.info("Application shutdown complete.") + FASTAPI_APP: FastAPI = FastAPI(title="webhook-server", lifespan=lifespan) @@ -126,12 +170,9 @@ def healthcheck() -> dict[str, Any]: @FASTAPI_APP.post(APP_URL_ROOT_PATH, dependencies=[Depends(gate_by_allowlist_ips)]) async def process_webhook(request: Request, background_tasks: BackgroundTasks) -> dict[str, Any]: - logger_name: str = "main" - logger = get_logger_with_params(name=logger_name) - payload_body = await request.body() - config = Config(logger=logger) + config = Config(logger=LOGGER) root_config = config.root_data webhook_secret = root_config.get("webhook-secret") @@ -145,20 +186,18 @@ async def process_webhook(request: Request, background_tasks: BackgroundTasks) - log_context = f"[Event: {event_type}][Delivery: {delivery_id}]" try: - hook_data: dict[Any, Any] = await request.json() + hook_data: dict[Any, Any] = json.loads(payload_body) + except Exception as e: - logger.error(f"{log_context} Error parsing JSON body: {e}") + LOGGER.error(f"{log_context} Error parsing JSON body: {e}") raise HTTPException(status_code=400, detail="Invalid JSON payload") - logger = get_logger_with_params(name=logger_name, repository_name=hook_data["repository"]["name"]) + logger = get_logger_with_params(name="main", repository_name=hook_data["repository"]["name"]) async def process_with_error_handling(_api: GithubWebhook, _logger: logging.Logger) -> None: try: await _api.process() - except NoPullRequestError: - return - except Exception as e: _logger.exception(f"{log_context} Error in background task: {e}") @@ -176,10 +215,6 @@ async def process_with_error_handling(_api: GithubWebhook, _logger: logging.Logg logger.exception(f"{log_context} API connection error: {e}") raise HTTPException(status_code=503, detail=f"API Connection Error: {e}") - except NoPullRequestError as e: - logger.debug(f"{log_context} Processing skipped: {e}") - return {"status": "OK", "message": f"Processing skipped: {e}"} - except HTTPException: raise diff --git a/webhook_server/libs/check_run_handler.py b/webhook_server/libs/check_run_handler.py index 3e0a81f5..f032e0a1 100644 --- a/webhook_server/libs/check_run_handler.py +++ b/webhook_server/libs/check_run_handler.py @@ -1,7 +1,8 @@ -import functools +import asyncio from typing import Any from github.CheckRun import CheckRun +from github.PullRequest import PullRequest from webhook_server.utils.constants import ( BUILD_CONTAINER_STR, @@ -26,9 +27,8 @@ def __init__(self, github_webhook: Any): self.logger = self.github_webhook.logger self.log_prefix = self.github_webhook.log_prefix self.repository = self.github_webhook.repository - self.pull_request = getattr(self.github_webhook, "pull_request", None) - def process_pull_request_check_run_webhook_data(self) -> bool: + async def process_pull_request_check_run_webhook_data(self) -> bool: """Return True if check_if_can_be_merged need to run""" _check_run: dict[str, Any] = self.hook_data["check_run"] @@ -52,106 +52,114 @@ def process_pull_request_check_run_webhook_data(self) -> bool: return True - def set_verify_check_queued(self) -> None: - return self.set_check_run_status(check_run=VERIFIED_LABEL_STR, status=QUEUED_STR) + async def set_verify_check_queued(self) -> None: + return await self.set_check_run_status(check_run=VERIFIED_LABEL_STR, status=QUEUED_STR) - def set_verify_check_success(self) -> None: - return self.set_check_run_status(check_run=VERIFIED_LABEL_STR, conclusion=SUCCESS_STR) + async def set_verify_check_success(self) -> None: + return await self.set_check_run_status(check_run=VERIFIED_LABEL_STR, conclusion=SUCCESS_STR) - def set_run_tox_check_queued(self) -> None: + async def set_run_tox_check_queued(self) -> None: if not self.github_webhook.tox: return - return self.set_check_run_status(check_run=TOX_STR, status=QUEUED_STR) + return await self.set_check_run_status(check_run=TOX_STR, status=QUEUED_STR) - def set_run_tox_check_in_progress(self) -> None: - return self.set_check_run_status(check_run=TOX_STR, status=IN_PROGRESS_STR) + async def set_run_tox_check_in_progress(self) -> None: + return await self.set_check_run_status(check_run=TOX_STR, status=IN_PROGRESS_STR) - def set_run_tox_check_failure(self, output: dict[str, Any]) -> None: - return self.set_check_run_status(check_run=TOX_STR, conclusion=FAILURE_STR, output=output) + async def set_run_tox_check_failure(self, output: dict[str, Any]) -> None: + return await self.set_check_run_status(check_run=TOX_STR, conclusion=FAILURE_STR, output=output) - def set_run_tox_check_success(self, output: dict[str, Any]) -> None: - return self.set_check_run_status(check_run=TOX_STR, conclusion=SUCCESS_STR, output=output) + async def set_run_tox_check_success(self, output: dict[str, Any]) -> None: + return await self.set_check_run_status(check_run=TOX_STR, conclusion=SUCCESS_STR, output=output) - def set_run_pre_commit_check_queued(self) -> None: + async def set_run_pre_commit_check_queued(self) -> None: if not self.github_webhook.pre_commit: return - return self.set_check_run_status(check_run=PRE_COMMIT_STR, status=QUEUED_STR) + return await self.set_check_run_status(check_run=PRE_COMMIT_STR, status=QUEUED_STR) - def set_run_pre_commit_check_in_progress(self) -> None: - return self.set_check_run_status(check_run=PRE_COMMIT_STR, status=IN_PROGRESS_STR) + async def set_run_pre_commit_check_in_progress(self) -> None: + return await self.set_check_run_status(check_run=PRE_COMMIT_STR, status=IN_PROGRESS_STR) - def set_run_pre_commit_check_failure(self, output: dict[str, Any] | None = None) -> None: - return self.set_check_run_status(check_run=PRE_COMMIT_STR, conclusion=FAILURE_STR, output=output) + async def set_run_pre_commit_check_failure(self, output: dict[str, Any] | None = None) -> None: + return await self.set_check_run_status(check_run=PRE_COMMIT_STR, conclusion=FAILURE_STR, output=output) - def set_run_pre_commit_check_success(self, output: dict[str, Any] | None = None) -> None: - return self.set_check_run_status(check_run=PRE_COMMIT_STR, conclusion=SUCCESS_STR, output=output) + async def set_run_pre_commit_check_success(self, output: dict[str, Any] | None = None) -> None: + return await self.set_check_run_status(check_run=PRE_COMMIT_STR, conclusion=SUCCESS_STR, output=output) - def set_merge_check_queued(self, output: dict[str, Any] | None = None) -> None: - return self.set_check_run_status(check_run=CAN_BE_MERGED_STR, status=QUEUED_STR, output=output) + async def set_merge_check_queued(self, output: dict[str, Any] | None = None) -> None: + return await self.set_check_run_status(check_run=CAN_BE_MERGED_STR, status=QUEUED_STR, output=output) - def set_merge_check_in_progress(self) -> None: - return self.set_check_run_status(check_run=CAN_BE_MERGED_STR, status=IN_PROGRESS_STR) + async def set_merge_check_in_progress(self) -> None: + return await self.set_check_run_status(check_run=CAN_BE_MERGED_STR, status=IN_PROGRESS_STR) - def set_merge_check_success(self) -> None: - return self.set_check_run_status(check_run=CAN_BE_MERGED_STR, conclusion=SUCCESS_STR) + async def set_merge_check_success(self) -> None: + return await self.set_check_run_status(check_run=CAN_BE_MERGED_STR, conclusion=SUCCESS_STR) - def set_merge_check_failure(self, output: dict[str, Any]) -> None: - return self.set_check_run_status(check_run=CAN_BE_MERGED_STR, conclusion=FAILURE_STR, output=output) + async def set_merge_check_failure(self, output: dict[str, Any]) -> None: + return await self.set_check_run_status(check_run=CAN_BE_MERGED_STR, conclusion=FAILURE_STR, output=output) - def set_container_build_queued(self) -> None: + async def set_container_build_queued(self) -> None: if not self.github_webhook.build_and_push_container: return - return self.set_check_run_status(check_run=BUILD_CONTAINER_STR, status=QUEUED_STR) + return await self.set_check_run_status(check_run=BUILD_CONTAINER_STR, status=QUEUED_STR) - def set_container_build_in_progress(self) -> None: - return self.set_check_run_status(check_run=BUILD_CONTAINER_STR, status=IN_PROGRESS_STR) + async def set_container_build_in_progress(self) -> None: + return await self.set_check_run_status(check_run=BUILD_CONTAINER_STR, status=IN_PROGRESS_STR) - def set_container_build_success(self, output: dict[str, Any]) -> None: - return self.set_check_run_status(check_run=BUILD_CONTAINER_STR, conclusion=SUCCESS_STR, output=output) + async def set_container_build_success(self, output: dict[str, Any]) -> None: + return await self.set_check_run_status(check_run=BUILD_CONTAINER_STR, conclusion=SUCCESS_STR, output=output) - def set_container_build_failure(self, output: dict[str, Any]) -> None: - return self.set_check_run_status(check_run=BUILD_CONTAINER_STR, conclusion=FAILURE_STR, output=output) + async def set_container_build_failure(self, output: dict[str, Any]) -> None: + return await self.set_check_run_status(check_run=BUILD_CONTAINER_STR, conclusion=FAILURE_STR, output=output) - def set_python_module_install_queued(self) -> None: + async def set_python_module_install_queued(self) -> None: if not self.github_webhook.pypi: return - return self.set_check_run_status(check_run=PYTHON_MODULE_INSTALL_STR, status=QUEUED_STR) + return await self.set_check_run_status(check_run=PYTHON_MODULE_INSTALL_STR, status=QUEUED_STR) - def set_python_module_install_in_progress(self) -> None: - return self.set_check_run_status(check_run=PYTHON_MODULE_INSTALL_STR, status=IN_PROGRESS_STR) + async def set_python_module_install_in_progress(self) -> None: + return await self.set_check_run_status(check_run=PYTHON_MODULE_INSTALL_STR, status=IN_PROGRESS_STR) - def set_python_module_install_success(self, output: dict[str, Any]) -> None: - return self.set_check_run_status(check_run=PYTHON_MODULE_INSTALL_STR, conclusion=SUCCESS_STR, output=output) + async def set_python_module_install_success(self, output: dict[str, Any]) -> None: + return await self.set_check_run_status( + check_run=PYTHON_MODULE_INSTALL_STR, conclusion=SUCCESS_STR, output=output + ) - def set_python_module_install_failure(self, output: dict[str, Any]) -> None: - return self.set_check_run_status(check_run=PYTHON_MODULE_INSTALL_STR, conclusion=FAILURE_STR, output=output) + async def set_python_module_install_failure(self, output: dict[str, Any]) -> None: + return await self.set_check_run_status( + check_run=PYTHON_MODULE_INSTALL_STR, conclusion=FAILURE_STR, output=output + ) - def set_conventional_title_queued(self) -> None: - return self.set_check_run_status(check_run=CONVENTIONAL_TITLE_STR, status=QUEUED_STR) + async def set_conventional_title_queued(self) -> None: + return await self.set_check_run_status(check_run=CONVENTIONAL_TITLE_STR, status=QUEUED_STR) - def set_conventional_title_in_progress(self) -> None: - return self.set_check_run_status(check_run=CONVENTIONAL_TITLE_STR, status=IN_PROGRESS_STR) + async def set_conventional_title_in_progress(self) -> None: + return await self.set_check_run_status(check_run=CONVENTIONAL_TITLE_STR, status=IN_PROGRESS_STR) - def set_conventional_title_success(self, output: dict[str, Any]) -> None: - return self.set_check_run_status(check_run=CONVENTIONAL_TITLE_STR, conclusion=SUCCESS_STR, output=output) + async def set_conventional_title_success(self, output: dict[str, Any]) -> None: + return await self.set_check_run_status(check_run=CONVENTIONAL_TITLE_STR, conclusion=SUCCESS_STR, output=output) - def set_conventional_title_failure(self, output: dict[str, Any]) -> None: - return self.set_check_run_status(check_run=CONVENTIONAL_TITLE_STR, conclusion=FAILURE_STR, output=output) + async def set_conventional_title_failure(self, output: dict[str, Any]) -> None: + return await self.set_check_run_status(check_run=CONVENTIONAL_TITLE_STR, conclusion=FAILURE_STR, output=output) - def set_cherry_pick_in_progress(self) -> None: - return self.set_check_run_status(check_run=CHERRY_PICKED_LABEL_PREFIX, status=IN_PROGRESS_STR) + async def set_cherry_pick_in_progress(self) -> None: + return await self.set_check_run_status(check_run=CHERRY_PICKED_LABEL_PREFIX, status=IN_PROGRESS_STR) - def set_cherry_pick_success(self, output: dict[str, Any]) -> None: - return self.set_check_run_status(check_run=CHERRY_PICKED_LABEL_PREFIX, conclusion=SUCCESS_STR, output=output) + async def set_cherry_pick_success(self, output: dict[str, Any]) -> None: + return await self.set_check_run_status( + check_run=CHERRY_PICKED_LABEL_PREFIX, conclusion=SUCCESS_STR, output=output + ) - def set_cherry_pick_failure(self, output: dict[str, Any]) -> None: - return self.set_check_run_status(check_run=CHERRY_PICKED_LABEL_PREFIX, conclusion=FAILURE_STR, output=output) + async def set_cherry_pick_failure(self, output: dict[str, Any]) -> None: + return await self.set_check_run_status( + check_run=CHERRY_PICKED_LABEL_PREFIX, conclusion=FAILURE_STR, output=output + ) - def set_check_run_status( + async def set_check_run_status( self, check_run: str, status: str = "", @@ -172,7 +180,7 @@ def set_check_run_status( msg: str = f"{self.log_prefix} check run {check_run} status: {status or conclusion}" try: - self.github_webhook.repository_by_github_app.create_check_run(**kwargs) + await asyncio.to_thread(self.github_webhook.repository_by_github_app.create_check_run, **kwargs) if conclusion in (SUCCESS_STR, IN_PROGRESS_STR): self.logger.success(msg) return @@ -180,7 +188,7 @@ def set_check_run_status( except Exception as ex: self.logger.debug(f"{self.log_prefix} Failed to set {check_run} check to {status or conclusion}, {ex}") kwargs["conclusion"] = FAILURE_STR - self.github_webhook.repository_by_github_app.create_check_run(**kwargs) + await asyncio.to_thread(self.github_webhook.repository_by_github_app.create_check_run, **kwargs) def get_check_run_text(self, err: str, out: str) -> str: total_len: int = len(err) + len(out) @@ -206,26 +214,35 @@ def get_check_run_text(self, err: str, out: str) -> str: return _output - def is_check_run_in_progress(self, check_run: str) -> bool: + async def is_check_run_in_progress(self, check_run: str) -> bool: if self.github_webhook.last_commit: - for run in self.github_webhook.last_commit.get_check_runs(): + for run in await asyncio.to_thread(self.github_webhook.last_commit.get_check_runs): if run.name == check_run and run.status == IN_PROGRESS_STR: return True return False - def required_check_failed(self, last_commit_check_runs: list[CheckRun], check_runs_in_progress: list[str]) -> str: + async def required_check_failed_or_no_status( + self, pull_request: PullRequest, last_commit_check_runs: list[CheckRun], check_runs_in_progress: list[str] + ) -> str: failed_check_runs = [] + no_status_check_runs = [] + for check_run in last_commit_check_runs: - self.logger.debug(f"{self.log_prefix} Check if {check_run.name} failed.") + self.logger.debug(f"{self.log_prefix} Check if {check_run.name} failed or do not have status.") if ( check_run.name == CAN_BE_MERGED_STR or check_run.conclusion == SUCCESS_STR - or check_run.conclusion == QUEUED_STR - or check_run.name not in self.all_required_status_checks + or check_run.name not in await self.all_required_status_checks(pull_request=pull_request) ): continue - failed_check_runs.append(check_run.name) + if check_run.conclusion is None: + no_status_check_runs.append(check_run.name) + + else: + failed_check_runs.append(check_run.name) + + msg = "" if failed_check_runs: exclude_in_progress = [ @@ -233,17 +250,17 @@ def required_check_failed(self, last_commit_check_runs: list[CheckRun], check_ru for failed_check_run in failed_check_runs if failed_check_run not in check_runs_in_progress ] - return f"Some check runs failed: {', '.join(exclude_in_progress)}\n" + msg += f"Some check runs failed: {', '.join(exclude_in_progress)}\n" - return "" + if no_status_check_runs: + msg += f"Some check runs not started: {', '.join(no_status_check_runs)}\n" - @functools.cached_property - def all_required_status_checks(self) -> list[str]: - if self.pull_request and not hasattr(self, "pull_request_branch"): - self.pull_request_branch = self.pull_request.base.ref + return msg + async def all_required_status_checks(self, pull_request: PullRequest) -> list[str]: all_required_status_checks: list[str] = [] - branch_required_status_checks = self.get_branch_required_status_checks() + branch_required_status_checks = await self.get_branch_required_status_checks(pull_request=pull_request) + if self.github_webhook.tox: all_required_status_checks.append(TOX_STR) @@ -263,26 +280,28 @@ def all_required_status_checks(self) -> list[str]: self.logger.debug(f"{self.log_prefix} All required status checks: {_all_required_status_checks}") return _all_required_status_checks - def get_branch_required_status_checks(self) -> list[str]: + async def get_branch_required_status_checks(self, pull_request: PullRequest) -> list[str]: if self.repository.private: self.logger.info( f"{self.log_prefix} Repository is private, skipping getting branch protection required status checks" ) return [] - pull_request_branch = self.repository.get_branch(self.pull_request_branch) - branch_protection = pull_request_branch.get_protection() + pull_request_branch = await asyncio.to_thread(self.repository.get_branch, pull_request.base.ref) + branch_protection = await asyncio.to_thread(pull_request_branch.get_protection) return branch_protection.required_status_checks.contexts - def required_check_in_progress(self, last_commit_check_runs: list[CheckRun]) -> tuple[str, list[str]]: - last_commit_check_runs = list(self.github_webhook.last_commit.get_check_runs()) + async def required_check_in_progress( + self, pull_request: PullRequest, last_commit_check_runs: list[CheckRun] + ) -> tuple[str, list[str]]: self.logger.debug(f"{self.log_prefix} Check if any required check runs in progress.") + check_runs_in_progress = [ check_run.name for check_run in last_commit_check_runs if check_run.status == IN_PROGRESS_STR and check_run.name != CAN_BE_MERGED_STR - and check_run.name in self.all_required_status_checks + and check_run.name in await self.all_required_status_checks(pull_request=pull_request) ] if check_runs_in_progress: self.logger.debug( diff --git a/webhook_server/libs/config.py b/webhook_server/libs/config.py index d9652d84..2a64c74c 100644 --- a/webhook_server/libs/config.py +++ b/webhook_server/libs/config.py @@ -47,7 +47,7 @@ def repository_local_data(self, github_api: github.Github, repository_full_name: from webhook_server.utils.helpers import get_github_repo_api try: - repo = get_github_repo_api(github_api=github_api, repository=repository_full_name) + repo = get_github_repo_api(github_app_api=github_api, repository=repository_full_name) try: _path = repo.get_contents(".github-webhook-server.yaml") except UnknownObjectException: diff --git a/webhook_server/libs/exceptions.py b/webhook_server/libs/exceptions.py index 2442a7dd..32b32745 100644 --- a/webhook_server/libs/exceptions.py +++ b/webhook_server/libs/exceptions.py @@ -1,7 +1,3 @@ -class NoPullRequestError(Exception): - pass - - class RepositoryNotFoundError(Exception): pass @@ -10,3 +6,7 @@ class ProcessGithubWebhookError(Exception): def __init__(self, err: dict[str, str]): self.err = err super().__init__(str(err)) + + +class NoApiTokenError(Exception): + pass diff --git a/webhook_server/libs/github_api.py b/webhook_server/libs/github_api.py index 5605fd5e..8176fbc6 100644 --- a/webhook_server/libs/github_api.py +++ b/webhook_server/libs/github_api.py @@ -1,18 +1,15 @@ from __future__ import annotations +import asyncio import contextlib import json import logging import os import random -from pathlib import Path from typing import Any import requests -import yaml -from fastapi.exceptions import HTTPException from github import GithubException -from github.Branch import Branch from github.Commit import Commit from github.PullRequest import PullRequest from starlette.datastructures import Headers @@ -20,8 +17,9 @@ from webhook_server.libs.check_run_handler import CheckRunHandler from webhook_server.libs.config import Config -from webhook_server.libs.exceptions import NoPullRequestError, RepositoryNotFoundError +from webhook_server.libs.exceptions import RepositoryNotFoundError from webhook_server.libs.issue_comment_handler import IssueCommentHandler +from webhook_server.libs.owners_files_handler import OwnersFileHandler from webhook_server.libs.pull_request_handler import PullRequestHandler from webhook_server.libs.pull_request_review_handler import PullRequestReviewHandler from webhook_server.libs.push_handler import PushHandler @@ -46,18 +44,14 @@ class GithubWebhook: def __init__(self, hook_data: dict[Any, Any], headers: Headers, logger: logging.Logger) -> None: + logger.name = "GithubWebhook" self.logger = logger - self.logger.name = "GithubWebhook" self.hook_data = hook_data - self.headers = headers self.repository_name: str = hook_data["repository"]["name"] self.repository_full_name: str = hook_data["repository"]["full_name"] self.parent_committer: str = "" - self.issue_title: str = "" - self.x_github_delivery: str = self.headers.get("X-GitHub-Delivery", "") - self.github_event: str = self.headers["X-GitHub-Event"] - self.owners_content: dict[str, Any] = {} - + self.x_github_delivery: str = headers.get("X-GitHub-Delivery", "") + self.github_event: str = headers["X-GitHub-Event"] self.config = Config(repository=self.repository_name, logger=self.logger) if not self.config.repository: @@ -65,15 +59,15 @@ def __init__(self, hook_data: dict[Any, Any], headers: Headers, logger: logging. # Get config without .github-webhook-server.yaml data self._repo_data_from_config(repository_config={}) - self.github_api, self.token, self.api_user = get_api_with_highest_rate_limit( + github_api, self.token, self.api_user = get_api_with_highest_rate_limit( config=self.config, repository_name=self.repository_name ) - if self.github_api and self.token: - self.repository = get_github_repo_api(github_api=self.github_api, repository=self.repository_full_name) + if github_api and self.token: + self.repository = get_github_repo_api(github_app_api=github_api, repository=self.repository_full_name) # Once we have a repository, we can get the config from .github-webhook-server.yaml local_repository_config = self.config.repository_local_data( - github_api=self.github_api, repository_full_name=self.repository_full_name + github_api=github_api, repository_full_name=self.repository_full_name ) # Call _repo_data_from_config() again to update self args from .github-webhook-server.yaml self._repo_data_from_config(repository_config=local_repository_config) @@ -84,11 +78,9 @@ def __init__(self, hook_data: dict[Any, Any], headers: Headers, logger: logging. self.log_prefix = self.prepare_log_prefix() - self.github_app_api = get_repository_github_app_api( - config_=self.config, repository_name=self.repository_full_name - ) + github_app_api = get_repository_github_app_api(config_=self.config, repository_name=self.repository_full_name) - if not self.github_app_api: + if not github_app_api: self.logger.error( ( f"{self.log_prefix} not found by manage-repositories-app, " @@ -98,7 +90,7 @@ def __init__(self, hook_data: dict[Any, Any], headers: Headers, logger: logging. return self.repository_by_github_app = get_github_repo_api( - github_api=self.github_app_api, repository=self.repository_full_name + github_app_api=github_app_api, repository=self.repository_full_name ) if not (self.repository or self.repository_by_github_app): @@ -114,55 +106,74 @@ def __init__(self, hook_data: dict[Any, Any], headers: Headers, logger: logging. ) async def process(self) -> Any: + event_log: str = f"Event type: {self.github_event}. event ID: {self.x_github_delivery}" + if self.github_event == "ping": + self.logger.debug(f"{self.log_prefix} {event_log}") return {"status": requests.codes.ok, "message": "pong"} - event_log: str = f"Event type: {self.github_event}. event ID: {self.x_github_delivery}" + if self.github_event == "push": + self.logger.debug(f"{self.log_prefix} {event_log}") + return await PushHandler(github_webhook=self).process_push_webhook_data() - try: - self.pull_request = self._get_pull_request() - self.log_prefix = self.prepare_log_prefix(pull_request=self.pull_request) + if pull_request := await self.get_pull_request(): + self.log_prefix = self.prepare_log_prefix(pull_request=pull_request) self.logger.debug(f"{self.log_prefix} {event_log}") - self.last_commit = self._get_last_commit() - self.parent_committer = self.pull_request.user.login + + if pull_request.draft: + self.logger.debug(f"{self.log_prefix} Pull request is draft, doing nothing") + return None + + self.last_commit = await self._get_last_commit(pull_request=pull_request) + self.parent_committer = pull_request.user.login self.last_committer = getattr(self.last_commit.committer, "login", self.parent_committer) - self.changed_files = self.list_changed_files() - self.pull_request_branch = self.pull_request.base.ref - self.all_repository_approvers_and_reviewers = self.get_all_repository_approvers_and_reviewers() - self.all_repository_approvers = self.get_all_repository_approvers() - self.all_repository_reviewers = self.get_all_repository_reviewers() - self.all_pull_request_approvers = self.get_all_pull_request_approvers() - self.all_pull_request_reviewers = self.get_all_pull_request_reviewers() if self.github_event == "issue_comment": - return IssueCommentHandler(github_webhook=self).process_comment_webhook_data() + owners_file_handler = OwnersFileHandler(github_webhook=self) + owners_file_handler = await owners_file_handler.initialize(pull_request=pull_request) - if self.github_event == "pull_request": - return PullRequestHandler(github_webhook=self).process_pull_request_webhook_data() + return await IssueCommentHandler( + github_webhook=self, owners_file_handler=owners_file_handler + ).process_comment_webhook_data(pull_request=pull_request) - if self.github_event == "pull_request_review": - return PullRequestReviewHandler(github_webhook=self).process_pull_request_review_webhook_data() + elif self.github_event == "pull_request": + owners_file_handler = OwnersFileHandler(github_webhook=self) + owners_file_handler = await owners_file_handler.initialize(pull_request=pull_request) - if self.github_event == "check_run": - if CheckRunHandler(github_webhook=self).process_pull_request_check_run_webhook_data(): - PullRequestHandler(github_webhook=self).check_if_can_be_merged() + return await PullRequestHandler( + github_webhook=self, owners_file_handler=owners_file_handler + ).process_pull_request_webhook_data(pull_request=pull_request) - except NoPullRequestError: - self.logger.debug(f"{self.log_prefix} {event_log}. [No pull request found in hook data]") + elif self.github_event == "pull_request_review": + owners_file_handler = OwnersFileHandler(github_webhook=self) + owners_file_handler = await owners_file_handler.initialize(pull_request=pull_request) - if self.github_event == "push": - return PushHandler(github_webhook=self).process_push_webhook_data() + return await PullRequestReviewHandler( + github_webhook=self, owners_file_handler=owners_file_handler + ).process_pull_request_review_webhook_data( + pull_request=pull_request, + ) - raise + 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) - except Exception as e: - self.logger.error(f"{self.log_prefix} {event_log}. Exception: {e}") - raise HTTPException(status_code=404, detail=str(e)) + return await PullRequestHandler( + github_webhook=self, owners_file_handler=owners_file_handler + ).check_if_can_be_merged(pull_request=pull_request) @property def add_api_users_to_auto_verified_and_merged_users(self) -> None: apis_and_tokens = get_apis_and_tokes_from_config(config=self.config) - self.auto_verified_and_merged_users.extend([_api[0].get_user().login for _api in apis_and_tokens]) + for _api, _ in apis_and_tokens: + if _api.rate_limiting[-1] == 60: + self.logger.warning( + f"{self.log_prefix} API has rate limit set to 60 which indicates an invalid token, skipping" + ) + continue + + self.auto_verified_and_merged_users.append(_api.get_user().login) def _get_reposiroty_color_for_log_prefix(self) -> str: def _get_random_color(_colors: list[str], _json: dict[str, str]) -> str: @@ -217,9 +228,9 @@ def prepare_log_prefix(self, pull_request: PullRequest | None = None) -> str: _repository_color = self._get_reposiroty_color_for_log_prefix() return ( - f"{_repository_color}[{self.github_event}][{self.x_github_delivery}][{self.api_user}][PR {pull_request.number}]:" + f"{_repository_color} [{self.github_event}][{self.x_github_delivery}][{self.api_user}][PR {pull_request.number}]:" if pull_request - else f"{_repository_color}[{self.github_event}][{self.x_github_delivery}][{self.api_user}]:" + else f"{_repository_color} [{self.github_event}][{self.x_github_delivery}][{self.api_user}]:" ) def _repo_data_from_config(self, repository_config: dict[str, Any]) -> None: @@ -265,52 +276,36 @@ def _repo_data_from_config(self, repository_config: dict[str, Any]) -> None: value="minimum-lgtm", return_on_none=0, extra_dict=repository_config ) - def _get_pull_request(self, number: int | None = None) -> PullRequest: + async def get_pull_request(self, number: int | None = None) -> PullRequest | None: if number: - return self.repository.get_pull(number) + return await asyncio.to_thread(self.repository.get_pull, number) for _number in extract_key_from_dict(key="number", _dict=self.hook_data): try: - return self.repository.get_pull(_number) + return await asyncio.to_thread(self.repository.get_pull, _number) except GithubException: continue commit: dict[str, Any] = self.hook_data.get("commit", {}) if commit: - commit_obj = self.repository.get_commit(commit["sha"]) + commit_obj = await asyncio.to_thread(self.repository.get_commit, commit["sha"]) with contextlib.suppress(Exception): - return commit_obj.get_pulls()[0] + _pulls = await asyncio.to_thread(commit_obj.get_pulls) + return _pulls[0] if self.github_event == "check_run": - for _pull_request in self.repository.get_pulls(state="open"): + for _pull_request in await asyncio.to_thread(self.repository.get_pulls, state="open"): if _pull_request.head.sha == self.hook_data["check_run"]["head_sha"]: self.logger.debug( f"{self.log_prefix} Found pull request {_pull_request.title} [{_pull_request.number}] for check run {self.hook_data['check_run']['name']}" ) return _pull_request - raise NoPullRequestError(f"{self.log_prefix} No issue or pull_request found in hook data") - - def _get_last_commit(self) -> Commit: - return list(self.pull_request.get_commits())[-1] + return None - def is_branch_exists(self, branch: str) -> Branch: - return self.repository.get_branch(branch) - - @property - def root_reviewers(self) -> list[str]: - _reviewers = self.all_repository_approvers_and_reviewers.get(".", {}).get("reviewers", []) - self.logger.debug(f"{self.log_prefix} ROOT Reviewers: {_reviewers}") - return _reviewers - - @property - def root_approvers(self) -> list[str]: - _approvers = self.all_repository_approvers_and_reviewers.get(".", {}).get("approvers", []) - self.logger.debug(f"{self.log_prefix} ROOT Approvers: {_approvers}") - return _approvers - - def list_changed_files(self) -> list[str]: - return [_file.filename for _file in self.pull_request.get_files()] + async def _get_last_commit(self, pull_request: PullRequest) -> Commit: + _commits = await asyncio.to_thread(pull_request.get_commits) + return list(_commits)[-1] @staticmethod def _comment_with_details(title: str, body: str) -> str: @@ -321,24 +316,29 @@ def _comment_with_details(title: str, body: str) -> str: """ - def _container_repository_and_tag(self, is_merged: bool = False, tag: str = "") -> str: + def container_repository_and_tag( + self, is_merged: bool = False, tag: str = "", pull_request: PullRequest | None = None + ) -> str | None: if not tag: + if not pull_request: + return None + if is_merged: + pull_request_branch = pull_request.base.ref tag = ( - self.pull_request_branch - if self.pull_request_branch not in (OTHER_MAIN_BRANCH, "main") + pull_request_branch + if pull_request_branch not in (OTHER_MAIN_BRANCH, "main") else self.container_tag ) else: - if self.pull_request: - tag = f"pr-{self.pull_request.number}" + tag = f"pr-{pull_request.number}" if tag: self.logger.debug(f"{self.log_prefix} container tag is: {tag}") return f"{self.container_repository}:{tag}" self.logger.error(f"{self.log_prefix} container tag not found") - return f"{self.container_repository}:webhook-server-tag-not-found" + return None def send_slack_message(self, message: str, webhook_url: str) -> None: slack_data: dict[str, str] = {"text": message} @@ -372,150 +372,3 @@ def _current_pull_request_supported_retest(self) -> list[str]: if self.conventional_title: current_pull_request_supported_retest.append(CONVENTIONAL_TITLE_STR) return current_pull_request_supported_retest - - def get_all_repository_approvers_and_reviewers(self) -> dict[str, dict[str, Any]]: - # Dictionary mapping OWNERS file paths to their approvers and reviewers - _owners: dict[str, dict[str, Any]] = {} - - max_owners_files = 1000 # Configurable limit - owners_count = 0 - - self.logger.debug(f"{self.log_prefix} Get git tree") - tree = self.repository.get_git_tree(self.pull_request_branch, recursive=True) - for element in tree.tree: - if element.type == "blob" and element.path.endswith("OWNERS"): - owners_count += 1 - if owners_count > max_owners_files: - self.logger.error(f"{self.log_prefix} Too many OWNERS files (>{max_owners_files})") - break - - content_path = element.path - self.logger.debug(f"{self.log_prefix} Get OWNERS file from {content_path}") - _path = self.repository.get_contents(content_path, self.pull_request_branch) - if isinstance(_path, list): - _path = _path[0] - - try: - content = yaml.safe_load(_path.decoded_content) - if self._validate_owners_content(content, content_path): - parent_path = str(Path(content_path).parent) - if not parent_path: - parent_path = "." - _owners[parent_path] = content - - except yaml.YAMLError as exp: - self.logger.error(f"{self.log_prefix} Invalid OWNERS file {content_path}: {exp}") - continue - - return _owners - - def get_all_repository_approvers(self) -> list[str]: - _approvers: list[str] = [] - - for value in self.all_repository_approvers_and_reviewers.values(): - for key, val in value.items(): - if key == "approvers": - _approvers.extend(val) - - return _approvers - - def get_all_repository_reviewers(self) -> list[str]: - _reviewers: list[str] = [] - - for value in self.all_repository_approvers_and_reviewers.values(): - for key, val in value.items(): - if key == "reviewers": - _reviewers.extend(val) - - return _reviewers - - def get_all_pull_request_approvers(self) -> list[str]: - _approvers: list[str] = [] - for list_of_approvers in self.owners_data_for_changed_files().values(): - for _approver in list_of_approvers.get("approvers", []): - _approvers.append(_approver) - - _approvers.sort() - return _approvers - - def get_all_pull_request_reviewers(self) -> list[str]: - _reviewers: list[str] = [] - for list_of_reviewers in self.owners_data_for_changed_files().values(): - for _reviewer in list_of_reviewers.get("reviewers", []): - _reviewers.append(_reviewer) - - _reviewers.sort() - return _reviewers - - def owners_data_for_changed_files(self) -> dict[str, dict[str, Any]]: - data: dict[str, dict[str, Any]] = {} - - changed_folders = {Path(cf).parent for cf in self.changed_files} - - changed_folder_match: list[Path] = [] - - require_root_approvers: bool | None = None - - for owners_dir, owners_data in self.all_repository_approvers_and_reviewers.items(): - if owners_dir == ".": - continue - - _owners_dir = Path(owners_dir) - - for changed_folder in changed_folders: - if changed_folder == _owners_dir or _owners_dir in changed_folder.parents: - data[owners_dir] = owners_data - changed_folder_match.append(_owners_dir) - if require_root_approvers is None: - require_root_approvers = owners_data.get("root-approvers", True) - - if require_root_approvers or require_root_approvers is None: - self.logger.debug(f"{self.log_prefix} require root_approvers") - data["."] = self.all_repository_approvers_and_reviewers.get(".", {}) - - else: - for _folder in changed_folders: - for _changed_path in changed_folder_match: - if _folder == _changed_path or _changed_path in _folder.parents: - continue - else: - data["."] = self.all_repository_approvers_and_reviewers.get(".", {}) - break - - self.logger.debug(f"{self.log_prefix} Owners data for current pull request: {yaml.dump(data)}") - return data - - def _validate_owners_content(self, content: Any, path: str) -> bool: - """Validate OWNERS file content structure.""" - try: - if not isinstance(content, dict): - raise ValueError("OWNERS file must contain a dictionary") - - for key in ["approvers", "reviewers"]: - if key in content: - if not isinstance(content[key], list): - raise ValueError(f"{key} must be a list") - - if not all(isinstance(_elm, str) for _elm in content[key]): - raise ValueError(f"All {key} must be strings") - - return True - - except ValueError as e: - self.logger.error(f"{self.log_prefix} Invalid OWNERS file {path}: {e}") - return False - - def assign_reviewers(self) -> None: - self.logger.info(f"{self.log_prefix} Assign reviewers") - - _to_add: list[str] = list(set(self.all_pull_request_reviewers)) - self.logger.debug(f"{self.log_prefix} Reviewers to add: {', '.join(_to_add)}") - - for reviewer in _to_add: - if reviewer != self.pull_request.user.login: - self.logger.debug(f"{self.log_prefix} Adding reviewer {reviewer}") - try: - self.pull_request.create_review_request([reviewer]) - except GithubException as ex: - self.logger.debug(f"{self.log_prefix} Failed to add reviewer {reviewer}. {ex}") - self.pull_request.create_issue_comment(f"{reviewer} can not be added as reviewer. {ex}") diff --git a/webhook_server/libs/issue_comment_handler.py b/webhook_server/libs/issue_comment_handler.py index d980725d..21355871 100644 --- a/webhook_server/libs/issue_comment_handler.py +++ b/webhook_server/libs/issue_comment_handler.py @@ -1,10 +1,13 @@ from __future__ import annotations -from concurrent.futures import Future, ThreadPoolExecutor, as_completed +import asyncio from typing import Any, Callable +from github.PullRequest import PullRequest + from webhook_server.libs.check_run_handler import CheckRunHandler from webhook_server.libs.labels_handler import LabelsHandler +from webhook_server.libs.owners_files_handler import OwnersFileHandler from webhook_server.libs.pull_request_handler import PullRequestHandler from webhook_server.libs.runner_handler import RunnerHandler from webhook_server.utils.constants import ( @@ -29,20 +32,29 @@ class IssueCommentHandler: - def __init__(self, github_webhook: Any): + def __init__(self, github_webhook: Any, owners_file_handler: OwnersFileHandler): 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 - self.pull_request = self.github_webhook.pull_request - self.labels_handler = LabelsHandler(github_webhook=self.github_webhook) + 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.pull_request_handler = PullRequestHandler(github_webhook=self.github_webhook) - self.runner_handler = RunnerHandler(github_webhook=self.github_webhook) + self.pull_request_handler = PullRequestHandler( + 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 + ) + + async def process_comment_webhook_data(self, pull_request: PullRequest) -> None: + comment_action = self.hook_data["action"] - def process_comment_webhook_data(self) -> None: - if comment_action := self.hook_data["action"] in ("edited", "deleted"): + if comment_action in ("edited", "deleted"): self.logger.debug(f"{self.log_prefix} Not processing comment. action is {comment_action}") return @@ -51,22 +63,23 @@ def process_comment_webhook_data(self) -> None: body: str = self.hook_data["comment"]["body"] if self.github_webhook.issue_url_for_welcome_msg in body: - self.logger.debug( - f"{self.log_prefix} Welcome message found in issue {self.pull_request.title}. Not processing" - ) + self.logger.debug(f"{self.log_prefix} Welcome message found in issue {pull_request.title}. Not processing") return _user_commands: list[str] = [_cmd.strip("/") for _cmd in body.strip().splitlines() if _cmd.startswith("/")] user_login: str = self.hook_data["sender"]["login"] for user_command in _user_commands: - self.user_commands( + await self.user_commands( + pull_request=pull_request, command=user_command, reviewed_user=user_login, issue_comment_id=self.hook_data["comment"]["id"], ) - def user_commands(self, command: str, reviewed_user: str, issue_comment_id: int) -> None: + async def user_commands( + self, pull_request: PullRequest, command: str, reviewed_user: str, issue_comment_id: int + ) -> None: available_commands: list[str] = [ COMMAND_RETEST_STR, COMMAND_CHERRY_PICK_STR, @@ -96,148 +109,164 @@ def user_commands(self, command: str, reviewed_user: str, issue_comment_id: int) missing_command_arg_comment_msg: str = f"{_command} requires an argument" error_msg: str = f"{self.log_prefix} {missing_command_arg_comment_msg}" self.logger.debug(error_msg) - self.pull_request.create_issue_comment(missing_command_arg_comment_msg) + await asyncio.to_thread(pull_request.create_issue_comment, body=missing_command_arg_comment_msg) return - self.create_comment_reaction(issue_comment_id=issue_comment_id, reaction=REACTIONS.ok) + await self.create_comment_reaction( + pull_request=pull_request, issue_comment_id=issue_comment_id, reaction=REACTIONS.ok + ) if _command == COMMAND_ASSIGN_REVIEWER_STR: - self._add_reviewer_by_user_comment(reviewer=_args) + await self._add_reviewer_by_user_comment(pull_request=pull_request, reviewer=_args) elif _command == COMMAND_ASSIGN_REVIEWERS_STR: - self.github_webhook.assign_reviewers() + await self.owners_file_handler.assign_reviewers(pull_request=pull_request) elif _command == COMMAND_CHECK_CAN_MERGE_STR: - self.pull_request_handler.check_if_can_be_merged() + await self.pull_request_handler.check_if_can_be_merged(pull_request=pull_request) elif _command == COMMAND_CHERRY_PICK_STR: - self.process_cherry_pick_command(command_args=_args, reviewed_user=reviewed_user) + await self.process_cherry_pick_command( + pull_request=pull_request, command_args=_args, reviewed_user=reviewed_user + ) elif _command == COMMAND_RETEST_STR: - self.process_retest_command(command_args=_args, reviewed_user=reviewed_user) + await self.process_retest_command( + pull_request=pull_request, command_args=_args, reviewed_user=reviewed_user + ) elif _command == BUILD_AND_PUSH_CONTAINER_STR: if self.github_webhook.build_and_push_container: - self.runner_handler._run_build_container( - push=True, set_check=False, command_args=_args, reviewed_user=reviewed_user + await self.runner_handler.run_build_container( + push=True, + set_check=False, + command_args=_args, + reviewed_user=reviewed_user, + pull_request=pull_request, ) else: msg = f"No {BUILD_AND_PUSH_CONTAINER_STR} configured for this repository" error_msg = f"{self.log_prefix} {msg}" self.logger.debug(error_msg) - self.pull_request.create_issue_comment(msg) + await asyncio.to_thread(pull_request.create_issue_comment, msg) elif _command == WIP_STR: wip_for_title: str = f"{WIP_STR.upper()}:" if remove: - self.labels_handler._remove_label(label=WIP_STR) - self.pull_request.edit(title=self.pull_request.title.replace(wip_for_title, "")) + await self.labels_handler._remove_label(pull_request=pull_request, label=WIP_STR) + await asyncio.to_thread(pull_request.edit, title=pull_request.title.replace(wip_for_title, "")) else: - self.labels_handler._add_label(label=WIP_STR) - self.pull_request.edit(title=f"{wip_for_title} {self.pull_request.title}") + await self.labels_handler._add_label(pull_request=pull_request, label=WIP_STR) + await asyncio.to_thread(pull_request.edit, title=f"{wip_for_title} {pull_request.title}") elif _command == HOLD_LABEL_STR: - if reviewed_user not in self.github_webhook.all_pull_request_approvers: - self.pull_request.create_issue_comment( - f"{reviewed_user} is not part of the approver, only approvers can mark pull request with hold" + if reviewed_user not in self.owners_file_handler.all_pull_request_approvers: + await asyncio.to_thread( + pull_request.create_issue_comment, + f"{reviewed_user} is not part of the approver, only approvers can mark pull request with hold", ) else: if remove: - self.labels_handler._remove_label(label=HOLD_LABEL_STR) + await self.labels_handler._remove_label(pull_request=pull_request, label=HOLD_LABEL_STR) else: - self.labels_handler._add_label(label=HOLD_LABEL_STR) + await self.labels_handler._add_label(pull_request=pull_request, label=HOLD_LABEL_STR) - self.pull_request_handler.check_if_can_be_merged() + await self.pull_request_handler.check_if_can_be_merged(pull_request=pull_request) elif _command == VERIFIED_LABEL_STR: if remove: - self.labels_handler._remove_label(label=VERIFIED_LABEL_STR) - self.check_run_handler.set_verify_check_queued() + await self.labels_handler._remove_label(pull_request=pull_request, label=VERIFIED_LABEL_STR) + await self.check_run_handler.set_verify_check_queued() else: - self.labels_handler._add_label(label=VERIFIED_LABEL_STR) - self.check_run_handler.set_verify_check_success() + await self.labels_handler._add_label(pull_request=pull_request, label=VERIFIED_LABEL_STR) + await self.check_run_handler.set_verify_check_success() else: - self.labels_handler.label_by_user_comment( + await self.labels_handler.label_by_user_comment( + pull_request=pull_request, user_requested_label=_command, remove=remove, reviewed_user=reviewed_user, ) - def create_comment_reaction(self, issue_comment_id: int, reaction: str) -> None: - _comment = self.pull_request.get_issue_comment(issue_comment_id) - _comment.create_reaction(reaction) + async def create_comment_reaction(self, pull_request: PullRequest, issue_comment_id: int, reaction: str) -> None: + _comment = await asyncio.to_thread(pull_request.get_issue_comment, issue_comment_id) + await asyncio.to_thread(_comment.create_reaction, reaction) - def _add_reviewer_by_user_comment(self, reviewer: str) -> None: + async def _add_reviewer_by_user_comment(self, pull_request: PullRequest, reviewer: str) -> None: reviewer = reviewer.strip("@") self.logger.info(f"{self.log_prefix} Adding reviewer {reviewer} by user comment") - for contributer in self.repository.get_contributors(): + for contributer in await asyncio.to_thread(self.repository.get_contributors): if contributer.login == reviewer: - self.pull_request.create_review_request([reviewer]) + await asyncio.to_thread(pull_request.create_review_request, [reviewer]) return _err = f"not adding reviewer {reviewer} by user comment, {reviewer} is not part of contributers" self.logger.debug(f"{self.log_prefix} {_err}") - self.pull_request.create_issue_comment(_err) + await asyncio.to_thread(pull_request.create_issue_comment, _err) - def process_cherry_pick_command(self, command_args: str, reviewed_user: str) -> None: + async def process_cherry_pick_command( + self, pull_request: PullRequest, command_args: str, reviewed_user: str + ) -> None: _target_branches: list[str] = command_args.split() _exits_target_branches: set[str] = set() _non_exits_target_branches_msg: str = "" for _target_branch in _target_branches: try: - self.repository.get_branch(_target_branch) + await asyncio.to_thread(self.repository.get_branch, _target_branch) + _exits_target_branches.add(_target_branch) except Exception: _non_exits_target_branches_msg += f"Target branch `{_target_branch}` does not exist\n" - _exits_target_branches.add(_target_branch) - if _non_exits_target_branches_msg: self.logger.info(f"{self.log_prefix} {_non_exits_target_branches_msg}") - self.pull_request.create_issue_comment(_non_exits_target_branches_msg) + await asyncio.to_thread(pull_request.create_issue_comment, _non_exits_target_branches_msg) if _exits_target_branches: - if not self.pull_request.is_merged(): + if not await asyncio.to_thread(pull_request.is_merged): cp_labels: list[str] = [ f"{CHERRY_PICK_LABEL_PREFIX}{_target_branch}" for _target_branch in _exits_target_branches ] info_msg: str = f""" -Cherry-pick requested for PR: `{self.pull_request.title}` by user `{reviewed_user}` +Cherry-pick requested for PR: `{pull_request.title}` by user `{reviewed_user}` Adding label/s `{" ".join([_cp_label for _cp_label in cp_labels])}` for automatic cheery-pick once the PR is merged """ self.logger.info(f"{self.log_prefix} {info_msg}") - self.pull_request.create_issue_comment(info_msg) + await asyncio.to_thread(pull_request.create_issue_comment, info_msg) for _cp_label in cp_labels: - self.labels_handler._add_label(label=_cp_label) + await self.labels_handler._add_label(pull_request=pull_request, label=_cp_label) else: for _exits_target_branch in _exits_target_branches: - self.runner_handler.cherry_pick( + await self.runner_handler.cherry_pick( + pull_request=pull_request, target_branch=_exits_target_branch, reviewed_user=reviewed_user, ) - def process_retest_command(self, command_args: str, reviewed_user: str) -> None: - if not self.runner_handler._is_user_valid_to_run_commands(reviewed_user=reviewed_user): + async def process_retest_command(self, pull_request: PullRequest, command_args: str, reviewed_user: str) -> None: + if not await self.owners_file_handler.is_user_valid_to_run_commands( + pull_request=pull_request, reviewed_user=reviewed_user + ): return _target_tests: list[str] = command_args.split() _not_supported_retests: list[str] = [] _supported_retests: list[str] = [] _retests_to_func_map: dict[str, Callable] = { - TOX_STR: self.runner_handler._run_tox, - PRE_COMMIT_STR: self.runner_handler._run_pre_commit, - BUILD_CONTAINER_STR: self.runner_handler._run_build_container, - PYTHON_MODULE_INSTALL_STR: self.runner_handler._run_install_python_module, - CONVENTIONAL_TITLE_STR: self.runner_handler._run_conventional_title_check, + TOX_STR: self.runner_handler.run_tox, + PRE_COMMIT_STR: self.runner_handler.run_pre_commit, + BUILD_CONTAINER_STR: self.runner_handler.run_build_container, + PYTHON_MODULE_INSTALL_STR: self.runner_handler.run_install_python_module, + CONVENTIONAL_TITLE_STR: self.runner_handler.run_conventional_title_check, } if not _target_tests: msg = "No test defined to retest" error_msg = f"{self.log_prefix} {msg}." self.logger.debug(error_msg) - self.pull_request.create_issue_comment(msg) + await asyncio.to_thread(pull_request.create_issue_comment, msg) return if "all" in command_args: @@ -245,7 +274,7 @@ def process_retest_command(self, command_args: str, reviewed_user: str) -> None: msg = "Invalid command. `all` cannot be used with other tests" error_msg = f"{self.log_prefix} {msg}." self.logger.debug(error_msg) - self.pull_request.create_issue_comment(msg) + await asyncio.to_thread(pull_request.create_issue_comment, msg) return else: @@ -263,14 +292,16 @@ def process_retest_command(self, command_args: str, reviewed_user: str) -> None: msg = f"No {' '.join(_not_supported_retests)} configured for this repository" error_msg = f"{self.log_prefix} {msg}." self.logger.debug(error_msg) - self.pull_request.create_issue_comment(msg) + await asyncio.to_thread(pull_request.create_issue_comment, msg) if _supported_retests: - _retest_to_exec: list[Future] = [] - with ThreadPoolExecutor() as executor: - for _test in _supported_retests: - _retest_to_exec.append(executor.submit(_retests_to_func_map[_test])) - - for result in as_completed(_retest_to_exec): - if _exp := result.exception(): - self.logger.error(f"{self.log_prefix} {_exp}") + tasks = [] + for _test in _supported_retests: + self.logger.debug(f"{self.log_prefix} running retest {_test}") + task = asyncio.create_task(_retests_to_func_map[_test](pull_request=pull_request)) + tasks.append(task) + + results = await asyncio.gather(*tasks, return_exceptions=True) + for result in results: + if isinstance(result, Exception): + self.logger.error(f"{self.log_prefix} Async task failed: {result}") diff --git a/webhook_server/libs/labels_handler.py b/webhook_server/libs/labels_handler.py index acbac0ca..1d160db0 100644 --- a/webhook_server/libs/labels_handler.py +++ b/webhook_server/libs/labels_handler.py @@ -1,8 +1,11 @@ +import asyncio from typing import Any from github.GithubException import UnknownObjectException -from timeout_sampler import TimeoutExpiredError, TimeoutSampler +from github.PullRequest import PullRequest +from timeout_sampler import TimeoutWatch +from webhook_server.libs.owners_files_handler import OwnersFileHandler from webhook_server.utils.constants import ( ADD_STR, APPROVE_STR, @@ -21,26 +24,28 @@ class LabelsHandler: - def __init__(self, github_webhook: Any) -> None: + def __init__(self, github_webhook: Any, owners_file_handler: OwnersFileHandler) -> 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 - self.pull_request = self.github_webhook.pull_request - def label_exists_in_pull_request(self, label: str) -> bool: - return label in self.pull_request_labels_names() + async def label_exists_in_pull_request(self, pull_request: PullRequest, label: str) -> bool: + return label in await self.pull_request_labels_names(pull_request=pull_request) - def pull_request_labels_names(self) -> list[str]: - return [lb.name for lb in self.pull_request.labels] + async def pull_request_labels_names(self, pull_request: PullRequest) -> list[str]: + labels = await asyncio.to_thread(pull_request.get_labels) + return [lb.name for lb in labels] - def _remove_label(self, label: str) -> bool: + async def _remove_label(self, pull_request: PullRequest, label: str) -> bool: try: - if self.label_exists_in_pull_request(label=label): + if await self.label_exists_in_pull_request(pull_request=pull_request, label=label): self.logger.info(f"{self.log_prefix} Removing label {label}") - self.pull_request.remove_from_labels(label) - return self.wait_for_label(label=label, exists=False) + await asyncio.to_thread(pull_request.remove_from_labels, label) + return await self.wait_for_label(pull_request=pull_request, label=label, exists=False) except Exception as exp: self.logger.debug(f"{self.log_prefix} Failed to remove {label} label. Exception: {exp}") return False @@ -48,19 +53,19 @@ def _remove_label(self, label: str) -> bool: self.logger.debug(f"{self.log_prefix} Label {label} not found and cannot be removed") return False - def _add_label(self, label: str) -> None: + async def _add_label(self, pull_request: PullRequest, label: str) -> None: label = label.strip() if len(label) > 49: self.logger.debug(f"{label} is too long, not adding.") return - if self.label_exists_in_pull_request(label=label): + if await self.label_exists_in_pull_request(pull_request=pull_request, label=label): self.logger.debug(f"{self.log_prefix} Label {label} already assign") return if label in STATIC_LABELS_DICT: self.logger.info(f"{self.log_prefix} Adding pull request label {label}") - self.pull_request.add_to_labels(label) + await asyncio.to_thread(pull_request.add_to_labels, label) return _color = [DYNAMIC_LABELS_DICT[_label] for _label in DYNAMIC_LABELS_DICT if _label in label] @@ -69,38 +74,33 @@ def _add_label(self, label: str) -> None: _with_color_msg = f"repository label {label} with color {color}" try: - _repo_label = self.repository.get_label(label) - _repo_label.edit(name=_repo_label.name, color=color) + _repo_label = await asyncio.to_thread(self.repository.get_label, label) + await asyncio.to_thread(_repo_label.edit, name=_repo_label.name, color=color) self.logger.debug(f"{self.log_prefix} Edit {_with_color_msg}") except UnknownObjectException: self.logger.debug(f"{self.log_prefix} Add {_with_color_msg}") - self.repository.create_label(name=label, color=color) + await asyncio.to_thread(self.repository.create_label, name=label, color=color) self.logger.info(f"{self.log_prefix} Adding pull request label {label}") - self.pull_request.add_to_labels(label) - self.wait_for_label(label=label, exists=True) + await asyncio.to_thread(pull_request.add_to_labels, label) + await self.wait_for_label(pull_request=pull_request, label=label, exists=True) - def wait_for_label(self, label: str, exists: bool) -> bool: - try: - for sample in TimeoutSampler( - wait_timeout=30, - sleep=5, - func=self.label_exists_in_pull_request, - label=label, - ): - if sample == exists: - return True + async def wait_for_label(self, pull_request: PullRequest, label: str, exists: bool) -> bool: + while TimeoutWatch(timeout=30).remaining_time() > 0: + res = await self.label_exists_in_pull_request(pull_request=pull_request, label=label) + if res == exists: + return True - except TimeoutExpiredError: - self.logger.debug(f"{self.log_prefix} Label {label} {'not found' if exists else 'found'}") + await asyncio.sleep(5) + self.logger.debug(f"{self.log_prefix} Label {label} {'not found' if exists else 'found'}") return False - def get_size(self) -> str: + def get_size(self, pull_request: PullRequest) -> str: """Calculates size label based on additions and deletions.""" - size = self.pull_request.additions + self.pull_request.deletions + size = pull_request.additions + pull_request.deletions # Define label thresholds in a more readable way threshold_sizes = [20, 50, 100, 300, 500] @@ -113,25 +113,30 @@ def get_size(self) -> str: return f"{SIZE_LABEL_PREFIX}XXL" - def add_size_label(self) -> None: + async def add_size_label(self, pull_request: PullRequest) -> None: """Add a size label to the pull request based on its additions and deletions.""" - size_label = self.get_size() + size_label = self.get_size(pull_request=pull_request) if not size_label: self.logger.debug(f"{self.log_prefix} Size label not found") return - if size_label in self.pull_request_labels_names(): + if size_label in await self.pull_request_labels_names(pull_request=pull_request): return - exists_size_label = [label for label in self.pull_request_labels_names() if label.startswith(SIZE_LABEL_PREFIX)] + exists_size_label = [ + label + for label in await self.pull_request_labels_names(pull_request=pull_request) + if label.startswith(SIZE_LABEL_PREFIX) + ] if exists_size_label: - self._remove_label(label=exists_size_label[0]) + await self._remove_label(pull_request=pull_request, label=exists_size_label[0]) - self._add_label(label=size_label) + await self._add_label(pull_request=pull_request, label=size_label) - def label_by_user_comment( + async def label_by_user_comment( self, + pull_request: PullRequest, user_requested_label: str, remove: bool, reviewed_user: str, @@ -142,7 +147,8 @@ def label_by_user_comment( ) if user_requested_label in (LGTM_STR, APPROVE_STR): - self.manage_reviewed_by_label( + await self.manage_reviewed_by_label( + pull_request=pull_request, review_state=user_requested_label, action=DELETE_STR if remove else ADD_STR, reviewed_user=reviewed_user, @@ -150,9 +156,11 @@ def label_by_user_comment( else: label_func = self._remove_label if remove else self._add_label - label_func(label=user_requested_label) + await label_func(pull_request=pull_request, label=user_requested_label) - def manage_reviewed_by_label(self, review_state: str, action: str, reviewed_user: str) -> None: + async def manage_reviewed_by_label( + self, pull_request: PullRequest, review_state: str, action: str, reviewed_user: str + ) -> None: self.logger.info( f"{self.log_prefix} " f"Processing label for review from {reviewed_user}. " @@ -162,7 +170,10 @@ def manage_reviewed_by_label(self, review_state: str, action: str, reviewed_user label_to_remove: str = "" if review_state == APPROVE_STR: - if reviewed_user in self.github_webhook.all_pull_request_approvers: + if ( + reviewed_user + in self.owners_file_handler.all_pull_request_approvers + self.owners_file_handler.root_approvers + ): label_prefix = APPROVED_BY_LABEL_PREFIX label_to_remove = f"{CHANGED_REQUESTED_BY_LABEL_PREFIX}{reviewed_user}" @@ -193,14 +204,14 @@ def manage_reviewed_by_label(self, review_state: str, action: str, reviewed_user reviewer_label = f"{label_prefix}{reviewed_user}" if action == ADD_STR: - self._add_label(label=reviewer_label) - self._remove_label(label=label_to_remove) + await self._add_label(pull_request=pull_request, label=reviewer_label) + await self._remove_label(pull_request=pull_request, label=label_to_remove) if action == DELETE_STR: - self._remove_label(label=reviewer_label) + await self._remove_label(pull_request=pull_request, label=reviewer_label) else: self.logger.warning( - f"{self.log_prefix} PR {self.pull_request.number} got unsupported review state: {review_state}" + f"{self.log_prefix} PR {pull_request.number} got unsupported review state: {review_state}" ) def wip_or_hold_lables_exists(self, labels: list[str]) -> str: diff --git a/webhook_server/libs/owners_files_handler.py b/webhook_server/libs/owners_files_handler.py new file mode 100644 index 00000000..8fa31282 --- /dev/null +++ b/webhook_server/libs/owners_files_handler.py @@ -0,0 +1,301 @@ +import asyncio +from pathlib import Path +from typing import Any + +import yaml +from asyncstdlib import functools +from github.ContentFile import ContentFile +from github.GithubException import GithubException +from github.NamedUser import NamedUser +from github.PaginatedList import PaginatedList +from github.PullRequest import PullRequest + + +class OwnersFileHandler: + def __init__(self, github_webhook: Any) -> None: + self.github_webhook = github_webhook + self.logger = self.github_webhook.logger + self.log_prefix = self.github_webhook.log_prefix + self.repository = self.github_webhook.repository + + async def initialize(self, pull_request: PullRequest) -> "OwnersFileHandler": + self.changed_files = await self.list_changed_files(pull_request=pull_request) + self.all_repository_approvers_and_reviewers = await self.get_all_repository_approvers_and_reviewers( + pull_request=pull_request + ) + self.all_repository_approvers = await self.get_all_repository_approvers() + self.all_repository_reviewers = await self.get_all_repository_reviewers() + self.all_pull_request_approvers = await self.get_all_pull_request_approvers() + self.all_pull_request_reviewers = await self.get_all_pull_request_reviewers() + + return self + + def _ensure_initialized(self) -> None: + if not hasattr(self, "changed_files"): + raise RuntimeError("OwnersFileHandler.initialize() must be called before using this method") + + @property + def root_reviewers(self) -> list[str]: + self._ensure_initialized() + + _reviewers = self.all_repository_approvers_and_reviewers.get(".", {}).get("reviewers", []) + self.logger.debug(f"{self.log_prefix} ROOT Reviewers: {_reviewers}") + return _reviewers + + @property + def root_approvers(self) -> list[str]: + self._ensure_initialized() + + _approvers = self.all_repository_approvers_and_reviewers.get(".", {}).get("approvers", []) + self.logger.debug(f"{self.log_prefix} ROOT Approvers: {_approvers}") + return _approvers + + async def list_changed_files(self, pull_request: PullRequest) -> list[str]: + return [_file.filename for _file in await asyncio.to_thread(pull_request.get_files)] + + def _validate_owners_content(self, content: Any, path: str) -> bool: + """Validate OWNERS file content structure.""" + try: + if not isinstance(content, dict): + raise ValueError("OWNERS file must contain a dictionary") + + for key in ["approvers", "reviewers"]: + if key in content: + if not isinstance(content[key], list): + raise ValueError(f"{key} must be a list") + + if not all(isinstance(_elm, str) for _elm in content[key]): + raise ValueError(f"All {key} must be strings") + + return True + + except ValueError as e: + self.logger.error(f"{self.log_prefix} Invalid OWNERS file {path}: {e}") + return False + + async def _get_file_content(self, content_path: str, pull_request: PullRequest) -> tuple[ContentFile, str]: + self.logger.debug(f"{self.log_prefix} Get OWNERS file from {content_path}") + + _path = await asyncio.to_thread(self.repository.get_contents, content_path, pull_request.base.ref) + + if isinstance(_path, list): + _path = _path[0] + + return _path, content_path + + @functools.lru_cache + async def get_all_repository_approvers_and_reviewers(self, pull_request: PullRequest) -> dict[str, dict[str, Any]]: + # Dictionary mapping OWNERS file paths to their approvers and reviewers + _owners: dict[str, dict[str, Any]] = {} + tasks = [] + + max_owners_files = 1000 # Configurable limit + owners_count = 0 + + self.logger.debug(f"{self.log_prefix} Get git tree") + tree = await asyncio.to_thread(self.repository.get_git_tree, pull_request.base.ref, recursive=True) + + for element in tree.tree: + if element.type == "blob" and element.path.endswith("OWNERS"): + owners_count += 1 + if owners_count > max_owners_files: + self.logger.error(f"{self.log_prefix} Too many OWNERS files (>{max_owners_files})") + break + + content_path = element.path + tasks.append(self._get_file_content(content_path, pull_request)) + + results = await asyncio.gather(*tasks) + + for result in results: + _path, _content_path = result + + try: + content = yaml.safe_load(_path.decoded_content) + if self._validate_owners_content(content, _content_path): + parent_path = str(Path(_content_path).parent) + if not parent_path: + parent_path = "." + _owners[parent_path] = content + + except yaml.YAMLError as exp: + self.logger.error(f"{self.log_prefix} Invalid OWNERS file {_content_path}: {exp}") + continue + + return _owners + + async def get_all_repository_approvers(self) -> list[str]: + self._ensure_initialized() + + _approvers: list[str] = [] + + for value in self.all_repository_approvers_and_reviewers.values(): + for key, val in value.items(): + if key == "approvers": + _approvers.extend(val) + + return _approvers + + async def get_all_repository_reviewers(self) -> list[str]: + self._ensure_initialized() + + _reviewers: list[str] = [] + + for value in self.all_repository_approvers_and_reviewers.values(): + for key, val in value.items(): + if key == "reviewers": + _reviewers.extend(val) + + return _reviewers + + async def get_all_pull_request_approvers(self) -> list[str]: + _approvers: list[str] = [] + changed_files = await self.owners_data_for_changed_files() + + for list_of_approvers in changed_files.values(): + for _approver in list_of_approvers.get("approvers", []): + _approvers.append(_approver) + + _approvers.sort() + return _approvers + + async def get_all_pull_request_reviewers(self) -> list[str]: + _reviewers: list[str] = [] + changed_files = await self.owners_data_for_changed_files() + + for list_of_reviewers in changed_files.values(): + for _reviewer in list_of_reviewers.get("reviewers", []): + _reviewers.append(_reviewer) + + _reviewers.sort() + return _reviewers + + async def owners_data_for_changed_files(self) -> dict[str, dict[str, Any]]: + self._ensure_initialized() + + data: dict[str, dict[str, Any]] = {} + + changed_folders = {Path(cf).parent for cf in self.changed_files} + + changed_folder_match: list[Path] = [] + + require_root_approvers: bool | None = None + + for owners_dir, owners_data in self.all_repository_approvers_and_reviewers.items(): + if owners_dir == ".": + continue + + _owners_dir = Path(owners_dir) + + for changed_folder in changed_folders: + if changed_folder == _owners_dir or _owners_dir in changed_folder.parents: + data[owners_dir] = owners_data + changed_folder_match.append(_owners_dir) + if require_root_approvers is None: + require_root_approvers = owners_data.get("root-approvers", True) + + if require_root_approvers or require_root_approvers is None: + self.logger.debug(f"{self.log_prefix} require root_approvers") + data["."] = self.all_repository_approvers_and_reviewers.get(".", {}) + + else: + for _folder in changed_folders: + for _changed_path in changed_folder_match: + if _folder == _changed_path or _changed_path in _folder.parents: + continue + else: + data["."] = self.all_repository_approvers_and_reviewers.get(".", {}) + break + + return data + + async def assign_reviewers(self, pull_request: PullRequest) -> None: + self._ensure_initialized() + + self.logger.info(f"{self.log_prefix} Assign reviewers") + + _to_add: list[str] = list(set(self.all_pull_request_reviewers)) + self.logger.debug(f"{self.log_prefix} Reviewers to add: {', '.join(_to_add)}") + + for reviewer in _to_add: + if reviewer != pull_request.user.login: + self.logger.debug(f"{self.log_prefix} Adding reviewer {reviewer}") + try: + await asyncio.to_thread(pull_request.create_review_request, [reviewer]) + + except GithubException as ex: + self.logger.debug(f"{self.log_prefix} Failed to add reviewer {reviewer}. {ex}") + await asyncio.to_thread( + pull_request.create_issue_comment, f"{reviewer} can not be added as reviewer. {ex}" + ) + + async def is_user_valid_to_run_commands(self, pull_request: PullRequest, reviewed_user: str) -> bool: + self._ensure_initialized() + + allowed_user_to_approve = await self.get_all_repository_maintainers() + self.all_repository_approvers + allow_user_comment = f"/add-allowed-user @{reviewed_user}" + + comment_msg = f""" +{reviewed_user} is not allowed to run retest commands. +maintainers can allow it by comment `{allow_user_comment}` +Maintainers: + - {"\n - @".join(allowed_user_to_approve)} +""" + valid_users = await self.valid_users_to_run_commands + + if reviewed_user not in valid_users: + comments_from_approvers = [ + comment.body + for comment in await asyncio.to_thread(pull_request.get_issue_comments) + if comment.user.login in allowed_user_to_approve + ] + for comment in comments_from_approvers: + if allow_user_comment in comment: + return True + + self.logger.debug(f"{self.log_prefix} {reviewed_user} is not in {valid_users}") + await asyncio.to_thread(pull_request.create_issue_comment, comment_msg) + return False + + return True + + @functools.cached_property + async def valid_users_to_run_commands(self) -> set[str]: + self._ensure_initialized() + + repository_collaborators = await self.get_all_repository_collaborators() + repository_contributors = await self.get_all_repository_contributors() + + return set(( + *repository_collaborators, + *repository_contributors, + *self.all_repository_approvers, + *self.all_pull_request_reviewers, + )) + + async def get_all_repository_contributors(self) -> list[str]: + contributors = await self.repository_contributors + return [val.login for val in contributors] + + async def get_all_repository_collaborators(self) -> list[str]: + collaborators = await self.repository_collaborators + return [val.login for val in collaborators] + + async def get_all_repository_maintainers(self) -> list[str]: + maintainers: list[str] = [] + + for user in await self.repository_collaborators: + permissions = user.permissions + + if permissions.admin or permissions.maintain: + maintainers.append(user.login) + + return maintainers + + @functools.cached_property + async def repository_collaborators(self) -> PaginatedList[NamedUser]: + return await asyncio.to_thread(self.repository.get_collaborators) + + @functools.cached_property + async def repository_contributors(self) -> PaginatedList[NamedUser]: + return await asyncio.to_thread(self.repository.get_contributors) diff --git a/webhook_server/libs/pull_request_handler.py b/webhook_server/libs/pull_request_handler.py index a4715d38..08b13f83 100644 --- a/webhook_server/libs/pull_request_handler.py +++ b/webhook_server/libs/pull_request_handler.py @@ -1,11 +1,13 @@ from __future__ import annotations -import time -from concurrent.futures import Future, ThreadPoolExecutor, as_completed +import asyncio from typing import Any +from github.PullRequest import PullRequest + from webhook_server.libs.check_run_handler import CheckRunHandler from webhook_server.libs.labels_handler import LabelsHandler +from webhook_server.libs.owners_files_handler import OwnersFileHandler from webhook_server.libs.runner_handler import RunnerHandler from webhook_server.utils.constants import ( APPROVED_BY_LABEL_PREFIX, @@ -32,78 +34,83 @@ class PullRequestHandler: - def __init__(self, github_webhook: Any): + def __init__(self, github_webhook: Any, owners_file_handler: OwnersFileHandler): 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 - self.pull_request = self.github_webhook.pull_request - self.labels_handler = LabelsHandler(github_webhook=self.github_webhook) + 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.runner_handler = RunnerHandler(github_webhook=self.github_webhook) + self.runner_handler = RunnerHandler( + github_webhook=self.github_webhook, owners_file_handler=self.owners_file_handler + ) - def process_pull_request_webhook_data(self) -> None: + async def process_pull_request_webhook_data(self, pull_request: PullRequest) -> None: hook_action: str = self.hook_data["action"] self.logger.info(f"{self.log_prefix} hook_action is: {hook_action}") pull_request_data: dict[str, Any] = self.hook_data["pull_request"] if hook_action == "edited": - self.set_wip_label_based_on_title() + await self.set_wip_label_based_on_title(pull_request=pull_request) if hook_action in ("opened", "reopened"): - pull_request_opened_futures: list[Future] = [] - with ThreadPoolExecutor() as executor: - if hook_action == "opened": - welcome_msg = self._prepare_welcome_comment() - pull_request_opened_futures.append( - executor.submit(self.pull_request.create_issue_comment, **{"body": welcome_msg}) - ) - pull_request_opened_futures.append(executor.submit(self.create_issue_for_new_pull_request)) - pull_request_opened_futures.append(executor.submit(self.set_wip_label_based_on_title)) - pull_request_opened_futures.append(executor.submit(self.process_opened_or_synchronize_pull_request)) + tasks = [] + + if hook_action == "opened": + welcome_msg = self._prepare_welcome_comment() + tasks.append(asyncio.to_thread(pull_request.create_issue_comment, body=welcome_msg)) - for result in as_completed(pull_request_opened_futures): - if _exp := result.exception(): - self.logger.error(f"{self.log_prefix} {_exp}") + tasks.append(self.create_issue_for_new_pull_request(pull_request=pull_request)) + tasks.append(self.set_wip_label_based_on_title(pull_request=pull_request)) + tasks.append(self.process_opened_or_synchronize_pull_request(pull_request=pull_request)) - # Set automerge only after all initialization of a new PR is done. - self.set_pull_request_automerge() + results = await asyncio.gather(*tasks, return_exceptions=True) + for result in results: + if isinstance(result, Exception): + self.logger.error(f"{self.log_prefix} Async task failed: {results}") + + # Set auto merge only after all initialization of a new PR is done. + await self.set_pull_request_automerge(pull_request=pull_request) if hook_action == "synchronize": - pull_request_synchronize_futures: list[Future] = [] - with ThreadPoolExecutor() as executor: - pull_request_synchronize_futures.append(executor.submit(self.remove_labels_when_pull_request_sync)) - pull_request_synchronize_futures.append( - executor.submit(self.process_opened_or_synchronize_pull_request) - ) + tasks = [] - for result in as_completed(pull_request_synchronize_futures): - if _exp := result.exception(): - self.logger.error(f"{self.log_prefix} {_exp}") + tasks.append(self.process_opened_or_synchronize_pull_request(pull_request=pull_request)) + tasks.append(self.remove_labels_when_pull_request_sync(pull_request=pull_request)) + + results = await asyncio.gather(*tasks, return_exceptions=True) + + for result in results: + if isinstance(result, Exception): + self.logger.error(f"{self.log_prefix} Async task failed: {result}") if hook_action == "closed": - self.close_issue_for_merged_or_closed_pr(hook_action=hook_action) - self.delete_remote_tag_for_merged_or_closed_pr() + await self.close_issue_for_merged_or_closed_pr(pull_request=pull_request, hook_action=hook_action) + await self.delete_remote_tag_for_merged_or_closed_pr(pull_request=pull_request) if is_merged := pull_request_data.get("merged", False): self.logger.info(f"{self.log_prefix} PR is merged") - for _label in self.pull_request.labels: + for _label in pull_request.labels: _label_name = _label.name if _label_name.startswith(CHERRY_PICK_LABEL_PREFIX): - self.runner_handler.cherry_pick(target_branch=_label_name.replace(CHERRY_PICK_LABEL_PREFIX, "")) + await self.runner_handler.cherry_pick( + pull_request=pull_request, target_branch=_label_name.replace(CHERRY_PICK_LABEL_PREFIX, "") + ) - self.runner_handler._run_build_container( + await self.runner_handler.run_build_container( push=True, set_check=False, is_merged=is_merged, + pull_request=pull_request, ) - # label_by_pull_requests_merge_state_after_merged will override self.pull_request - original_pull_request = self.pull_request - self.label_all_opened_pull_requests_merge_state_after_merged() - self.pull_request = original_pull_request + await self.label_all_opened_pull_requests_merge_state_after_merged() if hook_action in ("labeled", "unlabeled"): _check_for_merge: bool = False @@ -115,7 +122,7 @@ def process_pull_request_webhook_data(self) -> None: if labeled_lower == CAN_BE_MERGED_STR: return - self.logger.info(f"{self.log_prefix} PR {self.pull_request.number} {hook_action} with {labeled}") + self.logger.info(f"{self.log_prefix} PR {pull_request.number} {hook_action} with {labeled}") _split_label = labeled.split(LABELS_SEPARATOR, 1) @@ -129,8 +136,8 @@ def process_pull_request_webhook_data(self) -> None: ): if ( _user - in self.github_webhook.all_pull_request_reviewers - + self.github_webhook.all_pull_request_approvers + in self.owners_file_handler.all_pull_request_reviewers + + self.owners_file_handler.all_pull_request_approvers ): _check_for_merge = True @@ -138,28 +145,26 @@ def process_pull_request_webhook_data(self) -> None: _check_for_merge = True if action_labeled: - self.check_run_handler.set_verify_check_success() + await self.check_run_handler.set_verify_check_success() else: - self.check_run_handler.set_verify_check_queued() + await self.check_run_handler.set_verify_check_queued() if labeled_lower in (WIP_STR, HOLD_LABEL_STR): _check_for_merge = True if _check_for_merge: - self.check_if_can_be_merged() + await self.check_if_can_be_merged(pull_request=pull_request) - def set_wip_label_based_on_title(self) -> None: - if self.pull_request.title.lower().startswith(f"{WIP_STR}:"): - self.logger.debug( - f"{self.log_prefix} Found {WIP_STR} in {self.pull_request.title}; adding {WIP_STR} label." - ) - self.labels_handler._add_label(label=WIP_STR) + async def set_wip_label_based_on_title(self, pull_request: PullRequest) -> None: + if pull_request.title.lower().startswith(f"{WIP_STR}:"): + self.logger.debug(f"{self.log_prefix} Found {WIP_STR} in {pull_request.title}; adding {WIP_STR} label.") + await self.labels_handler._add_label(pull_request=pull_request, label=WIP_STR) else: self.logger.debug( - f"{self.log_prefix} {WIP_STR} not found in {self.pull_request.title}; removing {WIP_STR} label." + f"{self.log_prefix} {WIP_STR} not found in {pull_request.title}; removing {WIP_STR} label." ) - self.labels_handler._remove_label(label=WIP_STR) + await self.labels_handler._remove_label(pull_request=pull_request, label=WIP_STR) def _prepare_welcome_comment(self) -> str: self.logger.info(f"{self.log_prefix} Prepare welcome comment") @@ -216,10 +221,10 @@ def _prepare_owners_welcome_comment(self) -> str: body_approvers: str = " * Approvers:\n" body_reviewers: str = " * Reviewers:\n" - for _approver in self.github_webhook.all_pull_request_approvers: + for _approver in self.owners_file_handler.all_pull_request_approvers: body_approvers += f" * {_approver}\n" - for _reviewer in self.github_webhook.all_pull_request_reviewers: + for _reviewer in self.owners_file_handler.all_pull_request_reviewers: body_reviewers += f" * {_reviewer}\n" return f""" @@ -251,7 +256,7 @@ def _prepare_retest_welcome_comment(self) -> str: return " * This repository does not support retest actions" if not retest_msg else retest_msg - def label_all_opened_pull_requests_merge_state_after_merged(self) -> None: + async def label_all_opened_pull_requests_merge_state_after_merged(self) -> None: """ Labels pull requests based on their mergeable state. @@ -260,19 +265,18 @@ def label_all_opened_pull_requests_merge_state_after_merged(self) -> None: """ time_sleep = 30 self.logger.info(f"{self.log_prefix} Sleep for {time_sleep} seconds before getting all opened PRs") - time.sleep(time_sleep) + await asyncio.sleep(time_sleep) for pull_request in self.repository.get_pulls(state="open"): - self.pull_request = pull_request self.logger.info(f"{self.log_prefix} check label pull request after merge") - self.label_pull_request_by_merge_state() + await self.label_pull_request_by_merge_state(pull_request=pull_request) - def delete_remote_tag_for_merged_or_closed_pr(self) -> None: + async def delete_remote_tag_for_merged_or_closed_pr(self, pull_request: PullRequest) -> None: if not self.github_webhook.build_and_push_container: self.logger.info(f"{self.log_prefix} repository do not have container configured") return - repository_full_tag = self.github_webhook._container_repository_and_tag() + repository_full_tag = self.github_webhook.container_repository_and_tag(pull_request=pull_request) if not repository_full_tag: return @@ -294,18 +298,21 @@ def delete_remote_tag_for_merged_or_closed_pr(self) -> None: f"-p {self.github_webhook.container_repository_password}" ) - rc, out, err = self.runner_handler.run_podman_command(command=reg_login_cmd) + rc, out, err = await self.runner_handler.run_podman_command(command=reg_login_cmd) if rc: try: tag_ls_cmd = f"regctl tag ls {self.github_webhook.container_repository} --include {pr_tag}" - rc, out, err = self.runner_handler.run_podman_command(command=tag_ls_cmd) + rc, out, err = await self.runner_handler.run_podman_command(command=tag_ls_cmd) if rc and out: tag_del_cmd = f"regctl tag delete {repository_full_tag}" - if self.runner_handler.run_podman_command(command=tag_del_cmd)[0]: - self.pull_request.create_issue_comment(f"Successfully removed PR tag: {repository_full_tag}.") + rc, _, _ = await self.runner_handler.run_podman_command(command=tag_del_cmd) + if rc: + await asyncio.to_thread( + pull_request.create_issue_comment, f"Successfully removed PR tag: {repository_full_tag}." + ) else: self.logger.error( f"{self.log_prefix} Failed to delete tag: {repository_full_tag}. OUT:{out}. ERR:{err}" @@ -316,59 +323,65 @@ def delete_remote_tag_for_merged_or_closed_pr(self) -> None: f"OUT:{out}. ERR:{err}" ) finally: - self.runner_handler.run_podman_command(command="regctl registry logout") + await self.runner_handler.run_podman_command(command="regctl registry logout") else: - self.pull_request.create_issue_comment( - f"Failed to delete tag: {repository_full_tag}. Please delete it manually." + await asyncio.to_thread( + pull_request.create_issue_comment, + f"Failed to delete tag: {repository_full_tag}. Please delete it manually.", ) self.logger.error(f"{self.log_prefix} Failed to delete tag: {repository_full_tag}. OUT:{out}. ERR:{err}") - def close_issue_for_merged_or_closed_pr(self, hook_action: str) -> None: - for issue in self.repository.get_issues(): - if issue.body == self._generate_issue_body(): - self.logger.info(f"{self.log_prefix} Closing issue {issue.title} for PR: {self.pull_request.title}") - issue.create_comment( - f"{self.log_prefix} Closing issue for PR: {self.pull_request.title}.\nPR was {hook_action}." + async def close_issue_for_merged_or_closed_pr(self, pull_request: PullRequest, hook_action: str) -> None: + for issue in await asyncio.to_thread(self.repository.get_issues): + if issue.body == self._generate_issue_body(pull_request=pull_request): + self.logger.info(f"{self.log_prefix} Closing issue {issue.title} for PR: {pull_request.title}") + await asyncio.to_thread( + issue.create_comment, + f"{self.log_prefix} Closing issue for PR: {pull_request.title}.\nPR was {hook_action}.", ) - issue.edit(state="closed") + await asyncio.to_thread(issue.edit, state="closed") + break - def process_opened_or_synchronize_pull_request(self) -> None: - prepare_pull_futures: list[Future] = [] + async def process_opened_or_synchronize_pull_request(self, pull_request: PullRequest) -> None: + tasks = [] - with ThreadPoolExecutor() as executor: - prepare_pull_futures.append(executor.submit(self.github_webhook.assign_reviewers)) - prepare_pull_futures.append( - executor.submit( - self.labels_handler._add_label, - **{"label": f"{BRANCH_LABEL_PREFIX}{self.github_webhook.pull_request_branch}"}, - ) + tasks.append(self.owners_file_handler.assign_reviewers(pull_request=pull_request)) + tasks.append( + self.labels_handler._add_label( + **{ + "pull_request": pull_request, + "label": f"{BRANCH_LABEL_PREFIX}{pull_request.base.ref}", + }, ) - prepare_pull_futures.append(executor.submit(self.label_pull_request_by_merge_state)) - prepare_pull_futures.append(executor.submit(self.check_run_handler.set_merge_check_queued)) - prepare_pull_futures.append(executor.submit(self.check_run_handler.set_run_tox_check_queued)) - prepare_pull_futures.append(executor.submit(self.check_run_handler.set_run_pre_commit_check_queued)) - prepare_pull_futures.append(executor.submit(self.check_run_handler.set_python_module_install_queued)) - prepare_pull_futures.append(executor.submit(self.check_run_handler.set_container_build_queued)) - prepare_pull_futures.append(executor.submit(self._process_verified_for_update_or_new_pull_request)) - prepare_pull_futures.append(executor.submit(self.labels_handler.add_size_label)) - prepare_pull_futures.append(executor.submit(self.add_pull_request_owner_as_assingee)) - - prepare_pull_futures.append(executor.submit(self.runner_handler._run_tox)) - prepare_pull_futures.append(executor.submit(self.runner_handler._run_pre_commit)) - prepare_pull_futures.append(executor.submit(self.runner_handler._run_install_python_module)) - prepare_pull_futures.append(executor.submit(self.runner_handler._run_build_container)) - - if self.github_webhook.conventional_title: - prepare_pull_futures.append(executor.submit(self.check_run_handler.set_conventional_title_queued)) - prepare_pull_futures.append(executor.submit(self.runner_handler._run_conventional_title_check)) - - for result in as_completed(prepare_pull_futures): - if _exp := result.exception(): - self.logger.error(f"{self.log_prefix} {_exp}") - - def create_issue_for_new_pull_request(self) -> None: + ) + tasks.append(self.label_pull_request_by_merge_state(pull_request=pull_request)) + tasks.append(self.check_run_handler.set_merge_check_queued()) + tasks.append(self.check_run_handler.set_run_tox_check_queued()) + tasks.append(self.check_run_handler.set_run_pre_commit_check_queued()) + tasks.append(self.check_run_handler.set_python_module_install_queued()) + tasks.append(self.check_run_handler.set_container_build_queued()) + tasks.append(self._process_verified_for_update_or_new_pull_request(pull_request=pull_request)) + tasks.append(self.labels_handler.add_size_label(pull_request=pull_request)) + tasks.append(self.add_pull_request_owner_as_assingee(pull_request=pull_request)) + + tasks.append(self.runner_handler.run_tox(pull_request=pull_request)) + tasks.append(self.runner_handler.run_pre_commit(pull_request=pull_request)) + tasks.append(self.runner_handler.run_install_python_module(pull_request=pull_request)) + tasks.append(self.runner_handler.run_build_container(pull_request=pull_request)) + + if self.github_webhook.conventional_title: + tasks.append(self.check_run_handler.set_conventional_title_queued()) + tasks.append(self.runner_handler.run_conventional_title_check(pull_request=pull_request)) + + results = await asyncio.gather(*tasks, return_exceptions=True) + + for result in results: + if isinstance(result, Exception): + self.logger.error(f"{self.log_prefix} Async task failed: {result}") + + async def create_issue_for_new_pull_request(self, pull_request: PullRequest) -> None: if self.github_webhook.parent_committer in self.github_webhook.auto_verified_and_merged_users: self.logger.info( f"{self.log_prefix} Committer {self.github_webhook.parent_committer} is part of " @@ -376,84 +389,85 @@ def create_issue_for_new_pull_request(self) -> None: ) return - self.logger.info(f"{self.log_prefix} Creating issue for new PR: {self.pull_request.title}") - self.repository.create_issue( - title=self._generate_issue_title(), - body=self._generate_issue_body(), - assignee=self.pull_request.user.login, + self.logger.info(f"{self.log_prefix} Creating issue for new PR: {pull_request.title}") + await asyncio.to_thread( + self.repository.create_issue, + title=self._generate_issue_title(pull_request=pull_request), + body=self._generate_issue_body(pull_request=pull_request), + assignee=pull_request.user.login, ) - def _generate_issue_title(self) -> str: - return f"{self.pull_request.title} - {self.pull_request.number}" + def _generate_issue_title(self, pull_request: PullRequest) -> str: + return f"{pull_request.title} - {pull_request.number}" - def _generate_issue_body(self) -> str: - return f"[Auto generated]\nNumber: [#{self.pull_request.number}]" + def _generate_issue_body(self, pull_request: PullRequest) -> str: + return f"[Auto generated]\nNumber: [#{pull_request.number}]" - def set_pull_request_automerge(self) -> None: + async def set_pull_request_automerge(self, pull_request: PullRequest) -> None: auto_merge = ( - self.github_webhook.pull_request_branch in self.github_webhook.set_auto_merge_prs + pull_request.base.ref in self.github_webhook.set_auto_merge_prs or self.github_webhook.parent_committer in self.github_webhook.auto_verified_and_merged_users ) - self.logger.debug( - f"{self.log_prefix} auto_merge: {auto_merge}, branch: {self.github_webhook.pull_request_branch}" - ) + self.logger.debug(f"{self.log_prefix} auto_merge: {auto_merge}, branch: {pull_request.base.ref}") if auto_merge: try: - if not self.pull_request.raw_data.get("auto_merge"): + if not pull_request.raw_data.get("auto_merge"): self.logger.info( f"{self.log_prefix} will be merged automatically. owner: {self.github_webhook.parent_committer} " f"is part of auto merge enabled rules" ) - self.pull_request.enable_automerge(merge_method="SQUASH") + await asyncio.to_thread(pull_request.enable_automerge, merge_method="SQUASH") else: self.logger.debug(f"{self.log_prefix} is already set to auto merge") except Exception as exp: self.logger.error(f"{self.log_prefix} Exception while setting auto merge: {exp}") - def remove_labels_when_pull_request_sync(self) -> None: - futures = [] - with ThreadPoolExecutor() as executor: - for _label in self.pull_request.labels: - _label_name = _label.name - if ( - _label_name.startswith(APPROVED_BY_LABEL_PREFIX) - or _label_name.startswith(COMMENTED_BY_LABEL_PREFIX) - or _label_name.startswith(CHANGED_REQUESTED_BY_LABEL_PREFIX) - or _label_name.startswith(LGTM_BY_LABEL_PREFIX) - ): - futures.append( - executor.submit( - self.labels_handler._remove_label, - **{ - "label": _label_name, - }, - ) + async def remove_labels_when_pull_request_sync(self, pull_request: PullRequest) -> None: + tasks = [] + for _label in pull_request.labels: + _label_name = _label.name + if ( + _label_name.startswith(APPROVED_BY_LABEL_PREFIX) + or _label_name.startswith(COMMENTED_BY_LABEL_PREFIX) + or _label_name.startswith(CHANGED_REQUESTED_BY_LABEL_PREFIX) + or _label_name.startswith(LGTM_BY_LABEL_PREFIX) + ): + tasks.append( + self.labels_handler._remove_label( + **{ + "pull_request": pull_request, + "label": _label_name, + }, ) - for _ in as_completed(futures): - # wait for all tasks to complete - pass + ) + + results = await asyncio.gather(*tasks, return_exceptions=True) + + for result in results: + if isinstance(result, Exception): + self.logger.error(f"{self.log_prefix} Async task failed: {result}") - def label_pull_request_by_merge_state(self) -> None: - merge_state = self.pull_request.mergeable_state + async def label_pull_request_by_merge_state(self, pull_request: PullRequest) -> None: + merge_state = pull_request.mergeable_state self.logger.debug(f"{self.log_prefix} Mergeable state is {merge_state}") if merge_state == "unknown": return if merge_state == "behind": - self.labels_handler._add_label(label=NEEDS_REBASE_LABEL_STR) + await self.labels_handler._add_label(pull_request=pull_request, label=NEEDS_REBASE_LABEL_STR) else: - self.labels_handler._remove_label(label=NEEDS_REBASE_LABEL_STR) + await self.labels_handler._remove_label(pull_request=pull_request, label=NEEDS_REBASE_LABEL_STR) if merge_state == "dirty": - self.labels_handler._add_label(label=HAS_CONFLICTS_LABEL_STR) + await self.labels_handler._add_label(pull_request=pull_request, label=HAS_CONFLICTS_LABEL_STR) else: - self.labels_handler._remove_label(label=HAS_CONFLICTS_LABEL_STR) + await self.labels_handler._remove_label(pull_request=pull_request, label=HAS_CONFLICTS_LABEL_STR) - def _process_verified_for_update_or_new_pull_request(self) -> None: + async def _process_verified_for_update_or_new_pull_request(self, pull_request: PullRequest) -> None: if not self.github_webhook.verified_job: return @@ -462,26 +476,26 @@ def _process_verified_for_update_or_new_pull_request(self) -> None: f"{self.log_prefix} Committer {self.github_webhook.parent_committer} is part of {self.github_webhook.auto_verified_and_merged_users}" ", Setting verified label" ) - self.labels_handler._add_label(label=VERIFIED_LABEL_STR) - self.check_run_handler.set_verify_check_success() + await self.labels_handler._add_label(pull_request=pull_request, label=VERIFIED_LABEL_STR) + await self.check_run_handler.set_verify_check_success() else: self.logger.info(f"{self.log_prefix} Processing reset {VERIFIED_LABEL_STR} label on new commit push") # Remove verified label - self.labels_handler._remove_label(label=VERIFIED_LABEL_STR) - self.check_run_handler.set_verify_check_queued() + await self.labels_handler._remove_label(pull_request=pull_request, label=VERIFIED_LABEL_STR) + await self.check_run_handler.set_verify_check_queued() - def add_pull_request_owner_as_assingee(self) -> None: + async def add_pull_request_owner_as_assingee(self, pull_request: PullRequest) -> None: try: self.logger.info(f"{self.log_prefix} Adding PR owner as assignee") - self.pull_request.add_to_assignees(self.pull_request.user.login) + pull_request.add_to_assignees(pull_request.user.login) except Exception as exp: self.logger.debug(f"{self.log_prefix} Exception while adding PR owner as assignee: {exp}") if self.github_webhook.root_approvers: self.logger.debug(f"{self.log_prefix} Falling back to first approver as assignee") - self.pull_request.add_to_assignees(self.github_webhook.root_approvers[0]) + pull_request.add_to_assignees(self.github_webhook.root_approvers[0]) - def check_if_can_be_merged(self) -> None: + async def check_if_can_be_merged(self, pull_request: PullRequest) -> None: """ Check if PR can be merged and set the job for it @@ -493,7 +507,7 @@ def check_if_can_be_merged(self) -> None: PR status is not 'dirty'. PR has no changed requests from approvers. """ - if self.skip_if_pull_request_already_merged(): + if self.skip_if_pull_request_already_merged(pull_request=pull_request): self.logger.debug(f"{self.log_prefix} Pull request already merged") return @@ -506,17 +520,21 @@ def check_if_can_be_merged(self) -> None: try: self.logger.info(f"{self.log_prefix} Check if {CAN_BE_MERGED_STR}.") - self.check_run_handler.set_merge_check_in_progress() - last_commit_check_runs = list(self.github_webhook.last_commit.get_check_runs()) - _labels = self.labels_handler.pull_request_labels_names() + await self.check_run_handler.set_merge_check_in_progress() + _last_commit_check_runs = await asyncio.to_thread(self.github_webhook.last_commit.get_check_runs) + last_commit_check_runs = list(_last_commit_check_runs) + _labels = await self.labels_handler.pull_request_labels_names(pull_request=pull_request) self.logger.debug(f"{self.log_prefix} check if can be merged. PR labels are: {_labels}") - is_pr_mergable = self.pull_request.mergeable + is_pr_mergable = pull_request.mergeable if not is_pr_mergable: failure_output += f"PR is not mergeable: {is_pr_mergable}\n" - required_check_in_progress_failure_output, check_runs_in_progress = ( - self.check_run_handler.required_check_in_progress(last_commit_check_runs=last_commit_check_runs) + ( + required_check_in_progress_failure_output, + check_runs_in_progress, + ) = await self.check_run_handler.required_check_in_progress( + pull_request=pull_request, last_commit_check_runs=last_commit_check_runs ) if required_check_in_progress_failure_output: failure_output += required_check_in_progress_failure_output @@ -525,8 +543,10 @@ def check_if_can_be_merged(self) -> None: if labels_failure_output: failure_output += labels_failure_output - required_check_failed_failure_output = self.check_run_handler.required_check_failed( - last_commit_check_runs=last_commit_check_runs, check_runs_in_progress=check_runs_in_progress + required_check_failed_failure_output = await self.check_run_handler.required_check_failed_or_no_status( + pull_request=pull_request, + last_commit_check_runs=last_commit_check_runs, + check_runs_in_progress=check_runs_in_progress, ) if required_check_failed_failure_output: failure_output += required_check_failed_failure_output @@ -535,21 +555,21 @@ def check_if_can_be_merged(self) -> None: if labels_failure_output: failure_output += labels_failure_output - pr_approvered_failure_output = self._check_if_pr_approved(labels=_labels) + pr_approvered_failure_output = await self._check_if_pr_approved(labels=_labels) if pr_approvered_failure_output: failure_output += pr_approvered_failure_output if not failure_output: - self.labels_handler._add_label(label=CAN_BE_MERGED_STR) - self.check_run_handler.set_merge_check_success() + await self.labels_handler._add_label(pull_request=pull_request, label=CAN_BE_MERGED_STR) + await self.check_run_handler.set_merge_check_success() self.logger.info(f"{self.log_prefix} Pull request can be merged") return self.logger.debug(f"{self.log_prefix} cannot be merged: {failure_output}") output["text"] = failure_output - self.labels_handler._remove_label(label=CAN_BE_MERGED_STR) - self.check_run_handler.set_merge_check_failure(output=output) + await self.labels_handler._remove_label(pull_request=pull_request, label=CAN_BE_MERGED_STR) + await self.check_run_handler.set_merge_check_failure(output=output) except Exception as ex: self.logger.error( @@ -557,10 +577,10 @@ def check_if_can_be_merged(self) -> None: ) _err = "Failed to check if can be merged, check logs" output["text"] = _err - self.labels_handler._remove_label(label=CAN_BE_MERGED_STR) - self.check_run_handler.set_merge_check_failure(output=output) + await self.labels_handler._remove_label(pull_request=pull_request, label=CAN_BE_MERGED_STR) + await self.check_run_handler.set_merge_check_failure(output=output) - def _check_if_pr_approved(self, labels: list[str]) -> str: + async def _check_if_pr_approved(self, labels: list[str]) -> str: self.logger.info(f"{self.log_prefix} Check if pull request is approved by pull request labels.") error: str = "" @@ -568,9 +588,9 @@ def _check_if_pr_approved(self, labels: list[str]) -> str: lgtm_count: int = 0 all_reviewers = ( - self.github_webhook.all_pull_request_reviewers.copy() - + self.github_webhook.root_approvers.copy() - + self.github_webhook.root_reviewers.copy() + self.owners_file_handler.all_pull_request_reviewers.copy() + + self.owners_file_handler.root_approvers.copy() + + self.owners_file_handler.root_reviewers.copy() ) all_reviewers_without_pr_owner = { _reviewer for _reviewer in all_reviewers if _reviewer != self.github_webhook.parent_committer @@ -586,18 +606,27 @@ def _check_if_pr_approved(self, labels: list[str]) -> str: if APPROVED_BY_LABEL_PREFIX.lower() in _label.lower(): approved_by.append(_label.split(LABELS_SEPARATOR)[-1]) - missing_approvers = self.github_webhook.all_pull_request_approvers.copy() + missing_approvers = list(set(self.owners_file_handler.all_pull_request_approvers.copy())) + owners_data_changed_files = await self.owners_file_handler.owners_data_for_changed_files() + + # If any of root approvers is in approved_by list, the pull request is approved + for _approver in approved_by: + if _approver in self.owners_file_handler.root_approvers: + missing_approvers = [] + break + + if missing_approvers: + for data in owners_data_changed_files.values(): + required_pr_approvers = data.get("approvers", []) - for data in self.github_webhook.owners_data_for_changed_files().values(): - required_pr_approvers = data.get("approvers", []) - for required_pr_approver in required_pr_approvers: - if required_pr_approver in approved_by: - # Once we found approver in approved_by list, we remove all approvers from missing_approvers list for this owners file - for _approver in required_pr_approvers: - if _approver in missing_approvers: - missing_approvers.remove(_approver) + for required_pr_approver in required_pr_approvers: + if required_pr_approver in approved_by: + # Once we found approver in approved_by list, we remove all approvers from missing_approvers list for this owners file + for _approver in required_pr_approvers: + if _approver in missing_approvers: + missing_approvers.remove(_approver) - break + break missing_approvers = list(set(missing_approvers)) @@ -624,7 +653,7 @@ def _check_lables_for_can_be_merged(self, labels: list[str]) -> str: for _label in labels: if CHANGED_REQUESTED_BY_LABEL_PREFIX.lower() in _label.lower(): change_request_user = _label.split(LABELS_SEPARATOR)[-1] - if change_request_user in self.github_webhook.all_pull_request_approvers: + if change_request_user in self.owners_file_handler.all_pull_request_approvers: failure_output += "PR has changed requests from approvers\n" missing_required_labels = [] @@ -637,8 +666,8 @@ def _check_lables_for_can_be_merged(self, labels: list[str]) -> str: return failure_output - def skip_if_pull_request_already_merged(self) -> bool: - if self.pull_request and self.pull_request.is_merged(): + def skip_if_pull_request_already_merged(self, pull_request: PullRequest) -> bool: + if pull_request and pull_request.is_merged(): self.logger.info(f"{self.log_prefix}: PR is merged, not processing") return True diff --git a/webhook_server/libs/pull_request_review_handler.py b/webhook_server/libs/pull_request_review_handler.py index 82b0f52d..e10b1bec 100644 --- a/webhook_server/libs/pull_request_review_handler.py +++ b/webhook_server/libs/pull_request_review_handler.py @@ -1,20 +1,23 @@ from typing import Any +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 ADD_STR, APPROVE_STR class PullRequestReviewHandler: - def __init__(self, github_webhook: Any): + def __init__(self, github_webhook: Any, owners_file_handler: OwnersFileHandler): 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 - self.pull_request = self.github_webhook.pull_request - self.labels_handler = LabelsHandler(github_webhook=self.github_webhook) + self.labels_handler = LabelsHandler( + github_webhook=self.github_webhook, owners_file_handler=self.owners_file_handler + ) - def process_pull_request_review_webhook_data(self) -> None: + async def process_pull_request_review_webhook_data(self, pull_request: PullRequest) -> None: if self.hook_data["action"] == "submitted": """ Available actions: @@ -25,7 +28,8 @@ def process_pull_request_review_webhook_data(self) -> None: reviewed_user = self.hook_data["review"]["user"]["login"] review_state = self.hook_data["review"]["state"] - self.labels_handler.manage_reviewed_by_label( + await self.labels_handler.manage_reviewed_by_label( + pull_request=pull_request, review_state=review_state, action=ADD_STR, reviewed_user=reviewed_user, @@ -33,7 +37,8 @@ def process_pull_request_review_webhook_data(self) -> None: if body := self.hook_data["review"]["body"]: if f"/{APPROVE_STR}" in body: - self.labels_handler.label_by_user_comment( + await self.labels_handler.label_by_user_comment( + pull_request=pull_request, user_requested_label=APPROVE_STR, remove=False, reviewed_user=reviewed_user, diff --git a/webhook_server/libs/push_handler.py b/webhook_server/libs/push_handler.py index e94be345..033eddb0 100644 --- a/webhook_server/libs/push_handler.py +++ b/webhook_server/libs/push_handler.py @@ -10,6 +10,7 @@ class PushHandler: def __init__(self, github_webhook: Any): self.github_webhook = github_webhook + self.hook_data = self.github_webhook.hook_data self.logger = self.github_webhook.logger self.log_prefix = self.github_webhook.log_prefix @@ -17,20 +18,20 @@ def __init__(self, github_webhook: Any): self.check_run_handler = CheckRunHandler(github_webhook=self.github_webhook) self.runner_handler = RunnerHandler(github_webhook=self.github_webhook) - def process_push_webhook_data(self) -> None: + async def process_push_webhook_data(self) -> None: tag = re.search(r"refs/tags/?(.*)", self.hook_data["ref"]) if tag: tag_name = tag.group(1) self.logger.info(f"{self.log_prefix} Processing push for tag: {tag.group(1)}") if self.github_webhook.pypi: self.logger.info(f"{self.log_prefix} Processing upload to pypi for tag: {tag_name}") - self.upload_to_pypi(tag_name=tag_name) + await self.upload_to_pypi(tag_name=tag_name) if self.github_webhook.build_and_push_container and self.github_webhook.container_release: self.logger.info(f"{self.log_prefix} Processing build and push container for tag: {tag_name}") - self.runner_handler._run_build_container(push=True, set_check=False, tag=tag_name) + await self.runner_handler.run_build_container(push=True, set_check=False, tag=tag_name) - def upload_to_pypi(self, tag_name: str) -> None: + async def upload_to_pypi(self, tag_name: str) -> None: def _issue_on_error(_error: str) -> None: self.repository.create_issue( title=_error, @@ -45,19 +46,21 @@ def _issue_on_error(_error: str) -> None: self.logger.info(f"{self.log_prefix} Start uploading to pypi") _dist_dir: str = f"{clone_repo_dir}/pypi-dist" - with self.runner_handler._prepare_cloned_repo_dir(checkout=tag_name, clone_repo_dir=clone_repo_dir) as _res: + async with self.runner_handler._prepare_cloned_repo_dir( + checkout=tag_name, clone_repo_dir=clone_repo_dir + ) as _res: if not _res[0]: _error = self.check_run_handler.get_check_run_text(out=_res[1], err=_res[2]) return _issue_on_error(_error=_error) - rc, out, err = run_command( + rc, out, err = await run_command( command=f"uv {uv_cmd_dir} build --sdist --out-dir {_dist_dir}", log_prefix=self.log_prefix ) if not rc: _error = self.check_run_handler.get_check_run_text(out=out, err=err) return _issue_on_error(_error=_error) - rc, tar_gz_file, err = run_command(command=f"ls {_dist_dir}", log_prefix=self.log_prefix) + rc, tar_gz_file, err = await run_command(command=f"ls {_dist_dir}", log_prefix=self.log_prefix) if not rc: _error = self.check_run_handler.get_check_run_text(out=tar_gz_file, err=err) return _issue_on_error(_error=_error) @@ -70,7 +73,7 @@ def _issue_on_error(_error: str) -> None: ] for cmd in commands: - rc, out, err = run_command(command=cmd, log_prefix=self.log_prefix) + rc, out, err = await run_command(command=cmd, log_prefix=self.log_prefix) if not rc: _error = self.check_run_handler.get_check_run_text(out=out, err=err) return _issue_on_error(_error=_error) diff --git a/webhook_server/libs/runner_handler.py b/webhook_server/libs/runner_handler.py index e5490c93..1e5458b4 100644 --- a/webhook_server/libs/runner_handler.py +++ b/webhook_server/libs/runner_handler.py @@ -1,15 +1,15 @@ +import asyncio import contextlib -import functools import shutil -from typing import Any, Generator +from typing import Any, AsyncGenerator from uuid import uuid4 import shortuuid -from github.NamedUser import NamedUser -from github.PaginatedList import PaginatedList +from github.Branch import Branch +from github.PullRequest import PullRequest from webhook_server.libs.check_run_handler import CheckRunHandler -from webhook_server.libs.exceptions import NoPullRequestError +from webhook_server.libs.owners_files_handler import OwnersFileHandler from webhook_server.utils.constants import ( BUILD_CONTAINER_STR, CHERRY_PICKED_LABEL_PREFIX, @@ -22,31 +22,32 @@ class RunnerHandler: - def __init__(self, github_webhook: Any): + def __init__(self, github_webhook: Any, 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 - self.pull_request = getattr(self.github_webhook, "pull_request", None) self.check_run_handler = CheckRunHandler(github_webhook=self.github_webhook) - @contextlib.contextmanager - def _prepare_cloned_repo_dir( + @contextlib.asynccontextmanager + async def _prepare_cloned_repo_dir( self, clone_repo_dir: str, + pull_request: PullRequest | None = None, is_merged: bool = False, checkout: str = "", tag_name: str = "", - ) -> Generator[tuple[bool, Any, Any], None, None]: + ) -> AsyncGenerator[tuple[bool, Any, Any], None]: git_cmd = f"git --work-tree={clone_repo_dir} --git-dir={clone_repo_dir}/.git" result: tuple[bool, str, str] = (True, "", "") success = True try: # Clone the repository - rc, out, err = run_command( + rc, out, err = await run_command( command=f"git clone {self.repository.clone_url.replace('https://', f'https://{self.github_webhook.token}@')} " f"{clone_repo_dir}", log_prefix=self.log_prefix, @@ -56,7 +57,7 @@ def _prepare_cloned_repo_dir( success = False if success: - rc, out, err = run_command( + rc, out, err = await run_command( command=f"{git_cmd} config user.name '{self.repository.owner.login}'", log_prefix=self.log_prefix ) if not rc: @@ -64,7 +65,7 @@ def _prepare_cloned_repo_dir( success = False if success: - rc, out, err = run_command( + rc, out, err = await run_command( f"{git_cmd} config user.email '{self.repository.owner.email}'", log_prefix=self.log_prefix ) if not rc: @@ -72,7 +73,7 @@ def _prepare_cloned_repo_dir( success = False if success: - rc, out, err = run_command( + rc, out, err = await run_command( command=f"{git_cmd} config --local --add remote.origin.fetch +refs/pull/*/head:refs/remotes/origin/pr/*", log_prefix=self.log_prefix, ) @@ -81,21 +82,21 @@ def _prepare_cloned_repo_dir( success = False if success: - rc, out, err = run_command(command=f"{git_cmd} remote update", log_prefix=self.log_prefix) + rc, out, err = await run_command(command=f"{git_cmd} remote update", log_prefix=self.log_prefix) if not rc: result = (rc, out, err) success = False # Checkout to requested branch/tag if checkout and success: - rc, out, err = run_command(f"{git_cmd} checkout {checkout}", log_prefix=self.log_prefix) + rc, out, err = await run_command(f"{git_cmd} checkout {checkout}", log_prefix=self.log_prefix) if not rc: result = (rc, out, err) success = False - if success and self.pull_request: - rc, out, err = run_command( - f"{git_cmd} merge origin/{self.github_webhook.pull_request_branch} -m 'Merge {self.github_webhook.pull_request_branch}'", + if success and pull_request: + rc, out, err = await run_command( + f"{git_cmd} merge origin/{pull_request.base.ref} -m 'Merge {pull_request.base.ref}'", log_prefix=self.log_prefix, ) if not rc: @@ -105,9 +106,9 @@ def _prepare_cloned_repo_dir( # Checkout the branch if pull request is merged or for release else: if success: - if is_merged: - rc, out, err = run_command( - command=f"{git_cmd} checkout {self.github_webhook.pull_request_branch}", + if is_merged and pull_request: + rc, out, err = await run_command( + command=f"{git_cmd} checkout {pull_request.base.ref}", log_prefix=self.log_prefix, ) if not rc: @@ -115,35 +116,32 @@ def _prepare_cloned_repo_dir( success = False elif tag_name: - rc, out, err = run_command(command=f"{git_cmd} checkout {tag_name}", log_prefix=self.log_prefix) + rc, out, err = await run_command( + command=f"{git_cmd} checkout {tag_name}", log_prefix=self.log_prefix + ) if not rc: result = (rc, out, err) success = False # Checkout the pull request else: - try: - pull_request = self.github_webhook._get_pull_request() - rc, out, err = run_command( - command=f"{git_cmd} checkout origin/pr/{pull_request.number}", + if _pull_request := await self.github_webhook.get_pull_request(): + rc, out, err = await run_command( + command=f"{git_cmd} checkout origin/pr/{_pull_request.number}", log_prefix=self.log_prefix, ) if not rc: result = (rc, out, err) success = False - if self.pull_request and success: - rc, out, err = run_command( - f"{git_cmd} merge origin/{self.github_webhook.pull_request_branch} -m 'Merge {self.github_webhook.pull_request_branch}'", + if pull_request and success: + rc, out, err = await run_command( + f"{git_cmd} merge origin/{pull_request.base.ref} -m 'Merge {pull_request.base.ref}'", log_prefix=self.log_prefix, ) if not rc: result = (rc, out, err) - except NoPullRequestError: - self.logger.error(f"{self.log_prefix} [func:_run_in_container] No pull request found") - result = (False, "", "[func:_run_in_container] No pull request found") - finally: yield result self.logger.debug(f"{self.log_prefix} Deleting {clone_repo_dir}") @@ -158,23 +156,23 @@ def fix_podman_bug(self) -> None: shutil.rmtree("/tmp/storage-run-1000/containers", ignore_errors=True) shutil.rmtree("/tmp/storage-run-1000/libpod/tmp", ignore_errors=True) - def run_podman_command(self, command: str, pipe: bool = False) -> tuple[bool, str, str]: - rc, out, err = run_command(command=command, log_prefix=self.log_prefix, pipe=pipe) + async def run_podman_command(self, command: str) -> tuple[bool, str, str]: + rc, out, err = await run_command(command=command, log_prefix=self.log_prefix) if rc: return rc, out, err if self.is_podman_bug(err=err): self.fix_podman_bug() - return run_command(command=command, log_prefix=self.log_prefix, pipe=pipe) + return await run_command(command=command, log_prefix=self.log_prefix) return rc, out, err - def _run_tox(self) -> None: + async def run_tox(self, pull_request: PullRequest) -> None: if not self.github_webhook.tox: return - if self.check_run_handler.is_check_run_in_progress(check_run=TOX_STR): + if await self.check_run_handler.is_check_run_in_progress(check_run=TOX_STR): self.logger.debug(f"{self.log_prefix} Check run is in progress, re-running {TOX_STR}.") clone_repo_dir = f"{self.github_webhook.clone_repo_dir}-{uuid4()}" @@ -182,13 +180,13 @@ def _run_tox(self) -> None: f"--python={self.github_webhook.tox_python_version}" if self.github_webhook.tox_python_version else "" ) 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(self.github_webhook.pull_request_branch, "") + _tox_tests = self.github_webhook.tox.get(pull_request.base.ref, "") if _tox_tests and _tox_tests != "all": tests = _tox_tests.replace(" ", "") cmd += f" -e {tests}" - self.check_run_handler.set_run_tox_check_in_progress() - with self._prepare_cloned_repo_dir(clone_repo_dir=clone_repo_dir) as _res: + await self.check_run_handler.set_run_tox_check_in_progress() + async with self._prepare_cloned_repo_dir(clone_repo_dir=clone_repo_dir, pull_request=pull_request) as _res: output: dict[str, Any] = { "title": "Tox", "summary": "", @@ -196,28 +194,28 @@ def _run_tox(self) -> None: } if not _res[0]: output["text"] = self.check_run_handler.get_check_run_text(out=_res[1], err=_res[2]) - return self.check_run_handler.set_run_tox_check_failure(output=output) + return await self.check_run_handler.set_run_tox_check_failure(output=output) - rc, out, err = run_command(command=cmd, log_prefix=self.log_prefix) + rc, out, err = await run_command(command=cmd, log_prefix=self.log_prefix) output["text"] = self.check_run_handler.get_check_run_text(err=err, out=out) if rc: - return self.check_run_handler.set_run_tox_check_success(output=output) + return await self.check_run_handler.set_run_tox_check_success(output=output) else: - return self.check_run_handler.set_run_tox_check_failure(output=output) + return await self.check_run_handler.set_run_tox_check_failure(output=output) - def _run_pre_commit(self) -> None: + async def run_pre_commit(self, pull_request: PullRequest) -> None: if not self.github_webhook.pre_commit: return - if self.check_run_handler.is_check_run_in_progress(check_run=PRE_COMMIT_STR): + if await self.check_run_handler.is_check_run_in_progress(check_run=PRE_COMMIT_STR): self.logger.debug(f"{self.log_prefix} Check run is in progress, re-running {PRE_COMMIT_STR}.") clone_repo_dir = f"{self.github_webhook.clone_repo_dir}-{uuid4()}" cmd = f" uvx --directory {clone_repo_dir} {PRE_COMMIT_STR} run --all-files" - self.check_run_handler.set_run_pre_commit_check_in_progress() - with self._prepare_cloned_repo_dir(clone_repo_dir=clone_repo_dir) as _res: + await self.check_run_handler.set_run_pre_commit_check_in_progress() + async with self._prepare_cloned_repo_dir(pull_request=pull_request, clone_repo_dir=clone_repo_dir) as _res: output: dict[str, Any] = { "title": "Pre-Commit", "summary": "", @@ -225,19 +223,20 @@ def _run_pre_commit(self) -> None: } if not _res[0]: output["text"] = self.check_run_handler.get_check_run_text(out=_res[1], err=_res[2]) - return self.check_run_handler.set_run_pre_commit_check_failure(output=output) + return await self.check_run_handler.set_run_pre_commit_check_failure(output=output) - rc, out, err = run_command(command=cmd, log_prefix=self.log_prefix) + rc, out, err = await run_command(command=cmd, log_prefix=self.log_prefix) output["text"] = self.check_run_handler.get_check_run_text(err=err, out=out) if rc: - return self.check_run_handler.set_run_pre_commit_check_success(output=output) + return await self.check_run_handler.set_run_pre_commit_check_success(output=output) else: - return self.check_run_handler.set_run_pre_commit_check_failure(output=output) + return await self.check_run_handler.set_run_pre_commit_check_failure(output=output) - def _run_build_container( + async def run_build_container( self, + pull_request: PullRequest | None = None, set_check: bool = True, push: bool = False, is_merged: bool = False, @@ -248,21 +247,27 @@ def _run_build_container( if not self.github_webhook.build_and_push_container: return - if reviewed_user and not self._is_user_valid_to_run_commands(reviewed_user=reviewed_user): + if ( + self.owners_file_handler + and reviewed_user + and pull_request + and not await self.owners_file_handler.is_user_valid_to_run_commands( + reviewed_user=reviewed_user, pull_request=pull_request + ) + ): return - if self.check_run_handler.is_check_run_in_progress(check_run=BUILD_CONTAINER_STR): - self.logger.info(f"{self.log_prefix} Check run is in progress, re-running {BUILD_CONTAINER_STR}.") - clone_repo_dir = f"{self.github_webhook.clone_repo_dir}-{uuid4()}" - if self.pull_request and set_check: - if self.check_run_handler.is_check_run_in_progress(check_run=BUILD_CONTAINER_STR) and not is_merged: + if pull_request and set_check: + if await self.check_run_handler.is_check_run_in_progress(check_run=BUILD_CONTAINER_STR) and not is_merged: self.logger.info(f"{self.log_prefix} Check run is in progress, re-running {BUILD_CONTAINER_STR}.") - self.check_run_handler.set_container_build_in_progress() + await self.check_run_handler.set_container_build_in_progress() - _container_repository_and_tag = self.github_webhook._container_repository_and_tag(is_merged=is_merged, tag=tag) + _container_repository_and_tag = self.github_webhook.container_repository_and_tag( + pull_request=pull_request, is_merged=is_merged, tag=tag + ) no_cache: str = " --no-cache" if is_merged else "" build_cmd: str = f"--network=host {no_cache} -f {clone_repo_dir}/{self.github_webhook.dockerfile} {clone_repo_dir} -t {_container_repository_and_tag}" @@ -277,7 +282,8 @@ def _run_build_container( build_cmd = f"{command_args} {build_cmd}" podman_build_cmd: str = f"podman build {build_cmd}" - with self._prepare_cloned_repo_dir( + async with self._prepare_cloned_repo_dir( + pull_request=pull_request, is_merged=is_merged, tag_name=tag, clone_repo_dir=clone_repo_dir, @@ -289,28 +295,28 @@ def _run_build_container( } if not _res[0]: output["text"] = self.check_run_handler.get_check_run_text(out=_res[1], err=_res[2]) - if self.pull_request and set_check: - return self.check_run_handler.set_container_build_failure(output=output) + if pull_request and set_check: + return await self.check_run_handler.set_container_build_failure(output=output) - build_rc, build_out, build_err = self.run_podman_command(command=podman_build_cmd, pipe=True) + build_rc, build_out, build_err = await self.run_podman_command(command=podman_build_cmd) output["text"] = self.check_run_handler.get_check_run_text(err=build_err, out=build_out) if build_rc: self.logger.info(f"{self.log_prefix} Done building {_container_repository_and_tag}") - if self.pull_request and set_check: - return self.check_run_handler.set_container_build_success(output=output) + if pull_request and set_check: + return await self.check_run_handler.set_container_build_success(output=output) else: self.logger.error(f"{self.log_prefix} Failed to build {_container_repository_and_tag}") - if self.pull_request and set_check: - return self.check_run_handler.set_container_build_failure(output=output) + if pull_request and set_check: + return await self.check_run_handler.set_container_build_failure(output=output) if push and build_rc: cmd = f"podman push --creds {self.github_webhook.container_repository_username}:{self.github_webhook.container_repository_password} {_container_repository_and_tag}" - push_rc, _, _ = self.run_podman_command(command=cmd) + push_rc, _, _ = await self.run_podman_command(command=cmd) if push_rc: push_msg: str = f"New container for {_container_repository_and_tag} published" - if self.pull_request: - self.pull_request.create_issue_comment(push_msg) + if pull_request: + await asyncio.to_thread(pull_request.create_issue_comment, push_msg) if self.github_webhook.slack_webhook_url: message = f""" @@ -325,8 +331,8 @@ def _run_build_container( self.logger.info(f"{self.log_prefix} Done push {_container_repository_and_tag}") else: err_msg: str = f"Failed to build and push {_container_repository_and_tag}" - if self.pull_request: - self.pull_request.create_issue_comment(err_msg) + if pull_request: + await asyncio.to_thread(pull_request.create_issue_comment, err_msg) if self.github_webhook.slack_webhook_url: message = f""" @@ -338,17 +344,18 @@ def _run_build_container( message=message, webhook_url=self.github_webhook.slack_webhook_url ) - def _run_install_python_module(self) -> None: + async def run_install_python_module(self, pull_request: PullRequest) -> None: if not self.github_webhook.pypi: return - if self.check_run_handler.is_check_run_in_progress(check_run=PYTHON_MODULE_INSTALL_STR): + if await self.check_run_handler.is_check_run_in_progress(check_run=PYTHON_MODULE_INSTALL_STR): self.logger.info(f"{self.log_prefix} Check run is in progress, re-running {PYTHON_MODULE_INSTALL_STR}.") clone_repo_dir = f"{self.github_webhook.clone_repo_dir}-{uuid4()}" self.logger.info(f"{self.log_prefix} Installing python module") - self.check_run_handler.set_python_module_install_in_progress() - with self._prepare_cloned_repo_dir( + await self.check_run_handler.set_python_module_install_in_progress() + async with self._prepare_cloned_repo_dir( + pull_request=pull_request, clone_repo_dir=clone_repo_dir, ) as _res: output: dict[str, Any] = { @@ -358,9 +365,9 @@ def _run_install_python_module(self) -> None: } if not _res[0]: output["text"] = self.check_run_handler.get_check_run_text(out=_res[1], err=_res[2]) - return self.check_run_handler.set_python_module_install_failure(output=output) + return await self.check_run_handler.set_python_module_install_failure(output=output) - rc, out, err = run_command( + rc, out, err = await run_command( command=f"uvx pip wheel --no-cache-dir -w {clone_repo_dir}/dist {clone_repo_dir}", log_prefix=self.log_prefix, ) @@ -368,116 +375,49 @@ def _run_install_python_module(self) -> None: output["text"] = self.check_run_handler.get_check_run_text(err=err, out=out) if rc: - return self.check_run_handler.set_python_module_install_success(output=output) - - return self.check_run_handler.set_python_module_install_failure(output=output) + return await self.check_run_handler.set_python_module_install_success(output=output) - def _run_conventional_title_check(self) -> None: - if not self.pull_request: - return + return await self.check_run_handler.set_python_module_install_failure(output=output) + async def run_conventional_title_check(self, pull_request: PullRequest) -> None: output: dict[str, str] = { "title": "Conventional Title", "summary": "", "text": "", } - if self.check_run_handler.is_check_run_in_progress(check_run=CONVENTIONAL_TITLE_STR): + if await self.check_run_handler.is_check_run_in_progress(check_run=CONVENTIONAL_TITLE_STR): self.logger.info(f"{self.log_prefix} Check run is in progress, re-running {CONVENTIONAL_TITLE_STR}.") - self.check_run_handler.set_conventional_title_in_progress() + await self.check_run_handler.set_conventional_title_in_progress() allowed_names = self.github_webhook.conventional_title.split(",") - title = self.pull_request.title + title = pull_request.title if any([title.startswith(f"{_name}:") for _name in allowed_names]): - self.check_run_handler.set_conventional_title_success(output=output) + await self.check_run_handler.set_conventional_title_success(output=output) else: output["summary"] = "Failed" output["text"] = f"Pull request title must starts with allowed title: {': ,'.join(allowed_names)}" - self.check_run_handler.set_conventional_title_failure(output=output) - - def _is_user_valid_to_run_commands(self, reviewed_user: str) -> bool: - if not self.pull_request: - return False - - allowed_user_to_approve = self.get_all_repository_maintainers() + self.github_webhook.all_repository_approvers - allow_user_comment = f"/add-allowed-user @{reviewed_user}" - - comment_msg = f""" -{reviewed_user} is not allowed to run retest commands. -maintainers can allow it by comment `{allow_user_comment}` -Maintainers: - - {"\n - @".join(allowed_user_to_approve)} -""" - - if reviewed_user not in self.valid_users_to_run_commands: - comments_from_approvers = [ - comment.body - for comment in self.pull_request.get_issue_comments() - if comment.user.login in allowed_user_to_approve - ] - for comment in comments_from_approvers: - if allow_user_comment in comment: - return True - - self.logger.debug(f"{self.log_prefix} {reviewed_user} is not in {self.valid_users_to_run_commands}") - self.pull_request.create_issue_comment(comment_msg) - return False - - return True - - @functools.cached_property - def valid_users_to_run_commands(self) -> set[str]: - return set(( - *self.get_all_repository_contributors(), - *self.get_all_repository_collaborators(), - *self.github_webhook.all_repository_approvers, - *self.github_webhook.all_pull_request_reviewers, - )) - - def get_all_repository_maintainers(self) -> list[str]: - maintainers: list[str] = [] - - for user in self.repository_collaborators: - permmissions = user.permissions - - if permmissions.admin or permmissions.maintain: - maintainers.append(user.login) - - return maintainers + await self.check_run_handler.set_conventional_title_failure(output=output) - @functools.cached_property - def repository_collaborators(self) -> PaginatedList[NamedUser]: - return self.repository.get_collaborators() - - @functools.cached_property - def repository_contributors(self) -> PaginatedList[NamedUser]: - return self.repository.get_contributors() - - def get_all_repository_contributors(self) -> list[str]: - return [val.login for val in self.repository_contributors] - - def get_all_repository_collaborators(self) -> list[str]: - return [val.login for val in self.repository_collaborators] - - def cherry_pick(self, target_branch: str, reviewed_user: str = "") -> None: - if not self.pull_request: - return + async def is_branch_exists(self, branch: str) -> Branch: + return await asyncio.to_thread(self.repository.get_branch, branch) + async def cherry_pick(self, pull_request: PullRequest, target_branch: str, reviewed_user: str = "") -> None: requested_by = reviewed_user or "by target-branch label" self.logger.info(f"{self.log_prefix} Cherry-pick requested by user: {requested_by}") - new_branch_name = f"{CHERRY_PICKED_LABEL_PREFIX}-{self.pull_request.head.ref}-{shortuuid.uuid()[:5]}" - if not self.github_webhook.is_branch_exists(branch=target_branch): + new_branch_name = f"{CHERRY_PICKED_LABEL_PREFIX}-{pull_request.head.ref}-{shortuuid.uuid()[:5]}" + if not await self.is_branch_exists(branch=target_branch): err_msg = f"cherry-pick failed: {target_branch} does not exists" self.logger.error(err_msg) - self.pull_request.create_issue_comment(err_msg) + await asyncio.to_thread(pull_request.create_issue_comment, err_msg) else: - self.check_run_handler.set_cherry_pick_in_progress() - commit_hash = self.pull_request.merge_commit_sha - commit_msg_striped = self.pull_request.title.replace("'", "") - pull_request_url = self.pull_request.html_url + await self.check_run_handler.set_cherry_pick_in_progress() + commit_hash = pull_request.merge_commit_sha + commit_msg_striped = pull_request.title.replace("'", "") + pull_request_url = pull_request.html_url clone_repo_dir = f"{self.github_webhook.clone_repo_dir}-{uuid4()}" git_cmd = f"git --work-tree={clone_repo_dir} --git-dir={clone_repo_dir}/.git" hub_cmd = f"GITHUB_TOKEN={self.github_webhook.token} hub --work-tree={clone_repo_dir} --git-dir={clone_repo_dir}/.git" @@ -491,7 +431,7 @@ def cherry_pick(self, target_branch: str, reviewed_user: str = "") -> None: ] rc, out, err = None, "", "" - with self._prepare_cloned_repo_dir(clone_repo_dir=clone_repo_dir) as _res: + async with self._prepare_cloned_repo_dir(pull_request=pull_request, clone_repo_dir=clone_repo_dir) as _res: output = { "title": "Cherry-pick details", "summary": "", @@ -499,16 +439,17 @@ def cherry_pick(self, target_branch: str, reviewed_user: str = "") -> None: } if not _res[0]: output["text"] = self.check_run_handler.get_check_run_text(out=_res[1], err=_res[2]) - self.check_run_handler.set_cherry_pick_failure(output=output) + await self.check_run_handler.set_cherry_pick_failure(output=output) for cmd in commands: - rc, out, err = run_command(command=cmd, log_prefix=self.log_prefix) + rc, out, err = await run_command(command=cmd, log_prefix=self.log_prefix) if not rc: output["text"] = self.check_run_handler.get_check_run_text(err=err, out=out) - self.check_run_handler.set_cherry_pick_failure(output=output) + await self.check_run_handler.set_cherry_pick_failure(output=output) self.logger.error(f"{self.log_prefix} Cherry pick failed: {out} --- {err}") - local_branch_name = f"{self.pull_request.head.ref}-{target_branch}" - self.pull_request.create_issue_comment( + local_branch_name = f"{pull_request.head.ref}-{target_branch}" + await asyncio.to_thread( + pull_request.create_issue_comment, f"**Manual cherry-pick is needed**\nCherry pick failed for " f"{commit_hash} to {target_branch}:\n" f"To cherry-pick run:\n" @@ -519,11 +460,13 @@ def cherry_pick(self, target_branch: str, reviewed_user: str = "") -> None: f"git checkout -b {local_branch_name}\n" f"git cherry-pick {commit_hash}\n" f"git push origin {local_branch_name}\n" - "```" + "```", ) return output["text"] = self.check_run_handler.get_check_run_text(err=err, out=out) - self.check_run_handler.set_cherry_pick_success(output=output) - self.pull_request.create_issue_comment(f"Cherry-picked PR {self.pull_request.title} into {target_branch}") + await self.check_run_handler.set_cherry_pick_success(output=output) + await asyncio.to_thread( + pull_request.create_issue_comment, f"Cherry-picked PR {pull_request.title} into {target_branch}" + ) diff --git a/webhook_server/tests/conftest.py b/webhook_server/tests/conftest.py index a46df809..f793612e 100644 --- a/webhook_server/tests/conftest.py +++ b/webhook_server/tests/conftest.py @@ -5,17 +5,10 @@ from simple_logger.logger import logging from starlette.datastructures import Headers -from webhook_server.libs.github_api import GithubWebhook +from webhook_server.libs.owners_files_handler import OwnersFileHandler -ALL_CHANGED_FILES = [ - "OWNERS", - "folder1/OWNERS", - "code/file.py", - "README.md", - "folder2/lib.py", - "folder/folder4/another_file.txt", - "folder5/file", -] +os.environ["WEBHOOK_SERVER_DATA_DIR"] = "webhook_server/tests/manifests" +from webhook_server.libs.github_api import GithubWebhook class Tree: @@ -54,7 +47,7 @@ def __init__(self): def get_git_tree(self, sha: str, recursive: bool): return Tree("") - def get_contents(self, path: str): + def get_contents(self, path: str, ref: str): owners_data = yaml.dump({ "approvers": ["root_approver1", "root_approver2"], "reviewers": ["root_reviewer1", "root_reviewer2"], @@ -94,31 +87,56 @@ def get_contents(self, path: str): return ContentFile(folder5_owners_data) +class Label: + def __init__(self, name: str): + self.name = name + + class PullRequest: - def __init__(self): ... + def __init__(self, additions: int | None = None, deletions: int | None = None): + self.additions = additions + self.deletions = deletions + + class base: + ref = "refs/heads/main" + + def create_issue_comment(self, *args, **kwargs): ... + + def create_review_request(self, *args, **kwargs): ... + + def get_files(self): ... @pytest.fixture(scope="function") -def process_github_webhook(mocker, request): +def pull_request(): + return PullRequest() + + +@pytest.fixture(scope="function") +def github_webhook(mocker, request): base_import_path = "webhook_server.libs.github_api" - os.environ["WEBHOOK_SERVER_DATA_DIR"] = "webhook_server/tests/manifests" mocker.patch(f"{base_import_path}.get_repository_github_app_api", return_value=True) mocker.patch("github.AuthenticatedUser", return_value=True) mocker.patch(f"{base_import_path}.get_api_with_highest_rate_limit", return_value=("API", "TOKEN", "USER")) mocker.patch(f"{base_import_path}.get_github_repo_api", return_value=Repository()) + mocker.patch(f"{base_import_path}.GithubWebhook.add_api_users_to_auto_verified_and_merged_users", return_value=None) process_github_webhook = GithubWebhook( hook_data={"repository": {"name": Repository().name, "full_name": Repository().full_name}}, headers=Headers({"X-GitHub-Event": "test-event"}), logger=logging.getLogger(), ) - process_github_webhook.pull_request_branch = "main" - if hasattr(request, "param") and request.param: - process_github_webhook.changed_files = request.param[0] + owners_file_handler = OwnersFileHandler(github_webhook=process_github_webhook) + + return process_github_webhook, owners_file_handler - else: - process_github_webhook.changed_files = ALL_CHANGED_FILES - process_github_webhook.pull_request = PullRequest() - return process_github_webhook +@pytest.fixture(scope="function") +def process_github_webhook(github_webhook): + return github_webhook[0] + + +@pytest.fixture(scope="function") +def owners_file_handler(github_webhook): + return github_webhook[1] diff --git a/webhook_server/tests/test_add_reviewer_action.py b/webhook_server/tests/test_add_reviewer_action.py index 68d1c24f..096aa82a 100644 --- a/webhook_server/tests/test_add_reviewer_action.py +++ b/webhook_server/tests/test_add_reviewer_action.py @@ -1,5 +1,7 @@ import logging +import pytest + from webhook_server.libs.issue_comment_handler import IssueCommentHandler @@ -27,19 +29,27 @@ def create_review_request(self, _): return -def test_add_reviewer_by_user_comment(caplog, process_github_webhook): +@pytest.mark.asyncio +async def test_add_reviewer_by_user_comment(caplog, process_github_webhook, owners_file_handler, pull_request): process_github_webhook.repository = Repository() process_github_webhook.pull_request = PullRequest() - issue_comment_handler = IssueCommentHandler(github_webhook=process_github_webhook) - issue_comment_handler._add_reviewer_by_user_comment("user1") + issue_comment_handler = IssueCommentHandler( + github_webhook=process_github_webhook, owners_file_handler=owners_file_handler + ) + await issue_comment_handler._add_reviewer_by_user_comment(pull_request=pull_request, reviewer="user1") caplog.set_level(logging.DEBUG) assert "Adding reviewer user1 by user comment" in caplog.text -def test_add_reviewer_by_user_comment_invalid_user(caplog, process_github_webhook): +@pytest.mark.asyncio +async def test_add_reviewer_by_user_comment_invalid_user( + caplog, process_github_webhook, owners_file_handler, pull_request +): process_github_webhook.repository = Repository() process_github_webhook.pull_request = PullRequest() - issue_comment_handler = IssueCommentHandler(github_webhook=process_github_webhook) - issue_comment_handler._add_reviewer_by_user_comment("user2") + issue_comment_handler = IssueCommentHandler( + github_webhook=process_github_webhook, owners_file_handler=owners_file_handler + ) + await issue_comment_handler._add_reviewer_by_user_comment(pull_request=pull_request, reviewer="user2") caplog.set_level(logging.DEBUG) assert "not adding reviewer user2 by user comment, user2 is not part of contributers" in caplog.text diff --git a/webhook_server/tests/test_prepare_retest_wellcome_comment.py b/webhook_server/tests/test_prepare_retest_wellcome_comment.py index 48f29a55..6119ed85 100644 --- a/webhook_server/tests/test_prepare_retest_wellcome_comment.py +++ b/webhook_server/tests/test_prepare_retest_wellcome_comment.py @@ -52,7 +52,15 @@ class TestPrepareRetestWellcomeMsg: ], ) def test_prepare_retest_wellcome_comment( - self, process_github_webhook, tox, build_and_push_container, pypi, pre_commit, conventional_title, expected + self, + process_github_webhook, + owners_file_handler, + tox, + build_and_push_container, + pypi, + pre_commit, + conventional_title, + expected, ): process_github_webhook.tox = tox process_github_webhook.build_and_push_container = build_and_push_container @@ -60,6 +68,8 @@ def test_prepare_retest_wellcome_comment( process_github_webhook.pre_commit = pre_commit process_github_webhook.conventional_title = conventional_title process_github_webhook.pull_request = None - pull_request_handler = PullRequestHandler(github_webhook=process_github_webhook) + pull_request_handler = PullRequestHandler( + github_webhook=process_github_webhook, owners_file_handler=owners_file_handler + ) assert pull_request_handler._prepare_retest_welcome_comment == expected diff --git a/webhook_server/tests/test_pull_request_owners.py b/webhook_server/tests/test_pull_request_owners.py index d798f9e4..d99a8074 100644 --- a/webhook_server/tests/test_pull_request_owners.py +++ b/webhook_server/tests/test_pull_request_owners.py @@ -5,6 +5,16 @@ from webhook_server.tests.conftest import ContentFile, Tree from webhook_server.utils.constants import APPROVED_BY_LABEL_PREFIX +ALL_CHANGED_FILES = [ + "OWNERS", + "folder1/OWNERS", + "code/file.py", + "README.md", + "folder2/lib.py", + "folder/folder4/another_file.txt", + "folder5/file", +] + class Repository: def __init__(self): @@ -13,7 +23,7 @@ def __init__(self): def get_git_tree(self, sha: str, recursive: bool): return Tree("") - def get_contents(self, path: str, ref): + def get_contents(self, path: str, ref: str): owners_data = yaml.dump({ "approvers": ["root_approver1", "root_approver2"], "reviewers": ["root_reviewer1", "root_reviewer2"], @@ -54,8 +64,17 @@ def get_contents(self, path: str, ref): @pytest.fixture(scope="function") -def all_repository_approvers_and_reviewers(process_github_webhook): - process_github_webhook.all_repository_approvers_and_reviewers = { +def changed_files(request, owners_file_handler): + if hasattr(request, "param") and request.param: + owners_file_handler.changed_files = request.param[0] + + else: + owners_file_handler.changed_files = ALL_CHANGED_FILES + + +@pytest.fixture(scope="function") +def all_repository_approvers_and_reviewers(owners_file_handler): + owners_file_handler.all_repository_approvers_and_reviewers = { ".": {"approvers": ["root_approver1", "root_approver2"], "reviewers": ["root_reviewer1", "root_reviewer2"]}, "folder1": { "approvers": ["folder1_approver1", "folder1_approver2"], @@ -75,8 +94,8 @@ def all_repository_approvers_and_reviewers(process_github_webhook): @pytest.fixture(scope="function") -def all_approvers_reviewers(process_github_webhook): - process_github_webhook.all_pull_request_approvers = [ +def all_approvers_reviewers(owners_file_handler): + owners_file_handler.all_pull_request_approvers = [ "folder1_approver1", "folder1_approver2", "folder4_approver1", @@ -87,9 +106,9 @@ def all_approvers_reviewers(process_github_webhook): "root_approver2", ] - process_github_webhook.all_pull_request_approvers.sort() + owners_file_handler.all_pull_request_approvers.sort() - process_github_webhook.all_pull_request_reviewers = [ + owners_file_handler.all_pull_request_reviewers = [ "folder1_reviewer1", "folder1_reviewer2", "folder4_reviewer1", @@ -100,17 +119,23 @@ def all_approvers_reviewers(process_github_webhook): "root_reviewer2", ] - process_github_webhook.all_pull_request_reviewers.sort() + owners_file_handler.all_pull_request_reviewers.sort() -def test_get_all_repository_approvers_and_reviewers(process_github_webhook, all_repository_approvers_and_reviewers): +@pytest.mark.asyncio +async def test_get_all_repository_approvers_and_reviewers( + changed_files, process_github_webhook, owners_file_handler, pull_request, all_repository_approvers_and_reviewers +): process_github_webhook.repository = Repository() - read_owners_result = process_github_webhook.get_all_repository_approvers_and_reviewers() - assert read_owners_result == process_github_webhook.all_repository_approvers_and_reviewers + read_owners_result = await owners_file_handler.get_all_repository_approvers_and_reviewers(pull_request=pull_request) + assert read_owners_result == owners_file_handler.all_repository_approvers_and_reviewers -def test_owners_data_for_changed_files(process_github_webhook, all_repository_approvers_and_reviewers): - owners_data_chaged_files_result = process_github_webhook.owners_data_for_changed_files() +@pytest.mark.asyncio +async def test_owners_data_for_changed_files( + changed_files, process_github_webhook, owners_file_handler, all_repository_approvers_and_reviewers +): + owners_data_changed_files_result = await owners_file_handler.owners_data_for_changed_files() owners_data_chaged_files_expected = { "folder5": { "approvers": ["folder5_approver1", "folder5_approver2"], @@ -129,24 +154,39 @@ def test_owners_data_for_changed_files(process_github_webhook, all_repository_ap }, } - assert owners_data_chaged_files_result == owners_data_chaged_files_expected + assert owners_data_changed_files_result == owners_data_chaged_files_expected -def test_all_approvers_reviewers( - process_github_webhook, all_repository_approvers_and_reviewers, all_approvers_reviewers +@pytest.mark.asyncio +async def test_all_approvers_reviewers( + changed_files, + process_github_webhook, + owners_file_handler, + all_repository_approvers_and_reviewers, + all_approvers_reviewers, ): - all_approvers = process_github_webhook.get_all_pull_request_approvers() - assert all_approvers == process_github_webhook.all_pull_request_approvers + all_approvers = await owners_file_handler.get_all_pull_request_approvers() + assert all_approvers == owners_file_handler.all_pull_request_approvers - all_pull_request_reviewers = process_github_webhook.get_all_pull_request_reviewers() - assert all_pull_request_reviewers == process_github_webhook.all_pull_request_reviewers + all_pull_request_reviewers = await owners_file_handler.get_all_pull_request_reviewers() + assert all_pull_request_reviewers == owners_file_handler.all_pull_request_reviewers -def test_check_pr_approved(process_github_webhook, all_repository_approvers_and_reviewers, all_approvers_reviewers): - process_github_webhook.all_pull_request_approvers = process_github_webhook.get_all_pull_request_approvers() - pull_request_handler = PullRequestHandler(github_webhook=process_github_webhook) - pull_request_handler.parent_committer = "test" - check_if_pr_approved = pull_request_handler._check_if_pr_approved( +@pytest.mark.asyncio +async def test_check_pr_approved( + changed_files, + process_github_webhook, + owners_file_handler, + all_repository_approvers_and_reviewers, + all_approvers_reviewers, +): + owners_file_handler.all_pull_request_approvers = await owners_file_handler.get_all_pull_request_approvers() + pull_request_handler = PullRequestHandler( + github_webhook=process_github_webhook, + owners_file_handler=owners_file_handler, + ) + process_github_webhook.parent_committer = "test" + check_if_pr_approved = await pull_request_handler._check_if_pr_approved( labels=[ f"{APPROVED_BY_LABEL_PREFIX}root_approver1", f"{APPROVED_BY_LABEL_PREFIX}root_approver2", @@ -161,13 +201,20 @@ def test_check_pr_approved(process_github_webhook, all_repository_approvers_and_ assert check_if_pr_approved == "" -def test_check_pr_minimum_approved( - process_github_webhook, all_repository_approvers_and_reviewers, all_approvers_reviewers +@pytest.mark.asyncio +async def test_check_pr_minimum_approved( + changed_files, + process_github_webhook, + owners_file_handler, + all_repository_approvers_and_reviewers, + all_approvers_reviewers, ): - process_github_webhook.all_pull_request_approvers = process_github_webhook.get_all_pull_request_approvers() - pull_request_handler = PullRequestHandler(github_webhook=process_github_webhook) - pull_request_handler.parent_committer = "test" - check_if_pr_approved = pull_request_handler._check_if_pr_approved( + owners_file_handler.all_pull_request_approvers = await owners_file_handler.get_all_pull_request_approvers() + pull_request_handler = PullRequestHandler( + github_webhook=process_github_webhook, + owners_file_handler=owners_file_handler, + ) + check_if_pr_approved = await pull_request_handler._check_if_pr_approved( labels=[ f"{APPROVED_BY_LABEL_PREFIX}root_approver1", f"{APPROVED_BY_LABEL_PREFIX}folder1_approver1", @@ -178,20 +225,29 @@ def test_check_pr_minimum_approved( assert check_if_pr_approved == "" -def test_check_pr_not_approved(process_github_webhook, all_repository_approvers_and_reviewers, all_approvers_reviewers): - process_github_webhook.all_pull_request_approvers = process_github_webhook.get_all_pull_request_approvers() - pull_request_handler = PullRequestHandler(github_webhook=process_github_webhook) - pull_request_handler.parent_committer = "test" - check_if_pr_approved = pull_request_handler._check_if_pr_approved( +@pytest.mark.asyncio +async def test_check_pr_not_approved( + changed_files, + process_github_webhook, + owners_file_handler, + all_repository_approvers_and_reviewers, + all_approvers_reviewers, +): + owners_file_handler.all_pull_request_approvers = await owners_file_handler.get_all_pull_request_approvers() + pull_request_handler = PullRequestHandler( + github_webhook=process_github_webhook, + owners_file_handler=owners_file_handler, + ) + check_if_pr_approved = await pull_request_handler._check_if_pr_approved( labels=[ - f"{APPROVED_BY_LABEL_PREFIX}root_approver1", + f"{APPROVED_BY_LABEL_PREFIX}folder1_approver1", ] ) missing_approvers = [appr.strip() for appr in check_if_pr_approved.split(":")[-1].strip().split(",")] missing_approvers.sort() expected_approvers = [ - "folder1_approver1", - "folder1_approver2", + "root_approver1", + "root_approver2", "folder4_approver1", "folder4_approver2", "folder5_approver1", @@ -201,25 +257,29 @@ def test_check_pr_not_approved(process_github_webhook, all_repository_approvers_ assert missing_approvers == expected_approvers -def test_check_pr_partial_approved( - process_github_webhook, all_repository_approvers_and_reviewers, all_approvers_reviewers +@pytest.mark.asyncio +async def test_check_pr_partial_approved( + changed_files, + process_github_webhook, + owners_file_handler, + all_repository_approvers_and_reviewers, + all_approvers_reviewers, ): - process_github_webhook.all_pull_request_approvers = process_github_webhook.get_all_pull_request_approvers() - pull_request_handler = PullRequestHandler(github_webhook=process_github_webhook) - pull_request_handler.parent_committer = "test" - check_if_pr_approved = pull_request_handler._check_if_pr_approved( + owners_file_handler.all_pull_request_approvers = await owners_file_handler.get_all_pull_request_approvers() + pull_request_handler = PullRequestHandler( + github_webhook=process_github_webhook, owners_file_handler=owners_file_handler + ) + check_if_pr_approved = await pull_request_handler._check_if_pr_approved( labels=[ - f"{APPROVED_BY_LABEL_PREFIX}root_approver1", - f"{APPROVED_BY_LABEL_PREFIX}root_approver2", + f"{APPROVED_BY_LABEL_PREFIX}folder1_approver1", + f"{APPROVED_BY_LABEL_PREFIX}folder4_approver2", ] ) missing_approvers = [appr.strip() for appr in check_if_pr_approved.split(":")[-1].strip().split(",")] missing_approvers.sort() expected_approvers = [ - "folder1_approver1", - "folder1_approver2", - "folder4_approver1", - "folder4_approver2", + "root_approver1", + "root_approver2", "folder5_approver1", "folder5_approver2", ] @@ -228,7 +288,7 @@ def test_check_pr_partial_approved( @pytest.mark.parametrize( - "process_github_webhook", + "changed_files", [ pytest.param([ [ @@ -240,13 +300,19 @@ def test_check_pr_partial_approved( ], indirect=True, ) -def test_check_pr_approved_specific_folder( - process_github_webhook, all_repository_approvers_and_reviewers, all_approvers_reviewers +@pytest.mark.asyncio +async def test_check_pr_approved_specific_folder( + changed_files, + process_github_webhook, + owners_file_handler, + all_repository_approvers_and_reviewers, + all_approvers_reviewers, ): - process_github_webhook.all_pull_request_approvers = process_github_webhook.get_all_pull_request_approvers() - pull_request_handler = PullRequestHandler(github_webhook=process_github_webhook) - pull_request_handler.parent_committer = "test" - check_if_pr_approved = pull_request_handler._check_if_pr_approved( + owners_file_handler.all_pull_request_approvers = await owners_file_handler.get_all_pull_request_approvers() + pull_request_handler = PullRequestHandler( + github_webhook=process_github_webhook, owners_file_handler=owners_file_handler + ) + check_if_pr_approved = await pull_request_handler._check_if_pr_approved( labels=[ f"{APPROVED_BY_LABEL_PREFIX}root_approver1", f"{APPROVED_BY_LABEL_PREFIX}folder4_approver1", @@ -256,7 +322,7 @@ def test_check_pr_approved_specific_folder( @pytest.mark.parametrize( - "process_github_webhook", + "changed_files", [ pytest.param([ [ @@ -268,13 +334,19 @@ def test_check_pr_approved_specific_folder( ], indirect=True, ) -def test_check_pr_approved_nested_folder_no_owners( - process_github_webhook, all_repository_approvers_and_reviewers, all_approvers_reviewers +@pytest.mark.asyncio +async def test_check_pr_approved_nested_folder_no_owners( + changed_files, + process_github_webhook, + owners_file_handler, + all_repository_approvers_and_reviewers, + all_approvers_reviewers, ): - process_github_webhook.all_pull_request_approvers = process_github_webhook.get_all_pull_request_approvers() - pull_request_handler = PullRequestHandler(github_webhook=process_github_webhook) - pull_request_handler.parent_committer = "test" - check_if_pr_approved = pull_request_handler._check_if_pr_approved( + owners_file_handler.all_pull_request_approvers = await owners_file_handler.get_all_pull_request_approvers() + pull_request_handler = PullRequestHandler( + github_webhook=process_github_webhook, owners_file_handler=owners_file_handler + ) + check_if_pr_approved = await pull_request_handler._check_if_pr_approved( labels=[ f"{APPROVED_BY_LABEL_PREFIX}root_approver1", ] @@ -283,7 +355,7 @@ def test_check_pr_approved_nested_folder_no_owners( @pytest.mark.parametrize( - "process_github_webhook", + "changed_files", [ pytest.param([ [ @@ -293,13 +365,19 @@ def test_check_pr_approved_nested_folder_no_owners( ], indirect=True, ) -def test_check_pr_approved_specific_folder_with_root_approvers( - process_github_webhook, all_repository_approvers_and_reviewers, all_approvers_reviewers +@pytest.mark.asyncio +async def test_check_pr_approved_specific_folder_with_root_approvers( + changed_files, + process_github_webhook, + owners_file_handler, + all_repository_approvers_and_reviewers, + all_approvers_reviewers, ): - process_github_webhook.all_pull_request_approvers = process_github_webhook.get_all_pull_request_approvers() - pull_request_handler = PullRequestHandler(github_webhook=process_github_webhook) - pull_request_handler.parent_committer = "test" - check_if_pr_approved = pull_request_handler._check_if_pr_approved( + owners_file_handler.all_pull_request_approvers = await owners_file_handler.get_all_pull_request_approvers() + pull_request_handler = PullRequestHandler( + github_webhook=process_github_webhook, owners_file_handler=owners_file_handler + ) + check_if_pr_approved = await pull_request_handler._check_if_pr_approved( labels=[ f"{APPROVED_BY_LABEL_PREFIX}folder1_approver1", ] @@ -315,7 +393,7 @@ def test_check_pr_approved_specific_folder_with_root_approvers( @pytest.mark.parametrize( - "process_github_webhook", + "changed_files", [ pytest.param([ [ @@ -325,13 +403,19 @@ def test_check_pr_approved_specific_folder_with_root_approvers( ], indirect=True, ) -def test_check_pr_approved_specific_folder_no_root_approvers( - process_github_webhook, all_repository_approvers_and_reviewers, all_approvers_reviewers +@pytest.mark.asyncio +async def test_check_pr_approved_specific_folder_no_root_approvers( + changed_files, + process_github_webhook, + owners_file_handler, + all_repository_approvers_and_reviewers, + all_approvers_reviewers, ): - process_github_webhook.all_pull_request_approvers = process_github_webhook.get_all_pull_request_approvers() - pull_request_handler = PullRequestHandler(github_webhook=process_github_webhook) - pull_request_handler.parent_committer = "test" - check_if_pr_approved = pull_request_handler._check_if_pr_approved( + owners_file_handler.all_pull_request_approvers = await owners_file_handler.get_all_pull_request_approvers() + pull_request_handler = PullRequestHandler( + github_webhook=process_github_webhook, owners_file_handler=owners_file_handler + ) + check_if_pr_approved = await pull_request_handler._check_if_pr_approved( labels=[ f"{APPROVED_BY_LABEL_PREFIX}folder5_approver1", ] @@ -340,7 +424,7 @@ def test_check_pr_approved_specific_folder_no_root_approvers( @pytest.mark.parametrize( - "process_github_webhook", + "changed_files", [ pytest.param([ [ @@ -350,13 +434,19 @@ def test_check_pr_approved_specific_folder_no_root_approvers( ], indirect=True, ) -def test_check_pr_not_approved_specific_folder_without_owners( - process_github_webhook, all_repository_approvers_and_reviewers, all_approvers_reviewers +@pytest.mark.asyncio +async def test_check_pr_not_approved_specific_folder_without_owners( + changed_files, + process_github_webhook, + owners_file_handler, + all_repository_approvers_and_reviewers, + all_approvers_reviewers, ): - process_github_webhook.all_pull_request_approvers = process_github_webhook.get_all_pull_request_approvers() - pull_request_handler = PullRequestHandler(github_webhook=process_github_webhook) - pull_request_handler.parent_committer = "test" - check_if_pr_approved = pull_request_handler._check_if_pr_approved( + owners_file_handler.all_pull_request_approvers = await owners_file_handler.get_all_pull_request_approvers() + pull_request_handler = PullRequestHandler( + github_webhook=process_github_webhook, owners_file_handler=owners_file_handler + ) + check_if_pr_approved = await pull_request_handler._check_if_pr_approved( labels=[ f"{APPROVED_BY_LABEL_PREFIX}folder5_approver1", ] @@ -372,7 +462,7 @@ def test_check_pr_not_approved_specific_folder_without_owners( @pytest.mark.parametrize( - "process_github_webhook", + "changed_files", [ pytest.param([ [ @@ -382,13 +472,19 @@ def test_check_pr_not_approved_specific_folder_without_owners( ], indirect=True, ) -def test_check_pr_approved_specific_folder_without_owners( - process_github_webhook, all_repository_approvers_and_reviewers, all_approvers_reviewers +@pytest.mark.asyncio +async def test_check_pr_approved_specific_folder_without_owners( + changed_files, + process_github_webhook, + owners_file_handler, + all_repository_approvers_and_reviewers, + all_approvers_reviewers, ): - process_github_webhook.all_pull_request_approvers = process_github_webhook.get_all_pull_request_approvers() - pull_request_handler = PullRequestHandler(github_webhook=process_github_webhook) - pull_request_handler.parent_committer = "test" - check_if_pr_approved = pull_request_handler._check_if_pr_approved( + owners_file_handler.all_pull_request_approvers = await owners_file_handler.get_all_pull_request_approvers() + pull_request_handler = PullRequestHandler( + github_webhook=process_github_webhook, owners_file_handler=owners_file_handler + ) + check_if_pr_approved = await pull_request_handler._check_if_pr_approved( labels=[ f"{APPROVED_BY_LABEL_PREFIX}root_approver1", ] @@ -397,7 +493,7 @@ def test_check_pr_approved_specific_folder_without_owners( @pytest.mark.parametrize( - "process_github_webhook", + "changed_files", [ pytest.param([ [ @@ -408,13 +504,19 @@ def test_check_pr_approved_specific_folder_without_owners( ], indirect=True, ) -def test_check_pr_approved_folder_with_no_owners_and_folder_without_root_approvers( - process_github_webhook, all_repository_approvers_and_reviewers, all_approvers_reviewers +@pytest.mark.asyncio +async def test_check_pr_approved_folder_with_no_owners_and_folder_without_root_approvers( + changed_files, + process_github_webhook, + owners_file_handler, + all_repository_approvers_and_reviewers, + all_approvers_reviewers, ): - process_github_webhook.all_pull_request_approvers = process_github_webhook.get_all_pull_request_approvers() - pull_request_handler = PullRequestHandler(github_webhook=process_github_webhook) - pull_request_handler.parent_committer = "test" - check_if_pr_approved = pull_request_handler._check_if_pr_approved( + owners_file_handler.all_pull_request_approvers = await owners_file_handler.get_all_pull_request_approvers() + pull_request_handler = PullRequestHandler( + github_webhook=process_github_webhook, owners_file_handler=owners_file_handler + ) + check_if_pr_approved = await pull_request_handler._check_if_pr_approved( labels=[ f"{APPROVED_BY_LABEL_PREFIX}root_approver1", f"{APPROVED_BY_LABEL_PREFIX}folder5_approver1", @@ -424,7 +526,7 @@ def test_check_pr_approved_folder_with_no_owners_and_folder_without_root_approve @pytest.mark.parametrize( - "process_github_webhook", + "changed_files", [ pytest.param([ [ @@ -435,13 +537,19 @@ def test_check_pr_approved_folder_with_no_owners_and_folder_without_root_approve ], indirect=True, ) -def test_check_pr_not_approved_folder_with_no_owners_and_folder_without_root_approvers( - process_github_webhook, all_repository_approvers_and_reviewers, all_approvers_reviewers +@pytest.mark.asyncio +async def test_check_pr_not_approved_folder_with_no_owners_and_folder_without_root_approvers( + changed_files, + process_github_webhook, + owners_file_handler, + all_repository_approvers_and_reviewers, + all_approvers_reviewers, ): - process_github_webhook.all_pull_request_approvers = process_github_webhook.get_all_pull_request_approvers() - pull_request_handler = PullRequestHandler(github_webhook=process_github_webhook) - pull_request_handler.parent_committer = "test" - check_if_pr_approved = pull_request_handler._check_if_pr_approved( + owners_file_handler.all_pull_request_approvers = await owners_file_handler.get_all_pull_request_approvers() + pull_request_handler = PullRequestHandler( + github_webhook=process_github_webhook, owners_file_handler=owners_file_handler + ) + check_if_pr_approved = await pull_request_handler._check_if_pr_approved( labels=[ f"{APPROVED_BY_LABEL_PREFIX}folder5_approver1", ] @@ -457,39 +565,7 @@ def test_check_pr_not_approved_folder_with_no_owners_and_folder_without_root_app @pytest.mark.parametrize( - "process_github_webhook", - [ - pytest.param([ - [ - "folder1/sub_folder1/file", - ] - ]) - ], - indirect=True, -) -def test_check_pr_not_approved_subfolder_with_owners( - process_github_webhook, all_repository_approvers_and_reviewers, all_approvers_reviewers -): - process_github_webhook.all_pull_request_approvers = process_github_webhook.get_all_pull_request_approvers() - pull_request_handler = PullRequestHandler(github_webhook=process_github_webhook) - pull_request_handler.parent_committer = "test" - check_if_pr_approved = pull_request_handler._check_if_pr_approved( - labels=[ - f"{APPROVED_BY_LABEL_PREFIX}root_approver1", - ] - ) - missing_approvers = [appr.strip() for appr in check_if_pr_approved.split(":")[-1].strip().split(",")] - missing_approvers.sort() - expected_approvers = [ - "folder1_approver1", - "folder1_approver2", - ] - expected_approvers.sort() - assert missing_approvers == expected_approvers - - -@pytest.mark.parametrize( - "process_github_webhook", + "changed_files", [ pytest.param([ [ @@ -499,13 +575,19 @@ def test_check_pr_not_approved_subfolder_with_owners( ], indirect=True, ) -def test_check_pr_approved_subfolder_with_owners_no_root_approvers( - process_github_webhook, all_repository_approvers_and_reviewers, all_approvers_reviewers +@pytest.mark.asyncio +async def test_check_pr_approved_subfolder_with_owners_no_root_approvers( + changed_files, + process_github_webhook, + owners_file_handler, + all_repository_approvers_and_reviewers, + all_approvers_reviewers, ): - process_github_webhook.all_pull_request_approvers = process_github_webhook.get_all_pull_request_approvers() - pull_request_handler = PullRequestHandler(github_webhook=process_github_webhook) - pull_request_handler.parent_committer = "test" - check_if_pr_approved = pull_request_handler._check_if_pr_approved( + owners_file_handler.all_pull_request_approvers = await owners_file_handler.get_all_pull_request_approvers() + pull_request_handler = PullRequestHandler( + github_webhook=process_github_webhook, owners_file_handler=owners_file_handler + ) + check_if_pr_approved = await pull_request_handler._check_if_pr_approved( labels=[ f"{APPROVED_BY_LABEL_PREFIX}folder5_approver1", ] diff --git a/webhook_server/tests/test_pull_request_size.py b/webhook_server/tests/test_pull_request_size.py index 3278e2ce..e0728e2d 100644 --- a/webhook_server/tests/test_pull_request_size.py +++ b/webhook_server/tests/test_pull_request_size.py @@ -1,25 +1,10 @@ import pytest from webhook_server.libs.labels_handler import LabelsHandler +from webhook_server.tests.conftest import PullRequest from webhook_server.utils.constants import SIZE_LABEL_PREFIX -class Label: - def __init__(self, name: str): - self.name = name - - -class PullRequest: - def __init__(self, additions: int, deletions: int, labels: list[str] | None = None): - self.additions = additions - self.deletions = deletions - self.labels = labels or [] - - @property - def lables(self) -> list[Label]: - return [Label(label) for label in self.labels] - - @pytest.mark.parametrize( "additions, deletions, expected_label", [ @@ -32,9 +17,9 @@ def lables(self) -> list[Label]: (1000, 1, "XXL"), ], ) -def test_get_size_thresholds(process_github_webhook, additions, deletions, expected_label): - process_github_webhook.pull_request = PullRequest(additions=additions, deletions=deletions) - lables_handler = LabelsHandler(github_webhook=process_github_webhook) - result = lables_handler.get_size() +def test_get_size_thresholds(process_github_webhook, owners_file_handler, additions, deletions, expected_label): + pull_request = PullRequest(additions=additions, deletions=deletions) + lables_handler = LabelsHandler(github_webhook=process_github_webhook, owners_file_handler=owners_file_handler) + result = lables_handler.get_size(pull_request=pull_request) assert result == f"{SIZE_LABEL_PREFIX}{expected_label}" diff --git a/webhook_server/utils/github_repository_and_webhook_settings.py b/webhook_server/utils/github_repository_and_webhook_settings.py index 5f1d0086..8d6fd2c8 100644 --- a/webhook_server/utils/github_repository_and_webhook_settings.py +++ b/webhook_server/utils/github_repository_and_webhook_settings.py @@ -20,7 +20,7 @@ def get_repository_api(repository: str) -> tuple[str, github.Github | None, str] return repository, github_api, api_user -def repository_and_webhook_settings(webhook_secret: str | None = None) -> None: +async def repository_and_webhook_settings(webhook_secret: str | None = None) -> None: config = Config(logger=LOGGER) apis_dict: dict[str, dict[str, Any]] = {} @@ -40,6 +40,6 @@ def repository_and_webhook_settings(webhook_secret: str | None = None) -> None: LOGGER.debug(f"Repositories APIs: {apis_dict}") - set_repositories_settings(config=config, apis_dict=apis_dict) + await set_repositories_settings(config=config, apis_dict=apis_dict) set_all_in_progress_check_runs_to_queued(repo_config=config, apis_dict=apis_dict) create_webhook(config=config, apis_dict=apis_dict, secret=webhook_secret) diff --git a/webhook_server/utils/github_repository_settings.py b/webhook_server/utils/github_repository_settings.py index df72694c..f4e44c93 100644 --- a/webhook_server/utils/github_repository_settings.py +++ b/webhook_server/utils/github_repository_settings.py @@ -60,7 +60,6 @@ def set_branch_protection( branch: Branch, repository: Repository, required_status_checks: list[str], - github_api: Github, strict: bool, require_code_owner_reviews: bool, dismiss_stale_reviews: bool, @@ -192,7 +191,7 @@ def get_repo_branch_protection_rules(config: Config) -> dict[str, Any]: return branch_protection -def set_repositories_settings(config: Config, apis_dict: dict[str, dict[str, Any]]) -> None: +async def set_repositories_settings(config: Config, apis_dict: dict[str, dict[str, Any]]) -> None: LOGGER.info("Processing repositories") config_data = config.root_data @@ -201,9 +200,7 @@ def set_repositories_settings(config: Config, apis_dict: dict[str, dict[str, Any LOGGER.info("Login in to docker.io") docker_username: str = docker["username"] docker_password: str = docker["password"] - run_command( - log_prefix="", command=f"podman login -u {docker_username} -p {docker_password} docker.io", check=True - ) + await run_command(log_prefix="", command=f"podman login -u {docker_username} -p {docker_password} docker.io") futures = [] with ThreadPoolExecutor() as executor: @@ -292,7 +289,6 @@ def set_repository( "branch": branch, "repository": repo, "required_status_checks": required_status_checks, - "github_api": github_api, "api_user": api_user, }, **branch_protection, @@ -380,6 +376,7 @@ def set_repository_check_runs_to_queued( def get_repository_github_app_api(config_: Config, repository_name: str) -> Github | None: LOGGER.debug("Getting repositories GitHub app API") + with open(os.path.join(config_.data_dir, "webhook-server.private-key.pem")) as fd: private_key = fd.read() @@ -389,11 +386,14 @@ def get_repository_github_app_api(config_: Config, repository_name: str) -> Gith owner: str repo: str owner, repo = repository_name.split("/") + try: return app_instance.get_repo_installation(owner=owner, repo=repo).get_github_for_installation() - except UnknownObjectException: + + except Exception: LOGGER.error( f"Repository {repository_name} not found by manage-repositories-app, " f"make sure the app installed (https://github.com/apps/manage-repositories-app)" ) + return None diff --git a/webhook_server/utils/helpers.py b/webhook_server/utils/helpers.py index b325b9cd..7b13c003 100644 --- a/webhook_server/utils/helpers.py +++ b/webhook_server/utils/helpers.py @@ -1,5 +1,6 @@ from __future__ import annotations +import asyncio import datetime import os import shlex @@ -15,27 +16,44 @@ from simple_logger.logger import get_logger from webhook_server.libs.config import Config +from webhook_server.libs.exceptions import NoApiTokenError def get_logger_with_params( - name: str, repository_name: str = "", mask_sensitive: bool = False, mask_sensitive_patterns: list[str] | None = None + name: str, + repository_name: str = "", + mask_sensitive: bool = True, ) -> Logger: + mask_sensitive_patterns: list[str] = [ + "container_repository_password", + "-p", + "password", + "token", + "apikey", + "secret", + ] + _config = Config(repository=repository_name) log_level: str = _config.get_value(value="log-level", return_on_none="INFO") log_file: str = _config.get_value(value="log-file") if log_file and not log_file.startswith("/"): - log_file = os.path.join(_config.data_dir, "logs", log_file) + log_file_path = os.path.join(_config.data_dir, "logs") + + if not os.path.isdir(log_file_path): + os.makedirs(log_file_path, exist_ok=True) + + log_file = os.path.join(log_file_path, log_file) return get_logger( name=name, filename=log_file, level=log_level, - file_max_bytes=1048576 * 50, + file_max_bytes=1024 * 1024 * 10, mask_sensitive=mask_sensitive, mask_sensitive_patterns=mask_sensitive_patterns, - ) # 50MB + ) def extract_key_from_dict(key: Any, _dict: dict[Any, Any]) -> Any: @@ -52,19 +70,17 @@ def extract_key_from_dict(key: Any, _dict: dict[Any, Any]) -> Any: yield result -def get_github_repo_api(github_api: github.Github, repository: int | str) -> Repository: - return github_api.get_repo(repository) +def get_github_repo_api(github_app_api: github.Github, repository: int | str) -> Repository: + logger = get_logger_with_params(name="helpers") + logger.debug(f"Get GitHub API for repository {repository}") + + return github_app_api.get_repo(repository) -def run_command( +async def run_command( command: str, log_prefix: str, verify_stderr: bool = False, - shell: bool = False, - timeout: int | None = None, - capture_output: bool = True, - check: bool = False, - pipe: bool = False, **kwargs: Any, ) -> tuple[bool, Any, Any]: """ @@ -74,47 +90,28 @@ def run_command( command (str): Command to run log_prefix (str): Prefix for log messages verify_stderr (bool, default False): Check command stderr - shell (bool, default False): run subprocess with shell toggle - timeout (int, optional): Command wait timeout - capture_output (bool, default True): Capture command output - check (bool, default False): If check is True and the exit code was non-zero, it raises a - CalledProcessError - pipe (bool, default False): If pipe is True, text and capture_output would be set to False. stdout and - stderr, would be set to subprocess.PIPE and passed to subprocess.run call - Returns: tuple: True, out if command succeeded, False, err otherwise. """ - mask_sensitive_patterns: list[str] = ["-p", "password", "token", "apikey", "secret"] - logger = get_logger_with_params( - name="helpers", mask_sensitive=True, mask_sensitive_patterns=mask_sensitive_patterns - ) - text = True + logger = get_logger_with_params(name="helpers") out_decoded: str = "" err_decoded: str = "" - if pipe: - capture_output = False - text = False - kwargs["stdout"] = subprocess.PIPE - kwargs["stderr"] = subprocess.PIPE + kwargs["stdout"] = subprocess.PIPE + kwargs["stderr"] = subprocess.PIPE + try: logger.debug(f"{log_prefix} Running '{command}' command") - sub_process = subprocess.run( - shlex.split(command), - capture_output=capture_output, - check=check, - shell=shell, - text=text, - timeout=timeout, + command_list = shlex.split(command) + + sub_process = await asyncio.create_subprocess_exec( + *command_list, **kwargs, ) - out_decoded = ( - sub_process.stdout.decode(errors="ignore") if isinstance(sub_process.stdout, bytes) else sub_process.stdout - ) - err_decoded = ( - sub_process.stderr.decode(errors="ignore") if isinstance(sub_process.stderr, bytes) else sub_process.stderr - ) + + stdout, stderr = await sub_process.communicate() + out_decoded = stdout.decode(errors="ignore") if isinstance(stdout, bytes) else stdout + err_decoded = stderr.decode(errors="ignore") if isinstance(stderr, bytes) else stderr error_msg = ( f"{log_prefix} Failed to run '{command}'. " @@ -131,6 +128,7 @@ def run_command( return False, out_decoded, err_decoded return True, out_decoded, err_decoded + except Exception as ex: logger.error(f"{log_prefix} Failed to run '{command}' command: {ex}") return False, out_decoded, err_decoded @@ -146,9 +144,7 @@ def get_apis_and_tokes_from_config(config: Config) -> list[tuple[github.Github, return apis_and_tokens -def get_api_with_highest_rate_limit( - config: Config, repository_name: str = "" -) -> tuple[github.Github | None, str | None, str]: +def get_api_with_highest_rate_limit(config: Config, repository_name: str = "") -> tuple[github.Github, str, str]: """ Get API with the highest rate limit @@ -168,7 +164,7 @@ def get_api_with_highest_rate_limit( remaining = 0 - msg = "Get API and token" + msg = "Get API and tokens" if repository_name: msg += f" for repository {repository_name}" @@ -178,15 +174,28 @@ def get_api_with_highest_rate_limit( apis_and_tokens = get_apis_and_tokes_from_config(config=config) for _api, _token in apis_and_tokens: - _api_user = _api.get_user().login - rate_limit = _api.get_rate_limit() - if rate_limit.core.remaining > remaining: - remaining = rate_limit.core.remaining - api, token = _api, _token + if _api.rate_limiting[-1] == 60: + logger.warning("API has rate limit set to 60 which indicates an invalid token, skipping") + continue + + try: + _api_user = _api.get_user().login + except Exception as ex: + logger.warning(f"Failed to get API user for API {_api}, skipping. {ex}") + continue + + _rate_limit = _api.get_rate_limit() + + if _rate_limit.core.remaining > remaining: + remaining = _rate_limit.core.remaining + api, token, _api_user, rate_limit = _api, _token, _api_user, _rate_limit if rate_limit: log_rate_limit(rate_limit=rate_limit, api_user=_api_user) + if not _api_user or not api or not token: + raise NoApiTokenError("Failed to get API with highest rate limit") + logger.info(f"API user {_api_user} selected with highest rate limit: {remaining}") return api, token, _api_user diff --git a/webhook_server/utils/webhook.py b/webhook_server/utils/webhook.py index 23a4e8c4..015ddc27 100644 --- a/webhook_server/utils/webhook.py +++ b/webhook_server/utils/webhook.py @@ -27,7 +27,7 @@ def process_github_webhook( if not github_api: return False, f"{full_repository_name}: Failed to get github api", LOGGER.error - repo = get_github_repo_api(github_api=github_api, repository=full_repository_name) + repo = get_github_repo_api(github_app_api=github_api, repository=full_repository_name) if not repo: return False, f"[API user {api_user}] - Could not find repository {full_repository_name}", LOGGER.error