=== modified file 'doc/changes.rst'
--- doc/changes.rst	2012-03-13 03:44:57 +0000
+++ doc/changes.rst	2012-03-13 03:47:38 +0000
@@ -5,6 +5,8 @@
 
 Version 0.5.9 (UNRELEASED)
 ==========================
+* Make the validation of the job file that happens before a job starts
+  more rigorous.
 
 .. _version_0_5_8:
 

=== modified file 'lava-dispatch'
--- lava-dispatch	2012-03-01 17:10:04 +0000
+++ lava-dispatch	2012-03-08 22:30:45 +0000
@@ -26,7 +26,7 @@
 
 from json_schema_validator.errors import ValidationError
 
-from lava_dispatcher.job import LavaTestJob
+from lava_dispatcher.job import LavaTestJob, validate_job_data
 from lava_dispatcher.config import get_config
 
 parser = optparse.OptionParser('%prog: lava-dispatch <json job file>')
@@ -83,7 +83,7 @@
 #FIXME Return status
 if options.validate:
     try:
-        job.validate()
+        validate_job_data(job.job_data)
     except ValidationError as e:
         print e
 else:

=== modified file 'lava_dispatcher/actions/__init__.py'
--- lava_dispatcher/actions/__init__.py	2012-02-24 01:36:44 +0000
+++ lava_dispatcher/actions/__init__.py	2012-03-11 22:12:52 +0000
@@ -24,6 +24,15 @@
 import imp
 import os
 
+from json_schema_validator.schema import Schema
+from json_schema_validator.validator import Validator
+
+
+null_or_empty_schema = {
+    'type': ['object', 'null'],
+    'additionalProperties': False,
+    }
+
 
 class classproperty(object):
     """Like the builtin @property, but binds to the class not instances."""
@@ -56,6 +65,14 @@
     def test_name(self, **params):
         return self.command_name
 
+    param_schema = None
+
+    @classmethod
+    def validate_parameters(cls, params):
+        if cls.parameters_schema:
+            schema = Schema(cls.parameters_schema)
+            Validator.validate(schema, params)
+
 
 def _find_commands(module):
     cmds = {}

=== modified file 'lava_dispatcher/actions/android_deploy.py'
--- lava_dispatcher/actions/android_deploy.py	2011-12-19 02:23:53 +0000
+++ lava_dispatcher/actions/android_deploy.py	2012-03-11 23:37:07 +0000
@@ -23,5 +23,19 @@
 
 
 class cmd_deploy_linaro_android_image(BaseAction):
+
+    parameters_schema = {
+        'type': 'object',
+        'properties': {
+            'boot': {'type': 'string'},
+            'system': {'type': 'string'},
+            'data': {'type': 'string'},
+            'pkg': {'type': 'string', 'optional': True},
+            'use_cache': {'type': 'bool', 'optional': True, 'default': True},
+            'rootfstype': {'type': 'string', 'optional': True, 'default': 'ext4'},
+            },
+        'additionalProperties': False,
+        }
+
     def run(self, boot, system, data, pkg=None, use_cache=True, rootfstype='ext4'):
         self.client.deploy_linaro_android(boot, system, data, pkg, use_cache, rootfstype)

=== modified file 'lava_dispatcher/actions/android_install_binaries.py'
--- lava_dispatcher/actions/android_install_binaries.py	2012-01-27 03:10:08 +0000
+++ lava_dispatcher/actions/android_install_binaries.py	2012-03-11 22:12:52 +0000
@@ -19,11 +19,14 @@
 
 import ConfigParser
 import logging
-from lava_dispatcher.actions import BaseAction
+from lava_dispatcher.actions import BaseAction, null_or_empty_schema
 from lava_dispatcher.client.master import _deploy_tarball_to_board
 
 
 class cmd_android_install_binaries(BaseAction):
