From 2fde8e2ffb478ee9dae0d317323cdd49f00468f7 Mon Sep 17 00:00:00 2001
From: Gav <gavin.mcguigan@checkmk.com>
Date: Wed, 15 Nov 2023 18:21:04 +0100
Subject: [PATCH] rules: key, operator and value are now required fields

CMK-15035

Change-Id: Iee4d65065c869ebbde6f0c2fcf51a4c83e4e4fa4
---
 cmk/gui/openapi/endpoints/rule/fields.py      |  8 +++++
 tests/testlib/rest_api_client.py              |  2 +-
 .../cmk/gui/openapi/test_openapi_rules.py     | 32 ++++++++++++++++++-
 3 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/cmk/gui/openapi/endpoints/rule/fields.py b/cmk/gui/openapi/endpoints/rule/fields.py
index 6c2dc3624b6..c974c0d818c 100644
--- a/cmk/gui/openapi/endpoints/rule/fields.py
+++ b/cmk/gui/openapi/endpoints/rule/fields.py
@@ -149,6 +149,7 @@ class LabelConditionSchema(base.BaseSchema):
     )
     operator = fields.String(
         enum=["is", "is_not"],
+        required=True,
         description="How the label should be matched.",
     )
     value = fields.String(
@@ -213,6 +214,7 @@ class TagConditionSchemaBase(base.BaseSchema):
     operator_type: str
 
     key = fields.String(
+        required=True,
         description="The name of the tag.",
     )
 
@@ -313,10 +315,12 @@ class TagConditionScalarSchemaBase(TagConditionSchemaBase):
 
     # field defined in superclass
     operator = fields.String(
+        required=True,
         description="If the tag's value should match what is given under the field `value`.",
         enum=list(allowed_operators),  # Our serializer only wants to know lists.
     )
     value = fields.String(
+        required=True,
         description="The value of a tag.",
     )
 
@@ -347,11 +351,13 @@ class TagConditionConditionSchemaBase(TagConditionSchemaBase):
 
     # field defined in superclass
     operator = fields.String(
+        required=True,
         description="If the matched tag should be one of the given values, or not.",
         enum=list(allowed_operators),  # Our serializer only wants to know lists.
     )
     value = fields.List(
         fields.String(description="The value of a tag."),
+        required=True,
         description="A list of values for the tag.",
     )
 
@@ -508,10 +514,12 @@ def __init__(
 
     match_on = fields.List(
         fields.String(),
+        required=True,
         description="A list of string matching regular expressions.",
     )
     operator = fields.String(
         enum=["one_of", "none_of"],
+        required=True,
         description=(
             "How the hosts or services should be matched.\n"
             " * one_of - will match if any of the hosts or services is matched\n"
diff --git a/tests/testlib/rest_api_client.py b/tests/testlib/rest_api_client.py
index 5a1cc5300a6..c126475a877 100644
--- a/tests/testlib/rest_api_client.py
+++ b/tests/testlib/rest_api_client.py
@@ -224,7 +224,7 @@ def default_rule_properties() -> RuleProperties:
     return {"disabled": False}
 
 
-class StringMatcher(TypedDict):
+class StringMatcher(TypedDict, total=False):
     match_on: list[str]
     operator: Literal["one_of", "none_of"]
 
diff --git a/tests/unit/cmk/gui/openapi/test_openapi_rules.py b/tests/unit/cmk/gui/openapi/test_openapi_rules.py
index 93e5dfef0f4..a7473766e22 100644
--- a/tests/unit/cmk/gui/openapi/test_openapi_rules.py
+++ b/tests/unit/cmk/gui/openapi/test_openapi_rules.py
@@ -70,6 +70,7 @@ def _create_rule(
     ruleset: str = "inventory_df_rules",
     value: dict[str, Any] | list[Any] | tuple | str | None = None,
     value_raw: str | None = DEFAULT_VALUE_RAW,
+    conditions: RuleConditions | None = None,
     expect_ok: bool = True,
 ) -> tuple[Response, dict[str, Any]]:
     if value is None:
@@ -87,7 +88,8 @@ def _create_rule(
     if documentation_url:
         properties["documentation_url"] = documentation_url
 
-    conditions: RuleConditions = DEFAULT_CONDITIONS
+    if conditions is None:
+        conditions = DEFAULT_CONDITIONS
 
     values = {
         "ruleset": ruleset,
@@ -702,3 +704,31 @@ def test_update_rule_no_value_raw(
     )
     resp.assert_status_code(400)
     assert resp.json["detail"] == "These fields have problems: value_raw"
+
+
+def test_create_rule_missing_match_on(clients: ClientRegistry) -> None:
+    conditions: RuleConditions = {"service_description": {"operator": "one_of"}}
+    resp, _ = _create_rule(
+        clients=clients,
+        folder="/",
+        comment="They made me do it!",
+        description="This is my title for this very important rule.",
+        documentation_url="http://example.com/",
+        conditions=conditions,
+        expect_ok=False,
+    )
+    resp.assert_status_code(400)
+
+
+def test_create_rule_missing_operator(clients: ClientRegistry) -> None:
+    conditions: RuleConditions = {"service_description": {"match_on": []}}
+    resp, _ = _create_rule(
+        clients=clients,
+        folder="/",
+        comment="They made me do it!",
+        description="This is my title for this very important rule.",
+        documentation_url="http://example.com/",
+        conditions=conditions,
+        expect_ok=False,
+    )
+    resp.assert_status_code(400)