Make the git web url a template

We have deployed our Gerrit with cgit so the old gitweb urls provided by
Zuul's gerrit connection no longer work. Add in a new config option on
Gerrit connections to specify a url template string which we can modify
to point at our cgit instance. This should in theory also support github
users too.

The default is to continue pointing at Gerrit's built in gitweb
instance.

Change-Id: I91d77e309cfeea0e90a85f926aca9b8c347b0385

6 files changed, 111 insertions, 9 deletions

diff --git a/doc/source/admin/drivers/gerrit.rst b/doc/source/admin/drivers/gerrit.rst
index ac42bd38f..935cb3290 100644
--- a/doc/source/admin/drivers/gerrit.rst
+++ b/doc/source/admin/drivers/gerrit.rst
@@ -61,6 +61,17 @@ The supported options in ``zuul.conf`` connections are:
 
       Path to Gerrit web interface.
 
+   .. attr:: gitweb_url_template
+      :default: {baseurl}/gitweb?p={project.name}.git;a=commitdiff;h={sha}
+
+      Url template for links to specific git shas. By default this will
+      point at Gerrit's built in gitweb but you can customize this value
+      to point elsewhere (like cgit or github).
+
+      The three values available for string interpolation are baseurl
+      which points back to Gerrit, project and all of its safe attributes,
+      and sha which is the git sha1.
+
    .. attr:: user
       :default: zuul
 
diff --git a/tests/fixtures/zuul-connections-cgit.conf b/tests/fixtures/zuul-connections-cgit.conf
new file mode 100644
index 000000000..39dc0bb69
--- /dev/null
+++ b/tests/fixtures/zuul-connections-cgit.conf
@@ -0,0 +1,27 @@
+[gearman]
+server=127.0.0.1
+
+[scheduler]
+tenant_config=main.yaml
+
+[merger]
+git_dir=/tmp/zuul-test/merger-git
+git_user_email=zuul@example.com
+git_user_name=zuul
+
+[executor]
+git_dir=/tmp/zuul-test/executor-git
+
+[connection gerrit]
+driver=gerrit
+server=review.example.com
+user=jenkins
+sshkey=none
+gitweb_url_template=https://cgit.example.com/cgit/{project.name}/commit/?id={sha}
+
+[connection outgoing_smtp]
+driver=smtp
+server=localhost
+port=25
+default_from=zuul@example.com
+default_to=you@example.com
diff --git a/tests/fixtures/zuul-connections-gitweb.conf b/tests/fixtures/zuul-connections-gitweb.conf
new file mode 100644
index 000000000..172208e48
--- /dev/null
+++ b/tests/fixtures/zuul-connections-gitweb.conf
@@ -0,0 +1,26 @@
+[gearman]
+server=127.0.0.1
+
+[scheduler]
+tenant_config=main.yaml
+
+[merger]
+git_dir=/tmp/zuul-test/merger-git
+git_user_email=zuul@example.com
+git_user_name=zuul
+
+[executor]
+git_dir=/tmp/zuul-test/executor-git
+
+[connection gerrit]
+driver=gerrit
+server=review.example.com
+user=jenkins
+sshkey=none
+
+[connection outgoing_smtp]
+driver=smtp
+server=localhost
+port=25
+default_from=zuul@example.com
+default_to=you@example.com
diff --git a/tests/unit/test_connection.py b/tests/unit/test_connection.py
index 719f307b4..c882d3a0a 100644
--- a/tests/unit/test_connection.py
+++ b/tests/unit/test_connection.py
@@ -338,3 +338,32 @@ class TestConnectionsMerger(ZuulTestCase):
         self.assertNotIn("sql", self.connections.connections)
         self.assertNotIn("timer", self.connections.connections)
         self.assertNotIn("zuul", self.connections.connections)
