changeset 649:eed0149c302a

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.
author osimons
date Mon, 24 Aug 2009 22:41:08 +0000
parents a04e46a0bce3
children b1a50f2d92eb
files bitten/master.py bitten/model.py bitten/slave.py bitten/tests/master.py
diffstat 4 files changed, 145 insertions(+), 47 deletions(-) [+]
line wrap: on
line diff
--- 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)
--- 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):
--- 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']
             ],
--- 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("""<slave name="hal">
+        inbody = StringIO("""<slave name="hal" version="%d">
   <platform>Power Macintosh</platform>
   <os family="posix" version="8.1.0">Darwin</os>
   <package name="java" version="2.4.3"/>
-</slave>""")
+</slave>""" % 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("""<slave name="hal" version="%d">
+  <platform>Power Macintosh</platform>
+  <os family="posix" version="8.1.0">Darwin</os>
+</slave>""" % 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("""<slave name="hal" version="%d">
+  <platform>Power Macintosh</platform>
+  <os family="posix" version="8.1.0">Darwin</os>
+</slave>""" % (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("""<slave name="hal">
   <platform>Power Macintosh</platform>
   <os family="posix" version="8.1.0">Darwin</os>
@@ -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("""<result step="foo" status="success"
@@ -310,7 +373,8 @@
                    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=123'))
         module = BuildMaster(self.env)
         assert module.match_request(req)
 
@@ -340,7 +404,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("""<result step="foo" status="success"
@@ -361,7 +425,8 @@
                    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=123'))
         module = BuildMaster(self.env)
         assert module.match_request(req)
 
@@ -399,7 +464,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("""<result step="foo" status="success"
@@ -422,7 +487,8 @@
                    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=123'))
         module = BuildMaster(self.env)
         assert module.match_request(req)
 
@@ -468,7 +534,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("""<result step="foo" status="success"
@@ -494,7 +560,8 @@
                    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=123'))
         module = BuildMaster(self.env)
         assert module.match_request(req)
 
@@ -542,7 +609,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] = '192.168.1.1';
+        build.slave_info[Build.TOKEN] = '123';
         build.insert()
 
         inbody = StringIO("""<result step="foo" status="success"
@@ -563,14 +630,15 @@
                    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(403, outheaders['Status'])
-        self.assertEqual('Build 1 has been invalidated for host 127.0.0.1.',
+        self.assertEqual(409, outheaders['Status'])
+        self.assertEqual('Token mismatch (wrong slave): slave=, build=123',
                         outbody.getvalue())
 
         build = Build.fetch(self.env, build.id)
@@ -591,7 +659,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("""<result step="foo" status="success"
@@ -612,7 +680,8 @@
                    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=123'))
         module = BuildMaster(self.env)
         assert module.match_request(req)
 
@@ -650,13 +719,14 @@
                    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=123'))
         module = BuildMaster(self.env)
         assert module.match_request(req)
 
         self.assertRaises(RequestDone, module.process_request, req)
 
-        self.assertEquals(403, outheaders['Status'])
+        self.assertEquals(409, outheaders['Status'])
         self.assertEquals('Build 1 has been invalidated for host 127.0.0.1.',
                         outbody.getvalue())            
 
@@ -672,7 +742,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("""<result step="foo" status="failure"
@@ -689,7 +759,8 @@
                    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=123'))
         module = BuildMaster(self.env)
         assert module.match_request(req)
 
@@ -719,7 +790,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()
 
@@ -737,7 +808,8 @@
                    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=123'))
         module = BuildMaster(self.env)
         assert module.match_request(req)
 
@@ -779,7 +851,8 @@
                    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)
@@ -798,7 +871,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("""<result step="foo" status="success"
@@ -814,7 +887,8 @@
                    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=123'))
 
         module = BuildMaster(self.env)
         assert module.match_request(req)
@@ -840,7 +914,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)
Copyright (C) 2012-2017 Edgewall Software