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

Aline Manera alinefm at linux.vnet.ibm.com
Thu Jan 8 18:35:35 UTC 2015


Please, run "make check-local" and fix the issues.

I got:

alinefm at alinefm:~/kimchi$ sudo make check-local
PYTHONPATH=src contrib/check_i18n.py plugins/*/i18n.py src/kimchi/i18n.py
Checking for invalid i18n string...
   KCHAPI0004E is obsolete, it is no longer in use
Makefile:828: recipe for target 'check-local' failed
make: *** [check-local] Error 1

Also we need to update the copyright date to include 2015 when modifying 
files.

And one more comment below:

On 05/01/2015 06:44, lvroyce at linux.vnet.ibm.com wrote:
> From: Royce Lv <lvroyce at linux.vnet.ibm.com>
>
> Delete self-defined update param validation,
> and use additionalProperties in json schema validator instead.
>
> Signed-off-by: Royce Lv <lvroyce at linux.vnet.ibm.com>
> ---
>   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

Seems vmiface_update was not updated in API.json

> 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)




More information about the Kimchi-devel mailing list