diff --git a/CHANGELOG.md b/CHANGELOG.md index c323e73..e3f19f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,10 +4,11 @@ ### Added +- Added support for auto-merging of PRs under certain conditions. ([#110](https://github.com/eclipse-csi/otterdog/issues/110)) +- Added handling for settings that require access to the Web UI. ([#208](https://github.com/eclipse-csi/otterdog/issues/208)) - Added support for repository setting `private_vulnerability_reporting_enabled`. ([#205](https://github.com/eclipse-csi/otterdog/issues/205)) - Added a graphql based query interface to the dashboard. ([#204](https://github.com/eclipse-csi/otterdog/issues/204)) - ## [0.5.0] - 05/03/2024 Note: this version includes lots of additions and changes related to the GitHub App mode which are not diff --git a/DEPENDENCIES b/DEPENDENCIES index bd63125..a422f0c 100644 --- a/DEPENDENCIES +++ b/DEPENDENCIES @@ -71,6 +71,7 @@ pypi/pypi/-/python-dotenv/1.0.1 pypi/pypi/-/pyyaml/6.0.1 pypi/pypi/-/pyyaml-env-tag/0.1 pypi/pypi/-/quart/0.19.4 +pypi/pypi/-/quart-flask-patch/0.3.0 pypi/pypi/-/quart-redis/2.0.0 pypi/pypi/-/redis/4.6.0 pypi/pypi/-/referencing/0.34.0 diff --git a/docker/Dockerfile b/docker/Dockerfile index e8140d1..01dd52a 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -40,10 +40,6 @@ RUN poetry config virtualenvs.in-project true && \ poetry build && \ poetry install --only-root -WORKDIR /app/.venv/lib/python3.12/site-packages -COPY ./docker/odmantic.patch ./ -RUN patch -p1 < odmantic.patch - # create the final image having python3.12 as base FROM python:3.12.2-slim diff --git a/docker/odmantic.patch b/docker/odmantic.patch deleted file mode 100644 index c4e8a92..0000000 --- a/docker/odmantic.patch +++ /dev/null @@ -1,17 +0,0 @@ ---- a/odmantic/model.py -+++ b/odmantic/model.py -@@ -194,11 +194,9 @@ - # generics is found - # https://github.com/pydantic/pydantic/issues/8354 - if type_origin is Union: -- new_root = Union[ -- int, str -- ] # We don't care about int,str since they will be replaced -- setattr(new_root, "__args__", new_arg_types) -- type_ = new_root # type: ignore -+ # as new_arg_types is a tuple, we can directly create a matching Union instance, -+ # instead of hacking our way around it: https://stackoverflow.com/a/72884529/3784643 -+ type_ = Union[new_arg_types] # type: ignore - else: - type_ = GenericAlias(type_origin, new_arg_types) # type: ignore - return type_ diff --git a/otterdog/__init__.py b/otterdog/__init__.py index def72cd..f0f5264 100644 --- a/otterdog/__init__.py +++ b/otterdog/__init__.py @@ -5,3 +5,7 @@ # which is available at http://www.eclipse.org/legal/epl-v20.html # SPDX-License-Identifier: EPL-2.0 # ******************************************************************************* + +import importlib.metadata + +__version__ = importlib.metadata.version(__name__) diff --git a/otterdog/cli.py b/otterdog/cli.py index 6a872c0..af452f7 100644 --- a/otterdog/cli.py +++ b/otterdog/cli.py @@ -7,7 +7,6 @@ # ******************************************************************************* import asyncio -import importlib.metadata import sys import traceback from typing import Any @@ -15,6 +14,7 @@ import click from click.shell_completion import CompletionItem +from . import __version__ from .config import OtterdogConfig from .operations import Operation from .operations.apply import ApplyOperation @@ -37,10 +37,6 @@ from .utils import IndentingPrinter, init, is_debug_enabled, print_error _CONFIG_FILE = "otterdog.json" - -_DISTRIBUTION_METADATA = importlib.metadata.metadata("otterdog") -_VERSION = _DISTRIBUTION_METADATA["Version"] - _CONTEXT_SETTINGS = dict(help_option_names=["-h", "--help"], max_content_width=120) _CONFIG: OtterdogConfig | None = None @@ -122,7 +118,7 @@ def invoke(self, ctx: click.Context) -> Any: @click.group(context_settings=_CONTEXT_SETTINGS) -@click.version_option(version=_VERSION, prog_name="otterdog.sh") +@click.version_option(version=__version__, prog_name="otterdog.sh") def cli(): """ Managing GitHub organizations at scale. diff --git a/otterdog/models/__init__.py b/otterdog/models/__init__.py index 1f4e4a2..3aa485c 100644 --- a/otterdog/models/__init__.py +++ b/otterdog/models/__init__.py @@ -105,6 +105,20 @@ def of_changes( LivePatchType.CHANGE, expected_object, current_object, changes, parent_object, forced_update, fn ) + def requires_web_ui(self) -> bool: + match self.patch_type: + case LivePatchType.ADD: + assert self.expected_object is not None + return self.expected_object.requires_web_ui() + + case LivePatchType.REMOVE: + return False + + case LivePatchType.CHANGE: + assert self.expected_object is not None + assert self.changes is not None + return self.expected_object.changes_require_web_ui(self.changes) + def requires_secrets(self) -> bool: match self.patch_type: case LivePatchType.ADD: @@ -437,6 +451,12 @@ def to_model_dict(self, for_diff: bool = False, include_nested_models: bool = Fa return result + def requires_web_ui(self) -> bool: + return False + + def changes_require_web_ui(self, changes: dict[str, Change]) -> bool: + return False + def contains_secrets(self) -> bool: return False diff --git a/otterdog/models/organization_settings.py b/otterdog/models/organization_settings.py index 7213ae0..81d9abc 100644 --- a/otterdog/models/organization_settings.py +++ b/otterdog/models/organization_settings.py @@ -195,6 +195,11 @@ async def get_mapping_to_provider( def get_jsonnet_template_function(self, jsonnet_config: JsonnetConfig, extend: bool) -> str | None: return None + def changes_require_web_ui(self, changes: dict[str, Change]) -> bool: + from otterdog.providers.github import is_org_settings_key_retrieved_via_web_ui + + return any(map(lambda key: is_org_settings_key_retrieved_via_web_ui(key), changes.keys())) + def to_jsonnet( self, printer: IndentingPrinter, diff --git a/otterdog/providers/github/__init__.py b/otterdog/providers/github/__init__.py index 80abc3e..ef49696 100644 --- a/otterdog/providers/github/__init__.py +++ b/otterdog/providers/github/__init__.py @@ -21,24 +21,22 @@ _ORG_SETTINGS_SCHEMA = json.loads(files(resources).joinpath("schemas/settings.json").read_text()) +# collect supported rest api keys +_SETTINGS_RESTAPI_KEYS = {k for k, v in _ORG_SETTINGS_SCHEMA["properties"].items() if v.get("provider") == "restapi"} +# collect supported web interface keys +_SETTINGS_WEB_KEYS = {k for k, v in _ORG_SETTINGS_SCHEMA["properties"].items() if v.get("provider") == "web"} +# TODO: make this cleaner +_SETTINGS_WEB_KEYS.add("discussion_source_repository_id") + + +def is_org_settings_key_retrieved_via_web_ui(key: str) -> bool: + return key in _SETTINGS_WEB_KEYS + class GitHubProvider: def __init__(self, credentials: Credentials | None): self._credentials = credentials - self._settings_schema = _ORG_SETTINGS_SCHEMA - # collect supported rest api keys - self._settings_restapi_keys = { - k for k, v in self._settings_schema["properties"].items() if v.get("provider") == "restapi" - } - - # collect supported web interface keys - self._settings_web_keys = { - k for k, v in self._settings_schema["properties"].items() if v.get("provider") == "web" - } - # TODO: make this cleaner - self._settings_web_keys.add("discussion_source_repository_id") - if credentials is not None: self._init_clients() @@ -73,13 +71,13 @@ async def update_content( async def get_org_settings(self, org_id: str, included_keys: set[str], no_web_ui: bool) -> dict[str, Any]: # first, get supported settings via the rest api. - required_rest_keys = {x for x in included_keys if x in self._settings_restapi_keys} + required_rest_keys = {x for x in included_keys if x in _SETTINGS_RESTAPI_KEYS} merged_settings = await self.rest_api.org.get_settings(org_id, required_rest_keys) # second, get settings only accessible via the web interface and merge # them with the other settings, unless --no-web-ui is specified. if not no_web_ui: - required_web_keys = {x for x in included_keys if x in self._settings_web_keys} + required_web_keys = {x for x in included_keys if x in _SETTINGS_WEB_KEYS} if len(required_web_keys) > 0: web_settings = await self.web_client.get_org_settings(org_id, required_web_keys) merged_settings.update(web_settings) @@ -95,9 +93,9 @@ async def update_org_settings(self, org_id: str, settings: dict[str, Any]) -> No # split up settings to be updated whether they need be updated # via rest api or web interface. for k, v in sorted(settings.items()): - if k in self._settings_restapi_keys: + if k in _SETTINGS_RESTAPI_KEYS: rest_fields[k] = v - elif k in self._settings_web_keys: + elif k in _SETTINGS_WEB_KEYS: web_fields[k] = v else: utils.print_warn(f"encountered unknown field '{k}' during update, ignoring") diff --git a/otterdog/providers/github/rest/pull_request_client.py b/otterdog/providers/github/rest/pull_request_client.py index 095e720..81ae5f8 100644 --- a/otterdog/providers/github/rest/pull_request_client.py +++ b/otterdog/providers/github/rest/pull_request_client.py @@ -42,3 +42,42 @@ async def get_pull_requests( except GitHubException as ex: tb = ex.__traceback__ raise RuntimeError(f"failed retrieving pull requests:\n{ex}").with_traceback(tb) + + async def get_reviews(self, org_id: str, repo_name: str, pull_request_number: str) -> list[dict[str, Any]]: + print_debug(f"getting reviews for pull request #{pull_request_number} from repo '{org_id}/{repo_name}'") + + try: + return await self.requester.request_paged_json( + "GET", f"/repos/{org_id}/{repo_name}/pulls/{pull_request_number}/reviews" + ) + except GitHubException as ex: + tb = ex.__traceback__ + raise RuntimeError(f"failed retrieving pull request reviews:\n{ex}").with_traceback(tb) + + async def merge( + self, + org_id: str, + repo_name: str, + pull_request_number: str, + commit_message: str | None = None, + merge_method: str = "squash", + ) -> bool: + print_debug(f"merging pull request #{pull_request_number} from repo '{org_id}/{repo_name}'") + + try: + data = { + "merge_method": merge_method, + } + + if commit_message is not None: + data.update({"commit_message": commit_message}) + + response = await self.requester.request_json( + "PUT", + f"/repos/{org_id}/{repo_name}/pulls/{pull_request_number}/merge", + data=data, + ) + return response["merged"] + except GitHubException as ex: + tb = ex.__traceback__ + raise RuntimeError(f"failed merging pull request:\n{ex}").with_traceback(tb) diff --git a/otterdog/webapp/__init__.py b/otterdog/webapp/__init__.py index afc4a6d..b1e4a7b 100644 --- a/otterdog/webapp/__init__.py +++ b/otterdog/webapp/__init__.py @@ -15,6 +15,7 @@ from importlib.util import find_spec from typing import TYPE_CHECKING +import quart_flask_patch # type: ignore # noqa: F401 from quart import Quart from quart.json.provider import DefaultJSONProvider from quart_redis import RedisHandler # type: ignore diff --git a/otterdog/webapp/db/models.py b/otterdog/webapp/db/models.py index 5c0517b..3e0108e 100644 --- a/otterdog/webapp/db/models.py +++ b/otterdog/webapp/db/models.py @@ -94,7 +94,9 @@ class PullRequestModel(Model): valid: Optional[bool] = None in_sync: Optional[bool] = None - requires_manual_apply: bool = False + requires_manual_apply: Optional[bool] = None + supports_auto_merge: Optional[bool] = None + has_required_approval: Optional[bool] = None created_at: datetime = Field(index=True) updated_at: datetime = Field(index=True) @@ -113,6 +115,14 @@ class PullRequestModel(Model): ], } + def can_be_automerged(self) -> bool: + return ( + self.valid is True + and self.in_sync is True + and self.supports_auto_merge is True + and self.has_required_approval is True + ) + class StatisticsModel(Model): project_name: str = Field(primary_field=True) diff --git a/otterdog/webapp/db/service.py b/otterdog/webapp/db/service.py index dc1c394..9c8df3d 100644 --- a/otterdog/webapp/db/service.py +++ b/otterdog/webapp/db/service.py @@ -307,8 +307,10 @@ async def update_or_create_pull_request( valid: bool | None = None, in_sync: bool | None = None, requires_manual_apply: bool | None = None, + supports_auto_merge: bool | None = None, + has_required_approvals: bool | None = None, apply_status: ApplyStatus | None = None, -) -> None: +) -> PullRequestModel: pull_request_status = PullRequestStatus[pull_request.get_pr_status()] pr_model = await find_pull_request(owner, repo, pull_request.number) @@ -344,7 +346,14 @@ async def update_or_create_pull_request( if requires_manual_apply is not None: pr_model.requires_manual_apply = requires_manual_apply + if supports_auto_merge is not None: + pr_model.supports_auto_merge = supports_auto_merge + + if has_required_approvals is not None: + pr_model.has_required_approval = has_required_approvals + await update_pull_request(pr_model) + return pr_model async def update_pull_request(pull_request: PullRequestModel) -> None: diff --git a/otterdog/webapp/home/routes.py b/otterdog/webapp/home/routes.py index 590edf0..1b5a61f 100644 --- a/otterdog/webapp/home/routes.py +++ b/otterdog/webapp/home/routes.py @@ -46,8 +46,8 @@ async def route_default(): @blueprint.route("/robots.txt") @blueprint.route("/favicon.ico") -def static_from_root(): - return send_from_directory(current_app.static_folder, request.path[1:]) +async def static_from_root(): + return await send_from_directory(current_app.static_folder, request.path[1:]) @blueprint.route("/index") diff --git a/otterdog/webapp/tasks/__init__.py b/otterdog/webapp/tasks/__init__.py index fb5f21a..2e59d58 100644 --- a/otterdog/webapp/tasks/__init__.py +++ b/otterdog/webapp/tasks/__init__.py @@ -6,6 +6,8 @@ # SPDX-License-Identifier: EPL-2.0 # ******************************************************************************* +from __future__ import annotations + import contextlib from abc import ABC, abstractmethod from collections.abc import AsyncIterator @@ -14,6 +16,7 @@ from typing import Any, Generic, Protocol, TypeVar import aiofiles +from quart import current_app from otterdog.config import OrganizationConfig from otterdog.providers.github import GitHubProvider, GraphQLClient @@ -216,6 +219,18 @@ async def minimize_outdated_comments( if bool(is_minimized) is False and matching_header in body: await graphql_api.minimize_comment(comment_id, "OUTDATED") + def schedule_automerge_task(self, org_id: str, repo_name: str, pull_request_number: int) -> None: + from .auto_merge_comment import AutoMergeCommentTask + + current_app.add_background_task( + AutoMergeCommentTask( + self.installation_id, + org_id, + repo_name, + pull_request_number, + ) + ) + async def _cleanup(self) -> None: if self.__rest_api is not None: await self.__rest_api.close() diff --git a/otterdog/webapp/tasks/apply_changes.py b/otterdog/webapp/tasks/apply_changes.py index 7af6461..548230f 100644 --- a/otterdog/webapp/tasks/apply_changes.py +++ b/otterdog/webapp/tasks/apply_changes.py @@ -178,7 +178,7 @@ async def _execute(self) -> ApplyResult: operation_result = await operation.execute(org_config) apply_result.apply_output = output.getvalue() apply_result.apply_success = operation_result == 0 - apply_result.partial = self._pr_model.requires_manual_apply + apply_result.partial = self._pr_model.requires_manual_apply is True except Exception as ex: self.logger.exception("exception during apply", exc_info=ex) diff --git a/otterdog/webapp/tasks/auto_merge_comment.py b/otterdog/webapp/tasks/auto_merge_comment.py new file mode 100644 index 0000000..081893c --- /dev/null +++ b/otterdog/webapp/tasks/auto_merge_comment.py @@ -0,0 +1,58 @@ +# ******************************************************************************* +# Copyright (c) 2024 Eclipse Foundation and others. +# This program and the accompanying materials are made available +# under the terms of the Eclipse Public License 2.0 +# which is available at http://www.eclipse.org/legal/epl-v20.html +# SPDX-License-Identifier: EPL-2.0 +# ******************************************************************************* + +from dataclasses import dataclass + +from quart import render_template + +from otterdog.webapp.db.models import TaskModel +from otterdog.webapp.tasks import InstallationBasedTask, Task + + +@dataclass(repr=False) +class AutoMergeCommentTask(InstallationBasedTask, Task[None]): + installation_id: int + org_id: str + repo_name: str + pull_request_number: int + + def create_task_model(self): + return TaskModel( + type=type(self).__name__, + org_id=self.org_id, + repo_name=self.repo_name, + pull_request=self.pull_request_number, + ) + + async def _execute(self) -> None: + self.logger.info( + "adding auto merge comment to pull request #%d of repo '%s/%s'", + self.pull_request_number, + self.org_id, + self.repo_name, + ) + + rest_api = await self.rest_api + comment = await render_template("comment/auto_merge_comment.txt") + + await self.minimize_outdated_comments( + self.org_id, + self.repo_name, + self.pull_request_number, + "", + ) + + await rest_api.issue.create_comment( + self.org_id, + self.repo_name, + str(self.pull_request_number), + comment, + ) + + def __repr__(self) -> str: + return f"AutoMergeCommentTask(repo='{self.org_id}/{self.repo_name}', pull_request=#{self.pull_request_number})" diff --git a/otterdog/webapp/tasks/check_sync.py b/otterdog/webapp/tasks/check_sync.py index 741906e..fe2f237 100644 --- a/otterdog/webapp/tasks/check_sync.py +++ b/otterdog/webapp/tasks/check_sync.py @@ -144,6 +144,7 @@ def sync_callback(org_id: str, diff_status: DiffStatus, patches: list[LivePatch] "", ) + rest_api = await self.rest_api await rest_api.issue.create_comment( self.org_id, org_config.config_repo, @@ -193,13 +194,16 @@ async def _update_final_status(self, config_in_sync: bool) -> None: desc, ) - await update_or_create_pull_request( + pull_request_model = await update_or_create_pull_request( self.org_id, self.repo_name, self._pull_request, in_sync=config_in_sync, ) + if pull_request_model.can_be_automerged(): + self.schedule_automerge_task(self.org_id, self.repo_name, self.pull_request_number) + def __repr__(self) -> str: return ( f"CheckConfigurationInSyncTask(repo='{self.org_id}/{self.repo_name}', " diff --git a/otterdog/webapp/tasks/merge_pull_request.py b/otterdog/webapp/tasks/merge_pull_request.py new file mode 100644 index 0000000..b50cdbe --- /dev/null +++ b/otterdog/webapp/tasks/merge_pull_request.py @@ -0,0 +1,95 @@ +# ******************************************************************************* +# Copyright (c) 2024 Eclipse Foundation and others. +# This program and the accompanying materials are made available +# under the terms of the Eclipse Public License 2.0 +# which is available at http://www.eclipse.org/legal/epl-v20.html +# SPDX-License-Identifier: EPL-2.0 +# ******************************************************************************* + +from dataclasses import dataclass + +from quart import render_template + +from otterdog.webapp.db.models import PullRequestStatus, TaskModel +from otterdog.webapp.db.service import find_pull_request +from otterdog.webapp.tasks import InstallationBasedTask, Task +from otterdog.webapp.webhook.github_models import PullRequest + + +@dataclass(repr=False) +class MergePullRequestTask(InstallationBasedTask, Task[None]): + installation_id: int + org_id: str + repo_name: str + pull_request_number: int + author: str + + def create_task_model(self): + return TaskModel( + type=type(self).__name__, + org_id=self.org_id, + repo_name=self.repo_name, + pull_request=self.pull_request_number, + ) + + async def _pre_execute(self) -> bool: + self.logger.info( + "auto-merging pull request #%d on behalf of user '%s' for repo '%s/%s'", + self.pull_request_number, + self.author, + self.org_id, + self.repo_name, + ) + + pr_model = await find_pull_request(self.org_id, self.repo_name, self.pull_request_number) + if pr_model is None: + raise RuntimeError( + f"failed to fetch pull request #{self.pull_request_number} in repo '{self.org_id}/{self.repo_name}'" + ) + else: + self._pr_model = pr_model + + if pr_model.status != PullRequestStatus.OPEN: + self.logger.info( + f"pull request #{self.pull_request_number} for repo '{self.org_id}/{self.repo_name}' " + "is not open, skipping" + ) + return False + + if pr_model.can_be_automerged() is False: + self.logger.info( + f"pull request #{self.pull_request_number} for repo '{self.org_id}/{self.repo_name}' " + "is not eligible for auto-merge, skipping" + ) + return False + + rest_api = await self.rest_api + response = await rest_api.pull_request.get_pull_request( + self.org_id, self.repo_name, str(self.pull_request_number) + ) + pull_request = PullRequest.model_validate(response) + + if self.author != pull_request.user.login: + comment = await render_template("comment/wrong_user_merge_comment.txt") + await rest_api.issue.create_comment(self.org_id, self.repo_name, str(self.pull_request_number), comment) + + self.logger.error( + f"merge for pull request #{self.pull_request_number} triggered by user '{self.author}' " + "who is not the author, skipping" + ) + + return False + + return True + + async def _execute(self) -> None: + rest_api = await self.rest_api + merged = await rest_api.pull_request.merge(self.org_id, self.repo_name, str(self.pull_request_number)) + + if merged is True: + self.logger.info(f"Pull Request #{self.pull_request_number} auto-merged") + else: + self.logger.error(f"Pull Request #{self.pull_request_number} failed to auto-merge") + + def __repr__(self) -> str: + return f"MergePullRequestTask(repo='{self.org_id}/{self.repo_name}', pull_request=#{self.pull_request_number})" diff --git a/otterdog/webapp/tasks/update_pull_request.py b/otterdog/webapp/tasks/update_pull_request.py index d276b81..c9fb64a 100644 --- a/otterdog/webapp/tasks/update_pull_request.py +++ b/otterdog/webapp/tasks/update_pull_request.py @@ -6,12 +6,16 @@ # SPDX-License-Identifier: EPL-2.0 # ******************************************************************************* +from collections.abc import Iterable from dataclasses import dataclass from otterdog.webapp.db.models import TaskModel -from otterdog.webapp.db.service import update_or_create_pull_request +from otterdog.webapp.db.service import ( + update_or_create_pull_request, + update_pull_request, +) from otterdog.webapp.tasks import InstallationBasedTask, Task -from otterdog.webapp.webhook.github_models import PullRequest +from otterdog.webapp.webhook.github_models import PullRequest, Review @dataclass(repr=False) @@ -20,6 +24,7 @@ class UpdatePullRequestTask(InstallationBasedTask, Task[None]): org_id: str repo_name: str pull_request: PullRequest + review: Review | None = None @property def pull_request_number(self) -> int: @@ -41,10 +46,80 @@ async def _execute(self) -> None: self.repo_name, ) - await update_or_create_pull_request( - self.org_id, - self.repo_name, - self.pull_request, + if self.review is not None: + # TODO: simplify the logic, if we already figured out that the author of the PR + # is eligible for auto-merge, no need to check that again when a review arrives. + + rest_api = await self.rest_api + + # check if the PR was approved by a team eligible for auto-merge + reviews = await rest_api.pull_request.get_reviews( + self.org_id, self.repo_name, str(self.pull_request_number) + ) + + approved_by_users = list( + map( + lambda x: x["user"]["login"], + filter(lambda x: x["state"] == "APPROVED", reviews), + ) + ) + + self.logger.debug(f"approved by users: {approved_by_users}") + + graphql_api = await self.graphql_api + approved_by_teams: set[str] = set() + for user_login in approved_by_users: + teams = await graphql_api.get_team_membership(self.org_id, user_login) + for team in teams: + approved_by_teams.add(team["name"]) + + self.logger.debug(f"approved by teams: {approved_by_teams}") + + has_required_approvals = self._contains_valid_team_for_approval(approved_by_teams, True) + + # if no approval yet, check if the author is member of a team that is eligible for auto-merge + if has_required_approvals is False and self.pull_request.author_association == "MEMBER": + author_teams = map( + lambda x: x["name"], + await graphql_api.get_team_membership(self.org_id, self.pull_request.user.login), + ) + has_required_approvals = self._contains_valid_team_for_approval(author_teams, False) + + pull_request_model = await update_or_create_pull_request( + self.org_id, + self.repo_name, + self.pull_request, + has_required_approvals=has_required_approvals, + ) + + if pull_request_model.can_be_automerged(): + self.schedule_automerge_task(self.org_id, self.repo_name, self.pull_request_number) + else: + pull_request_model = await update_or_create_pull_request( + self.org_id, + self.repo_name, + self.pull_request, + ) + + if pull_request_model.has_required_approval is None: + graphql_api = await self.graphql_api + author_teams = map( + lambda x: x["name"], + await graphql_api.get_team_membership(self.org_id, self.pull_request.user.login), + ) + + pull_request_model.has_required_approval = self._contains_valid_team_for_approval(author_teams, False) + await update_pull_request(pull_request_model) + + @staticmethod + def _contains_valid_team_for_approval(teams: Iterable[str], include_admin: bool) -> bool: + # FIXME: team names must be made configurable + return any( + map( + lambda x: x.endswith("project-leads") + or (include_admin and (x == "eclipsefdn-security" or x == "eclipsefdn-releng")), + teams, + ) ) def __repr__(self) -> str: diff --git a/otterdog/webapp/tasks/validate_pull_request.py b/otterdog/webapp/tasks/validate_pull_request.py index 3e8db3b..676f682 100644 --- a/otterdog/webapp/tasks/validate_pull_request.py +++ b/otterdog/webapp/tasks/validate_pull_request.py @@ -14,7 +14,7 @@ from quart import current_app, render_template -from otterdog.models import LivePatch +from otterdog.models import LivePatch, LivePatchType from otterdog.operations.diff_operation import DiffStatus from otterdog.operations.local_plan import LocalPlanOperation from otterdog.utils import IndentingPrinter, LogLevel @@ -33,8 +33,10 @@ @dataclass class ValidationResult: plan_output: str = "" - validation_success: bool = True - requires_secrets: bool = False + validation_success: bool = False + requires_secrets: bool | None = None + requires_web_ui: bool | None = None + includes_deletions: bool | None = None @dataclass(repr=False) @@ -139,6 +141,11 @@ async def _execute(self) -> ValidationResult: def callback(org_id: str, diff_status: DiffStatus, patches: list[LivePatch]): validation_result.requires_secrets = any(list(map(lambda x: x.requires_secrets(), patches))) + validation_result.requires_web_ui = any(list(map(lambda x: x.requires_web_ui(), patches))) + + validation_result.includes_deletions = any( + list(map(lambda x: x.patch_type == LivePatchType.REMOVE, patches)) + ) otterdog_config = await get_otterdog_config() @@ -160,6 +167,10 @@ def callback(org_id: str, diff_status: DiffStatus, patches: list[LivePatch]): warnings = [] if validation_result.requires_secrets: warnings.append("some of requested changes require secrets, need to apply these changes manually") + if validation_result.requires_web_ui: + warnings.append( + "some of requested changes require accessing the Web UI, need to apply these changes manually" + ) comment = await render_template( "comment/validation_comment.txt", @@ -177,8 +188,12 @@ def callback(org_id: str, diff_status: DiffStatus, patches: list[LivePatch]): ) # add a comment about the validation result to the PR + rest_api = await self.rest_api await rest_api.issue.create_comment( - self.org_id, org_config.config_repo, str(self.pull_request_number), comment + self.org_id, + self.repo_name, + str(self.pull_request_number), + comment, ) return validation_result @@ -223,14 +238,18 @@ async def _update_final_status(self, validation_result: ValidationResult) -> Non desc, ) - await update_or_create_pull_request( + pull_request_model = await update_or_create_pull_request( self.org_id, self.repo_name, self._pull_request, valid=validation_result.validation_success, - requires_manual_apply=validation_result.requires_secrets, + requires_manual_apply=validation_result.requires_secrets or validation_result.requires_web_ui, + supports_auto_merge=not (validation_result.requires_secrets or validation_result.requires_web_ui), ) + if pull_request_model.can_be_automerged(): + self.schedule_automerge_task(self.org_id, self.repo_name, self.pull_request_number) + def __repr__(self) -> str: return ( f"ValidatePullRequestTask(repo='{self.org_id}/{self.repo_name}', " diff --git a/otterdog/webapp/templates/comment/auto_merge_comment.txt b/otterdog/webapp/templates/comment/auto_merge_comment.txt new file mode 100644 index 0000000..091b1e0 --- /dev/null +++ b/otterdog/webapp/templates/comment/auto_merge_comment.txt @@ -0,0 +1,10 @@ + +This is your friendly self-service bot. +This Pull Request is eligible for auto-merging as it passed the following checks: + +- valid configuration +- does not require any secrets +- does not require accessing the GitHub Web UI +- approved by a project lead or a member of the EF security team + +In order to automatically merge and apply the changes, add a comment `/merge` :rocket:. diff --git a/otterdog/webapp/templates/comment/help_comment.txt b/otterdog/webapp/templates/comment/help_comment.txt index fdd9980..4d73f5e 100644 --- a/otterdog/webapp/templates/comment/help_comment.txt +++ b/otterdog/webapp/templates/comment/help_comment.txt @@ -5,5 +5,6 @@ This is your friendly self-service bot. The following commands are supported: - `/validate`: validates the configuration change if this PR touches the configuration - `/validate info`: validates the configuration change, printing also validation infos - `/check-sync`: checks if the base ref is in sync with live settings +- `/merge`: merges and applies the changes if the pull request is eligible to auto-merging (only accessible for the author) - `/done`: notifies the self-service bot that required manual apply operation has been performed (only accessible for members of the admin team) - `/apply`: re-apply a previously failed attempt (only accessible for members of the admin team) diff --git a/otterdog/webapp/templates/comment/wrong_user_merge_comment.txt b/otterdog/webapp/templates/comment/wrong_user_merge_comment.txt new file mode 100644 index 0000000..fa47a91 --- /dev/null +++ b/otterdog/webapp/templates/comment/wrong_user_merge_comment.txt @@ -0,0 +1,2 @@ +This is your friendly self-service bot. +Only the author of the pull request is allowed to auto-merge it. diff --git a/otterdog/webapp/templates/home/pullrequests.html b/otterdog/webapp/templates/home/pullrequests.html index 72f27e4..9db0993 100644 --- a/otterdog/webapp/templates/home/pullrequests.html +++ b/otterdog/webapp/templates/home/pullrequests.html @@ -57,8 +57,10 @@

