Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add function to get specific app's details #470

Merged
merged 1 commit into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions database/models/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions helpers/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ class RepositoryWithoutValidBotError(Exception):
pass


class RequestedGithubAppNotFound(Exception):
pass


class OwnerWithoutValidBotError(Exception):
pass

Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 39 additions & 0 deletions services/bots/github_apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
)
from helpers.exceptions import (
NoConfiguredAppsAvailable,
RequestedGithubAppNotFound,
)
from services.bots.types import TokenWithOwner
from services.github import get_github_integration_token
Expand Down Expand Up @@ -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,
*,
Expand Down Expand Up @@ -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,
Expand Down
40 changes: 32 additions & 8 deletions services/bots/tests/test_bots.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
51 changes: 51 additions & 0 deletions services/bots/tests/test_github_apps.py
Original file line number Diff line number Diff line change
@@ -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")
10 changes: 9 additions & 1 deletion services/tests/test_bots.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down
7 changes: 6 additions & 1 deletion services/tests/test_owner_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
8 changes: 7 additions & 1 deletion services/tests/test_repository_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,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
Expand Down
Loading