Use correct token for git pushes

After plenty of investigation done I was able to find that the token I
need for dist git pushes is OIDC token generated on ipsilon. I was able
to make it working on small reproducer. So I'm just implementing the
same changes here.

This also removes silent failures during git push, this will be caught
by the catch in main method and will let user know that there is some
issue in the code instead of silently failing.
This commit is contained in:
Michal Konecny 2025-05-21 15:48:50 +02:00
parent b0156c969e
commit af4209b88e
5 changed files with 41 additions and 67 deletions

View file

@ -1559,7 +1559,7 @@ class TestProcessNewRepo:
self.toddler.dist_git.get_project.return_value = None self.toddler.dist_git.get_project.return_value = None
self.toddler.dist_git._pagure_url = dist_git_url self.toddler.dist_git._pagure_url = dist_git_url
self.toddler.dist_git.has_dead_package_on_branch.return_value = None self.toddler.dist_git.has_dead_package_on_branch.return_value = None
self.toddler.dist_git_token = "token" self.toddler.oidc_distgit_token = "token"
mock_dir = MagicMock() mock_dir = MagicMock()
mock_dir.__enter__.return_value = "dir" mock_dir.__enter__.return_value = "dir"
@ -1588,7 +1588,9 @@ class TestProcessNewRepo:
) )
mock_git_repo.retire_branch.assert_called_once_with( mock_git_repo.retire_branch.assert_called_once_with(
"EPEL only package", self.toddler.pagure_user, self.toddler.dist_git_token "EPEL only package",
self.toddler.pagure_user,
self.toddler.oidc_distgit_token,
) )
message = "The Pagure repository was created at {0}/{1}/{2}".format( message = "The Pagure repository was created at {0}/{1}/{2}".format(
@ -1718,18 +1720,19 @@ class TestProcessNewRepo:
self.toddler.dist_git.get_project.return_value = None self.toddler.dist_git.get_project.return_value = None
self.toddler.dist_git._pagure_url = dist_git_url self.toddler.dist_git._pagure_url = dist_git_url
self.toddler.dist_git.has_dead_package_on_branch.return_value = None self.toddler.dist_git.has_dead_package_on_branch.return_value = None
self.toddler.dist_git_token = "token" self.toddler.oidc_distgit_token = "token"
mock_dir = MagicMock() mock_dir = MagicMock()
mock_dir.__enter__.return_value = "dir" mock_dir.__enter__.return_value = "dir"
mock_temp_dir.return_value = mock_dir mock_temp_dir.return_value = mock_dir
mock_git_repo = MagicMock() mock_git_repo = MagicMock()
mock_git_repo.retire_branch.return_value = False mock_git_repo.retire_branch.side_effect = Exception("Error")
mock_git.clone_repo.return_value = mock_git_repo mock_git.clone_repo.return_value = mock_git_repo
# test # test
self.toddler.process_new_repo(issue, json) with pytest.raises(Exception):
self.toddler.process_new_repo(issue, json)
# asserts # asserts
mock_validate_review_bug.assert_called_with( mock_validate_review_bug.assert_called_with(
@ -1748,14 +1751,9 @@ class TestProcessNewRepo:
) )
mock_git_repo.retire_branch.assert_called_once_with( mock_git_repo.retire_branch.assert_called_once_with(
"EPEL only package", self.toddler.pagure_user, self.toddler.dist_git_token "EPEL only package",
) self.toddler.pagure_user,
self.toddler.oidc_distgit_token,
self.toddler.pagure_io.close_issue.assert_called_with(
100,
namespace=scm_request_processor.PROJECT_NAMESPACE,
message="Failure during retirement of rawhide branch for EPEL only package.",
reason="Invalid",
) )

View file

@ -4,6 +4,8 @@ Unit tests for `toddlers.utils.git`.
from unittest.mock import call, MagicMock, Mock, patch from unittest.mock import call, MagicMock, Mock, patch
import pytest
from toddlers.utils.git import clone_repo, GitRepo from toddlers.utils.git import clone_repo, GitRepo
@ -343,17 +345,14 @@ class TestGitRepoRetireBranch:
mock_commit.tree = [mock_blob] mock_commit.tree = [mock_blob]
self.repo.repo.commit.return_value = mock_commit self.repo.repo.commit.return_value = mock_commit
result = self.repo.retire_branch( self.repo.retire_branch("Retire branch", "bot", "token", "feature_branch")
"Retire branch", "bot", "token", "feature_branch"
)
assert result
self.repo.repo.git.checkout.assert_called_once_with("feature_branch") self.repo.repo.git.checkout.assert_called_once_with("feature_branch")
self.repo.repo.index.remove.assert_called_once_with("README", working_tree=True) self.repo.repo.index.remove.assert_called_once_with("README", working_tree=True)
self.repo.repo.index.add.assert_called_once_with(["dead.package"]) self.repo.repo.index.add.assert_called_once_with(["dead.package"])
self.repo.repo.index.commit.assert_called_once_with("Retire branch") self.repo.repo.index.commit.assert_called_once_with("Retire branch")
mock_git_cmd.push.assert_called_once_with( mock_git_cmd.push.assert_called_once_with(
"-u", "https://bot:token@example.com", "origin", "feature_branch" "-u", "https://bot:token@example.com", "feature_branch"
) )
def test_retire_branch_exception(self): def test_retire_branch_exception(self):
@ -361,9 +360,7 @@ class TestGitRepoRetireBranch:
self.repo.repo.remote.return_value = mock_origin self.repo.repo.remote.return_value = mock_origin
self.repo.repo.git.checkout.side_effect = Exception("Revert error") self.repo.repo.git.checkout.side_effect = Exception("Revert error")
result = self.repo.retire_branch( with pytest.raises(Exception):
"Revert message", "bot", "token", "feature_branch" self.repo.retire_branch("Revert message", "bot", "token", "feature_branch")
)
assert not result
self.repo.repo.commit.assert_not_called() self.repo.repo.commit.assert_not_called()

View file

@ -251,6 +251,8 @@ pagure_api_key = "API token for pagure"
monitor_choices = ['no-monitoring', 'monitoring', 'monitoring-with-scratch'] monitor_choices = ['no-monitoring', 'monitoring', 'monitoring-with-scratch']
# Text for the ping if the ticket needs to be manually verified # Text for the ping if the ticket needs to be manually verified
ping_comment = "This request wants to skip bugzilla validation! {maintainers} could you check if this is correct? If yes, please respond to this ticket with 'valid' comment" ping_comment = "This request wants to skip bugzilla validation! {maintainers} could you check if this is correct? If yes, please respond to this ticket with 'valid' comment"
# This is a OIDC token that allows pagure_user to push changes to dist git
oidc_distgit_token = "OIDC token used to push git changes using pagure_user"
# Pagure mapping to bugzilla # Pagure mapping to bugzilla

View file

@ -159,6 +159,7 @@ class SCMRequestProcessor(ToddlerBase):
self.ping_comment = config.get("ping_comment", "") self.ping_comment = config.get("ping_comment", "")
self.pagure_user = config.get("pagure_user", "") self.pagure_user = config.get("pagure_user", "")
self.dist_git_token = config.get("dist_git_token", "") self.dist_git_token = config.get("dist_git_token", "")
self.oidc_distgit_token = config.get("oidc_distgit_token", "")
# Check if the message isn't generated by toddler user # Check if the message isn't generated by toddler user
# Ignore the ticket if it is # Ignore the ticket if it is
@ -720,25 +721,10 @@ class SCMRequestProcessor(ToddlerBase):
self.dist_git._pagure_url.rstrip("/"), namespace, repo self.dist_git._pagure_url.rstrip("/"), namespace, repo
) )
git_repo = git.clone_repo(dist_git_url, tmp_dir) git_repo = git.clone_repo(dist_git_url, tmp_dir)
if not git_repo.retire_branch( git_repo.retire_branch(
"EPEL only package", self.pagure_user, self.dist_git_token "EPEL only package", self.pagure_user, self.oidc_distgit_token
): )
_log.info( _log.info("Rawhide branch retired for {0}.".format(repo))
"Failure during retirement of rawhide branch {0}.".format(
repo
)
)
self.pagure_io.close_issue(
issue["id"],
namespace=PROJECT_NAMESPACE,
message=(
"Failure during retirement of rawhide branch for EPEL only package."
),
reason="Invalid",
)
return
else:
_log.info("Rawhide branch retired for {0}.".format(repo))
if branch_name == default_branch: if branch_name == default_branch:
new_repo_comment = "The Pagure repository was created at {0}".format( new_repo_comment = "The Pagure repository was created at {0}".format(

View file

@ -158,7 +158,7 @@ class GitRepo:
def retire_branch( def retire_branch(
self, message: str, user: str, token: str, branch: str = "rawhide" self, message: str, user: str, token: str, branch: str = "rawhide"
) -> bool: ):
""" """
Retire branch on git repository. This will create dead.package on Retire branch on git repository. This will create dead.package on
branch and pushes the changes. branch and pushes the changes.
@ -168,34 +168,25 @@ class GitRepo:
user: User pushing the changes user: User pushing the changes
token: ACL token to use for authentication token: ACL token to use for authentication
branch: The branch to retire branch: The branch to retire
Returns:
True if retired succesfully, False otherwise.
""" """
try: # checkout to the correct branch
# checkout to the correct branch self.repo.git.checkout(branch)
self.repo.git.checkout(branch)
# remove all files first # remove all files first
for entry in self.repo.commit().tree: for entry in self.repo.commit().tree:
self.repo.index.remove(entry.name, working_tree=True) self.repo.index.remove(entry.name, working_tree=True)
file_name = os.path.join(str(self.repo.working_tree_dir), "dead.package") file_name = os.path.join(str(self.repo.working_tree_dir), "dead.package")
with open(file_name, "w") as file: with open(file_name, "w") as file:
file.write(message) file.write(message)
self.repo.index.add(["dead.package"]) self.repo.index.add(["dead.package"])
self.repo.index.commit("Retire branch") self.repo.index.commit("Retire branch")
origin = self.repo.remote("origin") origin = self.repo.remote("origin")
git_cmd = self.repo.git git_cmd = self.repo.git
push_url = origin.url.replace( push_url = origin.url.replace(
"https://", "https://{0}:{1}@".format(user, token) "https://", "https://{0}:{1}@".format(user, token)
) )
git_cmd.push("-u", push_url, "origin", branch) git_cmd.push("-u", push_url, branch)
except Exception as error:
print(error, "\nError during retirement of {0} branch".format(branch))
return False
return True