+
+    parameters_schema = null_or_empty_schema
+
     def run(self):
         try:
             driver_tarball = self.client.device_option(

=== modified file 'lava_dispatcher/actions/boot_control.py'
--- lava_dispatcher/actions/boot_control.py	2012-01-26 10:20:09 +0000
+++ lava_dispatcher/actions/boot_control.py	2012-03-11 22:12:52 +0000
@@ -22,12 +22,16 @@
 
 import logging
 
-from lava_dispatcher.actions import BaseAction
+from lava_dispatcher.actions import BaseAction, null_or_empty_schema
 from lava_dispatcher.client.base import CriticalError
 
+
 class cmd_boot_linaro_android_image(BaseAction):
     """ Call client code to boot to the master image
     """
+
+    parameters_schema = null_or_empty_schema
+
     def run(self):
         client = self.client
         try:
@@ -39,6 +43,9 @@
 class cmd_boot_linaro_image(BaseAction):
     """ Call client code to boot to the test image
     """
+
+    parameters_schema = null_or_empty_schema
+
     def run(self):
         client = self.client
         status = 'pass'
@@ -55,6 +62,9 @@
 class cmd_boot_master_image(BaseAction):
     """ Call client code to boot to the master image
     """
+
+    parameters_schema = null_or_empty_schema
+
     def run(self):
         client = self.client
         client.boot_master_image()

=== modified file 'lava_dispatcher/actions/deploy.py'
--- lava_dispatcher/actions/deploy.py	2012-01-31 01:20:47 +0000
+++ lava_dispatcher/actions/deploy.py	2012-03-11 23:37:07 +0000
@@ -21,6 +21,63 @@
 
 
 class cmd_deploy_linaro_image(BaseAction):
-    def run(self, hwpack=None, rootfs=None, image=None, kernel_matrix=None, use_cache=True, rootfstype='ext3'):
+
+    # This is how the schema for parameters should look, but there are bugs in
+    # json_schema_validation that means it doesn't work (see
+    # https://github.com/zyga/json-schema-validator/pull/6).
+
+    ## parameters_schema = {
+    ##     'type': [
+    ##         {
+    ##             'type': 'object',
+    ##             'properties': {
+    ##                 'image': {'type': 'string'},
+    ##                 },
+    ##             'additionalProperties': False,
+    ##             },
+    ##         {
+    ##             'type': 'object',
+    ##             'properties': {
+    ##                 'hwpack': {'type': 'string'},
+    ##                 'rootfs': {'type': 'string'},
+    ##                 'kernel_matrix': {'type': 'string', 'optional': True},
+    ##                 'use_cache': {'type': 'bool', 'optional': True, 'default': True},
+    ##                 'rootfstype': {'type': 'string', 'optional': True, 'default': 'ext3'},
+    ##                 },
+    ##             'additionalProperties': False,
+    ##             },
+    ##         ],
+    ##     }
+
+    parameters_schema = {
+        'type': 'object',
+        'properties': {
+            'hwpack': {'type': 'string', 'optional': True},
+            'rootfs': {'type': 'string', 'optional': True},
+            'image': {'type': 'string', 'optional': True},
+            'kernel_matrix': {'type': 'string', 'optional': True},
+            'use_cache': {'type': 'bool', 'optional': True},
+            'rootfstype': {'type': 'string', 'optional': True},
+            },
+        'additionalProperties': False,
+        }
+
+    @classmethod
+    def validate_parameters(cls, parameters):
+        super(cmd_deploy_linaro_image, cls).validate_parameters(parameters)
+        if 'hwpack' in parameters:
+            if 'rootfs' not in parameters:
+                raise ValueError('must specify rootfs when specifying hwpack')
+            if 'image' in parameters:
+                raise ValueError('cannot specify image and hwpack')
+        elif 'image' not in parameters:
+            raise ValueError('must specify image if not specifying a hwpack')
+        elif 'kernel_matrix' in parameters:
+            raise ValueError('cannot specify kernel_matrix with an image')
+
+    def run(self, hwpack=None, rootfs=None, image=None, kernel_matrix=None,
+            use_cache=True, rootfstype='ext3'):
         self.client.deploy_linaro(
-            hwpack=hwpack, rootfs=rootfs, image=image, kernel_matrix=kernel_matrix, use_cache=use_cache, rootfstype=rootfstype)
+            hwpack=hwpack, rootfs=rootfs, image=image,
+            kernel_matrix=kernel_matrix, use_cache=use_cache,
+            rootfstype=rootfstype)

=== modified file 'lava_dispatcher/actions/launch_control.py'
--- lava_dispatcher/actions/launch_control.py	2012-02-24 02:13:45 +0000
+++ lava_dispatcher/actions/launch_control.py	2012-03-08 22:29:23 +0000
@@ -77,6 +77,17 @@
 
 class cmd_submit_results(BaseAction):
 
+    parameters_schema = {
+        'type': 'object',
+        'properties': {
+            'server': {'type': 'string'},
+            'stream': {'type': 'string'},
+            'result_disk': {'type': 'string', 'optional': True},
+            'token': {'type': 'string', 'optional': True},
+            },
+        'additionalProperties': False,
+        }
+
     def _get_bundles_from_device(self, result_disk):
         err_msg = ''
         status = 'fail'

=== modified file 'lava_dispatcher/actions/lava-android-test.py'
--- lava_dispatcher/actions/lava-android-test.py	2011-11-24 03:00:54 +0000
+++ lava_dispatcher/actions/lava-android-test.py	2012-03-08 22:29:23 +0000
@@ -36,6 +36,15 @@
 
 class cmd_lava_android_test_run(AndroidTestAction):
 
+    parameters_schema = {
+        'type': 'object',
+        'properties': {
+            'test_name': {'type': 'string'},
+            'timeout': {'type': 'integer', 'optional': True},
+            },
+        'additionalProperties': False,
+        }
+
     def test_name(self, test_name, timeout=-1):
         return super(cmd_lava_android_test_run, self).test_name() + \
                ' (%s)' % test_name
@@ -61,6 +70,17 @@
     """
     lava-test deployment to test image rootfs by chroot
     """
