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

lvroyce at linux.vnet.ibm.com lvroyce at linux.vnet.ibm.com
Tue Jan 13 09:04:16 UTC 2015


From: Royce Lv <lvroyce at linux.vnet.ibm.com>

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 at linux.vnet.ibm.com>
---
 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




More information about the Kimchi-devel mailing list