diff options
Diffstat (limited to 'ironic')
| -rw-r--r-- | ironic/api/controllers/v1/portgroup.py | 436 | ||||
| -rw-r--r-- | ironic/tests/unit/api/controllers/v1/test_port.py | 2 | ||||
| -rw-r--r-- | ironic/tests/unit/api/controllers/v1/test_portgroup.py | 14 | ||||
| -rw-r--r-- | ironic/tests/unit/api/utils.py | 4 |
4 files changed, 175 insertions, 281 deletions
diff --git a/ironic/api/controllers/v1/portgroup.py b/ironic/api/controllers/v1/portgroup.py index 2697ec4e7..e1e49d652 100644 --- a/ironic/api/controllers/v1/portgroup.py +++ b/ironic/api/controllers/v1/portgroup.py @@ -10,7 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. -import datetime +import copy from http import client as http_client from ironic_lib import metrics_utils @@ -18,15 +18,13 @@ from oslo_utils import uuidutils import pecan from ironic import api -from ironic.api.controllers import base from ironic.api.controllers import link from ironic.api.controllers.v1 import collection from ironic.api.controllers.v1 import notification_utils as notify from ironic.api.controllers.v1 import port -from ironic.api.controllers.v1 import types from ironic.api.controllers.v1 import utils as api_utils -from ironic.api import expose -from ironic.api import types as atypes +from ironic.api import method +from ironic.common import args from ironic.common import exception from ironic.common.i18n import _ from ironic.common import policy @@ -35,217 +33,103 @@ from ironic import objects METRICS = metrics_utils.get_metrics_logger(__name__) -_DEFAULT_RETURN_FIELDS = ('uuid', 'address', 'name') - - -class Portgroup(base.APIBase): - """API representation of a portgroup. - - This class enforces type checking and value constraints, and converts - between the internal object model and the API representation of a - portgroup. - """ - - _node_uuid = None - - def _get_node_uuid(self): - return self._node_uuid - - def _set_node_uuid(self, value): - if value and self._node_uuid != value: - if not api_utils.allow_portgroups(): - self._node_uuid = atypes.Unset - return - try: - node = objects.Node.get(api.request.context, value) - self._node_uuid = node.uuid - # NOTE: Create the node_id attribute on-the-fly - # to satisfy the api -> rpc object - # conversion. - self.node_id = node.id - except exception.NodeNotFound as e: - # Change error code because 404 (NotFound) is inappropriate - # response for a POST request to create a Portgroup - e.code = http_client.BAD_REQUEST - raise e - elif value == atypes.Unset: - self._node_uuid = atypes.Unset - - uuid = types.uuid - """Unique UUID for this portgroup""" - - address = atypes.wsattr(types.macaddress) - """MAC Address for this portgroup""" - - extra = {str: types.jsontype} - """This portgroup's meta data""" - - internal_info = atypes.wsattr({str: types.jsontype}, readonly=True) - """This portgroup's internal info""" - - node_uuid = atypes.wsproperty(types.uuid, _get_node_uuid, _set_node_uuid, - mandatory=True) - """The UUID of the node this portgroup belongs to""" - - name = atypes.wsattr(str) - """The logical name for this portgroup""" - - links = None - """A list containing a self link and associated portgroup links""" - - standalone_ports_supported = types.boolean - """Indicates whether ports of this portgroup may be used as - single NIC ports""" - - mode = atypes.wsattr(str) - """The mode for this portgroup. See linux bonding - documentation for details: - https://www.kernel.org/doc/Documentation/networking/bonding.txt""" - - properties = {str: types.jsontype} - """This portgroup's properties""" - - ports = None - """Links to the collection of ports of this portgroup""" - - def __init__(self, **kwargs): - self.fields = [] - fields = list(objects.Portgroup.fields) - # NOTE: node_uuid is not part of objects.Portgroup.fields - # because it's an API-only attribute - fields.append('node_uuid') - for field in fields: - # Skip fields we do not expose. - if not hasattr(self, field): - continue - self.fields.append(field) - setattr(self, field, kwargs.get(field, atypes.Unset)) - - # NOTE: node_id is an attribute created on-the-fly - # by _set_node_uuid(), it needs to be present in the fields so - # that as_dict() will contain node_id field when converting it - # before saving it in the database. - self.fields.append('node_id') - setattr(self, 'node_uuid', kwargs.get('node_id', atypes.Unset)) - - @staticmethod - def _convert_with_links(portgroup, url, fields=None): - """Add links to the portgroup.""" - if fields is None: - portgroup.ports = [ - link.make_link('self', url, 'portgroups', - portgroup.uuid + "/ports"), - link.make_link('bookmark', url, 'portgroups', - portgroup.uuid + "/ports", bookmark=True) - ] - - # never expose the node_id attribute - portgroup.node_id = atypes.Unset - - portgroup.links = [link.make_link('self', url, - 'portgroups', portgroup.uuid), - link.make_link('bookmark', url, - 'portgroups', portgroup.uuid, - bookmark=True) - ] +_DEFAULT_RETURN_FIELDS = ['uuid', 'address', 'name'] + +PORTGROUP_SCHEMA = { + 'type': 'object', + 'properties': { + 'address': {'type': ['string', 'null']}, + 'extra': {'type': ['object', 'null']}, + 'mode': {'type': ['string', 'null']}, + 'name': {'type': ['string', 'null']}, + 'node_uuid': {'type': 'string'}, + 'properties': {'type': ['object', 'null']}, + 'standalone_ports_supported': {'type': ['string', 'boolean', 'null']}, + 'uuid': {'type': ['string', 'null']}, + }, + 'required': ['node_uuid'], + 'additionalProperties': False, +} + +PORTGROUP_PATCH_SCHEMA = copy.deepcopy(PORTGROUP_SCHEMA) +# patch supports patching some internal_info values +PORTGROUP_PATCH_SCHEMA['properties']['internal_info'] = { + 'type': ['null', 'object']} + +PORTGROUP_VALIDATOR_EXTRA = args.dict_valid( + address=args.mac_address, + node_uuid=args.uuid, + standalone_ports_supported=args.boolean, + uuid=args.uuid +) +PORTGROUP_VALIDATOR = args.and_valid( + args.schema(PORTGROUP_SCHEMA), + PORTGROUP_VALIDATOR_EXTRA +) + +PORTGROUP_PATCH_VALIDATOR = args.and_valid( + args.schema(PORTGROUP_PATCH_SCHEMA), + PORTGROUP_VALIDATOR_EXTRA +) + +PATCH_ALLOWED_FIELDS = [ + 'address', + 'extra', + 'mode', + 'name', + 'node_uuid', + 'properties', + 'standalone_ports_supported' +] + + +def convert_with_links(rpc_portgroup, fields=None, sanitize=True): + """Add links to the portgroup.""" + portgroup = api_utils.object_to_dict( + rpc_portgroup, + link_resource='portgroups', + fields=( + 'address', + 'extra', + 'internal_info', + 'mode', + 'name', + 'properties', + 'standalone_ports_supported' + ) + ) + api_utils.populate_node_uuid(rpc_portgroup, portgroup) + url = api.request.public_url + portgroup['ports'] = [ + link.make_link('self', url, 'portgroups', + rpc_portgroup.uuid + "/ports"), + link.make_link('bookmark', url, 'portgroups', + rpc_portgroup.uuid + "/ports", bookmark=True) + ] + + if fields is not None: + api_utils.check_for_invalid_fields(fields, portgroup) + + if not sanitize: return portgroup - @classmethod - def convert_with_links(cls, rpc_portgroup, fields=None, sanitize=True): - """Add links to the portgroup.""" - portgroup = Portgroup(**rpc_portgroup.as_dict()) + api_utils.sanitize_dict(portgroup, fields) - if fields is not None: - api_utils.check_for_invalid_fields(fields, portgroup.as_dict()) + return portgroup - portgroup = cls._convert_with_links(portgroup, api.request.host_url, - fields=fields) - if not sanitize: - return portgroup - - portgroup.sanitize(fields) - - return portgroup - - def sanitize(self, fields=None): - """Removes sensitive and unrequested data. - - Will only keep the fields specified in the ``fields`` parameter. - - :param fields: - list of fields to preserve, or ``None`` to preserve them all - :type fields: list of str - """ - - if fields is not None: - self.unset_fields_except(fields) - - # never expose the node_id attribute - self.node_id = atypes.Unset - - @classmethod - def sample(cls, expand=True): - """Return a sample of the portgroup.""" - sample = cls(uuid='a594544a-2daf-420c-8775-17a8c3e0852f', - address='fe:54:00:77:07:d9', - name='node1-portgroup-01', - extra={'foo': 'bar'}, - internal_info={'baz': 'boo'}, - standalone_ports_supported=True, - mode='active-backup', - properties={}, - created_at=datetime.datetime(2000, 1, 1, 12, 0, 0), - updated_at=datetime.datetime(2000, 1, 1, 12, 0, 0)) - # NOTE(lucasagomes): node_uuid getter() method look at the - # _node_uuid variable - sample._node_uuid = '7ae81bb3-dec3-4289-8d6c-da80bd8001ae' - fields = None if expand else _DEFAULT_RETURN_FIELDS - return cls._convert_with_links(sample, 'http://localhost:6385', - fields=fields) - - -class PortgroupPatchType(types.JsonPatchType): - - _api_base = Portgroup - _extra_non_removable_attrs = {'/mode'} - - @staticmethod - def internal_attrs(): - defaults = types.JsonPatchType.internal_attrs() - return defaults + ['/internal_info'] - - -class PortgroupCollection(collection.Collection): - """API representation of a collection of portgroups.""" - - portgroups = [Portgroup] - """A list containing portgroup objects""" - - def __init__(self, **kwargs): - self._type = 'portgroups' - - @staticmethod - def convert_with_links(rpc_portgroups, limit, url=None, fields=None, - **kwargs): - collection = PortgroupCollection() - collection.portgroups = [Portgroup.convert_with_links(p, fields=fields, - sanitize=False) - for p in rpc_portgroups] - collection.next = collection.get_next(limit, url=url, fields=fields, - **kwargs) - - for item in collection.portgroups: - item.sanitize(fields=fields) - - return collection - - @classmethod - def sample(cls): - """Return a sample of the portgroup.""" - sample = cls() - sample.portgroups = [Portgroup.sample(expand=False)] - return sample +def list_convert_with_links(rpc_portgroups, limit, url=None, fields=None, + **kwargs): + return collection.list_convert_with_links( + items=[convert_with_links(p, fields=fields, sanitize=False) + for p in rpc_portgroups], + item_name='portgroups', + limit=limit, + url=url, + fields=fields, + sanitize_func=api_utils.sanitize_dict, + **kwargs + ) class PortgroupsController(pecan.rest.RestController): @@ -266,8 +150,8 @@ class PortgroupsController(pecan.rest.RestController): if not api_utils.allow_portgroups(): pecan.abort(http_client.NOT_FOUND) try: - ident = types.uuid_or_name.validate(ident) - except exception.InvalidUuidOrName as e: + ident = args.uuid_or_name('portgroup', ident) + except exception.InvalidParameterValue as e: pecan.abort(http_client.BAD_REQUEST, e.args[0]) if not remainder: return @@ -333,12 +217,12 @@ class PortgroupsController(pecan.rest.RestController): if detail is not None: parameters['detail'] = detail - return PortgroupCollection.convert_with_links(portgroups, limit, - url=resource_url, - fields=fields, - sort_key=sort_key, - sort_dir=sort_dir, - **parameters) + return list_convert_with_links(portgroups, limit, + url=resource_url, + fields=fields, + sort_key=sort_key, + sort_dir=sort_dir, + **parameters) def _get_portgroups_by_address(self, address): """Retrieve a portgroup by its address. @@ -357,9 +241,11 @@ class PortgroupsController(pecan.rest.RestController): return [] @METRICS.timer('PortgroupsController.get_all') - @expose.expose(PortgroupCollection, types.uuid_or_name, types.macaddress, - types.uuid, int, str, str, types.listtype, - types.boolean) + @method.expose() + @args.validate(node=args.uuid_or_name, address=args.mac_address, + marker=args.uuid, limit=args.integer, sort_key=args.string, + sort_dir=args.string, fields=args.string_list, + detail=args.boolean) def get_all(self, node=None, address=None, marker=None, limit=None, sort_key='id', sort_dir='asc', fields=None, detail=None): @@ -398,8 +284,10 @@ class PortgroupsController(pecan.rest.RestController): detail=detail) @METRICS.timer('PortgroupsController.detail') - @expose.expose(PortgroupCollection, types.uuid_or_name, types.macaddress, - types.uuid, int, str, str) + @method.expose() + @args.validate(node=args.uuid_or_name, address=args.mac_address, + marker=args.uuid, limit=args.integer, sort_key=args.string, + sort_dir=args.string) def detail(self, node=None, address=None, marker=None, limit=None, sort_key='id', sort_dir='asc'): """Retrieve a list of portgroups with detail. @@ -434,7 +322,8 @@ class PortgroupsController(pecan.rest.RestController): resource_url=resource_url) @METRICS.timer('PortgroupsController.get_one') - @expose.expose(Portgroup, types.uuid_or_name, types.listtype) + @method.expose() + @args.validate(portgroup_ident=args.uuid_or_name, fields=args.string_list) def get_one(self, portgroup_ident, fields=None): """Retrieve information about the given portgroup. @@ -455,10 +344,12 @@ class PortgroupsController(pecan.rest.RestController): rpc_portgroup = api_utils.get_rpc_portgroup_with_suffix( portgroup_ident) - return Portgroup.convert_with_links(rpc_portgroup, fields=fields) + return convert_with_links(rpc_portgroup, fields=fields) @METRICS.timer('PortgroupsController.post') - @expose.expose(Portgroup, body=Portgroup, status_code=http_client.CREATED) + @method.expose(status_code=http_client.CREATED) + @method.body('portgroup') + @args.validate(portgroup=PORTGROUP_VALIDATOR) def post(self, portgroup): """Create a new portgroup. @@ -475,43 +366,43 @@ class PortgroupsController(pecan.rest.RestController): raise exception.OperationNotPermitted() if (not api_utils.allow_portgroup_mode_properties() - and (portgroup.mode is not atypes.Unset - or portgroup.properties is not atypes.Unset)): + and (portgroup.get('mode') or portgroup.get('properties'))): raise exception.NotAcceptable() - if (portgroup.name - and not api_utils.is_valid_logical_name(portgroup.name)): + if (portgroup.get('name') + and not api_utils.is_valid_logical_name(portgroup['name'])): error_msg = _("Cannot create portgroup with invalid name " - "'%(name)s'") % {'name': portgroup.name} + "'%(name)s'") % {'name': portgroup['name']} raise exception.ClientSideError( error_msg, status_code=http_client.BAD_REQUEST) - pg_dict = portgroup.as_dict() - - api_utils.handle_post_port_like_extra_vif(pg_dict) + api_utils.handle_post_port_like_extra_vif(portgroup) # NOTE(yuriyz): UUID is mandatory for notifications payload - if not pg_dict.get('uuid'): - pg_dict['uuid'] = uuidutils.generate_uuid() + if not portgroup.get('uuid'): + portgroup['uuid'] = uuidutils.generate_uuid() - new_portgroup = objects.Portgroup(context, **pg_dict) + node = api_utils.replace_node_uuid_with_id(portgroup) + + new_portgroup = objects.Portgroup(context, **portgroup) notify.emit_start_notification(context, new_portgroup, 'create', - node_uuid=portgroup.node_uuid) + node_uuid=node.uuid) with notify.handle_error_notification(context, new_portgroup, 'create', - node_uuid=portgroup.node_uuid): + node_uuid=node.uuid): new_portgroup.create() notify.emit_end_notification(context, new_portgroup, 'create', - node_uuid=portgroup.node_uuid) + node_uuid=node.uuid) # Set the HTTP Location Header api.response.location = link.build_url('portgroups', new_portgroup.uuid) - return Portgroup.convert_with_links(new_portgroup) + return convert_with_links(new_portgroup) @METRICS.timer('PortgroupsController.patch') - @expose.validate(types.uuid_or_name, [PortgroupPatchType]) - @expose.expose(Portgroup, types.uuid_or_name, body=[PortgroupPatchType]) + @method.expose() + @method.body('patch') + @args.validate(portgroup_ident=args.uuid_or_name, patch=args.patch) def patch(self, portgroup_ident, patch): """Update an existing portgroup. @@ -533,6 +424,8 @@ class PortgroupsController(pecan.rest.RestController): or api_utils.is_path_updated(patch, '/properties'))): raise exception.NotAcceptable() + api_utils.patch_validate_allowed_fields(patch, PATCH_ALLOWED_FIELDS) + rpc_portgroup = api_utils.get_rpc_portgroup_with_suffix( portgroup_ident) @@ -547,30 +440,41 @@ class PortgroupsController(pecan.rest.RestController): error_msg, status_code=http_client.BAD_REQUEST) portgroup_dict = rpc_portgroup.as_dict() + # NOTE: # 1) Remove node_id because it's an internal value and # not present in the API object # 2) Add node_uuid - portgroup_dict['node_uuid'] = portgroup_dict.pop('node_id', None) - portgroup = Portgroup(**api_utils.apply_jsonpatch(portgroup_dict, - patch)) + rpc_node = api_utils.replace_node_id_with_uuid(portgroup_dict) + + portgroup_dict = api_utils.apply_jsonpatch(portgroup_dict, patch) + + if 'mode' not in portgroup_dict: + msg = _("'mode' is a mandatory attribute and can not be removed") + raise exception.ClientSideError(msg) + + try: + if portgroup_dict['node_uuid'] != rpc_node.uuid: + rpc_node = objects.Node.get(api.request.context, + portgroup_dict['node_uuid']) + + except exception.NodeNotFound as e: + # Change error code because 404 (NotFound) is inappropriate + # response for a POST request to patch a Portgroup + e.code = http_client.BAD_REQUEST # BadRequest + raise api_utils.handle_patch_port_like_extra_vif( - rpc_portgroup, portgroup.internal_info, patch) - - # Update only the fields that have changed - for field in objects.Portgroup.fields: - try: - patch_val = getattr(portgroup, field) - except AttributeError: - # Ignore fields that aren't exposed in the API - continue - if patch_val == atypes.Unset: - patch_val = None - if rpc_portgroup[field] != patch_val: - rpc_portgroup[field] = patch_val - - rpc_node = objects.Node.get_by_id(context, rpc_portgroup.node_id) + rpc_portgroup, portgroup_dict.get('internal_info'), patch) + + api_utils.patched_validate_with_schema( + portgroup_dict, PORTGROUP_PATCH_SCHEMA, PORTGROUP_PATCH_VALIDATOR) + + api_utils.patch_update_changed_fields( + portgroup_dict, rpc_portgroup, fields=objects.Portgroup.fields, + schema=PORTGROUP_PATCH_SCHEMA, id_map={'node_id': rpc_node.id} + ) + if (rpc_node.provision_state == ir_states.INSPECTING and api_utils.allow_inspect_wait_state()): msg = _('Cannot update portgroup "%(portgroup)s" on node ' @@ -588,15 +492,15 @@ class PortgroupsController(pecan.rest.RestController): new_portgroup = api.request.rpcapi.update_portgroup( context, rpc_portgroup, topic) - api_portgroup = Portgroup.convert_with_links(new_portgroup) + api_portgroup = convert_with_links(new_portgroup) notify.emit_end_notification(context, new_portgroup, 'update', - node_uuid=api_portgroup.node_uuid) + node_uuid=rpc_node.uuid) return api_portgroup @METRICS.timer('PortgroupsController.delete') - @expose.expose(None, types.uuid_or_name, - status_code=http_client.NO_CONTENT) + @method.expose(status_code=http_client.NO_CONTENT) + @args.validate(portgroup_ident=args.uuid_or_name) def delete(self, portgroup_ident): """Delete a portgroup. diff --git a/ironic/tests/unit/api/controllers/v1/test_port.py b/ironic/tests/unit/api/controllers/v1/test_port.py index dfdddc470..b2065e7bb 100644 --- a/ironic/tests/unit/api/controllers/v1/test_port.py +++ b/ironic/tests/unit/api/controllers/v1/test_port.py @@ -1112,7 +1112,7 @@ class TestListPorts(test_api_base.BaseApiTest): headers={api_base.Version.string: '1.24'}, expect_errors=True) self.assertEqual(http_client.BAD_REQUEST, response.status_int) - self.assertIn('Expected a logical name or UUID', + self.assertIn('Expected UUID or name for portgroup', response.json['error_message']) diff --git a/ironic/tests/unit/api/controllers/v1/test_portgroup.py b/ironic/tests/unit/api/controllers/v1/test_portgroup.py index 8d66c414d..1f18564a0 100644 --- a/ironic/tests/unit/api/controllers/v1/test_portgroup.py +++ b/ironic/tests/unit/api/controllers/v1/test_portgroup.py @@ -26,16 +26,13 @@ from testtools.matchers import HasLength from ironic.api.controllers import base as api_base from ironic.api.controllers import v1 as api_v1 from ironic.api.controllers.v1 import notification_utils -from ironic.api.controllers.v1 import portgroup as api_portgroup from ironic.api.controllers.v1 import utils as api_utils -from ironic.api import types as atypes from ironic.common import exception from ironic.common import states from ironic.common import utils as common_utils from ironic.conductor import rpcapi from ironic import objects from ironic.objects import fields as obj_fields -from ironic.tests import base from ironic.tests.unit.api import base as test_api_base from ironic.tests.unit.api import utils as apiutils from ironic.tests.unit.objects import utils as obj_utils @@ -51,15 +48,6 @@ def _rpcapi_update_portgroup(self, context, portgroup, topic): return portgroup -class TestPortgroupObject(base.TestCase): - - def test_portgroup_init(self): - portgroup_dict = apiutils.portgroup_post_data(node_id=None) - del portgroup_dict['extra'] - portgroup = api_portgroup.Portgroup(**portgroup_dict) - self.assertEqual(atypes.Unset, portgroup.extra) - - class TestListPortgroups(test_api_base.BaseApiTest): headers = {api_base.Version.string: str(api_v1.max_version())} @@ -1007,6 +995,7 @@ class TestPatch(test_api_base.BaseApiTest): self.assertFalse(mock_upd.called) def test_update_portgroup_internal_info_not_allowed(self, mock_upd): + mock_upd.return_value = self.portgroup response = self.patch_json('/portgroups/%s' % self.portgroup.uuid, [{'path': '/internal_info', 'value': False, @@ -1052,6 +1041,7 @@ class TestPatch(test_api_base.BaseApiTest): [{'path': '/properties/abc', 'op': 'add', 'value': 123}], mock_upd) def test_remove_mode_not_allowed(self, mock_upd): + mock_upd.return_value = self.portgroup response = self.patch_json('/portgroups/%s' % self.portgroup.uuid, [{'path': '/mode', 'op': 'remove'}], diff --git a/ironic/tests/unit/api/utils.py b/ironic/tests/unit/api/utils.py index 28566a4c6..ce5d95866 100644 --- a/ironic/tests/unit/api/utils.py +++ b/ironic/tests/unit/api/utils.py @@ -167,8 +167,8 @@ def portgroup_post_data(**kw): if arg not in kw: portgroup.pop(arg) - internal = portgroup_controller.PortgroupPatchType.internal_attrs() - return remove_internal(portgroup, internal) + return remove_other_fields( + portgroup, portgroup_controller.PORTGROUP_SCHEMA['properties']) def post_get_test_portgroup(**kw): |