Merge "Set remote url on every getRepo in merger"

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,
+            con = FakeGithubConnection(
+                driver, name, config,
                 self.rpcclient,
                 changes_db=db,
-                                       upstream_root=self.upstream_root)
+                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

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)

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:
+            self._git_set_remote_url(repo, self.remote_url)
-                config_writer.set('url', 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: