Add manual interference to scm_request_processor

If manual interference is needed for SCM request, smc_request_processor first
adds comment to ticket notifying the maintainers of fedora-scm-requests
repository and then waits for subsequent comment that will validate
the issue.

Signed-off-by: Michal Konečný <mkonecny@redhat.com>

tests/plugins/test_scm_request_processor.py

@@ -550,6 +550,167 @@ class TestCreateNewRepo:
             reason="Invalid",
         )
 
+    def test_create_new_repo_exception_validated_by_invalid_user(self):
+        """
+        Assert that processing will be interrupted if the ticket is validated by
+        wrong user.
+        """
+        # Preparation
+        user = "zlopez"
+        invalid_user = "Tzeentch"
+        issue = {
+            "id": 100,
+            "user": {"name": user},
+            "comments": [{
+                "comment": "valid",
+                "user": {
+                    "name": invalid_user
+                }
+            }]
+        }
+
+        repo = "repo"
+        branch = "main"
+        namespace = "tests"
+        bug_id = ""
+        action = "new_repo"
+        sls = {branch: "2050-06-01"}
+        monitor = "monitor"
+        exception = False
+        json = {
+            "repo": repo,
+            "branch": branch,
+            "namespace": namespace,
+            "bug_id": bug_id,
+            "action": action,
+            "sls": sls,
+            "monitor": monitor,
+            "exception": exception,
+        }
+        self.toddler.pagure_io.get_project_contributors.return_value = {
+            "users": {
+                "admin": [user],
+                "commit": [],
+                "ticket": []
+            }
+        }
+        self.toddler.validation_comment = "valid"
+
+        self.toddler.create_new_repo(issue, json)
+
+        # asserts
+        self.toddler.pagure_io.get_project_contributors.assert_called_with(
+            scm_request_processor.PROJECT_NAMESPACE.split("/")[0],
+            scm_request_processor.PROJECT_NAMESPACE.split("/")[1]
+        )
+
+    def test_create_new_repo_exception_no_validation_comment(self):
+        """
+        Assert that processing will be interrupted if the ticket validation comment
+        is not found.
+        """
+        # Preparation
+        user = "zlopez"
+        issue = {
+            "id": 100,
+            "user": {"name": user},
+            "comments": [{
+                "comment": "comment",
+                "user": {
+                    "name": user
+                }
+            }]
+        }
+
+        repo = "repo"
+        branch = "main"
+        namespace = "tests"
+        bug_id = ""
+        action = "new_repo"
+        sls = {branch: "2050-06-01"}
+        monitor = "monitor"
+        exception = False
+        json = {
+            "repo": repo,
+            "branch": branch,
+            "namespace": namespace,
+            "bug_id": bug_id,
+            "action": action,
+            "sls": sls,
+            "monitor": monitor,
+            "exception": exception,
+        }
+        self.toddler.pagure_io.get_project_contributors.return_value = {
+            "users": {
+                "admin": [user],
+                "commit": [],
+                "ticket": []
+            }
+        }
+        self.toddler.validation_comment = "valid"
+
+        self.toddler.create_new_repo(issue, json)
+
+        # asserts
+        self.toddler.pagure_io.get_project_contributors.assert_called_with(
+            scm_request_processor.PROJECT_NAMESPACE.split("/")[0],
+            scm_request_processor.PROJECT_NAMESPACE.split("/")[1]
+        )
+
+    def test_create_new_repo_exception_notify_maintainers(self):
+        """
+        Assert that comment will be added when the ticket needs manual
+        validation.
+        """
+        # Preparation
+        user = "zlopez"
+        issue = {
+            "id": 100,
+            "user": {"name": user},
+            "comments": []
+        }
+
+        repo = "repo"
+        branch = "main"
+        namespace = "tests"
+        bug_id = ""
+        action = "new_repo"
+        sls = {branch: "2050-06-01"}
+        monitor = "monitor"
+        exception = False
+        json = {
+            "repo": repo,
+            "branch": branch,
+            "namespace": namespace,
+            "bug_id": bug_id,
+            "action": action,
+            "sls": sls,
+            "monitor": monitor,
+            "exception": exception,
+        }
+        self.toddler.pagure_io.get_project_contributors.return_value = {
+            "users": {
+                "admin": [user, "Tzeentch"],
+                "commit": [],
+                "ticket": []
+            }
+        }
+        self.toddler.ping_comment = "Look at this comment {maintainers}"
+        self.toddler.create_new_repo(issue, json)
+
+        # asserts
+        self.toddler.pagure_io.get_project_contributors.assert_called_with(
+            scm_request_processor.PROJECT_NAMESPACE.split("/")[0],
+            scm_request_processor.PROJECT_NAMESPACE.split("/")[1]
+        )
+
+        message = "Look at this comment @{} @Tzeentch".format(user)
+
+        self.toddler.pagure_io.add_comment_to_issue.assert_called_with(
+            100, namespace=scm_request_processor.PROJECT_NAMESPACE,
+            comment=message
+        )
+
     def test_create_new_repo_missing_bug_id(self):
         """
         Assert that ticket will be closed if Bugzilla bug id is not provided.
@@ -612,7 +773,10 @@ class TestCreateNewRepo:
     @patch(
         "toddlers.plugins.scm_request_processor.SCMRequestProcessor.valid_epel_package"
     )
-    def test_create_new_repo_invalid_epel(self, mock_valid_epel_package):
+    @patch(
+        "toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug"
+    )
+    def test_create_new_repo_invalid_epel(self, mock_validate_review_bug, mock_valid_epel_package):
         """
         Assert that ticket will be closed if repo is invalid EPEL repo.
         """
@@ -625,7 +789,7 @@ class TestCreateNewRepo:
         action = "new_repo"
         sls = {"rawhide": "2050-06-01"}
         monitor = "monitor"
-        exception = True
+        exception = False
         json = {
             "repo": repo,
             "branch": branch,
@@ -643,6 +807,9 @@ class TestCreateNewRepo:
 
         mock_valid_epel_package.assert_called_with(repo, branch)
 
+        mock_validate_review_bug.assert_called_with(
+            bug_id, repo, branch, namespace=namespace, pagure_user="zlopez")
+
         self.toddler.pagure_io.close_issue.assert_called_with(
             100,
             namespace=scm_request_processor.PROJECT_NAMESPACE,
@@ -650,7 +817,10 @@ class TestCreateNewRepo:
             reason="Invalid",
         )
 
-    def test_create_new_repo_requester_not_in_dist_git(self):
+    @patch(
+        "toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug"
+    )
+    def test_create_new_repo_requester_not_in_dist_git(self, mock_validate_review_bug):
         """
         Assert that ticket will be commented on when requester doesn't
         have a valid dist git account.
