openstack/ironic
Code Issues Proposed changes

Refactor API code for checking microversions

Browse Source 
Copy-paste, copy-paste everywhere! This change reduces the number of
places where essentially the same checks were applied in the API code.

Change-Id: I0fe6a6bf8757f3fa99308f66f082f899680ad33c
This commit is contained in:
Dmitry Tantsur 2018-10-18 17:39:03 +02:00
parent 7a551c4f4a
commit 754361489d
4 changed files with 77 additions and 216 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -119,73 +119,44 @@ def get_nodes_controller_reserved_names():
return _NODES_CONTROLLER_RESERVED_WORDS return _NODES_CONTROLLER_RESERVED_WORDS
def _hide_fields_in_newer_versions_part_one(obj):
if pecan.request.version.minor < versions.MINOR_3_DRIVER_INTERNAL_INFO:
obj.driver_internal_info = wsme.Unset
if not api_utils.allow_node_logical_names():
obj.name = wsme.Unset
# if requested version is < 1.6, hide inspection_*_at fields
if pecan.request.version.minor < versions.MINOR_6_INSPECT_STATE:
obj.inspection_finished_at = wsme.Unset
obj.inspection_started_at = wsme.Unset
if pecan.request.version.minor < versions.MINOR_7_NODE_CLEAN:
obj.clean_step = wsme.Unset
if pecan.request.version.minor < versions.MINOR_12_RAID_CONFIG:
obj.raid_config = wsme.Unset
obj.target_raid_config = wsme.Unset
if pecan.request.version.minor < versions.MINOR_20_NETWORK_INTERFACE:
obj.network_interface = wsme.Unset
if pecan.request.version.minor < versions.MINOR_42_FAULT:
obj.fault = wsme.Unset
if pecan.request.version.minor < versions.MINOR_44_NODE_DEPLOY_STEP:
obj.deploy_step = wsme.Unset
def _hide_fields_in_newer_versions_part_two(obj):
if not api_utils.allow_resource_class():
obj.resource_class = wsme.Unset
if not api_utils.allow_dynamic_interfaces():
for field in api_utils.V31_FIELDS:
setattr(obj, field, wsme.Unset)
if not api_utils.allow_storage_interface():
obj.storage_interface = wsme.Unset
if not api_utils.allow_traits():
obj.traits = wsme.Unset
if not api_utils.allow_rescue_interface():
obj.rescue_interface = wsme.Unset
if not api_utils.allow_bios_interface():
obj.bios_interface = wsme.Unset
if not api_utils.allow_conductor_group():
obj.conductor_group = wsme.Unset
if not api_utils.allow_automated_clean():
obj.automated_clean = wsme.Unset
def hide_fields_in_newer_versions(obj): def hide_fields_in_newer_versions(obj):
"""This method hides fields that were added in newer API versions. """This method hides fields that were added in newer API versions.
Certain node fields were introduced at certain API versions. Certain node fields were introduced at certain API versions.
These fields are only made available when the request's API version These fields are only made available when the request's API version
matches or exceeds the versions when these fields were introduced. matches or exceeds the versions when these fields were introduced.
This is broken into two methods for cyclomatic complexity's sake.
""" """
_hide_fields_in_newer_versions_part_one(obj) for field in api_utils.disallowed_fields():
_hide_fields_in_newer_versions_part_two(obj) setattr(obj, field, wsme.Unset)
def reject_fields_in_newer_versions(obj):
"""When creating an object, reject fields that appear in newer versions."""
for field in api_utils.disallowed_fields():
if field == 'conductor_group':
# NOTE(jroll) this is special-cased to "" and not Unset,
# because it is used in hash ring calculations
empty_value = ''
elif field == 'name' and obj.name is None:
# NOTE(dtantsur): for some reason we allow specifying name=None
# explicitly even in old API versions..
continue
else:
empty_value = wtypes.Unset
if getattr(obj, field, empty_value) != empty_value:
LOG.debug('Field %(field)s is not acceptable in version %(ver)s',
{'field': field, 'ver': pecan.request.version})
raise exception.NotAcceptable()
def reject_patch_in_newer_versions(patch):
for field in api_utils.disallowed_fields():
value = api_utils.get_patch_values(patch, '/%s' % field)
if value:
LOG.debug('Field %(field)s is not acceptable in version %(ver)s',
{'field': field, 'ver': pecan.request.version})
raise exception.NotAcceptable()
def update_state_in_older_versions(obj): def update_state_in_older_versions(obj):
@ -1904,47 +1875,13 @@ class NodesController(rest.RestController):
if self.from_chassis: if self.from_chassis:
raise exception.OperationNotPermitted() raise exception.OperationNotPermitted()
if (not api_utils.allow_resource_class() reject_fields_in_newer_versions(node)
and node.resource_class is not wtypes.Unset):
raise exception.NotAcceptable()
n_interface = node.network_interface
if (not api_utils.allow_network_interface()
and n_interface is not wtypes.Unset):
raise exception.NotAcceptable()
if not api_utils.allow_dynamic_interfaces():
for field in api_utils.V31_FIELDS:
if getattr(node, field) is not wsme.Unset:
raise exception.NotAcceptable()
if (not api_utils.allow_storage_interface()
and node.storage_interface is not wtypes.Unset):
raise exception.NotAcceptable()
if (not api_utils.allow_bios_interface() and
node.bios_interface is not wtypes.Unset):
raise exception.NotAcceptable()
if node.traits is not wtypes.Unset: if node.traits is not wtypes.Unset:
msg = _("Cannot specify node traits on node creation. Traits must " msg = _("Cannot specify node traits on node creation. Traits must "
"be set via the node traits API.") "be set via the node traits API.")
raise exception.Invalid(msg) raise exception.Invalid(msg)
if (not api_utils.allow_rescue_interface()
and node.rescue_interface is not wtypes.Unset):
raise exception.NotAcceptable()
# NOTE(jroll) this is special-cased to "" and not Unset,
# because it is used in hash ring calculations
if (not api_utils.allow_conductor_group()
and node.conductor_group != ""):
raise exception.NotAcceptable()
if (not api_utils.allow_automated_clean()
and node.automated_clean is not wtypes.Unset):
raise exception.NotAcceptable()
# NOTE(deva): get_topic_for checks if node.driver is in the hash ring # NOTE(deva): get_topic_for checks if node.driver is in the hash ring
# and raises NoValidHost if it is not. # and raises NoValidHost if it is not.
# We need to ensure that node has a UUID before it can # We need to ensure that node has a UUID before it can
@ -1983,27 +1920,11 @@ class NodesController(rest.RestController):
chassis_uuid=api_node.chassis_uuid) chassis_uuid=api_node.chassis_uuid)
return api_node return api_node
# NOTE (yolanda): isolate validation to avoid patch too complex pep error
def _validate_patch(self, patch, reset_interfaces): def _validate_patch(self, patch, reset_interfaces):
if self.from_chassis: if self.from_chassis:
raise exception.OperationNotPermitted() raise exception.OperationNotPermitted()
resource_class = api_utils.get_patch_values(patch, '/resource_class') reject_patch_in_newer_versions(patch)
if resource_class and not api_utils.allow_resource_class():
raise exception.NotAcceptable()
n_interfaces = api_utils.get_patch_values(patch, '/network_interface')
if n_interfaces and not api_utils.allow_network_interface():
raise exception.NotAcceptable()
if not api_utils.allow_dynamic_interfaces():
for field in api_utils.V31_FIELDS:
if api_utils.get_patch_values(patch, '/%s' % field):
raise exception.NotAcceptable()
s_interface = api_utils.get_patch_values(patch, '/storage_interface')
if s_interface and not api_utils.allow_storage_interface():
raise exception.NotAcceptable()
traits = api_utils.get_patch_values(patch, '/traits') traits = api_utils.get_patch_values(patch, '/traits')
if traits: if traits:
@ -2011,28 +1932,12 @@ class NodesController(rest.RestController):
"should be updated via the node traits API.") "should be updated via the node traits API.")
raise exception.Invalid(msg) raise exception.Invalid(msg)
r_interface = api_utils.get_patch_values(patch, '/rescue_interface')
if r_interface and not api_utils.allow_rescue_interface():
raise exception.NotAcceptable()
b_interface = api_utils.get_patch_values(patch, '/bios_interface')
if b_interface and not api_utils.allow_bios_interface():
raise exception.NotAcceptable()
driver = api_utils.get_patch_values(patch, '/driver') driver = api_utils.get_patch_values(patch, '/driver')
if reset_interfaces and not driver: if reset_interfaces and not driver:
msg = _("The reset_interfaces parameter can only be used when " msg = _("The reset_interfaces parameter can only be used when "
"changing the node's driver.") "changing the node's driver.")
raise exception.Invalid(msg) raise exception.Invalid(msg)
conductor_group = api_utils.get_patch_values(patch, '/conductor_group')
if conductor_group and not api_utils.allow_conductor_group():
raise exception.NotAcceptable()
automated_clean = api_utils.get_patch_values(patch, '/automated_clean')
if automated_clean and not api_utils.allow_automated_clean():
raise exception.NotAcceptable()
@METRICS.timer('NodesController.patch') @METRICS.timer('NodesController.patch')
@wsme.validate(types.uuid, types.boolean, [NodePatchType]) @wsme.validate(types.uuid, types.boolean, [NodePatchType])
@expose.expose(Node, types.uuid_or_name, types.boolean, @expose.expose(Node, types.uuid_or_name, types.boolean,

96
ironic/api/controllers/v1/utils.py
View File

@ -358,6 +358,42 @@ def check_allow_specify_fields(fields):
raise exception.NotAcceptable() raise exception.NotAcceptable()
VERSIONED_FIELDS = {
'driver_internal_info': versions.MINOR_3_DRIVER_INTERNAL_INFO,
'name': versions.MINOR_5_NODE_NAME,
'inspection_finished_at': versions.MINOR_6_INSPECT_STATE,
'inspection_started_at': versions.MINOR_6_INSPECT_STATE,
'clean_step': versions.MINOR_7_NODE_CLEAN,
'raid_config': versions.MINOR_12_RAID_CONFIG,
'target_raid_config': versions.MINOR_12_RAID_CONFIG,
'network_interface': versions.MINOR_20_NETWORK_INTERFACE,
'resource_class': versions.MINOR_21_RESOURCE_CLASS,
'storage_interface': versions.MINOR_33_STORAGE_INTERFACE,
'traits': versions.MINOR_37_NODE_TRAITS,
'rescue_interface': versions.MINOR_38_RESCUE_INTERFACE,
'bios_interface': versions.MINOR_40_BIOS_INTERFACE,
'fault': versions.MINOR_42_FAULT,
'deploy_step': versions.MINOR_44_NODE_DEPLOY_STEP,
'conductor_group': versions.MINOR_46_NODE_CONDUCTOR_GROUP,
'automated_clean': versions.MINOR_47_NODE_AUTOMATED_CLEAN,
}
for field in V31_FIELDS:
VERSIONED_FIELDS[field] = versions.MINOR_31_DYNAMIC_INTERFACES
def allow_field(field):
"""Check if a field is allowed in the current version."""
return pecan.request.version.minor >= VERSIONED_FIELDS[field]
def disallowed_fields():
"""Generator of fields not allowed in the current request."""
for field in VERSIONED_FIELDS:
if not allow_field(field):
yield field
def check_allowed_fields(fields): def check_allowed_fields(fields):
"""Check if fetching a particular field is allowed. """Check if fetching a particular field is allowed.
@ -366,24 +402,8 @@ def check_allowed_fields(fields):
""" """
if fields is None: if fields is None:
return return
if 'bios_interface' in fields and not allow_bios_interface(): for field in disallowed_fields():
raise exception.NotAcceptable() if field in fields:
if 'network_interface' in fields and not allow_network_interface():
raise exception.NotAcceptable()
if 'resource_class' in fields and not allow_resource_class():
raise exception.NotAcceptable()
if not allow_dynamic_interfaces():
if set(V31_FIELDS).intersection(set(fields)):
raise exception.NotAcceptable()
if 'storage_interface' in fields and not allow_storage_interface():
raise exception.NotAcceptable()
if 'traits' in fields and not allow_traits():
raise exception.NotAcceptable()
if 'rescue_interface' in fields and not allow_rescue_interface():
raise exception.NotAcceptable()
if 'conductor_group' in fields and not allow_conductor_group():
raise exception.NotAcceptable()
if 'automated_clean' in fields and not allow_automated_clean():
raise exception.NotAcceptable() raise exception.NotAcceptable()
@ -585,24 +605,6 @@ def allow_port_advanced_net_fields():
>= versions.MINOR_19_PORT_ADVANCED_NET_FIELDS) >= versions.MINOR_19_PORT_ADVANCED_NET_FIELDS)
def allow_network_interface():
"""Check if we should support network_interface node field.
Version 1.20 of the API added support for network interfaces.
"""
return (pecan.request.version.minor
>= versions.MINOR_20_NETWORK_INTERFACE)
def allow_resource_class():
"""Check if we should support resource_class node field.
Version 1.21 of the API added support for resource_class.
"""
return (pecan.request.version.minor
>= versions.MINOR_21_RESOURCE_CLASS)
def allow_ramdisk_endpoints(): def allow_ramdisk_endpoints():
"""Check if heartbeat and lookup endpoints are allowed. """Check if heartbeat and lookup endpoints are allowed.
@ -689,7 +691,7 @@ def allow_volume():
def allow_storage_interface(): def allow_storage_interface():
"""Check if we should support storage_interface node field. """Check if we should support storage_interface node and driver fields.
Version 1.33 of the API added support for storage interfaces. Version 1.33 of the API added support for storage interfaces.
""" """
@ -738,7 +740,7 @@ def allow_rescue_interface():
def allow_bios_interface(): def allow_bios_interface():
"""Check if we should support bios interface. """Check if we should support bios interface and endpoints.
Version 1.40 of the API added support for bios interface. Version 1.40 of the API added support for bios interface.
""" """
@ -889,24 +891,6 @@ def allow_reset_interfaces():
versions.MINOR_45_RESET_INTERFACES) versions.MINOR_45_RESET_INTERFACES)
def allow_conductor_group():
"""Check if passing a conductor_group for a node is allowed.
Version 1.46 exposes this field.
"""
return (pecan.request.version.minor >=
versions.MINOR_46_NODE_CONDUCTOR_GROUP)
def allow_automated_clean():
"""Check if passing automated_clean for a node is allowed.
Version 1.47 exposes this field.
"""
return (pecan.request.version.minor >=
versions.MINOR_47_NODE_AUTOMATED_CLEAN)
def get_request_return_fields(fields, detail, default_fields): def get_request_return_fields(fields, detail, default_fields):
"""Calculate fields to return from an API request """Calculate fields to return from an API request

