Fix tests

enroll/harvest.py

@@ -132,7 +132,7 @@ def _safe_name(s: str) -> str:
 
 
 def _role_id(raw: str) -> str:
-    # normalize separators first
+    # normalise separators first
     s = re.sub(r"[^A-Za-z0-9]+", "_", raw)
     # split CamelCase -> snake_case
     s = re.sub(r"([a-z0-9])([A-Z])", r"\1_\2", s)

enroll/manifest.py

@@ -48,8 +48,24 @@ def _yaml_dump_mapping(obj: Dict[str, Any], *, sort_keys: bool = True) -> str:
         for k, v in sorted(obj.items()) if sort_keys else obj.items():
             lines.append(f"{k}: {v!r}")
         return "\n".join(lines).rstrip() + "\n"
+
+    # ansible-lint/yamllint's indentation rules are stricter than YAML itself.
+    # In particular, they expect sequences nested under a mapping key to be
+    # indented (e.g. `foo:\n  - a`), whereas PyYAML's default is often
+    # `foo:\n- a`.
+    class _IndentDumper(yaml.SafeDumper):  # type: ignore
+        def increase_indent(self, flow: bool = False, indentless: bool = False):
+            return super().increase_indent(flow, False)
+
     return (
-        yaml.safe_dump(obj, default_flow_style=False, sort_keys=sort_keys).rstrip()
+        yaml.dump(
+            obj,
+            Dumper=_IndentDumper,
+            default_flow_style=False,
+            sort_keys=sort_keys,
+            indent=2,
+            allow_unicode=True,
+        ).rstrip()
         + "\n"
     )
 
@@ -124,7 +140,7 @@ def _extract_jinjaturtle_block(text: str) -> str:
     return text.strip() + "\n"
 
 
-def _normalize_jinjaturtle_vars_text(vars_text: str) -> str:
+def _normalise_jinjaturtle_vars_text(vars_text: str) -> str:
     """Deduplicate keys in a vars fragment by parsing as YAML and dumping it back."""
     m = _yaml_load_mapping(vars_text)
     if not m:
@@ -166,14 +182,14 @@ def _copy_artifacts(
             dst = os.path.join(dst_files_dir, rel)
 
             # If a file was successfully templatised by JinjaTurtle, do NOT
-            # also materialize the raw copy in the destination files dir.
+            # also materialise the raw copy in the destination files dir.
             # (This keeps the output minimal and avoids redundant "raw" files.)
             if exclude_rels and rel in exclude_rels:
                 try:
                     if os.path.isfile(dst):
                         os.remove(dst)
                 except Exception:
-                    pass
+                    pass  # nosec
                 continue
 
             if preserve_existing and os.path.exists(dst):
@@ -342,7 +358,7 @@ def _jinjify_managed_files(
         except Exception:
             # If jinjaturtle cannot process a file for any reason, skip silently.
             # (Enroll's core promise is to be optimistic and non-interactive.)
-            continue
+            continue  # nosec
 
         tmpl_rel = src_rel + ".j2"
         tmpl_dst = os.path.join(role_dir, "templates", tmpl_rel)
@@ -372,7 +388,7 @@ def _hostvars_only_jinjaturtle(vars_text: str) -> str:
 def _defaults_with_jinjaturtle(base_defaults: str, vars_text: str) -> str:
     if not vars_text.strip():
         return base_defaults.rstrip() + "\n"
-    vars_text = _normalize_jinjaturtle_vars_text(vars_text)
+    vars_text = _normalise_jinjaturtle_vars_text(vars_text)
     # Always regenerate the block (we regenerate whole defaults files anyway)
     return (
         base_defaults.rstrip()
@@ -450,7 +466,11 @@ def _render_generic_files_tasks(
     owner: "{{{{ item.owner }}}}"
     group: "{{{{ item.group }}}}"
     mode: "{{{{ item.mode }}}}"
-  loop: "{{{{ {var_prefix}_managed_files | default([]) | selectattr('is_systemd_unit','equalto', true) | selectattr('kind','equalto','template') | list }}}}"
+  loop: >-
+    {{{{ {var_prefix}_managed_files | default([])
+      | selectattr('is_systemd_unit', 'equalto', true)
+      | selectattr('kind', 'equalto', 'template')
+      | list }}}}
   notify: "{{{{ item.notify | default([]) }}}}"
 
 - name: Deploy systemd unit files (copies)
@@ -465,12 +485,20 @@ def _render_generic_files_tasks(
     owner: "{{{{ item.owner }}}}"
     group: "{{{{ item.group }}}}"
     mode: "{{{{ item.mode }}}}"
-  loop: "{{{{ {var_prefix}_managed_files | default([]) | selectattr('is_systemd_unit','equalto', true) | selectattr('kind','equalto','copy') | list }}}}"
+  loop: >-
+    {{{{ {var_prefix}_managed_files | default([])
+      | selectattr('is_systemd_unit', 'equalto', true)
+      | selectattr('kind', 'equalto', 'copy')
+      | list }}}}
   notify: "{{{{ item.notify | default([]) }}}}"
 
 - name: Reload systemd to pick up unit changes
   ansible.builtin.meta: flush_handlers
-  when: "({var_prefix}_managed_files | default([]) | selectattr('is_systemd_unit','equalto', true) | list | length) > 0"
+  when: >-
+    ({var_prefix}_managed_files | default([])
+      | selectattr('is_systemd_unit', 'equalto', true)
+      | list
+      | length) > 0
 
 - name: Deploy other managed files (templates)
   ansible.builtin.template:
@@ -479,7 +507,11 @@ def _render_generic_files_tasks(
     owner: "{{{{ item.owner }}}}"
     group: "{{{{ item.group }}}}"
     mode: "{{{{ item.mode }}}}"
-  loop: "{{{{ {var_prefix}_managed_files | default([]) | selectattr('is_systemd_unit','equalto', false) | selectattr('kind','equalto','template') | list }}}}"
+  loop: >-
+    {{{{ {var_prefix}_managed_files | default([])
+      | selectattr('is_systemd_unit', 'equalto', false)
+      | selectattr('kind', 'equalto', 'template')
+      | list }}}}
   notify: "{{{{ item.notify | default([]) }}}}"
 
 - name: Deploy other managed files (copies)
@@ -494,7 +526,11 @@ def _render_generic_files_tasks(
     owner: "{{{{ item.owner }}}}"
     group: "{{{{ item.group }}}}"
     mode: "{{{{ item.mode }}}}"
-  loop: "{{{{ {var_prefix}_managed_files | default([]) | selectattr('is_systemd_unit','equalto', false) | selectattr('kind','equalto','copy') | list }}}}"
+  loop: >-
+    {{{{ {var_prefix}_managed_files | default([])
+      | selectattr('is_systemd_unit', 'equalto', false)
+      | selectattr('kind', 'equalto', 'copy')
+      | list }}}}
   notify: "{{{{ item.notify | default([]) }}}}"
 """
 

tests.sh

@@ -11,7 +11,7 @@ rm -rf "${BUNDLE_DIR}" "${ANSIBLE_DIR}"
 
 # Generate data
 poetry run \
-	enroll enroll \
+	enroll single-shot \
 	  --harvest "${BUNDLE_DIR}" \
 	  --out "${ANSIBLE_DIR}"
 

tests/test_cli.py

@@ -22,9 +22,12 @@ def test_cli_harvest_subcommand_calls_harvest(monkeypatch, capsys, tmp_path):
 def test_cli_manifest_subcommand_calls_manifest(monkeypatch, tmp_path):
     called = {}
 
-    def fake_manifest(harvest_dir: str, out_dir: str):
+    def fake_manifest(harvest_dir: str, out_dir: str, **kwargs):
         called["harvest"] = harvest_dir
         called["out"] = out_dir
+        # Common manifest args should be passed through by the CLI.
+        called["fqdn"] = kwargs.get("fqdn")
+        called["jinjaturtle"] = kwargs.get("jinjaturtle")
 
     monkeypatch.setattr(cli, "manifest", fake_manifest)
     monkeypatch.setattr(
@@ -43,6 +46,8 @@ def test_cli_manifest_subcommand_calls_manifest(monkeypatch, tmp_path):
     cli.main()
     assert called["harvest"] == str(tmp_path / "bundle")
     assert called["out"] == str(tmp_path / "ansible")
+    assert called["fqdn"] is None
+    assert called["jinjaturtle"] == "auto"
 
 
 def test_cli_enroll_subcommand_runs_harvest_then_manifest(monkeypatch, tmp_path):
@@ -52,8 +57,16 @@ def test_cli_enroll_subcommand_runs_harvest_then_manifest(monkeypatch, tmp_path)
         calls.append(("harvest", bundle_dir))
         return str(tmp_path / "bundle" / "state.json")
 
-    def fake_manifest(bundle_dir: str, out_dir: str):
-        calls.append(("manifest", bundle_dir, out_dir))
+    def fake_manifest(bundle_dir: str, out_dir: str, **kwargs):
+        calls.append(
+            (
+                "manifest",
+                bundle_dir,
+                out_dir,
+                kwargs.get("fqdn"),
+                kwargs.get("jinjaturtle"),
+            )
+        )
 
     monkeypatch.setattr(cli, "harvest", fake_harvest)
     monkeypatch.setattr(cli, "manifest", fake_manifest)
@@ -62,7 +75,7 @@ def test_cli_enroll_subcommand_runs_harvest_then_manifest(monkeypatch, tmp_path)
         "argv",
         [
             "enroll",
-            "enroll",
+            "single-shot",
             "--harvest",
             str(tmp_path / "bundle"),
             "--out",
@@ -73,5 +86,38 @@ def test_cli_enroll_subcommand_runs_harvest_then_manifest(monkeypatch, tmp_path)
     cli.main()
     assert calls == [
         ("harvest", str(tmp_path / "bundle")),
-        ("manifest", str(tmp_path / "bundle"), str(tmp_path / "ansible")),
+        ("manifest", str(tmp_path / "bundle"), str(tmp_path / "ansible"), None, "auto"),
     ]
+
+
+def test_cli_manifest_common_args(monkeypatch, tmp_path):
+    """Ensure --fqdn and jinjaturtle mode flags are forwarded correctly."""
+
+    called = {}
+
+    def fake_manifest(harvest_dir: str, out_dir: str, **kwargs):
+        called["harvest"] = harvest_dir
+        called["out"] = out_dir
+        called["fqdn"] = kwargs.get("fqdn")
+        called["jinjaturtle"] = kwargs.get("jinjaturtle")
+
+    monkeypatch.setattr(cli, "manifest", fake_manifest)
+    monkeypatch.setattr(
+        sys,
+        "argv",
+        [
+            "enroll",
+            "manifest",
+            "--harvest",
+            str(tmp_path / "bundle"),
+            "--out",
+            str(tmp_path / "ansible"),
+            "--fqdn",
+            "example.test",
+            "--no-jinjaturtle",
+        ],
+    )
+
+    cli.main()
+    assert called["fqdn"] == "example.test"
+    assert called["jinjaturtle"] == "off"

tests/test_jinjaturtle.py

@@ -0,0 +1,99 @@
+import json
+from pathlib import Path
+
+import enroll.manifest as manifest_mod
+from enroll.jinjaturtle import JinjifyResult
+
+
+def test_manifest_uses_jinjaturtle_templates_and_does_not_copy_raw(
+    monkeypatch, tmp_path: Path
+):
+    """If jinjaturtle can templatisize a file, we should store a template in the role
+    and avoid keeping the raw file copy in the destination files area.
+
+    This test stubs out jinjaturtle execution so it doesn't depend on the external tool.
+    """
+
+    bundle = tmp_path / "bundle"
+    out = tmp_path / "ansible"
+
+    # A jinjaturtle-compatible config file.
+    (bundle / "artifacts" / "foo" / "etc").mkdir(parents=True, exist_ok=True)
+    (bundle / "artifacts" / "foo" / "etc" / "foo.ini").write_text(
+        "[main]\nkey = 1\n", encoding="utf-8"
+    )
+
+    state = {
+        "host": {"hostname": "test", "os": "debian"},
+        "users": {
+            "role_name": "users",
+            "users": [],
+            "managed_files": [],
+            "excluded": [],
+            "notes": [],
+        },
+        "etc_custom": {
+            "role_name": "etc_custom",
+            "managed_files": [],
+            "excluded": [],
+            "notes": [],
+        },
+        "services": [
+            {
+                "unit": "foo.service",
+                "role_name": "foo",
+                "packages": ["foo"],
+                "active_state": "inactive",
+                "sub_state": "dead",
+                "unit_file_state": "disabled",
+                "condition_result": "no",
+                "managed_files": [
+                    {
+                        "path": "/etc/foo.ini",
+                        "src_rel": "etc/foo.ini",
+                        "owner": "root",
+                        "group": "root",
+                        "mode": "0644",
+                        "reason": "modified_conffile",
+                    }
+                ],
+                "excluded": [],
+                "notes": [],
+            }
+        ],
+        "package_roles": [],
+    }
+
+    bundle.mkdir(parents=True, exist_ok=True)
+    (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8")
+
+    # Pretend jinjaturtle exists.
+    monkeypatch.setattr(
+        manifest_mod, "find_jinjaturtle_cmd", lambda: "/usr/bin/jinjaturtle"
+    )
+
+    # Stub jinjaturtle output.
+    def fake_run_jinjaturtle(
+        jt_exe: str, src_path: str, *, role_name: str, force_format=None
+    ):
+        assert role_name == "foo"
+        return JinjifyResult(
+            template_text="[main]\nkey = {{ foo_key }}\n",
+            vars_text="foo_key: 1\n",
+        )
+
+    monkeypatch.setattr(manifest_mod, "run_jinjaturtle", fake_run_jinjaturtle)
+
+    manifest_mod.manifest(str(bundle), str(out), jinjaturtle="on")
+
+    # Template should exist in the role.
+    assert (out / "roles" / "foo" / "templates" / "etc" / "foo.ini.j2").exists()
+
+    # Raw file should NOT be copied into role files/ because it was templatised.
+    assert not (out / "roles" / "foo" / "files" / "etc" / "foo.ini").exists()
+
+    # Defaults should include jinjaturtle vars.
+    defaults = (out / "roles" / "foo" / "defaults" / "main.yml").read_text(
+        encoding="utf-8"
+    )
+    assert "foo_key: 1" in defaults

tests/test_manifest.py

@@ -94,10 +94,16 @@ def test_manifest_writes_roles_and_playbook_with_clean_when(tmp_path: Path):
 
     manifest(str(bundle), str(out))
 
-    # Service role: conditional start must be a clean Ansible expression
+    # Service role: systemd management should be gated on foo_manage_unit and a probe.
     tasks = (out / "roles" / "foo" / "tasks" / "main.yml").read_text(encoding="utf-8")
-    assert "when:\n    - _unit_probe is succeeded\n    - foo_start | bool\n" in tasks
-    # Ensure we didn't emit deprecated/broken '{{ }}' delimiters in when:
+    assert "- name: Probe whether systemd unit exists and is manageable" in tasks
+    assert "when: foo_manage_unit | default(false)" in tasks
+    assert (
+        "when:\n    - foo_manage_unit | default(false)\n    - _unit_probe is succeeded\n"
+        in tasks
+    )
+
+    # Ensure we didn't emit deprecated/broken '{{ }}' delimiters in when: lines.
     for line in tasks.splitlines():
         if line.lstrip().startswith("when:"):
             assert "{{" not in line and "}}" not in line
@@ -105,7 +111,9 @@ def test_manifest_writes_roles_and_playbook_with_clean_when(tmp_path: Path):
     defaults = (out / "roles" / "foo" / "defaults" / "main.yml").read_text(
         encoding="utf-8"
     )
-    assert "foo_start: false" in defaults
+    assert "foo_manage_unit: true" in defaults
+    assert "foo_systemd_enabled: true" in defaults
+    assert "foo_systemd_state: stopped" in defaults
 
     # Playbook should include users, etc_custom, packages, and services
     pb = (out / "playbook.yml").read_text(encoding="utf-8")
@@ -113,3 +121,105 @@ def test_manifest_writes_roles_and_playbook_with_clean_when(tmp_path: Path):
     assert "- etc_custom" in pb
     assert "- curl" in pb
     assert "- foo" in pb
+
+
+def test_manifest_site_mode_creates_host_inventory_and_raw_files(tmp_path: Path):
+    """In --fqdn mode, host-specific state goes into inventory/host_vars."""
+
+    fqdn = "host1.example.test"
+    bundle = tmp_path / "bundle"
+    out = tmp_path / "ansible"
+
+    # Artifacts for a service-managed file.
+    (bundle / "artifacts" / "foo" / "etc").mkdir(parents=True, exist_ok=True)
+    (bundle / "artifacts" / "foo" / "etc" / "foo.conf").write_text(
+        "x", encoding="utf-8"
+    )
+
+    # Artifacts for etc_custom file so copy works.
+    (bundle / "artifacts" / "etc_custom" / "etc" / "default").mkdir(
+        parents=True, exist_ok=True
+    )
+    (bundle / "artifacts" / "etc_custom" / "etc" / "default" / "keyboard").write_text(
+        "kbd", encoding="utf-8"
+    )
+
+    state = {
+        "host": {"hostname": "test", "os": "debian"},
+        "users": {
+            "role_name": "users",
+            "users": [],
+            "managed_files": [],
+            "excluded": [],
+            "notes": [],
+        },
+        "etc_custom": {
+            "role_name": "etc_custom",
+            "managed_files": [
+                {
+                    "path": "/etc/default/keyboard",
+                    "src_rel": "etc/default/keyboard",
+                    "owner": "root",
+                    "group": "root",
+                    "mode": "0644",
+                    "reason": "custom_unowned",
+                }
+            ],
+            "excluded": [],
+            "notes": [],
+        },
+        "services": [
+            {
+                "unit": "foo.service",
+                "role_name": "foo",
+                "packages": ["foo"],
+                "active_state": "active",
+                "sub_state": "running",
+                "unit_file_state": "enabled",
+                "condition_result": "yes",
+                "managed_files": [
+                    {
+                        "path": "/etc/foo.conf",
+                        "src_rel": "etc/foo.conf",
+                        "owner": "root",
+                        "group": "root",
+                        "mode": "0644",
+                        "reason": "modified_conffile",
+                    }
+                ],
+                "excluded": [],
+                "notes": [],
+            }
+        ],
+        "package_roles": [],
+    }
+
+    bundle.mkdir(parents=True, exist_ok=True)
+    (bundle / "state.json").write_text(json.dumps(state, indent=2), encoding="utf-8")
+
+    manifest(str(bundle), str(out), fqdn=fqdn)
+
+    # Host playbook exists.
+    assert (out / "playbooks" / f"{fqdn}.yml").exists()
+
+    # Role defaults are safe/host-agnostic in site mode.
+    foo_defaults = (out / "roles" / "foo" / "defaults" / "main.yml").read_text(
+        encoding="utf-8"
+    )
+    assert "foo_packages: []" in foo_defaults
+    assert "foo_managed_files: []" in foo_defaults
+    assert "foo_manage_unit: false" in foo_defaults
+
+    # Host vars contain host-specific state.
+    foo_hostvars = (out / "inventory" / "host_vars" / fqdn / "foo.yml").read_text(
+        encoding="utf-8"
+    )
+    assert "foo_packages" in foo_hostvars
+    assert "foo_managed_files" in foo_hostvars
+    assert "foo_manage_unit: true" in foo_hostvars
+    assert "foo_systemd_state: started" in foo_hostvars
+
+    # Non-templated raw config is stored per-host under .files.
+    assert (
+        out / "inventory" / "host_vars" / fqdn / "foo" / ".files" / "etc" / "foo.conf"
+    ).exists()