Merge pull request #2359 from coreos-inc/fix-gitlab-tag

Fix handling of gitlab web hooks when tagging

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
+  found_commit = JSONPathDict({})
-  # any order, so we cannot simply index into the commits list.
+  if payload['object_kind'] == 'push':
-  found_commit = None
+    # Find the commit associated with the checkout_sha. Gitlab doesn't (necessary) send this in
-  for commit in commits:
+    # any order, so we cannot simply index into the commits list.
-    if commit['id'] == payload['checkout_sha']:
+    found_commit = None
-      found_commit = JSONPathDict(commit)
+    for commit in commits:
-      break
+      if commit['id'] == payload['checkout_sha']:
+        found_commit = JSONPathDict(commit)
+        break
 
-  if found_commit is None:
+    if found_commit is None:
-    raise SkipRequestException
+      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.

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',

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
+  }
+}

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
+}