+
+    parameters_schema = {
+        'type': 'object',
+        'properties': {
+            'tests': {'type': 'array', 'items': {'type': 'string'}},
+            'option': {'type': 'string', 'optional': True},
+            'timeout': {'type': 'integer', 'optional': True},
+            },
+        'additionalProperties': False,
+        }
+
     def run(self, tests, option=None, timeout=2400):
         self.check_lava_android_test_installed()
         with self.client.android_tester_session() as session:

=== modified file 'lava_dispatcher/actions/lava-test.py'
--- lava_dispatcher/actions/lava-test.py	2012-03-08 21:52:44 +0000
+++ lava_dispatcher/actions/lava-test.py	2012-03-09 01:00:42 +0000
@@ -49,6 +49,16 @@
 
 class cmd_lava_test_run(BaseAction):
 
+    parameters_schema = {
+        'type': 'object',
+        'properties': {
+            'test_name': {'type': 'string'},
+            'test_options': {'type': 'string', 'optional': True},
+            'timeout': {'type': 'integer', 'optional': True},
+            },
+        'additionalProperties': False,
+        }
+
     def test_name(self, test_name, test_options = "", timeout=-1):
         return super(cmd_lava_test_run, self).test_name() + ' (%s)' % test_name
 
@@ -85,6 +95,22 @@
     """
     lava-test deployment to test image rootfs by chroot
     """
+
+    parameters_schema = {
+        'type': 'object',
+        'properties': {
+            'tests': {'type': 'array', 'items': {'type': 'string'}},
+            'install_python': {
+                'type': 'array', 'items': {'type': 'string'}, 'optional': True
+                },
+            'register': {
+                'type': 'array', 'items': {'type': 'string'}, 'optional': True
+                },
+            'timeout': {'type': 'integer', 'optional': True},
+            },
+        'additionalProperties': False,
+        }
+
     def run(self, tests, install_python = None, register = None, timeout=2400):
         logging.info("Executing lava_test_install (%s) command" % ",".join(tests))
 
@@ -112,6 +138,15 @@
     add apt repository to test image rootfs by chroot
     arg could be 'deb uri distribution [component1] [component2][...]' or ppa:<ppa_name>
     """
+
+    parameters_schema = {
+        'type': 'object',
+        'properties': {
+            'arg': {'type': 'string'},
+            },
+        'additionalProperties': False,
+        }
+
     def run(self, arg):
         with self.client.reliable_session() as session:
 

=== modified file 'lava_dispatcher/job.py'
--- lava_dispatcher/job.py	2012-03-12 02:03:31 +0000
+++ lava_dispatcher/job.py	2012-03-13 03:47:38 +0000
@@ -28,8 +28,7 @@
 
 from lava_dispatcher.actions import get_all_cmds
 from lava_dispatcher.client.base import CriticalError, GeneralError
-from lava_dispatcher.config import get_config
-from lava_dispatcher.context import LavaContext 
+from lava_dispatcher.context import LavaContext
 
 
 job_schema = {
@@ -38,12 +37,15 @@
     'properties': {
         'actions': {
             'items': {
+                'type': 'object',
                 'properties': {
                     'command': {
                         'optional': False,
+                        'type': 'string',
                         },
                     'parameters': {
                         'optional': True,
+                        'type': 'object',
                         },
                     'metadata': {
                         'optional': True,
@@ -53,9 +55,11 @@
                 },
             },
         'device_type': {
+            'type': 'string',
             'optional': True,
             },
         'job_name': {
+            'type': 'string',
             'optional': True,
             },
         'health_check': {
@@ -63,6 +67,7 @@
             'default': False,
             },
         'target': {
+            'type': 'string',
             'optional': True,
             },
         'timeout': {
@@ -70,11 +75,26 @@
             'optional': False,
             },
         'logging_level': {
+            'type': 'string',
+            'enum': ["CRITICAL", "ERROR", "WARNING", "INFO", "DEBUG"],
             'optional': True,
             },
         },
     }
 
+
+def validate_job_data(job_data):
+    schema = Schema(job_schema)
+    Validator.validate(schema, job_data)
+    lava_commands = get_all_cmds()
+    for action in job_data['actions']:
+        command_name = action['command']
+        command = lava_commands.get(command_name)
+        if command is None:
+            raise ValueError("action %r not known" % command_name)
+        command.validate_parameters(action.get('parameters'))
+
+
 class LavaTestJob(object):
     def __init__(self, job_json, oob_file, config):
         self.job_status = 'pass'
@@ -96,19 +116,8 @@
         except :
             return None
 
-    def validate(self):
-        schema = Schema(job_schema)
-        validator = Validator()
-        validator.validate(schema, self.job_data)
-
-        lava_commands = get_all_cmds()
-        for action in self.job_data['actions']:
-            command_name = action['command']
-            if command_name not in lava_commands:
-                raise CriticalError("action %r not known" % command_name)
-
     def run(self):
-        self.validate()
+        validate_job_data(self.job_data)
         self._set_logging_level()
         lava_commands = get_all_cmds()
 

