diff mbox

[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
State Accepted
Headers show

Commit Message

Michael-Doyle Hudson March 13, 2012, 3:56 a.m. UTC
Merge authors:
  Michael Hudson-Doyle (mwhudson)
------------------------------------------------------------
revno: 149 [merge]
committer: Michael Hudson-Doyle <michael.hudson@linaro.org>
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
diff mbox

Patch

=== 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",