21
ironic/tests/unit/api/controllers/v1/test_utils.py
View File

@ -404,20 +404,6 @@ class TestApiUtils(base.TestCase):
mock_request.version.minor = 18 mock_request.version.minor = 18
self.assertFalse(utils.allow_port_advanced_net_fields()) self.assertFalse(utils.allow_port_advanced_net_fields())
@mock.patch.object(pecan, 'request', spec_set=['version'])
def test_allow_network_interface(self, mock_request):
mock_request.version.minor = 20
self.assertTrue(utils.allow_network_interface())
mock_request.version.minor = 19
self.assertFalse(utils.allow_network_interface())
@mock.patch.object(pecan, 'request', spec_set=['version'])
def test_allow_resource_class(self, mock_request):
mock_request.version.minor = 21
self.assertTrue(utils.allow_resource_class())
mock_request.version.minor = 20
self.assertFalse(utils.allow_resource_class())
@mock.patch.object(pecan, 'request', spec_set=['version']) @mock.patch.object(pecan, 'request', spec_set=['version'])
def test_allow_ramdisk_endpoints(self, mock_request): def test_allow_ramdisk_endpoints(self, mock_request):
mock_request.version.minor = 22 mock_request.version.minor = 22
@ -537,13 +523,6 @@ class TestApiUtils(base.TestCase):
mock_request.version.minor = 40 mock_request.version.minor = 40
self.assertFalse(utils.allow_inspect_abort()) self.assertFalse(utils.allow_inspect_abort())
@mock.patch.object(pecan, 'request', spec_set=['version'])
def test_allow_conductor_group(self, mock_request):
mock_request.version.minor = 46
self.assertTrue(utils.allow_conductor_group())
mock_request.version.minor = 45
self.assertFalse(utils.allow_conductor_group())
class TestNodeIdent(base.TestCase): class TestNodeIdent(base.TestCase):

