diff --git a/tests/base.py b/tests/base.py index 0f2df353fc..ec94c48a20 100755 --- a/tests/base.py +++ b/tests/base.py @@ -941,7 +941,7 @@ class FakeGithubConnection(githubconnection.GithubConnection): 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 @@ class FakeGithubConnection(githubconnection.GithubConnection): 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 @@ class FakeGithubConnection(githubconnection.GithubConnection): 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) @@ -1907,6 +1914,7 @@ class ZuulTestCase(BaseTestCase): 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, @@ -2076,10 +2084,12 @@ class ZuulTestCase(BaseTestCase): 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 fb2f1999ea..984644fc56 100644 --- a/tests/unit/test_merger_repo.py +++ b/tests/unit/test_merger_repo.py @@ -22,7 +22,7 @@ import git 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 @@ class TestMergerRepo(ZuulTestCase): # 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 5e102b450e..aba8645c0a 100644 --- a/zuul/merger/merger.py +++ b/zuul/merger/merger.py @@ -79,6 +79,8 @@ class Repo(object): 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 @@ class Repo(object): 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 @@ class Repo(object): 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) @@ -350,6 +355,13 @@ class Repo(object): 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, @@ -397,7 +409,9 @@ class Merger(object): 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: