Conditions on PR labels (#2333)

Conditions on PR labels

TODO:

 docs

Fixes #2269 #2186

RELEASE NOTES BEGIN
We have introduced new configuration options require.label.present and require.label.absent. By configuring these you can specify labels that need to be present or absent on a pull request for Packit to react on such PR.
RELEASE NOTES END

Reviewed-by: František Lachman <[email protected]>

packit_service/utils.py

@@ -6,8 +6,9 @@
 from io import StringIO
 from logging import StreamHandler
 from re import search
-from typing import List, Tuple
+from typing import List, Tuple, Optional
 
+from ogr.abstract import PullRequest
 from packit.config import JobConfig, PackageConfig
 from packit.schema import JobConfigSchema, PackageConfigSchema
 from packit.utils import PackitFormatter
@@ -222,3 +223,33 @@ def get_koji_task_id_and_url_from_stdout(stdout: str):
         task_url = task_url_match.group(0)
 
     return task_id, task_url
+
+
+def pr_labels_match_configuration(
+    pull_request: Optional[PullRequest],
+    configured_labels_present: list[str],
+    configured_labels_absent: list[str],
+) -> bool:
+    """
+    Do the PR labels match the configuration of the labels?
+    """
+    if not pull_request:
+        logger.debug("No PR to check the labels on.")
+        return True
+
+    logger.info(
+        f"About to check whether PR labels in PR {pull_request.id} "
+        f"match to the labels configuration "
+        f"(label.present: {configured_labels_present}, label.absent: {configured_labels_absent})"
+    )
+
+    pr_labels = pull_request.labels
+    logger.info(f"Labels on PR: {pr_labels}")
+
+    return (
+        not configured_labels_present
+        or any(label in pr_labels for label in configured_labels_present)
+    ) and (
+        not configured_labels_absent
+        or all(label not in pr_labels for label in configured_labels_absent)
+    )

packit_service/worker/checker/distgit.py

@@ -7,6 +7,7 @@
 from ogr.abstract import AccessLevel
 from packit.config.aliases import get_branches
 from packit_service.constants import MSG_GET_IN_TOUCH, KojiAllowedAccountsAlias
+from packit_service.utils import pr_labels_match_configuration
 from packit_service.worker.checker.abstract import Checker, ActorChecker
 from packit_service.worker.events import (
     PushPagureEvent,
@@ -24,6 +25,21 @@
 logger = logging.getLogger(__name__)
 
 
+class LabelsOnDistgitPR(Checker, GetPagurePullRequestMixin):
+    def pre_check(self) -> bool:
+        if self.data.event_type not in (PushPagureEvent.__name__,) or not (
+            self.job_config.require.label.present
+            or self.job_config.require.label.absent
+        ):
+            return True
+
+        return pr_labels_match_configuration(
+            self.pull_request,
+            self.job_config.require.label.present,
+            self.job_config.require.label.absent,
+        )
+
+
 class PermissionOnDistgit(Checker, GetPagurePullRequestMixin):
     def contains_specfile_change(self):
         """

packit_service/worker/events/comment.py

@@ -7,8 +7,7 @@
 from logging import getLogger
 from typing import Dict, Optional, Set
 
-from ogr.abstract import Comment, PullRequest, Issue
-
+from ogr.abstract import Comment, Issue
 from packit_service.models import TestingFarmResult, BuildStatus
 from packit_service.service.db_project_events import (
     AddIssueEventToDb,
@@ -70,7 +69,6 @@ def __init__(
         self._comment_object = comment_object
         self._build_targets_override = build_targets_override
         self._tests_targets_override = tests_targets_override
-        self._pull_request_object = None
 
     @property
     def commit_sha(self) -> str:  # type:ignore
@@ -79,12 +77,6 @@ def commit_sha(self) -> str:  # type:ignore
             self._commit_sha = self.project.get_pr(pr_id=self.pr_id).head_commit
         return self._commit_sha
 
-    @property
-    def pull_request_object(self) -> PullRequest:
-        if not self._pull_request_object:
-            self._pull_request_object = self.project.get_pr(self.pr_id)
-        return self._pull_request_object
-
     @property
     def comment_object(self) -> Optional[Comment]:
         if not self._comment_object:
@@ -121,7 +113,6 @@ def get_dict(self, default_dict: Optional[Dict] = None) -> dict:
         result["commit_sha"] = self.commit_sha
         result.pop("_build_targets_override")
         result.pop("_tests_targets_override")
-        result.pop("_pull_request_object")
         return result
 
 

packit_service/worker/events/event.py

@@ -9,7 +9,7 @@
 from logging import getLogger
 from typing import Dict, Optional, Type, Union, Set, List
 
-from ogr.abstract import GitProject
+from ogr.abstract import GitProject, PullRequest
 from ogr.parsing import RepoUrl
 
 from packit.config import JobConfigTriggerType, PackageConfig
@@ -461,6 +461,7 @@ def __init__(
         self._pr_id = pr_id
         self.fail_when_config_file_missing = False
         self.actor = actor
+        self._pull_request_object = None
 
     @property
     def project(self):
@@ -491,6 +492,12 @@ def get_db_project_event(self) -> Optional[ProjectEventModel]:
     def pr_id(self) -> Optional[int]:
         return self._pr_id
 
+    @property
+    def pull_request_object(self) -> Optional[PullRequest]:
+        if not self._pull_request_object and self.pr_id:
+            self._pull_request_object = self.project.get_pr(self.pr_id)
+        return self._pull_request_object
+
     def get_project(self) -> Optional[GitProject]:
         if not (self.project_url or self.db_project_object):
             return None
@@ -552,6 +559,11 @@ def get_all_build_targets_by_status(
             statuses_to_filter_with=statuses_to_filter_with,
         )
 
+    def get_dict(self, default_dict: Optional[Dict] = None) -> dict:
+        result = super().get_dict()
+        result.pop("_pull_request_object")
+        return result
+
 
 class AbstractResultEvent(AbstractForgeIndependentEvent):
     """

packit_service/worker/handlers/distgit.py

@@ -60,6 +60,7 @@
     ValidInformationForPullFromUpstream,
     HasIssueCommenterRetriggeringPermissions,
     IsUpstreamTagMatchingConfig,
+    LabelsOnDistgitPR,
 )
 from packit_service.worker.events import (
     PushPagureEvent,
@@ -678,7 +679,11 @@ def __init__(
 
     @staticmethod
     def get_checkers() -> Tuple[Type[Checker], ...]:
-        return (PermissionOnDistgit, HasIssueCommenterRetriggeringPermissions)
+        return (
+            LabelsOnDistgitPR,
+            PermissionOnDistgit,
+            HasIssueCommenterRetriggeringPermissions,
+        )
 
     def _get_or_create_koji_group_model(self) -> KojiBuildGroupModel:
         if self._koji_group_model_id is not None:

packit_service/worker/jobs.py

@@ -24,7 +24,11 @@
     COMMENT_REACTION,
     PACKIT_VERIFY_FAS_COMMAND,
 )
-from packit_service.utils import get_packit_commands_from_comment, elapsed_seconds
+from packit_service.utils import (
+    get_packit_commands_from_comment,
+    elapsed_seconds,
+    pr_labels_match_configuration,
+)
 from packit_service.worker.allowlist import Allowlist
 from packit_service.worker.events import (
     Event,
@@ -39,7 +43,10 @@
     AbstractIssueCommentEvent,
     AbstractPRCommentEvent,
 )
-from packit_service.worker.events.event import AbstractResultEvent
+from packit_service.worker.events.event import (
+    AbstractResultEvent,
+    AbstractForgeIndependentEvent,
+)
 from packit_service.worker.handlers import (
     CoprBuildHandler,
     GithubAppInstallationHandler,
@@ -666,6 +673,16 @@ def get_jobs_matching_event(self) -> List[JobConfig]:
                         for event_type in MANUAL_OR_RESULT_EVENTS
                     )
                 )
+                and (
+                    job.trigger != JobConfigTriggerType.pull_request
+                    or not (job.require.label.present or job.require.label.absent)
+                    or not isinstance(self.event, AbstractForgeIndependentEvent)
+                    or pr_labels_match_configuration(
+                        pull_request=self.event.pull_request_object,
+                        configured_labels_absent=job.require.label.absent,
+                        configured_labels_present=job.require.label.present,
+                    )
+                )
             ):
                 jobs_matching_trigger.append(job)
 

