From 605364ad78407e3176dd79d0e4fe84cf87c8ce8e Mon Sep 17 00:00:00 2001 From: Thomas Morin Date: Wed, 11 Oct 2017 17:57:49 +0200 Subject: [PATCH] Support that an extension extends a sub-resource The neutron.api.extensions code assumes that the resource to update are base resources and updates its dictionary. When the resource is a sub-resource, ie. a {'parent': ... , 'parameters': {..} } dictionary, the result of the update is that the content of 'parameters' is overwritten. The correct thing to do here, in the case where the extended resource is a sub-resource, is to update the content of parameters with the new/changed attributes. This change also removes a workaround that was made in the qos-bw-limit-direction extension, and which after the change in API extension code, is not needed anymore. Needed-By: I263e1ee6cf4e1a91be91a4a78f4a160f64d33cc6 Change-Id: I4cb61481205c3689c41e62670cec113adb2a0362 Closes-Bug: 1722842 --- neutron/api/extensions.py | 17 +++++- neutron/extensions/qos_bw_limit_direction.py | 7 +-- neutron/tests/unit/api/test_extensions.py | 61 ++++++++++++++++++++ 3 files changed, 79 insertions(+), 6 deletions(-) diff --git a/neutron/api/extensions.py b/neutron/api/extensions.py index bcddb6f69e1..8111b142612 100644 --- a/neutron/api/extensions.py +++ b/neutron/api/extensions.py @@ -345,7 +345,16 @@ class ExtensionManager(object): continue extended_attrs = ext.get_extended_resources(version) for res, resource_attrs in extended_attrs.items(): - attr_map.setdefault(res, {}).update(resource_attrs) + res_to_update = attr_map.setdefault(res, {}) + if self._is_sub_resource(res_to_update): + # in the case of an existing sub-resource, we need to + # update the parameters content rather than overwrite + # it, and also keep the description of the parent + # resource unmodified + res_to_update['parameters'].update( + resource_attrs['parameters']) + else: + res_to_update.update(resource_attrs) processed_exts[ext_name] = ext del exts_to_process[ext_name] if len(processed_exts) == processed_ext_count: @@ -369,6 +378,12 @@ class ExtensionManager(object): for ext in processed_exts.values(): ext.update_attributes_map(attr_map) + def _is_sub_resource(self, resource): + return ('parent' in resource and + isinstance(resource['parent'], dict) and + 'member_name' in resource['parent'] and + 'parameters' in resource) + def _check_faulty_extensions(self, faulty_extensions): """Raise for non-default faulty extensions. diff --git a/neutron/extensions/qos_bw_limit_direction.py b/neutron/extensions/qos_bw_limit_direction.py index b935cb796ad..99931f7c6be 100644 --- a/neutron/extensions/qos_bw_limit_direction.py +++ b/neutron/extensions/qos_bw_limit_direction.py @@ -37,17 +37,14 @@ OPTIONAL_EXTENSIONS = None # The resource attribute map for the extension. SUB_RESOURCE_ATTRIBUTE_MAP = { qos_apidef.BANDWIDTH_LIMIT_RULES: { - 'parameters': dict( - qos_apidef.SUB_RESOURCE_ATTRIBUTE_MAP[ - qos_apidef.BANDWIDTH_LIMIT_RULES]['parameters'], - **{'direction': { + 'parameters': { + 'direction': { 'allow_post': True, 'allow_put': True, 'is_visible': True, 'default': common_constants.EGRESS_DIRECTION, 'validate': { 'type:values': common_constants.VALID_DIRECTIONS}}} - ) } } diff --git a/neutron/tests/unit/api/test_extensions.py b/neutron/tests/unit/api/test_extensions.py index a91ecfd0eba..d0793807113 100644 --- a/neutron/tests/unit/api/test_extensions.py +++ b/neutron/tests/unit/api/test_extensions.py @@ -669,6 +669,67 @@ class ExtensionManagerTest(base.BaseTestCase): self.assertNotIn('ext1-attr', attr_map['ext2']) self.assertNotIn('ext2-attr', attr_map['ext1']) + def test_extension_extends_sub_resource(self): + """Unit test for bug 1722842 + + Check that an extension can extend a sub-resource + """ + RESOURCE = "test_resource" + SUB_RESOURCE_NAME = "test_sub_resource" + INITIAL_PARAM = "dummy_param1" + ADDITIONAL_PARAM = "dummy_param2" + + SUB_RESOURCE = { + 'parent': {'member_name': RESOURCE}, + 'parameters': { + INITIAL_PARAM: {'allow_post': False, + 'allow_put': False, + 'validate': {'type:uuid': None}, + 'is_visible': True} + } + } + + class BaseExtension(ext_stubs.StubExtension): + + def get_extended_resources(self, version): + return { + SUB_RESOURCE_NAME: SUB_RESOURCE + } + + class ExtensionExtendingASubresource(ext_stubs.StubExtension): + + def get_extended_resources(self, version): + return { + SUB_RESOURCE_NAME: { + 'parameters': { + ADDITIONAL_PARAM: {'allow_post': False, + 'allow_put': False, + 'validate': {'type:uuid': None}, + 'is_visible': True} + } + } + } + + def get_required_extensions(self): + return ['base_extension'] + + ext_mgr = extensions.ExtensionManager('') + attr_map = {} + ext_mgr.add_extension(BaseExtension('base_extension')) + ext_mgr.add_extension( + ExtensionExtendingASubresource()) + ext_mgr.extend_resources("2.0", attr_map) + + # check that the parent descriptor is untouched + self.assertEqual(SUB_RESOURCE['parent'], + attr_map[SUB_RESOURCE_NAME]['parent']) + # check that the initial attribute is still here + self.assertIn(INITIAL_PARAM, + attr_map[SUB_RESOURCE_NAME]['parameters']) + # check that the new attribute is here as well + self.assertIn(ADDITIONAL_PARAM, + attr_map[SUB_RESOURCE_NAME]['parameters']) + class PluginAwareExtensionManagerTest(base.BaseTestCase):