Merge "Allow nested action value formatting"

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
-<https://docs.python.org/2/library/string.html#formatspec>`_ ::
+from introspection, using `python string formatting notation
+<https://docs.python.org/2/library/string.html#formatspec>`_::
 
     {"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
 ~~~~~~~
 

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',)
 

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.
 

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',

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.