From a34d18b9ea9b7c537f2c8956b68fc33bc4f525c5 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 15 Feb 2017 14:30:21 -0500 Subject: [PATCH] Fix handling of gitlab web hooks when tagging Gitlab doesn't send any commit information for tagging events (because... reasons), and so we have to perform the lookup ourselves to have full metadata. Fixes #1467 --- buildtrigger/gitlabhandler.py | 53 +++++++++++++----- test/test_prepare_trigger.py | 62 ++++++++++++++++++++++ test/triggerjson/gitlab_webhook_other.json | 14 +++++ test/triggerjson/gitlab_webhook_tag.json | 38 +++++++++++++ 4 files changed, 154 insertions(+), 13 deletions(-) create mode 100644 test/triggerjson/gitlab_webhook_other.json create mode 100644 test/triggerjson/gitlab_webhook_tag.json diff --git a/buildtrigger/gitlabhandler.py b/buildtrigger/gitlabhandler.py index 777555b35..47d288144 100644 --- a/buildtrigger/gitlabhandler.py +++ b/buildtrigger/gitlabhandler.py @@ -82,7 +82,8 @@ def _catch_timeouts(func): return wrapper -def get_transformed_webhook_payload(gl_payload, default_branch=None, lookup_user=None): +def get_transformed_webhook_payload(gl_payload, default_branch=None, lookup_user=None, + lookup_commit=None): """ Returns the Gitlab webhook JSON payload transformed into our own payload format. If the gl_payload is not valid, returns None. """ @@ -93,9 +94,13 @@ def get_transformed_webhook_payload(gl_payload, default_branch=None, lookup_user payload = JSONPathDict(gl_payload) + if payload['object_kind'] != 'push' and payload['object_kind'] != 'tag_push': + # Unknown kind of webhook. + raise SkipRequestException + # Check for empty commits. The commits list will be empty if the branch is deleted. commits = payload['commits'] - if not commits: + if payload['object_kind'] == 'push' and not commits: raise SkipRequestException config = SafeDictSetter() @@ -104,16 +109,25 @@ def get_transformed_webhook_payload(gl_payload, default_branch=None, lookup_user config['default_branch'] = default_branch config['git_url'] = payload['repository.git_ssh_url'] - # Find the commit associated with the checkout_sha. Gitlab doesn't (necessary) send this in - # any order, so we cannot simply index into the commits list. - found_commit = None - for commit in commits: - if commit['id'] == payload['checkout_sha']: - found_commit = JSONPathDict(commit) - break + found_commit = JSONPathDict({}) + if payload['object_kind'] == 'push': + # Find the commit associated with the checkout_sha. Gitlab doesn't (necessary) send this in + # any order, so we cannot simply index into the commits list. + found_commit = None + for commit in commits: + if commit['id'] == payload['checkout_sha']: + found_commit = JSONPathDict(commit) + break - if found_commit is None: - raise SkipRequestException + if found_commit is None: + raise SkipRequestException + + elif payload['object_kind'] == 'tag_push': + # Gitlab doesn't send commit information for tag pushes (WHY?!), so we need to lookup the + # commit SHA directly. + if lookup_commit: + found_commit_info = lookup_commit(payload['project_id'], payload['checkout_sha']) + found_commit = JSONPathDict(found_commit_info or {}) config['commit_info.url'] = found_commit['url'] config['commit_info.message'] = found_commit['message'] @@ -121,7 +135,7 @@ def get_transformed_webhook_payload(gl_payload, default_branch=None, lookup_user # Note: Gitlab does not send full user information with the payload, so we have to # (optionally) look it up. - author_email = found_commit['author.email'] + author_email = found_commit['author.email'] or found_commit['author_email'] if lookup_user and author_email: author_info = lookup_user(author_email) if author_info: @@ -333,6 +347,18 @@ class GitLabBuildTrigger(BuildTriggerHandler): def get_repository_url(self): return gitlab_trigger.get_public_url(self.config['build_source']) + @_catch_timeouts + def lookup_commit(self, repo_id, commit_sha): + if repo_id is None: + return None + + gl_client = self._get_authorized_client() + commit = gl_client.getrepositorycommit(repo_id, commit_sha) + if commit is False: + return None + + return commit + @_catch_timeouts def lookup_user(self, email): gl_client = self._get_authorized_client() @@ -441,7 +467,8 @@ class GitLabBuildTrigger(BuildTriggerHandler): default_branch = repo['default_branch'] metadata = get_transformed_webhook_payload(payload, default_branch=default_branch, - lookup_user=self.lookup_user) + lookup_user=self.lookup_user, + lookup_commit=self.lookup_commit) prepared = self.prepare_build(metadata) # Check if we should skip this build. diff --git a/test/test_prepare_trigger.py b/test/test_prepare_trigger.py index fec64ea23..f572f4d32 100644 --- a/test/test_prepare_trigger.py +++ b/test/test_prepare_trigger.py @@ -392,6 +392,68 @@ class TestPrepareTrigger(unittest.TestCase): self.assertSchema('gitlab_webhook_multicommit', expected, gl_webhook, lookup_user=lookup_user) + def test_gitlab_webhook_for_tag(self): + expected = { + 'commit': u'82b3d5ae55f7080f1e6022629cdb57bfae7cccc7', + 'commit_info': { + 'author': { + 'avatar_url': 'http://some/avatar/url', + 'url': 'http://gitlab.com/jzelinskie', + 'username': 'jzelinskie' + }, + 'date': '2015-08-13T19:33:18+00:00', + 'message': 'Fix link\n', + 'url': 'https://some/url', + }, + 'git_url': u'git@example.com:jsmith/example.git', + 'ref': u'refs/tags/v1.0.0', + } + + def lookup_user(_): + return { + 'username': 'jzelinskie', + 'html_url': 'http://gitlab.com/jzelinskie', + 'avatar_url': 'http://some/avatar/url', + } + + def lookup_commit(repo_id, commit_sha): + if commit_sha == '82b3d5ae55f7080f1e6022629cdb57bfae7cccc7': + return { + "id": "82b3d5ae55f7080f1e6022629cdb57bfae7cccc7", + "message": "Fix link\n", + "timestamp": "2015-08-13T19:33:18+00:00", + "url": "https://some/url", + "author_name": "Foo Guy", + "author_email": "foo@bar.com", + } + + return None + + self.assertSchema('gitlab_webhook_tag', expected, gl_webhook, lookup_user=lookup_user, + lookup_commit=lookup_commit) + + + def test_gitlab_webhook_for_tag_nocommit(self): + expected = { + 'commit': u'82b3d5ae55f7080f1e6022629cdb57bfae7cccc7', + 'git_url': u'git@example.com:jsmith/example.git', + 'ref': u'refs/tags/v1.0.0', + } + + def lookup_user(_): + return { + 'username': 'jzelinskie', + 'html_url': 'http://gitlab.com/jzelinskie', + 'avatar_url': 'http://some/avatar/url', + } + + self.assertSchema('gitlab_webhook_tag', expected, gl_webhook, lookup_user=lookup_user) + + + def test_gitlab_webhook_for_other(self): + self.assertSkipped('gitlab_webhook_other', gl_webhook) + + def test_gitlab_webhook_payload_with_lookup(self): expected = { 'commit': u'fb88379ee45de28a0a4590fddcbd8eff8b36026e', diff --git a/test/triggerjson/gitlab_webhook_other.json b/test/triggerjson/gitlab_webhook_other.json new file mode 100644 index 000000000..0704a7b6b --- /dev/null +++ b/test/triggerjson/gitlab_webhook_other.json @@ -0,0 +1,14 @@ +{ + "object_kind": "someother", + "ref": "refs/tags/v1.0.0", + "checkout_sha": "82b3d5ae55f7080f1e6022629cdb57bfae7cccc7", + "repository":{ + "name": "Example", + "url": "ssh://git@example.com/jsmith/example.git", + "description": "", + "homepage": "http://example.com/jsmith/example", + "git_http_url":"http://example.com/jsmith/example.git", + "git_ssh_url":"git@example.com:jsmith/example.git", + "visibility_level":0 + } +} \ No newline at end of file diff --git a/test/triggerjson/gitlab_webhook_tag.json b/test/triggerjson/gitlab_webhook_tag.json new file mode 100644 index 000000000..86dff4ce5 --- /dev/null +++ b/test/triggerjson/gitlab_webhook_tag.json @@ -0,0 +1,38 @@ +{ + "object_kind": "tag_push", + "before": "0000000000000000000000000000000000000000", + "after": "82b3d5ae55f7080f1e6022629cdb57bfae7cccc7", + "ref": "refs/tags/v1.0.0", + "checkout_sha": "82b3d5ae55f7080f1e6022629cdb57bfae7cccc7", + "user_id": 1, + "user_name": "John Smith", + "user_avatar": "https://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=8://s.gravatar.com/avatar/d4c74594d841139328695756648b6bd6?s=80", + "project_id": 1, + "project":{ + "name":"Example", + "description":"", + "web_url":"http://example.com/jsmith/example", + "avatar_url":null, + "git_ssh_url":"git@example.com:jsmith/example.git", + "git_http_url":"http://example.com/jsmith/example.git", + "namespace":"Jsmith", + "visibility_level":0, + "path_with_namespace":"jsmith/example", + "default_branch":"master", + "homepage":"http://example.com/jsmith/example", + "url":"git@example.com:jsmith/example.git", + "ssh_url":"git@example.com:jsmith/example.git", + "http_url":"http://example.com/jsmith/example.git" + }, + "repository":{ + "name": "Example", + "url": "ssh://git@example.com/jsmith/example.git", + "description": "", + "homepage": "http://example.com/jsmith/example", + "git_http_url":"http://example.com/jsmith/example.git", + "git_ssh_url":"git@example.com:jsmith/example.git", + "visibility_level":0 + }, + "commits": [], + "total_commits_count": 0 +} \ No newline at end of file