[Kimchi-devel] [kimchi-devel][PATCHv2] Improve PUT param checking

From: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; v1>v2, updated copyright information, also fix make check-local. Delete self-defined update param validation, and use additionalProperties in json schema validator instead. Signed-off-by: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; --- src/kimchi/API.json | 15 ++++++++++----- src/kimchi/control/base.py | 12 +----------- src/kimchi/control/debugreports.py | 3 +-- src/kimchi/control/host.py | 3 +-- src/kimchi/control/storagepools.py | 3 +-- src/kimchi/control/templates.py | 6 +----- src/kimchi/control/vm/ifaces.py | 3 +-- src/kimchi/control/vm/storages.py | 3 +-- src/kimchi/control/vms.py | 4 +--- src/kimchi/i18n.py | 3 +-- tests/test_rest.py | 6 +++++- 11 files changed, 24 insertions(+), 37 deletions(-) diff --git a/src/kimchi/API.json b/src/kimchi/API.json index c5d7bdb..0cfa20c 100644 --- a/src/kimchi/API.json +++ b/src/kimchi/API.json @@ -80,7 +80,8 @@ "pattern": "^[_A-Za-z0-9-]*$", "error": "KCHDR0007E" } - } + }, + "additionalProperties": false }, "storagepools_create": { "type": "object", @@ -182,7 +183,8 @@ "minItems": 1, "uniqueItems": true } - } + }, + "additionalProperties": false }, "storagevolumes_create": { "type": "object", @@ -303,7 +305,8 @@ "minimum": 512, "error": "KCHTMPL0013E" } - } + }, + "additionalProperties": false }, "networks_create": { "type": "object", @@ -382,7 +385,8 @@ "enum": ["ne2k_pci", "i82551", "i82557b", "i82559er", "rtl8139", "e1000", "pcnet", "virtio", "spapr-vlan"], "error": "KCHVMIF0006E" } - } + }, + "additionalProperties": false }, "templates_create": { "type": "object", @@ -554,7 +558,8 @@ "required": true, "error": "KCHVMSTOR0003E" } - } + }, + "additionalProperties": false }, "template_update": { "type": "object", diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py index 60db1df..97e789f 100644 --- a/src/kimchi/control/base.py +++ b/src/kimchi/control/base.py @@ -1,7 +1,7 @@ # # Project Kimchi # -# Copyright IBM, Corp. 2013-2014 +# Copyright IBM, Corp. 2013-2015 # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -54,7 +54,6 @@ class Resource(object): self.model = model self.ident = ident self.model_args = (ident,) - self.update_params = [] self.role_key = None self.admin_methods = [] @@ -193,15 +192,6 @@ class Resource(object): params = parse_request() validate_params(params, self, 'update') - if self.update_params is not None: - invalids = [v for v in params.keys() if - v not in self.update_params] - if invalids: - msg_args = {'params': ", ".join(invalids), - 'resource': get_class_name(self)} - e = InvalidOperation('KCHAPI0004E', msg_args) - raise cherrypy.HTTPError(405, e.message) - args = list(self.model_args) + [params] ident = update(*args) self._redirect(ident) diff --git a/src/kimchi/control/debugreports.py b/src/kimchi/control/debugreports.py index 1d9ce59..377d002 100644 --- a/src/kimchi/control/debugreports.py +++ b/src/kimchi/control/debugreports.py @@ -1,7 +1,7 @@ # # Project Kimchi # -# Copyright IBM, Corp. 2013-2014 +# Copyright IBM, Corp. 2013-2015 # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -40,7 +40,6 @@ class DebugReport(Resource): super(DebugReport, self).__init__(model, ident) self.role_key = 'host' self.admin_methods = ['GET', 'PUT', 'POST'] - self.update_params = ["name"] self.uri_fmt = '/debugreports/%s' self.content = DebugReportContent(model, ident) diff --git a/src/kimchi/control/host.py b/src/kimchi/control/host.py index c690945..5e736db 100644 --- a/src/kimchi/control/host.py +++ b/src/kimchi/control/host.py @@ -1,7 +1,7 @@ # # Project Kimchi # -# Copyright IBM, Corp. 2013-2014 +# Copyright IBM, Corp. 2013-2015 # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -147,7 +147,6 @@ class Repository(Resource): super(Repository, self).__init__(model, id) self.role_key = 'host' self.admin_methods = ['GET', 'PUT', 'POST', 'DELETE'] - self.update_params = ["config", "baseurl"] self.uri_fmt = "/host/repositories/%s" self.enable = self.generate_action_handler('enable') self.disable = self.generate_action_handler('disable') diff --git a/src/kimchi/control/storagepools.py b/src/kimchi/control/storagepools.py index 460beb1..872ad04 100644 --- a/src/kimchi/control/storagepools.py +++ b/src/kimchi/control/storagepools.py @@ -1,7 +1,7 @@ # # Project Kimchi # -# Copyright IBM, Corp. 2013-2014 +# Copyright IBM, Corp. 2013-2015 # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -77,7 +77,6 @@ class StoragePool(Resource): super(StoragePool, self).__init__(model, ident) self.role_key = 'storage' self.admin_methods = ['PUT', 'POST', 'DELETE'] - self.update_params = ["autostart", "disks"] self.uri_fmt = "/storagepools/%s" self.activate = self.generate_action_handler('activate') self.deactivate = self.generate_action_handler('deactivate') diff --git a/src/kimchi/control/templates.py b/src/kimchi/control/templates.py index 70c9457..4c45910 100644 --- a/src/kimchi/control/templates.py +++ b/src/kimchi/control/templates.py @@ -1,7 +1,7 @@ # # Project Kimchi # -# Copyright IBM, Corp. 2013-2014 +# Copyright IBM, Corp. 2013-2015 # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -35,10 +35,6 @@ class Template(Resource): super(Template, self).__init__(model, ident) self.role_key = 'templates' self.admin_methods = ['PUT', 'POST', 'DELETE'] - self.update_params = ["name", "folder", "icon", "os_distro", - "storagepool", "os_version", "cpus", - "memory", "cdrom", "disks", "networks", - "graphics", "cpu_info"] self.uri_fmt = "/templates/%s" self.clone = self.generate_action_handler('clone') diff --git a/src/kimchi/control/vm/ifaces.py b/src/kimchi/control/vm/ifaces.py index ebcb044..e31210e 100644 --- a/src/kimchi/control/vm/ifaces.py +++ b/src/kimchi/control/vm/ifaces.py @@ -1,7 +1,7 @@ # # Project Kimchi # -# Copyright IBM, Corp. 2014 +# Copyright IBM, Corp. 2014-2015 # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -34,7 +34,6 @@ class VMIfaces(Collection): class VMIface(Resource): def __init__(self, model, vm, ident): super(VMIface, self).__init__(model, ident) - self.update_params = ["model", "network"] self.vm = vm self.ident = ident self.info = {} diff --git a/src/kimchi/control/vm/storages.py b/src/kimchi/control/vm/storages.py index 984c4d2..81a5d48 100644 --- a/src/kimchi/control/vm/storages.py +++ b/src/kimchi/control/vm/storages.py @@ -1,7 +1,7 @@ # # Project Kimchi # -# Copyright IBM, Corp. 2014 +# Copyright IBM, Corp. 2014-2015 # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -39,7 +39,6 @@ class VMStorage(Resource): self.info = {} self.model_args = [self.vm, self.ident] self.uri_fmt = '/vms/%s/storages/%s' - self.update_params = ['path'] @property def data(self): diff --git a/src/kimchi/control/vms.py b/src/kimchi/control/vms.py index a1589ef..1a84b6c 100644 --- a/src/kimchi/control/vms.py +++ b/src/kimchi/control/vms.py @@ -1,7 +1,7 @@ # # Project Kimchi # -# Copyright IBM, Corp. 2013-2014 +# Copyright IBM, Corp. 2013-2015 # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -35,8 +35,6 @@ class VM(Resource): def __init__(self, model, ident): super(VM, self).__init__(model, ident) self.role_key = 'guests' - self.update_params = ["name", "users", "groups", "cpus", "memory", - "graphics"] self.screenshot = VMScreenShot(model, ident) self.uri_fmt = '/vms/%s' for ident, node in sub_nodes.items(): diff --git a/src/kimchi/i18n.py b/src/kimchi/i18n.py index e3a051c..8717ada 100644 --- a/src/kimchi/i18n.py +++ b/src/kimchi/i18n.py @@ -1,7 +1,7 @@ # # Project Kimchi # -# Copyright IBM, Corp. 2014 +# Copyright IBM, Corp. 2014-2015 # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -26,7 +26,6 @@ messages = { "KCHAPI0001E": _("Unknown parameter %(value)s"), "KCHAPI0002E": _("Delete is not allowed for %(resource)s"), "KCHAPI0003E": _("%(resource)s does not implement update method"), - "KCHAPI0004E": _("Parameters %(params)s are not allowed to be updated in %(resource)s"), "KCHAPI0005E": _("Create is not allowed for %(resource)s"), "KCHAPI0006E": _("Unable to parse JSON request"), "KCHAPI0007E": _("This API only supports JSON"), diff --git a/tests/test_rest.py b/tests/test_rest.py index e72a2e0..4e79b55 100644 --- a/tests/test_rest.py +++ b/tests/test_rest.py @@ -217,6 +217,10 @@ class RestTests(unittest.TestCase): resp = self.request('/vms/vm-1/start', '{}', 'POST') self.assertEquals(200, resp.status) + req = json.dumps({'unsupported-attr': 'attr'}) + resp = self.request('/vms/vm-1', req, 'PUT') + self.assertEquals(400, resp.status) + req = json.dumps({'name': 'new-vm'}) resp = self.request('/vms/vm-1', req, 'PUT') self.assertEquals(400, resp.status) @@ -271,7 +275,7 @@ class RestTests(unittest.TestCase): req = json.dumps({'name': 'new-name', 'cpus': 5, 'UUID': 'notallowed'}) resp = self.request('/vms/vm-1', req, 'PUT') - self.assertEquals(405, resp.status) + self.assertEquals(400, resp.status) params = {'name': u'∨м-црdαtеd', 'cpus': 5, 'memory': 4096} req = json.dumps(params) -- 1.9.3