diff --git a/packit_service/worker/checker/bodhi.py b/packit_service/worker/checker/bodhi.py index d22eef0994..c6fdaecf15 100644 --- a/packit_service/worker/checker/bodhi.py +++ b/packit_service/worker/checker/bodhi.py @@ -7,36 +7,72 @@ from packit_service.constants import KojiBuildState -from packit_service.worker.checker.abstract import Checker -from packit_service.worker.handlers.mixin import GetKojiBuildEventMixin +from packit_service.worker.checker.abstract import ActorChecker, Checker +from packit_service.worker.handlers.mixin import ( + GetKojiBuildData, + GetKojiBuildDataFromKojiBuildEventMixin, + GetKojiBuildDataFromKojiServiceMixin, + GetKojiBuildEventMixin, +) +from packit_service.worker.mixin import ConfigMixin logger = logging.getLogger(__name__) -class IsKojiBuildCompleteAndBranchConfigured(Checker, GetKojiBuildEventMixin): +class IsKojiBuildCompleteAndBranchConfigured(Checker, GetKojiBuildData): def pre_check(self) -> bool: """Check if builds are finished (=KojiBuildState.complete) and branches are configured. By default, we use `fedora-stable` alias. (Rawhide updates are already created automatically.) """ - if self.koji_build_event.state != KojiBuildState.complete: + if self.state != KojiBuildState.complete: logger.debug( - f"Skipping build '{self.koji_build_event.build_id}' " - f"on '{self.koji_build_event.git_ref}'. " + f"Skipping build '{self.build_id}' " + f"on '{self.dist_git_branch}'. " f"Build not finished yet." ) return False - if self.koji_build_event.git_ref not in ( + if self.dist_git_branch not in ( configured_branches := get_branches( *(self.job_config.dist_git_branches or {"fedora-stable"}), default_dg_branch="rawhide", # Koji calls it rawhide, not main ) ): logger.info( - f"Skipping build on '{self.data.git_ref}'. " + f"Skipping build on '{self.dist_git_branch}'. " f"Bodhi update configured only for '{configured_branches}'." ) return False return True + + +class IsKojiBuildCompleteAndBranchConfiguredCheckEvent( + IsKojiBuildCompleteAndBranchConfigured, + GetKojiBuildEventMixin, + GetKojiBuildDataFromKojiBuildEventMixin, +): + ... + + +class IsKojiBuildCompleteAndBranchConfiguredCheckService( + IsKojiBuildCompleteAndBranchConfigured, GetKojiBuildDataFromKojiServiceMixin +): + ... + + +class HasAuthorWriteAccess(ActorChecker, ConfigMixin): + def _pre_check(self) -> bool: + logger.debug( + f"Bodhi update will be re-triggered via dist-git PR comment by user {self.actor}." + ) + + if not self.project.has_write_access(user=self.actor): + logger.info( + f"Re-triggering Bodhi update via dist-git comment in PR#{self.data.pr_id}" + f" and project {self.project.repo} is not allowed for the user: {self.actor}." + ) + return False + + return True diff --git a/packit_service/worker/handlers/abstract.py b/packit_service/worker/handlers/abstract.py index fb4681a0d3..668e8c3120 100644 --- a/packit_service/worker/handlers/abstract.py +++ b/packit_service/worker/handlers/abstract.py @@ -210,6 +210,7 @@ class TaskName(str, enum.Enum): # downstream_koji_build_report = "task.run_downstream_koji_build_report_handler" sync_from_downstream = "task.run_sync_from_downstream_handler" bodhi_update = "task.bodhi_update" + retrigger_bodhi_update = "task.retrigger_bodhi_update" github_fas_verification = "task.github_fas_verification" diff --git a/packit_service/worker/handlers/bodhi.py b/packit_service/worker/handlers/bodhi.py index f02eb306a9..a2a10e4dee 100644 --- a/packit_service/worker/handlers/bodhi.py +++ b/packit_service/worker/handlers/bodhi.py @@ -12,6 +12,7 @@ from packit.exceptions import PackitException + from packit.config import JobConfig, JobType, PackageConfig from packit_service.config import PackageConfigGetter from packit_service.constants import ( @@ -19,32 +20,39 @@ RETRY_INTERVAL_IN_MINUTES_WHEN_USER_ACTION_IS_NEEDED, ) from packit_service.worker.checker.abstract import Checker -from packit_service.worker.checker.bodhi import IsKojiBuildCompleteAndBranchConfigured +from packit_service.worker.checker.bodhi import ( + HasAuthorWriteAccess, + IsKojiBuildCompleteAndBranchConfiguredCheckEvent, + IsKojiBuildCompleteAndBranchConfiguredCheckService, +) +from packit_service.worker.events import PullRequestCommentPagureEvent from packit_service.worker.events.koji import KojiBuildEvent from packit_service.worker.handlers.abstract import ( TaskName, configured_as, reacts_to, RetriableJobHandler, + run_for_comment, +) +from packit_service.worker.handlers.mixin import ( + GetKojiBuildData, + GetKojiBuildDataFromKojiBuildEventMixin, + GetKojiBuildDataFromKojiServiceMixin, + GetKojiBuildEventMixin, +) +from packit_service.worker.mixin import ( + PackitAPIWithDownstreamMixin, ) -from packit_service.worker.handlers.mixin import GetKojiBuildEventMixin -from packit_service.worker.mixin import LocalProjectMixin from packit_service.worker.result import TaskResults logger = logging.getLogger(__name__) -@configured_as(job_type=JobType.bodhi_update) -@reacts_to(event=KojiBuildEvent) -class CreateBodhiUpdateHandler( - RetriableJobHandler, LocalProjectMixin, GetKojiBuildEventMixin +class BodhiUpdateHandler( + RetriableJobHandler, PackitAPIWithDownstreamMixin, GetKojiBuildData ): - """ - This handler can create a bodhi update for successful Koji builds. - """ topic = "org.fedoraproject.prod.buildsys.build.state.change" - task_name = TaskName.bodhi_update def __init__( self, @@ -60,28 +68,19 @@ def __init__( celery_task=celery_task, ) - @staticmethod - def get_checkers() -> Tuple[Type[Checker], ...]: - """We react only on finished builds (=KojiBuildState.complete) - and configured branches. - """ - return (IsKojiBuildCompleteAndBranchConfigured,) - def run(self) -> TaskResults: try: self.packit_api.create_update( - dist_git_branch=self.koji_build_event.git_ref, + dist_git_branch=self.dist_git_branch, update_type="enhancement", - koji_builds=[ - self.koji_build_event.nvr # it accepts NVRs, not build IDs - ], + koji_builds=[self.nvr], # it accepts NVRs, not build IDs ) except PackitException as ex: logger.debug(f"Bodhi update failed to be created: {ex}") if isinstance(ex.__cause__, AuthError): body = ( - f"Bodhi update creation failed for `{self.koji_build_event.nvr}` " + f"Bodhi update creation failed for `{self.nvr}` " f"because of the missing permissions.\n\n" f"Please, give {self.service_config.fas_user} user `commit` rights in the " f"[dist-git settings]({self.data.project_url}/adduser).\n\n" @@ -110,7 +109,7 @@ def run(self) -> TaskResults: known_error = True else: body = ( - f"Bodhi update creation failed for `{self.koji_build_event.nvr}`:\n" + f"Bodhi update creation failed for `{self.nvr}`:\n" "```\n" f"{ex}\n" "```" @@ -167,3 +166,48 @@ def notify_user_about_failure(self, body: str) -> None: + f"\n\n---\n\n*Get in [touch with us]({CONTACTS_URL}) if you need some help.*", comment_to_existing=body, ) + + +@configured_as(job_type=JobType.bodhi_update) +@reacts_to(event=KojiBuildEvent) +class CreateBodhiUpdateHandler( + BodhiUpdateHandler, + RetriableJobHandler, + GetKojiBuildEventMixin, + GetKojiBuildDataFromKojiBuildEventMixin, +): + """ + This handler can create a bodhi update for successful Koji builds. + """ + + task_name = TaskName.bodhi_update + + @staticmethod + def get_checkers() -> Tuple[Type[Checker], ...]: + """We react only on finished builds (=KojiBuildState.complete) + and configured branches. + """ + return (IsKojiBuildCompleteAndBranchConfiguredCheckEvent,) + + +@configured_as(job_type=JobType.bodhi_update) +@reacts_to(event=PullRequestCommentPagureEvent) +@run_for_comment(command="create-update") +class RetriggerBodhiUpdateHandler( + BodhiUpdateHandler, GetKojiBuildDataFromKojiServiceMixin +): + """ + This handler can re-trigger a bodhi update if any successfull Koji build. + """ + + task_name = TaskName.retrigger_bodhi_update + + @staticmethod + def get_checkers() -> Tuple[Type[Checker], ...]: + """We react only on finished builds (=KojiBuildState.complete) + and configured branches. + """ + return ( + HasAuthorWriteAccess, + IsKojiBuildCompleteAndBranchConfiguredCheckService, + ) diff --git a/packit_service/worker/handlers/mixin.py b/packit_service/worker/handlers/mixin.py index e1a8614f1e..3b0c8dbd92 100644 --- a/packit_service/worker/handlers/mixin.py +++ b/packit_service/worker/handlers/mixin.py @@ -1,13 +1,15 @@ # Copyright Contributors to the Packit project. # SPDX-License-Identifier: MIT -from abc import abstractmethod import logging -from typing import Optional, Protocol +from abc import abstractmethod +from typing import Any, Optional, Protocol +from koji import ClientSession from packit.config import PackageConfig, JobConfig +from packit.constants import KOJI_BASEURL from packit_service.utils import get_packit_commands_from_comment from packit_service.config import ProjectToSync -from packit_service.constants import COPR_SRPM_CHROOT +from packit_service.constants import COPR_SRPM_CHROOT, KojiBuildState from packit_service.models import ( AbstractTriggerDbType, CoprBuildTargetModel, @@ -27,6 +29,8 @@ from packit_service.worker.events.koji import KojiBuildEvent from packit_service.worker.monitoring import Pushgateway +from packit.distgit import DistGit + logger = logging.getLogger(__name__) @@ -80,6 +84,83 @@ def koji_build_helper(self) -> KojiBuildJobHelper: return self._koji_build_helper +class GetKojiBuildData(Protocol): + @property + @abstractmethod + def nvr(self) -> str: + ... + + @property + @abstractmethod + def build_id(self) -> int: + ... + + @property + @abstractmethod + def dist_git_branch(self) -> str: + ... + + @property + @abstractmethod + def state(self) -> KojiBuildState: + ... + + +class GetKojiBuildDataFromKojiBuildEventMixin(GetKojiBuildData, GetKojiBuildEvent): + @property + def nvr(self) -> str: + return self.koji_build_event.nvr + + @property + def build_id(self) -> int: + return self.koji_build_event.build_id + + @property + def dist_git_branch(self) -> str: + return self.koji_build_event.git_ref + + @property + def state(self) -> KojiBuildState: + return self.koji_build_event.state + + +class GetKojiBuildDataFromKojiServiceMixin(ConfigMixin, GetKojiBuildData): + _nvr: Optional[str] = None + _build: Optional[Any] = None + + def _query_service_for_nvr(self): + self._nvr = DistGit.get_latest_build_in_tag( + downstream_package_name=self.project.repo, + dist_git_branch=self.dist_git_branch, + ) + + def _query_service_for_build(self): + session = ClientSession(baseurl=KOJI_BASEURL) + self._build = session.getBuild(buildInfo=self.nvr) + + @property + def nvr(self) -> str: + if not self._nvr: + self._query_service_for_nvr() + return self._nvr + + @property + def build_id(self) -> int: + if not self._build: + self._query_service_for_build() + return self._build["build_id"] + + @property + def dist_git_branch(self) -> str: + return self.project.get_pr(self.data.pr_id).target_branch + + @property + def state(self) -> KojiBuildState: + if not self._build: + self._query_service_for_build() + return KojiBuildState.from_number(self._build["state"]) + + class GetCoprBuildEvent(Protocol): data: EventData diff --git a/packit_service/worker/jobs.py b/packit_service/worker/jobs.py index 6ed65ff191..290f973180 100644 --- a/packit_service/worker/jobs.py +++ b/packit_service/worker/jobs.py @@ -470,7 +470,7 @@ def is_project_public_or_enabled_private(self) -> bool: return True - def check_explicit_matching(self): + def check_explicit_matching(self) -> List[JobConfig]: """Force explicit event/jobs matching for triggers Returns: @@ -480,7 +480,7 @@ def check_explicit_matching(self): if isinstance(self.event, PullRequestCommentPagureEvent): for job in self.event.package_config.jobs: if ( - job.type == JobType.koji_build + job.type in [JobType.koji_build, JobType.bodhi_update] and job.trigger == JobConfigTriggerType.commit and self.event.job_config_trigger_type == JobConfigTriggerType.pull_request diff --git a/packit_service/worker/tasks.py b/packit_service/worker/tasks.py index 92c66ebd3e..784746a8ef 100644 --- a/packit_service/worker/tasks.py +++ b/packit_service/worker/tasks.py @@ -32,7 +32,10 @@ TestingFarmResultsHandler, ) from packit_service.worker.handlers.abstract import TaskName -from packit_service.worker.handlers.bodhi import CreateBodhiUpdateHandler +from packit_service.worker.handlers.bodhi import ( + CreateBodhiUpdateHandler, + RetriggerBodhiUpdateHandler, +) from packit_service.worker.handlers.distgit import DownstreamKojiBuildHandler from packit_service.worker.handlers.forges import GithubFasVerificationHandler from packit_service.worker.handlers.koji import KojiBuildReportHandler @@ -312,6 +315,24 @@ def run_bodhi_update(self, event: dict, package_config: dict, job_config: dict): return get_handlers_task_results(handler.run_job(), event) +@celery_app.task( + bind=True, + name=TaskName.retrigger_bodhi_update, + base=HandlerTaskWithRetry, + queue="long-running", +) +def run_retrigger_bodhi_update( + self, event: dict, package_config: dict, job_config: dict +): + handler = RetriggerBodhiUpdateHandler( + package_config=load_package_config(package_config), + job_config=load_job_config(job_config), + event=event, + celery_task=self, + ) + return get_handlers_task_results(handler.run_job(), event) + + def get_handlers_task_results(results: dict, event: dict) -> dict: # include original event to provide more info return {"job": results, "event": event} diff --git a/tests/conftest.py b/tests/conftest.py index dcb3d97413..04c84706f5 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -429,3 +429,9 @@ def pytest_assertrepr_compare(op, left, right): schema = PackageConfigSchema() return [str(DeepDiff(schema.dump(left), schema.dump(right)))] + + +@pytest.fixture() +def pagure_pr_comment_added(): + with open(DATA_DIR / "fedmsg" / "pagure_pr_comment.json") as outfile: + return json.load(outfile) diff --git a/tests/integration/test_pr_comment.py b/tests/integration/test_pr_comment.py index 0199a911e9..a1adf109a1 100644 --- a/tests/integration/test_pr_comment.py +++ b/tests/integration/test_pr_comment.py @@ -8,13 +8,20 @@ from celery.canvas import Signature from flexmock import flexmock from github import Github -from ogr.services.github import GithubProject -from ogr.utils import RequestResponse import packit_service.models import packit_service.service.urls as urls -from packit.config import JobConfigTriggerType + +from ogr.utils import RequestResponse +from ogr.services.pagure import PagureProject +from ogr.services.github import GithubProject +from packit.api import PackitAPI +from packit.config import ( + JobConfigTriggerType, +) +from packit.constants import DEFAULT_BODHI_NOTE from packit.exceptions import PackitConfigException +from packit.distgit import DistGit from packit.local_project import LocalProject from packit_service.config import ServiceConfig from packit_service.constants import ( @@ -41,6 +48,9 @@ from packit_service.worker.allowlist import Allowlist from packit_service.worker.celery_task import CeleryTask from packit_service.worker.events.event import AbstractForgeIndependentEvent +from packit_service.worker.handlers.bodhi import ( + RetriggerBodhiUpdateHandler, +) from packit_service.worker.helpers.build import copr_build from packit_service.worker.helpers.build.copr_build import CoprBuildJobHelper from packit_service.worker.helpers.build.koji_build import KojiBuildJobHelper @@ -53,6 +63,7 @@ from packit_service.worker.tasks import ( run_copr_build_handler, run_koji_build_handler, + run_retrigger_bodhi_update, run_testing_farm_handler, ) from tests.spellbook import DATA_DIR, first_dict_value, get_parameters_from_results @@ -2447,3 +2458,75 @@ def test_pr_test_command_handler_multiple_builds(pr_embedded_command_comment_eve event=event_dict, job_config=job_config, ) + + +def test_bodhi_update_retrigger_via_dist_git_pr_comment(pagure_pr_comment_added): + pagure_pr_comment_added["pullrequest"]["comments"][0][ + "comment" + ] = "/packit create-update" + project = pagure_pr_comment_added["pullrequest"]["project"] + project["full_url"] = "https://src.fedoraproject.org/rpms/jouduv-dort" + project["fullname"] = "rpms/jouduv-dort" + project["name"] = "jouduv-dort" + project["url_path"] = "rpms/jouduv-dort" + + packit_yaml = ( + "{'specfile_path': 'jouduv-dort.spec', 'synced_files': []," + "'jobs': [{'trigger': 'commit', 'job': 'bodhi_update'}]," + "'downstream_package_name': 'jouduv-dort'}" + ) + + trigger = flexmock( + job_config_trigger_type=JobConfigTriggerType.pull_request, id=123 + ) + flexmock(AddPullRequestDbTrigger).should_receive("db_trigger").and_return(trigger) + flexmock(PullRequestModel).should_receive("get_by_id").with_args(123).and_return( + trigger + ) + + pagure_project = flexmock( + PagureProject, + full_repo_name="rpms/jouduv-dort", + get_web_url=lambda: "https://src.fedoraproject.org/rpms/jouduv-dort", + default_branch="main", + get_pr=lambda id: flexmock(target_branch="the_distgit_branch"), + ) + + flexmock(DistGit).should_receive("get_latest_build_in_tag").and_return("123") + + pagure_project.should_receive("get_files").with_args( + ref="beaf90bcecc51968a46663f8d6f092bfdc92e682", filter_regex=r".+\.spec$" + ).and_return(["jouduv-dort.spec"]) + pagure_project.should_receive("get_file_content").with_args( + path=".distro/source-git.yaml", + ref="beaf90bcecc51968a46663f8d6f092bfdc92e682", + ).and_raise(FileNotFoundError, "Not found.") + pagure_project.should_receive("get_file_content").with_args( + path=".packit.yaml", ref="beaf90bcecc51968a46663f8d6f092bfdc92e682" + ).and_return(packit_yaml) + + flexmock(RetriggerBodhiUpdateHandler).should_receive("pre_check").and_return(True) + + flexmock(LocalProject, refresh_the_arguments=lambda: None) + flexmock(Signature).should_receive("apply_async").once() + flexmock(PackitAPI).should_receive("create_update").with_args( + dist_git_branch="the_distgit_branch", + update_type="enhancement", + update_notes=DEFAULT_BODHI_NOTE, + koji_builds=["123"], + ).once() + + flexmock(Pushgateway).should_receive("push").once().and_return() + + processing_results = SteveJobs().process_message(pagure_pr_comment_added) + event_dict, job, job_config, package_config = get_parameters_from_results( + processing_results + ) + assert json.dumps(event_dict) + results = run_retrigger_bodhi_update( + event=event_dict, + package_config=package_config, + job_config=job_config, + ) + + assert first_dict_value(results["job"])["success"] diff --git a/tests/unit/test_bodhi_update_checks.py b/tests/unit/test_bodhi_update_checks.py new file mode 100644 index 0000000000..fa8d4777f0 --- /dev/null +++ b/tests/unit/test_bodhi_update_checks.py @@ -0,0 +1,72 @@ +import pytest + +from flexmock import flexmock + +from packit_service.worker.checker.bodhi import HasAuthorWriteAccess +from packit.config import ( + JobConfig, + JobConfigTriggerType, + JobType, + PackageConfig, + CommonPackageConfig, +) + + +@pytest.mark.parametrize( + "event_type, has_write_access, result", + [ + pytest.param( + "PullRequestCommentPagureEvent", + True, + True, + ), + pytest.param( + "PullRequestCommentPagureEvent", + False, + False, + ), + pytest.param( + "AnotherEvent", + True, + True, + ), + ], +) +def test_check_has_author_write_access( + event_type: str, has_write_access: bool, result: bool +): + package_config = PackageConfig( + jobs=[ + JobConfig( + type=JobType.bodhi_update, + trigger=JobConfigTriggerType.pull_request, + packages={ + "package": CommonPackageConfig( + identifier="first", + ) + }, + ), + ] + ) + job_config = JobConfig( + type=JobType.bodhi_update, + trigger=JobConfigTriggerType.pull_request, + packages={ + "package": CommonPackageConfig( + identifier="first", + ) + }, + ) + data = dict( + event_type=event_type, + actor="happy-packit-user", + pr_id=123, + ) + project = flexmock( + has_write_access=lambda user: has_write_access, + repo="playground-for-pencils", + ) + + checker = HasAuthorWriteAccess(package_config, job_config, data) + checker._project = project + assert checker.pre_check() == result diff --git a/tests/unit/test_events.py b/tests/unit/test_events.py index 3d2e7a9dd4..34fa42225b 100644 --- a/tests/unit/test_events.py +++ b/tests/unit/test_events.py @@ -190,11 +190,6 @@ def pagure_pr_flag_updated(self): with open(DATA_DIR / "fedmsg" / "pagure_pr_flag_updated.json") as outfile: return json.load(outfile) - @pytest.fixture() - def pagure_pr_comment_added(self): - with open(DATA_DIR / "fedmsg" / "pagure_pr_comment.json") as outfile: - return json.load(outfile) - @pytest.fixture() def distgit_commit(self): with open(DATA_DIR / "fedmsg" / "distgit_commit.json") as outfile: diff --git a/tests/unit/test_jobs.py b/tests/unit/test_jobs.py index e62cad4774..c2aad6dfb9 100644 --- a/tests/unit/test_jobs.py +++ b/tests/unit/test_jobs.py @@ -2958,6 +2958,33 @@ def __init__(self): ], {"job_identifier": "first"}, ), + pytest.param( + PullRequestCommentPagureEvent, + JobConfigTriggerType.pull_request, + [ + JobConfig( + type=JobType.bodhi_update, + trigger=JobConfigTriggerType.commit, + packages={ + "package": CommonPackageConfig( + identifier="first", + ) + }, + ), + ], + [ + JobConfig( + type=JobType.bodhi_update, + trigger=JobConfigTriggerType.commit, + packages={ + "package": CommonPackageConfig( + identifier="first", + ) + }, + ), + ], + {}, + ), ], ) def test_get_jobs_matching_trigger(