Split github hook ingest and processing
We were taking in the hook and processing the content fully while the
connection to github remained open. This created a long delay and
blocked the thread from continuing, which can lead to timeouts.
This change splits things apart to model gerrit a bit more, so that when
a hook comes in, we minimally validate it and toss it into a queue, so
that we close the connection quickly. A second thread will iterate over
the queue to process the (potential) events.
This change drops handling of ping events, which would validate if the
project is one we know about. A follow up change will introduce project
validation at a higher level.
Change-Id: I463f4b888be056a3e2175ccdab0286d2ef4fa2b2
Signed-off-by: Jesse Keating <omgjlk@us.ibm.com>
diff --git a/tests/base.py b/tests/base.py
index 4214809..45d724d 100755
--- a/tests/base.py
+++ b/tests/base.py
@@ -2145,6 +2145,7 @@
def getGithubConnection(driver, name, config):
con = FakeGithubConnection(driver, name, config,
upstream_root=self.upstream_root)
+ self.event_queues.append(con.event_queue)
setattr(self, 'fake_' + name, con)
return con
diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py
index a088236..ebb5e1c 100644
--- a/tests/unit/test_github_driver.py
+++ b/tests/unit/test_github_driver.py
@@ -17,6 +17,7 @@
from testtools.matchers import MatchesRegex, StartsWith
import urllib
import time
+from unittest import skip
import git
@@ -685,6 +686,8 @@
# New timestamp should be greater than the old timestamp
self.assertLess(old, new)
+ # TODO(jlk): Make this a more generic test for unknown project
+ @skip("Skipped for rewrite of webhook handler")
@simple_layout('layouts/basic-github.yaml', driver='github')
def test_ping_event(self):
# Test valid ping
diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py
index 0ce6ef5..fca36c8 100644
--- a/zuul/driver/github/githubconnection.py
+++ b/zuul/driver/github/githubconnection.py
@@ -17,6 +17,8 @@
import logging
import hmac
import hashlib
+import queue
+import threading
import time
import re
@@ -80,11 +82,10 @@
delivery=delivery))
self._validate_signature(request)
+ # TODO(jlk): Validate project in the request is a project we know
try:
self.__dispatch_event(request)
- except webob.exc.HTTPNotFound:
- raise
except:
self.log.exception("Exception handling Github event:")
@@ -98,20 +99,58 @@
'header.')
try:
- method = getattr(self, '_event_' + event)
- except AttributeError:
- message = "Unhandled X-Github-Event: {0}".format(event)
- self.log.debug(message)
- # Returns empty 200 on unhandled events
- raise webob.exc.HTTPOk()
-
- try:
json_body = request.json_body
+ self.connection.addEvent(json_body, event)
except:
message = 'Exception deserializing JSON body'
self.log.exception(message)
raise webob.exc.HTTPBadRequest(message)
+ def _validate_signature(self, request):
+ secret = self.connection.connection_config.get('webhook_token', None)
+ if secret is None:
+ raise RuntimeError("webhook_token is required")
+
+ body = request.body
+ try:
+ request_signature = request.headers['X-Hub-Signature']
+ except KeyError:
+ raise webob.exc.HTTPUnauthorized(
+ 'Please specify a X-Hub-Signature header with secret.')
+
+ payload_signature = _sign_request(body, secret)
+
+ self.log.debug("Payload Signature: {0}".format(str(payload_signature)))
+ self.log.debug("Request Signature: {0}".format(str(request_signature)))
+ if not hmac.compare_digest(
+ str(payload_signature), str(request_signature)):
+ raise webob.exc.HTTPUnauthorized(
+ 'Request signature does not match calculated payload '
+ 'signature. Check that secret is correct.')
+
+ return True
+
+
+class GithubEventConnector(threading.Thread):
+ """Move events from GitHub into the scheduler"""
+
+ log = logging.getLogger("zuul.GithubEventConnector")
+
+ def __init__(self, connection):
+ super(GithubEventConnector, self).__init__()
+ self.daemon = True
+ self.connection = connection
+ self._stopped = False
+
+ def stop(self):
+ self._stopped = True
+ self.connection.addEvent(None)
+
+ def _handleEvent(self):
+ json_body, event_type = self.connection.getEvent()
+ if self._stopped:
+ return
+
# If there's any installation mapping information in the body then
# update the project mapping before any requests are made.
installation_id = json_body.get('installation', {}).get('id')
@@ -127,9 +166,17 @@
self.connection.installation_map[project_name] = installation_id
try:
+ method = getattr(self, '_event_' + event_type)
+ except AttributeError:
+ # TODO(jlk): Gracefully handle event types we don't care about
+ # instead of logging an exception.
+ message = "Unhandled X-Github-Event: {0}".format(event_type)
+ self.log.debug(message)
+ # Returns empty on unhandled events
+ return
+
+ try:
event = method(json_body)
- except webob.exc.HTTPNotFound:
- raise
except:
self.log.exception('Exception when handling event:')
event = None
@@ -240,14 +287,6 @@
event.action = body.get('action')
return event
- def _event_ping(self, body):
- project_name = body['repository']['full_name']
- if not self.connection.getProject(project_name):
- self.log.warning("Ping received for unknown project %s" %
- project_name)
- raise webob.exc.HTTPNotFound("Sorry, this project is not "
- "registered")
-
def _event_status(self, body):
action = body.get('action')
if action == 'pending':
@@ -277,30 +316,6 @@
(number, project_name))
return pr_body
- def _validate_signature(self, request):
- secret = self.connection.connection_config.get('webhook_token', None)
- if secret is None:
- raise RuntimeError("webhook_token is required")
-
- body = request.body
- try:
- request_signature = request.headers['X-Hub-Signature']
- except KeyError:
- raise webob.exc.HTTPUnauthorized(
- 'Please specify a X-Hub-Signature header with secret.')
-
- payload_signature = _sign_request(body, secret)
-
- self.log.debug("Payload Signature: {0}".format(str(payload_signature)))
- self.log.debug("Request Signature: {0}".format(str(request_signature)))
- if not hmac.compare_digest(
- str(payload_signature), str(request_signature)):
- raise webob.exc.HTTPUnauthorized(
- 'Request signature does not match calculated payload '
- 'signature. Check that secret is correct.')
-
- return True
-
def _pull_request_to_event(self, pr_body):
event = GithubTriggerEvent()
event.trigger_name = 'github'
@@ -327,6 +342,17 @@
if login:
return self.connection.getUser(login)
+ def run(self):
+ while True:
+ if self._stopped:
+ return
+ try:
+ self._handleEvent()
+ except:
+ self.log.exception("Exception moving GitHub event:")
+ finally:
+ self.connection.eventDone()
+
class GithubUser(collections.Mapping):
log = logging.getLogger('zuul.GithubUser')
@@ -376,6 +402,7 @@
self.canonical_hostname = self.connection_config.get(
'canonical_hostname', self.server)
self.source = driver.getSource(self)
+ self.event_queue = queue.Queue()
# ssl verification must default to true
verify_ssl = self.connection_config.get('verify_ssl', 'true')
@@ -408,9 +435,20 @@
self.registerHttpHandler(self.payload_path,
webhook_listener.handle_request)
self._authenticateGithubAPI()
+ self._start_event_connector()
def onStop(self):
self.unregisterHttpHandler(self.payload_path)
+ self._stop_event_connector()
+
+ def _start_event_connector(self):
+ self.github_event_connector = GithubEventConnector(self)
+ self.github_event_connector.start()
+
+ def _stop_event_connector(self):
+ if self.github_event_connector:
+ self.github_event_connector.stop()
+ self.github_event_connector.join()
def _createGithubClient(self):
if self.server != 'github.com':
@@ -504,6 +542,15 @@
return token
+ def addEvent(self, data, event=None):
+ return self.event_queue.put((data, event))
+
+ def getEvent(self):
+ return self.event_queue.get()
+
+ def eventDone(self):
+ self.event_queue.task_done()
+
def getGithubClient(self,
project=None,
user_id=None,