diff --git a/pyproject.toml b/pyproject.toml index 8500cbef..619a31e0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ dependencies = [ "jsonschema[format-nongpl]", "json-e>=2.5.0", "PyYAML", - "taskcluster>=40", + "taskcluster>=106", "taskcluster-taskgraph", ] diff --git a/scriptworker.yaml.tmpl b/scriptworker.yaml.tmpl index 79357b4b..6aab3f81 100644 --- a/scriptworker.yaml.tmpl +++ b/scriptworker.yaml.tmpl @@ -45,9 +45,12 @@ verify_cot_signature: false # Chain of Trust job type, e.g. signing cot_job_type: scriptworker cot_product: firefox -# Calls to Github API are limited to 60 an hour. Using an API token allows to raise the limit to -# 5000 per hour. https://developer.github.com/v3/#rate-limiting -github_oauth_token: somegithubtoken + +# Scriptworker first tries to obtain a repository scoped token from Taskcluster's auth service. +# This token is used as a fallback if that fails (e.g. missing scopes or app not configured). +# Without either, calls to the Github API are unauthenticated and limited to 60 an hour. See +# https://developer.github.com/v3/#rate-limiting +# github_oauth_token: somegithubtoken #----------------------------------------------------------------------------------------------- diff --git a/src/scriptworker/constants.py b/src/scriptworker/constants.py index cce34c35..bb41d107 100644 --- a/src/scriptworker/constants.py +++ b/src/scriptworker/constants.py @@ -79,6 +79,8 @@ "max_chain_length": 20, # Calls to Github API are limited to 60 an hour. Using an API token allows to raise the limit to # 5000 per hour. https://developer.github.com/v3/#rate-limiting + # Scriptworker first tries to obtain a repository scoped token from Taskcluster's auth + # service, falling back to this token if that fails. "github_oauth_token": "", # ed25519 settings "ed25519_private_key_path": "...", diff --git a/src/scriptworker/cot/verify.py b/src/scriptworker/cot/verify.py index 1a33481f..f041fb09 100644 --- a/src/scriptworker/cot/verify.py +++ b/src/scriptworker/cot/verify.py @@ -1118,7 +1118,7 @@ async def _get_additional_github_releases_jsone_context(decision_link): repo_owner, repo_name = extract_github_repo_owner_and_name(repo_url) tag_name = get_revision(task, source_env_prefix) - github_repo = GitHubRepository(repo_owner, repo_name, context.config["github_oauth_token"]) + github_repo = GitHubRepository(context, repo_owner, repo_name) release_data = await github_repo.get_release(tag_name) # The release data expose by the API[1] is not the same as the original event[2]. That's why @@ -1200,9 +1200,8 @@ async def _get_additional_github_pull_request_jsone_context(decision_link): repo_url = repo_url.replace("git@github.com:", "ssh://github.com/", 1) repo_owner, repo_name = extract_github_repo_owner_and_name(repo_url) pull_request_number = get_pull_request_number(task, source_env_prefix) - token = context.config["github_oauth_token"] - github_repo = GitHubRepository(repo_owner, repo_name, token) + github_repo = GitHubRepository(context, repo_owner, repo_name) repo_definition = github_repo.definition # We need to query the repository where the pull request was made to extract @@ -1210,7 +1209,7 @@ async def _get_additional_github_pull_request_jsone_context(decision_link): # the commit, or an upstream repo. We can compare the base and head repo URLs # to infer where the pull request lives. if repo_definition["fork"] and base_repo_url != repo_url: - github_repo = GitHubRepository(owner=repo_definition["parent"]["owner"]["login"], repo_name=repo_definition["parent"]["name"], token=token) + github_repo = GitHubRepository(context, repo_definition["parent"]["owner"]["login"], repo_definition["parent"]["name"]) pull_request_data = await github_repo.get_pull_request(pull_request_number) # Even though pull_request_data['head']['repo']['pushed_at'] does exist, @@ -1245,7 +1244,7 @@ async def _get_additional_github_push_jsone_context(decision_link): repo_owner, repo_name = extract_github_repo_owner_and_name(repo_url) commit_hash = get_revision(task, source_env_prefix) - github_repo = GitHubRepository(repo_owner, repo_name, context.config["github_oauth_token"]) + github_repo = GitHubRepository(context, repo_owner, repo_name) commit_data = await github_repo.get_commit(commit_hash) committer = commit_data["committer"] or {} diff --git a/src/scriptworker/github.py b/src/scriptworker/github.py index 0a7b2154..a7bf0bf0 100644 --- a/src/scriptworker/github.py +++ b/src/scriptworker/github.py @@ -6,7 +6,9 @@ from github3 import GitHub from github3.exceptions import GitHubException +from taskcluster.exceptions import TaskclusterFailure +import taskcluster from scriptworker.exceptions import ConfigError from scriptworker.utils import get_parts_of_url_path, get_single_item_from_sequence, retry_async_decorator, retry_request, retry_sync @@ -23,21 +25,51 @@ class GitHubRepository: """Wrapper around GitHub API. Used to access public data.""" - def __init__(self, owner, repo_name, token=""): + GITHUB_APP_NAME = "read" + GITHUB_PERMISSIONS = {"contents": "read", "metadata": "read", "pull_requests": "read"} + + def __init__(self, context, owner, repo_name): """Build the GitHub API URL which points to the definition of the repository. Args: - owner (str): the owner's GitHub username + context (scriptworker.context.Context): the scriptworker context + owner (str): the owner of the repository repo_name (str): the name of the repository - token (str): the GitHub API token Returns: dict: a representation of the repo definition """ + token = self._get_token(context, owner, repo_name) github = retry_sync(GitHub, kwargs={"token": token}, sleeptime_kwargs=_GITHUB_LIBRARY_SLEEP_TIME_KWARGS) self._github_repository = retry_sync(github.repository, args=(owner, repo_name), sleeptime_kwargs=_GITHUB_LIBRARY_SLEEP_TIME_KWARGS) + def _get_token(self, context, owner, repo_name): + """Get a repository-scoped GitHub token from Taskcluster's auth service. + + Falls back to ``context.config["github_oauth_token"]`` if the auth service call + fails, e.g. because of missing scopes. + + Args: + context (scriptworker.context.Context): the scriptworker context + owner (str): the owner of the repository + repo_name (str): the name of the repository + + Returns: + str: the scoped GitHub token, or the fallback token + + """ + if not context.credentials: + return context.config.get("github_oauth_token", "") + + try: + auth = taskcluster.Auth(options={"rootUrl": context.config["taskcluster_root_url"], "credentials": context.credentials}) + response = auth.githubRepoToken(self.GITHUB_APP_NAME, owner, payload={"repositories": [repo_name], "permissions": self.GITHUB_PERMISSIONS}) + return response["token"] + except TaskclusterFailure as e: + log.warning(f"Could not obtain Github token from Taskcluster for {owner}/{repo_name}, falling back to `github_oauth_token`: {e}") + return context.config.get("github_oauth_token", "") + @property def definition(self): """Fetch the definition of the repository, exposed by the GitHub API. diff --git a/src/scriptworker/task.py b/src/scriptworker/task.py index 19fc6660..7284c3ce 100644 --- a/src/scriptworker/task.py +++ b/src/scriptworker/task.py @@ -550,7 +550,7 @@ async def is_pull_request(context, task): if not revision and can_skip: continue - github_repository = GitHubRepository(repo_owner, repo_name, context.config["github_oauth_token"]) + github_repository = GitHubRepository(context, repo_owner, repo_name) conditions.append(not await github_repository.has_commit_landed_on_repository(context, revision)) return any(conditions) diff --git a/tests/test_cot_verify.py b/tests/test_cot_verify.py index c5e1dd74..b1c9f4fc 100644 --- a/tests/test_cot_verify.py +++ b/tests/test_cot_verify.py @@ -9,7 +9,7 @@ import time from copy import deepcopy from functools import partial -from unittest.mock import MagicMock +from unittest.mock import ANY, MagicMock import aiohttp import jsone @@ -1130,7 +1130,7 @@ async def get_release_mock(release_name, *args, **kwargs): context = await cotverify.populate_jsone_context(mobile_chain, mobile_github_release_link, mobile_github_release_link, tasks_for="github-release") - github_repo_class_mock.assert_called_once_with("mozilla-mobile", "reference-browser", "fakegithubtoken") + github_repo_class_mock.assert_called_once_with(ANY, "mozilla-mobile", "reference-browser") del context["as_slugid"] assert context == { "event": { @@ -1210,7 +1210,7 @@ async def get_commit_mock(commit_hash, *args, **kwargs): context = await cotverify.populate_jsone_context(mobile_chain, mobile_github_push_link, mobile_github_push_link, tasks_for="github-push") - github_repo_class_mock.assert_called_once_with("mozilla-mobile", "reference-browser", "fakegithubtoken") + github_repo_class_mock.assert_called_once_with(ANY, "mozilla-mobile", "reference-browser") del context["as_slugid"] assert context == { "event": { @@ -1320,10 +1320,10 @@ async def get_pull_request_mock(pull_request_number, *args, **kwargs): mobile_chain_pull_request, mobile_github_pull_request_link, mobile_github_pull_request_link, tasks_for=tasks_for ) - github_repo_class_mock.assert_any_call("JohanLorenzo", "reference-browser", "fakegithubtoken") + github_repo_class_mock.assert_any_call(ANY, "JohanLorenzo", "reference-browser") if expected_use_parent: - github_repo_class_mock.assert_any_call(owner="mozilla-mobile", repo_name="reference-browser", token="fakegithubtoken") + github_repo_class_mock.assert_any_call(ANY, "mozilla-mobile", "reference-browser") assert len(github_repo_class_mock.call_args_list) == 2 else: assert len(github_repo_class_mock.call_args_list) == 1 diff --git a/tests/test_github.py b/tests/test_github.py index 4d315f76..d2eb2fbe 100644 --- a/tests/test_github.py +++ b/tests/test_github.py @@ -4,6 +4,7 @@ from unittest.mock import MagicMock, patch import pytest +import taskcluster.exceptions from scriptworker import github from scriptworker.exceptions import ConfigError, ScriptWorkerRetryException @@ -31,7 +32,15 @@ def vpn_context(vpn_private_rw_context): @pytest.fixture(scope="function") -def github_repository(mocker): +def token_context(): + return SimpleNamespace( + config={"github_oauth_token": "fallback-token", "taskcluster_root_url": "https://tc.example.com"}, + credentials={"a": "b"}, + ) + + +@pytest.fixture(scope="function") +def github_repository(mocker, token_context): github_repository_mock = MagicMock() github_repository_mock.__name__ = "GithubRepositoryMock" github_repository_mock.html_url = "https://github.com/some-user/some-repo/" @@ -43,28 +52,56 @@ def github_repository(mocker): github_instance_mock.repository.return_value = github_repository_mock github_class_mock = mocker.patch.object(github, "GitHub", return_value=github_instance_mock) github_class_mock.__name__ = github_class_mock.name - yield github.GitHubRepository("some-user", "some-repo") + mocker.patch.object(github.taskcluster, "Auth", side_effect=taskcluster.exceptions.TaskclusterFailure("disabled in tests")) + yield github.GitHubRepository(token_context, "some-user", "some-repo") + + +def test_constructor(mocker, token_context): + github_instance_mock = MagicMock() + github_instance_mock.repository.__name__ = "github_instance_repository_mock" + github_class_mock = mocker.patch.object(github, "GitHub", return_value=github_instance_mock) + github_class_mock.__name__ = github_class_mock.name + mocker.patch.object(github.taskcluster, "Auth", side_effect=taskcluster.exceptions.TaskclusterFailure("disabled in tests")) + + github.GitHubRepository(token_context, "some-user", "some-repo") + + github_class_mock.assert_called_once_with(token="fallback-token") + github_instance_mock.repository.assert_called_once_with("some-user", "some-repo") @pytest.mark.parametrize( - "args, expected_class_kwargs", ((("some-user", "some-repo", "some-token"), {"token": "some-token"}), (("some-user", "some-repo"), {"token": ""})) + "raises, expected_token", + ( + (False, "scoped-token"), + (True, "fallback-token"), + ), ) -def test_constructor(mocker, args, expected_class_kwargs): +def test_constructor_with_context(mocker, token_context, raises, expected_token): github_instance_mock = MagicMock() github_instance_mock.repository.__name__ = "github_instance_repository_mock" github_class_mock = mocker.patch.object(github, "GitHub", return_value=github_instance_mock) github_class_mock.__name__ = github_class_mock.name - github.GitHubRepository(*args) + auth_instance_mock = MagicMock() + if raises: + auth_instance_mock.githubRepoToken.side_effect = taskcluster.exceptions.TaskclusterRestFailure("missing scopes", None, status_code=403) + else: + auth_instance_mock.githubRepoToken.return_value = {"token": "scoped-token", "expires": "2020-01-01T00:00:00Z"} + auth_class_mock = mocker.patch.object(github.taskcluster, "Auth", return_value=auth_instance_mock) - github_class_mock.assert_called_once_with(**expected_class_kwargs) - github_instance_mock.repository.assert_called_once_with("some-user", "some-repo") + github.GitHubRepository(token_context, "some-user", "some-repo") + + github_class_mock.assert_called_once_with(token=expected_token) + auth_class_mock.assert_called_once_with(options={"rootUrl": "https://tc.example.com", "credentials": {"a": "b"}}) + auth_instance_mock.githubRepoToken.assert_called_once_with( + github.GitHubRepository.GITHUB_APP_NAME, "some-user", payload={"repositories": ["some-repo"], "permissions": github.GitHubRepository.GITHUB_PERMISSIONS} + ) retry_count = {} -def test_constructor_uses_retry_sync(mocker): +def test_constructor_uses_retry_sync(mocker, token_context): global retry_count retry_count["fail_first"] = 0 @@ -80,8 +117,9 @@ def fail_first(*args, **kwargs): github_class_mock = mocker.patch.object(github, "GitHub", side_effect=fail_first) github_class_mock.__name__ = github_class_mock.name + mocker.patch.object(github.taskcluster, "Auth", side_effect=taskcluster.exceptions.TaskclusterFailure("disabled in tests")) mocker.patch.object(github, "_GITHUB_LIBRARY_SLEEP_TIME_KWARGS", {"delay_factor": 0.1}) - github.GitHubRepository("some-user", "some-repo", "some-token") + github.GitHubRepository(token_context, "some-user", "some-repo") assert retry_count["fail_first"] == 2