From patchwork Tue Mar 13 03:56:11 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael-Doyle Hudson X-Patchwork-Id: 7256 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id D27DA23E13 for ; Tue, 13 Mar 2012 04:01:50 +0000 (UTC) Received: from mail-iy0-f180.google.com (mail-iy0-f180.google.com [209.85.210.180]) by fiordland.canonical.com (Postfix) with ESMTP id 69716A184C3 for ; Tue, 13 Mar 2012 04:01:50 +0000 (UTC) Received: by mail-iy0-f180.google.com with SMTP id e36so285460iag.11 for ; Mon, 12 Mar 2012 21:01:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-forwarded-to:x-forwarded-for:delivered-to:received-spf :content-type:mime-version:x-launchpad-project:x-launchpad-branch :x-launchpad-message-rationale:x-launchpad-branch-revision-number :x-launchpad-notification-type:to:from:subject:message-id:date :reply-to:sender:errors-to:precedence:x-generated-by :x-launchpad-hash:x-gm-message-state; bh=YjMiR4GagM+ZRf50kOGZGCNRQx6xobAFLqlze9Q/AbI=; b=MeeT0u+bx3BuYpatqmYvjeeAbglRtmu390ngOgoF004KBExnCAGeNr5XquETj3iyec qQ52Yc901qEw7/km80dC/0og8GVwv26IdDBXOGFNIDKfGItNoWTfHnYYu5QqQ5Ul3qk3 bLZRHwhTu0hHIikkNTM8iBbOveq2FI5u2QdoViXNQDFewgWva9ST+wR5cMfJ/GW9OPXi b4FPkjr5yyki9d0o0hY42XR2RdrBqlCJAxTywZTcnkdjC8KRmTzKd/1YHOvX4vKqdTfk bUUimSCiHUtCYuT0p1pHErAAzcgWQ+JufYxnB7Zh6pCPZ1c8NejmxYEvenVObUCp+aTA cCfQ== Received: by 10.50.183.137 with SMTP id em9mr1910955igc.58.1331611310190; Mon, 12 Mar 2012 21:01:50 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.231.53.18 with SMTP id k18csp56715ibg; Mon, 12 Mar 2012 21:01:49 -0700 (PDT) Received: by 10.180.102.231 with SMTP id fr7mr3530428wib.10.1331611308520; Mon, 12 Mar 2012 21:01:48 -0700 (PDT) Received: from indium.canonical.com (indium.canonical.com. [91.189.90.7]) by mx.google.com with ESMTPS id s74si20446466weq.56.2012.03.12.21.01.48 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 12 Mar 2012 21:01:48 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of bounces@canonical.com designates 91.189.90.7 as permitted sender) client-ip=91.189.90.7; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of bounces@canonical.com designates 91.189.90.7 as permitted sender) smtp.mail=bounces@canonical.com Received: from ackee.canonical.com ([91.189.89.26]) by indium.canonical.com with esmtp (Exim 4.71 #1 (Debian)) id 1S7Ivj-0005rN-Ux for ; Tue, 13 Mar 2012 04:01:47 +0000 Received: from ackee.canonical.com (localhost [127.0.0.1]) by ackee.canonical.com (Postfix) with ESMTP id 70FB2E01F1 for ; Tue, 13 Mar 2012 03:56:11 +0000 (UTC) MIME-Version: 1.0 X-Launchpad-Project: lava-scheduler X-Launchpad-Branch: ~linaro-validation/lava-scheduler/trunk X-Launchpad-Message-Rationale: Subscriber X-Launchpad-Branch-Revision-Number: 149 X-Launchpad-Notification-Type: branch-revision To: Linaro Patch Tracker From: noreply@launchpad.net Subject: [Branch ~linaro-validation/lava-scheduler/trunk] Rev 149: * Validate the job file much more thoroughly when it is submitted. Message-Id: <20120313035611.18790.1137.launchpad@ackee.canonical.com> Date: Tue, 13 Mar 2012 03:56:11 -0000 Reply-To: noreply@launchpad.net Sender: bounces@canonical.com Errors-To: bounces@canonical.com Precedence: bulk X-Generated-By: Launchpad (canonical.com); Revision="14933"; Instance="launchpad-lazr.conf" X-Launchpad-Hash: d03e259577be6584c4216951f1588a7b90ef42db X-Gm-Message-State: ALoCoQn/tBylTiqSVcZFxiyHYiad6iESmtrqT3FK0yDe3f4IX/ApcNCZeSJxi3aBGrLxerE6ghtB Merge authors: Michael Hudson-Doyle (mwhudson) ------------------------------------------------------------ revno: 149 [merge] committer: Michael Hudson-Doyle branch nick: trunk timestamp: Tue 2012-03-13 16:54:01 +1300 message: * Validate the job file much more thoroughly when it is submitted. * Allow for the creation of private jobs by copying the access data from the target bundle stream over to the created job at submit time. modified: doc/changes.rst lava_scheduler_app/models.py lava_scheduler_app/tests.py lava_scheduler_daemon/dbjobsource.py setup.py --- lp:lava-scheduler https://code.launchpad.net/~linaro-validation/lava-scheduler/trunk You are subscribed to branch lp:lava-scheduler. To unsubscribe from this branch go to https://code.launchpad.net/~linaro-validation/lava-scheduler/trunk/+edit-subscription === modified file 'doc/changes.rst' --- doc/changes.rst 2012-03-13 00:24:38 +0000 +++ doc/changes.rst 2012-03-13 03:53:34 +0000 @@ -12,7 +12,10 @@ restrictions. * Add admin action to set the health_status of all boards with pass status to unknown status -- for use after a rollout. - +* Validate the job file much more thoroughly when it is submitted. +* Allow for the creation of private jobs by copying the access data + from the target bundle stream over to the created job at submit + time. .. _version_0_10.1: === modified file 'lava_scheduler_app/models.py' --- lava_scheduler_app/models.py 2012-03-07 01:56:57 +0000 +++ lava_scheduler_app/models.py 2012-03-09 01:46:30 +0000 @@ -3,16 +3,17 @@ from django.contrib.auth.models import User from django.core.exceptions import ValidationError from django.db import models -from django.db.models.query import QuerySet from django.utils.translation import ugettext as _ - -from django_restricted_resource.managers import RestrictedResourceManager from django_restricted_resource.models import RestrictedResource -from django_restricted_resource.utils import filter_bogus_users + +from dashboard_app.models import BundleStream + +from lava_dispatcher.job import validate_job_data from linaro_django_xmlrpc.models import AuthToken + class JSONDataError(ValueError): """Error raised when JSON is syntactically valid but ill-formed.""" @@ -30,12 +31,9 @@ def validate_job_json(data): try: ob = simplejson.loads(data) + validate_job_data(ob) except ValueError, e: raise ValidationError(str(e)) - else: - if not isinstance(ob, dict): - raise ValidationError( - "job json must be an object, not %s" % type(ob).__name__) class DeviceType(models.Model): @@ -266,6 +264,7 @@ @classmethod def from_json_and_user(cls, json_data, user): job_data = simplejson.loads(json_data) + validate_job_data(job_data) if 'target' in job_data: target = Device.objects.get(hostname=job_data['target']) device_type = None @@ -279,6 +278,25 @@ is_check = job_data.get('health_check', False) + submitter = user + group = None + is_public = True + + for action in job_data['actions']: + if not action['command'].startswith('submit_results'): + continue + stream = action['parameters']['stream'] + try: + bundle_stream = BundleStream.objects.get(pathname=stream) + except BundleStream.DoesNotExist: + raise ValueError("stream %s not found" % stream) + if not bundle_stream.is_owned_by(submitter): + raise ValueError( + "you cannot submit to the stream %s" % stream) + user, group, is_public = (bundle_stream.user, + bundle_stream.group, + bundle_stream.is_public) + tags = [] for tag_name in job_data.get('device_tags', []): try: @@ -286,9 +304,10 @@ except Tag.DoesNotExist: raise JSONDataError("tag %r does not exist" % tag_name) job = TestJob( - definition=json_data, submitter=user, requested_device=target, - requested_device_type=device_type, description=job_name, - health_check=is_check, user=user) + definition=json_data, submitter=submitter, + requested_device=target, requested_device_type=device_type, + description=job_name, health_check=is_check, user=user, + group=group, is_public=is_public) job.save() for tag in tags: job.tags.add(tag) === modified file 'lava_scheduler_app/tests.py' --- lava_scheduler_app/tests.py 2012-02-28 03:53:16 +0000 +++ lava_scheduler_app/tests.py 2012-03-09 04:07:06 +0000 @@ -3,7 +3,9 @@ import json import xmlrpclib -from django.contrib.auth.models import Permission, User +from dashboard_app.models import BundleStream + +from django.contrib.auth.models import Group, Permission, User from django.test import TransactionTestCase from django.test.client import Client @@ -84,9 +86,22 @@ device.save() return device + def make_job_data(self, actions=[], **kw): + data = {'actions': actions, 'timeout': 1} + data.update(kw) + if 'target' not in data and 'device_type' not in data: + if DeviceType.objects.all(): + data['device_type'] = DeviceType.objects.all()[0].name + else: + data['device_type'] = self.ensure_device_type().name + return data + + def make_job_json(self, **kw): + return json.dumps(self.make_job_data(**kw)) + def make_testjob(self, definition=None, submitter=None, **kwargs): if definition is None: - definition = json.dumps({}) + definition = self.make_job_json() if submitter is None: submitter = self.make_user() if 'user' not in kwargs: @@ -107,70 +122,68 @@ class TestTestJob(TestCaseWithFactory): def test_from_json_and_user_sets_definition(self): - self.factory.ensure_device_type(name='panda') - definition = json.dumps({'device_type':'panda'}) + definition = self.factory.make_job_json() job = TestJob.from_json_and_user(definition, self.factory.make_user()) self.assertEqual(definition, job.definition) def test_from_json_and_user_sets_submitter(self): - self.factory.ensure_device_type(name='panda') user = self.factory.make_user() job = TestJob.from_json_and_user( - json.dumps({'device_type':'panda'}), user) + self.factory.make_job_json(), user) self.assertEqual(user, job.submitter) def test_from_json_and_user_sets_device_type(self): panda_type = self.factory.ensure_device_type(name='panda') job = TestJob.from_json_and_user( - json.dumps({'device_type':'panda'}), self.factory.make_user()) + self.factory.make_job_json(device_type='panda'), + self.factory.make_user()) self.assertEqual(panda_type, job.requested_device_type) def test_from_json_and_user_sets_target(self): panda_board = self.factory.make_device(hostname='panda01') job = TestJob.from_json_and_user( - json.dumps({'target':'panda01'}), self.factory.make_user()) + self.factory.make_job_json(target='panda01'), + self.factory.make_user()) self.assertEqual(panda_board, job.requested_device) def test_from_json_and_user_does_not_set_device_type_from_target(self): panda_type = self.factory.ensure_device_type(name='panda') self.factory.make_device(device_type=panda_type, hostname='panda01') job = TestJob.from_json_and_user( - json.dumps({'target':'panda01'}), self.factory.make_user()) + self.factory.make_job_json(target='panda01'), + self.factory.make_user()) self.assertEqual(None, job.requested_device_type) def test_from_json_and_user_sets_date_submitted(self): - self.factory.ensure_device_type(name='panda') before = datetime.datetime.now() job = TestJob.from_json_and_user( - json.dumps({'device_type':'panda'}), self.factory.make_user()) + self.factory.make_job_json(), + self.factory.make_user()) after = datetime.datetime.now() self.assertTrue(before < job.submit_time < after) def test_from_json_and_user_sets_status_to_SUBMITTED(self): - self.factory.ensure_device_type(name='panda') job = TestJob.from_json_and_user( - json.dumps({'device_type':'panda'}), self.factory.make_user()) + self.factory.make_job_json(), + self.factory.make_user()) self.assertEqual(job.status, TestJob.SUBMITTED) def test_from_json_and_user_sets_no_tags_if_no_tags(self): - self.factory.ensure_device_type(name='panda') job = TestJob.from_json_and_user( - json.dumps({'device_type':'panda', 'device_tags':[]}), + self.factory.make_job_json(device_tags=[]), self.factory.make_user()) self.assertEqual(set(job.tags.all()), set([])) def test_from_json_and_user_errors_on_unknown_tags(self): - self.factory.ensure_device_type(name='panda') self.assertRaises( JSONDataError, TestJob.from_json_and_user, - json.dumps({'device_type':'panda', 'device_tags':['unknown']}), + self.factory.make_job_json(device_tags=['unknown']), self.factory.make_user()) def test_from_json_and_user_sets_tag_from_device_tags(self): - self.factory.ensure_device_type(name='panda') self.factory.ensure_tag('tag') job = TestJob.from_json_and_user( - json.dumps({'device_type':'panda', 'device_tags':['tag']}), + self.factory.make_job_json(device_tags=['tag']), self.factory.make_user()) self.assertEqual( set(tag.name for tag in job.tags.all()), set(['tag'])) @@ -180,7 +193,7 @@ self.factory.ensure_tag('tag1') self.factory.ensure_tag('tag2') job = TestJob.from_json_and_user( - json.dumps({'device_type':'panda', 'device_tags':['tag1', 'tag2']}), + self.factory.make_job_json(device_tags=['tag1', 'tag2']), self.factory.make_user()) self.assertEqual( set(tag.name for tag in job.tags.all()), set(['tag1', 'tag2'])) @@ -189,15 +202,75 @@ self.factory.ensure_device_type(name='panda') self.factory.ensure_tag('tag') job1 = TestJob.from_json_and_user( - json.dumps({'device_type':'panda', 'device_tags':['tag']}), + self.factory.make_job_json(device_tags=['tag']), self.factory.make_user()) job2 = TestJob.from_json_and_user( - json.dumps({'device_type':'panda', 'device_tags':['tag']}), + self.factory.make_job_json(device_tags=['tag']), self.factory.make_user()) self.assertEqual( set(tag.pk for tag in job1.tags.all()), set(tag.pk for tag in job2.tags.all())) + def test_from_json_and_user_rejects_invalid_json(self): + self.assertRaises( + ValueError, TestJob.from_json_and_user, '{', + self.factory.make_user()) + + def test_from_json_and_user_rejects_invalid_job(self): + # job data must have the 'actions' and 'timeout' properties, so this + # will be rejected. + self.assertRaises( + ValueError, TestJob.from_json_and_user, '{}', + self.factory.make_user()) + + def make_job_json_for_stream_name(self, stream_name): + return self.factory.make_job_json( + actions=[ + { + 'command':'submit_results', + 'parameters': { + 'server': '...', + 'stream': stream_name, + } + } + ]) + + def test_from_json_and_user_sets_group_from_bundlestream(self): + group = Group.objects.create(name='group') + user = self.factory.make_user() + user.groups.add(group) + b = BundleStream.objects.create( + group=group, slug='blah', is_public=True) + b.save() + j = self.make_job_json_for_stream_name(b.pathname) + job = TestJob.from_json_and_user(j, user) + self.assertEqual(group, job.group) + + def test_from_json_and_user_sets_is_public_from_bundlestream(self): + group = Group.objects.create(name='group') + user = self.factory.make_user() + user.groups.add(group) + b = BundleStream.objects.create( + group=group, slug='blah', is_public=False) + b.save() + j = self.make_job_json_for_stream_name(b.pathname) + job = TestJob.from_json_and_user(j, user) + self.assertEqual(False, job.is_public) + + def test_from_json_and_user_rejects_missing_bundlestream(self): + user = self.factory.make_user() + j = self.make_job_json_for_stream_name('no such stream') + self.assertRaises(ValueError, TestJob.from_json_and_user, j, user) + + def test_from_json_and_user_rejects_inaccessible_bundlestream(self): + stream_user = self.factory.make_user() + job_user = self.factory.make_user() + b = BundleStream.objects.create( + user=stream_user, slug='blah', is_public=True) + b.save() + j = self.make_job_json_for_stream_name(b.pathname) + self.assertRaises(ValueError, TestJob.from_json_and_user, j, job_user) + class TestSchedulerAPI(TestCaseWithFactory): @@ -231,8 +304,7 @@ Permission.objects.get(codename='add_testjob')) user.save() server = self.server_proxy('test', 'test') - self.factory.ensure_device_type(name='panda') - definition = json.dumps({'device_type':'panda'}) + definition = self.factory.make_job_json() job_id = server.scheduler.submit_job(definition) job = TestJob.objects.get(id=job_id) self.assertEqual(definition, job.definition) @@ -296,14 +368,19 @@ def test_getJobForBoard_returns_json(self): device = self.factory.make_device(hostname='panda01') - definition = {'foo': 'bar', 'target': 'panda01'} + definition = self.factory.make_job_data(target='panda01') self.factory.make_testjob( requested_device=device, definition=json.dumps(definition)) self.assertEqual( definition, self.source.getJobForBoard('panda01')) - health_job = json.dumps({'health_check': True}) - ordinary_job = json.dumps({'health_check': False}) + @property + def health_job(self): + return self.factory.make_job_json(health_check=True) + + @property + def ordinary_job(self): + return self.factory.make_job_json(health_check=False) def assertHealthJobAssigned(self, device): job_data = self.source.getJobForBoard(device.hostname) @@ -371,7 +448,7 @@ def test_getJobForBoard_considers_device_type(self): panda_type = self.factory.ensure_device_type(name='panda') self.factory.make_device(hostname='panda01', device_type=panda_type) - definition = {'foo': 'bar'} + definition = self.factory.make_job_data() self.factory.make_testjob( requested_device_type=panda_type, definition=json.dumps(definition)) @@ -383,8 +460,8 @@ panda_type = self.factory.ensure_device_type(name='panda') panda01 = self.factory.make_device( hostname='panda01', device_type=panda_type) - first_definition = {'foo': 'bar', 'target': 'panda01'} - second_definition = {'foo': 'baz', 'target': 'panda01'} + first_definition = self.factory.make_job_data(foo='bar', target='panda01') + second_definition = self.factory.make_job_data(foo='baz', target='panda01') self.factory.make_testjob( requested_device=panda01, definition=json.dumps(first_definition), submit_time=datetime.datetime.now() - datetime.timedelta(days=1)) @@ -399,12 +476,13 @@ panda_type = self.factory.ensure_device_type(name='panda') panda01 = self.factory.make_device( hostname='panda01', device_type=panda_type) - type_definition = {'foo': 'bar'} + type_definition = self.factory.make_job_data() self.factory.make_testjob( requested_device_type=panda_type, definition=json.dumps(type_definition), submit_time=datetime.datetime.now() - datetime.timedelta(days=1)) - device_definition = {'foo': 'baz', 'target': 'panda01'} + device_definition = self.factory.make_job_data( + foo='baz', target='panda01') self.factory.make_testjob( requested_device=panda01, definition=json.dumps(device_definition)) @@ -417,7 +495,7 @@ panda01 = self.factory.make_device( hostname='panda01', device_type=panda_type) self.factory.make_device(hostname='panda02', device_type=panda_type) - definition = {'foo': 'bar', 'target': 'panda01'} + definition = self.factory.make_job_data(foo='bar', target='panda01') self.factory.make_testjob( requested_device=panda01, definition=json.dumps(definition)) @@ -515,7 +593,7 @@ def test_getJobForBoard_inserts_target_into_json(self): panda_type = self.factory.ensure_device_type(name='panda') self.factory.make_device(hostname='panda01', device_type=panda_type) - definition = {'foo': 'bar'} + definition = self.factory.make_job_data(device_type='panda') self.factory.make_testjob( requested_device_type=panda_type, definition=json.dumps(definition)) === modified file 'lava_scheduler_daemon/dbjobsource.py' --- lava_scheduler_daemon/dbjobsource.py 2012-03-07 01:59:51 +0000 +++ lava_scheduler_daemon/dbjobsource.py 2012-03-09 01:27:51 +0000 @@ -88,25 +88,11 @@ def _get_json_data(self, job): json_data = json.loads(job.definition) json_data['target'] = job.actual_device.hostname - # The rather extreme paranoia in what follows could be much reduced if - # we thoroughly validated job data in submit_job. We don't (yet?) - # and there is no sane way to report errors at this stage, so, - # paranoia (the dispatcher will choke on bogus input in a more - # informative way). - if 'actions' not in json_data: - return json_data - actions = json_data['actions'] - for action in actions: - if not isinstance(action, dict): - continue - if action.get('command') != 'submit_results': - continue - params = action.get('parameters') - if not isinstance(params, dict): - continue + for action in json_data['actions']: + if not action['command'].startswith('submit_results'): + continue + params = action['parameters'] params['token'] = job.submit_token.secret - if not 'server' in params or not isinstance(params['server'], unicode): - continue parsed = urlparse.urlsplit(params['server']) netloc = job.submitter.username + '@' + parsed.hostname parsed = list(parsed) === modified file 'setup.py' --- setup.py 2012-03-13 01:19:49 +0000 +++ setup.py 2012-03-13 03:49:52 +0000 @@ -35,6 +35,8 @@ install_requires=[ "django-restricted-resource", "django-tables2 >= 0.9.4", + "lava-dashboard", + "lava-dispatcher >= 0.5.9.dev253", "lava-server >= 0.11.dev355", "simplejson", "south >= 0.7.3",