From patchwork Tue Mar 13 03:49:10 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: 7254 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 9F4B923E29 for ; Tue, 13 Mar 2012 03:49:13 +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 15948A184CD for ; Tue, 13 Mar 2012 03:49:12 +0000 (UTC) Received: by iage36 with SMTP id e36so268819iag.11 for ; Mon, 12 Mar 2012 20:49:12 -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=RLuxsyFkhMwj+aAArvOWRoR3LYLe8Vd0IKVCIAocilk=; b=SsDPo5f7J/D7YoujHtCUNc3Dz01olG+fp6hJnKqtPYeIwFTWK1TmdjaIHgdNVwQPhk 9v0a6HZwTZyYyXxO2pu8CO0dvh5HeuyzlL8CiNZkr20Ct4DVHmn2/sZj6LrA7r8MoNmE vtQ5F9p5lI70pUiL/wWvf2JRmO5owSY60V94nuVpe6Qa0mwscHBCOjFLHrdusdgMx96b HI3DCRTVbw0FNMAUpvCIfKRPtxWl6iZUX8Qn/1VWRqsjBcOsID/ZSzpB/SeJ+gDQSitE nYmtHzx2Hb0fikiVA27ZB+DlNCnjcmju21YFdOp7ZWWZS7V1rndWFZWh19rJKEE6eHpd BQvQ== Received: by 10.50.222.233 with SMTP id qp9mr1868654igc.58.1331610552395; Mon, 12 Mar 2012 20:49:12 -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 k18csp56604ibg; Mon, 12 Mar 2012 20:49:11 -0700 (PDT) Received: by 10.180.83.97 with SMTP id p1mr3416989wiy.19.1331610550627; Mon, 12 Mar 2012 20:49:10 -0700 (PDT) Received: from indium.canonical.com (indium.canonical.com. [91.189.90.7]) by mx.google.com with ESMTPS id cr10si17648539wib.34.2012.03.12.20.49.10 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 12 Mar 2012 20:49:10 -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 1S7IjW-0002Kn-44 for ; Tue, 13 Mar 2012 03:49:10 +0000 Received: from ackee.canonical.com (localhost [127.0.0.1]) by ackee.canonical.com (Postfix) with ESMTP id 11E27E016F for ; Tue, 13 Mar 2012 03:49:10 +0000 (UTC) MIME-Version: 1.0 X-Launchpad-Project: lava-dispatcher X-Launchpad-Branch: ~linaro-validation/lava-dispatcher/trunk X-Launchpad-Message-Rationale: Subscriber X-Launchpad-Branch-Revision-Number: 253 X-Launchpad-Notification-Type: branch-revision To: Linaro Patch Tracker From: noreply@launchpad.net Subject: [Branch ~linaro-validation/lava-dispatcher/trunk] Rev 253: make the validation of the job file that happens before a job starts much more rigorous Message-Id: <20120313034910.16864.19803.launchpad@ackee.canonical.com> Date: Tue, 13 Mar 2012 03:49:10 -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: 325ae0daa469f37af37e49abd57361503ea756d6 X-Gm-Message-State: ALoCoQn81wzzLWsNZkH6P1/0h82/L3edzHwWK3bVZuXAgqbOzG2VmIVcmcr7CwP2fvebzM4jUAb5 Merge authors: Michael Hudson-Doyle (mwhudson) Related merge proposals: https://code.launchpad.net/~mwhudson/lava-dispatcher/more-validation/+merge/96507 proposed by: Michael Hudson-Doyle (mwhudson) review: Approve - Zygmunt Krynicki (zkrynicki) ------------------------------------------------------------ revno: 253 [merge] committer: Michael Hudson-Doyle branch nick: trunk timestamp: Tue 2012-03-13 16:47:38 +1300 message: make the validation of the job file that happens before a job starts much more rigorous modified: doc/changes.rst lava-dispatch lava_dispatcher/actions/__init__.py lava_dispatcher/actions/android_deploy.py lava_dispatcher/actions/android_install_binaries.py lava_dispatcher/actions/boot_control.py lava_dispatcher/actions/deploy.py lava_dispatcher/actions/launch_control.py lava_dispatcher/actions/lava-android-test.py lava_dispatcher/actions/lava-test.py lava_dispatcher/job.py --- lp:lava-dispatcher https://code.launchpad.net/~linaro-validation/lava-dispatcher/trunk You are subscribed to branch lp:lava-dispatcher. To unsubscribe from this branch go to https://code.launchpad.net/~linaro-validation/lava-dispatcher/trunk/+edit-subscription === 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 ') @@ -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: """ + + 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()