openstack
/
ironic
Code Issues Proposed changes

Get mandatory patch attrs from WSME properties 

Attributes which are mandatory (ie, required for object creation) should
not be removable. However, some attributes (such as Node.chassis_uuid)
are not required for object creation, but should not be removable if
they are set. This commit does the following:

 - rename JsonPatchType.mandatory_attrs to non_removable_attrs to better
   describe its meaning,
 - change its return type to set-of-strings for faster lookup
 - ensure all mandatory attributes on the type being patched are
   included in the set of non-removable attributes,
 - add a new field, JsonPatchType._extra_non_removable_attrs, which
   should be a set of attributes that are not required for creation but
   should not be removed if set.

Since the object to be patched does not exist at patch-validation time,
we leave the validation logic in methods of JsonPatchType and
subclasses. This means introspecting the types to be patched.

Closes-Bug: #1284781

Change-Id: I2bb7fed2c776c8d63535c5ee19cdc218e57806e3
This commit is contained in:
Miles Gould 2015-10-28 17:59:42 +00:00
parent ced6d2f557
commit 34b3589ea3
6 changed files with 72 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
ironic/api/controllers/v1/chassis.py
View File

@ -36,10 +36,6 @@ from ironic import objects
_DEFAULT_RETURN_FIELDS = ('uuid', 'description')
class ChassisPatchType(types.JsonPatchType):
pass
class Chassis(base.APIBase):
"""API representation of a chassis.
@ -122,6 +118,11 @@ class Chassis(base.APIBase):
fields=fields)
class ChassisPatchType(types.JsonPatchType):
_api_base = Chassis
class ChassisCollection(collection.Collection):
"""API representation of a collection of chassis."""

40
ironic/api/controllers/v1/node.py
View File

@ -110,26 +110,6 @@ def check_allow_management_verbs(verb):
raise exception.NotAcceptable()
class NodePatchType(types.JsonPatchType):
@staticmethod
def internal_attrs():
defaults = types.JsonPatchType.internal_attrs()
# TODO(lucasagomes): Include maintenance once the endpoint
# v1/nodes/<uuid>/maintenance do more things than updating the DB.
return defaults + ['/console_enabled', '/last_error',
'/power_state', '/provision_state', '/reservation',
'/target_power_state', '/target_provision_state',
'/provision_updated_at', '/maintenance_reason',
'/driver_internal_info', '/inspection_finished_at',
'/inspection_started_at', '/clean_step',
'/raid_config', '/target_raid_config']
@staticmethod
def mandatory_attrs():
return ['/chassis_uuid', '/driver']
class BootDeviceController(rest.RestController):
_custom_actions = {
@ -735,6 +715,26 @@ class Node(base.APIBase):
fields=fields)
class NodePatchType(types.JsonPatchType):
_api_base = Node
_extra_non_removable_attrs = {'/chassis_uuid'}
@staticmethod
def internal_attrs():
defaults = types.JsonPatchType.internal_attrs()
# TODO(lucasagomes): Include maintenance once the endpoint
# v1/nodes/<uuid>/maintenance do more things than updating the DB.
return defaults + ['/console_enabled', '/last_error',
'/power_state', '/provision_state', '/reservation',
'/target_power_state', '/target_provision_state',
'/provision_updated_at', '/maintenance_reason',
'/driver_internal_info', '/inspection_finished_at',
'/inspection_started_at', '/clean_step',
'/raid_config', '/target_raid_config']
class NodeCollection(collection.Collection):
"""API representation of a collection of nodes."""

11
ironic/api/controllers/v1/port.py
View File

@ -36,13 +36,6 @@ from ironic import objects
_DEFAULT_RETURN_FIELDS = ('uuid', 'address')
class PortPatchType(types.JsonPatchType):
@staticmethod
def mandatory_attrs():
return ['/address', '/node_uuid']
class Port(base.APIBase):
"""API representation of a port.
@ -155,6 +148,10 @@ class Port(base.APIBase):
fields=fields)
class PortPatchType(types.JsonPatchType):
_api_base = Port
class PortCollection(collection.Collection):
"""API representation of a collection of ports."""

