Fix nested substitution

kpops/components/base_components/base_defaults_component.py

@@ -88,8 +88,6 @@ def __init__(self, **values: Any) -> None:
             tmp_self = cls(**values, enrich=False)
             values = tmp_self.model_dump(mode="json", by_alias=True)
             values = cls.substitute_in_component(tmp_self.config, **values)
-            # HACK: why is double substitution necessary for test_substitute_in_component
-            values = cls.substitute_in_component(tmp_self.config, **values)
             self.__init__(
                 enrich=False,
                 validate=True,

kpops/utils/yaml.py

@@ -1,3 +1,4 @@
+import json
 from collections.abc import Mapping
 from pathlib import Path
 from typing import Any
@@ -44,14 +45,22 @@ def prepare_substitution(substitution: Mapping[str, Any]) -> dict[str, Any]:
     return ImprovedTemplate(input).safe_substitute(**prepare_substitution(substitution))
 
 
+def _diff_substituted_str(s1: str, s2: str):
+    """Compare 2 strings, raise exception if not equal.
+
+    :param s1: String to compare
+    :param s2: String to compare
+    :raises ValueError: An infinite loop condition detected. Check substitution variables.
+    """
+    if s1 != s2:
+        msg = "An infinite loop condition detected. Check substitution variables."
+        raise ValueError(msg)
+
+
 def substitute_nested(input: str, **kwargs) -> str:
     """Allow for multiple substitutions to be passed.
 
     Will make as many passes as needed to substitute all possible placeholders.
-    A ceiling is set to avoid infinite loops.
-
-    HINT: If :param input: is a ``Mapping`` that you converted into ``str``,
-    You can pass it as a string, and as a ``Mapping`` to enable self-reference.
 
     :Example:
 
@@ -63,26 +72,43 @@ def substitute_nested(input: str, **kwargs) -> str:
         }
     >>> input = "${a}, ${b}, ${c}, ${d}"
     >>> print("Substituted string: " + substitute_nested(input, **substitution))
-    0, 0, 0, 0
+    "0, 0, 0, 0"
 
     :param input: The raw input containing $-placeholders
     :param **kwargs: Substitutions
-    :raises Exception: An infinite loop condition detected. Check substitution variables.
+    :raises ValueError: An infinite loop condition detected. Check substitution variables.
     :return: Substituted input string
     """
     if not kwargs:
         return input
+    kwargs = substitute_in_self(kwargs)
     old_str, new_str = "", substitute(input, kwargs)
     steps = set()
     while new_str not in steps:
         steps.add(new_str)
         old_str, new_str = new_str, substitute(new_str, kwargs)
-    if new_str != old_str:
-        msg = "An infinite loop condition detected. Check substitution variables."
-        raise ValueError(msg)
+    _diff_substituted_str(new_str, old_str)
     return old_str
 
 
+def substitute_in_self(input: dict[str, Any]) -> dict[str, Any]:
+    """Substitute all self-references in mapping.
+
+    Will make as many passes as needed to substitute all possible placeholders.
+
+    :param input: Mapping containing $-placeholders
+    :raises ValueError: An infinite loop condition detected. Check substitution variables.
+    :return: Substituted input mapping as dict
+    """
+    old_str, new_str = "", substitute(json.dumps(input), input)
+    steps = set()
+    while new_str not in steps:
+        steps.add(new_str)
+        old_str, new_str = new_str, substitute(new_str, json.loads(new_str))
+    _diff_substituted_str(new_str, old_str)
+    return json.loads(old_str)
+
+
 def print_yaml(data: Mapping | str, *, substitution: dict | None = None) -> None:
     """Print YAML object with syntax highlighting.
 

tests/components/resources/defaults.yaml

@@ -8,4 +8,4 @@ child:
   nested:
     foo: foo
 env-var-test:
-  name: $pipeline_name
+  name: ${pipeline_name}

tests/components/test_base_defaults_component.py

@@ -189,11 +189,12 @@ def test_multiple_generations(
     def test_env_var_substitution(
         self, config: KpopsConfig, handlers: ComponentHandlers
     ):
-        ENV["pipeline_name"] = str(DEFAULTS_PATH)
+        ENV["pipeline_name"] = DEFAULTS_PATH.as_posix()
         component = EnvVarTest(config=config, handlers=handlers)
 
-        assert component.name == str(
-            DEFAULTS_PATH
+        assert component.name
+        assert (
+            Path(component.name) == DEFAULTS_PATH
         ), "Environment variables should be substituted"
 
     def test_merge_defaults(self, config: KpopsConfig, handlers: ComponentHandlers):

tests/pipeline/resources/component-type-substitution/infinite_pipeline.yaml

@@ -2,5 +2,6 @@
   app:
     labels:
       l_1: ${component.app.labels.l_2}
-      l_2: ${component.app.labels.l_1}
+      l_2: ${component.app.labels.l_3}
+      l_3: ${component.app.labels.l_1}
       infinite_nesting: ${component.app.labels}

tests/utils/test_yaml.py

@@ -0,0 +1,29 @@
+import pytest
+
+from kpops.utils.yaml import substitute_nested
+
+
+@pytest.mark.parametrize(
+    ("input", "substitution", "expected"),
+    [
+        pytest.param(
+            '"a": "b", "${a}": "c", "a_${b}": "d", "e": "${a_${b}}"',
+            {"a": "b", "${a}": "c", "a_${b}": "d", "e": "${a_${b}}"},
+            '"a": "b", "b": "c", "a_c": "d", "e": "d"',
+            id="requires-multiple-passes",
+        ),
+        pytest.param(
+            "${a}, ${b}, ${c}, ${d}",
+            {
+                "a": "0",
+                "b": "${a}",
+                "c": "${b}",
+                "d": "${a}",
+            },
+            "0, 0, 0, 0",
+            id="chained-references",
+        ),
+    ],
+)
+def test_substitute_nested(input: str, substitution: dict[str, str], expected: str):
+    assert substitute_nested(input, **substitution) == expected