From a0ac28f2137eb6a2211d712e54f9d5ff95a63559 Mon Sep 17 00:00:00 2001 From: Miguel Jacq Date: Sun, 21 Jun 2026 12:38:10 +1000 Subject: [PATCH] Support '--enforce' mode in 'enroll diff' with '--target' to use a specific config manager to run to enforce --- README.md | 17 +- enroll/cli.py | 14 +- enroll/diff.py | 201 ++++++++++++++---- ...st_diff_ignore_versions_exclude_enforce.py | 156 ++++++++++++++ 4 files changed, 334 insertions(+), 54 deletions(-) diff --git a/README.md b/README.md index 3463866..d2a8735 100644 --- a/README.md +++ b/README.md @@ -174,23 +174,24 @@ Compare two harvest bundles and report what changed. - `--sops` when comparing SOPS-encrypted harvest bundles - `--exclude-path ` (repeatable) to ignore file/dir drift under matching paths (same pattern syntax as harvest) - `--ignore-package-versions` to ignore package version-only drift (upgrades/downgrades) -- `--enforce` to apply the **old** harvest state locally (requires `ansible-playbook` on `PATH`) +- `--enforce` to apply the **old** harvest state locally (requires the relevant config manager tool on `PATH` - defaults to `ansible-playbook`) +- `--target` when using `--enforce`, to set the desired config manager tool to manifest to and run) **Noise suppression** - `--exclude-path` is useful for things that change often but you still want in the harvest baseline (e.g. `/var/anacron`). - `--ignore-package-versions` keeps routine upgrades from alerting; package add/remove drift is still reported. -**Enforcement (`--enforce`)** -If a diff exists and `ansible-playbook` is available, Enroll will: +**Enforcement (`--enforce` (`--target`))** +If a diff exists and the config manager defined by `--target` (default: ansible) is on the PATH, Enroll will: 1) generate a manifest from the **old** harvest into a temporary directory -2) run `ansible-playbook -i localhost, -c local /playbook.yml` (often with `--tags role_<...>` to limit runtime) +2) run the config manager tool against that manifest 3) record in the diff report that the old harvest was enforced Enforcement is intentionally “safe”: - reinstalls packages that were removed (`state: present`), but does **not** attempt downgrades/pinning - restores users, files (contents + permissions/ownership), and service enable/start state -If `ansible-playbook` is not on `PATH`, Enroll returns an error and does not enforce. +If the config manager tool is not on `PATH`, Enroll returns an error and does not enforce. **Output formats** @@ -557,7 +558,7 @@ enroll diff --old /path/to/harvestA --new /path/to/harvestB --exclude-path /var/ enroll diff --old /path/to/harvestA --new /path/to/harvestB --ignore-package-versions ``` -### Enforce the old harvest state when drift is detected (requires Ansible) +### Enforce the old harvest state when drift is detected ```bash enroll diff --old /path/to/harvestA --new /path/to/harvestB --enforce --ignore-package-versions --exclude-path /var/anacron ``` @@ -692,12 +693,14 @@ exclude_path = /usr/local/bin/docker-*, /usr/local/bin/some-tool # you can set defaults here too, e.g. no_jinjaturtle = true sops = 54A91143AE0AB4F7743B01FE888ED1B423A3BC99 +# target = ansible (the default), or salt, or puppet [diff] # ignore noisy drift exclude_path = /var/anacron ignore_package_versions = true -# enforce = true # requires ansible-playbook on PATH +# enforce = true # requires the target config manager on PATH +# target = puppet (for example, as per manifest) [single-shot] # if you use single-shot, put its defaults here. diff --git a/enroll/cli.py b/enroll/cli.py index 7586cd2..e3816f0 100644 --- a/enroll/cli.py +++ b/enroll/cli.py @@ -634,10 +634,19 @@ def main() -> None: action="store_true", help=( "If differences are detected, attempt to enforce the old harvest state locally by generating a manifest and " - "running ansible-playbook. Requires ansible-playbook on PATH. " + "running the selected local apply tool. " "Enroll does not attempt to downgrade packages; if the only drift is package version upgrades (or newly installed packages), enforcement is skipped." ), ) + d.add_argument( + "--target", + choices=["ansible", "puppet", "salt"], + default="ansible", + help=( + "Configuration-management target to use with --enforce (default: ansible). " + "Requires ansible-playbook, puppet, or salt-call on PATH as appropriate." + ), + ) d.add_argument( "--out", help="Write the report to this file instead of stdout.", @@ -945,7 +954,7 @@ def main() -> None: ) # Optional enforcement: if drift is detected, attempt to restore the - # system to the *old* (baseline) state using ansible-playbook. + # system to the *old* (baseline) state using the selected target. if bool(getattr(args, "enforce", False)): if has_changes: if not has_enforceable_drift(report): @@ -963,6 +972,7 @@ def main() -> None: args.old, sops_mode=bool(getattr(args, "sops", False)), report=report, + target=getattr(args, "target", "ansible"), ) except Exception as e: raise SystemExit( diff --git a/enroll/diff.py b/enroll/diff.py index 4784119..9a9bee4 100644 --- a/enroll/diff.py +++ b/enroll/diff.py @@ -658,6 +658,113 @@ def _role_tag(role: str) -> str: return f"role_{safe}" +def _normalise_enforcement_target(target: str) -> str: + t = str(target or "ansible").strip().lower() + if t not in {"ansible", "puppet", "salt"}: + raise ValueError(f"unsupported enforcement target: {target!r}") + return t + + +def _enforcement_tool(target: str) -> Tuple[str, str]: + """Return (binary-name, human-label) for a local enforcement target.""" + if target == "puppet": + return "puppet", "puppet apply" + if target == "salt": + return "salt-call", "salt-call" + return "ansible-playbook", "ansible-playbook" + + +def _require_enforcement_tool(target: str) -> Tuple[str, str]: + binary, label = _enforcement_tool(target) + exe = shutil.which(binary) + if not exe: + install_hint = { + "ansible": "Ansible", + "puppet": "Puppet", + "salt": "Salt", + }.get(target, target) + raise RuntimeError( + f"{binary} not found on PATH " + f"(cannot enforce with target {target}; install {install_hint})" + ) + return exe, label + + +def _enforcement_command( + target: str, + exe: str, + manifest_dir: Path, + *, + tags: Optional[List[str]] = None, +) -> Tuple[List[str], Dict[str, str]]: + """Return the local apply command and environment for a rendered manifest.""" + env = dict(os.environ) + + if target == "ansible": + playbook = manifest_dir / "playbook.yml" + if not playbook.exists(): + raise RuntimeError( + f"manifest did not produce expected playbook.yml at {playbook}" + ) + + cfg = manifest_dir / "ansible.cfg" + if cfg.exists(): + env["ANSIBLE_CONFIG"] = str(cfg) + + cmd = [ + exe, + "-i", + "localhost,", + "-c", + "local", + str(playbook), + ] + if tags: + cmd.extend(["--tags", ",".join(tags)]) + return cmd, env + + if target == "puppet": + site_pp = manifest_dir / "manifests" / "site.pp" + if not site_pp.exists(): + raise RuntimeError( + f"manifest did not produce expected Puppet site.pp at {site_pp}" + ) + + cmd = [ + exe, + "apply", + "--modulepath", + str(manifest_dir / "modules"), + ] + hiera_config = manifest_dir / "hiera.yaml" + if hiera_config.exists(): + cmd.extend(["--hiera_config", str(hiera_config)]) + cmd.append(str(site_pp)) + return cmd, env + + if target == "salt": + states_dir = manifest_dir / "states" + top_sls = states_dir / "top.sls" + if not top_sls.exists(): + raise RuntimeError( + f"manifest did not produce expected Salt top.sls at {top_sls}" + ) + + cmd = [ + exe, + "--local", + "--file-root", + str(states_dir), + ] + pillar_dir = manifest_dir / "pillar" + if pillar_dir.exists(): + cmd.extend(["--pillar-root", str(pillar_dir)]) + cmd.extend(["state.apply"]) + return cmd, env + + raise ValueError(f"unsupported enforcement target: {target!r}") + + def _enforcement_plan( report: Dict[str, Any], old_state: Dict[str, Any], @@ -767,22 +874,22 @@ def enforce_old_harvest( *, sops_mode: bool = False, report: Optional[Dict[str, Any]] = None, + target: str = "ansible", ) -> Dict[str, Any]: """Enforce the *old* (baseline) harvest state on the current machine. - When Ansible is available, this: - 1) renders a temporary manifest from the old harvest, and - 2) runs ansible-playbook locally to apply it. + This renders a temporary manifest from the old harvest using the requested + target, then runs the target's local apply command: + - ansible: ansible-playbook -i localhost, -c local playbook.yml + - puppet: puppet apply --modulepath ./modules manifests/site.pp + - salt: salt-call --local --file-root ./states state.apply Returns a dict suitable for attaching to the diff report under report['enforcement']. """ - ansible_playbook = shutil.which("ansible-playbook") - if not ansible_playbook: - raise RuntimeError( - "ansible-playbook not found on PATH (cannot enforce; install Ansible)" - ) + target = _normalise_enforcement_target(target) + tool_exe, tool_label = _require_enforcement_tool(target) # Import lazily to avoid heavy import cost and potential CLI cycles. from .manifest import manifest @@ -802,8 +909,12 @@ def enforce_old_harvest( if report is not None: plan = _enforcement_plan(report, old_state, old_b.dir) roles = list(plan.get("roles") or []) - t = list(plan.get("tags") or []) - tags = t if t else None + # Only Ansible has generated per-role tags that can safely narrow + # the apply scope. Puppet and Salt enforcement deliberately run the + # full generated local manifest/catalog for now. + if target == "ansible": + t = list(plan.get("tags") or []) + tags = t if t else None with tempfile.TemporaryDirectory(prefix="enroll-enforce-") as td: td_path = Path(td) @@ -813,30 +924,15 @@ def enforce_old_harvest( pass # 1) Generate a manifest in a temp directory. - manifest(str(old_b.dir), str(td_path)) - - playbook = td_path / "playbook.yml" - if not playbook.exists(): - raise RuntimeError( - f"manifest did not produce expected playbook.yml at {playbook}" - ) + manifest(str(old_b.dir), str(td_path), target=target) # 2) Apply it locally. - env = dict(os.environ) - cfg = td_path / "ansible.cfg" - if cfg.exists(): - env["ANSIBLE_CONFIG"] = str(cfg) - - cmd = [ - ansible_playbook, - "-i", - "localhost,", - "-c", - "local", - str(playbook), - ] - if tags: - cmd.extend(["--tags", ",".join(tags)]) + cmd, env = _enforcement_command( + target, + tool_exe, + td_path, + tags=tags, + ) spinner: Optional[_Spinner] = None p: Optional[subprocess.CompletedProcess[str]] = None @@ -844,12 +940,12 @@ def enforce_old_harvest( if _progress_enabled(): if tags: sys.stderr.write( - f"Enforce: running ansible-playbook (tags: {','.join(tags)})\n", + f"Enforce: running {tool_label} (tags: {','.join(tags)})\n", ) else: - sys.stderr.write("Enforce: running ansible-playbook\n") + sys.stderr.write(f"Enforce: running {tool_label}\n") sys.stderr.flush() - spinner = _Spinner(" ansible-playbook") + spinner = _Spinner(f" {tool_label}") spinner.start() try: @@ -867,8 +963,8 @@ def enforce_old_harvest( rc = p.returncode if p is not None else None spinner.stop( final_line=( - f"Enforce: ansible-playbook finished in {elapsed:0.1f}s" - + (f" (rc={rc})" if rc is not None else ""), + f"Enforce: {tool_label} finished in {elapsed:0.1f}s" + + (f" (rc={rc})" if rc is not None else "") ), ) @@ -876,23 +972,32 @@ def enforce_old_harvest( info: Dict[str, Any] = { "status": "applied" if p.returncode == 0 else "failed", + "target": target, + "tool": tool_label, + "executable": tool_exe, "started_at": started_at, "finished_at": finished_at, - "ansible_playbook": ansible_playbook, "command": cmd, "returncode": int(p.returncode), } + # Keep the original Ansible-specific field for compatibility with + # existing consumers of the JSON report. + if target == "ansible": + info["ansible_playbook"] = tool_exe + elif target == "puppet": + info["puppet"] = tool_exe + elif target == "salt": + info["salt_call"] = tool_exe - # Record tag selection (if we could attribute drift to specific roles). info["roles"] = roles info["tags"] = list(tags or []) if not tags: - info["scope"] = "full_playbook" + info["scope"] = "full_manifest" if p.returncode != 0: err = (p.stderr or p.stdout or "").strip() raise RuntimeError( - "ansible-playbook failed" + f"{tool_label} failed" + (f" (rc={p.returncode})" if p.returncode is not None else "") + (f": {err}" if err else "") ) @@ -937,6 +1042,9 @@ def _report_text(report: Dict[str, Any]) -> str: if enf: lines.append("\nEnforcement") status = str(enf.get("status") or "").strip().lower() + tool = str(enf.get("tool") or "ansible-playbook") + target = str(enf.get("target") or "ansible") + via = f"{tool} ({target})" if target and target not in tool else tool if status == "applied": extra = "" tags = enf.get("tags") or [] @@ -946,7 +1054,7 @@ def _report_text(report: Dict[str, Any]) -> str: elif scope: extra = f" ({scope})" lines.append( - f" applied old harvest via ansible-playbook (rc={enf.get('returncode')})" + f" applied old harvest via {via} (rc={enf.get('returncode')})" + extra + ( f" (finished {enf.get('finished_at')})" @@ -956,7 +1064,7 @@ def _report_text(report: Dict[str, Any]) -> str: ) elif status == "failed": lines.append( - f" attempted enforcement but ansible-playbook failed (rc={enf.get('returncode')})" + f" attempted enforcement but {via} failed (rc={enf.get('returncode')})" ) elif status == "skipped": r = enf.get("reason") @@ -1096,6 +1204,9 @@ def _report_markdown(report: Dict[str, Any]) -> str: if enf: out.append("\n## Enforcement\n") status = str(enf.get("status") or "").strip().lower() + tool = str(enf.get("tool") or "ansible-playbook") + target = str(enf.get("target") or "ansible") + via = f"{tool} ({target})" if target and target not in tool else tool if status == "applied": extra = "" tags = enf.get("tags") or [] @@ -1105,7 +1216,7 @@ def _report_markdown(report: Dict[str, Any]) -> str: elif scope: extra = f" ({scope})" out.append( - "- ✅ Applied old harvest via ansible-playbook" + f"- ✅ Applied old harvest via {via}" + extra + ( f" (rc={enf.get('returncode')})" @@ -1121,7 +1232,7 @@ def _report_markdown(report: Dict[str, Any]) -> str: ) elif status == "failed": out.append( - "- ⚠️ Attempted enforcement but ansible-playbook failed" + f"- ⚠️ Attempted enforcement but {via} failed" + ( f" (rc={enf.get('returncode')})" if enf.get("returncode") is not None diff --git a/tests/test_diff_ignore_versions_exclude_enforce.py b/tests/test_diff_ignore_versions_exclude_enforce.py index fd0524f..d08c60e 100644 --- a/tests/test_diff_ignore_versions_exclude_enforce.py +++ b/tests/test_diff_ignore_versions_exclude_enforce.py @@ -309,6 +309,162 @@ def test_enforce_old_harvest_runs_ansible_with_tags_from_file_drift( assert "role_usr_local_custom" in str(argv[i + 1]) +def test_enforce_old_harvest_runs_puppet_target(monkeypatch, tmp_path: Path): + import enroll.diff as d + import enroll.manifest as mf + + monkeypatch.setattr( + d.shutil, + "which", + lambda name: "/usr/bin/puppet" if name == "puppet" else None, + ) + + calls: dict[str, object] = {} + + def fake_manifest(_harvest_dir: str, out_dir: str, **kwargs): + calls["manifest_target"] = kwargs.get("target") + out = Path(out_dir) + (out / "manifests").mkdir(parents=True) + (out / "modules").mkdir(parents=True) + (out / "manifests" / "site.pp").write_text( + "node default { }\n", encoding="utf-8" + ) + + monkeypatch.setattr(mf, "manifest", fake_manifest) + + def fake_run( + argv, cwd=None, env=None, capture_output=False, text=False, check=False + ): + calls["argv"] = list(argv) + calls["cwd"] = cwd + return types.SimpleNamespace(returncode=0, stdout="ok", stderr="") + + monkeypatch.setattr(d.subprocess, "run", fake_run) + + old = tmp_path / "old" + _write_bundle(old, {"inventory": {"packages": {}}, "roles": _minimal_roles()}) + + report = { + "packages": {"added": [], "removed": ["curl"], "version_changed": []}, + "services": {"enabled_added": [], "enabled_removed": [], "changed": []}, + "users": {"added": [], "removed": [], "changed": []}, + "files": {"added": [], "removed": [], "changed": []}, + } + + info = d.enforce_old_harvest(str(old), report=report, target="puppet") + + assert info["status"] == "applied" + assert info["target"] == "puppet" + assert info["tool"] == "puppet apply" + assert info["scope"] == "full_manifest" + assert info["tags"] == [] + assert calls["manifest_target"] == "puppet" + + argv = calls.get("argv") + assert argv and argv[:2] == ["/usr/bin/puppet", "apply"] + assert "--modulepath" in argv + assert str(Path(calls["cwd"]) / "manifests" / "site.pp") in argv + + +def test_enforce_old_harvest_runs_salt_target(monkeypatch, tmp_path: Path): + import enroll.diff as d + import enroll.manifest as mf + + monkeypatch.setattr( + d.shutil, + "which", + lambda name: "/usr/bin/salt-call" if name == "salt-call" else None, + ) + + calls: dict[str, object] = {} + + def fake_manifest(_harvest_dir: str, out_dir: str, **kwargs): + calls["manifest_target"] = kwargs.get("target") + out = Path(out_dir) + (out / "states").mkdir(parents=True) + (out / "states" / "top.sls").write_text("base:\n '*': []\n", encoding="utf-8") + + monkeypatch.setattr(mf, "manifest", fake_manifest) + + def fake_run( + argv, cwd=None, env=None, capture_output=False, text=False, check=False + ): + calls["argv"] = list(argv) + calls["cwd"] = cwd + return types.SimpleNamespace(returncode=0, stdout="ok", stderr="") + + monkeypatch.setattr(d.subprocess, "run", fake_run) + + old = tmp_path / "old" + _write_bundle(old, {"inventory": {"packages": {}}, "roles": _minimal_roles()}) + + report = { + "packages": {"added": [], "removed": ["curl"], "version_changed": []}, + "services": {"enabled_added": [], "enabled_removed": [], "changed": []}, + "users": {"added": [], "removed": [], "changed": []}, + "files": {"added": [], "removed": [], "changed": []}, + } + + info = d.enforce_old_harvest(str(old), report=report, target="salt") + + assert info["status"] == "applied" + assert info["target"] == "salt" + assert info["tool"] == "salt-call" + assert info["scope"] == "full_manifest" + assert calls["manifest_target"] == "salt" + + argv = calls.get("argv") + assert argv and argv[0] == "/usr/bin/salt-call" + assert "--local" in argv + assert "--file-root" in argv + assert "state.apply" in argv + assert str(Path(calls["cwd"]) / "states") in argv + + +def test_cli_diff_enforce_forwards_target(monkeypatch): + import enroll.cli as cli + + report = { + "packages": {"added": [], "removed": ["curl"], "version_changed": []}, + "services": {"enabled_added": [], "enabled_removed": [], "changed": []}, + "users": {"added": [], "removed": [], "changed": []}, + "files": {"added": [], "removed": [], "changed": []}, + } + + monkeypatch.setattr(cli, "compare_harvests", lambda *a, **k: (report, True)) + monkeypatch.setattr(cli, "has_enforceable_drift", lambda r: True) + + calls: dict[str, object] = {} + + def fake_enforce(old, **kwargs): + calls["old"] = old + calls.update(kwargs) + return {"status": "applied", "target": kwargs.get("target"), "returncode": 0} + + monkeypatch.setattr(cli, "enforce_old_harvest", fake_enforce) + monkeypatch.setattr(cli, "format_report", lambda report, fmt="text": "R\n") + monkeypatch.setattr( + sys, + "argv", + [ + "enroll", + "diff", + "--old", + "/tmp/old", + "--new", + "/tmp/new", + "--enforce", + "--target", + "puppet", + ], + ) + + cli.main() + assert calls["old"] == "/tmp/old" + assert calls["target"] == "puppet" + assert calls["report"] is report + + def test_cli_diff_forwards_exclude_and_ignore_flags(monkeypatch, capsys): import enroll.cli as cli