diff --git a/master/buildbot/status/github.py b/master/buildbot/status/github.py index 122a2c13b7a..c83622cad15 100644 --- a/master/buildbot/status/github.py +++ b/master/buildbot/status/github.py @@ -14,52 +14,69 @@ # Copyright Buildbot Team Members from __future__ import absolute_import -import datetime +from datetime import datetime from twisted.internet import defer from twisted.python import log -from txgithub.api import GithubApi as GitHubAPI +try: + from txgithub.api import GithubApi as GitHubAPI +except ImportError: + GitHubAPI = None from zope.interface import implements +from buildbot import config from buildbot.interfaces import IStatusReceiver from buildbot.process.properties import Interpolate from buildbot.status.base import StatusReceiverMultiService from buildbot.status.builder import FAILURE from buildbot.status.builder import SUCCESS +from buildbot.util import human_readable_delta +_STATE_MAP = { + SUCCESS: 'success', + FAILURE: 'failure', +} -class GitHubStatus(StatusReceiverMultiService): - implements(IStatusReceiver) +def _getGitHubState(results): + """ + Convert Buildbot states into GitHub states. + """ + # GitHub defines `success`, `failure` and `error` states. + # We explicitly map success and failure. Any other BuildBot status + # is converted to `error`. + return _STATE_MAP.get(results, 'error') + + +class GitHubStatus(StatusReceiverMultiService): """ Send build status to GitHub. For more details see Buildbot's user manual. """ + implements(IStatusReceiver) + def __init__(self, token, repoOwner, repoName, sha=None, - startDescription=None, endDescription=None): + startDescription=None, endDescription=None, + baseURL=None): """ Token for GitHub API. """ - StatusReceiverMultiService.__init__(self) - - if not sha: - sha = Interpolate("%(src::revision)s") - - if not startDescription: - startDescription = "Build started." - self._startDescription = startDescription + if not GitHubAPI: + config.error('GitHubStatus requires txgithub package installed') - if not endDescription: - endDescription = "Build done." - self._endDescription = endDescription + StatusReceiverMultiService.__init__(self) - self._token = token - self._sha = sha + self._sha = sha or Interpolate("%(src::revision)s") self._repoOwner = repoOwner self._repoName = repoName - self._github = GitHubAPI(oauth2_token=self._token) + self._startDescription = startDescription or "Build started." + self._endDescription = endDescription or "Build done." + + self._github = GitHubAPI(oauth2_token=token, baseURL=baseURL) + + self._status = None def startService(self): StatusReceiverMultiService.startService(self) @@ -70,7 +87,7 @@ def stopService(self): StatusReceiverMultiService.stopService(self) self._status.unsubscribe(self) - def builderAdded(self, name, builder): + def builderAdded(self, name_, builder_): """ Subscribe to all builders. """ @@ -81,10 +98,9 @@ def buildStarted(self, builderName, build): See: C{IStatusReceiver}. """ d = self._sendStartStatus(builderName, build) - d.addErrback( - log.err, - 'While sending start status to GitHub for %s.' % ( - builderName)) + d.addErrback(log.err, + 'While sending start status to GitHub for %s.' % + (builderName,)) @defer.inlineCallbacks def _sendStartStatus(self, builderName, build): @@ -95,7 +111,7 @@ def _sendStartStatus(self, builderName, build): if not status: defer.returnValue(None) - (startTime, endTime) = build.getTimes() + startTime, _ = build.getTimes() description = yield build.render(self._startDescription) @@ -103,8 +119,7 @@ def _sendStartStatus(self, builderName, build): 'state': 'pending', 'description': description, 'builderName': builderName, - 'startDateTime': datetime.datetime.fromtimestamp( - startTime).isoformat(' '), + 'startDateTime': datetime.fromtimestamp(startTime).isoformat(' '), 'endDateTime': 'In progress', 'duration': 'In progress', }) @@ -116,10 +131,9 @@ def buildFinished(self, builderName, build, results): See: C{IStatusReceiver}. """ d = self._sendFinishStatus(builderName, build, results) - d.addErrback( - log.err, - 'While sending finish status to GitHub for %s.' % ( - builderName)) + d.addErrback(log.err, + 'While sending finish status to GitHub for %s.' % + (builderName,)) @defer.inlineCallbacks def _sendFinishStatus(self, builderName, build, results): @@ -130,52 +144,23 @@ def _sendFinishStatus(self, builderName, build, results): if not status: defer.returnValue(None) - state = self._getGitHubState(results) - (startTime, endTime) = build.getTimes() - duration = self._timeDeltaToHumanReadable(startTime, endTime) + state = _getGitHubState(results) + startTime, endTime = build.getTimes() + duration = human_readable_delta(startTime, endTime) description = yield build.render(self._endDescription) status.update({ 'state': state, 'description': description, 'builderName': builderName, - 'startDateTime': datetime.datetime.fromtimestamp( - startTime).isoformat(' '), - 'endDateTime': datetime.datetime.fromtimestamp( - endTime).isoformat(' '), + 'startDateTime': datetime.fromtimestamp(startTime).isoformat(' '), + 'endDateTime': datetime.fromtimestamp(endTime).isoformat(' '), 'duration': duration, }) result = yield self._sendGitHubStatus(status) defer.returnValue(result) - def _timeDeltaToHumanReadable(self, start, end): - """ - Return a string of human readable time delta. - """ - start_date = datetime.datetime.fromtimestamp(start) - end_date = datetime.datetime.fromtimestamp(end) - delta = end_date - start_date - - result = [] - if delta.days > 0: - result.append('%d days' % (delta.days,)) - if delta.seconds > 0: - hours = delta.seconds / 3600 - if hours > 0: - result.append('%d hours' % (hours,)) - minutes = (delta.seconds - hours * 3600) / 60 - if minutes: - result.append('%d minutes' % (minutes,)) - seconds = delta.seconds % 60 - if seconds > 0: - result.append('%d seconds' % (seconds,)) - result = ', '.join(result) - if not result: - return 'super fast' - else: - return result - @defer.inlineCallbacks def _getGitHubRepoProperties(self, build): """ @@ -203,23 +188,6 @@ def _getGitHubRepoProperties(self, build): } defer.returnValue(result) - def _getGitHubState(self, results): - """ - Convert Buildbot states into GitHub states. - """ - # GitHub defines `success`, `failure` and `error` states. - # We explicitly map success and failure. Any other BuildBot status - # is converted to `error`. - state_map = { - SUCCESS: 'success', - FAILURE: 'failure', - } - - try: - return state_map[results] - except KeyError: - return 'error' - def _sendGitHubStatus(self, status): """ Send status to GitHub API. diff --git a/master/buildbot/status/web/change_hook.py b/master/buildbot/status/web/change_hook.py index 801d39fa50f..422bd615de9 100644 --- a/master/buildbot/status/web/change_hook.py +++ b/master/buildbot/status/web/change_hook.py @@ -32,13 +32,16 @@ class ChangeHookResource(resource.Resource): contentType = "text/html; charset=utf-8" children = {} - def __init__(self, dialects={}): + def __init__(self, dialects=None): """ The keys of 'dialects' select a modules to load under master/buildbot/status/web/hooks/ The value is passed to the module's getChanges function, providing configuration options to the dialect. """ + if dialects is None: + dialects = {} + self.dialects = dialects self.request_dialect = None @@ -88,7 +91,9 @@ def err(why): log.err(why, "adding changes from web hook") request.setResponseCode(500) request.finish() + d.addCallbacks(ok, err) + return server.NOT_DONE_YET def getChanges(self, request): @@ -119,7 +124,7 @@ def getChanges(self, request): else: dialect = 'base' - if dialect in self.dialects.keys(): + if dialect in self.dialects: log.msg("Attempting to load module buildbot.status.web.hooks." + dialect) tempModule = namedModule('buildbot.status.web.hooks.' + dialect) changes, src = tempModule.getChanges(request, self.dialects[dialect]) diff --git a/master/buildbot/status/web/hooks/github.py b/master/buildbot/status/web/hooks/github.py index c47b2920a3d..19bd24c8b49 100644 --- a/master/buildbot/status/web/hooks/github.py +++ b/master/buildbot/status/web/hooks/github.py @@ -13,8 +13,11 @@ # # Copyright Buildbot Team Members -import re +import hmac import logging +import re + +from hashlib import sha1 from dateutil.parser import parse as dateparse from twisted.python import log @@ -25,109 +28,161 @@ except ImportError: import simplejson as json +_HEADER_CT = 'Content-Type' +_HEADER_EVENT = 'X-GitHub-Event' +_HEADER_SIGNATURE = 'X-Hub-Signature' -def getChanges(request, options=None): - """ - Responds only to POST events and starts the build process - :arguments: - request - the http request object - """ +class GitHubEventHandler(object): + def __init__(self, secret, strict, codebase=None): + self._secret = secret + self._strict = strict + self._codebase = codebase + + if self._strict and not self._secret: + raise ValueError('Strict mode is requested ' + 'while no secret is provided') + + def process(self, request): + payload = self._get_payload(request) + + event_type = request.getHeader(_HEADER_EVENT) + log.msg("X-GitHub-Event: %r" % (event_type,), logLevel=logging.DEBUG) + + handler = getattr(self, 'handle_%s' % event_type, None) + + if handler is None: + raise ValueError('Unknown event: %r' % (event_type,)) + + return handler(payload) + + def _get_payload(self, request): + content = request.content.read() + + signature = request.getHeader(_HEADER_SIGNATURE) + + if not signature and self._strict: + raise ValueError('Request has no required signature') + + if self._secret and signature: + try: + hash_type, hexdigest = signature.split('=') + except ValueError: + raise ValueError('Wrong signature format: %r' % (signature,)) + + if hash_type != 'sha1': + raise ValueError('Unknown hash type: %s' % (hash_type,)) + + mac = hmac.new(self._secret, msg=content, digestmod=sha1) + # NOTE: hmac.compare_digest should be used, but it's only available + # starting Python 2.7.7 + if mac.hexdigest() != hexdigest: + raise ValueError('Hash mismatch') + + content_type = request.getHeader(_HEADER_CT) + + if content_type == 'application/json': + payload = json.loads(content) + elif content_type == 'application/x-www-form-urlencoded': + payload = json.loads(request.args['payload'][0]) + else: + raise ValueError('Unknown content type: %r' % (content_type,)) + + log.msg("Payload: %r" % payload, logLevel=logging.DEBUG) + + return payload + + def handle_ping(self, _): + return [], 'git' + + def handle_push(self, payload): + # This field is unused: + user = None + # user = payload['pusher']['name'] + repo = payload['repository']['name'] + repo_url = payload['repository']['url'] + # NOTE: what would be a reasonable value for project? + # project = request.args.get('project', [''])[0] + project = payload['repository']['full_name'] + + changes = self._process_change(payload, user, repo, repo_url, project) + + log.msg("Received %d changes from github" % len(changes)) + + return changes, 'git' + + def _process_change(self, payload, user, repo, repo_url, project): + """ + Consumes the JSON as a python object and actually starts the build. + + :arguments: + payload + Python Object that represents the JSON sent by GitHub Service + Hook. + """ + changes = [] + refname = payload['ref'] + + # We only care about regular heads, i.e. branches + match = re.match(r"^refs\/heads\/(.+)$", refname) + if not match: + log.msg("Ignoring refname `%s': Not a branch" % refname) + return changes + + branch = match.group(1) + if payload.get('deleted'): + log.msg("Branch `%s' deleted, ignoring" % branch) + return changes + + for commit in payload['commits']: + if not commit.get('distinct', True): + log.msg('Commit `%s` is a non-distinct commit, ignoring...' % + (commit['id'],)) + continue - event_type = request.getHeader("X-GitHub-Event") - log.msg("X-GitHub-Event: %r" % event_type, logLevel=logging.DEBUG) + files = [] + for kind in ('added', 'modified', 'removed'): + files.extend(commit.get(kind, [])) - if event_type == "ping": - return ([], 'git') + when_timestamp = dateparse(commit['timestamp']) - # Reject non-push, non-ping events - if event_type != "push": - raise ValueError( - "Rejecting request. Expected a push event but received %r instead." % event_type) + log.msg("New revision: %s" % commit['id'][:8]) - content_type = request.getHeader("Content-Type") + change = { + 'author': '%s <%s>' % (commit['author']['name'], + commit['author']['email']), + 'files': files, + 'comments': commit['message'], + 'revision': commit['id'], + 'when_timestamp': when_timestamp, + 'branch': branch, + 'revlink': commit['url'], + 'repository': repo_url, + 'project': project + } - if content_type == "application/json": - payload = json.loads(request.content.read()) - elif content_type == "application/x-www-form-urlencoded": - payload = json.loads(request.args["payload"][0]) - else: - raise ValueError( - "Rejecting request. Unknown 'Content-Type', received %r" % content_type) + if self._codebase is not None: + change['codebase'] = self._codebase - log.msg("Payload: %r" % payload, logLevel=logging.DEBUG) + changes.append(change) - # This field is unused: - user = None - # user = payload['pusher']['name'] - repo = payload['repository']['name'] - repo_url = payload['repository']['url'] - project = request.args.get('project', [''])[0] - # This field is unused: - # private = payload['repository']['private'] - changes = process_change(payload, user, repo, repo_url, project) - log.msg("Received %d changes from github" % len(changes)) - return (changes, 'git') + return changes -def process_change(payload, user, repo, repo_url, project, codebase=None): +def getChanges(request, options=None): """ - Consumes the JSON as a python object and actually starts the build. + Responds only to POST events and starts the build process :arguments: - payload - Python Object that represents the JSON sent by GitHub Service - Hook. + request + the http request object """ - changes = [] - refname = payload['ref'] - - # We only care about regular heads, i.e. branches - match = re.match(r"^refs\/heads\/(.+)$", refname) - if not match: - log.msg("Ignoring refname `%s': Not a branch" % refname) - else: - branch = match.group(1) - if payload.get('deleted') is True: - log.msg("Branch `%s' deleted, ignoring" % branch) - else: - for commit in payload['commits']: - if 'distinct' in commit and not commit['distinct']: - log.msg( - 'Commit `%s` is a non-distinct commit, ignoring...' % ( - commit['id']) - ) - continue - - files = [] - if 'added' in commit: - files.extend(commit['added']) - if 'modified' in commit: - files.extend(commit['modified']) - if 'removed' in commit: - files.extend(commit['removed']) - when_timestamp = dateparse(commit['timestamp']) - - log.msg("New revision: %s" % commit['id'][:8]) - - change = { - 'author': '%s <%s>' % ( - commit['author']['name'], commit['author']['email'] - ), - 'files': files, - 'comments': commit['message'], - 'revision': commit['id'], - 'when_timestamp': when_timestamp, - 'branch': branch, - 'revlink': commit['url'], - 'repository': repo_url, - 'project': project - } - - if codebase is not None: - change['codebase'] = codebase - - changes.append(change) - - return changes + if options is None: + options = {} + + klass = options.get('class', GitHubEventHandler) + + handler = klass(options.get('secret', None), + options.get('strict', False), + options.get('codebase', None)) + return handler.process(request) diff --git a/master/buildbot/status/web/hooks/gitlab.py b/master/buildbot/status/web/hooks/gitlab.py index 4468f1a126a..082609c8499 100644 --- a/master/buildbot/status/web/hooks/gitlab.py +++ b/master/buildbot/status/web/hooks/gitlab.py @@ -13,11 +13,71 @@ # # Copyright Buildbot Team Members -from buildbot.status.web.hooks.github import process_change +import re + from buildbot.util import json +from dateutil.parser import parse as dateparse from twisted.python import log +def _process_change(payload, user, repo, repo_url, project, codebase=None): + """ + Consumes the JSON as a python object and actually starts the build. + + :arguments: + payload + Python Object that represents the JSON sent by GitHub Service + Hook. + """ + changes = [] + refname = payload['ref'] + + # We only care about regular heads, i.e. branches + match = re.match(r"^refs\/heads\/(.+)$", refname) + if not match: + log.msg("Ignoring refname `%s': Not a branch" % refname) + return changes + + branch = match.group(1) + if payload.get('deleted'): + log.msg("Branch `%s' deleted, ignoring" % branch) + return changes + + for commit in payload['commits']: + if not commit.get('distinct', True): + log.msg('Commit `%s` is a non-distinct commit, ignoring...' % + (commit['id'],)) + continue + + files = [] + for kind in ('added', 'modified', 'removed'): + files.extend(commit.get(kind, [])) + + when_timestamp = dateparse(commit['timestamp']) + + log.msg("New revision: %s" % commit['id'][:8]) + + change = { + 'author': '%s <%s>' % (commit['author']['name'], + commit['author']['email']), + 'files': files, + 'comments': commit['message'], + 'revision': commit['id'], + 'when_timestamp': when_timestamp, + 'branch': branch, + 'revlink': commit['url'], + 'repository': repo_url, + 'project': project + } + + if codebase is not None: + change['codebase'] = codebase + + changes.append(change) + + return changes + + def getChanges(request, options=None): """ Reponds only to POST events and starts the build process @@ -39,6 +99,6 @@ def getChanges(request, options=None): codebase = codebase[0] # This field is unused: # private = payload['repository']['private'] - changes = process_change(payload, user, repo, repo_url, project, codebase=codebase) + changes = _process_change(payload, user, repo, repo_url, project, codebase=codebase) log.msg("Received %s changes from gitlab" % len(changes)) return (changes, 'git') diff --git a/master/buildbot/test/fake/web.py b/master/buildbot/test/fake/web.py index 3d117522478..e2a67080455 100644 --- a/master/buildbot/test/fake/web.py +++ b/master/buildbot/test/fake/web.py @@ -36,9 +36,12 @@ class FakeRequest(Mock): redirected_to = None failure = None - def __init__(self, args={}, content=''): + def __init__(self, args=None, content=''): Mock.__init__(self, spec=server.Request) + if args is None: + args = {} + self.args = args self.content = StringIO(content) self.site = Mock(spec=server.Site) diff --git a/master/buildbot/test/unit/test_status_github.py b/master/buildbot/test/unit/test_status_github.py index 2b18e260665..ae1446d4f9b 100644 --- a/master/buildbot/test/unit/test_status_github.py +++ b/master/buildbot/test/unit/test_status_github.py @@ -21,6 +21,7 @@ from mock import Mock +from mock import patch from twisted.internet import defer from twisted.trial import unittest @@ -34,7 +35,12 @@ except ImportError: txgithub = None else: + # Import fully qualified module for patching. + import buildbot.status.github + buildbot.status.github + from buildbot.status.github import GitHubStatus + from buildbot.status.github import _getGitHubState from buildbot.test.fake.fakebuild import FakeBuild from buildbot.test.util import logging @@ -119,7 +125,6 @@ def test_initialization_required_arguments(self): status = GitHubStatus( token=token, repoOwner=repoOwner, repoName=repoName) - self.assertEqual(token, status._token) self.assertEqual(repoOwner, status._repoOwner) self.assertEqual(repoName, status._repoName) @@ -128,6 +133,29 @@ def test_initialization_required_arguments(self): self.assertEqual(status._startDescription, "Build started.") self.assertEqual(status._endDescription, "Build done.") + def test_custom_github_url(self): + """ + Check that the custom URL is passed as it should be + """ + with patch('buildbot.status.github.GitHubAPI') as mock: + token = 'GitHub-API-Token' + owner = Interpolate('owner') + name = Interpolate('name') + + GitHubStatus(token, owner, name) + + mock.assert_called_once_with(oauth2_token=token, baseURL=None) + + with patch('buildbot.status.github.GitHubAPI') as mock: + token = 'GitHub-API-Token' + owner = Interpolate('owner') + name = Interpolate('name') + url = 'https://my.example.com/api' + + GitHubStatus(token, owner, name, baseURL=url) + + mock.assert_called_once_with(oauth2_token=token, baseURL=url) + def test_startService(self): """ When started, it will set parent as '_status' and subscribe to parent. @@ -241,7 +269,7 @@ def test_sendStartStatus_ok(self): 'repoOwner': 'repo-owner', 'repoName': 'repo-name', 'sha': '123', - 'targetURL': 'http://domain.tld', + 'targetURL': 'http://example.tld', 'buildNumber': '1', } self.status._sendGitHubStatus = Mock(return_value=defer.succeed(None)) @@ -256,7 +284,7 @@ def test_sendStartStatus_ok(self): 'repoOwner': 'repo-owner', 'repoName': 'repo-name', 'sha': '123', - 'targetURL': 'http://domain.tld', + 'targetURL': 'http://example.tld', 'buildNumber': '1', # Augmented arguments. 'state': 'pending', @@ -300,7 +328,7 @@ def test_sendFinishStatus_ok(self): 'repoOwner': 'repo-owner', 'repoName': 'repo-name', 'sha': '123', - 'targetURL': 'http://domain.tld', + 'targetURL': 'http://example.tld', 'buildNumber': '1', } self.status._sendGitHubStatus = Mock(return_value=defer.succeed(None)) @@ -316,7 +344,7 @@ def test_sendFinishStatus_ok(self): 'repoOwner': 'repo-owner', 'repoName': 'repo-name', 'sha': '123', - 'targetURL': 'http://domain.tld', + 'targetURL': 'http://example.tld', 'buildNumber': '1', # Augmented arguments. 'state': 'success', @@ -327,37 +355,6 @@ def test_sendFinishStatus_ok(self): 'duration': '2 seconds', }) - def test_timeDeltaToHumanReadable(self): - """ - It will return a human readable time difference. - """ - result = self.status._timeDeltaToHumanReadable(1, 1) - self.assertEqual('super fast', result) - - result = self.status._timeDeltaToHumanReadable(1, 2) - self.assertEqual('1 seconds', result) - - result = self.status._timeDeltaToHumanReadable(1, 61) - self.assertEqual('1 minutes', result) - - result = self.status._timeDeltaToHumanReadable(1, 62) - self.assertEqual('1 minutes, 1 seconds', result) - - result = self.status._timeDeltaToHumanReadable(1, 60 * 60 + 1) - self.assertEqual('1 hours', result) - - result = self.status._timeDeltaToHumanReadable(1, 60 * 60 + 61) - self.assertEqual('1 hours, 1 minutes', result) - - result = self.status._timeDeltaToHumanReadable(1, 60 * 60 + 62) - self.assertEqual('1 hours, 1 minutes, 1 seconds', result) - - result = self.status._timeDeltaToHumanReadable(1, 24 * 60 * 60 + 1) - self.assertEqual('1 days', result) - - result = self.status._timeDeltaToHumanReadable(1, 24 * 60 * 60 + 2) - self.assertEqual('1 days, 1 seconds', result) - def test_getGitHubRepoProperties_skip_no_sha(self): """ An empty dict is returned when any of the repo name, owner and sha @@ -402,7 +399,7 @@ def test_getGitHubRepoProperties_ok(self): self.status._repoName = Interpolate('name') self.status._sha = Interpolate('sha') self.status._status = Mock() - self.status._status.getURLForThing = lambda build: 'http://thing' + self.status._status.getURLForThing = lambda build: 'http://example.org' self.build.getNumber = lambda: 1 d = self.status._getGitHubRepoProperties(self.build) @@ -413,7 +410,7 @@ def test_getGitHubRepoProperties_ok(self): 'repoName': 'name', 'repoOwner': 'owner', 'sha': 'sha', - 'targetURL': 'http://thing', + 'targetURL': 'http://example.org', }, result) @@ -422,14 +419,9 @@ def test_getGitHubState(self): _getGitHubState will try to translate BuildBot status into GitHub status. For unknown values will fallback to 'error'. """ - self.assertEqual( - 'success', self.status._getGitHubState(SUCCESS)) - - self.assertEqual( - 'failure', self.status._getGitHubState(FAILURE)) - - self.assertEqual( - 'error', self.status._getGitHubState('anything-else')) + self.assertEqual('success', _getGitHubState(SUCCESS)) + self.assertEqual('failure', _getGitHubState(FAILURE)) + self.assertEqual('error', _getGitHubState('anything-else')) def test_sendGitHubStatus_success(self): """ diff --git a/master/buildbot/test/unit/test_status_web_hooks_github.py b/master/buildbot/test/unit/test_status_web_hooks_github.py index 54031f61385..0e63c89f6b3 100644 --- a/master/buildbot/test/unit/test_status_web_hooks_github.py +++ b/master/buildbot/test/unit/test_status_web_hooks_github.py @@ -13,13 +13,20 @@ # # Copyright Buildbot Team Members -import calendar +import hmac -import buildbot.status.web.change_hook as change_hook +from calendar import timegm +from hashlib import sha1 + +from buildbot.status.web.change_hook import ChangeHookResource +from buildbot.status.web.hooks.github import _HEADER_CT +from buildbot.status.web.hooks.github import _HEADER_EVENT +from buildbot.status.web.hooks.github import _HEADER_SIGNATURE from buildbot.test.fake.web import FakeRequest from buildbot.test.util import compat +from StringIO import StringIO from twisted.trial import unittest # Sample GITHUB commit payload from http://help.github.com/post-receive-hooks/ @@ -31,6 +38,7 @@ "repository": { "url": "http://github.com/defunkt/github", "name": "github", + "full_name": "defunkt/github", "description": "You're lookin' at it.", "watchers": 5, "forks": 2, @@ -77,6 +85,7 @@ "repository": { "url": "http://github.com/defunkt/github", "name": "github", + "full_name": "defunkt/github", "description": "You're lookin' at it.", "watchers": 5, "forks": 2, @@ -111,6 +120,7 @@ "repository": { "url": "http://github.com/defunkt/github", "name": "github", + "full_name": "defunkt/github", "description": "You're lookin' at it.", "watchers": 5, "forks": 2, @@ -127,113 +137,191 @@ } """ +_CT_ENCODED = 'application/x-www-form-urlencoded' +_CT_JSON = 'application/json' + + +def _prepare_github_change_hook(**params): + return ChangeHookResource(dialects={ + 'github': params + }) + + +def _prepare_request(event, payload, _secret=None, headers=None): + if headers is None: + headers = dict() + + request = FakeRequest() + + request.uri = "/change_hook/github" + request.method = "GET" + request.received_headers = { + _HEADER_EVENT: event + } + + if isinstance(payload, str): + request.content = StringIO(payload) + request.received_headers[_HEADER_CT] = _CT_JSON + + if _secret is not None: + signature = hmac.new(_secret, msg=payload, digestmod=sha1) + request.received_headers[_HEADER_SIGNATURE] = \ + 'sha1=%s' % (signature.hexdigest(),) + else: + request.args['payload'] = payload + request.received_headers[_HEADER_CT] = _CT_ENCODED + + request.received_headers.update(headers) + + # print request.received_headers + + return request + class TestChangeHookConfiguredWithGitChange(unittest.TestCase): def setUp(self): - self.changeHook = change_hook.ChangeHookResource( - dialects={'github': True}) + self.changeHook = _prepare_github_change_hook(strict=False) + + @compat.usesFlushLoggedErrors + def test_unknown_event(self): + bad_event = 'whatever' + + self.request = _prepare_request(bad_event, gitJsonPayload) + + d = self.request.test_render(self.changeHook) + + @d.addCallback + def check_result(result): + expected = 'Unknown event: %r' % (bad_event,) + self.assertEqual(len(self.request.addedChanges), 0) + self.assertEqual(self.request.written, expected) + + return d + + @compat.usesFlushLoggedErrors + def test_unknown_content_type(self): + bad_content_type = 'application/x-useful' + + self.request = _prepare_request('push', gitJsonPayload, headers={ + _HEADER_CT: bad_content_type + }) + + d = self.request.test_render(self.changeHook) + + @d.addCallback + def check_result(result): + expected = 'Unknown content type: %r' % (bad_content_type,) + self.assertEqual(len(self.request.addedChanges), 0) + self.assertEqual(self.request.written, expected) + + return d + + def _check_ping(self, payload): + self.request = _prepare_request('ping', payload) + + d = self.request.test_render(self.changeHook) + + @d.addCallback + def check_changes(_): + self.assertEqual(len(self.request.addedChanges), 0) + + return d + + def test_ping_encoded(self): + self._check_ping(['{}']) + + def test_ping_json(self): + self._check_ping('{}') # Test 'base' hook with attributes. We should get a json string # representing a Change object as a dictionary. All values show be set. - def testGitWithChange(self): - changeDict = {"payload": [gitJsonPayload]} - self.request = FakeRequest(changeDict) - self.request.uri = "/change_hook/github" - self.request.method = "GET" - self.request.received_headers = { - 'Content-Type': 'application/x-www-form-urlencoded', - 'X-GitHub-Event': 'push'} + def _check_git_with_change(self, payload): + self.request = _prepare_request('push', payload) + d = self.request.test_render(self.changeHook) - def check_changes(r): + @d.addCallback + def check_changes(_): self.assertEquals(len(self.request.addedChanges), 2) change = self.request.addedChanges[0] self.assertEquals(change['files'], ['filepath.rb']) - self.assertEquals( - change["repository"], "http://github.com/defunkt/github") - self.assertEquals( - calendar.timegm(change["when_timestamp"].utctimetuple()), - 1203116237 - ) - self.assertEquals( - change["author"], "Fred Flinstone ") - self.assertEquals( - change["revision"], '41a212ee83ca127e3c8cf465891ab7216a705f59') - self.assertEquals( - change["comments"], "okay i give in") - self.assertEquals( - change["branch"], "master") - self.assertEquals( - change["revlink"], - "http://github.com/defunkt/github/commit/" - "41a212ee83ca127e3c8cf465891ab7216a705f59" - ) + self.assertEquals(change["repository"], + "http://github.com/defunkt/github") + self.assertEquals(timegm(change["when_timestamp"].utctimetuple()), + 1203116237) + self.assertEquals(change["author"], + "Fred Flinstone ") + self.assertEquals(change["revision"], + '41a212ee83ca127e3c8cf465891ab7216a705f59') + self.assertEquals(change["comments"], + "okay i give in") + self.assertEquals(change["branch"], "master") + self.assertEquals(change["revlink"], + "http://github.com/defunkt/github/commit/" + "41a212ee83ca127e3c8cf465891ab7216a705f59") change = self.request.addedChanges[1] self.assertEquals(change['files'], ['modfile', 'removedFile']) - self.assertEquals( - change["repository"], "http://github.com/defunkt/github") - self.assertEquals( - calendar.timegm(change["when_timestamp"].utctimetuple()), - 1203114994 - ) - self.assertEquals( - change["author"], "Fred Flinstone ") + self.assertEquals(change["repository"], + "http://github.com/defunkt/github") + self.assertEquals(timegm(change["when_timestamp"].utctimetuple()), + 1203114994) + self.assertEquals(change["author"], + "Fred Flinstone ") self.assertEquals(change["src"], "git") - self.assertEquals( - change["revision"], 'de8251ff97ee194a289832576287d6f8ad74e3d0') + self.assertEquals(change["revision"], + 'de8251ff97ee194a289832576287d6f8ad74e3d0') self.assertEquals(change["comments"], "update pricing a tad") self.assertEquals(change["branch"], "master") - self.assertEquals( - change["revlink"], - "http://github.com/defunkt/github/commit/" - "de8251ff97ee194a289832576287d6f8ad74e3d0" - ) - - d.addCallback(check_changes) + self.assertEquals(change["revlink"], + "http://github.com/defunkt/github/commit/" + "de8251ff97ee194a289832576287d6f8ad74e3d0") return d + def test_git_with_change_encoded(self): + self._check_git_with_change([gitJsonPayload]) + + def test_git_with_change_json(self): + self._check_git_with_change(gitJsonPayload) + def testGitWithDistinctFalse(self): - changeDict = {"payload": [gitJsonPayload.replace( - '"distinct": true,', '"distinct": false,')] + changeDict = { + "payload": [gitJsonPayload.replace('"distinct": true,', + '"distinct": false,')] } self.request = FakeRequest(changeDict) self.request.uri = "/change_hook/github" self.request.method = "GET" self.request.received_headers = { - 'Content-Type': 'application/x-www-form-urlencoded', - 'X-GitHub-Event': 'push'} + _HEADER_CT: _CT_ENCODED, + _HEADER_EVENT: 'push' + } + d = self.request.test_render(self.changeHook) - def check_changes(r): + @d.addCallback + def check_changes(_): self.assertEqual(len(self.request.addedChanges), 1) change = self.request.addedChanges[0] - self.assertEqual( - change['files'], ['modfile', 'removedFile']) - self.assertEqual( - change["repository"], "http://github.com/defunkt/github") - self.assertEqual( - calendar.timegm(change["when_timestamp"].utctimetuple()), - 1203114994 - ) - self.assertEqual( - change["author"], "Fred Flinstone ") - self.assertEqual( - change["src"], "git") - self.assertEqual( - change["revision"], 'de8251ff97ee194a289832576287d6f8ad74e3d0') + self.assertEqual(change['files'], + ['modfile', 'removedFile']) + self.assertEqual(change["repository"], + "http://github.com/defunkt/github") + self.assertEqual(timegm(change["when_timestamp"].utctimetuple()), + 1203114994) + self.assertEqual(change["author"], + "Fred Flinstone ") + self.assertEqual(change["src"], "git") + self.assertEqual(change["revision"], + 'de8251ff97ee194a289832576287d6f8ad74e3d0') self.assertEqual(change["comments"], "update pricing a tad") self.assertEqual(change["branch"], "master") - self.assertEqual( - change["revlink"], - "http://github.com/defunkt/github/commit/" - "de8251ff97ee194a289832576287d6f8ad74e3d0" - ) - - d.addCallback(check_changes) + self.assertEqual(change["revlink"], + "http://github.com/defunkt/github/commit/" + "de8251ff97ee194a289832576287d6f8ad74e3d0") return d @compat.usesFlushLoggedErrors @@ -242,52 +330,192 @@ def testGitWithNoJson(self): self.request.uri = "/change_hook/github" self.request.method = "GET" self.request.received_headers = { - 'Content-Type': 'application/x-www-form-urlencoded', - 'X-GitHub-Event': 'push'} + _HEADER_CT: _CT_ENCODED, + _HEADER_EVENT: 'push' + } + d = self.request.test_render(self.changeHook) - def check_changes(r): + @d.addCallback + def check_changes(_): expected = "Error processing changes." self.assertEquals(len(self.request.addedChanges), 0) self.assertEqual(self.request.written, expected) self.request.setResponseCode.assert_called_with(500, expected) self.assertEqual(len(self.flushLoggedErrors()), 1) - - d.addCallback(check_changes) return d - def testGitWithNoChanges(self): - changeDict = {"payload": [gitJsonPayloadEmpty]} - self.request = FakeRequest(changeDict) - self.request.uri = "/change_hook/github" - self.request.method = "GET" - self.request.received_headers = { - 'Content-Type': 'application/x-www-form-urlencoded', - 'X-GitHub-Event': 'push'} + def _check_git_with_no_changes(self, payload): + self.request = _prepare_request('push', payload) + d = self.request.test_render(self.changeHook) - def check_changes(r): + @d.addCallback + def check_changes(_): expected = "no changes found" self.assertEquals(len(self.request.addedChanges), 0) self.assertEqual(self.request.written, expected) - - d.addCallback(check_changes) return d - def testGitWithNonBranchChanges(self): - changeDict = {"payload": [gitJsonPayloadNonBranch]} - self.request = FakeRequest(changeDict) - self.request.uri = "/change_hook/github" - self.request.method = "GET" - self.request.received_headers = { - 'Content-Type': 'application/x-www-form-urlencoded', - 'X-GitHub-Event': 'push'} + def test_git_with_no_changes_encoded(self): + self._check_git_with_no_changes([gitJsonPayloadEmpty]) + + def test_git_with_no_changes_json(self): + self._check_git_with_no_changes(gitJsonPayloadEmpty) + + def _check_git_with_non_branch_changes(self, payload): + self.request = _prepare_request('push', payload) + d = self.request.test_render(self.changeHook) - def check_changes(r): + @d.addCallback + def check_changes(_): expected = "no changes found" self.assertEquals(len(self.request.addedChanges), 0) self.assertEqual(self.request.written, expected) + return d + + def test_git_with_non_branch_changes_encoded(self): + self._check_git_with_non_branch_changes([gitJsonPayloadNonBranch]) + + def test_git_with_non_branch_changes_json(self): + self._check_git_with_non_branch_changes(gitJsonPayloadNonBranch) + + +class TestChangeHookConfiguredWithStrict(unittest.TestCase): + + _SECRET = 'somethingreallysecret' + + def setUp(self): + self.changeHook = _prepare_github_change_hook(strict=True, + secret=self._SECRET) + + def test_signature_ok(self): + self.request = _prepare_request('push', gitJsonPayload, + _secret=self._SECRET) + d = self.request.test_render(self.changeHook) + + # Can it somehow be merged w/ the same code above in a different class? + @d.addCallback + def check_changes(_): + self.assertEquals(len(self.request.addedChanges), 2) + change = self.request.addedChanges[0] + + self.assertEquals(change['files'], ['filepath.rb']) + self.assertEquals(change["repository"], + "http://github.com/defunkt/github") + self.assertEquals(timegm(change["when_timestamp"].utctimetuple()), + 1203116237) + self.assertEquals(change["author"], + "Fred Flinstone ") + self.assertEquals(change["revision"], + '41a212ee83ca127e3c8cf465891ab7216a705f59') + self.assertEquals(change["comments"], + "okay i give in") + self.assertEquals(change["branch"], "master") + self.assertEquals(change["revlink"], + "http://github.com/defunkt/github/commit/" + "41a212ee83ca127e3c8cf465891ab7216a705f59") + + change = self.request.addedChanges[1] + self.assertEquals(change['files'], ['modfile', 'removedFile']) + self.assertEquals(change["repository"], + "http://github.com/defunkt/github") + self.assertEquals(timegm(change["when_timestamp"].utctimetuple()), + 1203114994) + self.assertEquals(change["author"], + "Fred Flinstone ") + self.assertEquals(change["src"], "git") + self.assertEquals(change["revision"], + 'de8251ff97ee194a289832576287d6f8ad74e3d0') + self.assertEquals(change["comments"], "update pricing a tad") + self.assertEquals(change["branch"], "master") + self.assertEquals(change["revlink"], + "http://github.com/defunkt/github/commit/" + "de8251ff97ee194a289832576287d6f8ad74e3d0") + return d + + @compat.usesFlushLoggedErrors + def test_unknown_hash(self): + bad_hash_type = 'blah' + + self.request = _prepare_request('push', gitJsonPayload, headers={ + _HEADER_SIGNATURE: '%s=doesnotmatter' % (bad_hash_type,) + }) + + d = self.request.test_render(self.changeHook) + + @d.addCallback + def check_result(_): + expected = 'Unknown hash type: %s' % (bad_hash_type,) + self.assertEqual(len(self.request.addedChanges), 0) + self.assertEqual(self.request.written, expected) + + return d + + @compat.usesFlushLoggedErrors + def test_signature_nok(self): + bad_signature = 'sha1=wrongstuff' + + self.request = _prepare_request('push', gitJsonPayload, headers={ + _HEADER_SIGNATURE: bad_signature + }) + + d = self.request.test_render(self.changeHook) + + @d.addCallback + def check_result(_): + expected = 'Hash mismatch' + self.assertEqual(len(self.request.addedChanges), 0) + self.assertEqual(self.request.written, expected) + + return d + + @compat.usesFlushLoggedErrors + def test_missing_secret(self): + # override the value assigned in setUp + self.changeHook = _prepare_github_change_hook(strict=True) + + self.request = _prepare_request('push', gitJsonPayload) + + d = self.request.test_render(self.changeHook) + + @d.addCallback + def check_result(_): + expected = 'Strict mode is requested while no secret is provided' + self.assertEqual(len(self.request.addedChanges), 0) + self.assertEqual(self.request.written, expected) + + return d + + @compat.usesFlushLoggedErrors + def test_wrong_signature_format(self): + bad_signature = 'hash=value=something' + + self.request = _prepare_request('push', gitJsonPayload, headers={ + _HEADER_SIGNATURE: bad_signature + }) + + d = self.request.test_render(self.changeHook) + + @d.addCallback + def check_result(_): + expected = 'Wrong signature format: %r' % (bad_signature,) + self.assertEqual(len(self.request.addedChanges), 0) + self.assertEqual(self.request.written, expected) + + return d + + @compat.usesFlushLoggedErrors + def test_signature_missing(self): + self.request = _prepare_request('push', gitJsonPayload) + + d = self.request.test_render(self.changeHook) + + @d.addCallback + def check_result(result): + expected = 'Request has no required signature' + self.assertEqual(len(self.request.addedChanges), 0) + self.assertEqual(self.request.written, expected) - d.addCallback(check_changes) return d diff --git a/master/buildbot/test/unit/test_util.py b/master/buildbot/test/unit/test_util.py index 5f68146896e..7c05e45c966 100644 --- a/master/buildbot/test/unit/test_util.py +++ b/master/buildbot/test/unit/test_util.py @@ -60,6 +60,40 @@ def test_mixed(self): self.assertEqual(util.formatInterval(7392), "2 hrs, 3 mins, 12 secs") +class TestHumanReadableDelta(unittest.TestCase): + + def test_timeDeltaToHumanReadable(self): + """ + It will return a human readable time difference. + """ + result = util.human_readable_delta(1, 1) + self.assertEqual('super fast', result) + + result = util.human_readable_delta(1, 2) + self.assertEqual('1 seconds', result) + + result = util.human_readable_delta(1, 61) + self.assertEqual('1 minutes', result) + + result = util.human_readable_delta(1, 62) + self.assertEqual('1 minutes, 1 seconds', result) + + result = util.human_readable_delta(1, 60 * 60 + 1) + self.assertEqual('1 hours', result) + + result = util.human_readable_delta(1, 60 * 60 + 61) + self.assertEqual('1 hours, 1 minutes', result) + + result = util.human_readable_delta(1, 60 * 60 + 62) + self.assertEqual('1 hours, 1 minutes, 1 seconds', result) + + result = util.human_readable_delta(1, 24 * 60 * 60 + 1) + self.assertEqual('1 days', result) + + result = util.human_readable_delta(1, 24 * 60 * 60 + 2) + self.assertEqual('1 days, 1 seconds', result) + + class safeTranslate(unittest.TestCase): def test_str_good(self): diff --git a/master/buildbot/util/__init__.py b/master/buildbot/util/__init__.py index 85aad01c808..55ef96ebf63 100644 --- a/master/buildbot/util/__init__.py +++ b/master/buildbot/util/__init__.py @@ -202,6 +202,35 @@ def datetime2epoch(dt): return calendar.timegm(dt.utctimetuple()) +# TODO: maybe "merge" with formatInterval? +def human_readable_delta(start, end): + """ + Return a string of human readable time delta. + """ + start_date = datetime.datetime.fromtimestamp(start) + end_date = datetime.datetime.fromtimestamp(end) + delta = end_date - start_date + + result = [] + if delta.days > 0: + result.append('%d days' % (delta.days,)) + if delta.seconds > 0: + hours = delta.seconds / 3600 + if hours > 0: + result.append('%d hours' % (hours,)) + minutes = (delta.seconds - hours * 3600) / 60 + if minutes: + result.append('%d minutes' % (minutes,)) + seconds = delta.seconds % 60 + if seconds > 0: + result.append('%d seconds' % (seconds,)) + + if result: + return ', '.join(result) + else: + return 'super fast' + + def makeList(input): if isinstance(input, basestring): return [input] @@ -258,4 +287,5 @@ def check_functional_environment(config): 'naturalSort', 'now', 'formatInterval', 'ComparableMixin', 'json', 'safeTranslate', 'none_or_str', 'NotABranch', 'deferredLocked', 'SerializedInvocation', 'UTC', - 'diffSets', 'makeList', 'in_reactor', 'check_functional_environment'] + 'diffSets', 'makeList', 'in_reactor', 'check_functional_environment', + 'human_readable_delta'] diff --git a/master/contrib/github_buildbot.py b/master/contrib/github_buildbot.py index 8f3aad719b7..f768d5cf7ef 100755 --- a/master/contrib/github_buildbot.py +++ b/master/contrib/github_buildbot.py @@ -13,13 +13,18 @@ """ +import hmac import logging import os import sys -import tempfile -import traceback +from hashlib import sha1 +from httplib import ACCEPTED +from httplib import BAD_REQUEST +from httplib import INTERNAL_SERVER_ERROR +from httplib import OK from optparse import OptionParser + from twisted.cred import credentials from twisted.internet import reactor from twisted.spread import pb @@ -31,8 +36,6 @@ except ImportError: import simplejson as json -# - class GitHubBuildBot(resource.Resource): @@ -52,8 +55,79 @@ def render_POST(self, request): request the http request object """ + + # All responses are application/json + request.setHeader("Content-Type", "application/json") + + content = request.content.read() + + # Verify the message if a secret was provided + # + # NOTE: We always respond with '400 BAD REQUEST' if we can't + # validate the message. This is done to prevent malicious + # requests from learning about why they failed to POST data + # to us. + if self.secret is not None: + signature = request.getHeader("X-Hub-Signature") + + if signature is None: + logging.error("Rejecting request. Signature is missing.") + request.setResponseCode(BAD_REQUEST) + return json.dumps({"error": "Bad Request."}) + + try: + hash_type, hexdigest = signature.split("=") + + except ValueError: + logging.error("Rejecting request. Bad signature format.") + request.setResponseCode(BAD_REQUEST) + return json.dumps({"error": "Bad Request."}) + + else: + # sha1 is hard coded into github's source code so it's + # unlikely this will ever change. + if hash_type != "sha1": + logging.error("Rejecting request. Unexpected hash type.") + request.setResponseCode(BAD_REQUEST) + return json.dumps({"error": "Bad Request."}) + + mac = hmac.new(self.secret, msg=content, digestmod=sha1) + if mac.hexdigest() != hexdigest: + logging.error("Rejecting request. Hash mismatch.") + request.setResponseCode(BAD_REQUEST) + return json.dumps({"error": "Bad Request."}) + + event_type = request.getHeader("X-GitHub-Event") + logging.debug("X-GitHub-Event: %r", event_type) + + if event_type == "ping": + request.setResponseCode(OK) + return json.dumps({"result": "pong"}) + + # Reject non-push, non-ping events + if event_type != "push": + logging.info( + "Rejecting request. Expected a push event but received %r instead.", + event_type) + request.setResponseCode(BAD_REQUEST) + return json.dumps({"error": "Bad Request."}) + try: - payload = json.loads(request.content.read()) + + content_type = request.getHeader("Content-Type") + + if content_type == "application/json": + payload = json.loads(content) + elif content_type == "application/x-www-form-urlencoded": + payload = json.loads(request.args["payload"][0]) + else: + logging.info( + "Rejecting request. Unknown 'Content-Type', received %r", + content_type) + request.setResponseCode(BAD_REQUEST) + return json.dumps({"error": "Bad Request."}) + + logging.debug("Payload: %r", payload) user = payload['pusher']['name'] repo = payload['repository']['name'] repo_url = payload['repository']['url'] @@ -61,14 +135,15 @@ def render_POST(self, request): project = request.args.get('project', None) if project: project = project[0] - logging.debug("Payload: " + str(payload)) - self.process_change(payload, user, repo, repo_url, project) - except Exception: - logging.error("Encountered an exception:") - for msg in traceback.format_exception(*sys.exc_info()): - logging.error(msg.strip()) - - def process_change(self, payload, user, repo, repo_url, project): + self.process_change(payload, user, repo, repo_url, project, request) + return server.NOT_DONE_YET + + except Exception, e: + logging.exception(e) + request.setResponseCode(INTERNAL_SERVER_ERROR) + return json.dumps({"error": e.message}) + + def process_change(self, payload, user, repo, repo_url, project, request): """ Consumes the JSON as a python object and actually starts the build. @@ -77,49 +152,67 @@ def process_change(self, payload, user, repo, repo_url, project): Python Object that represents the JSON sent by GitHub Service Hook. """ - + changes = None branch = payload['ref'].split('/')[-1] if payload['deleted'] is True: - logging.info("Branch `%s' deleted, ignoring", branch) + logging.info("Branch %r deleted, ignoring", branch) else: - changes = [{'revision': c['id'], - 'revlink': c['url'], - 'who': c['author']['username'] + " <" + c['author']['email'] + "> ", - 'comments': c['message'], - 'repository': payload['repository']['url'], - 'files': c['added'] + c['removed'] + c['modified'], - 'project': project, - 'branch': branch} - for c in payload['commits']] + changes = [] + + for change in payload['commits']: + files = change['added'] + change['removed'] + change['modified'] + who = "%s <%s>" % ( + change['author']['username'], change['author']['email']) + + changes.append( + {'revision': change['id'], + 'revlink': change['url'], + 'who': who, + 'comments': change['message'], + 'repository': payload['repository']['url'], + 'files': files, + 'project': project, + 'branch': branch}) if not changes: logging.warning("No changes found") + request.setResponseCode(OK) + request.write(json.dumps({"result": "No changes found."})) + request.finish() return host, port = self.master.split(':') port = int(port) + if self.auth is not None: + auth = credentials.UsernamePassword(*self.auth.split(":")) + else: + auth = credentials.Anonymous() + factory = pb.PBClientFactory() - deferred = factory.login(credentials.UsernamePassword("change", - "changepw")) + deferred = factory.login(auth) reactor.connectTCP(host, port, factory) - deferred.addErrback(self.connectFailed) - deferred.addCallback(self.connected, changes) + deferred.addErrback(self.connectFailed, request) + deferred.addCallback(self.connected, changes, request) - def connectFailed(self, error): + def connectFailed(self, error, request): """ If connection is failed. Logs the error. """ logging.error("Could not connect to master: %s", error.getErrorMessage()) + request.setResponseCode(INTERNAL_SERVER_ERROR) + request.write( + json.dumps({"error": "Failed to connect to buildbot master."})) + request.finish() return error - def addChange(self, dummy, remote, changei, src='git'): + def addChange(self, _, remote, changei, src='git'): """ Sends changes from the commit to the buildmaster. """ - logging.debug("addChange %s, %s", repr(remote), repr(changei)) + logging.debug("addChange %r, %r", remote, changei) try: change = changei.next() except StopIteration: @@ -135,10 +228,17 @@ def addChange(self, dummy, remote, changei, src='git'): deferred.addCallback(self.addChange, remote, changei, src) return deferred - def connected(self, remote, changes): + def connected(self, remote, changes, request): """ Responds to the connected event. """ + # By this point we've connected to buildbot so + # we don't really need to keep github waiting any + # longer + request.setResponseCode(ACCEPTED) + request.write(json.dumps({"result": "Submitting changes."})) + request.finish() + return self.addChange(None, remote, changes.__iter__()) @@ -150,54 +250,70 @@ def setup_options(): parser = OptionParser(usage) parser.add_option("-p", "--port", - help="Port the HTTP server listens to for the GitHub Service Hook" - + " [default: %default]", default=9001, type=int, dest="port") + help="Port the HTTP server listens to for the GitHub " + "Service Hook [default: %default]", + default=9001, type=int, dest="port") parser.add_option("-m", "--buildmaster", - help="Buildbot Master host and port. ie: localhost:9989 [default:" - + " %default]", default="10.108.0.6:9989", dest="buildmaster") + help="Buildbot Master host and port. ie: localhost:9989 " + "[default: %default]", + default="localhost:9989", dest="buildmaster") + + parser.add_option("--auth", + help="The username and password, separated by a colon, " + "to use when connecting to buildbot over the " + "perspective broker.", + default="change:changepw", dest="auth") + + parser.add_option("--secret", + help="If provided then use the X-Hub-Signature header " + "to verify that the request is coming from " + "github. [default: %default]", + default=None, dest="secret") parser.add_option("-l", "--log", - help="The absolute path, including filename, to save the log to" - + " [default: %default]", - default=tempfile.gettempdir() + "/github_buildbot.log", - dest="log") + help="The absolute path, including filename, to save the " + "log to [default: %default]. This may also be " + "'stdout' indicating logs should output directly to " + "standard output instead.", + default="github_buildbot.log", dest="log") parser.add_option("-L", "--level", - help="The logging level: debug, info, warn, error, fatal [default:" - + " %default]", default='warn', dest="level") + help="The logging level: debug, info, warn, error, " + "fatal [default: %default]", default='warn', + dest="level", + choices=("debug", "info", "warn", "error", "fatal")) parser.add_option("-g", "--github", - help="The github server. Changing this is useful if you've specified" - + " a specific HOST handle in ~/.ssh/config for github " - + "[default: %default]", default='github.com', - dest="github") + help="The github server. Changing this is useful if" + " you've specified a specific HOST handle in " + "~/.ssh/config for github [default: %default]", + default='github.com', dest="github") parser.add_option("--pidfile", - help="Write the process identifier (PID) to this file on start." - + - " The file is removed on clean exit. [default: %default]", - default=None, - dest="pidfile") + help="Write the process identifier (PID) to this " + "file on start. The file is removed on clean " + "exit. [default: %default]", + default=None, dest="pidfile") (options, _) = parser.parse_args() + if options.auth is not None and ":" not in options.auth: + parser.error("--auth did not contain ':'") + if options.pidfile: with open(options.pidfile, 'w') as f: f.write(str(os.getpid())) - levels = { - 'debug': logging.DEBUG, - 'info': logging.INFO, - 'warn': logging.WARNING, - 'error': logging.ERROR, - 'fatal': logging.FATAL, - } - filename = options.log log_format = "%(asctime)s - %(levelname)s - %(message)s" - logging.basicConfig(filename=filename, format=log_format, - level=levels[options.level]) + if options.log != "stdout": + logging.basicConfig(filename=filename, format=log_format, + level=logging._levelNames[options.level.upper()]) + else: + logging.basicConfig(format=log_format, + handlers=[logging.StreamHandler(stream=sys.stdout)], + level=logging._levelNames[options.level.upper()]) return options @@ -206,6 +322,8 @@ def run_hook(options): github_bot = GitHubBuildBot() github_bot.github = options.github github_bot.master = options.buildmaster + github_bot.secret = options.secret + github_bot.auth = options.auth site = server.Site(github_bot) reactor.listenTCP(options.port, site) diff --git a/master/docs/manual/cfg-statustargets.rst b/master/docs/manual/cfg-statustargets.rst index b3315104a86..49191127b5b 100644 --- a/master/docs/manual/cfg-statustargets.rst +++ b/master/docs/manual/cfg-statustargets.rst @@ -101,7 +101,7 @@ If you are running the buildbot behind a reverse proxy, you'll probably need to If you would like to use an alternative root directory, add the ``public_html=`` option to the :class:`WebStatus` creation:: - c['status'].append(WebStatus(8080, public_html="/var/www/buildbot")) + c['status'].append(status.WebStatus(8080, public_html="/var/www/buildbot")) In addition, if you are familiar with twisted.web *Resource Trees*, you can write code to add additional pages at places inside this web space. Just use :meth:`webstatus.putChild` to place these resources. @@ -198,7 +198,7 @@ The table below lists all of the internal pages and the URLs that can be used to If you do have multiple branches, you should use the ``branch=`` query argument. The ``order_console_by_time`` option may help sorting revisions, although it depends on the date being set correctly in each commit:: - w = html.WebStatus(http_port=8080, order_console_by_time=True) + w = status.WebStatus(http_port=8080, order_console_by_time=True) ``/rss`` This provides a rss feed summarizing all failed builds. @@ -299,7 +299,7 @@ The table below lists all of the internal pages and the URLs that can be used to This page gives a brief summary of the Buildbot itself: software version, versions of some libraries that the Buildbot depends upon, etc. It also contains a link to the buildbot.net home page. -There are also a set of web-status resources that are intended for use by other programs, rather than humans. +There is also a set of web-status resources that are intended for use by other programs, rather than humans. ``/change_hook`` This provides an endpoint for web-based source change notification. @@ -571,11 +571,11 @@ Change_hook is disabled by default and each DIALECT has to be enabled separately An example WebStatus configuration line which enables change_hook and two DIALECTS:: - c['status'].append(html.WebStatus(http_port=8010,allowForce=True, + c['status'].append(status.WebStatus(http_port=8010, allowForce=True, change_hook_dialects={ 'base': True, - 'somehook': {'option1':True, - 'option2':False}})) + 'somehook': {'option1': True, + 'option2': False}})) Within the WebStatus arguments, the ``change_hook`` key enables/disables the module and ``change_hook_dialects`` whitelists DIALECTs where the keys are the module names and the values are optional arguments which will be passed to the hooks. @@ -583,6 +583,8 @@ The :file:`post_build_request.py` script in :file:`master/contrib` allows for th Run :command:`post_build_request.py --help` for more information. The ``base`` dialect must be enabled for this to work. +.. _GitHub-hook: + GitHub hook ########### @@ -591,13 +593,38 @@ GitHub hook There is a standalone HTTP server available for receiving GitHub notifications as well: :file:`contrib/github_buildbot.py`. This script may be useful in cases where you cannot expose the WebStatus for public consumption. -The GitHub hook is simple and takes no options: +The GitHub hook has the following parameters: + +``secret`` (default `None`) + Secret token to use to validate payloads +``strict`` (default `False`) + If the hook must be strict regarding valid payloads. + If the value is `False` (default), the signature will only be checked if a secret is specified and a signature was supplied with the payload. + If the value is `True`, a secret must be provided, and payloads without signature will be ignored. +``codebase`` (default `None`) + The codebase value to include with created changes. +``class`` (default `None`) + A class to be used for processing incoming payloads. + If the value is `None` (default), the default class -- :py:class:`buildbot.status.web.hooks.github.GitHubEventHandler` -- will be used. + The default class handles `ping` and `push` events only. + If you'd like to handle other events (see `Event Types & Payloads `_ for more information), you'd need to subclass `GitHubEventHandler` and add handler methods for the corresponding events. + For example, if you'd like to handle `blah` events, your code should look something like this:: + + from buildbot.status.web.hooks.github import GitHubEventHandler + + class MyBlahHandler(GitHubEventHandler): + + def handle_blah(self, payload): + # Do some magic here + return [], 'git' + +The simples way to use GitHub hook is as follows: .. code-block:: python - c['status'].append(html.WebStatus(..., - change_hook_dialects={'github': True}, - ...)) + c['status'].append(status.WebStatus(..., + change_hook_dialects={'github': {}}, + ...)) Having added this line, you should add a webhook for your GitHub project (see `Creating Webhooks page at GitHub `_). The parameters are: @@ -613,7 +640,16 @@ The parameters are: :guilabel:`Secret` Any value. - Currently this parameter is not supported. + If you provide a non-empty value (recommended), make sure that your hook is configured to use it:: + + c['status'].append(status.WebStatus(..., + change_hook_dialects={ + 'github': { + 'secret': 'MY-SECRET', + 'strict': True + } + }, + ...)) :guilabel:`Which events would you like to trigger this webhook?` Leave the default -- ``Just the push event`` -- other kind of events are not currently supported. @@ -623,22 +659,22 @@ And then press the ``Add Webhook`` button. .. warning:: The incoming HTTP requests for this hook are not authenticated by default. - Anyone who can access the web status can "fake" a request from GitHub, potentially causing the buildmaster to run arbitrary code. + If you do not specify a secret, anyone who can access the web status can "fake" a request from GitHub, potentially causing the buildmaster to run arbitrary code. -To protect URL against unauthorized access you should use ``change_hook_auth`` option:: +To protect URL against unauthorized access you either specify a secret, or you should use ``change_hook_auth`` option:: - c['status'].append(html.WebStatus(..., + c['status'].append(status.WebStatus(..., change_hook_auth=["file:changehook.passwd"], ... )) -And create a file ``changehook.passwd`` +create a file ``changehook.passwd``: .. code-block:: none user:password -Then change the the ``Payload URL`` of your GitHub webhook to ``http://user:password@builds.example.com/bbot/change_hook/github``. +and change the the ``Payload URL`` of your GitHub webhook to ``http://user:password@builds.example.com/bbot/change_hook/github``. See the `documentation for twisted cred `_ for more options to pass to ``change_hook_auth``. @@ -651,8 +687,8 @@ The BitBucket hook is as simple as GitHub one and it also takes no options. :: - c['status'].append(html.WebStatus(..., - change_hook_dialects={ 'bitbucket' : True })) + c['status'].append(status.WebStatus(..., + change_hook_dialects={'bitbucket': True})) When this is setup you should add a `POST` service pointing to ``/change_hook/bitbucket`` relative to the root of the web status. For example, it the grid URL is ``http://builds.mycompany.com/bbot/grid``, then point BitBucket to ``http://builds.mycompany.com/change_hook/bitbucket``. @@ -670,7 +706,7 @@ To protect URL against unauthorized access you should use ``change_hook_auth`` o :: - c['status'].append(html.WebStatus(..., + c['status'].append(status.WebStatus(..., change_hook_auth=["file:changehook.passwd"])) Then, create a BitBucket service hook (see https://confluence.atlassian.com/display/BITBUCKET/POST+Service+Management) with a WebHook URL like ``http://user:password@builds.mycompany.com/bbot/change_hook/bitbucket``. @@ -683,7 +719,7 @@ Google Code hook The Google Code hook is quite similar to the GitHub Hook. It has one option for the "Post-Commit Authentication Key" used to check if the request is legitimate:: - c['status'].append(html.WebStatus( + c['status'].append(status.WebStatus( # ... change_hook_dialects={'googlecode': {'secret_key': 'FSP3p-Ghdn4T0oqX'}} )) @@ -715,7 +751,7 @@ Suppose you have a poller configured like this:: And you configure your WebStatus to enable this hook:: - c['status'].append(html.WebStatus( + c['status'].append(status.WebStatus( # ... change_hook_dialects={'poller': True} )) @@ -731,7 +767,7 @@ If no ``poller`` argument is provided then the hook will trigger polling of all You can restrict which pollers the webhook has access to using the ``allowed`` option:: - c['status'].append(html.WebStatus( + c['status'].append(status.WebStatus( # ... change_hook_dialects={'poller': {'allowed': ['https://amanda.svn.sourceforge.net/svnroot/amanda/amanda']}} )) @@ -743,7 +779,7 @@ The GitLab hook is as simple as GitHub one and it also takes no options. :: - c['status'].append(html.WebStatus( + c['status'].append(status.WebStatus( # ... change_hook_dialects={ 'gitlab' : True } )) @@ -762,7 +798,7 @@ To protect URL against unauthorized access you should use ``change_hook_auth`` o :: - c['status'].append(html.WebStatus( + c['status'].append(status.WebStatus( # ... change_hook_auth=["file:changehook.passwd"] )) @@ -778,7 +814,7 @@ The Gitorious hook is as simple as GitHub one and it also takes no options. :: - c['status'].append(html.WebStatus( + c['status'].append(status.WebStatus( # ... change_hook_dialects={'gitorious': True} )) @@ -795,7 +831,7 @@ To protect URL against unauthorized access you should use ``change_hook_auth`` o :: - c['status'].append(html.WebStatus( + c['status'].append(status.WebStatus( # ... change_hook_auth=["file:changehook.passwd"] )) @@ -1561,6 +1597,10 @@ In case any of `repoOwner`, `repoName` or `sha` returns `None`, `False` or empty You can define custom start and end build messages using the `startDescription` and `endDescription` optional interpolation arguments. +Starting with Buildbot version 0.8.11, :class:`GitHubStatus` supports additional parameter -- ``baseURL`` -- that allows to specify a different API base endpoint. +This is required if you work with GitHub Enterprise installation. +This feature requires ``txgithub`` of version 0.2.0 or better. + .. [#] Apparently this is the same way http://buildd.debian.org displays build status .. [#] It may even be possible to provide SSL access by using a diff --git a/master/docs/relnotes/index.rst b/master/docs/relnotes/index.rst index 2aba83f220c..a23db31dcdc 100644 --- a/master/docs/relnotes/index.rst +++ b/master/docs/relnotes/index.rst @@ -74,6 +74,11 @@ Features * :bb:step:`MasterShellCommand` now renders the ``path`` argument. +* GitHub status target now allows to specify a different base URL for the API (usefule for GitHub enterprise installations). + This feature requires `txgithub` of version 0.2.0 or better. + +* GitHub change hook now supports payload validation using shared secret, see :ref:`GitHub-hook` for details. + Fixes ~~~~~