tests/unit/test_babysit_vm_image.py

@@ -7,6 +7,7 @@
 from flexmock import flexmock
 from flexmock import Mock
 from packit.config.job_config import JobConfigTriggerType, JobType
+from packit.config.requirements import RequirementsConfig, LabelRequirementsConfig
 from packit_service.config import ServiceConfig
 from packit_service.models import (
     VMImageBuildTargetModel,
@@ -117,6 +118,12 @@ def test_update_vm_image_build(stop_babysitting, build_status, vm_image_builder_
                     trigger=JobConfigTriggerType.pull_request,
                     type=JobType.vm_image_build,
                     manual_trigger=False,
+                    require=RequirementsConfig(
+                        LabelRequirementsConfig(
+                            absent=[],
+                            present=[],
+                        )
+                    ),
                 )
             ],
         )

tests/unit/test_checkers.py

@@ -6,7 +6,7 @@
 from flexmock import flexmock
 
 from ogr import PagureService
-from ogr.abstract import AccessLevel
+from ogr.abstract import AccessLevel, PRStatus
 from ogr.services.pagure import PagureProject
 from packit.config import (
     CommonPackageConfig,
@@ -18,6 +18,7 @@
 )
 
 from packit.config.commands import TestCommandConfig
+from packit.config.requirements import RequirementsConfig, LabelRequirementsConfig
 from packit_service.config import ServiceConfig
 from packit_service.models import CoprBuildTargetModel
 from packit_service.worker.checker.copr import (
@@ -27,6 +28,7 @@
 from packit_service.worker.checker.distgit import (
     IsUpstreamTagMatchingConfig,
     PermissionOnDistgit,
+    LabelsOnDistgitPR,
 )
 from packit_service.worker.checker.koji import (
     IsJobConfigTriggerMatching as IsJobConfigTriggerMatchingKoji,
@@ -992,3 +994,61 @@ def test_koji_check_allowed_accounts(
         package_config, job_config, distgit_push_event.get_dict()
     )
     assert checker.check_allowed_accounts(allowed_pr_authors, account) == should_pass
+
+
+@pytest.mark.parametrize(
+    "pr_labels,labels_present,labels_absent,should_pass",
+    (
+        ([], [], [], True),
+        (["allowed-1"], [], ["skip-ci"], True),
+        (["allowed-1"], ["allowed-1"], ["skip-ci"], True),
+        (["allowed-1"], ["allowed-1"], ["skip-ci"], True),
+        (["allowed-1", "skip-ci"], ["allowed-1"], ["skip-ci"], False),
+    ),
+)
+def test_labels_on_distgit_pr(
+    distgit_push_event,
+    pr_labels,
+    labels_present,
+    labels_absent,
+    should_pass,
+):
+    jobs = [
+        JobConfig(
+            type=JobType.koji_build,
+            trigger=JobConfigTriggerType.commit,
+            packages={
+                "package": CommonPackageConfig(
+                    dist_git_branches=["f36"],
+                    require=RequirementsConfig(
+                        LabelRequirementsConfig(
+                            absent=labels_absent,
+                            present=labels_present,
+                        )
+                    ),
+                )
+            },
+        ),
+    ]
+
+    package_config = PackageConfig(
+        jobs=jobs,
+        packages={"package": CommonPackageConfig()},
+    )
+    job_config = jobs[0]
+
+    flexmock(PagureProject).should_receive("get_pr_list").and_return(
+        [
+            flexmock(
+                id=5,
+                head_commit="ad0c308af91da45cf40b253cd82f07f63ea9cbbf",
+                status=PRStatus.open,
+                labels=pr_labels,
+            )
+        ]
+    )
+
+    checker = LabelsOnDistgitPR(
+        package_config, job_config, distgit_push_event.get_dict()
+    )
+    assert checker.pre_check() == should_pass