+
+
+class TestConnectionsCgit(ZuulTestCase):
+    config_file = 'zuul-connections-cgit.conf'
+    tenant_config_file = 'config/single-tenant/main.yaml'
+
+    def test_cgit_web_url(self):
+        self.assertIn("gerrit", self.connections.connections)
+        conn = self.connections.connections['gerrit']
+        source = conn.source
+        proj = source.getProject('foo/bar')
+        url = conn._getWebUrl(proj, '1')
+        self.assertEqual(url,
+                         'https://cgit.example.com/cgit/foo/bar/commit/?id=1')
+
+
+class TestConnectionsGitweb(ZuulTestCase):
+    config_file = 'zuul-connections-gitweb.conf'
+    tenant_config_file = 'config/single-tenant/main.yaml'
+
+    def test_gitweb_url(self):
+        self.assertIn("gerrit", self.connections.connections)
+        conn = self.connections.connections['gerrit']
+        source = conn.source
+        proj = source.getProject('foo/bar')
+        url = conn._getWebUrl(proj, '1')
+        url_should_be = 'https://review.example.com/' \
+                        'gitweb?p=foo/bar.git;a=commitdiff;h=1'
+        self.assertEqual(url, url_should_be)
diff --git a/zuul/driver/gerrit/gerritconnection.py b/zuul/driver/gerrit/gerritconnection.py
index c3f9ee2cd..59051bb16 100644
--- a/zuul/driver/gerrit/gerritconnection.py
+++ b/zuul/driver/gerrit/gerritconnection.py
@@ -299,6 +299,12 @@ class GerritConnection(BaseConnection):
 
         self.baseurl = self.connection_config.get('baseurl',
                                                   'https://%s' % self.server)
+        default_gitweb_url_template = '{baseurl}/gitweb?' \
+                                      'p={project.name}.git;' \
+                                      'a=commitdiff;h={sha}'
+        url_template = self.connection_config.get('gitweb_url_template',
+                                                  default_gitweb_url_template)
+        self.gitweb_url_template = url_template
 
         self._change_cache = {}
         self.projects = {}
@@ -338,7 +344,7 @@ class GerritConnection(BaseConnection):
             change.ref = event.ref
             change.oldrev = event.oldrev
             change.newrev = event.newrev
-            change.url = self._getGitwebUrl(project, sha=event.newrev)
+            change.url = self._getWebUrl(project, sha=event.newrev)
         elif event.ref and not event.ref.startswith('refs/'):
             # Pre 2.13 Gerrit ref-updated events don't have branch prefixes.
             project = self.source.getProject(event.project_name)
@@ -347,7 +353,7 @@ class GerritConnection(BaseConnection):
             change.ref = 'refs/heads/' + event.ref
             change.oldrev = event.oldrev
             change.newrev = event.newrev
-            change.url = self._getGitwebUrl(project, sha=event.newrev)
+            change.url = self._getWebUrl(project, sha=event.newrev)
         elif event.ref and event.ref.startswith('refs/heads/'):
             # From the timer trigger or Post 2.13 Gerrit
             project = self.source.getProject(event.project_name)
@@ -356,7 +362,7 @@ class GerritConnection(BaseConnection):
             change.branch = event.ref[len('refs/heads/'):]
             change.oldrev = event.oldrev
             change.newrev = event.newrev
-            change.url = self._getGitwebUrl(project, sha=event.newrev)
+            change.url = self._getWebUrl(project, sha=event.newrev)
         elif event.ref:
             # catch-all ref (ie, not a branch or head)
             project = self.source.getProject(event.project_name)
@@ -364,7 +370,7 @@ class GerritConnection(BaseConnection):
             change.ref = event.ref
             change.oldrev = event.oldrev
             change.newrev = event.newrev
-            change.url = self._getGitwebUrl(project, sha=event.newrev)
+            change.url = self._getWebUrl(project, sha=event.newrev)
         else:
             self.log.warning("Unable to get change for %s" % (event,))
             change = None
@@ -848,11 +854,11 @@ class GerritConnection(BaseConnection):
                                      project.name)
         return url
 
-    def _getGitwebUrl(self, project: Project, sha: str=None) -> str:
-        url = '%s/gitweb?p=%s.git' % (self.baseurl, project.name)
-        if sha:
-            url += ';a=commitdiff;h=' + sha
-        return url
+    def _getWebUrl(self, project: Project, sha: str=None) -> str:
+        return self.gitweb_url_template.format(
+            baseurl=self.baseurl,
+            project=project.getSafeAttributes(),
+            sha=sha)
 
     def onLoad(self):
         self.log.debug("Starting Gerrit Connection/Watchers")
diff --git a/zuul/model.py b/zuul/model.py
index ac2a75ede..0afc2ab58 100644
--- a/zuul/model.py
+++ b/zuul/model.py
@@ -352,6 +352,9 @@ class Project(object):
     def __repr__(self):
         return '<Project %s>' % (self.name)
 
+    def getSafeAttributes(self):
+        return Attributes(name=self.name)
+
 
 class Node(object):
     """A single node for use by a job.