openstack/ironic
Code Issues Proposed changes

JSON conversion followup change

Browse Source 
This change addresses nit-level review comments from this task.

Story: 1651346
Task: 10551
Change-Id: I01608004ce90facadb73e252203900a1e62cbea1
This commit is contained in:
Steve Baker 2020-11-18 21:18:48 +13:00
parent a08da8551a
commit e41893c9d0
14 changed files with 30 additions and 72 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
ironic/api/controllers/v1/allocation.py
View File

@ -77,11 +77,13 @@ def convert_with_links(rpc_allocation, fields=None, sanitize=True):
'owner'),
list_fields=('candidate_nodes', 'traits')
)
api_utils.populate_node_uuid(rpc_allocation, allocation,
raise_notfound=False)
try:
api_utils.populate_node_uuid(rpc_allocation, allocation)
except exception.NodeNotFound:
allocation['node_uuid'] = None
if fields is not None:
api_utils.check_for_invalid_fields(fields, allocation.keys())
api_utils.check_for_invalid_fields(fields, set(allocation))
if sanitize:
allocation_sanitize(allocation, fields)

3
ironic/api/controllers/v1/collection.py
View File

@ -37,7 +37,8 @@ def list_convert_with_links(items, item_name, limit, url=None, fields=None,
:param fields:
Optional fields to use for sanitize function
:param sanitize_func:
Optional sanitize function run on each item
Optional sanitize function run on each item, item changes will be
done in-place
:param key_field:
Key name for building next URL
:param kwargs:

3
ironic/api/controllers/v1/deploy_template.py
View File

@ -40,8 +40,7 @@ METRICS = metrics_utils.get_metrics_logger(__name__)
DEFAULT_RETURN_FIELDS = ['uuid', 'name']
INTERFACE_NAMES = list(
conductor_steps.DEPLOYING_INTERFACE_PRIORITY.keys())
INTERFACE_NAMES = list(conductor_steps.DEPLOYING_INTERFACE_PRIORITY)
STEP_SCHEMA = {
'type': 'object',

4
ironic/api/controllers/v1/driver.py
View File

@ -221,7 +221,7 @@ class DriverPassthruController(rest.RestController):
@method.expose()
@method.body('data')
@args.validate(driver_name=args.string, method=args.string)
def _default(self, driver_name, method=None, data=None):
def _default(self, driver_name, method, data=None):
"""Call a driver API extension.
:param driver_name: name of the driver to call.
@ -229,8 +229,6 @@ class DriverPassthruController(rest.RestController):
implementation.
:param data: body of data to supply to the specified method.
"""
if not method:
raise exception.MissingArgument('method')
cdict = api.request.context.to_policy_values()
policy.authorize('baremetal:driver:vendor_passthru', cdict, cdict)

4
ironic/api/controllers/v1/event.py
View File

@ -67,7 +67,7 @@ EVENTS_SCHEMA = {
'type': 'object',
'properties': {
'event': {'type': 'string',
'enum': list(EVENT_VALIDATORS.keys())},
'enum': list(EVENT_VALIDATORS)},
},
'required': ['event'],
'additionalProperties': True,
@ -80,7 +80,7 @@ EVENTS_SCHEMA = {
def events_valid(name, value):
'''Validator for events'''
"""Validator for events"""
for event in value['events']:
validator = EVENT_VALIDATORS[event['event']]

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

@ -1277,13 +1277,6 @@ def node_sanitize(node, fields):
node['driver_info'] = strutils.mask_dict_password(
node['driver_info'], "******")
# NOTE(derekh): mask ssh keys for the ssh power driver.
# As this driver is deprecated masking here (opposed to strutils)
# is simpler, and easier to backport. This can be removed along
# with support for the ssh power driver.
if node['driver_info'].get('ssh_key_contents'):
node['driver_info']['ssh_key_contents'] = "******"
if not show_instance_secrets and node.get('instance_info'):
node['instance_info'] = strutils.mask_dict_password(
node['instance_info'], "******")

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

@ -226,7 +226,7 @@ def object_to_dict(obj, created_at=True, updated_at=True, uuid=True,
return to_dict
def populate_node_uuid(obj, to_dict, raise_notfound=True):
def populate_node_uuid(obj, to_dict):
"""Look up the node referenced in the object and populate a dict.
The node is fetched with the object ``node_id`` attribute and the
@ -236,23 +236,15 @@ def populate_node_uuid(obj, to_dict, raise_notfound=True):
object to get the node_id attribute
:param to_dict:
dict to populate with a ``node_uuid`` value
:param raise_notfound:
If ``True`` raise a NodeNotFound exception if the node doesn't exist
otherwise set the dict ``node_uuid`` value to None.
:raises:
exception.NodeNotFound if raise_notfound and the node is not found
exception.NodeNotFound if the node is not found
"""
if not obj.node_id:
to_dict['node_uuid'] = None
return
try:
to_dict['node_uuid'] = objects.Node.get_by_id(
api.request.context,
obj.node_id).uuid
except exception.NodeNotFound:
if raise_notfound:
raise
to_dict['node_uuid'] = None
to_dict['node_uuid'] = objects.Node.get_by_id(
api.request.context,
obj.node_id).uuid
def replace_node_uuid_with_id(to_dict):
@ -343,7 +335,7 @@ def patched_validate_with_schema(patched_dict, schema, validator=None):
:raises: exception.Invalid if validation fails
"""
schema_fields = schema['properties']
for field in set(patched_dict.keys()):
for field in set(patched_dict):
if field not in schema_fields:
patched_dict.pop(field, None)
if not validator:
@ -385,7 +377,7 @@ def sanitize_dict(to_sanitize, fields):
if fields is None:
return
for key in set(to_sanitize.keys()):
for key in set(to_sanitize):
if key not in fields and key != 'links':
to_sanitize.pop(key, None)

3
ironic/api/method.py
View File

@ -61,6 +61,9 @@ def expose(status_code=None):
pecan.response.status = 500
def _empty():
# This is for a pecan workaround originally in WSME,
# but the original issue description is in an issue tracker
# that is now offline
pecan.request.pecan['content_type'] = None
pecan.response.content_type = None

11
ironic/common/args.py
View File

@ -100,7 +100,7 @@ def uuid_or_name(name, value):
if value is None:
return
if (not utils.is_valid_logical_name(value)
and not uuidutils.is_uuid_like(value)):
and not uuidutils.is_uuid_like(value)):
raise exception.InvalidParameterValue(
_('Expected UUID or name for %s: %s') % (name, value))
return value
@ -271,7 +271,9 @@ def types(*types):
:param: types one or more types to use for the isinstance test
:returns: validator function which takes name and value arguments
"""
return functools.partial(_validate_types, types=tuple(types))
# Replace None with the None type
types = tuple((type(None) if tp is None else tp) for tp in types)
return functools.partial(_validate_types, types=types)
def _apply_validator(name, value, val_functions):
@ -352,7 +354,7 @@ def validate(*args, **kwargs):
elif param.default == inspect.Parameter.empty:
# no argument was provided, and there is no default
# in the parameter, so this is a mandatory argument
raise exception.InvalidParameterValue(
raise exception.MissingParameterValue(
_('Missing mandatory parameter: %s') % param.name)
if param_positional:
@ -388,7 +390,8 @@ patch = schema({
'op': {'type': 'string', 'enum': ['add', 'replace', 'remove']},
'value': {}
},
'additionalProperties': False
'additionalProperties': False,
'required': ['op', 'path']
}
})
"""Validate a patch API operation"""

13
ironic/common/exception.py
View File

@ -774,19 +774,6 @@ class UnknownArgument(ClientSideError):
}
class MissingArgument(ClientSideError):
def __init__(self, argname, msg=''):
self.argname = argname
super(MissingArgument, self).__init__(msg)
@property
def faultstring(self):
return _('Missing argument: "%(argname)s"%(msg)s') % {
'argname': self.argname,
'msg': self.msg and ": " + self.msg or ""
}
class UnknownAttribute(ClientSideError):
def __init__(self, fieldname, attributes, msg=''):
self.fieldname = fieldname

2
ironic/tests/unit/api/controllers/v1/test_driver.py
View File

@ -363,7 +363,7 @@ class TestListDrivers(base.BaseApiTest):
self.assertEqual(http_client.BAD_REQUEST, response.status_int)
error = json.loads(response.json['error_message'])
self.assertEqual('Missing argument: "method"',
self.assertEqual('Missing mandatory parameter: method',
error['faultstring'])
@mock.patch.object(rpcapi.ConductorAPI,

12
ironic/tests/unit/api/controllers/v1/test_node.py
View File

@ -2198,18 +2198,6 @@ class TestListNodes(test_api_base.BaseApiTest):
mock_vdi.assert_called_once_with(mock.ANY, mock.ANY,
node.uuid, 'test-topic')
def test_ssh_creds_masked(self):
driver_info = {"ssh_password": "password", "ssh_key_contents": "key"}
node = obj_utils.create_test_node(self.context,
chassis_id=self.chassis.id,
driver_info=driver_info)
data = self.get_json(
'/nodes/%s' % node.uuid,
headers={api_base.Version.string: str(api_v1.max_version())})
self.assertEqual("******", data["driver_info"]["ssh_password"])
self.assertEqual("******", data["driver_info"]["ssh_key_contents"])
@mock.patch.object(rpcapi.ConductorAPI, 'get_indicator_state')
def test_get_indicator_state(self, mock_gis):
node = obj_utils.create_test_node(self.context)

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

@ -858,14 +858,6 @@ class TestNodeIdent(base.TestCase):
'node_uuid': '1be26c0b-03f2-4d2e-ae87-c02d7f33c123'
}, d)
# not found, don't raise
mock_gbi.side_effect = exception.NodeNotFound(node=port.node_id)
d = {}
utils.populate_node_uuid(port, d, raise_notfound=False)
self.assertEqual({
'node_uuid': None
}, d)
# not found, raise exception
mock_gbi.side_effect = exception.NodeNotFound(node=port.node_id)
d = {}

2
ironic/tests/unit/common/test_args.py
View File

@ -597,7 +597,7 @@ class ValidateTypesTest(BaseTest):
def test_types(self):
@args.validate(foo=args.types(type(None), dict, str))
@args.validate(foo=args.types(None, dict, str))
def doit(foo):
return foo