diff options
author | Steve Baker <sbaker@redhat.com> | 2020-11-18 21:18:48 +1300 |
---|---|---|
committer | Steve Baker <sbaker@redhat.com> | 2020-11-26 11:05:48 +1300 |
commit | e41893c9d085b4883366db65b9a74104f1949e1d (patch) | |
tree | 3a2b9362a8772e65ac40451cf8703dba62a56aa9 | |
parent | a08da8551a66815bedef7c6444fde5f9082a6aea (diff) | |
download | ironic-e41893c9d085b4883366db65b9a74104f1949e1d.tar.gz |
JSON conversion followup change
This change addresses nit-level review comments from this task.
Story: 1651346
Task: 10551
Change-Id: I01608004ce90facadb73e252203900a1e62cbea1
-rw-r--r-- | ironic/api/controllers/v1/allocation.py | 8 | ||||
-rw-r--r-- | ironic/api/controllers/v1/collection.py | 3 | ||||
-rw-r--r-- | ironic/api/controllers/v1/deploy_template.py | 3 | ||||
-rw-r--r-- | ironic/api/controllers/v1/driver.py | 4 | ||||
-rw-r--r-- | ironic/api/controllers/v1/event.py | 4 | ||||
-rw-r--r-- | ironic/api/controllers/v1/node.py | 7 | ||||
-rw-r--r-- | ironic/api/controllers/v1/utils.py | 22 | ||||
-rw-r--r-- | ironic/api/method.py | 3 | ||||
-rwxr-xr-x | ironic/common/args.py | 11 | ||||
-rw-r--r-- | ironic/common/exception.py | 13 | ||||
-rw-r--r-- | ironic/tests/unit/api/controllers/v1/test_driver.py | 2 | ||||
-rw-r--r-- | ironic/tests/unit/api/controllers/v1/test_node.py | 12 | ||||
-rw-r--r-- | ironic/tests/unit/api/controllers/v1/test_utils.py | 8 | ||||
-rw-r--r-- | ironic/tests/unit/common/test_args.py | 2 |
14 files changed, 30 insertions, 72 deletions
diff --git a/ironic/api/controllers/v1/allocation.py b/ironic/api/controllers/v1/allocation.py index 1734bedf6..fb876c381 100644 --- a/ironic/api/controllers/v1/allocation.py +++ b/ironic/api/controllers/v1/allocation.py @@ -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) diff --git a/ironic/api/controllers/v1/collection.py b/ironic/api/controllers/v1/collection.py index 8368069e8..db5b70764 100644 --- a/ironic/api/controllers/v1/collection.py +++ b/ironic/api/controllers/v1/collection.py @@ -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: diff --git a/ironic/api/controllers/v1/deploy_template.py b/ironic/api/controllers/v1/deploy_template.py index 6a56eec4a..9e774f948 100644 --- a/ironic/api/controllers/v1/deploy_template.py +++ b/ironic/api/controllers/v1/deploy_template.py @@ -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', diff --git a/ironic/api/controllers/v1/driver.py b/ironic/api/controllers/v1/driver.py index 4868fc4cf..d3d920cc4 100644 --- a/ironic/api/controllers/v1/driver.py +++ b/ironic/api/controllers/v1/driver.py @@ -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) diff --git a/ironic/api/controllers/v1/event.py b/ironic/api/controllers/v1/event.py index f11650d4e..8e17d3bfa 100644 --- a/ironic/api/controllers/v1/event.py +++ b/ironic/api/controllers/v1/event.py @@ -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']] diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index a436d29be..8340152a4 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -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'], "******") diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 2ad7cd7b4..474987f9a 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -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) diff --git a/ironic/api/method.py b/ironic/api/method.py index 6afdb1622..71ce6204a 100644 --- a/ironic/api/method.py +++ b/ironic/api/method.py @@ -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 diff --git a/ironic/common/args.py b/ironic/common/args.py index 014fbd318..94cfe8841 100755 --- a/ironic/common/args.py +++ b/ironic/common/args.py @@ -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""" diff --git a/ironic/common/exception.py b/ironic/common/exception.py index c2e5030e8..ba636e154 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -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 diff --git a/ironic/tests/unit/api/controllers/v1/test_driver.py b/ironic/tests/unit/api/controllers/v1/test_driver.py index aa4b4e273..7e8a0f471 100644 --- a/ironic/tests/unit/api/controllers/v1/test_driver.py +++ b/ironic/tests/unit/api/controllers/v1/test_driver.py @@ -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, diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index 14cb0d3c8..4f863a3a1 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -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) diff --git a/ironic/tests/unit/api/controllers/v1/test_utils.py b/ironic/tests/unit/api/controllers/v1/test_utils.py index df51e594e..0d9b489fe 100644 --- a/ironic/tests/unit/api/controllers/v1/test_utils.py +++ b/ironic/tests/unit/api/controllers/v1/test_utils.py @@ -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 = {} diff --git a/ironic/tests/unit/common/test_args.py b/ironic/tests/unit/common/test_args.py index c4b4d2e88..5695bb8fc 100644 --- a/ironic/tests/unit/common/test_args.py +++ b/ironic/tests/unit/common/test_args.py @@ -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 |