From 6c686e81ced340f2e301751d19f194e87a3c5185 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Mon, 14 Sep 2015 17:41:15 +0200 Subject: [PATCH] Add missing plugins for introspection rules Conditions: * in-net: checks if address is in a network Actions: * set-capability: sets a capability * extend-attribute: append value to a list attibute Helper method NodeInfo.replace_field is added to simplify writing similar action plugins. Implements: blueprint rules Change-Id: I7e47b3500624df1f2cb15445d05e1c9bca6dc9ae --- README.rst | 11 ++- ironic_inspector/node_cache.py | 26 +++++ ironic_inspector/plugins/rules.py | 48 ++++++++++ ironic_inspector/test/test_node_cache.py | 33 +++++++ ironic_inspector/test/test_plugins_rules.py | 100 ++++++++++++++++++++ ironic_inspector/utils.py | 3 +- requirements.txt | 1 + setup.cfg | 3 + 8 files changed, 223 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index 5f705aded..3d61a1d09 100644 --- a/README.rst +++ b/README.rst @@ -355,7 +355,8 @@ Conditions A condition is represented by an object with fields: ``op`` the type of comparison operation, default available operators include : -``eq``, ``le``, ``ge``, ``ne``, ``lt``, ``gt``. +``eq``, ``le``, ``ge``, ``ne``, ``lt``, ``gt`` (basic comparison operators), +``in-net`` (checks that IP address is in a given network). ``field`` a `JSON path `_ to the field in the introspection data to use in comparison. @@ -388,6 +389,14 @@ Default available actions include: field, which is the path to the attribute as used by ironic (e.g. ``/properties/something``), and a ``value`` to set. +* ``set-capability`` sets a capability on an Ironic node. Requires ``name`` + and ``value`` fields, which are the name and the value for a new capability + accordingly. Existing value for this same capability is replaced. + +* ``extend-attribute`` the same as ``set-attribute``, but treats existing + value as a list and appends value to it. If optional ``unique`` parameter is + set to ``True``, nothing will be added if given value is already in a list. + Setting IPMI Credentials ~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/ironic_inspector/node_cache.py b/ironic_inspector/node_cache.py index 7066f442a..323db9a08 100644 --- a/ironic_inspector/node_cache.py +++ b/ironic_inspector/node_cache.py @@ -13,6 +13,7 @@ """Cache for nodes currently under introspection.""" +import copy import json import time @@ -268,6 +269,31 @@ class NodeInfo(object): except AttributeError: raise KeyError(path) + def replace_field(self, path, func, **kwargs): + """Replace a field on ironic node. + + :param path: path to a field as used by the ironic client + :param func: function accepting an old value and returning a new one + :param kwargs: if 'default' value is passed here, it will be used when + no existing value is found. + :raises: KeyError if value is not found and default is not set + :raises: everything that patch() may raise + """ + try: + value = self.get_by_path(path) + op = 'replace' + except KeyError: + if 'default' in kwargs: + value = kwargs['default'] + op = 'add' + else: + raise + + ref_value = copy.deepcopy(value) + value = func(value) + if value != ref_value: + self.patch([{'op': op, 'path': path, 'value': value}]) + def add_node(uuid, **attributes): """Store information about a node under introspection. diff --git a/ironic_inspector/plugins/rules.py b/ironic_inspector/plugins/rules.py index 5b3873842..4c2936872 100644 --- a/ironic_inspector/plugins/rules.py +++ b/ironic_inspector/plugins/rules.py @@ -15,6 +15,7 @@ import operator +import netaddr from oslo_log import log from ironic_inspector.plugins import base @@ -65,6 +66,20 @@ class NeCondition(SimpleCondition): op = operator.ne +class NetCondition(base.RuleConditionPlugin): + def validate(self, params, **kwargs): + super(NetCondition, self).validate(params, **kwargs) + # Make sure it does not raise + try: + netaddr.IPNetwork(params['value']) + except netaddr.AddrFormatError as exc: + raise ValueError('invalid value: %s' % exc) + + def check(self, node_info, field, params, **kwargs): + network = netaddr.IPNetwork(params['value']) + return netaddr.IPAddress(field) in network + + class FailAction(base.RuleActionPlugin): REQUIRED_PARAMS = {'message'} @@ -90,3 +105,36 @@ class SetAttributeAction(base.RuleActionPlugin): return node_info.patch([{'op': 'remove', 'path': params['path']}]) + + +class SetCapabilityAction(base.RuleActionPlugin): + REQUIRED_PARAMS = {'name'} + OPTIONAL_PARAMS = {'value'} + + def apply(self, node_info, params, **kwargs): + node_info.update_capabilities( + **{params['name']: params.get('value')}) + + def rollback(self, node_info, params, **kwargs): + node_info.update_capabilities(**{params['name']: None}) + + +class ExtendAttributeAction(base.RuleActionPlugin): + REQUIRED_PARAMS = {'path', 'value'} + OPTIONAL_PARAMS = {'unique'} + # TODO(dtantsur): proper validation of path + + def apply(self, node_info, params, **kwargs): + def _replace(values): + value = params['value'] + if not params.get('unique') or value not in values: + values.append(value) + return values + + node_info.replace_field(params['path'], _replace, default=[]) + + def rollback(self, node_info, params, **kwargs): + def _replace(values): + return [v for v in values if v != params['value']] + + node_info.replace_field(params['path'], _replace, default=[]) diff --git a/ironic_inspector/test/test_node_cache.py b/ironic_inspector/test/test_node_cache.py index 298347e1b..aac90cdca 100644 --- a/ironic_inspector/test/test_node_cache.py +++ b/ironic_inspector/test/test_node_cache.py @@ -495,6 +495,39 @@ class TestUpdate(test_base.NodeTest): new_caps = utils.capabilities_to_dict(patch[0]['value']) self.assertEqual({'foo': 'bar', 'x': '1', 'y': '2'}, new_caps) + def test_replace_field(self): + self.ironic.node.update.return_value = mock.sentinel.node + self.node.extra['foo'] = 'bar' + + self.node_info.replace_field('/extra/foo', lambda v: v + '1') + + patch = [{'op': 'replace', 'path': '/extra/foo', 'value': 'bar1'}] + self.ironic.node.update.assert_called_once_with(self.uuid, patch) + self.assertIs(mock.sentinel.node, self.node_info.node()) + + def test_replace_field_not_found(self): + self.ironic.node.update.return_value = mock.sentinel.node + + self.assertRaises(KeyError, self.node_info.replace_field, + '/extra/foo', lambda v: v + '1') + + def test_replace_field_with_default(self): + self.ironic.node.update.return_value = mock.sentinel.node + + self.node_info.replace_field('/extra/foo', lambda v: v + [42], + default=[]) + + patch = [{'op': 'add', 'path': '/extra/foo', 'value': [42]}] + self.ironic.node.update.assert_called_once_with(self.uuid, patch) + self.assertIs(mock.sentinel.node, self.node_info.node()) + + def test_replace_field_same_value(self): + self.ironic.node.update.return_value = mock.sentinel.node + self.node.extra['foo'] = 'bar' + + self.node_info.replace_field('/extra/foo', lambda v: v) + self.assertFalse(self.ironic.node.update.called) + def test_patch_port(self): self.ironic.port.update.return_value = mock.sentinel.port diff --git a/ironic_inspector/test/test_plugins_rules.py b/ironic_inspector/test/test_plugins_rules.py index bf4d1c6e8..f2d11dbde 100644 --- a/ironic_inspector/test/test_plugins_rules.py +++ b/ironic_inspector/test/test_plugins_rules.py @@ -73,6 +73,20 @@ class TestSimpleConditions(test_base.BaseTest): self._test(cond, expected, *values) +class TestNetCondition(test_base.BaseTest): + cond = rules_plugins.NetCondition() + + def test_validate(self): + self.cond.validate({'value': '192.0.2.1/24'}) + self.assertRaises(ValueError, self.cond.validate, {'value': 'foo'}) + + def test_check(self): + self.assertTrue(self.cond.check(None, '192.0.2.4', + {'value': '192.0.2.1/24'})) + self.assertFalse(self.cond.check(None, '192.1.2.4', + {'value': '192.0.2.1/24'})) + + class TestFailAction(test_base.BaseTest): act = rules_plugins.FailAction() @@ -114,3 +128,89 @@ class TestSetAttributeAction(test_base.NodeTest): self.node.extra = {} self.act.rollback(self.node_info, self.params) self.assertFalse(mock_patch.called) + + +class TestSetCapabilityAction(test_base.NodeTest): + act = rules_plugins.SetCapabilityAction() + params = {'name': 'cap1', 'value': 'val'} + + def test_validate(self): + self.act.validate(self.params) + self.assertRaises(ValueError, self.act.validate, {'value': 42}) + + @mock.patch.object(node_cache.NodeInfo, 'patch') + def test_apply(self, mock_patch): + self.act.apply(self.node_info, self.params) + mock_patch.assert_called_once_with( + [{'op': 'add', 'path': '/properties/capabilities', + 'value': 'cap1:val'}]) + + @mock.patch.object(node_cache.NodeInfo, 'patch') + def test_apply_with_existing(self, mock_patch): + self.node.properties['capabilities'] = 'x:y,cap1:old_val,answer:42' + self.act.apply(self.node_info, self.params) + + patch = mock_patch.call_args[0][0] + new_caps = utils.capabilities_to_dict(patch[0]['value']) + self.assertEqual({'cap1': 'val', 'x': 'y', 'answer': '42'}, new_caps) + + @mock.patch.object(node_cache.NodeInfo, 'patch') + def test_rollback_with_existing(self, mock_patch): + self.node.properties = {'capabilities': 'foo:bar,cap1:val'} + self.act.rollback(self.node_info, self.params) + mock_patch.assert_called_once_with( + [{'op': 'add', 'path': '/properties/capabilities', + 'value': 'foo:bar'}]) + + @mock.patch.object(node_cache.NodeInfo, 'patch') + def test_rollback_no_existing(self, mock_patch): + self.node.properties = {'capabilities': 'foo:bar'} + self.act.rollback(self.node_info, self.params) + # TODO(dtantsur): make sure it's not called at all + mock_patch.assert_called_once_with( + [{'op': 'add', 'path': '/properties/capabilities', + 'value': 'foo:bar'}]) + + +class TestExtendAttributeAction(test_base.NodeTest): + act = rules_plugins.ExtendAttributeAction() + params = {'path': '/extra/value', 'value': 42} + + def test_validate(self): + self.act.validate(self.params) + self.assertRaises(ValueError, self.act.validate, {'value': 42}) + + @mock.patch.object(node_cache.NodeInfo, 'patch') + def test_apply(self, mock_patch): + self.act.apply(self.node_info, self.params) + mock_patch.assert_called_once_with( + [{'op': 'add', 'path': '/extra/value', 'value': [42]}]) + + @mock.patch.object(node_cache.NodeInfo, 'patch') + def test_apply_non_empty(self, mock_patch): + self.node.extra['value'] = [0] + self.act.apply(self.node_info, self.params) + + mock_patch.assert_called_once_with( + [{'op': 'replace', 'path': '/extra/value', 'value': [0, 42]}]) + + @mock.patch.object(node_cache.NodeInfo, 'patch') + def test_apply_unique_with_existing(self, mock_patch): + params = dict(unique=True, **self.params) + self.node.extra['value'] = [42] + self.act.apply(self.node_info, params) + self.assertFalse(mock_patch.called) + + @mock.patch.object(node_cache.NodeInfo, 'patch') + def test_rollback_with_existing(self, mock_patch): + self.node.extra['value'] = [1, 42, 0] + self.act.rollback(self.node_info, self.params) + + mock_patch.assert_called_once_with( + [{'op': 'replace', 'path': '/extra/value', 'value': [1, 0]}]) + + @mock.patch.object(node_cache.NodeInfo, 'patch') + def test_rollback_no_existing(self, mock_patch): + self.node.extra['value'] = [1, 0] + self.act.rollback(self.node_info, self.params) + self.assertFalse(mock_patch.called) diff --git a/ironic_inspector/utils.py b/ironic_inspector/utils.py index f8a2f6feb..d0182568b 100644 --- a/ironic_inspector/utils.py +++ b/ironic_inspector/utils.py @@ -197,4 +197,5 @@ def capabilities_to_dict(caps): def dict_to_capabilities(caps_dict): """Convert a dictionary into a string with the capabilities syntax.""" return ','.join(["%s:%s" % (key, value) - for key, value in caps_dict.items()]) + for key, value in caps_dict.items() + if value is not None]) diff --git a/requirements.txt b/requirements.txt index 3f65a20f0..812b1565f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,6 +7,7 @@ Flask<1.0,>=0.10 jsonpath-rw<2.0,>=1.2.0 jsonschema!=2.5.0,<3.0.0,>=2.0.0 keystonemiddleware>=2.0.0 +netaddr>=0.7.12,!=0.7.16 pbr>=1.6 python-ironicclient>=0.8.0 python-keystoneclient>=1.6.0 diff --git a/setup.cfg b/setup.cfg index 09f356c43..e80c11823 100644 --- a/setup.cfg +++ b/setup.cfg @@ -40,10 +40,13 @@ ironic_inspector.rules.conditions = le = ironic_inspector.plugins.rules:LeCondition ge = ironic_inspector.plugins.rules:GeCondition ne = ironic_inspector.plugins.rules:NeCondition + in-net = ironic_inspector.plugins.rules:NetCondition ironic_inspector.rules.actions = example = ironic_inspector.plugins.example:ExampleRuleAction fail = ironic_inspector.plugins.rules:FailAction set-attribute = ironic_inspector.plugins.rules:SetAttributeAction + set-capability = ironic_inspector.plugins.rules:SetCapabilityAction + extend-attribute = ironic_inspector.plugins.rules:ExtendAttributeAction oslo.config.opts = ironic_inspector = ironic_inspector.conf:list_opts ironic_inspector.common.swift = ironic_inspector.common.swift:list_opts