@@ -664,7 +834,7 @@ class TestCreateNewRepo:
         action = "new_repo"
         sls = {"rawhide": "2050-06-01"}
         monitor = "monitor"
-        exception = True
+        exception = False
         json = {
             "repo": repo,
             "branch": branch,
@@ -681,6 +851,9 @@ class TestCreateNewRepo:
 
         self.toddler.create_new_repo(issue, json)
 
+        mock_validate_review_bug.assert_called_with(
+            bug_id, repo, branch, namespace=namespace, pagure_user="zlopez")
+
         self.toddler.dist_git.user_exists.assert_called_with("zlopez")
 
         message = "@zlopez needs to login to {0} to sync accounts before we can proceed.".format(
@@ -693,7 +866,10 @@ class TestCreateNewRepo:
             comment=message,
         )
 
-    def test_create_new_repo_project_exists(self):
+    @patch(
+        "toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug"
+    )
+    def test_create_new_repo_project_exists(self, mock_validate_review_bug):
         """
         Assert that ticket will be closed when repo already exists in dist git.
         """
@@ -706,7 +882,7 @@ class TestCreateNewRepo:
         action = "new_repo"
         sls = {"rawhide": "2050-06-01"}
         monitor = "monitor"
-        exception = True
+        exception = False
         json = {
             "repo": repo,
             "branch": branch,
@@ -722,6 +898,9 @@ class TestCreateNewRepo:
 
         self.toddler.create_new_repo(issue, json)
 
+        mock_validate_review_bug.assert_called_with(
+            bug_id, repo, branch, namespace=namespace, pagure_user="zlopez")
+
         self.toddler.dist_git.get_project.assert_called_with(namespace, repo)
 
         self.toddler.pagure_io.close_issue.assert_called_with(
@@ -731,7 +910,10 @@ class TestCreateNewRepo:
             reason="Invalid",
         )
 
-    def test_create_new_repo_master_branch(self):
+    @patch(
+        "toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug"
+    )
+    def test_create_new_repo_master_branch(self, mock_validate_review_bug):
         """
         Assert that ticket will be closed when branch is set to master branch.
         Master branch is no longer allowed.
@@ -745,7 +927,7 @@ class TestCreateNewRepo:
         action = "new_repo"
         sls = {"rawhide": "2050-06-01"}
         monitor = "monitor"
-        exception = True
+        exception = False
         json = {
             "repo": repo,
             "branch": branch,
@@ -760,6 +942,9 @@ class TestCreateNewRepo:
 
         self.toddler.create_new_repo(issue, json)
 
+        mock_validate_review_bug.assert_called_with(
+            bug_id, repo, branch, namespace=namespace, pagure_user="zlopez")
+
         self.toddler.dist_git.get_project.assert_called_with(namespace, repo)
 
         self.toddler.pagure_io.close_issue.assert_called_with(
@@ -769,7 +954,10 @@ class TestCreateNewRepo:
             reason="Invalid",
         )
 
-    def test_create_new_repo_unsupported_namespace(self):
+    @patch(
+        "toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug"
+    )
+    def test_create_new_repo_unsupported_namespace(self, mock_validate_review_bug):
         """
         Assert that ticket will be closed when requested namespace is not recognized.
         """
@@ -782,7 +970,7 @@ class TestCreateNewRepo:
         action = "new_repo"
         sls = {"rawhide": "2050-06-01"}
         monitor = "monitor"
-        exception = True
+        exception = False
         json = {
             "repo": repo,
             "branch": branch,
@@ -797,6 +985,9 @@ class TestCreateNewRepo:
 
         self.toddler.create_new_repo(issue, json)
 
+        mock_validate_review_bug.assert_called_with(
+            bug_id, repo, branch, namespace=namespace, pagure_user="zlopez")
+
         self.toddler.dist_git.get_project.assert_called_with(namespace, repo)
 
         error = (
@@ -813,7 +1004,10 @@ class TestCreateNewRepo:
         )
 
     @patch("toddlers.utils.pdc.get_branch")
-    def test_create_new_repo_branch_exist(self, mock_pdc_get_branch):
+    @patch(
+        "toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug"
+    )
+    def test_create_new_repo_branch_exist(self, mock_validate_review_bug, mock_pdc_get_branch):
         """
         Assert that ticket will be closed when branch already exist in PDC.
         """
@@ -826,7 +1020,7 @@ class TestCreateNewRepo:
         action = "new_repo"
         sls = {"rawhide": "2050-06-01"}
         monitor = "monitor"
-        exception = True
+        exception = False
         json = {
             "repo": repo,
             "branch": branch,
@@ -842,6 +1036,9 @@ class TestCreateNewRepo:
 
         self.toddler.create_new_repo(issue, json)
 
+        mock_validate_review_bug.assert_called_with(
+            bug_id, repo, branch, namespace=namespace, pagure_user="zlopez")
+
         self.toddler.dist_git.get_project.assert_called_with(namespace, repo)
         mock_pdc_get_branch.assert_called_with(repo, branch, namespace.rstrip("s"))
 
@@ -866,7 +1063,18 @@ class TestCreateNewRepo:
         """
         Assert that ticket will be processed when everything is in order and namespace is correct.
         """
-        issue = {"id": 100, "user": {"name": "zlopez"}}
+        # Preparation
+        user = "zlopez"
+        issue = {
+            "id": 100,
+            "user": {"name": user},
+            "comments": [{
+                "comment": "valid",
+                "user": {
+                    "name": user
+                }
+            }]
+        }
 
         repo = "repo"
         bug_id = ""
@@ -887,8 +1095,17 @@ class TestCreateNewRepo:
         dist_git_url = "https://src.fp.o"
         self.toddler.dist_git.get_project.return_value = None
         self.toddler.dist_git._pagure_url = dist_git_url
+        self.toddler.pagure_io.get_project_contributors.return_value = {
+            "users": {
+                "admin": [user],
+                "commit": [],
+                "ticket": []
+            }
+        }
+        self.toddler.validation_comment = "valid"
         mock_pdc.get_branch.return_value = None
 
+        # Method to test
         self.toddler.create_new_repo(issue, json)
 
         # asserts
@@ -929,7 +1146,18 @@ class TestCreateNewRepo:
         Assert that ticket will be processed when everything is in order and namespace
         is set to tests.
         """
-        issue = {"id": 100, "user": {"name": "zlopez"}}
+        # Preparation
+        user = "zlopez"
+        issue = {
+            "id": 100,
+            "user": {"name": user},
+            "comments": [{
+                "comment": "valid",
+                "user": {
+                    "name": user
+                }
+            }]
+        }
 
         repo = "repo"
         branch = "main"
@@ -938,7 +1166,7 @@ class TestCreateNewRepo:
         action = "new_repo"
         sls = {branch: "2050-06-01"}
         monitor = "monitor"
-        exception = True
+        exception = False
         json = {
             "repo": repo,
             "branch": branch,
@@ -952,11 +1180,23 @@ class TestCreateNewRepo:
         dist_git_url = "https://src.fp.o"
         self.toddler.dist_git.get_project.return_value = None
         self.toddler.dist_git._pagure_url = dist_git_url
+        self.toddler.pagure_io.get_project_contributors.return_value = {
+            "users": {
+                "admin": [user],
+                "commit": [],
+                "ticket": []
+            }
+        }
+        self.toddler.validation_comment = "valid"
         mock_pdc.get_branch.return_value = None
 
         self.toddler.create_new_repo(issue, json)
 
         # asserts
+        self.toddler.pagure_io.get_project_contributors.assert_called_with(
+            scm_request_processor.PROJECT_NAMESPACE.split("/")[0],
+            scm_request_processor.PROJECT_NAMESPACE.split("/")[1]
+        )
         self.toddler.dist_git.get_project.assert_called_with(namespace, repo)
         self.toddler.dist_git.new_project.assert_called_with(
             namespace, repo, "", "", branch, initial_commit=True, alias=True
@@ -981,8 +1221,12 @@ class TestCreateNewRepo:
             reason="Processed",
         )
 
+    @patch("toddlers.plugins.scm_request_processor.bugzilla_system")
     @patch("toddlers.plugins.scm_request_processor.pdc")
-    def test_create_new_repo_non_default_branch(self, mock_pdc):
+    @patch(
+        "toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug"
+    )
+    def test_create_new_repo_non_default_branch(self, mock_validate_review_bug, mock_pdc, mock_bz):
         """
         Assert that ticket will be processed when everything is in order and requested
         branch is not default.
@@ -992,11 +1236,11 @@ class TestCreateNewRepo:
         repo = "repo"
         branch = "f35"
         namespace = "rpms"
-        bug_id = ""
+        bug_id = "123"
         action = "new_repo"
         sls = {branch: "2050-06-01"}
         monitor = "monitor"
-        exception = True
+        exception = False
         json = {
             "repo": repo,
             "branch": branch,
@@ -1017,6 +1261,9 @@ class TestCreateNewRepo:
         self.toddler.create_new_repo(issue, json)
 
         # asserts
+        mock_validate_review_bug.assert_called_with(
+            bug_id, repo, branch, namespace=namespace, pagure_user="zlopez")
+
         self.toddler.dist_git.get_project.assert_called_with(namespace, repo)
         self.toddler.dist_git.new_project.assert_called_with(
             namespace, repo, "", "", "rawhide", initial_commit=True, alias=True
@@ -1062,12 +1309,16 @@ class TestCreateNewRepo:
             message=message,
             reason="Processed",
         )
+        mock_bz.comment_on_bug.assert_called_with(bug_id, message)
 
     @patch("toddlers.plugins.scm_request_processor.bugzilla_system")
     @patch("toddlers.plugins.scm_request_processor.pdc")
-    def test_create_new_repo_update_bug(self, mock_pdc, mock_bz):
+    @patch(
+        "toddlers.plugins.scm_request_processor.SCMRequestProcessor.validate_review_bug"
+    )
+    def test_create_new_repo_default_brach(self, mock_validate_review_bug, mock_pdc, mock_bz):
         """
-        Assert that bug will be updated if the bug_id is provided.
+        Assert that repo will be created with default branch when everything is in order.
         """
         issue = {"id": 100, "user": {"name": "zlopez"}}
 
@@ -1078,7 +1329,7 @@ class TestCreateNewRepo:
         action = "new_repo"
         sls = {branch: "2050-06-01"}
         monitor = "monitor"
-        exception = True
+        exception = False
         json = {
             "repo": repo,
             "branch": branch,
@@ -1098,6 +1349,9 @@ class TestCreateNewRepo:
         self.toddler.create_new_repo(issue, json)
 
         # asserts
+        mock_validate_review_bug.assert_called_with(
+            bug_id, repo, branch, namespace=namespace, pagure_user="zlopez")
+
         self.toddler.dist_git.get_project.assert_called_with(namespace, repo)
         self.toddler.dist_git.new_project.assert_called_with(
             namespace, repo, "", "", branch, initial_commit=True, alias=True

toddlers.toml.example

@@ -230,6 +230,11 @@ pagure_url = "https:/pagure.io"
 pagure_api_key = "API token for pagure"
 # Monitoring choices for release-monitoring.org
 monitor_choices = ['no-monitoring', 'monitoring', 'monitoring-with-scratch']
+# What we should look for in validation comment
+validation_comment = "valid"
+# Text for the ping if the ticket needs to be manually verified
+ping_comment = This request wants to skip bugzilla validation! {maintainers} could you check if this is correct? If yes, please respond to this ticket with 'valid' comment
+
 
 # Pagure mapping to bugzilla
 [consumer_config.scm_request_processor.pagure_namespace_to_component]

toddlers/plugins/scm_request_processor.py

@@ -82,6 +82,15 @@ class SCMRequestProcessor(ToddlerBase):
     # Requests session
     requests_session: requests.requests.Session
 
+    # Validation comment
+    # This is the comment we will be looking for in the tickets
+    # That need to be manually verified
+    validation_comment: str = ""
+
+    # Ping comment
+    # Comment to sent when the ticket needs to be manually verified
+    ping_comment: str = ""
+
     def accepts_topic(self, topic: str) -> bool:
         """Returns a boolean whether this toddler is interested in messages
         from this specific topic.
@@ -136,6 +145,8 @@ class SCMRequestProcessor(ToddlerBase):
         self.pagure_namespace_to_product = config.get("pagure_namespace_to_product", {})
         self.temp_dir = config.get("temp_dir", "")
         self.requests_session = requests.make_session()
+        self.validation_comment = config.get("validation_comment", "")
+        self.ping_comment = config.get("ping_comment", "")
 
         _log.info("Setting up PDC client")
         pdc.set_pdc(config)
@@ -378,10 +389,39 @@ class SCMRequestProcessor(ToddlerBase):
             "tests",
             "container",
         ):
-            skip_msg = "- Skipping verification of RHBZ"
+            _log.info("- This ticket needs to be validated manually")
-            if bug_id:
+            contributors = self.pagure_io.get_project_contributors(
-                skip_msg = "{0} #{1}".format(skip_msg, bug_id)
+                PROJECT_NAMESPACE.split("/")[0], PROJECT_NAMESPACE.split("/")[1]
-            _log.info(skip_msg)
+            )
+            # Get the list of maintainers of the scm_requests repository
+            maintainers = (
+                set(contributors["users"]["admin"])
+                | set(contributors["users"]["commit"])
+                | set(contributors["users"]["ticket"])
+            )
+            # There is already a comment on this ticket, we should check if this is
+            # the validation ticket
+            if len(issue["comments"]) > 0:
+                valid = False
+                _log.info("- Check for validation comment")
+                for comment in issue["comments"]:
+                    if comment["comment"].strip() == self.validation_comment:
+                        if comment["user"]["name"] in maintainers:
+                            _log.info("- Ticket is validated by {}, continue processing".format(comment["user"]["name"]))
+                            valid = True
+                            break
+                if not valid:
+                    _log.info("- Ticket is not yet validated. Skipping...")
+                    return
+            # No comment on the ticket yet, the maintainers should be pinged
+            else:
+                _log.info("- Notify the responsible users")
+                self.pagure_io.add_comment_to_issue(
+                    issue["id"],
+                    namespace=PROJECT_NAMESPACE,
+                    comment=self.ping_comment.format(maintainers=" ".join(["@" + user for user in maintainers]))
+                )
+                return
         else:
             if not bug_id:
                 self.pagure_io.close_issue(