feat: add function to get specific app's details

Adds `get_specific_github_app_details` function to bots service. This allows us to get `GithubInstallationInfo` for the app we want. It fails with a new exception if the app can't be found. This is because this feature will be used to get the app that made a comment or status check. If we don't use the same to edit said comment it won't work. On top of that we need to start saving the ID of the apps we selected for Torngit adapter instances
codecov · May 28, 2024 · 56c2677 · 56c2677
1 parent 1b7be5f
commit 56c2677
Show file tree

Hide file tree

Showing 10 changed files with 156 additions and 59 deletions.
diff --git a/database/models/core.py b/database/models/core.py
@@ -504,6 +504,11 @@ class CommitNotification(CodecovBaseModel):
 
     id_ = Column("id", types.BigInteger, primary_key=True)
     commit_id = Column(types.BigInteger, ForeignKey("commits.id"))
+    gh_app_id = Column(
+        types.BigInteger,
+        ForeignKey("codecov_auth_githubappinstallation.id"),
+        nullable=True,
+    )
     notification_type = Column(
         postgresql.ENUM(Notification, values_callable=lambda x: [e.value for e in x]),
         nullable=False,

diff --git a/helpers/exceptions.py b/helpers/exceptions.py
@@ -10,6 +10,10 @@ class RepositoryWithoutValidBotError(Exception):
     pass
 
 
+class RequestedGithubAppNotFound(Exception):
+    pass
+
+
 class OwnerWithoutValidBotError(Exception):
     pass
 

diff --git a/requirements.in b/requirements.in
@@ -1,4 +1,4 @@
-https://github.com/codecov/shared/archive/4b51e8af9a42660c7c2f84831534349016ef5113.tar.gz#egg=shared
+https://github.com/codecov/shared/archive/7a7847acc7ddb97c3c172e3a9231d74ae18241fc.tar.gz#egg=shared
 https://github.com/codecov/opentelem-python/archive/refs/tags/v0.0.4a1.tar.gz#egg=codecovopentelem
 https://github.com/codecov/test-results-parser/archive/5515e960d5d38881036e9127f86320efca649f13.tar.gz#egg=test-results-parser
 boto3>=1.34

diff --git a/requirements.txt b/requirements.txt
@@ -2,7 +2,7 @@
 # This file is autogenerated by pip-compile with Python 3.12
 # by the following command:
 #
-#    pip-compile requirements.in
+#    pip-compile
 #
 amqp==5.2.0
     # via kombu
@@ -368,7 +368,7 @@ sentry-sdk==1.40.0
     # via
     #   -r requirements.in
     #   shared
-shared @ https://github.com/codecov/shared/archive/4b51e8af9a42660c7c2f84831534349016ef5113.tar.gz
+shared @ https://github.com/codecov/shared/archive/7a7847acc7ddb97c3c172e3a9231d74ae18241fc.tar.gz
     # via -r requirements.in
 six==1.16.0
     # via

diff --git a/services/bots/github_apps.py b/services/bots/github_apps.py
@@ -15,6 +15,7 @@
 )
 from helpers.exceptions import (
     NoConfiguredAppsAvailable,
+    RequestedGithubAppNotFound,
 )
 from services.bots.types import TokenWithOwner
 from services.github import get_github_integration_token
