diff --git a/ironic/objects/node.py b/ironic/objects/node.py index b358c3adf9..1c12229a0f 100644 --- a/ironic/objects/node.py +++ b/ironic/objects/node.py @@ -18,10 +18,13 @@ from oslo_utils import uuidutils from oslo_versionedobjects import base as object_base from ironic.common import exception +from ironic.common.i18n import _ from ironic.db import api as db_api from ironic.objects import base from ironic.objects import fields as object_fields +REQUIRED_INT_PROPERTIES = ['local_gb', 'cpus', 'memory_mb'] + @base.IronicObjectRegistry.register class Node(base.IronicObject, object_base.VersionedObjectDictCompat): @@ -40,7 +43,9 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): # Version 1.11: Add clean_step # Version 1.12: Add raid_config and target_raid_config # Version 1.13: Add touch_provisioning() - VERSION = '1.13' + # Version 1.14: Add _validate_property_values() and make create() + # and save() validate the input of property values. + VERSION = '1.14' dbapi = db_api.get_instance() @@ -106,6 +111,33 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): node.obj_reset_changes() return node + def _validate_property_values(self, properties): + """Check if the input of local_gb, cpus and memory_mb are valid. + + :param properties: a dict contains the node's information. + """ + if not properties: + return + invalid_msgs_list = [] + for param in REQUIRED_INT_PROPERTIES: + value = properties.get(param) + if value is None: + continue + try: + int_value = int(value) + assert int_value >= 0 + except (ValueError, AssertionError): + msg = (('%(param)s=%(value)s') % + {'param': param, 'value': value}) + invalid_msgs_list.append(msg) + + if invalid_msgs_list: + msg = (_('The following properties for node %(node)s ' + 'should be non-negative integers, ' + 'but provided values are: %(msgs)s') % + {'node': self.uuid, 'msgs': ', '.join(invalid_msgs_list)}) + raise exception.InvalidParameterValue(msg) + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable # methods can be used in the future to replace current explicit RPC calls. # Implications of calling new remote procedures should be thought through. @@ -263,9 +295,12 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): argument, even though we don't use it. A context should be set when instantiating the object, e.g.: Node(context) - + :raises: InvalidParameterValue if some property values are invalid. """ values = self.obj_get_changes() + properties = values.get('properties') + if properties: + self._validate_property_values(properties) db_node = self.dbapi.create_node(values) self._from_db_object(self, db_node) @@ -304,8 +339,12 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): argument, even though we don't use it. A context should be set when instantiating the object, e.g.: Node(context) + :raises: InvalidParameterValue if some property values are invalid. """ updates = self.obj_get_changes() + properties = updates.get('properties') + if properties: + self._validate_property_values(properties) if 'driver' in updates and 'driver_internal_info' not in updates: # Clean driver_internal_info when changes driver self.driver_internal_info = {} diff --git a/ironic/tests/unit/objects/test_node.py b/ironic/tests/unit/objects/test_node.py index 09ffc6effa..1d99de4c4c 100644 --- a/ironic/tests/unit/objects/test_node.py +++ b/ironic/tests/unit/objects/test_node.py @@ -149,3 +149,41 @@ class TestNodeObject(base.DbTestCase): node = objects.Node.get(self.context, self.fake_node['uuid']) node.touch_provisioning() mock_touch.assert_called_once_with(node.id) + + def test_create_with_invalid_properties(self): + node = objects.Node(self.context, **self.fake_node) + node.properties = {"local_gb": "5G"} + self.assertRaises(exception.InvalidParameterValue, node.create) + + def test_update_with_invalid_properties(self): + uuid = self.fake_node['uuid'] + with mock.patch.object(self.dbapi, 'get_node_by_uuid', + autospec=True) as mock_get_node: + mock_get_node.return_value = self.fake_node + node = objects.Node.get(self.context, uuid) + node.properties = {"local_gb": "5G", "memory_mb": "5", + 'cpus': '-1', 'cpu_arch': 'x86_64'} + expected_error_message = ( + ('The following properties for node %(node_uuid)s ' + 'should be non-negative integers, but provided values ' + 'are: local_gb=5G, cpus=-1') % + {'node_uuid': uuid}) + self.assertRaisesRegexp(exception.InvalidParameterValue, + expected_error_message, node.save) + mock_get_node.assert_called_once_with(uuid) + + def test__validate_property_values_success(self): + uuid = self.fake_node['uuid'] + with mock.patch.object(self.dbapi, 'get_node_by_uuid', + autospec=True) as mock_get_node: + mock_get_node.return_value = self.fake_node + node = objects.Node.get(self.context, uuid) + values = self.fake_node + expect = { + 'cpu_arch': 'x86_64', + "cpus": '8', + "local_gb": '10', + "memory_mb": '4096', + } + node._validate_property_values(values['properties']) + self.assertEqual(expect, values['properties'])