From b5fd510db42ca9a8702a8fa63f8aaf02b5085f5d Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 16 Feb 2016 12:03:07 +0100 Subject: [PATCH] Add invert option to rule conditions Some conditions do not have a native counterparts, so we need a way to invert them (aka NOT operation). This patch adds a new generic parameter "invert", defaulting to False. Change-Id: I50342689ba52346a5a4fbf362536b629fc688986 --- doc/source/usage.rst | 2 + ironic_inspector/db.py | 1 + ...1d88_add_invert_field_to_rule_condition.py | 37 +++++++++++++++++++ ironic_inspector/rules.py | 23 +++++++++--- ironic_inspector/test/functional.py | 5 ++- ironic_inspector/test/test_rules.py | 17 +++++++++ .../notes/rules-invert-2585173a11db3c31.yaml | 4 ++ 7 files changed, 83 insertions(+), 6 deletions(-) create mode 100644 ironic_inspector/migrations/versions/e169a4a81d88_add_invert_field_to_rule_condition.py create mode 100644 releasenotes/notes/rules-invert-2585173a11db3c31.yaml diff --git a/doc/source/usage.rst b/doc/source/usage.rst index e46b83fb5..b08bc6322 100644 --- a/doc/source/usage.rst +++ b/doc/source/usage.rst @@ -87,6 +87,8 @@ between values from introspection data and node. Both schemes use JSON path:: if scheme (node or data) is missing, condition compares data with introspection data. +``invert`` boolean value, whether to invert the result of the comparison. + ``multiple`` how to treat situations where the ``field`` query returns multiple results (e.g. the field contains a list), available options are: diff --git a/ironic_inspector/db.py b/ironic_inspector/db.py index 3ddc7d130..55ad168b8 100644 --- a/ironic_inspector/db.py +++ b/ironic_inspector/db.py @@ -92,6 +92,7 @@ class RuleCondition(Base): rule = Column(String(36), ForeignKey('rules.uuid')) op = Column(String(255), nullable=False) multiple = Column(String(255), nullable=False) + invert = Column(Boolean, default=False) # NOTE(dtantsur): while all operations now require a field, I can also # imagine user-defined operations that do not, thus it's nullable. field = Column(Text) diff --git a/ironic_inspector/migrations/versions/e169a4a81d88_add_invert_field_to_rule_condition.py b/ironic_inspector/migrations/versions/e169a4a81d88_add_invert_field_to_rule_condition.py new file mode 100644 index 000000000..dbe83ad42 --- /dev/null +++ b/ironic_inspector/migrations/versions/e169a4a81d88_add_invert_field_to_rule_condition.py @@ -0,0 +1,37 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Add invert field to rule condition + +Revision ID: e169a4a81d88 +Revises: d588418040d +Create Date: 2016-02-16 11:19:29.715615 + +""" + +# revision identifiers, used by Alembic. +revision = 'e169a4a81d88' +down_revision = 'd588418040d' +branch_labels = None +depends_on = None + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + op.add_column('rule_conditions', sa.Column('invert', sa.Boolean(), + nullable=True, default=False)) + + +def downgrade(): + op.drop_column('rule_conditions', 'invert') diff --git a/ironic_inspector/rules.py b/ironic_inspector/rules.py index 005d8664a..697cb555e 100644 --- a/ironic_inspector/rules.py +++ b/ironic_inspector/rules.py @@ -59,6 +59,10 @@ def conditions_schema(): "description": "how to treat multiple values", "enum": ["all", "any", "first"] }, + "invert": { + "description": "whether to invert the result", + "type": "boolean" + }, }, # other properties are validated by plugins "additionalProperties": True @@ -157,6 +161,9 @@ class IntrospectionRule(object): for value in field_values: result = cond_ext.check(node_info, value, cond.params) + if cond.invert: + result = not result + if (cond.multiple == 'first' or (cond.multiple == 'all' and not result) or (cond.multiple == 'any' and result)): @@ -276,6 +283,7 @@ def create(conditions_json, actions_json, uuid=None, act_mgr = plugins_base.rule_actions_manager() conditions = [] + reserved_params = {'op', 'field', 'multiple', 'invert'} for cond_json in conditions_json: field = cond_json['field'] @@ -293,7 +301,7 @@ def create(conditions_json, actions_json, uuid=None, plugin = cond_mgr[cond_json['op']].obj params = {k: v for k, v in cond_json.items() - if k not in ('op', 'field', 'multiple')} + if k not in reserved_params} try: plugin.validate(params) except ValueError as exc: @@ -301,8 +309,11 @@ def create(conditions_json, actions_json, uuid=None, '%(error)s') % {'op': cond_json['op'], 'error': exc}) - conditions.append((cond_json['field'], cond_json['op'], - cond_json.get('multiple', 'any'), params)) + conditions.append((cond_json['field'], + cond_json['op'], + cond_json.get('multiple', 'any'), + cond_json.get('invert', False), + params)) actions = [] for action_json in actions_json: @@ -322,9 +333,11 @@ def create(conditions_json, actions_json, uuid=None, rule = db.Rule(uuid=uuid, description=description, disabled=False, created_at=timeutils.utcnow()) - for field, op, multiple, params in conditions: - rule.conditions.append(db.RuleCondition(op=op, field=field, + for field, op, multiple, invert, params in conditions: + rule.conditions.append(db.RuleCondition(op=op, + field=field, multiple=multiple, + invert=invert, params=params)) for action, params in actions: diff --git a/ironic_inspector/test/functional.py b/ironic_inspector/test/functional.py index 1e6c92328..5fe4d5706 100644 --- a/ironic_inspector/test/functional.py +++ b/ironic_inspector/test/functional.py @@ -309,7 +309,10 @@ class Test(Base): {'field': 'local_gb', 'op': 'lt', 'value': 1000}, {'field': 'local_gb', 'op': 'matches', 'value': '[0-9]+'}, {'field': 'cpu_arch', 'op': 'contains', 'value': '[0-9]+'}, - {'field': 'root_disk.wwn', 'op': 'is-empty'} + {'field': 'root_disk.wwn', 'op': 'is-empty'}, + {'field': 'inventory.interfaces[*].ipv4_address', + 'op': 'contains', 'value': r'127\.0\.0\.1', + 'invert': True, 'multiple': 'all'}, ], 'actions': [ {'action': 'set-attribute', 'path': '/extra/foo', diff --git a/ironic_inspector/test/test_rules.py b/ironic_inspector/test/test_rules.py index b954f224c..8524d971c 100644 --- a/ironic_inspector/test/test_rules.py +++ b/ironic_inspector/test/test_rules.py @@ -206,6 +206,23 @@ class TestCheckConditions(BaseTest): self.cond_mock.check.call_count) self.assertTrue(res) + def test_invert(self, mock_ext_mgr): + self.conditions_json = [ + {'op': 'eq', 'field': 'memory_mb', 'value': 42, + 'invert': True}, + ] + self.rule = rules.create(conditions_json=self.conditions_json, + actions_json=self.actions_json) + + mock_ext_mgr.return_value.__getitem__.return_value = self.ext_mock + self.cond_mock.check.return_value = False + + res = self.rule.check_conditions(self.node_info, self.data) + + self.cond_mock.check.assert_called_once_with(self.node_info, 1024, + {'value': 42}) + self.assertTrue(res) + def test_no_field(self, mock_ext_mgr): mock_ext_mgr.return_value.__getitem__.return_value = self.ext_mock self.cond_mock.check.return_value = True diff --git a/releasenotes/notes/rules-invert-2585173a11db3c31.yaml b/releasenotes/notes/rules-invert-2585173a11db3c31.yaml new file mode 100644 index 000000000..5a56a1c2f --- /dev/null +++ b/releasenotes/notes/rules-invert-2585173a11db3c31.yaml @@ -0,0 +1,4 @@ +--- +features: + - Introspection rules conditions got a new generic "invert" parameter that + inverts the result of the condition.