@@ -131,6 +132,43 @@ def get_github_app_token(
     return installation_token, None
 
 
+def get_specific_github_app_details(
+    owner: Owner, github_app_id: int, commitid: str
+) -> GithubInstallationInfo:
+    """Gets the GithubInstallationInfo for GithubAppInstallation with id github_app_id.
+
+    Args:
+        owner (Owner): the owner of the app. We look only in the apps for this owner.
+        github_app_id (int): the ID of the GithubAppInstallation we're looking for
+        commitid (str): Commit.commitid, used for logging purposes
+
+    Raises:
+        RequestedGithubAppNotFound - if the app is not found in the given owner raise exception.
+            The assumption is that we need this specific app for a reason, and if we can't find the app
+            it's better to just fail
+    """
+    app: GithubAppInstallation = next(
+        (obj for obj in owner.github_app_installations if obj.id == github_app_id), None
+    )
+    if app is None:
+        log.exception(
+            "Can't find requested app",
+            extra=dict(ghapp_id=github_app_id, commitid=commitid),
+        )
+        raise RequestedGithubAppNotFound()
+    if not app.is_configured():
+        log.warning(
+            "Request for specific app that is not configured",
+            extra=dict(ghapp_id=id, commitid=commitid),
+        )
+    return GithubInstallationInfo(
+        id=app.id,
+        installation_id=app.installation_id,
+        app_id=app.app_id,
+        pem_path=app.pem_path,
+    )
+
+
 def get_github_app_info_for_owner(
     owner: Owner,
     *,
@@ -203,6 +241,7 @@ def get_github_app_info_for_owner(
         info_to_get_tokens = list(
             map(
                 lambda obj: GithubInstallationInfo(
+                    id=obj.id,
                     installation_id=obj.installation_id,
                     app_id=obj.app_id,
                     pem_path=obj.pem_path,

diff --git a/services/bots/tests/test_bots.py b/services/bots/tests/test_bots.py
@@ -114,7 +114,10 @@ def test_select_owner_single_installation(self, dbsession):
                 ),
                 token_owner=None,
                 selected_installation_info=GithubInstallationInfo(
-                    installation_id=1200, app_id=200, pem_path="pem_path"
+                    id=installations[0].id,
+                    installation_id=1200,
+                    app_id=200,
+                    pem_path="pem_path",
                 ),
                 fallback_installations=[],
                 token_type_mapping=None,
@@ -210,7 +213,10 @@ def test_select_owner_multiple_installations_default_name(self, dbsession):
                 ),
                 token_owner=None,
                 selected_installation_info=GithubInstallationInfo(
-                    installation_id=1200, app_id=200, pem_path="pem_path"
+                    id=installations[0].id,
+                    installation_id=1200,
+                    app_id=200,
+                    pem_path="pem_path",
                 ),
                 fallback_installations=[],
                 token_type_mapping=None,
@@ -250,11 +256,17 @@ def test_select_owner_multiple_installations_custom_name(self, dbsession):
                 ),
                 token_owner=None,
                 selected_installation_info=GithubInstallationInfo(
-                    installation_id=1300, app_id=300, pem_path="pem_path"
+                    id=installations[1].id,
+                    installation_id=1300,
+                    app_id=300,
+                    pem_path="pem_path",
                 ),
                 fallback_installations=[
                     GithubInstallationInfo(
-                        installation_id=1200, app_id=200, pem_path="pem_path"
+                        id=installations[0].id,
+                        installation_id=1200,
+                        app_id=200,
+                        pem_path="pem_path",
                     )
                 ],
                 token_type_mapping=None,
@@ -437,7 +449,10 @@ def test_select_repo_single_installation(self, dbsession):
                 ),
                 token_owner=None,
                 selected_installation_info=GithubInstallationInfo(
-                    installation_id=1200, app_id=200, pem_path="pem_path"
+                    id=installations[0].id,
+                    installation_id=1200,
+                    app_id=200,
+                    pem_path="pem_path",
                 ),
                 fallback_installations=[],
                 token_type_mapping=None,
@@ -504,7 +519,10 @@ def test_select_repo_multiple_installations_default_name(self, dbsession):
                 ),
                 token_owner=None,
                 selected_installation_info=GithubInstallationInfo(
-                    installation_id=1200, app_id=200, pem_path="pem_path"
+                    id=installations[0].id,
+                    installation_id=1200,
+                    app_id=200,
+                    pem_path="pem_path",
                 ),
                 fallback_installations=[],
                 token_type_mapping=None,
@@ -547,11 +565,17 @@ def test_select_repo_multiple_installations_custom_name(self, dbsession):
                 ),
                 token_owner=None,
                 selected_installation_info=GithubInstallationInfo(
-                    installation_id=1300, app_id=300, pem_path="pem_path"
+                    id=installations[1].id,
+                    installation_id=1300,
+                    app_id=300,
+                    pem_path="pem_path",
                 ),
                 fallback_installations=[
                     GithubInstallationInfo(
-                        installation_id=1200, app_id=200, pem_path="pem_path"
+                        id=installations[0].id,
+                        installation_id=1200,
+                        app_id=200,
+                        pem_path="pem_path",
                     )
                 ],
                 token_type_mapping=None,

