From 0646970f584f8036231ba5a4fb4fb8c3717ead16 Mon Sep 17 00:00:00 2001 From: Will Miller Date: Wed, 25 Jul 2018 14:58:37 +0000 Subject: [PATCH] Allow nested action value formatting Modify introspection rules to allow formatting to be applied to strings nested in dicts and lists in the actions. Change-Id: Ia53e0de98438f7789e9b9136dcd85c1b1274b713 Story: #1670768 Task: #11362 --- doc/source/user/usage.rst | 12 +++- ironic_inspector/main.py | 2 +- ironic_inspector/rules.py | 50 +++++++++---- ironic_inspector/test/unit/test_rules.py | 71 +++++++++++++++++++ ...ted-value-formatting-e04f187475e5e475.yaml | 4 ++ 5 files changed, 124 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/nested-value-formatting-e04f187475e5e475.yaml diff --git a/doc/source/user/usage.rst b/doc/source/user/usage.rst index 5258afc7a..efae0b9b8 100644 --- a/doc/source/user/usage.rst +++ b/doc/source/user/usage.rst @@ -124,12 +124,20 @@ Default available actions include: set to ``True``, nothing will be added if given value is already in a list. Starting from Mitaka release, ``value`` field in actions supports fetching data -from introspection, it's using `python string formatting notation -`_ :: +from introspection, using `python string formatting notation +`_:: {"action": "set-attribute", "path": "/driver_info/ipmi_address", "value": "{data[inventory][bmc_address]}"} +Note that any value referenced in this way will be converted to a string. + +If ``value`` is a dict or list, strings nested at any level within the +structure will be formatted as well:: + + {"action": "set-attribute", "path": "/properties/root_device", + "value": {"serial": "{data[root_device][serial]}"}} + Plugins ~~~~~~~ diff --git a/ironic_inspector/main.py b/ironic_inspector/main.py index a5aca2d83..4a4cd500e 100644 --- a/ironic_inspector/main.py +++ b/ironic_inspector/main.py @@ -40,7 +40,7 @@ app = flask.Flask(__name__) LOG = utils.getProcessingLogger(__name__) MINIMUM_API_VERSION = (1, 0) -CURRENT_API_VERSION = (1, 13) +CURRENT_API_VERSION = (1, 14) DEFAULT_API_VERSION = CURRENT_API_VERSION _LOGGING_EXCLUDED_KEYS = ('logs',) diff --git a/ironic_inspector/rules.py b/ironic_inspector/rules.py index 22bb87987..a4f95194a 100644 --- a/ironic_inspector/rules.py +++ b/ironic_inspector/rules.py @@ -196,19 +196,13 @@ class IntrospectionRule(object): ext = ext_mgr[act.action].obj for formatted_param in ext.FORMATTED_PARAMS: - value = act.params.get(formatted_param) - if not value or not isinstance(value, six.string_types): - continue - - # NOTE(aarefiev): verify provided value with introspection - # data format specifications. - # TODO(aarefiev): simple verify on import rule time. try: - act.params[formatted_param] = value.format(data=data) - except KeyError as e: - raise utils.Error(_('Invalid formatting variable key ' - 'provided: %s') % e, - node_info=node_info, data=data) + initial = act.params[formatted_param] + except KeyError: + # Ignore parameter that wasn't given. + continue + else: + act.params[formatted_param] = _format_value(initial, data) LOG.debug('Running action `%(action)s %(params)s`', {'action': act.action, 'params': act.params}, @@ -219,6 +213,38 @@ class IntrospectionRule(object): node_info=node_info, data=data) +def _format_value(value, data): + """Apply parameter formatting to a value. + + Format strings with the values from `data`. If `value` is a dict or + list, any string members (and any nested string members) will also be + formatted recursively. This includes both keys and values for dicts. + + :param value: The string to format, or container whose members to + format. + :param data: Introspection data. + :returns: `value`, formatted with the parameters from `data`. + """ + if isinstance(value, six.string_types): + # NOTE(aarefiev): verify provided value with introspection + # data format specifications. + # TODO(aarefiev): simple verify on import rule time. + try: + return value.format(data=data) + except KeyError as e: + raise utils.Error(_('Invalid formatting variable key ' + 'provided in value %(val)s: %(e)s'), + {'val': value, 'e': e}, data=data) + elif isinstance(value, dict): + return {_format_value(k, data): _format_value(v, data) + for k, v in six.iteritems(value)} + elif isinstance(value, list): + return [_format_value(v, data) for v in value] + else: + # Assume this is a 'primitive' value. + return value + + def _parse_path(path): """Parse path, extract scheme and path. diff --git a/ironic_inspector/test/unit/test_rules.py b/ironic_inspector/test/unit/test_rules.py index 58efc1f2d..8d53b0b6d 100644 --- a/ironic_inspector/test/unit/test_rules.py +++ b/ironic_inspector/test/unit/test_rules.py @@ -421,6 +421,64 @@ class TestApplyActions(BaseTest): self.assertEqual(1, self.act_mock.apply.call_count) + def test_apply_data_format_value_dict(self, mock_ext_mgr): + self.data.update({'val_outer': {'val_inner': 17}, + 'key_outer': {'key_inner': 'baz'}}) + + self.rule = rules.create(actions_json=[ + {'action': 'set-attribute', + 'path': '/driver_info/foo', + 'value': {'{data[key_outer][key_inner]}': + '{data[val_outer][val_inner]}'}}], + conditions_json=self.conditions_json + ) + mock_ext_mgr.return_value.__getitem__.return_value = self.ext_mock + + self.rule.apply_actions(self.node_info, data=self.data) + + self.act_mock.apply.assert_called_once_with(self.node_info, { + # String-formatted values will be coerced to be strings. + 'value': {'baz': '17'}, + 'path': '/driver_info/foo' + }) + + def test_apply_data_format_value_list(self, mock_ext_mgr): + self.data.update({'outer': {'inner': 'baz'}}) + + self.rule = rules.create(actions_json=[ + {'action': 'set-attribute', + 'path': '/driver_info/foo', + 'value': ['basic', ['{data[outer][inner]}']]}], + conditions_json=self.conditions_json + ) + mock_ext_mgr.return_value.__getitem__.return_value = self.ext_mock + + self.rule.apply_actions(self.node_info, data=self.data) + + self.act_mock.apply.assert_called_once_with(self.node_info, { + 'value': ['basic', ['baz']], + 'path': '/driver_info/foo' + }) + + def test_apply_data_format_value_primitives(self, mock_ext_mgr): + self.data.update({'outer': {'inner': False}}) + + self.rule = rules.create(actions_json=[ + {'action': 'set-attribute', + 'path': '/driver_info/foo', + 'value': {42: {True: [3.14, 'foo', '{data[outer][inner]}']}}}], + conditions_json=self.conditions_json + ) + mock_ext_mgr.return_value.__getitem__.return_value = self.ext_mock + + self.rule.apply_actions(self.node_info, data=self.data) + + self.act_mock.apply.assert_called_once_with(self.node_info, { + # String-formatted values will be coerced to be strings. + 'value': {42: {True: [3.14, 'foo', 'False']}}, + 'path': '/driver_info/foo' + }) + def test_apply_data_format_value_fail(self, mock_ext_mgr): self.rule = rules.create( actions_json=[ @@ -434,6 +492,19 @@ class TestApplyActions(BaseTest): self.assertRaises(utils.Error, self.rule.apply_actions, self.node_info, data=self.data) + def test_apply_data_format_value_nested_fail(self, mock_ext_mgr): + self.data.update({'outer': {'inner': 'baz'}}) + self.rule = rules.create(actions_json=[ + {'action': 'set-attribute', + 'path': '/driver_info/foo', + 'value': ['basic', ['{data[outer][nonexistent]}']]}], + conditions_json=self.conditions_json + ) + mock_ext_mgr.return_value.__getitem__.return_value = self.ext_mock + + self.assertRaises(utils.Error, self.rule.apply_actions, + self.node_info, data=self.data) + def test_apply_data_non_format_value(self, mock_ext_mgr): self.rule = rules.create(actions_json=[ {'action': 'set-attribute', diff --git a/releasenotes/notes/nested-value-formatting-e04f187475e5e475.yaml b/releasenotes/notes/nested-value-formatting-e04f187475e5e475.yaml new file mode 100644 index 000000000..2578e7296 --- /dev/null +++ b/releasenotes/notes/nested-value-formatting-e04f187475e5e475.yaml @@ -0,0 +1,4 @@ +--- +features: + - Modifies introspection rules to allow formatting to be applied to strings + nested in dicts and lists in the actions.