From 8cbde1423aa0c2bb78dbcbcde1d61722ce602cf8 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Sat, 20 Jun 2026 18:22:08 +1000 Subject: [PATCH 1/2] erb support, and fix notify services in puppet/salt in fqdn mode --- README.md | 7 +- enroll/ansible.py | 2 +- enroll/jinjaturtle.py | 70 +++++-- enroll/manifest.py | 2 + enroll/puppet.py | 317 +++++++++++++++++++++++++++----- enroll/salt.py | 80 +++++++- tests/test_manifest_puppet.py | 332 +++++++++++++++++++++++++++++++++- tests/test_manifest_salt.py | 78 +++++++- 8 files changed, 817 insertions(+), 71 deletions(-) diff --git a/README.md b/README.md index d7d23cc..3463866 100644 --- a/README.md +++ b/README.md @@ -295,7 +295,7 @@ If you intend to keep harvests/manifests long-term (especially in git), strongly ## JinjaTurtle integration -If [JinjaTurtle](https://git.mig5.net/mig5/jinjaturtle) is installed, `enroll` can generate Jinja2 templates for ini/json/xml/toml-style config in renderers that consume Jinja templates. +If [JinjaTurtle](https://git.mig5.net/mig5/jinjaturtle) is installed, `enroll` can generate templates for ini/json/xml/toml-style config in renderers. For Ansible: - Templates live in `roles//templates/...` @@ -308,7 +308,10 @@ For Salt: - `file.managed` uses `template: jinja` with per-file `context` values - In `--fqdn` mode, template context values are written to pillar with the file metadata -Puppet output does not use `.erb` templates at this time. +For Puppet: +- JinjaTurtle will use its 'erb' mode if you are running a recent-enough version. +- Templates will be stored in `modules//templates/.erb` +- In `--fqdn` mode, template context values are written to Hiera data. You can force template generation on with `--jinjaturtle` or disable it with `--no-jinjaturtle`. diff --git a/enroll/ansible.py b/enroll/ansible.py index 7d7ba9b..b2d9307 100644 --- a/enroll/ansible.py +++ b/enroll/ansible.py @@ -816,7 +816,7 @@ def _render_readme( - `roles//files/...` and `roles//templates/...` contain reusable role artifacts where applicable.""" apply = f"""```bash ansible-galaxy collection install -r requirements.yml -ansible-playbook -i inventory/hosts.ini playbooks/{fqdn}.yml --check +ansible-playbook -i inventory/hosts.ini playbooks/{fqdn}.yml --check --diff ```""" else: layout = """- `playbook.yml` applies the generated roles to the current inventory. diff --git a/enroll/jinjaturtle.py b/enroll/jinjaturtle.py index 88a64d4..b876177 100644 --- a/enroll/jinjaturtle.py +++ b/enroll/jinjaturtle.py @@ -81,6 +81,7 @@ _JINJA_FOR_RE = re.compile( r"{%\s*for\s+([A-Za-z_][A-Za-z0-9_]*)\s+in\s+([A-Za-z_][A-Za-z0-9_]*)\b" ) _JINJA_SPECIAL_VARS = {"loop", "true", "false", "none", "True", "False", "None"} +_ERB_INSTANCE_VAR_RE = re.compile(r"<%=?[^%]*@([A-Za-z_][A-Za-z0-9_]*)", re.S) def _find_undeclared_jinja_vars(template_text: str) -> Set[str]: @@ -119,6 +120,21 @@ def missing_jinja_template_vars( return {name for name in referenced if name not in context} +def missing_erb_template_vars(template_text: str, context: Dict[str, Any]) -> Set[str]: + """Return ERB ``@param`` references absent from Puppet Hiera/class data.""" + + local_names: Set[str] = set() + for key in context: + text = str(key) + if "::" in text: + local_names.add(text.split("::", 1)[1]) + else: + local_names.add(text) + + referenced = set(_ERB_INSTANCE_VAR_RE.findall(template_text)) + return {name for name in referenced if name not in local_names} + + def jinjify_artifact( bundle_dir: str | Path, artifact_role: str, @@ -130,14 +146,13 @@ def jinjify_artifact( jt_enabled: bool, overwrite_templates: bool = True, role_name: Optional[str] = None, + template_engine: str = "jinja2", + puppet_class: Optional[str] = None, ) -> Optional[JinjifiedArtifact]: - """Best-effort conversion of one harvested artifact into a Jinja2 template. + """Best-effort conversion of one harvested artifact into a template. - Puppet does not use JinjaTurtle, but Salt and Ansible both have the same - philosophical operation: take ``artifacts//``, ask - JinjaTurtle for a template and variable mapping, and write that template - under the renderer's template directory. Keeping that here prevents Salt - and Ansible from reimplementing the same probing/format/error handling. + Ansible/Salt use Jinja2 output. Puppet uses ERB output with Puppet Hiera + keys when a new enough JinjaTurtle is available. """ if not (jt_enabled and jt_exe and can_jinjify_path(dest_path)): return None @@ -147,22 +162,34 @@ def jinjify_artifact( return None try: - result = run_jinjaturtle( - jt_exe, - str(artifact_path), - role_name=role_name or artifact_role, - force_format=infer_other_formats(dest_path), - ) + run_kwargs: Dict[str, Any] = { + "role_name": role_name or artifact_role, + "force_format": infer_other_formats(dest_path), + } + # Keep the historical call shape for Ansible/Salt and for tests that + # monkeypatch run_jinjaturtle with the old signature. Puppet/ERB is + # the only path that needs the newer JinjaTurtle CLI switches. + if template_engine != "jinja2": + run_kwargs["template_engine"] = template_engine + if puppet_class: + run_kwargs["puppet_class"] = puppet_class + result = run_jinjaturtle(jt_exe, str(artifact_path), **run_kwargs) except Exception: return None # nosec - best-effort template generation - template_rel = Path(src_rel).as_posix() + ".j2" + ext = "erb" if template_engine == "erb" else "j2" + template_rel = Path(src_rel).as_posix() + f".{ext}" template_dst = Path(template_root) / template_rel context = yaml_load_mapping(result.vars_text) - if missing_jinja_template_vars(result.template_text, context): + missing = ( + missing_erb_template_vars(result.template_text, context) + if template_engine == "erb" + else missing_jinja_template_vars(result.template_text, context) + ) + if missing: # If this role was generated into an existing output directory, avoid - # leaving an obsolete .j2 behind after falling back to a raw copy. + # leaving an obsolete template behind after falling back to a raw copy. if overwrite_templates and template_dst.exists(): template_dst.unlink() return None @@ -311,6 +338,8 @@ def run_jinjaturtle( *, role_name: str, force_format: Optional[str] = None, + template_engine: str = "jinja2", + puppet_class: Optional[str] = None, ) -> JinjifyResult: """ Run jinjaturtle against src_path and return (template, defaults-yaml). @@ -318,6 +347,9 @@ def run_jinjaturtle( jinjaturtle CLI: jinjaturtle -r [-f ] [-d ] [-t ] + + Newer JinjaTurtle versions also support ``--template-engine erb`` and + ``--puppet-class`` for Puppet/Hiera output. """ src = Path(src_path) if not src.is_file(): @@ -326,7 +358,9 @@ def run_jinjaturtle( with tempfile.TemporaryDirectory(prefix="enroll-jt-") as td: td_path = Path(td) defaults_out = td_path / "defaults.yml" - template_out = td_path / "template.j2" + template_out = td_path / ( + "template.erb" if template_engine == "erb" else "template.j2" + ) cmd = [ jt_exe, @@ -340,6 +374,10 @@ def run_jinjaturtle( ] if force_format: cmd.extend(["-f", force_format]) + if template_engine != "jinja2": + cmd.extend(["--template-engine", template_engine]) + if puppet_class: + cmd.extend(["--puppet-class", puppet_class]) p = subprocess.run(cmd, text=True, capture_output=True) # nosec if p.returncode != 0: diff --git a/enroll/manifest.py b/enroll/manifest.py index c96a8dd..c9fca19 100644 --- a/enroll/manifest.py +++ b/enroll/manifest.py @@ -210,6 +210,7 @@ def manifest( out, fqdn=fqdn, no_common_roles=no_common_roles, + jinjaturtle=jinjaturtle, ) elif target == "salt": manifest_salt_from_bundle_dir( @@ -246,6 +247,7 @@ def manifest( str(tmp_out), fqdn=fqdn, no_common_roles=no_common_roles, + jinjaturtle=jinjaturtle, ) elif target == "salt": manifest_salt_from_bundle_dir( diff --git a/enroll/puppet.py b/enroll/puppet.py index 8a7e950..021ae5f 100644 --- a/enroll/puppet.py +++ b/enroll/puppet.py @@ -17,6 +17,12 @@ from .cm import ( markdown_list, ) from .state import inventory_packages_from_state, roles_from_state +from .jinjaturtle import ( + can_jinjify_path, + jinjify_artifact, + managed_file_var_prefix, + resolve_jinjaturtle_mode, +) class PuppetRole(CMModule): @@ -31,6 +37,7 @@ class PuppetRole(CMModule): self.flatpak_remotes: List[Dict[str, Any]] = [] self.flatpaks: List[Dict[str, Any]] = [] self.snaps: List[Dict[str, Any]] = [] + self.template_hiera: Dict[str, Any] = {} def has_resources(self) -> bool: return self.has_resources_or_attrs( @@ -133,8 +140,12 @@ class PuppetRole(CMModule): bundle_dir: str, artifact_role: str, module_files_dir: Path, + module_templates_dir: Optional[Path] = None, file_prefix: Optional[str] = None, notify_services: Optional[List[str]] = None, + jt_exe: Optional[str] = None, + jt_enabled: bool = False, + overwrite_templates: bool = True, ) -> None: for d in self.managed_dirs_from_snapshot(snap): path = str(d.get("path") or "").strip() @@ -146,33 +157,75 @@ class PuppetRole(CMModule): reason=d.get("reason") or "managed_dir", ) - for mf in self.managed_files_from_snapshot(snap): + managed_files = list(self.managed_files_from_snapshot(snap)) + candidates = [ + mf + for mf in managed_files + if str(mf.get("path") or "") + and str(mf.get("src_rel") or "") + and can_jinjify_path(str(mf.get("path") or "")) + ] + namespace_by_file = len(candidates) > 1 + + for mf in managed_files: path = str(mf.get("path") or "").strip() src_rel = str(mf.get("src_rel") or "").strip() if not path or not src_rel: continue - module_rel = _copy_artifact( - bundle_dir, - artifact_role, - src_rel, - module_files_dir, - dst_prefix=file_prefix, - ) - if not module_rel: - self.notes.append( - f"Skipped {path}: harvested artifact {artifact_role}/{src_rel} was not present." + + template_rel: Optional[str] = None + if module_templates_dir is not None: + role_prefix = ( + managed_file_var_prefix(self.module_name, src_rel) + if namespace_by_file + else self.module_name ) - continue + converted = jinjify_artifact( + bundle_dir, + artifact_role, + src_rel, + path, + module_templates_dir, + jt_exe=jt_exe, + jt_enabled=jt_enabled, + overwrite_templates=overwrite_templates, + role_name=role_prefix, + template_engine="erb", + puppet_class=self.module_name, + ) + if converted is not None: + template_rel = converted.template_rel + self.template_hiera.update(converted.context) + attrs: Dict[str, Any] = { "owner": mf.get("owner") or "root", "group": mf.get("group") or "root", "mode": mf.get("mode") or "0644", - "source": _source_uri(self.module_name, module_rel), "reason": mf.get("reason") or "managed_file", } + if template_rel is not None: + attrs["template"] = f"{self.module_name}/{template_rel}" + else: + module_rel = _copy_artifact( + bundle_dir, + artifact_role, + src_rel, + module_files_dir, + dst_prefix=file_prefix, + ) + if not module_rel: + self.notes.append( + f"Skipped {path}: harvested artifact {artifact_role}/{src_rel} was not present." + ) + continue + attrs["source"] = _source_uri(self.module_name, module_rel) if notify_services and not path.startswith("/etc/systemd/system/"): - refs = [f"Service[{_pp_quote(unit)}]" for unit in notify_services] - attrs["notify"] = refs[0] if len(refs) == 1 else f"[{', '.join(refs)}]" + notify_units = [unit for unit in notify_services if str(unit).strip()] + notify_value = _service_notify_value(notify_units) + if notify_value: + attrs["notify"] = notify_value + attrs["notify_services"] = notify_units + attrs["_notify_services"] = notify_units self.add_managed_file(path, **attrs) for ml in self.managed_links_from_snapshot(snap): @@ -380,6 +433,43 @@ def _pp_array(values: Iterable[Any]) -> str: return "[" + ", ".join(_pp_quote(v) for v in values) + "]" +def _pp_value(value: Any) -> str: + """Render a conservative Puppet literal for generated class defaults.""" + + if value is None: + return "undef" + if isinstance(value, bool): + return _pp_bool(value) + if isinstance(value, int) and not isinstance(value, bool): + return str(value) + if isinstance(value, float): + return repr(value) + if isinstance(value, list): + return "[" + ", ".join(_pp_value(v) for v in value) + "]" + if isinstance(value, dict): + parts = [] + for key in sorted(value, key=lambda k: str(k)): + parts.append(f"{_pp_quote(key)} => {_pp_value(value[key])}") + return "{" + ", ".join(parts) + "}" + return _pp_quote(value) + + +def _template_param_defaults(prole: PuppetRole) -> Dict[str, Any]: + prefix = f"{prole.module_name}::" + out: Dict[str, Any] = {} + for key, value in prole.template_hiera.items(): + key_s = str(key) + if key_s.startswith(prefix): + local = key_s[len(prefix) :] + elif "::" in key_s: + local = key_s.split("::", 1)[1] + else: + local = key_s + if local: + out[local] = value + return out + + def _puppet_exec_attrs( command: str, unless: str, @@ -469,6 +559,65 @@ def _render_firewall_runtime_execs( lines.append("") +def _active_service_snapshots_by_unit( + entries: Iterable[Dict[str, Any]], +) -> Dict[str, Dict[str, Any]]: + """Return active service snapshots keyed by systemd unit name.""" + + by_unit: Dict[str, Dict[str, Any]] = {} + for entry in entries: + if str(entry.get("kind") or "package") != "service": + continue + snap = entry.get("snapshot") or {} + if not isinstance(snap, dict): + continue + unit = str(snap.get("unit") or "").strip() + if not unit or str(snap.get("active_state") or "") != "active": + continue + by_unit.setdefault(unit, snap) + return by_unit + + +def _service_notify_value(units: Iterable[str]) -> Optional[str]: + refs = [f"Service[{_pp_quote(unit)}]" for unit in units if str(unit).strip()] + if not refs: + return None + return refs[0] if len(refs) == 1 else f"[{', '.join(refs)}]" + + +def _sync_service_notifications(puppet_roles: Iterable[PuppetRole]) -> None: + """Remove generated service notifications that do not target this catalog.""" + + roles = list(puppet_roles) + declared_services = {unit for role in roles for unit in role.services} + for role in roles: + for path, attrs in role.files.items(): + notify_units = [ + str(unit).strip() + for unit in (attrs.get("_notify_services") or []) + if str(unit).strip() + ] + if not notify_units: + attrs.pop("_notify_services", None) + continue + kept = [unit for unit in notify_units if unit in declared_services] + missing = sorted(set(notify_units) - set(kept)) + if missing: + role.notes.append( + "Skipped service notification for " + f"{path}: no generated Service resource for " + f"{', '.join(missing)}." + ) + notify_value = _service_notify_value(kept) + if notify_value: + attrs["notify"] = notify_value + attrs["notify_services"] = kept + else: + attrs.pop("notify", None) + attrs.pop("notify_services", None) + attrs.pop("_notify_services", None) + + def _copy_artifact( bundle_dir: str, role: str, @@ -515,6 +664,8 @@ def _collect_puppet_roles( *, fqdn: Optional[str] = None, no_common_roles: bool = False, + jt_exe: Optional[str] = None, + jt_enabled: bool = False, ) -> List[PuppetRole]: roles = roles_from_state(state) inventory_packages = inventory_packages_from_state(state) @@ -541,13 +692,18 @@ def _collect_puppet_roles( str(snap.get("role_name") or key), fallback="enroll_role" ) prole = ensure_role(role_name) - module_files_dir = modules_dir / prole.module_name / "files" + module_dir = modules_dir / prole.module_name + module_files_dir = module_dir / "files" prole.add_managed_content( snap, bundle_dir=bundle_dir, artifact_role=str(snap.get("role_name") or key), module_files_dir=module_files_dir, + module_templates_dir=module_dir / "templates", file_prefix=node_file_prefix, + jt_exe=jt_exe, + jt_enabled=jt_enabled, + overwrite_templates=not bool(fqdn), ) users_snap = roles.get("users") or {} @@ -557,12 +713,17 @@ def _collect_puppet_roles( ) prole = ensure_role(role_name) prole.add_users_snapshot(users_snap) + module_dir = modules_dir / prole.module_name prole.add_managed_content( users_snap, bundle_dir=bundle_dir, artifact_role=str(users_snap.get("role_name") or "users"), - module_files_dir=modules_dir / prole.module_name / "files", + module_files_dir=module_dir / "files", + module_templates_dir=module_dir / "templates", file_prefix=node_file_prefix, + jt_exe=jt_exe, + jt_enabled=jt_enabled, + overwrite_templates=not bool(fqdn), ) package_service_entries = list( @@ -573,6 +734,9 @@ def _collect_puppet_roles( service_units_by_package = CMModule.active_service_units_by_package( package_service_entries ) + service_snapshots_by_unit = _active_service_snapshots_by_unit( + package_service_entries + ) for entry in package_service_entries: snap = entry.get("snapshot") or {} @@ -598,13 +762,22 @@ def _collect_puppet_roles( notify_services = CMModule.active_service_units_for_package_snapshot( snap, service_units_by_package ) + for unit in notify_services: + service_snap = service_snapshots_by_unit.get(unit) + if service_snap is not None: + prole.add_service_snapshot(service_snap) + module_dir = modules_dir / prole.module_name prole.add_managed_content( snap, bundle_dir=bundle_dir, artifact_role=str(snap.get("role_name") or original_role_name), - module_files_dir=modules_dir / prole.module_name / "files", + module_files_dir=module_dir / "files", + module_templates_dir=module_dir / "templates", file_prefix=node_file_prefix, notify_services=notify_services, + jt_exe=jt_exe, + jt_enabled=jt_enabled, + overwrite_templates=not bool(fqdn), ) container_images = roles.get("container_images") or {} @@ -656,17 +829,29 @@ def _collect_puppet_roles( puppet_roles = sorted(out.values(), key=lambda r: role_order_key(r.role_name)) resolve_catalog_conflicts(puppet_roles) + _sync_service_notifications(puppet_roles) return [r for r in puppet_roles if r.has_resources()] def _render_role_class(prole: PuppetRole) -> str: has_sysctl_conf = "/etc/sysctl.d/99-enroll.conf" in prole.files + template_defaults = _template_param_defaults(prole) + params: List[str] = [] if has_sysctl_conf: + params.extend( + [ + " Boolean $sysctl_apply = true,", + " Boolean $sysctl_ignore_apply_errors = true,", + ] + ) + for name, value in sorted(template_defaults.items()): + params.append(f" Any ${name} = {_pp_value(value)},") + + if params: lines: List[str] = [ "# Generated by Enroll from harvest state.", f"class {prole.module_name} (", - " Boolean $sysctl_apply = true,", - " Boolean $sysctl_ignore_apply_errors = true,", + *params, ") {", "", ] @@ -718,19 +903,20 @@ def _render_role_class(prole: PuppetRole) -> str: ) for path, f in sorted(prole.files.items()): - _resource( - lines, - "file", - path, + file_attrs: List[Tuple[str, str]] = [("ensure", _pp_quote("file"))] + if f.get("template"): + file_attrs.append(("content", f"template({_pp_quote(f.get('template'))})")) + else: + file_attrs.append(("source", _pp_quote(f.get("source") or ""))) + file_attrs.extend( [ - ("ensure", _pp_quote("file")), - ("source", _pp_quote(f.get("source") or "")), ("owner", _pp_quote(f.get("owner") or "root")), ("group", _pp_quote(f.get("group") or "root")), ("mode", _pp_quote(f.get("mode") or "0644")), *([("notify", str(f.get("notify")))] if f.get("notify") else []), - ], + ] ) + _resource(lines, "file", path, file_attrs) for path, lnk in sorted(prole.links.items()): _resource( @@ -1031,7 +1217,14 @@ def _role_hiera_values(prole: PuppetRole) -> Dict[str, Any]: path: _attrs_with_ensure( prole.files[path], "file", - allowed={"source", "owner", "group", "mode", "notify"}, + allowed={ + "source", + "template", + "owner", + "group", + "mode", + "notify_services", + }, ) for path in sorted(prole.files) } @@ -1069,6 +1262,8 @@ def _role_hiera_values(prole: PuppetRole) -> Dict[str, Any]: if prole.notes: data[f"{prefix}notes"] = list(prole.notes) + data.update(prole.template_hiera) + if "/etc/sysctl.d/99-enroll.conf" in prole.files: data[f"{prefix}sysctl_apply"] = True data[f"{prefix}sysctl_ignore_apply_errors"] = True @@ -1098,6 +1293,10 @@ def _render_hiera_role_class(prole: PuppetRole) -> str: " Array[String] $notes = [],", " Boolean $sysctl_apply = true,", " Boolean $sysctl_ignore_apply_errors = true,", + *[ + f" Any ${name} = undef," + for name in sorted(_template_param_defaults(prole)) + ], ") {", "", " $packages.each |String $package_name| {", @@ -1124,24 +1323,50 @@ def _render_hiera_role_class(prole: PuppetRole) -> str: " }", " }", "", - " $files.each |String $resource_title, Hash $attrs| {", - " file { $resource_title:", + " # Declare services before files so file notify relationships can", + " # resolve in Hiera-driven classes.", + " $services.each |String $resource_title, Hash $attrs| {", + " service { $resource_title:", " * => $attrs,", " }", " }", "", + " $files.each |String $resource_title, Hash $attrs| {", + " $file_attrs = $attrs.filter |$key, $value| {", + " $key != 'template' and $key != 'notify_services'", + " }", + " if $attrs['notify_services'] {", + " $notify_targets = $attrs['notify_services'].map |String $unit| { Service[$unit] }", + " if $attrs['template'] {", + " file { $resource_title:", + " * => $file_attrs,", + " content => template($attrs['template']),", + " notify => $notify_targets,", + " }", + " } else {", + " file { $resource_title:", + " * => $file_attrs,", + " notify => $notify_targets,", + " }", + " }", + " } elsif $attrs['template'] {", + " file { $resource_title:", + " * => $file_attrs,", + " content => template($attrs['template']),", + " }", + " } else {", + " file { $resource_title:", + " * => $file_attrs,", + " }", + " }", + " }", + "", " $links.each |String $resource_title, Hash $attrs| {", " file { $resource_title:", " * => $attrs,", " }", " }", "", - " $services.each |String $resource_title, Hash $attrs| {", - " service { $resource_title:", - " * => $attrs,", - " }", - " }", - "", " $flatpak_remotes.each |Integer $idx, Hash $remote| {", " exec { $remote['state_id']:", " command => $remote['add_cmd'],", @@ -1385,7 +1610,8 @@ def _render_readme( - `hiera.yaml` configures per-node lookup from `data/nodes/%{{trusted.certname}}.yaml` with a fallback to `data/common.yaml`. - `data/nodes/{_node_data_filename(fqdn or '')}` contains this node's class list and class parameter data. - `modules//manifests/init.pp` contains reusable, data-driven classes. -- `modules//files/nodes//...` contains node-specific harvested file artifacts, avoiding clashes between hosts.""" +- `modules//files/nodes//...` contains node-specific harvested raw file artifacts, avoiding clashes between hosts. +- `modules//templates/` contains ERB templates when JinjaTurtle can convert a harvested config file.""" apply = f"""Run from this generated output directory, passing the node certname so Hiera selects the right node data: ```bash @@ -1395,14 +1621,15 @@ sudo puppet apply --modulepath ./modules --hiera_config ./hiera.yaml --certname If you depend on other pre-installed Puppet modules, you may need to pass in other modulepaths as well, e.g: ```bash -sudo puppet apply --modulepath ./modules:/etc/puppet/code/modules --hiera_config ./hiera.yaml --certname {fqdn} manifests/site.pp --noop +sudo puppet apply --modulepath ./modules:/etc/puppet/code/modules --hiera_config ./hiera.yaml --certname {fqdn} manifests/site.pp --noop --test ``` For Puppet agent/control-repo use, place this output where `hiera.yaml`, `data/`, `manifests/`, and `modules/` form the environment root. Re-running Enroll with another `--fqdn` into the same output directory adds or replaces that node's YAML without deleting existing node data.""" else: layout = """- `manifests/site.pp` declares a `node` block and includes the generated classes in manifest order. - `modules//manifests/init.pp` contains resources for each generated Enroll role/snapshot or common package group. -- `modules//files/` contains harvested file artifacts for that role or group. +- `modules//files/` contains harvested raw file artifacts for that role or group. +- `modules//templates/` contains ERB templates when JinjaTurtle can convert a harvested config file. - Generated module names avoid Puppet reserved words such as `default`.""" apply = """Run from this generated output directory so Puppet can find `./modules`, or pass an absolute module path: @@ -1413,7 +1640,7 @@ sudo puppet apply --modulepath ./modules manifests/site.pp --noop --test If you depend on other pre-installed Puppet modules, you may need to pass in other modulepaths as well, e.g: ```bash -sudo puppet apply --modulepath ./modules:/etc/puppet/code/modules manifests/site.pp --noop +sudo puppet apply --modulepath ./modules:/etc/puppet/code/modules manifests/site.pp --noop --test ```""" return f"""# Enroll Puppet manifest @@ -1449,7 +1676,7 @@ This Puppet target reuses the existing harvest state without changing harvesting ## Current limitations -- JinjaTurtle templating is currently Ansible/Salt-oriented and is not applied to Puppet output - there are no erb templates, just raw files. +- JinjaTurtle/ERB templating is best-effort. Files that JinjaTurtle cannot parse are copied as raw module files. - Review generated resources before applying them broadly across unlike hosts. ## Notes @@ -1468,11 +1695,13 @@ class PuppetManifestRenderer: *, fqdn: Optional[str] = None, no_common_roles: bool = False, + jinjaturtle: str = "auto", ) -> None: self.bundle_dir = bundle_dir self.out_dir = out_dir self.fqdn = fqdn self.no_common_roles = no_common_roles + self.jinjaturtle = jinjaturtle def render(self) -> None: """Render Puppet modules/site.pp from a harvest bundle.""" @@ -1492,12 +1721,16 @@ class PuppetManifestRenderer: manifests_dir.mkdir(parents=True, exist_ok=True) modules_dir.mkdir(parents=True, exist_ok=True) + jt_exe, jt_enabled = resolve_jinjaturtle_mode(self.jinjaturtle) + puppet_roles = _collect_puppet_roles( state, bundle_dir, modules_dir, fqdn=fqdn, no_common_roles=no_common_roles, + jt_exe=jt_exe, + jt_enabled=jt_enabled, ) for prole in puppet_roles: module_dir = modules_dir / prole.module_name @@ -1544,10 +1777,12 @@ def manifest_from_bundle_dir( *, fqdn: Optional[str] = None, no_common_roles: bool = False, + jinjaturtle: str = "auto", ) -> None: PuppetManifestRenderer( bundle_dir, out_dir, fqdn=fqdn, no_common_roles=no_common_roles, + jinjaturtle=jinjaturtle, ).render() diff --git a/enroll/salt.py b/enroll/salt.py index 8d29bbb..b841ac0 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, List, Optional, Tuple +from typing import Any, Dict, Iterable, List, Optional, Tuple import yaml @@ -150,6 +150,7 @@ class SaltRole(CMModule): jt_enabled: bool = False, overwrite_templates: bool = True, watch_services: Optional[List[str]] = None, + watch_service_states: Optional[List[str]] = None, ) -> None: for d in self.managed_dirs_from_snapshot(snap): path = str(d.get("path") or "").strip() @@ -162,6 +163,12 @@ class SaltRole(CMModule): reason=d.get("reason") or "managed_dir", ) + watch_state_ids = _service_watch_state_ids( + self.module_name, + watch_services=watch_services, + watch_service_states=watch_service_states, + ) + for mf in self.managed_files_from_snapshot(snap): path = str(mf.get("path") or "").strip() src_rel = str(mf.get("src_rel") or "").strip() @@ -190,10 +197,9 @@ class SaltRole(CMModule): "makedirs": True, "reason": mf.get("reason") or "managed_file", } - if watch_services and not path.startswith("/etc/systemd/system/"): + if watch_state_ids and not path.startswith("/etc/systemd/system/"): attrs["watch_in"] = [ - {"service": _state_id("service", unit, role=self.module_name)} - for unit in watch_services + {"service": state_id} for state_id in watch_state_ids ] self.add_managed_file(path, **attrs) continue @@ -218,10 +224,9 @@ class SaltRole(CMModule): "makedirs": True, "reason": mf.get("reason") or "managed_file", } - if watch_services and not path.startswith("/etc/systemd/system/"): + if watch_state_ids and not path.startswith("/etc/systemd/system/"): attrs["watch_in"] = [ - {"service": _state_id("service", unit, role=self.module_name)} - for unit in watch_services + {"service": state_id} for state_id in watch_state_ids ] self.add_managed_file(path, **attrs) @@ -271,6 +276,55 @@ def _state_id(prefix: str, value: Any, *, role: str = "") -> str: return "_".join(parts) +def _service_watch_state_ids( + role_name: str, + *, + watch_services: Optional[Iterable[str]] = None, + watch_service_states: Optional[Iterable[str]] = None, +) -> List[str]: + """Return de-duplicated Salt service state ids for watch_in requisites.""" + + out: List[str] = [] + seen = set() + for state_id in watch_service_states or []: + value = str(state_id or "").strip() + if value and value not in seen: + seen.add(value) + out.append(value) + for unit in watch_services or []: + unit_s = str(unit or "").strip() + if not unit_s: + continue + value = _state_id("service", unit_s, role=role_name) + if value not in seen: + seen.add(value) + out.append(value) + return out + + +def _active_service_state_ids_by_unit( + entries: Iterable[Dict[str, Any]], +) -> Dict[str, str]: + """Return generated Salt service state ids keyed by active systemd unit.""" + + by_unit: Dict[str, str] = {} + for entry in entries: + if str(entry.get("kind") or "package") != "service": + continue + snap = entry.get("snapshot") or {} + if not isinstance(snap, dict): + continue + unit = str(snap.get("unit") or "").strip() + if not unit or str(snap.get("active_state") or "") != "active": + continue + source_label = str(snap.get("role_name") or snap.get("unit") or "service") + role_name = _salt_name( + str(entry.get("role_label") or source_label), fallback="service" + ) + by_unit.setdefault(unit, _state_id("service", unit, role=role_name)) + return by_unit + + def _yaml_quote(value: Any) -> str: return json.dumps(str(value), ensure_ascii=False) @@ -639,6 +693,9 @@ def _collect_salt_roles( service_units_by_package = CMModule.active_service_units_by_package( package_service_entries ) + service_state_ids_by_unit = _active_service_state_ids_by_unit( + package_service_entries + ) for entry in package_service_entries: snap = entry.get("snapshot") or {} @@ -654,6 +711,7 @@ def _collect_salt_roles( ) srole = ensure_role(role_name) watch_services: List[str] = [] + watch_service_states: List[str] = [] if kind == "service": srole.add_service_snapshot(snap) unit = str(snap.get("unit") or "").strip() @@ -664,6 +722,13 @@ def _collect_salt_roles( watch_services = CMModule.active_service_units_for_package_snapshot( snap, service_units_by_package ) + watch_service_states = [ + service_state_ids_by_unit[unit] + for unit in watch_services + if unit in service_state_ids_by_unit + ] + if watch_service_states: + watch_services = [] srole.add_managed_content( snap, bundle_dir=bundle_dir, @@ -674,6 +739,7 @@ def _collect_salt_roles( jt_enabled=jt_enabled, overwrite_templates=not bool(fqdn), watch_services=watch_services, + watch_service_states=watch_service_states, ) container_images = roles.get("container_images") or {} diff --git a/tests/test_manifest_puppet.py b/tests/test_manifest_puppet.py index a4e9bff..74f0dc6 100644 --- a/tests/test_manifest_puppet.py +++ b/tests/test_manifest_puppet.py @@ -184,9 +184,9 @@ def test_manifest_puppet_writes_control_repo_style_output(tmp_path: Path): assert node_data["foo::files"]["/etc/foo/foo.conf"]["source"] == ( "puppet:///modules/foo/nodes/test.example/etc/foo.conf" ) - assert node_data["foo::files"]["/etc/foo/foo.conf"]["notify"] == ( - "Service['foo.service']" - ) + assert node_data["foo::files"]["/etc/foo/foo.conf"]["notify_services"] == [ + "foo.service" + ] assert node_data["foo::services"]["foo.service"] == { "ensure": "running", "enable": True, @@ -254,6 +254,111 @@ def test_manifest_puppet_writes_control_repo_style_output(tmp_path: Path): ).exists() +def test_manifest_puppet_fqdn_package_notify_service_declared_in_same_role( + tmp_path: Path, +): + bundle = tmp_path / "bundle" + out = tmp_path / "puppet" + artifact = bundle / "artifacts" / "apparmor" / "etc" / "apparmor" / "parser.conf" + artifact.parent.mkdir(parents=True, exist_ok=True) + artifact.write_text("cache-loc /var/cache/apparmor\n", encoding="utf-8") + + state = { + "schema_version": 3, + "host": {"hostname": "vpn-ssh", "os": "debian", "pkg_backend": "dpkg"}, + "inventory": {"packages": {"apparmor": {"section": "admin"}}}, + "roles": { + "services": [ + { + "unit": "apparmor.service", + "role_name": "apparmor_service", + "packages": ["apparmor"], + "active_state": "active", + "unit_file_state": "enabled", + "managed_dirs": [], + "managed_files": [], + "managed_links": [], + } + ], + "packages": [ + { + "package": "apparmor", + "role_name": "apparmor", + "section": "admin", + "managed_dirs": [], + "managed_files": [ + { + "path": "/etc/apparmor/parser.conf", + "src_rel": "etc/apparmor/parser.conf", + "owner": "root", + "group": "root", + "mode": "0644", + } + ], + "managed_links": [], + } + ], + "users": { + "role_name": "users", + "users": [], + "managed_dirs": [], + "managed_files": [], + }, + "apt_config": { + "role_name": "apt_config", + "managed_dirs": [], + "managed_files": [], + }, + "dnf_config": { + "role_name": "dnf_config", + "managed_dirs": [], + "managed_files": [], + }, + "sysctl": {"role_name": "sysctl", "managed_dirs": [], "managed_files": []}, + "firewall_runtime": {"role_name": "firewall_runtime", "packages": []}, + "etc_custom": { + "role_name": "etc_custom", + "managed_dirs": [], + "managed_files": [], + }, + "usr_local_custom": { + "role_name": "usr_local_custom", + "managed_dirs": [], + "managed_files": [], + }, + "extra_paths": { + "role_name": "extra_paths", + "managed_dirs": [], + "managed_files": [], + "managed_links": [], + }, + }, + } + _write_state(bundle, state) + + manifest.manifest(str(bundle), str(out), target="puppet", fqdn="vpn-ssh") + + node_data = yaml.safe_load( + (out / "data" / "nodes" / "vpn-ssh.yaml").read_text(encoding="utf-8") + ) + assert node_data["apparmor::files"]["/etc/apparmor/parser.conf"][ + "notify_services" + ] == ["apparmor.service"] + assert node_data["apparmor::services"]["apparmor.service"] == { + "ensure": "running", + "enable": True, + } + + apparmor_pp = (out / "modules" / "apparmor" / "manifests" / "init.pp").read_text( + encoding="utf-8" + ) + assert "Hash[String, Hash] $services = {}" in apparmor_pp + assert "service { $resource_title:" in apparmor_pp + assert apparmor_pp.index("$services.each") < apparmor_pp.index("$files.each") + assert "$attrs['notify_services'].map" in apparmor_pp + assert "notify => $notify_targets" in apparmor_pp + + def test_manifest_puppet_fqdn_mode_can_accumulate_separate_node_data( tmp_path: Path, ): @@ -999,3 +1104,224 @@ def test_puppet_names_are_sanitised_for_target_reserved_words() -> None: assert _puppet_name("123") == "role_123" assert _puppet_name("node") == "role_node" assert _puppet_name("web-app") == "web_app" + + +def test_manifest_puppet_uses_jinjaturtle_erb_templates(monkeypatch, tmp_path: Path): + import enroll.jinjaturtle as jinjaturtle_mod + from enroll.jinjaturtle import JinjifyResult + + bundle = tmp_path / "bundle" + out = tmp_path / "puppet" + artifact = bundle / "artifacts" / "foo" / "etc" / "foo.ini" + artifact.parent.mkdir(parents=True, exist_ok=True) + artifact.write_text("[main]\nkey = 1\n", encoding="utf-8") + + state = { + "schema_version": 3, + "host": {"hostname": "test", "os": "debian", "pkg_backend": "dpkg"}, + "inventory": {"packages": {}}, + "roles": { + "services": [ + { + "unit": "foo.service", + "role_name": "foo", + "packages": ["foo"], + "active_state": "active", + "unit_file_state": "enabled", + "managed_dirs": [], + "managed_files": [ + { + "path": "/etc/foo.ini", + "src_rel": "etc/foo.ini", + "owner": "root", + "group": "root", + "mode": "0644", + } + ], + "managed_links": [], + } + ], + "packages": [], + "users": { + "role_name": "users", + "users": [], + "managed_dirs": [], + "managed_files": [], + }, + "apt_config": { + "role_name": "apt_config", + "managed_dirs": [], + "managed_files": [], + }, + "dnf_config": { + "role_name": "dnf_config", + "managed_dirs": [], + "managed_files": [], + }, + "sysctl": {"role_name": "sysctl", "managed_dirs": [], "managed_files": []}, + "firewall_runtime": {"role_name": "firewall_runtime", "packages": []}, + "etc_custom": { + "role_name": "etc_custom", + "managed_dirs": [], + "managed_files": [], + }, + "usr_local_custom": { + "role_name": "usr_local_custom", + "managed_dirs": [], + "managed_files": [], + }, + "extra_paths": { + "role_name": "extra_paths", + "managed_dirs": [], + "managed_files": [], + "managed_links": [], + }, + }, + } + _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) + + calls = [] + + def fake_run_jinjaturtle( + jt_exe: str, + src_path: str, + *, + role_name: str, + force_format=None, + template_engine: str = "jinja2", + puppet_class=None, + ): + calls.append((role_name, template_engine, puppet_class)) + assert template_engine == "erb" + assert puppet_class == "foo" + return JinjifyResult( + template_text="[main]\nkey = <%= @main_key %>\n", + vars_text="foo::main_key: 1\n", + ) + + monkeypatch.setattr(jinjaturtle_mod, "run_jinjaturtle", fake_run_jinjaturtle) + + manifest.manifest( + str(bundle), + str(out), + target="puppet", + jinjaturtle="on", + no_common_roles=True, + ) + + assert calls == [("foo", "erb", "foo")] + assert (out / "modules" / "foo" / "templates" / "etc" / "foo.ini.erb").read_text( + encoding="utf-8" + ) == "[main]\nkey = <%= @main_key %>\n" + assert not (out / "modules" / "foo" / "files" / "etc" / "foo.ini").exists() + + init_pp = (out / "modules" / "foo" / "manifests" / "init.pp").read_text( + encoding="utf-8" + ) + assert "Any $main_key = 1," in init_pp + assert "content => template('foo/etc/foo.ini.erb')" in init_pp + assert "source =>" not in init_pp + + +def test_manifest_puppet_fqdn_writes_erb_template_values_to_node_hiera( + monkeypatch, tmp_path: Path +): + import enroll.jinjaturtle as jinjaturtle_mod + from enroll.jinjaturtle import JinjifyResult + + bundle = tmp_path / "bundle" + out = tmp_path / "puppet" + artifact = bundle / "artifacts" / "foo" / "etc" / "foo.ini" + artifact.parent.mkdir(parents=True, exist_ok=True) + artifact.write_text("[main]\nkey = 1\n", encoding="utf-8") + state = { + "schema_version": 3, + "host": {"hostname": "test", "os": "debian", "pkg_backend": "dpkg"}, + "inventory": {"packages": {}}, + "roles": { + "services": [ + { + "unit": "foo.service", + "role_name": "foo", + "packages": ["foo"], + "active_state": "active", + "unit_file_state": "enabled", + "managed_dirs": [], + "managed_files": [ + {"path": "/etc/foo.ini", "src_rel": "etc/foo.ini"} + ], + "managed_links": [], + } + ], + "packages": [], + "users": { + "role_name": "users", + "users": [], + "managed_dirs": [], + "managed_files": [], + }, + "apt_config": { + "role_name": "apt_config", + "managed_dirs": [], + "managed_files": [], + }, + "dnf_config": { + "role_name": "dnf_config", + "managed_dirs": [], + "managed_files": [], + }, + "sysctl": {"role_name": "sysctl", "managed_dirs": [], "managed_files": []}, + "firewall_runtime": {"role_name": "firewall_runtime", "packages": []}, + "etc_custom": { + "role_name": "etc_custom", + "managed_dirs": [], + "managed_files": [], + }, + "usr_local_custom": { + "role_name": "usr_local_custom", + "managed_dirs": [], + "managed_files": [], + }, + "extra_paths": { + "role_name": "extra_paths", + "managed_dirs": [], + "managed_files": [], + "managed_links": [], + }, + }, + } + _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) + monkeypatch.setattr( + jinjaturtle_mod, + "run_jinjaturtle", + lambda *a, **k: JinjifyResult( + template_text="[main]\nkey = <%= @main_key %>\n", + vars_text="foo::main_key: 1\n", + ), + ) + + manifest.manifest( + str(bundle), str(out), target="puppet", fqdn="test.example", jinjaturtle="on" + ) + + node_data = yaml.safe_load( + (out / "data" / "nodes" / "test.example.yaml").read_text(encoding="utf-8") + ) + assert node_data["foo::main_key"] == 1 + assert node_data["foo::files"]["/etc/foo.ini"]["template"] == "foo/etc/foo.ini.erb" + assert "source" not in node_data["foo::files"]["/etc/foo.ini"] + init_pp = (out / "modules" / "foo" / "manifests" / "init.pp").read_text( + encoding="utf-8" + ) + assert "Any $main_key = undef," in init_pp + assert "content => template($attrs['template'])" in init_pp diff --git a/tests/test_manifest_salt.py b/tests/test_manifest_salt.py index 7de2fae..73ca276 100644 --- a/tests/test_manifest_salt.py +++ b/tests/test_manifest_salt.py @@ -6,7 +6,13 @@ from pathlib import Path import yaml from enroll import manifest -from enroll.salt import SaltRole, _render_static_role, _role_pillar_values, _salt_name +from enroll.salt import ( + SaltRole, + _render_static_role, + _role_pillar_values, + _salt_name, + _state_id, +) def _write_state(bundle: Path, state: dict) -> None: @@ -188,6 +194,76 @@ def test_manifest_salt_writes_single_site_state_tree(tmp_path: Path): assert (out / "config" / "master.d" / "enroll.conf").exists() +def test_manifest_salt_fqdn_package_watch_targets_declared_service_role( + tmp_path: Path, +): + bundle = tmp_path / "bundle" + out = tmp_path / "salt" + artifact = bundle / "artifacts" / "apparmor" / "etc" / "apparmor" / "parser.conf" + artifact.parent.mkdir(parents=True, exist_ok=True) + artifact.write_text("cache-loc /var/cache/apparmor\n", encoding="utf-8") + + state = _sample_state() + state["inventory"] = {"packages": {"apparmor": {"section": "admin"}}} + state["roles"]["services"] = [ + { + "unit": "apparmor.service", + "role_name": "apparmor_service", + "packages": ["apparmor"], + "active_state": "active", + "unit_file_state": "enabled", + "managed_dirs": [], + "managed_files": [], + "managed_links": [], + } + ] + state["roles"]["packages"] = [ + { + "package": "apparmor", + "role_name": "apparmor", + "section": "admin", + "managed_dirs": [], + "managed_files": [ + { + "path": "/etc/apparmor/parser.conf", + "src_rel": "etc/apparmor/parser.conf", + "owner": "root", + "group": "root", + "mode": "0644", + } + ], + "managed_links": [], + } + ] + state["roles"]["sysctl"] = { + "role_name": "sysctl", + "managed_dirs": [], + "managed_files": [], + "managed_links": [], + } + _write_state(bundle, state) + + manifest.manifest(str(bundle), str(out), target="salt", fqdn="vpn-ssh") + + pillar_top = yaml.safe_load( + (out / "pillar" / "top.sls").read_text(encoding="utf-8") + ) + node_sls = pillar_top["base"]["vpn-ssh"][0] + pillar_path = out / "pillar" / Path(*node_sls.split(".")) + pillar = yaml.safe_load(pillar_path.with_suffix(".sls").read_text(encoding="utf-8")) + roles = pillar["enroll"]["roles"] + expected_service_state = _state_id( + "service", "apparmor.service", role="apparmor_service" + ) + + assert roles["apparmor"]["files"]["/etc/apparmor/parser.conf"]["watch_in"] == [ + {"service": expected_service_state} + ] + assert roles["apparmor_service"]["services"]["apparmor.service"]["state_id"] == ( + expected_service_state + ) + + def test_manifest_salt_fqdn_mode_uses_pillar_and_accumulates_nodes(tmp_path: Path): out = tmp_path / "salt" From f335077e595153cbd4afdbd281056c906daddbc6 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Sat, 20 Jun 2026 18:38:49 +1000 Subject: [PATCH 2/2] 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"