Currently open Pull Requests:

Created PR Status Draft - Config in-sync - Manual required + In-sync + Requires manual + Support automerge + Approved Apply Status @@ -73,6 +75,8 @@

Currently open Pull Requests:

{{ pr.draft }} {{ pr.in_sync }} {{ pr.requires_manual_apply }} + {{ pr.supports_auto_merge }} + {{ pr.has_required_approval }} {{ pr.apply_status }} {% endfor %} diff --git a/otterdog/webapp/webhook/__init__.py b/otterdog/webapp/webhook/__init__.py index 173c4d2..dd6832d 100644 --- a/otterdog/webapp/webhook/__init__.py +++ b/otterdog/webapp/webhook/__init__.py @@ -28,10 +28,12 @@ from otterdog.webapp.tasks.validate_pull_request import ValidatePullRequestTask from otterdog.webapp.utils import refresh_otterdog_config +from ..tasks.merge_pull_request import MergePullRequestTask from .github_models import ( InstallationEvent, IssueCommentEvent, PullRequestEvent, + PullRequestReviewEvent, PushEvent, ) from .github_webhook import GitHubWebhook @@ -111,6 +113,34 @@ async def on_pull_request_received(data): return success() +@webhook.hook("pull_request_review") +async def on_pull_request_review_received(data): + try: + event = PullRequestReviewEvent.model_validate(data) + except ValidationError: + logger.error("failed to load pull request review event data", exc_info=True) + return success() + + if event.installation is None or event.organization is None: + return success() + + if not await targets_config_repo(event.repository.name, event.installation.id): + return success() + + if event.action in ["submitted", "edited", "dismissed"]: + current_app.add_background_task( + UpdatePullRequestTask( + event.installation.id, + event.organization.login, + event.repository.name, + event.pull_request, + event.review, + ) + ) + + return success() + + @webhook.hook("issue_comment") async def on_issue_comment_received(data): try: @@ -185,6 +215,17 @@ async def on_issue_comment_received(data): ) ) return success() + elif re.match(r"\s*/merge\s*", event.comment.body) is not None: + current_app.add_background_task( + MergePullRequestTask( + installation_id, + org_id, + event.repository.name, + event.issue.number, + event.sender.login, + ) + ) + return success() m = re.match(r"\s*/validate(\s+info)?\s*", event.comment.body) if m is None: diff --git a/otterdog/webapp/webhook/github_models.py b/otterdog/webapp/webhook/github_models.py index bcd9908..f1b6f3f 100644 --- a/otterdog/webapp/webhook/github_models.py +++ b/otterdog/webapp/webhook/github_models.py @@ -100,6 +100,19 @@ def get_pr_status(self) -> str: raise RuntimeError(f"unexpected state '{self.state}'") +class Review(BaseModel): + """Represents a review on a pull request.""" + + author_association: str + body: str | None + commit_id: str + id: int + node_id: str + state: str + submitted_at: datetime | None + user: Actor | None + + class Comment(BaseModel): """Represents a comment in an issue.""" @@ -164,6 +177,15 @@ class PullRequestEvent(Event): repository: Repository +class PullRequestReviewEvent(Event): + """A payload sent for pull request review specific events.""" + + action: str + pull_request: PullRequest + review: Review + repository: Repository + + class PushEvent(Event): """A payload sent for push events.""" diff --git a/poetry.lock b/poetry.lock index 0615569..1aa3dc7 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2231,6 +2231,20 @@ werkzeug = ">=3.0.0" docs = ["pydata_sphinx_theme"] dotenv = ["python-dotenv"] +[[package]] +name = "quart-flask-patch" +version = "0.3.0" +description = "Quart-Flask-Patch is a Quart extension that patches Quart to work with Flask extensions." +optional = false +python-versions = "*" +files = [ + {file = "quart_flask_patch-0.3.0-py2.py3-none-any.whl", hash = "sha256:b5a1d97a8edee9f40082db278fb491687d712af9bc8eb774830ea6f92790b990"}, + {file = "quart_flask_patch-0.3.0.tar.gz", hash = "sha256:81dc073698d54ffe1d8c85a556881c13215923a920f9300a02b5005d5447268d"}, +] + +[package.dependencies] +quart = ">=0.19" + [[package]] name = "quart-redis" version = "2.0.0" @@ -2920,4 +2934,4 @@ multidict = ">=4.0" [metadata] lock-version = "2.0" python-versions = "^3.10" -content-hash = "df4798dcd1e8d963dcf1b18ba791f07f66f5cbd5e4be984e3c6ed2e957b0ab71" +content-hash = "3ffa04d4a5be01b5d8ef6486ce08839aae021bd781c57d2c83a0c96e03be8e81" diff --git a/pyproject.toml b/pyproject.toml index a3f4319..fdac19c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -57,6 +57,7 @@ requests = "^2.31" [tool.poetry.group.app.dependencies] quart = "^0.19" +quart-flask-patch = "^0.3" python-decouple = "^3.8" python-dotenv = "^1.0" pydantic = "^2.6"