From f335077e595153cbd4afdbd281056c906daddbc6 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Sat, 20 Jun 2026 18:38:49 +1000 Subject: [PATCH] Fix salt rendering of yaml/json --- enroll/salt.py | 81 ++++++++++++++++++++++++++++++++++--- tests/test_manifest_salt.py | 67 ++++++++++++++++++++++++++++++ 2 files changed, 143 insertions(+), 5 deletions(-) diff --git a/enroll/salt.py b/enroll/salt.py index b841ac0..1b0e043 100644 --- a/enroll/salt.py +++ b/enroll/salt.py @@ -6,7 +6,7 @@ import re import shlex import shutil from pathlib import Path -from typing import Any, Dict, Iterable, List, Optional, Tuple +from typing import Any, Dict, Iterable, List, Mapping, Optional, Tuple import yaml @@ -276,6 +276,56 @@ def _state_id(prefix: str, value: Any, *, role: str = "") -> str: return "_".join(parts) +def _plain_salt_data(value: Any) -> Any: + """Return data made from plain JSON/YAML-safe containers. + + Salt's Jinja ``yaml_encode`` filter cannot represent Salt/PyYAML + ``OrderedDict`` values. Normalise generated template contexts before we + write static SLS or pillar data, and before passing context to file.managed. + """ + + if isinstance(value, Mapping): + return {str(key): _plain_salt_data(inner) for key, inner in value.items()} + if isinstance(value, list): + return [_plain_salt_data(item) for item in value] + if isinstance(value, tuple): + return [_plain_salt_data(item) for item in value] + if isinstance(value, set): + return sorted(_plain_salt_data(item) for item in value) + return value + + +_TO_JSON_FILTER_RE = re.compile( + r"{{\s*([A-Za-z_][A-Za-z0-9_]*(?:\.[A-Za-z_][A-Za-z0-9_]*)?)\s*" + r"\|\s*to_json\s*\([^)]*\)\s*}}" +) + + +def _saltify_jinjaturtle_template( + template_text: str, context: Dict[str, Any] +) -> Tuple[str, Dict[str, Any]]: + """Translate JinjaTurtle's Ansible-oriented Jinja into Salt-safe Jinja. + + JinjaTurtle emits Ansible's ``to_json`` filter for JSON/TOML values. Salt's + Jinja environment does not provide that filter. For ordinary generated + context variables, pre-render a JSON string and substitute a plain variable + reference. For loop-local expressions such as ``item`` or ``item.name`` we + fall back to Jinja's built-in ``tojson`` filter. + """ + + salt_context = _plain_salt_data(context) + + def replace(match: re.Match[str]) -> str: + expr = match.group(1) + if "." not in expr and expr in salt_context: + json_var = f"{expr}__enroll_json" + salt_context[json_var] = json.dumps(salt_context[expr], ensure_ascii=False) + return "{{ " + json_var + " }}" + return "{{ " + expr + " | tojson }}" + + return _TO_JSON_FILTER_RE.sub(replace, template_text), salt_context + + def _service_watch_state_ids( role_name: str, *, @@ -601,7 +651,24 @@ def _jinjify_managed_file( ) if converted is None: return None - return converted.template_rel, converted.context + + template_text, context = _saltify_jinjaturtle_template( + converted.template_text, converted.context + ) + template_path = role_dir / "templates" / converted.template_rel + if template_text != converted.template_text: + existing = ( + template_path.read_text(encoding="utf-8") if template_path.exists() else "" + ) + if ( + overwrite_templates + or not template_path.exists() + or _TO_JSON_FILTER_RE.search(existing) + ): + template_path.parent.mkdir(parents=True, exist_ok=True) + template_path.write_text(template_text, encoding="utf-8") + + return converted.template_rel, context def _node_file_prefix(fqdn: str) -> str: @@ -798,7 +865,7 @@ def _append_yaml_value(lines: List[str], key: str, value: Any, *, indent: int) - prefix = " " * indent if isinstance(value, dict): dumped = yaml.safe_dump( - value, sort_keys=True, default_flow_style=False + _plain_salt_data(value), sort_keys=True, default_flow_style=False ).rstrip() if not dumped: lines.append(f"{prefix}- {key}: {{}}") @@ -1156,7 +1223,11 @@ def _role_pillar_values(srole: SaltRole) -> Dict[str, Any]: **( {"template": attrs.get("template")} if attrs.get("template") else {} ), - **({"context": attrs.get("context")} if attrs.get("context") else {}), + **( + {"context": _plain_salt_data(attrs.get("context"))} + if attrs.get("context") + else {} + ), **( {"watch_in": attrs.get("watch_in")} if attrs.get("watch_in") else {} ), @@ -1277,7 +1348,7 @@ def _render_pillar_role(srole: SaltRole) -> str: " - template: {{ attrs.get('template')|yaml_dquote }}", "{% endif %}", "{% if attrs.get('context') %}", - " - context: {{ attrs.get('context')|yaml_encode }}", + " - context: {{ attrs.get('context')|tojson }}", "{% endif %}", "{% if attrs.get('watch_in') %}", " - watch_in:", diff --git a/tests/test_manifest_salt.py b/tests/test_manifest_salt.py index 73ca276..9a35c2b 100644 --- a/tests/test_manifest_salt.py +++ b/tests/test_manifest_salt.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +from collections import OrderedDict from pathlib import Path import yaml @@ -8,6 +9,7 @@ import yaml from enroll import manifest from enroll.salt import ( SaltRole, + _render_pillar_role, _render_static_role, _role_pillar_values, _salt_name, @@ -616,6 +618,71 @@ def test_manifest_salt_uses_jinjaturtle_templates(monkeypatch, tmp_path: Path): assert file_data["context"] == {"foo_setting": True} +def test_manifest_salt_rewrites_jinjaturtle_json_filters(monkeypatch, tmp_path: Path): + import enroll.jinjaturtle as jinjaturtle_mod + from enroll.jinjaturtle import JinjifyResult + + bundle = tmp_path / "bundle" + out = tmp_path / "salt" + state = _sample_state() + _write_sample_artifacts(bundle) + _write_state(bundle, state) + + monkeypatch.setattr( + jinjaturtle_mod, "find_jinjaturtle_cmd", lambda: "/usr/bin/jinjaturtle" + ) + monkeypatch.setattr(jinjaturtle_mod, "can_jinjify_path", lambda _path: True) + + def fake_run_jinjaturtle( + jt_exe: str, src_path: str, *, role_name: str, force_format=None + ): + return JinjifyResult( + template_text='{ "setting": {{ foo_setting | to_json(ensure_ascii=False) }} }\n', + vars_text='foo_setting: "alpha"\n', + ) + + monkeypatch.setattr(jinjaturtle_mod, "run_jinjaturtle", fake_run_jinjaturtle) + + manifest.manifest(str(bundle), str(out), target="salt", jinjaturtle="on") + + template_text = ( + out / "states" / "roles" / "net" / "templates" / "etc" / "foo.conf.j2" + ).read_text(encoding="utf-8") + assert "to_json" not in template_text + assert "foo_setting__enroll_json" in template_text + sls = (out / "states" / "roles" / "net" / "init.sls").read_text(encoding="utf-8") + assert "foo_setting__enroll_json:" in sls + assert '"alpha"' in sls + + +def test_manifest_salt_pillar_role_uses_json_for_template_context() -> None: + role = SaltRole("foo") + role.add_managed_file( + "/etc/foo.json", + source="salt://roles/foo/templates/etc/foo.json.j2", + user="root", + group="root", + mode="0644", + makedirs=True, + template="jinja", + context=OrderedDict( + [("foo_name", "alpha"), ("foo_nested", OrderedDict([("x", 1)]))] + ), + ) + + pillar = _role_pillar_values(role) + assert type(pillar["files"]["/etc/foo.json"]["context"]) is dict + assert type(pillar["files"]["/etc/foo.json"]["context"]["foo_nested"]) is dict + + rendered = _render_static_role(role) + assert "foo_nested:" in rendered + context_block = ( + _render_pillar_role(role).split("context:", 1)[1].split("{% endif %}", 1)[0] + ) + assert "|yaml_encode" not in context_block + assert "|tojson" in _render_pillar_role(role) + + def test_manifest_salt_renders_firewall_runtime_states(tmp_path: Path): bundle = tmp_path / "bundle" out = tmp_path / "salt"