diff --git a/services/bots/tests/test_github_apps.py b/services/bots/tests/test_github_apps.py
@@ -0,0 +1,51 @@
+import pytest
+from shared.typings.torngit import GithubInstallationInfo
+
+from database.models.core import GithubAppInstallation
+from database.tests.factories.core import OwnerFactory
+from helpers.exceptions import RequestedGithubAppNotFound
+from services.bots.github_apps import get_specific_github_app_details
+
+
+class TestGetSpecificGithubAppDetails(object):
+    def _get_owner_with_apps(self, dbsession):
+        owner = OwnerFactory(service="github")
+        app_1 = GithubAppInstallation(
+            owner=owner,
+            installation_id=1200,
+            app_id=12,
+        )
+        app_2 = GithubAppInstallation(
+            owner=owner,
+            installation_id=1500,
+            app_id=15,
+            pem_path="some_path",
+        )
+        dbsession.add_all([owner, app_1, app_2])
+        dbsession.flush()
+        assert owner.github_app_installations == [app_1, app_2]
+        return owner
+
+    def test_get_specific_github_app_details(self, dbsession):
+        owner = self._get_owner_with_apps(dbsession)
+        assert get_specific_github_app_details(
+            owner, owner.github_app_installations[0].id, "commit_id_for_logs"
+        ) == GithubInstallationInfo(
+            id=owner.github_app_installations[0].id,
+            installation_id=1200,
+            app_id=12,
+            pem_path=None,
+        )
+        assert get_specific_github_app_details(
+            owner, owner.github_app_installations[1].id, "commit_id_for_logs"
+        ) == GithubInstallationInfo(
+            id=owner.github_app_installations[1].id,
+            installation_id=1500,
+            app_id=15,
+            pem_path="some_path",
+        )
+
+    def test_get_specific_github_app_not_found(self, dbsession):
+        owner = self._get_owner_with_apps(dbsession)
+        with pytest.raises(RequestedGithubAppNotFound):
+            get_specific_github_app_details(owner, 123456, "commit_id_for_logs")
diff --git a/services/tests/test_bots.py b/services/tests/test_bots.py
@@ -289,6 +289,7 @@ def test_get_repo_appropriate_bot_token_via_installation_covered_repo(
         installations = get_github_app_info_for_owner(repo.owner)
         assert installations == [
             {
+                "id": installation.id,
                 "installation_id": 12341234,
                 "app_id": None,
                 "pem_path": None,
@@ -360,6 +361,7 @@ def test_get_owner_installation_id_yes_installation_yes_legacy_integration(
         assert owner.github_app_installations == [installation]
         assert get_github_app_info_for_owner(owner) == [
             {
+                "id": installation.id,
                 "installation_id": 123456,
                 "app_id": None,
                 "pem_path": None,
@@ -389,11 +391,17 @@ def test_get_owner_installation_id_yes_installation_yes_legacy_integration_yes_f
         assert owner.github_app_installations == [installation_0, installation_1]
         assert get_github_app_info_for_owner(owner, installation_name="my_app") == [
             {
+                "id": installation_1.id,
                 "installation_id": 12000,
                 "app_id": 1212,
                 "pem_path": "path",
             },
-            {"installation_id": 123456, "app_id": None, "pem_path": None},
+            {
+                "id": installation_0.id,
+                "installation_id": 123456,
+                "app_id": None,
+                "pem_path": None,
+            },
         ]
 
     def test_get_owner_installation_id_yes_installation_all_rate_limited(

diff --git a/services/tests/test_owner_service.py b/services/tests/test_owner_service.py
@@ -57,7 +57,12 @@ def test_get_owner_provider_service_with_installation(self, dbsession, mocker):
                 "username": owner.username,
             },
             "repo": {},
-            "installation": {"installation_id": 1500, "pem_path": None, "app_id": None},
+            "installation": {
+                "id": installation.id,
+                "installation_id": 1500,
+                "pem_path": None,
+                "app_id": None,
+            },
             "fallback_installations": [],
         }
         assert res.service == "github"

diff --git a/services/tests/test_repository_service.py b/services/tests/test_repository_service.py
@@ -24,7 +24,6 @@
     RepositoryFactory,
 )
 from services.repository import (
-    _is_repo_using_integration,
     _pick_best_base_comparedto_pair,
     fetch_and_update_pull_request_information,
     fetch_and_update_pull_request_information_from_commit,
@@ -36,50 +35,6 @@
 )
 
 
-@pytest.mark.parametrize("using_integration", [True, False])
-def test__is_repo_using_integration_deprecated_flow(using_integration, dbsession):
-    repo = RepositoryFactory.create(using_integration=using_integration)
-    assert _is_repo_using_integration(repo) == using_integration
-
-
-def test__is_repo_using_integration_ghapp_covers_all_repos(dbsession):
-    owner = OwnerFactory.create(service="github")
-    repo = RepositoryFactory.create(owner=owner)
-    other_repo_same_owner = RepositoryFactory.create(owner=owner)
-    repo_different_owner = RepositoryFactory.create()
-    assert repo.owner != repo_different_owner.owner
-    ghapp_installation = GithubAppInstallation(
-        name=GITHUB_APP_INSTALLATION_DEFAULT_NAME,
-        owner=owner,
-        repository_service_ids=None,
-        installation_id=12345,
-    )
-    dbsession.add(ghapp_installation)
-    dbsession.flush()
-    assert _is_repo_using_integration(repo) == True
-    assert _is_repo_using_integration(other_repo_same_owner) == True
-    assert _is_repo_using_integration(repo_different_owner) == False
-
-
-def test__is_repo_using_integration_ghapp_covers_some_repos(dbsession):
-    owner = OwnerFactory.create(service="github")
-    repo = RepositoryFactory.create(owner=owner)
-    other_repo_same_owner = RepositoryFactory.create(owner=owner)
-    repo_different_owner = RepositoryFactory.create()
-    assert repo.owner != repo_different_owner.owner
-    ghapp_installation = GithubAppInstallation(
-        name=GITHUB_APP_INSTALLATION_DEFAULT_NAME,
-        owner=owner,
-        repository_service_ids=[repo.service_id],
-        installation_id=12345,
-    )
-    dbsession.add(ghapp_installation)
-    dbsession.flush()
-    assert _is_repo_using_integration(repo) == True
-    assert _is_repo_using_integration(other_repo_same_owner) == False
-    assert _is_repo_using_integration(repo_different_owner) == False
-
-
 class TestRepositoryServiceTestCase(object):
     def test_get_repo_provider_service_github(self, dbsession):
         repo = RepositoryFactory.create(
@@ -159,12 +114,18 @@ def test_get_repo_provider_service_github_with_installations(
                 "repoid": repo.repoid,
             },
             "installation": {
+                "id": installation_1.id,
                 "installation_id": 1300,
                 "app_id": 300,
                 "pem_path": "path",
             },
             "fallback_installations": [
-                {"app_id": 200, "installation_id": 1200, "pem_path": None}
+                {
+                    "id": installation_0.id,
+                    "app_id": 200,
+                    "installation_id": 1200,
+                    "pem_path": None,
+                }
             ],
         }
         assert res.data == expected_data