# HG changeset patch # User osimons # Date 1251153668 0 # Node ID eed0149c302a147e0b91c7a61ae4ff4ff709ccb0 # Parent a04e46a0bce39c336781bada32b1f03cccfb4408 0.6dev: Switching to use the new cookie-support, and using trac auth/session ID as unique identification for linking builds with slaves. This overcomes problems with IP address not being unique behind NAT, and also where IP address may change during a build. Closes #421. Additionally it introduces a version for the master-slave protocol to ensure that slaves are at a version that works with the master. Closes #223. diff --git a/bitten/master.py b/bitten/master.py --- a/bitten/master.py +++ b/bitten/master.py @@ -27,6 +27,7 @@ from bitten.main import BuildSystem from bitten.queue import BuildQueue from bitten.recipe import Recipe +from bitten.slave import PROTOCOL_VERSION from bitten.util import xmlio __all__ = ['BuildMaster'] @@ -91,16 +92,28 @@ def process_request(self, req): req.perm.assert_permission('BUILD_EXEC') + if 'trac_auth' in req.incookie: + slave_token = req.incookie['trac_auth'].value + else: + slave_token = req.session.sid + if 'id' not in req.args: if req.method != 'POST': self._send_error(req, HTTP_METHOD_NOT_ALLOWED, 'Only POST allowed for build creation') - return self._process_build_creation(req) + return self._process_build_creation(req, slave_token) build = Build.fetch(self.env, req.args['id']) if not build: self._send_error(req, HTTP_NOT_FOUND, 'No such build (%s)' % req.args['id']) + + build_token = build.slave_info.get('token', '') + if build_token != slave_token: + self._send_error(req, HTTP_CONFLICT, + 'Token mismatch (wrong slave): slave=%s, build=%s' \ + % (slave_token, build_token)) + config = BuildConfig.fetch(self.env, build.config) if not req.args['collection']: @@ -136,7 +149,7 @@ 'Content-Length': str(len(message))} self._send_response(req, code, body=message, headers=headers) - def _process_build_creation(self, req): + def _process_build_creation(self, req, slave_token): queue = BuildQueue(self.env, build_all=self.build_all, stabilize_wait=self.stabilize_wait, timeout=self.slave_timeout) @@ -149,10 +162,17 @@ exc_info=True) self._send_error(req, HTTP_BAD_REQUEST, 'XML parser error') + slave_version = int(elem.attr.get('version', 1)) + if slave_version != PROTOCOL_VERSION: + self._send_error(req, HTTP_BAD_REQUEST, + "Master-Slave version mismatch: master=%d, slave=%d" % \ + (PROTOCOL_VERSION, slave_version)) + slavename = elem.attr['name'] - properties = {'name': slavename, Build.IP_ADDRESS: req.remote_addr} - self.log.info('Build slave %r connected from %s', slavename, - req.remote_addr) + properties = {'name': slavename, Build.IP_ADDRESS: req.remote_addr, + Build.TOKEN: slave_token} + self.log.info('Build slave %r connected from %s with token %s', + slavename, req.remote_addr, slave_token) for child in elem.children(): if child.name == 'platform': @@ -235,10 +255,9 @@ stepname = elem.attr['step'] # make sure it's the right slave. - if build.status != Build.IN_PROGRESS or \ - build.slave_info.get(Build.IP_ADDRESS) != req.remote_addr: - self._send_error(req, HTTP_FORBIDDEN, - 'Build %s has been invalidated for host %s.' + if build.status != Build.IN_PROGRESS: + self._send_error(req, HTTP_CONFLICT, + 'Build %s has been invalidated for host %s.' \ % (build.id, req.remote_addr)) step = BuildStep.fetch(self.env, build=build.id, name=stepname) diff --git a/bitten/model.py b/bitten/model.py --- a/bitten/model.py +++ b/bitten/model.py @@ -362,6 +362,7 @@ OS_VERSION = 'version' MACHINE = 'machine' PROCESSOR = 'processor' + TOKEN = 'token' def __init__(self, env, config=None, rev=None, platform=None, slave=None, started=0, stopped=0, rev_time=0, status=PENDING): diff --git a/bitten/slave.py b/bitten/slave.py --- a/bitten/slave.py +++ b/bitten/slave.py @@ -32,6 +32,9 @@ from bitten.recipe import Recipe from bitten.util import xmlio +# The master-slave protocol/configuration version +PROTOCOL_VERSION = 2 + EX_OK = getattr(os, "EX_OK", 0) EX_UNAVAILABLE = getattr(os, "EX_UNAVAILABLE", 69) EX_PROTOCOL = getattr(os, "EX_PROTOCOL", 76) @@ -258,7 +261,7 @@ raise ExitSlave(EX_OK) def _create_build(self, url): - xml = xmlio.Element('slave', name=self.name)[ + xml = xmlio.Element('slave', name=self.name, version=PROTOCOL_VERSION)[ xmlio.Element('platform', processor=self.config['processor'])[ self.config['machine'] ], diff --git a/bitten/tests/master.py b/bitten/tests/master.py --- a/bitten/tests/master.py +++ b/bitten/tests/master.py @@ -14,6 +14,7 @@ from StringIO import StringIO import tempfile import unittest +from Cookie import SimpleCookie as Cookie from trac.db import DatabaseManager from trac.perm import PermissionCache, PermissionSystem @@ -25,7 +26,7 @@ from bitten.master import BuildMaster from bitten.model import BuildConfig, TargetPlatform, Build, BuildStep, \ BuildLog, Report, schema - +from bitten.slave import PROTOCOL_VERSION class BuildMasterTestCase(unittest.TestCase): @@ -73,11 +74,11 @@ ) inheaders = {'Content-Type': 'application/x-bitten+xml'} - inbody = StringIO(""" + inbody = StringIO(""" Power Macintosh Darwin -""") +""" % PROTOCOL_VERSION) outheaders = {} outbody = StringIO() req = Mock(method='POST', base_path='', path_info='/builds', @@ -87,7 +88,8 @@ get_header=lambda x: inheaders.get(x), read=inbody.read, send_response=lambda x: outheaders.setdefault('Status', x), send_header=lambda x, y: outheaders.setdefault(x, y), - write=outbody.write) + write=outbody.write, + incookie=Cookie('trac_auth=')) module = BuildMaster(self.env) assert module.match_request(req) @@ -115,7 +117,8 @@ get_header=lambda x: inheaders.get(x), read=inbody.read, send_response=lambda x: outheaders.setdefault('Status', x), send_header=lambda x, y: outheaders.setdefault(x, y), - write=outbody.write) + write=outbody.write, + incookie=Cookie('trac_auth=')) module = BuildMaster(self.env) assert module.match_request(req) @@ -133,7 +136,8 @@ perm=PermissionCache(self.env, 'hal'), send_response=lambda x: outheaders.setdefault('Status', x), send_header=lambda x, y: outheaders.setdefault(x, y), - write=outbody.write) + write=outbody.write, + incookie=Cookie('trac_auth=')) module = BuildMaster(self.env) assert module.match_request(req) @@ -146,6 +150,58 @@ def test_create_build_no_match(self): inheaders = {'Content-Type': 'application/x-bitten+xml'} + inbody = StringIO(""" + Power Macintosh + Darwin +""" % PROTOCOL_VERSION) + outheaders = {} + outbody = StringIO() + req = Mock(method='POST', base_path='', path_info='/builds', + href=Href('/trac'), remote_addr='127.0.0.1', args={}, + perm=PermissionCache(self.env, 'hal'), + get_header=lambda x: inheaders.get(x), read=inbody.read, + send_response=lambda x: outheaders.setdefault('Status', x), + send_header=lambda x, y: outheaders.setdefault(x, y), + write=outbody.write, + incookie=Cookie('trac_auth=')) + + module = BuildMaster(self.env) + assert module.match_request(req) + + self.assertRaises(RequestDone, module.process_request, req) + + self.assertEqual(204, outheaders['Status']) + self.assertEqual('', outbody.getvalue()) + + def test_create_build_protocol_wrong_version(self): + inheaders = {'Content-Type': 'application/x-bitten+xml'} + inbody = StringIO(""" + Power Macintosh + Darwin +""" % (PROTOCOL_VERSION-1,)) + outheaders = {} + outbody = StringIO() + req = Mock(method='POST', base_path='', path_info='/builds', + href=Href('/trac'), remote_addr='127.0.0.1', args={}, + perm=PermissionCache(self.env, 'hal'), + get_header=lambda x: inheaders.get(x), read=inbody.read, + send_response=lambda x: outheaders.setdefault('Status', x), + send_header=lambda x, y: outheaders.setdefault(x, y), + write=outbody.write, + incookie=Cookie('trac_auth=')) + + module = BuildMaster(self.env) + assert module.match_request(req) + + self.assertRaises(RequestDone, module.process_request, req) + + self.assertEqual(400, outheaders['Status']) + self.assertEqual('Master-Slave version mismatch: master=%d, slave=%d' \ + % (PROTOCOL_VERSION, PROTOCOL_VERSION-1), + outbody.getvalue()) + + def test_create_build_protocol_no_version(self): + inheaders = {'Content-Type': 'application/x-bitten+xml'} inbody = StringIO(""" Power Macintosh Darwin @@ -158,15 +214,18 @@ get_header=lambda x: inheaders.get(x), read=inbody.read, send_response=lambda x: outheaders.setdefault('Status', x), send_header=lambda x, y: outheaders.setdefault(x, y), - write=outbody.write) + write=outbody.write, + incookie=Cookie('trac_auth=')) module = BuildMaster(self.env) assert module.match_request(req) self.assertRaises(RequestDone, module.process_request, req) - self.assertEqual(204, outheaders['Status']) - self.assertEqual('', outbody.getvalue()) + self.assertEqual(400, outheaders['Status']) + self.assertEqual('Master-Slave version mismatch: master=%d, slave=1' \ + % (PROTOCOL_VERSION,), + outbody.getvalue()) def test_cancel_build(self): config = BuildConfig(self.env, 'test', path='somepath', active=True, @@ -184,7 +243,8 @@ perm=PermissionCache(self.env, 'hal'), send_response=lambda x: outheaders.setdefault('Status', x), send_header=lambda x, y: outheaders.setdefault(x, y), - write=outbody.write) + write=outbody.write, + incookie=Cookie('trac_auth=')) module = BuildMaster(self.env) assert module.match_request(req) @@ -219,7 +279,8 @@ perm=PermissionCache(self.env, 'hal'), send_response=lambda x: outheaders.setdefault('Status', x), send_header=lambda x, y: outheaders.setdefault(x, y), - write=outbody.write) + write=outbody.write, + incookie=Cookie('trac_auth=')) module = BuildMaster(self.env) assert module.match_request(req) @@ -250,7 +311,8 @@ perm=PermissionCache(self.env, 'hal'), send_response=lambda x: outheaders.setdefault('Status', x), send_header=lambda x, y: outheaders.setdefault(x, y), - write=outbody.write) + write=outbody.write, + incookie=Cookie('trac_auth=')) module = BuildMaster(self.env) assert module.match_request(req) @@ -274,7 +336,8 @@ perm=PermissionCache(self.env, 'hal'), send_response=lambda x: outheaders.setdefault('Status', x), send_header=lambda x, y: outheaders.setdefault(x, y), - write=outbody.write) + write=outbody.write, + incookie=Cookie('trac_auth=')) module = BuildMaster(self.env) assert module.match_request(req) @@ -293,7 +356,7 @@ recipe=recipe).insert() build = Build(self.env, 'test', '123', 1, slave='hal', rev_time=42, started=42, status=Build.IN_PROGRESS) - build.slave_info[Build.IP_ADDRESS] = '127.0.0.1'; + build.slave_info[Build.TOKEN] = '123'; build.insert() inbody = StringIO("""