15
ironic/tests/unit/api/utils.py
View File

@ -23,9 +23,9 @@ from ironic.api.controllers.v1 import chassis as chassis_controller
from ironic.api.controllers.v1 import node as node_controller from ironic.api.controllers.v1 import node as node_controller
from ironic.api.controllers.v1 import port as port_controller from ironic.api.controllers.v1 import port as port_controller
from ironic.api.controllers.v1 import portgroup as portgroup_controller from ironic.api.controllers.v1 import portgroup as portgroup_controller
from ironic.api.controllers.v1 import utils as api_utils
from ironic.api.controllers.v1 import volume_connector as vc_controller from ironic.api.controllers.v1 import volume_connector as vc_controller
from ironic.api.controllers.v1 import volume_target as vt_controller from ironic.api.controllers.v1 import volume_target as vt_controller
from ironic.drivers import base as drivers_base
from ironic.tests.unit.db import utils as db_utils from ironic.tests.unit.db import utils as db_utils
ADMIN_TOKEN = '4562138218392831' ADMIN_TOKEN = '4562138218392831'
@ -104,16 +104,9 @@ def node_post_data(**kw):
# NOTE(jroll): pop out fields that were introduced in later API versions, # NOTE(jroll): pop out fields that were introduced in later API versions,
# unless explicitly requested. Otherwise, these will cause tests using # unless explicitly requested. Otherwise, these will cause tests using
# older API versions to fail. # older API versions to fail.
for iface in drivers_base.ALL_INTERFACES: for field in api_utils.VERSIONED_FIELDS:
name = '%s_interface' % iface if field not in kw:
if name not in kw: node.pop(field, None)
node.pop(name)
if 'resource_class' not in kw:
node.pop('resource_class')
if 'fault' not in kw:
node.pop('fault')
if 'automated_clean' not in kw:
node.pop('automated_clean')
internal = node_controller.NodePatchType.internal_attrs() internal = node_controller.NodePatchType.internal_attrs()
return remove_internal(node, internal) return remove_internal(node, internal)