37
ironic/api/controllers/v1/types.py
View File

@ -15,6 +15,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import inspect
import json
from oslo_utils import strutils
@ -194,6 +195,17 @@ class JsonPatchType(wtypes.Base):
mandatory=True)
value = wsme.wsattr(jsontype, default=wtypes.Unset)
# The class of the objects being patched. Override this in subclasses.
# Should probably be a subclass of ironic.api.controllers.base.APIBase.
_api_base = None
# Attributes that are not required for construction, but which may not be
# removed if set. Override in subclasses if needed.
_extra_non_removable_attrs = set()
# Set of non-removable attributes, calculated lazily.
_non_removable_attrs = None
@staticmethod
def internal_attrs():
"""Returns a list of internal attributes.
@ -204,15 +216,24 @@ class JsonPatchType(wtypes.Base):
"""
return ['/created_at', '/id', '/links', '/updated_at', '/uuid']
@staticmethod
def mandatory_attrs():
"""Retruns a list of mandatory attributes.
Mandatory attributes can't be removed from the document. This
method should be overwritten by derived class.
@classmethod
def non_removable_attrs(cls):
"""Returns a set of names of attributes that may not be removed.
Attributes whose 'mandatory' property is True are automatically added
to this set. To add additional attributes to the set, override the
field _extra_non_removable_attrs in subclasses, with a set of the form
{'/foo', '/bar'}.
"""
return []
if cls._non_removable_attrs is None:
cls._non_removable_attrs = cls._extra_non_removable_attrs.copy()
if cls._api_base:
fields = inspect.getmembers(cls._api_base,
lambda a: not inspect.isroutine(a))
for name, field in fields:
if getattr(field, 'mandatory', False):
cls._non_removable_attrs.add('/%s' % name)
return cls._non_removable_attrs
@staticmethod
def validate(patch):
@ -221,7 +242,7 @@ class JsonPatchType(wtypes.Base):
msg = _("'%s' is an internal attribute and can not be updated")
raise wsme.exc.ClientSideError(msg % patch.path)
if patch.path in patch.mandatory_attrs() and patch.op == 'remove':
if patch.path in patch.non_removable_attrs() and patch.op == 'remove':
msg = _("'%s' is a mandatory attribute and can not be removed")
raise wsme.exc.ClientSideError(msg % patch.path)

1
ironic/tests/unit/api/v1/test_ports.py
View File

@ -562,6 +562,7 @@ class TestPatch(test_api_base.BaseApiTest):
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.BAD_REQUEST, response.status_code)
self.assertTrue(response.json['error_message'])
self.assertIn('mandatory attribute', response.json['error_message'])
self.assertFalse(mock_upd.called)
def test_add_root(self, mock_upd):

17
ironic/tests/unit/api/v1/test_types.py
View File

@ -88,12 +88,15 @@ class TestUuidOrNameType(base.TestCase):
types.UuidOrNameType.validate, 'inval#uuid%or*name')
class MyBaseType(object):
"""Helper class, patched by objects of type MyPatchType"""
mandatory = wsme.wsattr(wtypes.text, mandatory=True)
class MyPatchType(types.JsonPatchType):
"""Helper class for TestJsonPatchType tests."""
@staticmethod
def mandatory_attrs():
return ['/mandatory']
_api_base = MyBaseType
_extra_non_removable_attrs = {'/non_removable'}
@staticmethod
def internal_attrs():
@ -162,6 +165,12 @@ class TestJsonPatchType(base.TestCase):
self.assertEqual(http_client.BAD_REQUEST, ret.status_int)
self.assertTrue(ret.json['faultstring'])
def test_cannot_remove_extra_non_removable_attr(self):
patch = [{'op': 'remove', 'path': '/non_removable'}]
ret = self._patch_json(patch, True)
self.assertEqual(http_client.BAD_REQUEST, ret.status_int)
self.assertTrue(ret.json['faultstring'])
def test_missing_required_fields_path(self):
missing_path = [{'op': 'remove'}]
ret = self._patch_json(missing_path, True)