Fix inspection rules validation

Closes-Bug: #2115332
Change-Id: Idf2fa4bc5b844dd2865d98c6de4f78ebf614d3ca
Signed-off-by: Afonne-CID <afonnepaulc@gmail.com>

ironic/api/controllers/v1/inspection_rule.py

@@ -205,6 +205,7 @@ class InspectionRuleController(rest.RestController):
     @METRICS.timer('InspectionRuleController.post')
     @method.expose(status_code=http_client.CREATED)
     @method.body('inspection_rule')
+    @args.validate(inspection_rule=validation.VALIDATOR)
     def post(self, inspection_rule):
         """Create a new inspection rule.
 
@@ -231,7 +232,7 @@ class InspectionRuleController(rest.RestController):
     @METRICS.timer('InspectionRuleController.patch')
     @method.expose()
     @method.body('patch')
-    @args.validate(inspection_rule_uuid=args.uuid)
+    @args.validate(inspection_rule_uuid=args.uuid, patch=args.patch)
     def patch(self, inspection_rule_uuid, patch=None):
         """Update an existing inspection rule.
 

ironic/common/inspection_rules/actions.py

@@ -44,6 +44,10 @@ ACTIONS = {
 
 def get_action(op_name):
     """Get operator class by name."""
+    if op_name not in ACTIONS:
+        raise exception.Invalid(
+            _("Unsupported action '%s'.") % op_name
+        )
     class_name = ACTIONS[op_name]
     return globals()[class_name]
 

ironic/common/inspection_rules/operators.py

@@ -41,6 +41,10 @@ OPERATORS = {
 
 def get_operator(op_name):
     """Get operator class by name."""
+    if op_name not in OPERATORS:
+        raise exception.Invalid(
+            _("Unsupported operator '%s'.") % op_name
+        )
     class_name = OPERATORS[op_name]
     return globals()[class_name]
 
@@ -145,6 +149,18 @@ class SimpleOperator(OperatorBase):
                    for i in range(len(values) - 1))
 
 
+class LeOperator(SimpleOperator):
+    op = operator.le
+
+
+class GeOperator(SimpleOperator):
+    op = operator.ge
+
+
+class NeOperator(SimpleOperator):
+    op = operator.ne
+
+
 class EqOperator(SimpleOperator):
     op = operator.eq
 
@@ -211,7 +227,7 @@ class IsNoneOperator(OperatorBase):
     FORMATTED_ARGS = ['value']
 
     def __call__(self, task, value):
-        return str(value) == 'None'
+        return value is None
 
 
 class OneOfOperator(OperatorBase):

ironic/common/inspection_rules/validation.py

@@ -128,14 +128,13 @@ def validate_rule(rule):
     :param rule: The inspection rule to validate.
     :raises: Invalid if the rule is invalid.
     """
-    if not rule.get('conditions'):
-        rule['conditions'] = []
-
-    errors = []
     try:
         jsonschema.validate(rule, SCHEMA)
     except jsonschema.ValidationError as e:
-        errors.append(_('Validation failed for inspection rule: %s') % e)
+        raise exception.Invalid(
+            _('Validation failed for inspection rule: %s') % e)
+
+    errors = []
 
     phase = rule.get('phase', InspectionPhase.MAIN.value)
     if phase not in (p.value for p in InspectionPhase):

ironic/tests/unit/common/test_inspection_rule.py

@@ -19,6 +19,7 @@ from ironic.common import inspection_rules
 from ironic.common.inspection_rules import base
 from ironic.common.inspection_rules import engine
 from ironic.common.inspection_rules import utils
+from ironic.common.inspection_rules import validation
 from ironic.conductor import task_manager
 from ironic.tests.unit.db import base as db_base
 from ironic.tests.unit.objects import utils as obj_utils
@@ -462,14 +463,26 @@ class TestOperators(TestInspectionRules):
                 {'values': [5, 5]},
                 {'values': [5, 10]}
             ],
+            inspection_rules.operators.NeOperator: [
+                {'values': [5, 10]},
+                {'values': [5, 5]}
+            ],
             inspection_rules.operators.LtOperator: [
                 {'values': [5, 10]},
                 {'values': [10, 5]}
             ],
+            inspection_rules.operators.LeOperator: [
+                {'values': [5, 10]},
+                {'values': [10, 5]}
+            ],
             inspection_rules.operators.GtOperator: [
                 {'values': [10, 5]},
                 {'values': [5, 10]}
             ],
+            inspection_rules.operators.GeOperator: [
+                {'values': [10, 5]},
+                {'values': [5, 10]}
+            ],
             inspection_rules.operators.EmptyOperator: [
                 {'value': ''},
                 {'value': 'not empty'}
@@ -491,7 +504,7 @@ class TestOperators(TestInspectionRules):
                 {'value': 'z', 'values': ['a', 'b', 'c']}
             ],
             inspection_rules.operators.IsNoneOperator: [
-                {'value': 'None'},
+                {'value': None},
                 {'value': 'something'}
             ],
             inspection_rules.operators.IsTrueOperator: [
@@ -992,3 +1005,30 @@ class TestInterpolation(TestInspectionRules):
             result = base.Base.interpolate_variables(
                 value, task.node, self.inventory, self.plugin_data)
             self.assertEqual(value, result)
+
+
+class TestValidation(TestInspectionRules):
+    def test_unsupported_operator_rejected(self):
+        """Unsupported operator (even inverted) must raise Invalid."""
+        rule = {
+            'actions': [{'op': 'noop', 'args': {}}],
+            'conditions': [{'op': '!unknown', 'args': {}}]
+        }
+
+        self.assertRaises(exception.Invalid, validation.validate_rule, rule)
+
+    def test_conditions_not_list_raises_invalid(self):
+        rule = {
+            'actions': [{'op': 'noop', 'args': {}}],
+            'conditions': {'op': 'eq', 'args': [1, 1]}  # not a list
+        }
+
+        self.assertRaises(exception.Invalid, validation.validate_rule, rule)
+
+    def test_missing_actions_key_raises_invalid(self):
+        rule = {
+            'conditions': [{'op': 'eq', 'args': [1, 1]}]
+            # 'actions' is missing
+        }
+
+        self.assertRaises(exception.Invalid, validation.validate_rule, rule)

releasenotes/notes/fixes-inspection-rules-schema-validation-5cac6058d12ce030.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    Fixes schema validation by raising formatting and schema errors
+    early during inspection rule creation, updates and execution.
+
+  - |
+    Adds support for standard comparison operators (`le`, `ge`, `ne`)
+    to extend inspection rules capabilities for common logical conditions.