Set remote url on every getRepo in merger
In case we run on github with apps we have an access token in the git
url. This access token is only valid for a specific period of
time. Currently the merger caches its repos and never changes the git
url on subsequent merge or cat requests. After timeout the merger then
faces an exception [1].
The fix is to update the remote url on every getRepo call in the
merger. This makes sure that we have a recent access token on every
merge or cat call.
Further the executor has the same problems and gets fixed by this too
as it also uses getRepo from the merger.
[1] Exception:
2017-12-20 07:00:00,874 ERROR zuul.Merger: Unable to update github/sandbox/sandbox
Traceback (most recent call last):
File "/usr/lib/python3.6/site-packages/zuul/merger/merger.py", line 382, in updateRepo
repo.reset()
File "/usr/lib/python3.6/site-packages/zuul/merger/merger.py", line 143, in reset
self.update()
File "/usr/lib/python3.6/site-packages/zuul/merger/merger.py", line 293, in update
self._git_fetch(repo, 'origin', tags=True)
File "/usr/lib/python3.6/site-packages/zuul/merger/merger.py", line 133, in _git_fetch
**kwargs)
File "/usr/lib/python3.6/site-packages/git/cmd.py", line 551, in <lambda>
return lambda *args, **kwargs: self._call_process(name, *args, **kwargs)
File "/usr/lib/python3.6/site-packages/git/cmd.py", line 1010, in _call_process
return self.execute(call, **exec_kwargs)
File "/usr/lib/python3.6/site-packages/git/cmd.py", line 821, in execute
raise GitCommandError(command, status, stderr_value, stdout_value)
git.exc.GitCommandError: Cmd('git') failed due to: exit code(128)
cmdline: git fetch --tags origin
stderr: 'remote: Invalid username or password.
fatal: Authentication failed for 'https://x-access-token:v1.c8ec09e233b16871b64843adeec5fb048a383fba@github.example.com/sandbox/sandbox/''
Change-Id: If990dc48e6c10c24b6b32db5f5711fc3608bdfe4
diff --git a/tests/base.py b/tests/base.py
index f68f59a..c13519e 100755
--- a/tests/base.py
+++ b/tests/base.py
@@ -941,7 +941,7 @@
log = logging.getLogger("zuul.test.FakeGithubConnection")
def __init__(self, driver, connection_name, connection_config, rpcclient,
- changes_db=None, upstream_root=None):
+ changes_db=None, upstream_root=None, git_url_with_auth=False):
super(FakeGithubConnection, self).__init__(driver, connection_name,
connection_config)
self.connection_name = connection_name
@@ -953,6 +953,7 @@
self.merge_not_allowed_count = 0
self.reports = []
self.github_client = tests.fakegithub.FakeGithub(changes_db)
+ self.git_url_with_auth = git_url_with_auth
self.rpcclient = rpcclient
def getGithubClient(self,
@@ -1045,7 +1046,13 @@
return 'read'
def getGitUrl(self, project):
- return os.path.join(self.upstream_root, str(project))
+ if self.git_url_with_auth:
+ auth_token = ''.join(
+ random.choice(string.ascii_lowercase) for x in range(8))
+ prefix = 'file://x-access-token:%s@' % auth_token
+ else:
+ prefix = ''
+ return prefix + os.path.join(self.upstream_root, str(project))
def real_getGitUrl(self, project):
return super(FakeGithubConnection, self).getGitUrl(project)
@@ -1901,6 +1908,7 @@
run_ansible = False
create_project_keys = False
use_ssl = False
+ git_url_with_auth = False
def _startMerger(self):
self.merge_server = zuul.merger.server.MergeServer(self.config,
@@ -2070,10 +2078,12 @@
def getGithubConnection(driver, name, config):
server = config.get('server', 'github.com')
db = self.github_changes_dbs.setdefault(server, {})
- con = FakeGithubConnection(driver, name, config,
- self.rpcclient,
- changes_db=db,
- upstream_root=self.upstream_root)
+ con = FakeGithubConnection(
+ driver, name, config,
+ self.rpcclient,
+ changes_db=db,
+ upstream_root=self.upstream_root,
+ git_url_with_auth=self.git_url_with_auth)
self.event_queues.append(con.event_queue)
setattr(self, 'fake_' + name, con)
return con
diff --git a/tests/unit/test_merger_repo.py b/tests/unit/test_merger_repo.py
index fb2f199..984644f 100644
--- a/tests/unit/test_merger_repo.py
+++ b/tests/unit/test_merger_repo.py
@@ -22,7 +22,7 @@
import testtools
from zuul.merger.merger import Repo
-from tests.base import ZuulTestCase, FIXTURE_DIR
+from tests.base import ZuulTestCase, FIXTURE_DIR, simple_layout
class TestMergerRepo(ZuulTestCase):
@@ -116,3 +116,63 @@
# This is created on the second fetch
self.assertTrue(os.path.exists(os.path.join(
self.workspace_root, 'stamp2')))
+
+
+class TestMergerWithAuthUrl(ZuulTestCase):
+ config_file = 'zuul-github-driver.conf'
+
+ git_url_with_auth = True
+
+ @simple_layout('layouts/merging-github.yaml', driver='github')
+ def test_changing_url(self):
+ """
+ This test checks that if getGitUrl returns different urls for the same
+ repo (which happens if an access token is part of the url) then the
+ remote urls are changed in the merger accordingly. This tests directly
+ the merger.
+ """
+
+ merger = self.executor_server.merger
+ repo = merger.getRepo('github', 'org/project')
+ first_url = repo.remote_url
+
+ repo = merger.getRepo('github', 'org/project')
+ second_url = repo.remote_url
+
+ # the urls should differ
+ self.assertNotEqual(first_url, second_url)
+
+ @simple_layout('layouts/merging-github.yaml', driver='github')
+ def test_changing_url_end_to_end(self):
+ """
+ This test checks that if getGitUrl returns different urls for the same
+ repo (which happens if an access token is part of the url) then the
+ remote urls are changed in the merger accordingly. This is an end to
+ end test.
+ """
+
+ A = self.fake_github.openFakePullRequest('org/project', 'master',
+ 'PR title')
+ self.fake_github.emitEvent(A.getCommentAddedEvent('merge me'))
+ self.waitUntilSettled()
+ self.assertTrue(A.is_merged)
+
+ # get remote url of org/project in merger
+ repo = self.executor_server.merger.repos.get('github.com/org/project')
+ self.assertIsNotNone(repo)
+ git_repo = git.Repo(repo.local_path)
+ first_url = list(git_repo.remotes[0].urls)[0]
+
+ B = self.fake_github.openFakePullRequest('org/project', 'master',
+ 'PR title')
+ self.fake_github.emitEvent(B.getCommentAddedEvent('merge me again'))
+ self.waitUntilSettled()
+ self.assertTrue(B.is_merged)
+
+ repo = self.executor_server.merger.repos.get('github.com/org/project')
+ self.assertIsNotNone(repo)
+ git_repo = git.Repo(repo.local_path)
+ second_url = list(git_repo.remotes[0].urls)[0]
+
+ # the urls should differ
+ self.assertNotEqual(first_url, second_url)
diff --git a/zuul/merger/merger.py b/zuul/merger/merger.py
index 07f3e69..63bb5d5 100644
--- a/zuul/merger/merger.py
+++ b/zuul/merger/merger.py
@@ -79,6 +79,8 @@
self.retry_interval = retry_interval
try:
self._ensure_cloned()
+ self._git_set_remote_url(
+ git.Repo(self.local_path), self.remote_url)
except Exception:
self.log.exception("Unable to initialize repo for %s" % remote)
@@ -112,8 +114,7 @@
config_writer.set_value('user', 'name', self.username)
config_writer.write()
if rewrite_url:
- with repo.remotes.origin.config_writer as config_writer:
- config_writer.set('url', self.remote_url)
+ self._git_set_remote_url(repo, self.remote_url)
self._initialized = True
def isInitialized(self):
@@ -154,6 +155,10 @@
else:
raise
+ def _git_set_remote_url(self, repo, url):
+ with repo.remotes.origin.config_writer as config_writer:
+ config_writer.set('url', url)
+
def createRepoObject(self):
self._ensure_cloned()
repo = git.Repo(self.local_path)
@@ -358,6 +363,13 @@
repo = self.createRepoObject()
repo.delete_remote(repo.remotes[remote])
+ def setRemoteUrl(self, url):
+ if self.remote_url == url:
+ return
+ self.log.debug("Set remote url to %s" % url)
+ self.remote_url = url
+ self._git_set_remote_url(self.createRepoObject(), self.remote_url)
+
class Merger(object):
def __init__(self, working_root, connections, email, username,
@@ -405,7 +417,9 @@
url = source.getGitUrl(project)
key = '/'.join([hostname, project_name])
if key in self.repos:
- return self.repos[key]
+ repo = self.repos[key]
+ repo.setRemoteUrl(url)
+ return repo
sshkey = self.connections.connections.get(connection_name).\
connection_config.get('sshkey')
if not url: