Migrate to org.fedoraproject.prod.pagure.git.receive (#2351)

  Migrate to org.fedoraproject.prod.pagure.git.receive

Supersedes #2303
Related to #2305

RELEASE NOTES BEGIN
N/A
RELEASE NOTES END

Reviewed-by: Laura Barcziová
Reviewed-by: Nikola Forró

Author: softwarefactory-project-zuul[bot]

packit_service/worker/checker/distgit.py

@@ -41,25 +41,6 @@ def pre_check(self) -> bool:
 
 
 class PermissionOnDistgit(Checker, GetPagurePullRequestMixin):
-    def contains_specfile_change(self):
-        """
-        Check whether the dist-git commit contains
-        any specfile change (do the check only for pushes from PRs,
-        with direct pushes we do the filtering in fedmsg).
-        """
-        if not self.pull_request:
-            return True
-
-        pr_id = self.pull_request.id
-        logger.debug(f"PR {pr_id} status: {self.pull_request.status}")
-        # Pagure API tends to return ENOPRSTATS error when a pull request is transitioning
-        # from open to merged state, give it some extra time
-        diff = self.project.get_pr_files_diff(pr_id, retries=7, wait_seconds=7) or {}
-        if not any(change.endswith(".spec") for change in diff):
-            logger.info(f"PR {pr_id} does not contain a specfile change.")
-            return False
-        return True
-
     @staticmethod
     def is_koji_allowed_accounts_alias(value: str) -> bool:
         return any(value == alias.value for alias in KojiAllowedAccountsAlias)
@@ -139,17 +120,7 @@ def pre_check(self) -> bool:
                 )
                 return False
 
-            if self.data.event_dict["committer"] == "pagure":
-                if not self.pull_request:
-                    logger.debug(
-                        "Not able to get the pull request "
-                        "(may not be the head commit of the PR)."
-                    )
-                    return False
-
-                if not self.contains_specfile_change():
-                    return False
-
+            if self.pull_request:
                 pr_author = self.get_pr_author()
                 logger.debug(f"PR author: {pr_author}")
                 if not self.check_allowed_accounts(
@@ -162,6 +133,9 @@ def pre_check(self) -> bool:
                     )
                     return False
             else:
+                logger.debug(
+                    "Not able to get the pull request, we are handling direct push."
+                )
                 committer = self.data.event_dict["committer"]
                 logger.debug(f"Committer: {committer}")
                 if not self.check_allowed_accounts(

packit_service/worker/events/enums.py

@@ -28,6 +28,6 @@ class IssueCommentAction(Enum):
 
 
 class FedmsgTopic(Enum):
-    dist_git_push = "org.fedoraproject.prod.git.receive"
+    dist_git_push = "org.fedoraproject.prod.pagure.git.receive"
     copr_build_finished = "org.fedoraproject.prod.copr.build.end"
     copr_build_started = "org.fedoraproject.prod.copr.build.start"

packit_service/worker/handlers/distgit.py

@@ -504,7 +504,7 @@ class AbortSyncRelease(Exception):
 @reacts_to(event=AbstractIssueCommentEvent)
 @reacts_to(event=CheckRerunReleaseEvent)
 class ProposeDownstreamHandler(AbstractSyncReleaseHandler):
-    topic = "org.fedoraproject.prod.git.receive"
+    topic = "org.fedoraproject.prod.pagure.git.receive"
     task_name = TaskName.propose_downstream
     helper_kls = ProposeDownstreamJobHelper
     sync_release_job_type = SyncReleaseJobType.propose_downstream
@@ -664,7 +664,7 @@ class AbstractDownstreamKojiBuildHandler(
     This handler can submit a build in Koji from a dist-git.
     """
 
-    topic = "org.fedoraproject.prod.git.receive"
+    topic = "org.fedoraproject.prod.pagure.git.receive"
     task_name = TaskName.downstream_koji_build
 
     def __init__(

packit_service/worker/mixin.py

@@ -254,7 +254,7 @@ class GetPagurePullRequestMixin(GetPagurePullRequest):
 
     @property
     def pull_request(self):
-        if not self._pull_request and self.data.event_dict["committer"] == "pagure":
+        if not self._pull_request:
             logger.debug(
                 f"Getting pull request with head commit {self.data.commit_sha}"
                 f"for repo {self.project.namespace}/{self.project.repo}"

packit_service/worker/parser.py

@@ -1078,25 +1078,25 @@ def parse_release_event(event) -> Optional[ReleaseEvent]:
     def parse_pagure_push_event(event) -> Optional[PushPagureEvent]:
         """this corresponds to dist-git event when someone pushes new commits"""
         topic = event.get("topic")
-        if topic != "org.fedoraproject.prod.git.receive":
+        if topic != "org.fedoraproject.prod.pagure.git.receive":
             return None
 
         logger.info(f"Dist-git commit event, topic: {topic}")
 
-        dg_repo_namespace = nested_get(event, "commit", "namespace")
-        dg_repo_name = nested_get(event, "commit", "repo")
+        dg_repo_namespace = nested_get(event, "repo", "namespace")
+        dg_repo_name = nested_get(event, "repo", "name")
 
         if not (dg_repo_namespace and dg_repo_name):
             logger.warning("No full name of the repository.")
             return None
 
-        dg_branch = nested_get(event, "commit", "branch")
-        dg_commit = nested_get(event, "commit", "rev")
+        dg_branch = nested_get(event, "branch")
+        dg_commit = nested_get(event, "end_commit")
         if not (dg_branch and dg_commit):
             logger.warning("Target branch/rev for the new commits is not set.")
             return None
 
-        username = nested_get(event, "commit", "username")
+        username = nested_get(event, "agent")
 
         logger.info(
             f"New commits added to dist-git repo {dg_repo_namespace}/{dg_repo_name},"
@@ -1591,7 +1591,7 @@ def parse_new_hotness_update_event(event) -> Optional[NewHotnessUpdateEvent]:
             "pagure.pull-request.flag.added": parse_pagure_pr_flag_event.__func__,  # type: ignore
             "pagure.pull-request.flag.updated": parse_pagure_pr_flag_event.__func__,  # type: ignore
             "pagure.pull-request.comment.added": parse_pagure_pull_request_comment_event.__func__,  # type: ignore # noqa: E501
-            "git.receive": parse_pagure_push_event.__func__,  # type: ignore
+            "pagure.git.receive": parse_pagure_push_event.__func__,  # type: ignore
             "copr.build.start": parse_copr_event.__func__,  # type: ignore
             "copr.build.end": parse_copr_event.__func__,  # type: ignore
             "buildsys.task.state.change": parse_koji_task_event.__func__,  # type: ignore

tests/data/fedmsg/distgit_commit.json

@@ -1,42 +1,11 @@
 {
-  "topic": "org.fedoraproject.prod.git.receive",
-  "commit": {
-    "agent": "rhcontainerbot",
-    "branch": "main",
-    "email": "[email protected]",
-    "message": "buildah-1.12.0-0.73.dev.git1e6a70c\n\n- autobuilt 1e6a70c\n\nSigned-off-by: RH Container Bot <[email protected]>\n",
-    "name": "Packit",
+  "agent": "rhcontainerbot",
+  "branch": "main",
+  "end_commit": "abcd",
+  "total_commits": 1,
+  "repo": {
     "namespace": "rpms",
-    "path": "/srv/git/repositories/rpms/buildah.git",
-    "repo": "buildah",
-    "rev": "abcd",
-    "seen": false,
-    "stats": {
-      "files": {
-        ".gitignore": {
-          "additions": 1,
-          "deletions": 0,
-          "lines": 1
-        },
-        "buildah.spec": {
-          "additions": 5,
-          "deletions": 2,
-          "lines": 7
-        },
-        "sources": {
-          "additions": 1,
-          "deletions": 1,
-          "lines": 2
-        }
-      },
-      "total": {
-        "additions": 7,
-        "deletions": 3,
-        "files": 3,
-        "lines": 10
-      }
-    },
-    "summary": "buildah-1.12.0-0.73.dev.git1e6a70c",
-    "username": "rhcontainerbot"
-  }
+    "name": "buildah"
+  },
+  "topic": "org.fedoraproject.prod.pagure.git.receive"
 }

tests/data/fedmsg/distgit_push_packit.json

@@ -1,52 +1,11 @@
 {
-  "commit": {
-    "agent": "pagure",
-    "branch": "f36",
-    "email": "[email protected]",
-    "message": "[packit] 0.50.0 upstream release\n\nUpstream tag: 0.50.0\nUpstream commit: c3667a91\n\nSigned-off-by: Packit <[email protected]>\n",
-    "name": "Packit",
+  "agent": "pagure",
+  "branch": "f36",
+  "end_commit": "ad0c308af91da45cf40b253cd82f07f63ea9cbbf",
+  "total_commits": 1,
+  "repo": {
     "namespace": "rpms",
-    "path": "/srv/git/repositories/rpms/packit.git",
-    "repo": "packit",
-    "rev": "ad0c308af91da45cf40b253cd82f07f63ea9cbbf",
-    "seen": false,
-    "stats": {
-      "files": {
-        ".gitignore": {
-          "additions": 1,
-          "deletions": 0,
-          "lines": 1
-        },
-        "README.packit": {
-          "additions": 1,
-          "deletions": 1,
-          "lines": 2
-        },
-        "packit.spec": {
-          "additions": 9,
-          "deletions": 1,
-          "lines": 10
-        },
-        "plans/main.fmf": {
-          "additions": 1,
-          "deletions": 1,
-          "lines": 2
-        },
-        "sources": {
-          "additions": 1,
-          "deletions": 1,
-          "lines": 2
-        }
-      },
-      "total": {
-        "additions": 13,
-        "deletions": 4,
-        "files": 5,
-        "lines": 17
-      }
-    },
-    "summary": "[packit] 0.50.0 upstream release",
-    "username": "pagure"
+    "name": "packit"
   },
-  "topic": "org.fedoraproject.prod.git.receive"
+  "topic": "org.fedoraproject.prod.pagure.git.receive"
 }

tests/integration/test_dg_commit.py

@@ -288,6 +288,16 @@ def test_downstream_koji_build_failure_no_issue():
         get_web_url=lambda: "https://src.fedoraproject.org/rpms/buildah",
         default_branch="main",
     )
+    flexmock(PagureProject).should_receive("get_pr_list").and_return(
+        [
+            flexmock(
+                id=5,
+                author="author",
+                head_commit="not-the-same-commit-hash",
+                status=PRStatus.open,
+            )
+        ]
+    )
     pagure_project_mock.should_receive("get_files").with_args(
         ref="main", filter_regex=r".+\.spec$"
     ).and_return(["buildah.spec"])
@@ -377,6 +387,16 @@ def test_downstream_koji_build_failure_issue_created():
         get_web_url=lambda: "https://src.fedoraproject.org/rpms/buildah",
         default_branch="main",
     )
+    flexmock(PagureProject).should_receive("get_pr_list").and_return(
+        [
+            flexmock(
+                id=5,
+                author="author",
+                head_commit="not-the-same-commit-hash",
+                status=PRStatus.open,
+            )
+        ]
+    )
     pagure_project_mock.should_receive("get_files").with_args(
         ref="main", filter_regex=r".+\.spec$"
     ).and_return(["buildah.spec"])
@@ -472,6 +492,16 @@ def test_downstream_koji_build_failure_issue_comment():
         get_web_url=lambda: "https://src.fedoraproject.org/rpms/buildah",
         default_branch="main",
     )
+    flexmock(PagureProject).should_receive("get_pr_list").and_return(
+        [
+            flexmock(
+                id=5,
+                author="author",
+                head_commit="not-the-same-commit-hash",
+                status=PRStatus.open,
+            )
+        ]
+    )
     pagure_project_mock.should_receive("get_files").with_args(
         ref="main", filter_regex=r".+\.spec$"
     ).and_return(["buildah.spec"])
@@ -570,6 +600,16 @@ def test_downstream_koji_build_no_config():
         get_web_url=lambda: "https://src.fedoraproject.org/rpms/buildah",
         default_branch="main",
     )
+    flexmock(PagureProject).should_receive("get_pr_list").and_return(
+        [
+            flexmock(
+                id=5,
+                author="author",
+                head_commit="not-the-same-commit-hash",
+                status=PRStatus.open,
+            )
+        ]
+    )
     pagure_project.should_receive("get_files").with_args(
         ref="main", filter_regex=r".+\.spec$"
     ).and_return(["buildah.spec"])
@@ -642,6 +682,16 @@ def test_downstream_koji_build_where_multiple_branches_defined(jobs_config):
         f"'jobs': {jobs_config},"
         "'downstream_package_name': 'buildah'}"
     )
+    flexmock(PagureProject).should_receive("get_pr_list").and_return(
+        [
+            flexmock(
+                id=5,
+                author="author",
+                head_commit="not-the-same-commit-hash",
+                status=PRStatus.open,
+            )
+        ]
+    )
     pagure_project = flexmock(
         PagureProject,
         full_repo_name="rpms/buildah",
@@ -817,6 +867,16 @@ def test_precheck_koji_build_push(
     distgit_push_event, push_username, allowed_committers, should_pass
 ):
     distgit_push_event.committer = push_username
+    flexmock(PagureProject).should_receive("get_pr_list").and_return(
+        [
+            flexmock(
+                id=5,
+                author="author",
+                head_commit="not-the-same-commit-hash",
+                status=PRStatus.open,
+            )
+        ]
+    )
 
     flexmock(GitProjectModel).should_receive("get_or_create").with_args(
         namespace="rpms",
@@ -884,8 +944,6 @@ def test_precheck_koji_build_push_pr(
     allowed_pr_authors,
     should_pass,
 ):
-    distgit_push_event.committer = "pagure"
-
     flexmock(GitProjectModel).should_receive("get_or_create").with_args(
         namespace="rpms",
         project_url="https://src.fedoraproject.org/rpms/packit",

tests/unit/test_events.py

@@ -1454,11 +1454,6 @@ def test_distgit_pagure_push(self, distgit_commit):
         assert event_object.git_ref == "main"
         assert event_object.project_url == "https://src.fedoraproject.org/rpms/buildah"
 
-    def test_distgit_pagure_push_packit(self, distgit_push_packit):
-        event_object = Parser.parse_event(distgit_push_packit)
-        assert isinstance(event_object, PushPagureEvent)
-        assert event_object.committer == "pagure"
-
     def test_json_testing_farm_notification(
         self, testing_farm_notification, testing_farm_results
     ):