From 336afa63597ddbfb202c65e5b2aa8dec417a0379 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=B6rg=20Herbel?= <joerg.herbel@checkmk.com>
Date: Mon, 7 Oct 2024 11:34:41 +0200
Subject: [PATCH] New ruleset API: Fix conversion of agent rule configurations

In case the ruleset was the result of a migration from a previous non-dictionary
ruleset, the transforms of the new ruleset failed.

CMK-19497

Change-Id: I2acf01896caf0cc2506d4514dbc31f6e97b7ebb0
---
 cmk/gui/utils/rule_specs/legacy_converter.py  | 31 ++++++++++++++++---
 .../utils/rulespecs/test_legacy_converter.py  | 18 +++++++++++
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/cmk/gui/utils/rule_specs/legacy_converter.py b/cmk/gui/utils/rule_specs/legacy_converter.py
index 066890132b2..100af8afb0d 100644
--- a/cmk/gui/utils/rule_specs/legacy_converter.py
+++ b/cmk/gui/utils/rule_specs/legacy_converter.py
@@ -403,8 +403,7 @@ def _add_agent_config_match_type_key(value: object) -> object:
 
 def _remove_agent_config_match_type_key(value: object) -> object:
     if isinstance(value, dict):
-        value.pop("cmk-match-type", None)
-        return value
+        return {k: v for k, v in value.items() if k != "cmk-match-type"}
 
     raise TypeError(value)
 
@@ -412,10 +411,32 @@ def _remove_agent_config_match_type_key(value: object) -> object:
 def _transform_agent_config_rule_spec_match_type(
     parameter_form: FormSpecCallable, localizer: Callable[[str], str]
 ) -> legacy_valuespecs.ValueSpec:
+    legacy_vs = convert_to_legacy_valuespec(parameter_form, localizer)
+    inner_transform = (
+        legacy_vs if isinstance(legacy_vs, Transform) and parameter_form().migrate else None
+    )
+    if not inner_transform:
+        return Transform(
+            legacy_vs,
+            forth=_remove_agent_config_match_type_key,
+            back=_add_agent_config_match_type_key,
+        )
+
+    # We cannot simply wrap legacy_vs into a Transform to handle the match type key. Wrapping a
+    # valuespec into a Transform results in the following order of transformations:
+    # 1. outer transform   (_remove_agent_config_match_type_key)
+    # 2. inner transforms
+    # _remove_agent_config_match_type_key fails for non-dictionaries, however, it is the job of the
+    # inner transforms to migrate to a dictionairy in case of a migration from a non-dictionary
+    # rule spec.
     return Transform(
-        convert_to_legacy_valuespec(parameter_form, localizer),
-        forth=_remove_agent_config_match_type_key,
-        back=_add_agent_config_match_type_key,
+        valuespec=Transform(
+            inner_transform._valuespec,  # pylint: disable=protected-access
+            to_valuespec=_remove_agent_config_match_type_key,
+            from_valuespec=_add_agent_config_match_type_key,
+        ),
+        to_valuespec=inner_transform.to_valuespec,
+        from_valuespec=inner_transform.from_valuespec,
     )
 
 
diff --git a/tests/unit/cmk/gui/utils/rulespecs/test_legacy_converter.py b/tests/unit/cmk/gui/utils/rulespecs/test_legacy_converter.py
index c9f5955db71..11c6f3bb8d3 100644
--- a/tests/unit/cmk/gui/utils/rulespecs/test_legacy_converter.py
+++ b/tests/unit/cmk/gui/utils/rulespecs/test_legacy_converter.py
@@ -3096,3 +3096,21 @@ def test_dictionary_groups_validate(
     converted = convert_to_legacy_valuespec(to_convert, translate_to_current_language)
     with pytest.raises(MKUserError):
         converted.validate_value(value_to_validate, "")
+
+
+def test_agent_config_rule_spec_transformations_work_with_previous_non_dict_values() -> None:
+    legacy_rulespec = convert_to_legacy_rulespec(
+        api_v1.rule_specs.AgentConfig(
+            name="test_rulespec",
+            title=api_v1.Title("rulespec title"),
+            topic=api_v1.rule_specs.Topic.GENERAL,
+            parameter_form=lambda: api_v1.form_specs.Dictionary(
+                elements={},
+                migrate=lambda _x: {},
+            ),
+            help_text=api_v1.Help("help text"),
+        ),
+        Edition.CRE,
+        translate_to_current_language,
+    )
+    assert legacy_rulespec.valuespec.transform_value(()) == {"cmk-match-type": "dict"}