From a7f724c8f2a7fe5930afeb69c04adede6ae0d2a2 Mon Sep 17 00:00:00 2001 From: Clark Boylan Date: Wed, 25 Oct 2017 11:35:19 -0700 Subject: [PATCH] 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 --- doc/source/admin/drivers/gerrit.rst | 11 ++++++++ tests/fixtures/zuul-connections-cgit.conf | 27 +++++++++++++++++++ tests/fixtures/zuul-connections-gitweb.conf | 26 ++++++++++++++++++ tests/unit/test_connection.py | 29 +++++++++++++++++++++ zuul/driver/gerrit/gerritconnection.py | 24 ++++++++++------- zuul/model.py | 3 +++ 6 files changed, 111 insertions(+), 9 deletions(-) create mode 100644 tests/fixtures/zuul-connections-cgit.conf create mode 100644 tests/fixtures/zuul-connections-gitweb.conf diff --git a/doc/source/admin/drivers/gerrit.rst b/doc/source/admin/drivers/gerrit.rst index ac42bd38f9..935cb32901 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 0000000000..39dc0bb699 --- /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 0000000000..172208e485 --- /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 719f307b4b..c882d3a0a7 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 c3f9ee2cd7..59051bb167 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 ac2a75ede3..0afc2ab585 100644 --- a/zuul/model.py +++ b/zuul/model.py @@ -352,6 +352,9 @@ class Project(object): def __repr__(self): return '' % (self.name) + def getSafeAttributes(self): + return Attributes(name=self.name) + class Node(object): """A single node for use by a job.