Skip to content

Commit

Permalink
feat: use GithubAppInstallation in api (#347)
Browse files Browse the repository at this point in the history
* feat: use GithubAppInstallation in api

Uses the gh app installation WITHOUT compromising
the old usage (of `owner.integration_id`).
Deprecated paths are marked so.

These changes don't backfill old data, but start using
the new model with webhooks.

the sample data from tests (the keys) are taken from
[github docs](https://docs.github.com/en/webhooks/webhook-events-and-payloads#installation)
and some amount of empirical data.

depends on: #346

* refactor webhook handling related to gh app installation

* use ghapp when getting torngit adapter

Considers ghapp installations when deciding if a torngit adapter
is for a repo with integration or not.

If a ghapp installation exist then repo is considered to be using
integration if it's covered by the installation.
Otherwise previous behavior is used.

The test `test_get_adapter_sets_token_to_bot_when_user_not_authenticated`
includes `owner` as `None` when calling `RepoProviderService.get_adapter` so I'm assuming
that can happen, and changing the typehint in the function.

Other functions that were typehinted with `Owner` instead of `Optional[Owner]` are also
changed.
  • Loading branch information
giovanni-guidini committed Jan 24, 2024
1 parent fd02145 commit 86178ba
Show file tree
Hide file tree
Showing 8 changed files with 922 additions and 202 deletions.
36 changes: 30 additions & 6 deletions services/repo_providers.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
import logging
from os import getenv
from typing import Callable, Dict
from typing import Callable, Dict, Optional

from django.conf import settings
from shared.encryption.token import encode_token
from shared.torngit import get

from codecov.db import sync_to_async
from codecov_auth.models import Owner, Service
from codecov_auth.models import (
GITHUB_APP_INSTALLATION_DEFAULT_NAME,
GithubAppInstallation,
Owner,
Service,
)
from core.models import Repository
from utils.config import get_config
from utils.encryption import encryptor
Expand All @@ -24,12 +29,14 @@ class TorngitInitializationFailed(Exception):


def get_token_refresh_callback(
owner: Owner, service: Service
owner: Optional[Owner], service: Service
) -> Callable[[Dict], None]:
"""
Produces a callback function that will encode and update the oauth token of an owner.
This callback is passed to the TorngitAdapter for the service.
"""
if owner is None:
return None
if service == Service.BITBUCKET or service == Service.BITBUCKET_SERVER:
return None

Expand All @@ -46,7 +53,9 @@ def callback(new_token: Dict) -> None:
return callback


def get_generic_adapter_params(owner: Owner, service, use_ssl=False, token=None):
def get_generic_adapter_params(
owner: Optional[Owner], service, use_ssl=False, token=None
):
if use_ssl:
verify_ssl = (
get_config(service, "ssl_pem")
Expand Down Expand Up @@ -87,7 +96,21 @@ def get_provider(service, adapter_params):


class RepoProviderService(object):
def get_adapter(self, owner: Owner, repo: Repository, use_ssl=False, token=None):
def _is_using_integration(self, owner: Optional[Owner], repo: Repository) -> bool:
if owner is None:
return False
ghapp_installation: Optional[
GithubAppInstallation
] = owner.github_app_installations.filter(
name=GITHUB_APP_INSTALLATION_DEFAULT_NAME
).first()
if ghapp_installation:
return ghapp_installation.is_repo_covered_by_integration(repo)
return repo.using_integration

def get_adapter(
self, owner: Optional[Owner], repo: Repository, use_ssl=False, token=None
):
"""
Return the corresponding implementation for calling the repository provider
Expand All @@ -99,10 +122,11 @@ def get_adapter(self, owner: Owner, repo: Repository, use_ssl=False, token=None)
generic_adapter_params = get_generic_adapter_params(
owner, repo.author.service, use_ssl, token
)

owner_and_repo_params = {
"repo": {
"name": repo.name,
"using_integration": repo.using_integration or False,
"using_integration": self._is_using_integration(owner, repo),
"service_id": repo.service_id,
"private": repo.private,
"repoid": repo.repoid,
Expand Down
67 changes: 66 additions & 1 deletion services/tests/test_repo_providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@

from codecov.db import sync_to_async
from codecov.tests.base_test import InternalAPITest
from codecov_auth.models import Owner
from codecov_auth.models import (
GITHUB_APP_INSTALLATION_DEFAULT_NAME,
GithubAppInstallation,
Owner,
)
from codecov_auth.tests.factories import OwnerFactory
from core.tests.factories import RepositoryFactory
from services.repo_providers import RepoProviderService
Expand All @@ -33,6 +37,67 @@ def mock_get_env_ca_bundle(*args):
return "REQUESTS_CA_BUNDLE"


@pytest.mark.parametrize("using_integration", [True, False])
def test__is_using_integration_deprecated_flow(using_integration, db):
repo = RepositoryFactory.create(using_integration=using_integration)
assert (
RepoProviderService()._is_using_integration(repo.author, repo)
== using_integration
)


def test__is_using_integration_ghapp_covers_all_repos(db):
owner = OwnerFactory.create(service="github")
repo = RepositoryFactory.create(author=owner)
other_repo_same_owner = RepositoryFactory.create(author=owner)
repo_different_owner = RepositoryFactory.create()
assert repo.author != repo_different_owner.author
ghapp_installation = GithubAppInstallation(
name=GITHUB_APP_INSTALLATION_DEFAULT_NAME,
owner=owner,
repository_service_ids=None,
installation_id=12345,
)
ghapp_installation.save()
assert RepoProviderService()._is_using_integration(repo.author, repo) == True
assert (
RepoProviderService()._is_using_integration(repo.author, other_repo_same_owner)
== True
)
assert (
RepoProviderService()._is_using_integration(
repo_different_owner.author, repo_different_owner
)
== False
)


def test__is_using_integration_ghapp_covers_some_repos(db):
owner = OwnerFactory.create(service="github")
repo = RepositoryFactory.create(author=owner)
other_repo_same_owner = RepositoryFactory.create(author=owner)
repo_different_owner = RepositoryFactory.create()
assert repo.author != repo_different_owner.author
ghapp_installation = GithubAppInstallation(
name=GITHUB_APP_INSTALLATION_DEFAULT_NAME,
owner=owner,
repository_service_ids=[repo.service_id],
installation_id=12345,
)
ghapp_installation.save()
assert RepoProviderService()._is_using_integration(repo.author, repo) == True
assert (
RepoProviderService()._is_using_integration(repo.author, other_repo_same_owner)
== False
)
assert (
RepoProviderService()._is_using_integration(
repo_different_owner.author, repo_different_owner
)
== False
)


class TestRepoProviderService(InternalAPITest):
def setUp(self):
self.repo_gh = RepositoryFactory.create(
Expand Down
40 changes: 36 additions & 4 deletions upload/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import re
from datetime import timedelta
from json import dumps
from typing import Optional

import jwt
from asgiref.sync import async_to_sync
Expand All @@ -17,7 +18,13 @@
from shared.reports.enums import UploadType
from shared.torngit.exceptions import TorngitClientError, TorngitObjectNotFoundError

from codecov_auth.models import Owner
from codecov_auth.models import (
GITHUB_APP_INSTALLATION_DEFAULT_NAME,
SERVICE_GITHUB,
SERVICE_GITHUB_ENTERPRISE,
GithubAppInstallation,
Owner,
)
from core.models import Commit, CommitNotification, Pull, Repository
from plan.constants import USER_PLAN_REPRESENTATIONS
from reports.models import CommitReport, ReportSession
Expand Down Expand Up @@ -315,20 +322,45 @@ def determine_upload_pr_to_use(upload_params):
return upload_params.get("pr")


def ghapp_installation_id_to_use(repository: Repository) -> Optional[str]:
if (
repository.service != SERVICE_GITHUB
and repository.service != SERVICE_GITHUB_ENTERPRISE
):
return None

gh_app_default_installation: GithubAppInstallation = (
repository.author.github_app_installations.filter(
name=GITHUB_APP_INSTALLATION_DEFAULT_NAME
).first()
)
if (
gh_app_default_installation
and gh_app_default_installation.is_repo_covered_by_integration(repository)
):
return gh_app_default_installation.installation_id
elif repository.using_integration and repository.author.integration_id:
# THIS FLOW IS DEPRECATED
# it will (hopefully) be removed after the ghapp installation work is complete
# and the data is backfilles appropriately
return repository.author.integration_id


def try_to_get_best_possible_bot_token(repository):
if repository.using_integration and repository.author.integration_id:
ghapp_installation_id = ghapp_installation_id_to_use(repository)
if ghapp_installation_id is not None:
try:
github_token = get_github_integration_token(
repository.author.service,
integration_id=repository.author.integration_id,
installation_id=ghapp_installation_id,
)
return dict(key=github_token)
except InvalidInstallationError:
log.warning(
"Invalid installation error",
extra=dict(
service=repository.author.service,
integration_id=repository.author.integration_id,
integration_id=ghapp_installation_id,
),
)
# now we'll fallback to trying an OAuth token
Expand Down
30 changes: 28 additions & 2 deletions upload/tests/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,44 @@
import jwt
import pytest
from django.conf import settings
from django.test import TransactionTestCase
from rest_framework.exceptions import Throttled, ValidationError
from shared.config import get_config
from shared.github import InvalidInstallationError

from codecov_auth.models import GithubAppInstallation, Service
from core.tests.factories import CommitFactory, OwnerFactory, RepositoryFactory
from plan.constants import PlanName
from reports.tests.factories import CommitReportFactory, UploadFactory
from upload.helpers import (
check_commit_upload_constraints,
determine_repo_for_upload,
ghapp_installation_id_to_use,
try_to_get_best_possible_bot_token,
validate_activated_repo,
validate_upload,
)


class TestGithubAppInstallationUsage(TransactionTestCase):
def test_not_github_provider(self):
repo = RepositoryFactory(author__service=Service.GITLAB.value)
assert ghapp_installation_id_to_use(repo) is None

def test_github_app_installation_flow(self):
owner = OwnerFactory(service=Service.GITHUB.value, integration_id=None)
covered_repo = RepositoryFactory(author=owner)
not_covered_repo = RepositoryFactory(author=owner)
ghapp_installation = GithubAppInstallation(
owner=owner,
repository_service_ids=[covered_repo.service_id],
installation_id=200,
)
ghapp_installation.save()
assert ghapp_installation_id_to_use(covered_repo) == 200
assert ghapp_installation_id_to_use(not_covered_repo) is None


def test_try_to_get_best_possible_bot_token_no_repobot_no_ownerbot(db):
owner = OwnerFactory.create(unencrypted_oauth_token="super")
owner.save()
Expand Down Expand Up @@ -72,7 +94,9 @@ def test_try_to_get_best_possible_bot_token_using_integration(
assert try_to_get_best_possible_bot_token(repository) == {
"key": "test-token",
}
get_github_integration_token.assert_called_once_with("github", integration_id=12345)
get_github_integration_token.assert_called_once_with(
"github", installation_id=12345
)


@patch("upload.helpers.get_github_integration_token")
Expand All @@ -92,7 +116,9 @@ def test_try_to_get_best_possible_bot_token_using_invalid_integration(
"key": "bornana",
"secret": None,
}
get_github_integration_token.assert_called_once_with("github", integration_id=12345)
get_github_integration_token.assert_called_once_with(
"github", installation_id=12345
)


def test_try_to_get_best_possible_nothing_and_is_private(db):
Expand Down
4 changes: 2 additions & 2 deletions utils/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@


@cache.cache_function(ttl=480)
def get_github_integration_token(service, integration_id=None):
return _get_github_integration_token(service, integration_id=integration_id)
def get_github_integration_token(service, installation_id=None):
return _get_github_integration_token(service, integration_id=installation_id)

0 comments on commit 86178ba

Please sign in to comment.