[ISV-4528] Automatically merge approved PRs (#746)

* [ISV-4528] Automatically merge approved PRs

---------

Signed-off-by: Maurizio Porrato <[email protected]>

Author: mporrato

operator-pipeline-images/operatorcert/entrypoints/check_permissions.py

@@ -167,6 +167,34 @@ def github_repo_org(self) -> str:
         """
         return urllib.parse.urlparse(self.pull_request_url).path.split("/")[1]
 
+    @property
+    def github_repo_name(self) -> str:
+        """
+        The name of the github repo the pull request belongs to
+
+        Returns:
+            str: the github repo name in the form "organization_name/repo_name"
+        """
+        return "/".join(
+            urllib.parse.urlparse(self.pull_request_url).path.split("/")[1:3]
+        )
+
+    @property
+    def pr_labels(self) -> set[str]:
+        """
+        The set of labels applied to the pull request
+
+        Returns:
+            set[str]: the labels applied to the pull request
+        """
+        github_auth = Auth.Token(os.environ.get("GITHUB_TOKEN") or "")
+        github = Github(auth=github_auth)
+        pr_no = int(urllib.parse.urlparse(self.pull_request_url).path.split("/")[-1])
+        return {
+            x.name
+            for x in github.get_repo(self.github_repo_name).get_pull(pr_no).get_labels()
+        }
+
     def check_permissions(self) -> bool:
         """
         Check if the pull request owner has permissions to submit a PR for the operator
@@ -271,20 +299,27 @@ def check_permission_for_community(self) -> bool:
                 f"{self.operator} does not have any reviewers in the base repository "
                 "or is brand new."
             )
-        if self.pr_owner not in self.reviewers:
+
+        if self.pr_owner in self.reviewers:
             LOGGER.info(
-                "Pull request owner %s is not in the list of reviewers %s",
+                "Pull request owner %s can submit PR for operator %s",
                 self.pr_owner,
-                self.reviewers,
+                self.operator.operator_name,
             )
-            self.request_review_from_owners()
-            return False
+            return True
+
+        LOGGER.info("Checking if the pull request is approved by a reviewer")
+        if "approved" in self.pr_labels:
+            LOGGER.info("Pull request is approved by a reviewer")
+            return True
+
         LOGGER.info(
-            "Pull request owner %s can submit PR for operator %s",
+            "Pull request owner %s is not in the list of reviewers %s",
             self.pr_owner,
-            self.operator.operator_name,
+            self.reviewers,
         )
