From 135b568c75bb10c09a92291193d11abb287de36b Mon Sep 17 00:00:00 2001 From: Andrew Halberstadt Date: Wed, 2 Sep 2026 11:36:21 -0400 Subject: [PATCH 1/2] Bug 2068710 - Bump taskcluster dependency to at least 106.0.0 This is the first version that implements the auth service's ability to hand out Github tokens. --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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", ] From 94eff61c5ab18d1c41df1cd5b3de8e7dc4486777 Mon Sep 17 00:00:00 2001 From: Andrew Halberstadt Date: Wed, 2 Sep 2026 11:48:35 -0400 Subject: [PATCH 2/2] Bug 2068710 - Use Taskcluster Auth service to obtain Github token if possible If there's an error (e.g missing scopes or app not installed on the target repo), we fallback to the existing github_oauth_token. --- scriptworker.yaml.tmpl | 9 ++++-- src/scriptworker/constants.py | 2 ++ src/scriptworker/cot/verify.py | 9 +++--- src/scriptworker/github.py | 38 +++++++++++++++++++++-- src/scriptworker/task.py | 2 +- tests/test_cot_verify.py | 10 +++--- tests/test_github.py | 56 ++++++++++++++++++++++++++++------ 7 files changed, 100 insertions(+), 26 deletions(-) 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