diff --git a/services/repository.py b/services/repository.py index e7fe2c58a..0b461e0e7 100644 --- a/services/repository.py +++ b/services/repository.py @@ -27,6 +27,11 @@ from services.bots import ( get_adapter_auth_information, ) +from services.bots.github_apps import ( + get_github_app_token, + get_specific_github_app_details, +) +from services.github import get_github_app_for_commit from services.yaml import read_yaml_field log = logging.getLogger(__name__) @@ -34,29 +39,57 @@ merged_pull = re.compile(r".*Merged in [^\s]+ \(pull request \#(\d+)\).*").match -def _is_repo_using_integration(repo: Repository) -> bool: - owner = repo.owner - default_ghapp_installation = list( - filter( - lambda obj: obj.name == GITHUB_APP_INSTALLATION_DEFAULT_NAME, - owner.github_app_installations or [], - ) +def get_repo_provider_service_for_specific_commit( + commit: Commit, + fallback_installation_name: str = GITHUB_APP_INSTALLATION_DEFAULT_NAME, +) -> torngit.base.TorngitBaseAdapter: + """Gets a Torngit adapter using a specific github app as the authentication source. + + This is done specifically after emitting checks for a PR using GitHub apps, because only the app + that posted the check can edit it later on. This info is saved in Redis by the NotifyTask. + """ + repository = commit.repository + installation_for_commit = get_github_app_for_commit(commit) + if ( + repository.service not in ["github", "github_enterprise"] + or installation_for_commit is None + ): + return get_repo_provider_service(repository, fallback_installation_name) + + ghapp_details = get_specific_github_app_details( + repository.owner, int(installation_for_commit), commit.commitid + ) + token, _ = get_github_app_token(repository.service, ghapp_details) + + data = TorngitInstanceData( + repo=RepoInfo( + name=repository.name, + using_integration=True, + service_id=repository.service_id, + repoid=repository.repoid, + ), + owner=OwnerInfo( + service_id=repository.owner.service_id, + ownerid=repository.ownerid, + username=repository.owner.username, + ), + installation=ghapp_details, + fallback_installations=None, + ) + + adapter_params = dict( + token=token, + token_type_mapping=None, + on_token_refresh=None, + **data, ) - if default_ghapp_installation: - ghapp_installation = owner.github_app_installations[0] - return ghapp_installation.is_repo_covered_by_integration(repo) - return repo.using_integration + return _get_repo_provider_service_instance(repository.service, **adapter_params) def get_repo_provider_service( repository: Repository, installation_name_to_use: Optional[str] = GITHUB_APP_INSTALLATION_DEFAULT_NAME, ) -> torngit.base.TorngitBaseAdapter: - _timeouts = [ - get_config("setup", "http", "timeouts", "connect", default=30), - get_config("setup", "http", "timeouts", "receive", default=60), - ] - service = repository.owner.service adapter_auth_info = get_adapter_auth_information( repository.owner, repository=repository, @@ -65,7 +98,9 @@ def get_repo_provider_service( data = TorngitInstanceData( repo=RepoInfo( name=repository.name, - using_integration=_is_repo_using_integration(repository), + using_integration=( + adapter_auth_info["selected_installation_info"] is not None + ), service_id=repository.service_id, repoid=repository.repoid, ), @@ -81,20 +116,28 @@ def get_repo_provider_service( adapter_params = dict( token=adapter_auth_info["token"], token_type_mapping=adapter_auth_info["token_type_mapping"], - verify_ssl=get_verify_ssl(service), - timeouts=_timeouts, - oauth_consumer_token=dict( - key=get_config(service, "client_id"), - secret=get_config(service, "client_secret"), - ), on_token_refresh=get_token_refresh_callback(adapter_auth_info["token_owner"]), **data, ) return _get_repo_provider_service_instance(repository.service, **adapter_params) -def _get_repo_provider_service_instance(service_name, **adapter_params): - return torngit.get(service_name, **adapter_params) +def _get_repo_provider_service_instance(service: str, **adapter_params): + _timeouts = [ + get_config("setup", "http", "timeouts", "connect", default=30), + get_config("setup", "http", "timeouts", "receive", default=60), + ] + return torngit.get( + service, + # Args for the Torngit instance + timeouts=_timeouts, + verify_ssl=get_verify_ssl(service), + oauth_consumer_token=dict( + key=get_config(service, "client_id"), + secret=get_config(service, "client_secret"), + ), + **adapter_params, + ) async def fetch_appropriate_parent_for_commit( diff --git a/services/tests/test_repository_service.py b/services/tests/test_repository_service.py index 24fbb58e1..b23c93c8d 100644 --- a/services/tests/test_repository_service.py +++ b/services/tests/test_repository_service.py @@ -1,5 +1,6 @@ import inspect from datetime import datetime +from unittest.mock import MagicMock, patch import mock import pytest @@ -11,6 +12,12 @@ TorngitObjectNotFoundError, TorngitServerUnreachableError, ) +from shared.typings.torngit import ( + GithubInstallationInfo, + OwnerInfo, + RepoInfo, + TorngitInstanceData, +) from database.models import Owner from database.models.core import ( @@ -24,7 +31,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, @@ -32,54 +38,11 @@ get_or_create_author, get_repo_provider_service, get_repo_provider_service_by_id, + get_repo_provider_service_for_specific_commit, update_commit_from_provider_info, ) -@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( @@ -1109,6 +1072,106 @@ async def test_get_repo_gh_no_integration(self, dbsession, mocker): } +class TestGetRepoProviderServiceForSpecificCommit(object): + @pytest.fixture + def mock_get_repo_provider_service(self, mocker): + mock_get_repo_provider_service = mocker.patch( + "services.repository.get_repo_provider_service" + ) + return mock_get_repo_provider_service + + @pytest.fixture + def mock_redis(self, mocker): + fake_redis = MagicMock(name="fake_redis") + mock_conn = mocker.patch("services.github.get_redis_connection") + mock_conn.return_value = fake_redis + return fake_redis + + def test_get_repo_provider_service_for_specific_commit_not_gh( + self, dbsession, mock_get_repo_provider_service, mock_redis + ): + commit = CommitFactory(repository__owner__service="gitlab") + mock_get_repo_provider_service.return_value = "the TorngitAdapter" + response = get_repo_provider_service_for_specific_commit(commit, "some_name") + assert response == "the TorngitAdapter" + mock_get_repo_provider_service.assert_called_with( + commit.repository, "some_name" + ) + + def test_get_repo_provider_service_for_specific_commit_no_specific_app_for_commit( + self, dbsession, mock_get_repo_provider_service, mock_redis + ): + commit = CommitFactory(repository__owner__service="github") + assert commit.id not in [10000, 15000] + redis_keys = { + "app_to_use_for_commit_15000": "1200", + "app_to_use_for_commit_10000": "1000", + } + mock_redis.get.side_effect = lambda key: redis_keys.get(key) + + mock_get_repo_provider_service.return_value = "the TorngitAdapter" + response = get_repo_provider_service_for_specific_commit(commit, "some_name") + assert response == "the TorngitAdapter" + mock_get_repo_provider_service.assert_called_with( + commit.repository, "some_name" + ) + + @patch( + "services.repository.get_github_app_token", return_value=("the app token", None) + ) + @patch( + "services.repository._get_repo_provider_service_instance", + return_value="the TorngitAdapter", + ) + def test_get_repo_provider_service_for_specific_commit( + self, + mock_get_instance, + mock_get_app_token, + dbsession, + mock_get_repo_provider_service, + mock_redis, + ): + commit = CommitFactory(repository__owner__service="github") + app = GithubAppInstallation( + owner=commit.repository.owner, app_id=12, installation_id=1200 + ) + dbsession.add_all([commit, app]) + dbsession.flush() + assert commit.repository.owner.github_app_installations == [app] + redis_keys = { + f"app_to_use_for_commit_{commit.id}": str(app.id), + } + mock_redis.get.side_effect = lambda key: redis_keys.get(key) + response = get_repo_provider_service_for_specific_commit(commit, "some_name") + assert response == "the TorngitAdapter" + mock_get_instance.assert_called_once() + + data = TorngitInstanceData( + repo=RepoInfo( + name=commit.repository.name, + using_integration=True, + service_id=commit.repository.service_id, + repoid=commit.repository.repoid, + ), + owner=OwnerInfo( + service_id=commit.repository.owner.service_id, + ownerid=commit.repository.ownerid, + username=commit.repository.owner.username, + ), + installation=GithubInstallationInfo( + id=app.id, app_id=12, installation_id=1200, pem_path=None + ), + fallback_installations=None, + ) + mock_get_instance.assert_called_with( + "github", + **data, + token="the app token", + token_type_mapping=None, + on_token_refresh=None, + ) + + class TestPullRequestFetcher(object): @pytest.mark.asyncio async def test_fetch_and_update_pull_request_information_from_commit_new_pull_commits_in_place(