-        return True
+        self.request_review_from_owners()
+        return False
 
     def request_review_from_maintainers(self) -> None:
         """
@@ -312,11 +347,12 @@ def request_review_from_owners(self) -> None:
         """
         reviewers_with_at = ", ".join(map(lambda x: f"@{x}", self.reviewers))
         comment_text = (
-            "Author of the PR is not listed as one of the reviewers in ci.yaml.\n"
-            "Please review the PR and approve it with \`/lgtm\` comment.\n"  # pylint: disable=anomalous-backslash-in-string
-            f"{reviewers_with_at} \n\n"
-            "Consider adding author of the PR to the ci.yaml file if you want automated "
-            "approval for a followup submissions."
+            "The author of the PR is not listed as one of the reviewers in ci.yaml.\n"
+            f"{reviewers_with_at}: please review the PR and approve it with an "
+            "`/approve` comment.\n\n"
+            "Consider adding the author of the PR to the list of reviewers in "
+            "the ci.yaml file if you want automated merge without explicit "
+            "approval."
         )
 
         run_command(

operator-pipeline-images/tests/entrypoints/test_check_permissions.py

@@ -1,5 +1,5 @@
 from pathlib import Path
-from typing import Any
+from typing import Any, Optional
 from unittest import mock
 from unittest.mock import MagicMock, call, patch
 
@@ -105,6 +105,33 @@ def test_OperatorReview_github_repo_org(
     assert review_community.github_repo_org == "my-org"
 
 
+def test_OperatorReview_github_repo_name(
+    review_community: check_permissions.OperatorReview,
+) -> None:
+    assert review_community.github_repo_name == "my-org/repo-123"
+
+
+@patch("operatorcert.entrypoints.check_permissions.Github")
+@patch("operatorcert.entrypoints.check_permissions.Auth.Token")
+def test_OperatorReview_pr_labels(
+    mock_token: MagicMock,
+    mock_github: MagicMock,
+    review_community: check_permissions.OperatorReview,
+) -> None:
+    repo = MagicMock()
+    pull = MagicMock()
+
+    def _mock_label(name: str) -> MagicMock:
+        m = MagicMock()
+        m.name = name
+        return m
+
+    pull.get_labels.return_value = [_mock_label("foo"), _mock_label("bar")]
+    repo.get_pull.return_value = pull
+    mock_github.return_value.get_repo.return_value = repo
+    assert review_community.pr_labels == {"foo", "bar"}
+
+
 @pytest.mark.parametrize(
     "is_org_member, is_partner, permission_partner, permission_community, permission_partner_called, permission_community_called, expected_result",
     [
@@ -225,31 +252,73 @@ def test_OperatorReview_check_permission_for_partner(
             review_partner.check_permission_for_partner()
 
 
+@pytest.mark.parametrize(
+    ["reviewers", "labels", "owners_review", "result"],
+    [
+        pytest.param(
+            [],
+            set(),
+            False,
+            check_permissions.MaintainersReviewNeeded,
+            id="No reviewers",
+        ),
+        pytest.param(
+            ["owner", "foo"],
+            set(),
+            False,
+            True,
+            id="author is reviewer",
+        ),
+        pytest.param(
+            ["foo", "bar"],
+            set(),
+            True,
+            False,
+            id="author is not reviewer",
+        ),
+        pytest.param(
+            ["bar", "baz"],
+            {"approved"},
+            False,
+            True,
+            id="author is not reviewer, PR approved",
+        ),
+    ],
+)
 @patch(
     "operatorcert.entrypoints.check_permissions.OperatorReview.request_review_from_owners"
 )
 @patch(
     "operatorcert.entrypoints.check_permissions.OperatorReview.reviewers",
     new_callable=mock.PropertyMock,
 )
+@patch(
+    "operatorcert.entrypoints.check_permissions.OperatorReview.pr_labels",
+    new_callable=mock.PropertyMock,
+)
 def test_OperatorReview_check_permission_for_community(
+    mock_labels: MagicMock,
     mock_reviewers: MagicMock,
     mock_review_from_owners: MagicMock,
     review_community: check_permissions.OperatorReview,
+    reviewers: list[str],
+    labels: set[str],
+    owners_review: bool,
+    result: bool | type,
 ) -> None:
-    mock_reviewers.return_value = []
-    with pytest.raises(check_permissions.MaintainersReviewNeeded):
-        review_community.check_permission_for_community()
-
-    mock_reviewers.return_value = ["user1", "user2"]
+    mock_reviewers.return_value = reviewers
+    mock_labels.return_value = labels
 
-    assert review_community.check_permission_for_community() == False
-    mock_review_from_owners.assert_called_once()
+    if isinstance(result, bool):
+        assert review_community.check_permission_for_community() == result
+    else:
+        with pytest.raises(result):
+            review_community.check_permission_for_community()
 
-    mock_review_from_owners.reset_mock()
-    mock_reviewers.return_value = ["owner"]
-    assert review_community.check_permission_for_community() == True
-    mock_review_from_owners.assert_not_called()
+    if owners_review:
+        mock_review_from_owners.assert_called_once()
+    else:
+        mock_review_from_owners.assert_not_called()
 
 
 @patch("operatorcert.entrypoints.check_permissions.run_command")
@@ -283,10 +352,11 @@ def test_OperatorReview_request_review_from_owners(
             "comment",
             review_community.pull_request_url,
             "--body",
-            "Author of the PR is not listed as one of the reviewers in ci.yaml.\n"
-            "Please review the PR and approve it with \\`/lgtm\\` comment.\n"
-            "@user1, @user2 \n\nConsider adding author of the PR to the ci.yaml "
-            "file if you want automated approval for a followup submissions.",
+            "The author of the PR is not listed as one of the reviewers in ci.yaml.\n"
+            "@user1, @user2: please review the PR and approve it with an `/approve` comment.\n\n"
+            "Consider adding the author of the PR to the list of reviewers in "
+            "the ci.yaml file if you want automated merge without explicit "
+            "approval.",
         ]
     )