From c1b92b381a78fd7e6696343810320555fa99dee9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radek=20Ma=C5=88=C3=A1k?= Date: Sun, 9 Nov 2025 19:07:52 +0100 Subject: [PATCH 1/2] Migrate from github3.py to PyGithub --- pyproject.toml | 2 +- rebasebot/bot.py | 107 ++++++++++++++++---------------- rebasebot/cli.py | 4 +- rebasebot/github.py | 113 +++++++++++++++++++++++++--------- rebasebot/lifecycle_hooks.py | 68 ++++++++++++++------ tests/conftest.py | 2 + tests/test_bot.py | 92 ++++++++++++++------------- tests/test_github.py | 83 +++++++++++++++++++++++++ tests/test_lifecycle_hooks.py | 28 ++++++++- tests/test_rebase_summary.py | 23 ++++--- tests/test_rebases.py | 55 ++++++++--------- uv.lock | 99 ++++++++++++++++------------- 12 files changed, 444 insertions(+), 232 deletions(-) create mode 100644 tests/test_github.py diff --git a/pyproject.toml b/pyproject.toml index 3ebc2b2..d21f811 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,7 +10,7 @@ readme = "README.md" requires-python = ">=3.11" dependencies = [ - "github3-py==4.0.1", + "PyGithub==2.9.1", "gitpython==3.1.46", "requests==2.33.1", ] diff --git a/rebasebot/bot.py b/rebasebot/bot.py index 9254bfc..413d487 100755 --- a/rebasebot/bot.py +++ b/rebasebot/bot.py @@ -27,12 +27,11 @@ import git import git.compat -import github3 import requests from git.objects import Commit -from github3.pulls import ShortPullRequest -from github3.repos.commit import ShortCommit -from github3.repos.repo import Repository +from github import Github, GithubException +from github.PullRequest import PullRequest +from github.Repository import Repository from rebasebot import lifecycle_hooks from rebasebot.github import GithubAppProvider, GitHubBranch @@ -114,9 +113,9 @@ def _needs_rebase(gitwd: git.Repo, source: GitHubBranch, dest: GitHubBranch) -> def _is_pr_merged(pr_number: int, source_repo: Repository, gitwd: git.Repo, source_branch: str) -> bool: logging.info("Checking that PR %s has been merged and is included in %s", pr_number, source_branch) - gh_pr = source_repo.pull_request(pr_number) + gh_pr = source_repo.get_pull(pr_number) - if not gh_pr.is_merged(): + if not gh_pr.merged: return False merge_commit_sha = gh_pr.merge_commit_sha @@ -729,21 +728,24 @@ def _cherrypick_art_pull_request( """ logging.info("Checking for ART pull request") content_loss_warnings: list[ContentLossWarning] = [] - for pull_request in dest_repo.pull_requests(state="open", base=f"{dest.branch}"): - assert isinstance(pull_request, ShortPullRequest) # type hint + for pull_request in dest_repo.get_pulls(state="open", base=dest.branch): if "consistent with ART" in pull_request.title and pull_request.user.login == "openshift-bot": logging.info(f"Found open ART image update pull requst: {pull_request.title}") - remote = pull_request.head.repository + head = pull_request.head + if head is None or head.repo is None: + logging.warning("Skipping PR with deleted head repository: %s", pull_request.html_url) + continue + + remote = head.repo remote_name = remote.name if remote_name in gitwd.remotes: gitwd.remotes[remote_name].set_url(remote.html_url) else: gitwd.create_remote(remote_name, remote.html_url) - gitwd.remotes[remote_name].fetch(pull_request.head.ref) + gitwd.remotes[remote_name].fetch(head.ref) - for commit in pull_request.commits(): - assert isinstance(commit, ShortCommit) + for commit in pull_request.get_commits(): commit_message = commit.commit.message.split("\n", 1)[0] if commit.commit.message else commit.sha pick_result = _safe_cherry_pick( gitwd=gitwd, @@ -793,15 +795,20 @@ def _is_pr_required(gitwd: git.Repo, rebase: GitHubBranch, dest: GitHubBranch) - return True -def _is_pr_available(dest_repo: Repository, dest: GitHubBranch, rebase: GitHubBranch) -> tuple[ShortPullRequest, bool]: +def _is_pr_available( + dest_repo: Repository, dest: GitHubBranch, rebase: GitHubBranch +) -> tuple[PullRequest | None, bool]: logging.info("Checking for existing pull request") - pull_requests = dest_repo.pull_requests(base=dest.branch, state="open") + pull_requests = dest_repo.get_pulls(base=dest.branch, state="open") # Github does not support filtering cross-repository pull requests if both repositories # are owned by the same organization. We must filter client side. for pr in pull_requests: - pr_repo = pr.as_dict()["head"]["repo"]["full_name"] - if pr_repo == f"{rebase.ns}/{rebase.name}" and pr.head.ref == rebase.branch: + if pr.head.repo is None: + logging.warning(f"Skipping PR with deleted head repository: {pr.html_url}") + continue + pr_repo = pr.head.repo.full_name + if pr_repo == rebase.full_name and pr.head.ref == rebase.branch: logging.info('Found existing pull request: "%s" %s', pr.title, pr.html_url) return pr, True @@ -811,7 +818,7 @@ def _is_pr_available(dest_repo: Repository, dest: GitHubBranch, rebase: GitHubBr def _create_pr( *, - gh_app: github3.GitHub, + gh_app: Github, dest: GitHubBranch, source: GitHubBranch, rebase: GitHubBranch, @@ -828,31 +835,19 @@ def _create_pr( logging.info("Creating a pull request") - # FIXME(rmanak): This hack is because github3 doesn't support setting - # head_repo param when creating a PR. - # - # This param is required when creating cross-repository pull requests if both repositories - # are owned by the same organization. - # - # https://github.com/sigmavirus24/github3.py/issues/1190 - - gh_pr: requests.Response = gh_app._post( # pylint: disable=W0212 - f"https://api.github.com/repos/{dest.ns}/{dest.name}/pulls", - data={ - "title": title, - "body": build_pr_body(summary, source, dest, prow_job), - "head": rebase.branch, - "head_repo": f"{rebase.ns}/{rebase.name}", - "base": dest.branch, - "maintainer_can_modify": False, - }, - json=True, + dest_repo = gh_app.get_repo(dest.full_name) + + pr = dest_repo.create_pull( + title=title, + body=build_pr_body(summary, source, dest, prow_job), + head=f"{rebase.ns}:{rebase.branch}", + base=dest.branch, + maintainer_can_modify=False, ) - logging.debug(gh_pr.json()) - gh_pr.raise_for_status() + logging.info(f"Created pull request: {pr.html_url}") - return gh_pr.json()["html_url"] + return pr.html_url def is_ref_a_tag(gitwd: git.Repo, ref: str) -> bool: @@ -979,12 +974,12 @@ def _init_working_dir( return gitwd -def _manual_rebase_pr_in_repo(repo: Repository) -> ShortPullRequest | None: +def _manual_rebase_pr_in_repo(repo: Repository) -> PullRequest | None: """Checks for the presence of a rebase/manual label on the pull request.""" - prs = repo.pull_requests() + prs = repo.get_pulls() for pull_req in prs: for label in pull_req.labels: - if label["name"] == "rebase/manual": + if label.name == "rebase/manual": return pull_req return None @@ -998,7 +993,7 @@ def _push_rebase_branch(gitwd: git.Repo, rebase: GitHubBranch) -> None: def _update_pr_body( - pull_req: ShortPullRequest, + pull_req: PullRequest, summary: RebaseSummary, source: GitHubBranch, dest: GitHubBranch, @@ -1007,11 +1002,13 @@ def _update_pr_body( """Regenerate and overwrite the pull request body on every push.""" body = build_pr_body(summary, source, dest, prow_job) logging.info("Updating pull request body") - if not pull_req.update(body=body): - raise PullRequestUpdateException(f"Error updating body for pull request: {pull_req.html_url}") + try: + pull_req.edit(body=body) + except GithubException as ex: + raise PullRequestUpdateException(f"Error updating body for pull request: {pull_req.html_url}") from ex -def _update_pr_title(gitwd: git.Repo, pull_req: ShortPullRequest, source: GitHubBranch, dest: GitHubBranch) -> None: +def _update_pr_title(gitwd: git.Repo, pull_req: PullRequest, source: GitHubBranch, dest: GitHubBranch) -> None: """Updates the pull request title to match the current state of the rebase branch Only updates the title if the title contains the word Merge. Keeping everything before "Merge" and updating everything after. @@ -1028,8 +1025,10 @@ def _update_pr_title(gitwd: git.Repo, pull_req: ShortPullRequest, source: GitHub return logging.info(f"Updating pull request title: {computed_title}") - if not pull_req.update(title=computed_title): - raise PullRequestUpdateException(f"Error updating title for pull request: {pull_req.html_url}") + try: + pull_req.edit(title=computed_title) + except GithubException as ex: + raise PullRequestUpdateException(f"Error updating title for pull request: {pull_req.html_url}") from ex else: logging.info( f'Open pull request title "{pull_req.title}" does not match rebasebot format. Keeping the current title.' @@ -1097,7 +1096,7 @@ def run( conflict_policy: str = "auto", bot_emails: list, exclude_commits: list, - hooks: lifecycle_hooks.LifecycleHooks = None, + hooks: lifecycle_hooks.LifecycleHooks | None = None, update_go_modules: bool = False, dry_run: bool = False, ignore_manual_label: bool = False, @@ -1121,11 +1120,11 @@ def run( hooks = lifecycle_hooks.LifecycleHooks(tmp_script_dir=None, args=None) try: - dest_repo = gh_app.repository(dest.ns, dest.name) + dest_repo = gh_app.get_repo(dest.full_name) logging.info("Destination repository is %s", dest_repo.clone_url) - rebase_repo = gh_cloner_app.repository(rebase.ns, rebase.name) + rebase_repo = gh_cloner_app.get_repo(rebase.full_name) logging.info("rebase repository is %s", rebase_repo.clone_url) - source_repo = gh_app.repository(source.ns, source.name) + source_repo = gh_app.get_repo(source.full_name) logging.info("source repository is %s", source_repo.clone_url) if not ignore_manual_label: @@ -1326,8 +1325,8 @@ def run( ) notify_slack_error(notifier, manual_intervention_description, ex) return False - except requests.exceptions.HTTPError as ex: - logging.error(f"Failed to create a pull request: {ex}\n Response: %s", ex.response.text) + except GithubException as ex: + logging.error(f"Failed to create a pull request: {ex}") notify_slack_error(notifier, f"Failed to create a pull request for `{dest.label}`", ex) return False diff --git a/rebasebot/cli.py b/rebasebot/cli.py index 00be4dd..4966512 100755 --- a/rebasebot/cli.py +++ b/rebasebot/cli.py @@ -383,8 +383,8 @@ def main(): """Rebase Bot entry point function.""" args = _parse_cli_arguments() - # Silence info logs from github3 - logger = logging.getLogger("github3") + # Silence info logs from PyGithub + logger = logging.getLogger("github") logger.setLevel(logging.WARN) slack_webhook = None diff --git a/rebasebot/github.py b/rebasebot/github.py index be0a994..8a3c726 100644 --- a/rebasebot/github.py +++ b/rebasebot/github.py @@ -13,16 +13,16 @@ # under the License. """Contains GitHub related helper classes.""" -import builtins import logging import re from dataclasses import dataclass from functools import cached_property from urllib.parse import urlparse -import github3 +from github import Auth, Github, GithubIntegration, UnknownObjectException logger = logging.getLogger() +GITHUB_BRANCH_PATTERN = re.compile(r"^(?P[^/]+)/(?P[^:]+):(?P.*)$") @dataclass @@ -46,8 +46,13 @@ def label(self) -> str: """A short human-readable identifier, e.g. 'openshift/api:main'.""" return f"{self.ns}/{self.name}:{self.branch}" + @property + def full_name(self) -> str: + """Repository name in the form owner/name.""" + return f"{self.ns}/{self.name}" + -def parse_github_branch(repository_string) -> GitHubBranch: +def parse_github_branch(repository_string: str) -> GitHubBranch: """ parse_github_branch constructs GitHubBranch object from the provided location. The repository_string format is /: @@ -59,8 +64,7 @@ def parse_github_branch(repository_string) -> GitHubBranch: # Remove prefix if it's a URL repository_string = repository_string.removeprefix("https://github.com/") - github_regex = re.compile(r"^(?P[^/]+)/(?P[^:]+):(?P.*)$") - match = github_regex.match(repository_string) + match = GITHUB_BRANCH_PATTERN.match(repository_string) if match is None: raise ValueError("GitHub branch value must be in the form /:") @@ -124,6 +128,9 @@ def __init__( self._app_credentials = None self._cloner_app_credentials = None + if self.user_auth and not self.user_token: + raise ValueError("User authentication requires a GitHub user token") + if not user_auth: if not all((app_id, app_key, dest_branch, cloner_id, cloner_key, rebase_branch)): raise ValueError("Credentials for both, cloning and pushing app should be provided") @@ -140,7 +147,10 @@ def get_app_token(self) -> str: :return: str """ - return self.github_app.session.auth.token + if self.user_auth: + return self._require_user_token() + + return self._get_installation_token(self._app_installation_context, "app") def get_cloner_token(self) -> str: """ @@ -148,60 +158,107 @@ def get_cloner_token(self) -> str: :return: str """ - return self.github_cloner_app.session.auth.token + if self.user_auth: + return self._require_user_token() + + return self._get_installation_token(self._cloner_app_installation_context, "cloner") @cached_property - def github_app(self) -> github3.GitHub: + def github_app(self) -> Github: """ Authenticated GitHub app. In case `user_auth` = True, returns app authenticated with user token. In app mode `app_id`, `app_key` and `dest_branch` will be used for app authentication. - :return: github3.GitHub + :return: Github """ if self.user_auth: return self._get_github_user_logged_in_app() - return self._github_login_app(self._app_credentials) + return self._get_github_installation_app(self._app_installation_context) @cached_property - def github_cloner_app(self) -> github3.GitHub: + def github_cloner_app(self) -> Github: """ Authenticated GitHub app. In case `user_auth` = True, returns app authenticated with user token. In app mode `cloner_id`, `cloner_key` and `rebase_branch`will be used for app authentication. - :return: github3.GitHub + :return: Github """ if self.user_auth: return self._get_github_user_logged_in_app() - return self._github_login_app(self._cloner_app_credentials) + return self._get_github_installation_app(self._cloner_app_installation_context) + + @cached_property + def _app_installation_context(self) -> tuple[GithubIntegration, int]: + return self._get_installation_context(self._require_credentials(self._app_credentials, "app")) + + @cached_property + def _cloner_app_installation_context(self) -> tuple[GithubIntegration, int]: + return self._get_installation_context(self._require_credentials(self._cloner_app_credentials, "cloner")) + + def _require_user_token(self) -> str: + if self.user_token is None: + raise RuntimeError("GitHub user token is unavailable") + return self.user_token + + @staticmethod + def _require_credentials(credentials: GitHubAppCredentials | None, role: str) -> GitHubAppCredentials: + if credentials is None: + raise RuntimeError(f"GitHub {role} credentials are unavailable") + return credentials + + @staticmethod + def _get_github_installation_app(installation_context: tuple[GithubIntegration, int]) -> Github: + """ + Build a Github client authenticated as a GitHub App installation. + + :return: Github + """ + integration, installation_id = installation_context + return integration.get_github_for_installation(installation_id) @staticmethod - def _github_login_app(credentials: GitHubAppCredentials) -> github3.GitHub: + def _get_installation_token(installation_context: tuple[GithubIntegration, int], role: str) -> str: + """ + Fetch a fresh access token for a GitHub App installation. + + :return: access token + """ + integration, installation_id = installation_context + token = integration.get_access_token(installation_id).token + if not token: + raise RuntimeError(f"GitHub {role} token is unavailable") + return token + + @staticmethod + def _get_installation_context(credentials: GitHubAppCredentials) -> tuple[GithubIntegration, int]: + """ + Authenticate as a GitHub App and locate its installation for a repository. + + :return: tuple of (GithubIntegration, installation_id) + """ logging.info("Logging to GitHub as an Application for repository %s", credentials.github_branch.url) - gh_app = github3.GitHub() - app_id = str(credentials.app_id) - gh_app.login_as_app(credentials.app_key, app_id, expire_in=300) gh_branch = credentials.github_branch + app_auth = Auth.AppAuth(credentials.app_id, credentials.app_key.decode("utf-8")) + gi = GithubIntegration(auth=app_auth) + try: - install = gh_app.app_installation_for_repository(owner=gh_branch.ns, repository=gh_branch.name) - except github3.exceptions.NotFoundError as err: - msg = ( - f"App has not been authorized by {gh_branch.ns}, or repo {gh_branch.ns}/{gh_branch.name} does not exist" - ) + installation = gi.get_repo_installation(gh_branch.ns, gh_branch.name) + except UnknownObjectException as err: + msg = f"App has not been authorized by {gh_branch.ns}, or repo {gh_branch.full_name} does not exist" logging.error(msg) - raise builtins.Exception(msg) from err + raise RuntimeError(msg) from err - gh_app.login_as_app_installation(credentials.app_key, app_id, install.id) - return gh_app + return gi, installation.id - def _get_github_user_logged_in_app(self) -> github3.GitHub: + def _get_github_user_logged_in_app(self) -> Github: logging.info("Logging to GitHub as a User") - gh_app = github3.GitHub() - gh_app.login(token=self.user_token) + auth = Auth.Token(self._require_user_token()) + gh_app = Github(auth=auth) return gh_app diff --git a/rebasebot/lifecycle_hooks.py b/rebasebot/lifecycle_hooks.py index 0d89ca1..a18cfa3 100644 --- a/rebasebot/lifecycle_hooks.py +++ b/rebasebot/lifecycle_hooks.py @@ -25,7 +25,8 @@ import git import git.repo -from github3.repos.contents import Contents +from github import GithubException +from github.ContentFile import ContentFile from rebasebot.github import GithubAppProvider, parse_github_branch @@ -126,22 +127,34 @@ def _fetch_from_remote_git( raise ValueError(f"Failed to retrieve script from git reference {git_path}") from e def _fetch_from_github_api( - self, *, github, organization: str, name: str, git_repo_path_to_script: str, branch: str, script_file_path: str - ): + self, + *, + github: GithubAppProvider, + organization: str, + name: str, + git_repo_path_to_script: str, + branch: str, + script_file_path: str, + ) -> None: """Fetches script from GitHub API.""" + error_message = ( + f"Failed to retrieve script from github organization={organization}, " + f"name={name}, branch={branch}, path={git_repo_path_to_script}," + ) + + try: + script: ContentFile = _fetch_file_from_github(github, organization, name, branch, git_repo_path_to_script) + except ValueError: + raise + except GithubException as e: + raise ValueError(error_message) from e + try: - script: Contents = _fetch_file_from_github(github, organization, name, branch, git_repo_path_to_script) - with open( - script_file_path, - "wb", - ) as f: - f.write(script.decoded) + with open(script_file_path, "wb") as f: + f.write(script.decoded_content) os.chmod(script_file_path, 0o700) # Make it executable - except Exception as e: - raise ValueError( - f"Failed to retrieve script from github organization={organization}, " - f"name={name}, branch={branch}, path={git_repo_path_to_script}," - ) from e + except OSError as e: + raise ValueError(error_message) from e def _extract_script_details(self, script_location: str, temp_hook_dir: str) -> tuple[str, str, str]: """Extracts script details and generates the script file path.""" @@ -155,7 +168,12 @@ def _extract_script_details(self, script_location: str, temp_hook_dir: str) -> t script_file_path = f"{temp_hook_dir}/{basename}-{hash_suffix}{ext}" return git_ref, file_path, script_file_path - def fetch_script(self, temp_hook_dir: str, gitwd: git.Repo = None, github: GithubAppProvider = None): + def fetch_script( + self, + temp_hook_dir: str, + gitwd: git.Repo | None = None, + github: GithubAppProvider | None = None, + ) -> None: """Fetches the script from a git repository and stores it in a temporary directory. Prefers github API when source is GitHub, otherwise uses generic git library when available. """ @@ -205,7 +223,7 @@ def fetch_script(self, temp_hook_dir: str, gitwd: git.Repo = None, github: Githu def __str__(self): return self.script_location - def __call__(self, cwd: str = None) -> LifecycleHookScriptResult: + def __call__(self, cwd: str | None = None) -> LifecycleHookScriptResult: with subprocess.Popen( [self.script_file_path], stdout=subprocess.PIPE, @@ -236,10 +254,20 @@ def __call__(self, cwd: str = None) -> LifecycleHookScriptResult: return LifecycleHookScriptResult(return_code=return_code, stdout=stdout_lines, stderr=stderr_lines) -def _fetch_file_from_github(github, organization, name, branch, git_repo_path_to_script) -> Contents: - return github.github_cloner_app.repository(owner=organization, repository=name).file_contents( - git_repo_path_to_script, ref=branch - ) +def _fetch_file_from_github( + github: GithubAppProvider, organization: str, name: str, branch: str, git_repo_path_to_script: str +) -> ContentFile: + repo = github.github_cloner_app.get_repo(f"{organization}/{name}") + content = repo.get_contents(git_repo_path_to_script, ref=branch) + if isinstance(content, list): + raise ValueError( + f"Hook path '{git_repo_path_to_script}' in {organization}/{name}@{branch} is a directory, expected a file" + ) + if not isinstance(content, ContentFile): + raise ValueError( + f"Hook path '{git_repo_path_to_script}' in {organization}/{name}@{branch} did not resolve to a file" + ) + return content def _fetch_branch(gitwd: git.Repo, remote: str, branch: str, ref_filter: str | None = None): diff --git a/tests/conftest.py b/tests/conftest.py index d8acf9d..e37ebfd 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -70,6 +70,7 @@ def tmp_go_app_repo() -> YieldFixture[tuple[str, Repo]]: with repo.config_writer() as config: config.set_value("user", "email", "test@example.com") config.set_value("user", "name", "test") + config.set_value("commit", "gpgsign", "false") repo.git.add(all=True) repo.git.commit("-m", "Initial commit") yield tmpdir, repo @@ -171,6 +172,7 @@ def commit(self: CommitBuilder, commit_msg: str, committer_email: str = None) -> else: config.set_value("user", "email", f"{self.branch.name}_author@{self.branch.ns}.org") config.set_value("user", "name", f"{self.branch.name}_author") + config.set_value("commit", "gpgsign", "false") self.commited = True self.repo.git.commit("--allow-empty", "-m", commit_msg) return self.repo.head.commit diff --git a/tests/test_bot.py b/tests/test_bot.py index daefc29..d7dc57f 100644 --- a/tests/test_bot.py +++ b/tests/test_bot.py @@ -16,12 +16,14 @@ import pytest import requests +from github import GithubException from rebasebot import cli, lifecycle_hooks from rebasebot.bot import ( PullRequestUpdateException, RepoException, _add_to_rebase, + _cherrypick_art_pull_request, _is_pr_available, _is_pr_required, _report_result, @@ -199,45 +201,39 @@ def rebase(self): rebase = MagicMock() rebase.ns = "test-namespace" rebase.name = "rebase-repo" + rebase.full_name = "test-namespace/rebase-repo" rebase.branch = "rebase-branch" return rebase def test_is_pr_available(self, dest_repo, dest, rebase): # Test when pull request exists gh_pr = MagicMock() - gh_pr.as_dict.return_value = {"head": {"repo": {"full_name": "test-namespace/rebase-repo"}}} + gh_pr.head.repo.full_name = "test-namespace/rebase-repo" gh_pr.head.ref = rebase.branch gh_pr.state = "open" - dest_repo.pull_requests.return_value = [gh_pr] + dest_repo.get_pulls.return_value = [gh_pr] pr, pr_available = _is_pr_available(dest_repo, dest, rebase) - dest_repo.pull_requests.assert_called_once_with(base="dest-branch", state="open") + dest_repo.get_pulls.assert_called_once_with(base="dest-branch", state="open") assert pr == gh_pr assert pr_available is True def test_is_pr_available_not_found(self, dest_repo, dest, rebase): # Test when pull request doesn't exist - dest_repo.pull_requests.return_value = [] + dest_repo.get_pulls.return_value = [] pr, pr_available = _is_pr_available(dest_repo, dest, rebase) - dest_repo.pull_requests.assert_called_with(base="dest-branch", state="open") + dest_repo.get_pulls.assert_called_with(base="dest-branch", state="open") assert pr is None assert pr_available is False - def test_is_pr_available_closed(self, dest_repo, dest, rebase): + def test_is_pr_available_skips_deleted_head_repo(self, dest_repo, dest, rebase): gh_pr = MagicMock() - gh_pr.as_dict.return_value = {"head": {"repo": {"full_name": "test-namespace/rebase-repo"}}} - gh_pr.head.ref = rebase.branch - gh_pr.state = "closed" - - # Mock pull_requests to return only PRs that match the requested state - def mock_pull_requests(*, base, state): - all_prs = [gh_pr] - return [pr for pr in all_prs if pr.state == state] - - dest_repo.pull_requests.side_effect = mock_pull_requests + gh_pr.head.repo = None + gh_pr.html_url = "https://github.com/test-namespace/dest-repo/pull/1" + dest_repo.get_pulls.return_value = [gh_pr] pr, pr_available = _is_pr_available(dest_repo, dest, rebase) - dest_repo.pull_requests.assert_called_once_with(base="dest-branch", state="open") + dest_repo.get_pulls.assert_called_once_with(base="dest-branch", state="open") assert pr is None assert pr_available is False @@ -277,6 +273,29 @@ def test_missing_remote_refs_returns_true(self, dest, rebase): assert _is_pr_required(gitwd, rebase, dest) is True +class TestCherryPickArtPullRequest: + @patch("rebasebot.bot._safe_cherry_pick") + def test_skips_pull_request_with_deleted_head_repo(self, mocked_safe_cherry_pick, caplog): + gitwd = MagicMock() + gitwd.remotes = {} + dest_repo = MagicMock() + dest = MagicMock(branch="dest-branch") + caplog.set_level("WARNING") + pull_request = MagicMock() + pull_request.title = "Keep branch consistent with ART" + pull_request.user.login = "openshift-bot" + pull_request.head = MagicMock(repo=None) + pull_request.html_url = "https://github.com/test-namespace/dest-repo/pull/1" + dest_repo.get_pulls.return_value = [pull_request] + + _cherrypick_art_pull_request(gitwd, dest_repo, dest) + + dest_repo.get_pulls.assert_called_once_with(state="open", base="dest-branch") + gitwd.create_remote.assert_not_called() + mocked_safe_cherry_pick.assert_not_called() + assert "Skipping PR with deleted head repository" in caplog.text + + class TestReportResult: dest = GitHubBranch(url="https://github.com/user/repo", ns="user", name="repo", branch="main") label = dest.label @@ -431,7 +450,7 @@ class TestRunSlackErrorMessages: @patch("rebasebot.bot._push_rebase_branch") @patch("rebasebot.bot._is_pr_available") @patch("rebasebot.slack.requests.post") - def test_create_pr_http_error_fences_exception_and_response( + def test_create_pr_github_exception_fences_exception( self, mocked_post, mocked_is_pr_available, @@ -450,11 +469,8 @@ def test_create_pr_http_error_fences_exception_and_response( mocked_is_pr_available.return_value = None, False mocked_push_rebase_branch.return_value = True - mock_response = MagicMock() - mock_response.text = '{"message":"Validation Failed"}' - http_error = requests.exceptions.HTTPError("422 Client Error: Unprocessable Entity") - http_error.response = mock_response - mocked_create_pr.side_effect = http_error + github_error = GithubException(422, {"message": "Validation Failed"}, None) + mocked_create_pr.side_effect = github_error args = MagicMock() args.source = source @@ -482,10 +498,7 @@ def test_create_pr_http_error_fences_exception_and_response( result = cli.rebasebot_run(args, slack_webhook="test://webhook", github_app_wrapper=fake_github_provider) assert result is False - expected_message = ( - f"Failed to create a pull request for `{dest.label}`:\n" - f"```{http_error}```\nResponse:\n```{mock_response.text}```" - ) + expected_message = f"Failed to create a pull request for `{dest.label}`:\n```{github_error}```" expected_blocks = _build_slack_blocks(expected_message, "❌", None) mocked_post.assert_called_once_with( "test://webhook", @@ -502,7 +515,6 @@ def test_success(self): gitwd.git.rev_parse.return_value = "abcdefg" pull_req = MagicMock() pull_req.title = "Merge https://github.com/kubernetes/cloud-provider-aws:master (b80e8ef) into master" - pull_req.update.return_value = True source = MagicMock(branch="my-feature", url="https://github.com/my/repo") dest = MagicMock(branch="main") @@ -511,9 +523,7 @@ def test_success(self): except Exception as ex: raise AssertionError("Unexpected exception") from ex - pull_req.update.assert_called_once_with( - title=f"Merge {source.url}:{source.branch} (abcdefg) into {dest.branch}" - ) + pull_req.edit.assert_called_once_with(title=f"Merge {source.url}:{source.branch} (abcdefg) into {dest.branch}") def test_jira_link(self): gitwd = MagicMock() @@ -521,7 +531,6 @@ def test_jira_link(self): pull_req = MagicMock() pull_req.title = "OCPCLOUD-2051: Merge " "https://github.com/kubernetes/cloud-provider-aws:master (b80e8ef) into master" - pull_req.update.return_value = True source = MagicMock(branch="my-feature", url="https://github.com/my/repo") dest = MagicMock(branch="main") @@ -530,7 +539,7 @@ def test_jira_link(self): except Exception as ex: raise AssertionError("Unexpected exception") from ex - pull_req.update.assert_called_once_with( + pull_req.edit.assert_called_once_with( title=f"OCPCLOUD-2051: Merge {source.url}:{source.branch} (abcdefg) into {dest.branch}" ) @@ -541,7 +550,6 @@ def test_upstream_sync_prefix(self): pull_req.title = ( "UPSTREAM-SYNC: Merge https://github.com/kubernetes/cloud-provider-aws:master (b80e8ef) into master" ) - pull_req.update.return_value = True source = MagicMock(branch="my-feature", url="https://github.com/my/repo") dest = MagicMock(branch="main") @@ -550,7 +558,7 @@ def test_upstream_sync_prefix(self): except Exception as ex: raise AssertionError(f"Unexpected exception: {ex}") from ex - pull_req.update.assert_called_once_with( + pull_req.edit.assert_called_once_with( title=f"UPSTREAM-SYNC: Merge {source.url}:{source.branch} (abcdefg) into {dest.branch}" ) @@ -559,7 +567,6 @@ def test_unknown_format_keep_unchanged(self): gitwd.git.rev_parse.return_value = "abcdefg" pull_req = MagicMock() pull_req.title = "OCPCLOUD-2051: Manual rebase to lastest upstream version" - pull_req.update.return_value = True source = MagicMock(branch="my-feature", url="https://github.com/my/repo") dest = MagicMock(branch="main") @@ -568,29 +575,26 @@ def test_unknown_format_keep_unchanged(self): except Exception as ex: raise AssertionError("Unexpected exception") from ex - pull_req.update.assert_not_called() + pull_req.edit.assert_not_called() def test_failure(self): gitwd = MagicMock() gitwd.git.rev_parse.return_value = "abcdefg" pull_req = MagicMock() pull_req.title = "Merge https://github.com/kubernetes/cloud-provider-aws:master (b80e8ef) into master" - pull_req.update.return_value = False + pull_req.edit.side_effect = GithubException(500, {"message": "API error"}) source = MagicMock(branch="my-feature", url="https://github.com/my/repo") dest = MagicMock(branch="main") with pytest.raises(PullRequestUpdateException, match="Error updating title for pull request"): _update_pr_title(gitwd, pull_req, source, dest) - pull_req.update.assert_called_once_with( - title=f"Merge {source.url}:{source.branch} (abcdefg) into {dest.branch}" - ) + pull_req.edit.assert_called_once_with(title=f"Merge {source.url}:{source.branch} (abcdefg) into {dest.branch}") class TestUpdatePrBody: def test_success(self): pull_req = MagicMock() - pull_req.update.return_value = True source = GitHubBranch(url="https://github.com/upstream/repo", ns="upstream", name="repo", branch="main") dest = GitHubBranch(url="https://github.com/downstream/repo", ns="downstream", name="repo", branch="release") summary = RebaseSummary(upstream_commit_count=2) @@ -598,11 +602,11 @@ def test_success(self): _update_pr_body(pull_req, summary, source, dest, prow_job) - pull_req.update.assert_called_once_with(body=build_pr_body(summary, source, dest, prow_job)) + pull_req.edit.assert_called_once_with(body=build_pr_body(summary, source, dest, prow_job)) def test_failure(self): pull_req = MagicMock() - pull_req.update.return_value = False + pull_req.edit.side_effect = GithubException(500, {"message": "API error"}) pull_req.html_url = "https://github.com/downstream/repo/pull/1" source = GitHubBranch(url="https://github.com/upstream/repo", ns="upstream", name="repo", branch="main") dest = GitHubBranch(url="https://github.com/downstream/repo", ns="downstream", name="repo", branch="release") diff --git a/tests/test_github.py b/tests/test_github.py new file mode 100644 index 0000000..4c7f74f --- /dev/null +++ b/tests/test_github.py @@ -0,0 +1,83 @@ +from unittest.mock import MagicMock, patch + +import pytest + +from rebasebot.github import GithubAppProvider, GitHubBranch + + +class TestGithubAppProvider: + @staticmethod + def _github_branch(repo_name: str) -> GitHubBranch: + return GitHubBranch( + url=f"https://github.com/test-namespace/{repo_name}", + ns="test-namespace", + name=repo_name, + branch="main", + ) + + def _provider(self) -> GithubAppProvider: + return GithubAppProvider( + app_id=1, + app_key=b"app-key", + dest_branch=self._github_branch("dest-repo"), + cloner_id=2, + cloner_key=b"cloner-key", + rebase_branch=self._github_branch("rebase-repo"), + ) + + def test_user_auth_requires_user_token(self): + with pytest.raises(ValueError, match="User authentication requires a GitHub user token"): + GithubAppProvider(user_auth=True) + + @patch("rebasebot.github.GithubIntegration") + def test_github_app_uses_installation_client(self, mocked_integration_class): + provider = self._provider() + mocked_integration = mocked_integration_class.return_value + mocked_integration.get_repo_installation.return_value = MagicMock(id=123) + github_client = MagicMock() + mocked_integration.get_github_for_installation.return_value = github_client + + assert provider.github_app is github_client + + mocked_integration.get_repo_installation.assert_called_once_with("test-namespace", "dest-repo") + mocked_integration.get_github_for_installation.assert_called_once_with(123) + mocked_integration.get_access_token.assert_not_called() + + @patch("rebasebot.github.GithubIntegration") + def test_get_app_token_fetches_installation_token(self, mocked_integration_class): + provider = self._provider() + mocked_integration = mocked_integration_class.return_value + mocked_integration.get_repo_installation.return_value = MagicMock(id=123) + mocked_integration.get_access_token.return_value = MagicMock(token="app-token") + + assert provider.get_app_token() == "app-token" + + mocked_integration.get_repo_installation.assert_called_once_with("test-namespace", "dest-repo") + mocked_integration.get_access_token.assert_called_once_with(123) + + @patch("rebasebot.github.GithubIntegration") + def test_github_app_and_get_app_token_reuse_installation_context(self, mocked_integration_class): + provider = self._provider() + mocked_integration = mocked_integration_class.return_value + mocked_integration.get_repo_installation.return_value = MagicMock(id=123) + mocked_integration.get_github_for_installation.return_value = MagicMock() + mocked_integration.get_access_token.return_value = MagicMock(token="app-token") + + assert provider.github_app is mocked_integration.get_github_for_installation.return_value + assert provider.get_app_token() == "app-token" + + mocked_integration.get_repo_installation.assert_called_once_with("test-namespace", "dest-repo") + mocked_integration.get_github_for_installation.assert_called_once_with(123) + mocked_integration.get_access_token.assert_called_once_with(123) + + @patch("rebasebot.github.GithubIntegration") + def test_get_cloner_token_fetches_installation_token(self, mocked_integration_class): + provider = self._provider() + mocked_integration = mocked_integration_class.return_value + mocked_integration.get_repo_installation.return_value = MagicMock(id=456) + mocked_integration.get_access_token.return_value = MagicMock(token="cloner-token") + + assert provider.get_cloner_token() == "cloner-token" + + mocked_integration.get_repo_installation.assert_called_once_with("test-namespace", "rebase-repo") + mocked_integration.get_access_token.assert_called_once_with(456) diff --git a/tests/test_lifecycle_hooks.py b/tests/test_lifecycle_hooks.py index 72b5c9d..1283b28 100644 --- a/tests/test_lifecycle_hooks.py +++ b/tests/test_lifecycle_hooks.py @@ -1,4 +1,4 @@ -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest @@ -13,3 +13,29 @@ def test_fetch_script_rejects_malformed_git_location_before_extracting(tmp_path) script.fetch_script(str(tmp_path)) mock_extract.assert_not_called() + + +def test_fetch_file_from_github_rejects_directory_result(): + github = MagicMock() + repo = github.github_cloner_app.get_repo.return_value + repo.get_contents.return_value = [MagicMock()] + + with pytest.raises(ValueError, match=r"Hook path 'hooks' in org/repo@main is a directory, expected a file"): + lifecycle_hooks._fetch_file_from_github(github, "org", "repo", "main", "hooks") + + +def test_fetch_from_github_api_preserves_directory_validation_error(tmp_path): + script = lifecycle_hooks.LifecycleHookScript("git:https://github.com/org/repo/main:hooks") + github = MagicMock() + repo = github.github_cloner_app.get_repo.return_value + repo.get_contents.return_value = [MagicMock()] + + with pytest.raises(ValueError, match=r"Hook path 'hooks' in org/repo@main is a directory, expected a file"): + script._fetch_from_github_api( + github=github, + organization="org", + name="repo", + git_repo_path_to_script="hooks", + branch="main", + script_file_path=str(tmp_path / "hook.sh"), + ) diff --git a/tests/test_rebase_summary.py b/tests/test_rebase_summary.py index 6524075..06b0dd6 100644 --- a/tests/test_rebase_summary.py +++ b/tests/test_rebase_summary.py @@ -21,7 +21,6 @@ import pytest from git import Repo -from github3.pulls import ShortPullRequest from rebasebot.bot import _cherrypick_art_pull_request, _do_rebase, _init_working_dir, _prepare_rebase_branch from rebasebot.github import GitHubBranch @@ -72,7 +71,7 @@ def test_excluded_commit(self, working_repo_context, fake_github_provider): gitwd=ctx.working_repo, source=ctx.source, dest=ctx.dest, - source_repo=fake_github_provider.github_app.repository.return_value, + source_repo=fake_github_provider.github_app.get_repo.return_value, tag_policy="soft", bot_emails=[], exclude_commits=[excluded.hexsha], @@ -96,7 +95,7 @@ def test_tag_policy_drop(self, working_repo_context, fake_github_provider): gitwd=ctx.working_repo, source=ctx.source, dest=ctx.dest, - source_repo=fake_github_provider.github_app.repository.return_value, + source_repo=fake_github_provider.github_app.get_repo.return_value, tag_policy="strict", bot_emails=[], exclude_commits=[], @@ -119,7 +118,7 @@ def test_go_modules_drop(self, working_repo_context, fake_github_provider): gitwd=ctx.working_repo, source=ctx.source, dest=ctx.dest, - source_repo=fake_github_provider.github_app.repository.return_value, + source_repo=fake_github_provider.github_app.get_repo.return_value, tag_policy="soft", bot_emails=[], exclude_commits=[], @@ -149,7 +148,7 @@ def test_warn_policy_populates_content_loss_warnings(self, working_repo_context, gitwd=ctx.working_repo, source=ctx.source, dest=ctx.dest, - source_repo=fake_github_provider.github_app.repository.return_value, + source_repo=fake_github_provider.github_app.get_repo.return_value, tag_policy="soft", conflict_policy="warn", bot_emails=[], @@ -187,7 +186,7 @@ def test_auto_policy_leaves_content_loss_warnings_empty(self, working_repo_conte gitwd=ctx.working_repo, source=ctx.source, dest=ctx.dest, - source_repo=fake_github_provider.github_app.repository.return_value, + source_repo=fake_github_provider.github_app.get_repo.return_value, tag_policy="soft", conflict_policy="auto", bot_emails=[], @@ -202,7 +201,7 @@ class TestCherrypickArtPullRequest: @patch("rebasebot.bot._safe_cherry_pick") def test_returns_art_pr_info_when_matching_pr_exists(self, _mock_safe_cherry_pick, working_repo_context): ctx = working_repo_context - art_pr = MagicMock(spec=ShortPullRequest) + art_pr = MagicMock() art_pr.title = "Update build image to be consistent with ART" art_pr.user = MagicMock(login="openshift-bot") art_pr.number = 42 @@ -210,11 +209,11 @@ def test_returns_art_pr_info_when_matching_pr_exists(self, _mock_safe_cherry_pic repository = MagicMock() repository.name = "fork" repository.html_url = "https://github.com/downstream/fork" - art_pr.head = MagicMock(repository=repository, ref="art-update") - art_pr.commits.return_value = [] + art_pr.head = MagicMock(repo=repository, ref="art-update") + art_pr.get_commits.return_value = [] dest_repo = MagicMock() - dest_repo.pull_requests.return_value = [art_pr] + dest_repo.get_pulls.return_value = [art_pr] if "fork" not in [remote.name for remote in ctx.working_repo.remotes]: ctx.working_repo.create_remote("fork", ctx.dest.url) @@ -231,12 +230,12 @@ def test_returns_art_pr_info_when_matching_pr_exists(self, _mock_safe_cherry_pic def test_returns_none_when_no_matching_pr_exists(self, working_repo_context): ctx = working_repo_context - other_pr = MagicMock(spec=ShortPullRequest) + other_pr = MagicMock() other_pr.title = "Unrelated change" other_pr.user = MagicMock(login="openshift-bot") dest_repo = MagicMock() - dest_repo.pull_requests.return_value = [other_pr] + dest_repo.get_pulls.return_value = [other_pr] result, content_loss = _cherrypick_art_pull_request(ctx.working_repo, dest_repo, ctx.dest) diff --git a/tests/test_rebases.py b/tests/test_rebases.py index 90371c3..adb3d0f 100644 --- a/tests/test_rebases.py +++ b/tests/test_rebases.py @@ -509,17 +509,19 @@ def test_has_manual_rebase_pr( dest.clone_url = "https://github.com/dest/dest" pr = MagicMock() - pr.labels = [{"name": "rebase/manual"}] + label = MagicMock() + label.name = "rebase/manual" + pr.labels = [label] pr.html_url = "https://github.com/dest/dest/pull/1" mocked_manual_rebase_pr_in_repo.return_value = pr # Mock GitHub repository lookup function - def fake_repository_func(namespace, name): + def fake_get_repo_func(full_name): repository = MagicMock() - repository.clone_url = f"https://github.com/{namespace}/{name}" + repository.clone_url = f"https://github.com/{full_name}" return repository - fake_github_provider.github_app.repository = fake_repository_func + fake_github_provider.github_app.get_repo = fake_get_repo_func args = MagicMock() args.source = source @@ -600,7 +602,7 @@ def test_lifecyclehooks_remote( ): source, rebase, dest = init_test_repositories - mock_fetch_file_from_github.return_value.decoded = rb"""#!/bin/bash + mock_fetch_file_from_github.return_value.decoded_content = rb"""#!/bin/bash touch test-hook-script.success git add test-hook-script.success git commit -m 'UPSTREAM: : test-hook-script generated files' @@ -773,7 +775,7 @@ def test_source_ref_hook( cb.add_file("carry-file1", "content") cb.commit("UPSTREAM: : carry commit #1") - mock_fetch_file_from_github.return_value.decoded = rb"""#!/bin/sh + mock_fetch_file_from_github.return_value.decoded_content = rb"""#!/bin/sh echo main """ @@ -970,13 +972,17 @@ def test_always_run_hooks_preserves_carry_commits(self, init_test_repositories, @staticmethod def _setup_github_provider(fake_github_provider): - def fake_repository_func(namespace, name): - repository = MagicMock() - repository.clone_url = f"https://github.com/{namespace}/{name}" - return repository + repos = {} + + def fake_get_repo(full_name): + if full_name not in repos: + repository = MagicMock() + repository.clone_url = f"https://github.com/{full_name}" + repos[full_name] = repository + return repos[full_name] - fake_github_provider.github_app.repository = fake_repository_func - fake_github_provider.github_cloner_app.repository = fake_repository_func + fake_github_provider.github_app.get_repo = fake_get_repo + fake_github_provider.github_cloner_app.get_repo = fake_get_repo @staticmethod def _make_rebase_args(source, dest, rebase, tmpdir): @@ -1021,15 +1027,8 @@ def test_pr_body_on_create( mocked_is_pr_available.return_value = None, False mocked_push_rebase_branch.return_value = True - captured_pr_data = {} - - def fake_post(_url, data, json=True): - captured_pr_data.update(data) - response = MagicMock() - response.json.return_value = {"html_url": "https://github.com/dest/dest/pull/1"} - return response - - fake_github_provider.github_app._post = fake_post + dest_repo = fake_github_provider.github_app.get_repo(dest.full_name) + dest_repo.create_pull.return_value = MagicMock(html_url="https://github.com/dest/dest/pull/1") prow_job = ProwJobContext( job_name="periodic-openshift-release-rebasebot", @@ -1042,7 +1041,7 @@ def fake_post(_url, data, json=True): result = cli.rebasebot_run(args, slack_webhook=None, github_app_wrapper=fake_github_provider) assert result - body = captured_pr_data["body"] + body = dest_repo.create_pull.call_args.kwargs["body"] assert body assert "automated rebase PR generated by RebaseBot" in body assert f"{source.url}:{source.branch}" in body @@ -1069,7 +1068,7 @@ def test_pr_body_on_update( pull_req = MagicMock() pull_req.title = f"Merge {source.url}:{source.branch} (abcdefg) into {dest.branch}" - pull_req.update.return_value = True + pull_req.edit.return_value = True pull_req.html_url = "https://github.com/dest/dest/pull/1" mocked_is_pr_available.return_value = pull_req, True @@ -1084,7 +1083,7 @@ def test_pr_body_on_update( result = cli.rebasebot_run(args, slack_webhook=None, github_app_wrapper=fake_github_provider) assert result - body_updates = [call for call in pull_req.update.call_args_list if "body" in call.kwargs] + body_updates = [call for call in pull_req.edit.call_args_list if "body" in call.kwargs] assert len(body_updates) == 1 body = body_updates[0].kwargs["body"] assert body @@ -1115,7 +1114,7 @@ def test_pr_body_untouched_on_quiet_run( pull_req = MagicMock() pull_req.title = f"Merge {source.url}:{source.branch} (abcdefg) into {dest.branch}" - pull_req.update.return_value = True + pull_req.edit.return_value = True pull_req.html_url = "https://github.com/dest/dest/pull/1" mocked_is_pr_available.return_value = pull_req, True @@ -1130,7 +1129,7 @@ def test_pr_body_untouched_on_quiet_run( result = cli.rebasebot_run(args, slack_webhook=None, github_app_wrapper=fake_github_provider) assert result - body_updates = [call for call in pull_req.update.call_args_list if "body" in call.kwargs] + body_updates = [call for call in pull_req.edit.call_args_list if "body" in call.kwargs] assert not body_updates @patch("rebasebot.bot._push_rebase_branch") @@ -1160,7 +1159,7 @@ def test_pr_body_refreshed_on_hooks_only_run( pull_req = MagicMock() pull_req.title = f"Merge {source.url}:{source.branch} (abcdefg) into {dest.branch}" - pull_req.update.return_value = True + pull_req.edit.return_value = True pull_req.html_url = "https://github.com/dest/dest/pull/1" mocked_is_pr_available.return_value = pull_req, True @@ -1177,7 +1176,7 @@ def test_pr_body_refreshed_on_hooks_only_run( result = cli.rebasebot_run(args, slack_webhook=None, github_app_wrapper=fake_github_provider) assert result - body_updates = [call for call in pull_req.update.call_args_list if "body" in call.kwargs] + body_updates = [call for call in pull_req.edit.call_args_list if "body" in call.kwargs] assert len(body_updates) == 1 body = body_updates[0].kwargs["body"] assert "0 new upstream commits" in body diff --git a/uv.lock b/uv.lock index d06ad42..c76eee6 100644 --- a/uv.lock +++ b/uv.lock @@ -354,21 +354,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/a0/61/5c78b91c3143ed5c14207f463aecfc8f9dbb5092fb2869baf37c273b2705/gitdb-4.0.12-py3-none-any.whl", hash = "sha256:67073e15955400952c6565cc3e707c554a4eea2e428946f7a4c162fab9bd9bcf", size = 62794, upload-time = "2025-01-02T07:20:43.624Z" }, ] -[[package]] -name = "github3-py" -version = "4.0.1" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "pyjwt", extra = ["crypto"] }, - { name = "python-dateutil" }, - { name = "requests" }, - { name = "uritemplate" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/89/91/603bcaf8cd1b3927de64bf56c3a8915f6653ea7281919140c5bcff2bfe7b/github3.py-4.0.1.tar.gz", hash = "sha256:30d571076753efc389edc7f9aaef338a4fcb24b54d8968d5f39b1342f45ddd36", size = 36214038, upload-time = "2023-04-26T17:56:37.677Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/61/ad/2394d4fb542574678b0ba342daf734d4d811768da3c2ee0c84d509dcb26c/github3.py-4.0.1-py3-none-any.whl", hash = "sha256:a89af7de25650612d1da2f0609622bcdeb07ee8a45a1c06b2d16a05e4234e753", size = 151800, upload-time = "2023-04-26T17:56:25.015Z" }, -] - [[package]] name = "gitpython" version = "3.1.46" @@ -426,6 +411,22 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/0c/c3/44f3fbbfa403ea2a7c779186dc20772604442dde72947e7d01069cbe98e3/pycparser-3.0-py3-none-any.whl", hash = "sha256:b727414169a36b7d524c1c3e31839a521725078d7b2ff038656844266160a992", size = 48172, upload-time = "2026-01-21T14:26:50.693Z" }, ] +[[package]] +name = "pygithub" +version = "2.9.1" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pyjwt", extra = ["crypto"] }, + { name = "pynacl" }, + { name = "requests" }, + { name = "typing-extensions" }, + { name = "urllib3" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/ab/c3/8465a311197e16cf5ab68789fe689535e90f6b61ab524cc32a39e67237ae/pygithub-2.9.1.tar.gz", hash = "sha256:59771d7ff63d54d427be2e7d0dad2208dfffc2b0a045fec959263787739b611c", size = 2594989, upload-time = "2026-04-14T07:26:13.622Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/77/aa/81a5506f089a26338bff17535e4339b3b22049ebd1bcdeff756c4d7a7559/pygithub-2.9.1-py3-none-any.whl", hash = "sha256:2ec78fca30092d51a42d76f4ddb02131b6f0c666a35dfdf364cf302cdda115b9", size = 449710, upload-time = "2026-04-14T07:26:12.382Z" }, +] + [[package]] name = "pygments" version = "2.20.0" @@ -449,6 +450,41 @@ crypto = [ { name = "cryptography" }, ] +[[package]] +name = "pynacl" +version = "1.6.2" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "cffi", marker = "platform_python_implementation != 'PyPy'" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/d9/9a/4019b524b03a13438637b11538c82781a5eda427394380381af8f04f467a/pynacl-1.6.2.tar.gz", hash = "sha256:018494d6d696ae03c7e656e5e74cdfd8ea1326962cc401bcf018f1ed8436811c", size = 3511692, upload-time = "2026-01-01T17:48:10.851Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/4b/79/0e3c34dc3c4671f67d251c07aa8eb100916f250ee470df230b0ab89551b4/pynacl-1.6.2-cp314-cp314t-macosx_10_10_universal2.whl", hash = "sha256:622d7b07cc5c02c666795792931b50c91f3ce3c2649762efb1ef0d5684c81594", size = 390064, upload-time = "2026-01-01T17:31:57.264Z" }, + { url = "https://files.pythonhosted.org/packages/eb/1c/23a26e931736e13b16483795c8a6b2f641bf6a3d5238c22b070a5112722c/pynacl-1.6.2-cp314-cp314t-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:d071c6a9a4c94d79eb665db4ce5cedc537faf74f2355e4d502591d850d3913c0", size = 809370, upload-time = "2026-01-01T17:31:59.198Z" }, + { url = "https://files.pythonhosted.org/packages/87/74/8d4b718f8a22aea9e8dcc8b95deb76d4aae380e2f5b570cc70b5fd0a852d/pynacl-1.6.2-cp314-cp314t-manylinux2014_x86_64.manylinux_2_17_x86_64.whl", hash = "sha256:fe9847ca47d287af41e82be1dd5e23023d3c31a951da134121ab02e42ac218c9", size = 1408304, upload-time = "2026-01-01T17:32:01.162Z" }, + { url = "https://files.pythonhosted.org/packages/fd/73/be4fdd3a6a87fe8a4553380c2b47fbd1f7f58292eb820902f5c8ac7de7b0/pynacl-1.6.2-cp314-cp314t-manylinux_2_26_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:04316d1fc625d860b6c162fff704eb8426b1a8bcd3abacea11142cbd99a6b574", size = 844871, upload-time = "2026-01-01T17:32:02.824Z" }, + { url = "https://files.pythonhosted.org/packages/55/ad/6efc57ab75ee4422e96b5f2697d51bbcf6cdcc091e66310df91fbdc144a8/pynacl-1.6.2-cp314-cp314t-manylinux_2_26_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:44081faff368d6c5553ccf55322ef2819abb40e25afaec7e740f159f74813634", size = 1446356, upload-time = "2026-01-01T17:32:04.452Z" }, + { url = "https://files.pythonhosted.org/packages/78/b7/928ee9c4779caa0a915844311ab9fb5f99585621c5d6e4574538a17dca07/pynacl-1.6.2-cp314-cp314t-manylinux_2_34_aarch64.whl", hash = "sha256:a9f9932d8d2811ce1a8ffa79dcbdf3970e7355b5c8eb0c1a881a57e7f7d96e88", size = 826814, upload-time = "2026-01-01T17:32:06.078Z" }, + { url = "https://files.pythonhosted.org/packages/f7/a9/1bdba746a2be20f8809fee75c10e3159d75864ef69c6b0dd168fc60e485d/pynacl-1.6.2-cp314-cp314t-manylinux_2_34_x86_64.whl", hash = "sha256:bc4a36b28dd72fb4845e5d8f9760610588a96d5a51f01d84d8c6ff9849968c14", size = 1411742, upload-time = "2026-01-01T17:32:07.651Z" }, + { url = "https://files.pythonhosted.org/packages/f3/2f/5e7ea8d85f9f3ea5b6b87db1d8388daa3587eed181bdeb0306816fdbbe79/pynacl-1.6.2-cp314-cp314t-musllinux_1_2_aarch64.whl", hash = "sha256:3bffb6d0f6becacb6526f8f42adfb5efb26337056ee0831fb9a7044d1a964444", size = 801714, upload-time = "2026-01-01T17:32:09.558Z" }, + { url = "https://files.pythonhosted.org/packages/06/ea/43fe2f7eab5f200e40fb10d305bf6f87ea31b3bbc83443eac37cd34a9e1e/pynacl-1.6.2-cp314-cp314t-musllinux_1_2_x86_64.whl", hash = "sha256:2fef529ef3ee487ad8113d287a593fa26f48ee3620d92ecc6f1d09ea38e0709b", size = 1372257, upload-time = "2026-01-01T17:32:11.026Z" }, + { url = "https://files.pythonhosted.org/packages/4d/54/c9ea116412788629b1347e415f72195c25eb2f3809b2d3e7b25f5c79f13a/pynacl-1.6.2-cp314-cp314t-win32.whl", hash = "sha256:a84bf1c20339d06dc0c85d9aea9637a24f718f375d861b2668b2f9f96fa51145", size = 231319, upload-time = "2026-01-01T17:32:12.46Z" }, + { url = "https://files.pythonhosted.org/packages/ce/04/64e9d76646abac2dccf904fccba352a86e7d172647557f35b9fe2a5ee4a1/pynacl-1.6.2-cp314-cp314t-win_amd64.whl", hash = "sha256:320ef68a41c87547c91a8b58903c9caa641ab01e8512ce291085b5fe2fcb7590", size = 244044, upload-time = "2026-01-01T17:32:13.781Z" }, + { url = "https://files.pythonhosted.org/packages/33/33/7873dc161c6a06f43cda13dec67b6fe152cb2f982581151956fa5e5cdb47/pynacl-1.6.2-cp314-cp314t-win_arm64.whl", hash = "sha256:d29bfe37e20e015a7d8b23cfc8bd6aa7909c92a1b8f41ee416bbb3e79ef182b2", size = 188740, upload-time = "2026-01-01T17:32:15.083Z" }, + { url = "https://files.pythonhosted.org/packages/be/7b/4845bbf88e94586ec47a432da4e9107e3fc3ce37eb412b1398630a37f7dd/pynacl-1.6.2-cp38-abi3-macosx_10_10_universal2.whl", hash = "sha256:c949ea47e4206af7c8f604b8278093b674f7c79ed0d4719cc836902bf4517465", size = 388458, upload-time = "2026-01-01T17:32:16.829Z" }, + { url = "https://files.pythonhosted.org/packages/1e/b4/e927e0653ba63b02a4ca5b4d852a8d1d678afbf69b3dbf9c4d0785ac905c/pynacl-1.6.2-cp38-abi3-manylinux2014_aarch64.manylinux_2_17_aarch64.whl", hash = "sha256:8845c0631c0be43abdd865511c41eab235e0be69c81dc66a50911594198679b0", size = 800020, upload-time = "2026-01-01T17:32:18.34Z" }, + { url = "https://files.pythonhosted.org/packages/7f/81/d60984052df5c97b1d24365bc1e30024379b42c4edcd79d2436b1b9806f2/pynacl-1.6.2-cp38-abi3-manylinux2014_x86_64.manylinux_2_17_x86_64.whl", hash = "sha256:22de65bb9010a725b0dac248f353bb072969c94fa8d6b1f34b87d7953cf7bbe4", size = 1399174, upload-time = "2026-01-01T17:32:20.239Z" }, + { url = "https://files.pythonhosted.org/packages/68/f7/322f2f9915c4ef27d140101dd0ed26b479f7e6f5f183590fd32dfc48c4d3/pynacl-1.6.2-cp38-abi3-manylinux_2_26_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:46065496ab748469cdd999246d17e301b2c24ae2fdf739132e580a0e94c94a87", size = 835085, upload-time = "2026-01-01T17:32:22.24Z" }, + { url = "https://files.pythonhosted.org/packages/3e/d0/f301f83ac8dbe53442c5a43f6a39016f94f754d7a9815a875b65e218a307/pynacl-1.6.2-cp38-abi3-manylinux_2_26_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:8a66d6fb6ae7661c58995f9c6435bda2b1e68b54b598a6a10247bfcdadac996c", size = 1437614, upload-time = "2026-01-01T17:32:23.766Z" }, + { url = "https://files.pythonhosted.org/packages/c4/58/fc6e649762b029315325ace1a8c6be66125e42f67416d3dbd47b69563d61/pynacl-1.6.2-cp38-abi3-manylinux_2_34_aarch64.whl", hash = "sha256:26bfcd00dcf2cf160f122186af731ae30ab120c18e8375684ec2670dccd28130", size = 818251, upload-time = "2026-01-01T17:32:25.69Z" }, + { url = "https://files.pythonhosted.org/packages/c9/a8/b917096b1accc9acd878819a49d3d84875731a41eb665f6ebc826b1af99e/pynacl-1.6.2-cp38-abi3-manylinux_2_34_x86_64.whl", hash = "sha256:c8a231e36ec2cab018c4ad4358c386e36eede0319a0c41fed24f840b1dac59f6", size = 1402859, upload-time = "2026-01-01T17:32:27.215Z" }, + { url = "https://files.pythonhosted.org/packages/85/42/fe60b5f4473e12c72f977548e4028156f4d340b884c635ec6b063fe7e9a5/pynacl-1.6.2-cp38-abi3-musllinux_1_2_aarch64.whl", hash = "sha256:68be3a09455743ff9505491220b64440ced8973fe930f270c8e07ccfa25b1f9e", size = 791926, upload-time = "2026-01-01T17:32:29.314Z" }, + { url = "https://files.pythonhosted.org/packages/fa/f9/e40e318c604259301cc091a2a63f237d9e7b424c4851cafaea4ea7c4834e/pynacl-1.6.2-cp38-abi3-musllinux_1_2_x86_64.whl", hash = "sha256:8b097553b380236d51ed11356c953bf8ce36a29a3e596e934ecabe76c985a577", size = 1363101, upload-time = "2026-01-01T17:32:31.263Z" }, + { url = "https://files.pythonhosted.org/packages/48/47/e761c254f410c023a469284a9bc210933e18588ca87706ae93002c05114c/pynacl-1.6.2-cp38-abi3-win32.whl", hash = "sha256:5811c72b473b2f38f7e2a3dc4f8642e3a3e9b5e7317266e4ced1fba85cae41aa", size = 227421, upload-time = "2026-01-01T17:32:33.076Z" }, + { url = "https://files.pythonhosted.org/packages/41/ad/334600e8cacc7d86587fe5f565480fde569dfb487389c8e1be56ac21d8ac/pynacl-1.6.2-cp38-abi3-win_amd64.whl", hash = "sha256:62985f233210dee6548c223301b6c25440852e13d59a8b81490203c3227c5ba0", size = 239754, upload-time = "2026-01-01T17:32:34.557Z" }, + { url = "https://files.pythonhosted.org/packages/29/7d/5945b5af29534641820d3bd7b00962abbbdfee84ec7e19f0d5b3175f9a31/pynacl-1.6.2-cp38-abi3-win_arm64.whl", hash = "sha256:834a43af110f743a754448463e8fd61259cd4ab5bbedcf70f9dabad1d28a394c", size = 184801, upload-time = "2026-01-01T17:32:36.309Z" }, +] + [[package]] name = "pytest" version = "9.0.3" @@ -479,25 +515,13 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/9d/7a/d968e294073affff457b041c2be9868a40c1c71f4a35fcc1e45e5493067b/pytest_cov-7.1.0-py3-none-any.whl", hash = "sha256:a0461110b7865f9a271aa1b51e516c9a95de9d696734a2f71e3e78f46e1d4678", size = 22876, upload-time = "2026-03-21T20:11:14.438Z" }, ] -[[package]] -name = "python-dateutil" -version = "2.9.0.post0" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "six" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/66/c0/0c8b6ad9f17a802ee498c46e004a0eb49bc148f2fd230864601a86dcf6db/python-dateutil-2.9.0.post0.tar.gz", hash = "sha256:37dd54208da7e1cd875388217d5e00ebd4179249f90fb72437e91a35459a0ad3", size = 342432, upload-time = "2024-03-01T18:36:20.211Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/ec/57/56b9bcc3c9c6a792fcbaf139543cee77261f3651ca9da0c93f5c1221264b/python_dateutil-2.9.0.post0-py2.py3-none-any.whl", hash = "sha256:a8b2bc7bffae282281c8140a97d3aa9c14da0b136dfe83f850eea9a5f7470427", size = 229892, upload-time = "2024-03-01T18:36:18.57Z" }, -] - [[package]] name = "rebasebot" version = "0.0.1" source = { editable = "." } dependencies = [ - { name = "github3-py" }, { name = "gitpython" }, + { name = "pygithub" }, { name = "requests" }, ] @@ -510,8 +534,8 @@ dev = [ [package.metadata] requires-dist = [ - { name = "github3-py", specifier = "==4.0.1" }, { name = "gitpython", specifier = "==3.1.46" }, + { name = "pygithub", specifier = "==2.9.1" }, { name = "pytest", marker = "extra == 'dev'", specifier = "==9.0.3" }, { name = "pytest-cov", marker = "extra == 'dev'", specifier = "==7.1.0" }, { name = "requests", specifier = "==2.33.1" }, @@ -559,15 +583,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/58/ed/dea90a65b7d9e69888890fb14c90d7f51bf0c1e82ad800aeb0160e4bacfd/ruff-0.15.10-py3-none-win_arm64.whl", hash = "sha256:601d1610a9e1f1c2165a4f561eeaa2e2ea1e97f3287c5aa258d3dab8b57c6188", size = 11035607, upload-time = "2026-04-09T14:05:47.593Z" }, ] -[[package]] -name = "six" -version = "1.17.0" -source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/94/e7/b2c673351809dca68a0e064b6af791aa332cf192da575fd474ed7d6f16a2/six-1.17.0.tar.gz", hash = "sha256:ff70335d468e7eb6ec65b95b99d3a2836546063f63acc5171de367e834932a81", size = 34031, upload-time = "2024-12-04T17:35:28.174Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/b7/ce/149a00dd41f10bc29e5921b496af8b574d8413afcd5e30dfa0ed46c2cc5e/six-1.17.0-py2.py3-none-any.whl", hash = "sha256:4721f391ed90541fddacab5acf947aa0d3dc7d27b2e1e8eda2be8970586c3274", size = 11050, upload-time = "2024-12-04T17:35:26.475Z" }, -] - [[package]] name = "smmap" version = "5.0.3" @@ -632,12 +647,12 @@ wheels = [ ] [[package]] -name = "uritemplate" -version = "4.2.0" +name = "typing-extensions" +version = "4.15.0" source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/98/60/f174043244c5306c9988380d2cb10009f91563fc4b31293d27e17201af56/uritemplate-4.2.0.tar.gz", hash = "sha256:480c2ed180878955863323eea31b0ede668795de182617fef9c6ca09e6ec9d0e", size = 33267, upload-time = "2025-06-02T15:12:06.318Z" } +sdist = { url = "https://files.pythonhosted.org/packages/72/94/1a15dd82efb362ac84269196e94cf00f187f7ed21c242792a923cdb1c61f/typing_extensions-4.15.0.tar.gz", hash = "sha256:0cea48d173cc12fa28ecabc3b837ea3cf6f38c6d1136f85cbaaf598984861466", size = 109391, upload-time = "2025-08-25T13:49:26.313Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/a9/99/3ae339466c9183ea5b8ae87b34c0b897eda475d2aec2307cae60e5cd4f29/uritemplate-4.2.0-py3-none-any.whl", hash = "sha256:962201ba1c4edcab02e60f9a0d3821e82dfc5d2d6662a21abd533879bdb8a686", size = 11488, upload-time = "2025-06-02T15:12:03.405Z" }, + { url = "https://files.pythonhosted.org/packages/18/67/36e9267722cc04a6b9f15c7f3441c2363321a3ea07da7ae0c0707beb2a9c/typing_extensions-4.15.0-py3-none-any.whl", hash = "sha256:f0fa19c6845758ab08074a0cfa8b7aecb71c999ca73d62883bc25cc018c4e548", size = 44614, upload-time = "2025-08-25T13:49:24.86Z" }, ] [[package]] From 79557e05e984a54534ca02ea36902afcacc40e63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radek=20Ma=C5=88=C3=A1k?= Date: Fri, 3 Jul 2026 13:02:28 +0200 Subject: [PATCH 2/2] Tighten GITHUB_BRANCH_PATTERN to reject malformed refs Repository names could previously contain a slash and branches could be empty, which produced bogus GitHubBranch values instead of a clear parse error. Addresses CodeRabbit review comment on PR #71. --- rebasebot/github.py | 2 +- tests/test_cli.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/rebasebot/github.py b/rebasebot/github.py index 8a3c726..4e2588e 100644 --- a/rebasebot/github.py +++ b/rebasebot/github.py @@ -22,7 +22,7 @@ from github import Auth, Github, GithubIntegration, UnknownObjectException logger = logging.getLogger() -GITHUB_BRANCH_PATTERN = re.compile(r"^(?P[^/]+)/(?P[^:]+):(?P.*)$") +GITHUB_BRANCH_PATTERN = re.compile(r"^(?P[^/:]+)/(?P[^/:]+):(?P[^:]+)$") @dataclass diff --git a/tests/test_cli.py b/tests/test_cli.py index 562acf2..e04aa89 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -101,6 +101,8 @@ def test_github_branch_parse_valid(self, get_valid_cli_args, arg, github_ref, ex "/kubernetes/autoscaler:master", "fooo", "asdasdasdqwe/asdasd\\asdsadasd", + "kubernetes/auto/scaler:master", + "kubernetes/autoscaler:", ), ) @pytest.mark.parametrize("arg", ["source", "dest", "rebase"])