Re: [Kimchi-devel] [PATCH] Improve PUT param checking

I suppose this change affects Kimchi plug-ins as well, such as Ginger?

Daniel On 01/05/2015 06:44 AM, lvroyce(a)linux.vnet.ibm.com wrote: > From: Royce Lv <lvroyce(a)linux.vnet.ibm.com&gt; > > 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 | 12 ++++++++---- > src/kimchi/control/base.py | 10 ---------- > src/kimchi/control/debugreports.py | 1 - > src/kimchi/control/host.py | 1 - > src/kimchi/control/storagepools.py | 1 - > src/kimchi/control/templates.py | 4 ---- > src/kimchi/control/vm/ifaces.py | 1 - > src/kimchi/control/vm/storages.py | 1 - > src/kimchi/control/vms.py | 2 -- > tests/test_rest.py | 6 +++++- > 10 files changed, 13 insertions(+), 26 deletions(-) > > diff --git a/src/kimchi/API.json b/src/kimchi/API.json > index c5d7bdb..fd4c901 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", > @@ -554,7 +557,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..bb2ce06 100644 > --- a/src/kimchi/control/base.py > +++ b/src/kimchi/control/base.py > @@ -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..359fe5e 100644 > --- a/src/kimchi/control/debugreports.py > +++ b/src/kimchi/control/debugreports.py > @@ -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..2c037ac 100644 > --- a/src/kimchi/control/host.py > +++ b/src/kimchi/control/host.py > @@ -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..4f455b0 100644 > --- a/src/kimchi/control/storagepools.py > +++ b/src/kimchi/control/storagepools.py > @@ -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..98b949f 100644 > --- a/src/kimchi/control/templates.py > +++ b/src/kimchi/control/templates.py > @@ -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..f761660 100644 > --- a/src/kimchi/control/vm/ifaces.py > +++ b/src/kimchi/control/vm/ifaces.py > @@ -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..58b0771 100644 > --- a/src/kimchi/control/vm/storages.py > +++ b/src/kimchi/control/vm/storages.py > @@ -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..0d41e14 100644 > --- a/src/kimchi/control/vms.py > +++ b/src/kimchi/control/vms.py > @@ -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/tests/test_rest.py b/tests/test_rest.py > index fa15ef6..1ddbd86 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) _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel