diff --git a/library/file_checksums.py b/library/file_checksums.py index ef52f5d245..ebf30aab09 100644 --- a/library/file_checksums.py +++ b/library/file_checksums.py @@ -1,12 +1,14 @@ -"""Report the SHA-1 and mode of many paths in one module call. +"""Report the type, mode and SHA-1 of many paths in one module call. Args: paths: absolute paths to inspect; a symlink is reported, never followed. Returns: - states: mapping of every requested path to ``{isreg, mode, checksum}``. - ``isreg`` is false for a missing or unreadable path and for anything - but a regular file; ``mode`` and ``checksum`` are then ``None``. + states: mapping of every requested path to ``{isreg, isdir, mode, checksum}``. + ``isreg`` is true only for a regular file and ``isdir`` only for a + directory; both are false for a missing or unreadable path and for + anything else. ``mode`` is set for a regular file or a directory and + ``checksum`` only for a regular file; otherwise they are ``None``. """ import os @@ -16,22 +18,21 @@ from ansible.module_utils.basic import AnsibleModule ARGUMENT_SPEC = {"paths": {"type": "list", "elements": "str", "required": True}} -_UNKNOWN = {"isreg": False, "mode": None, "checksum": None} +_UNKNOWN = {"isreg": False, "isdir": False, "mode": None, "checksum": None} def path_state(module, path): try: info = os.lstat(path) + mode = f"{stat.S_IMODE(info.st_mode):04o}" + if stat.S_ISDIR(info.st_mode): + return {"isreg": False, "isdir": True, "mode": mode, "checksum": None} if not stat.S_ISREG(info.st_mode): return dict(_UNKNOWN) digest = module.sha1(path) except OSError: return dict(_UNKNOWN) - return { - "isreg": True, - "mode": f"{stat.S_IMODE(info.st_mode):04o}", - "checksum": digest, - } + return {"isreg": True, "isdir": False, "mode": mode, "checksum": digest} def main(): diff --git a/plugins/action/stack_host_copy.py b/plugins/action/stack_host_copy.py index bc418e5e99..183bb9adbe 100755 --- a/plugins/action/stack_host_copy.py +++ b/plugins/action/stack_host_copy.py @@ -94,11 +94,12 @@ class ActionModule(CopyActionModule): _copy_action._walk_dirs = walk def _unchanged_tree(self, task_vars: dict[str, Any]) -> dict[str, Any] | None: - """Settle a directory copy whose files all match in one remote call. + """Settle a directory copy whose entries all match in one remote call. - The copy action spends a stat and a file module call per file even - when nothing differs; any difference, and any argument or source - shape not modelled here, leaves the decision to the regular copy. + The copy action spends a stat and a file module call per file, and a + file module call per directory holding no file, even when nothing + differs; any difference, and any argument or source shape not + modelled here, leaves the decision to the regular copy. Args: task_vars: Variables of the task, handed to the remote call. @@ -133,14 +134,19 @@ class ActionModule(CopyActionModule): trailing_slash_detector=shell.path_has_trailing_slash, ) implicit = {parent for _, rel in tree["files"] for parent in _parents(rel)} + leaves = [rel for _, rel in tree["directories"] if rel not in implicit] + directory_mode = args.get("directory_mode") if ( len(tree["files"]) < 2 or tree["symlinks"] - or any(rel not in implicit for _, rel in tree["directories"]) + or ( + leaves and directory_mode is not None and _octal(directory_mode) is None + ) ): return None dest = shell.join_path(dest, "") + directories = [shell.join_path(dest, rel) for rel in leaves] decrypt = _to_bool(args.get("decrypt", True), strict=False) wanted = {} for full, rel in tree["files"]: @@ -155,7 +161,7 @@ class ActionModule(CopyActionModule): found = self._execute_module( module_name="ansible.legacy.file_checksums", - module_args={"paths": sorted(wanted)}, + module_args={"paths": sorted([*wanted, *directories])}, task_vars=task_vars, ) states = found.get("states") @@ -169,6 +175,13 @@ class ActionModule(CopyActionModule): or _octal(state.get("mode")) != mode ): return None + for path in directories: + state = states.get(path) or {} + if not state.get("isdir") or ( + directory_mode is not None + and _octal(state.get("mode")) != _octal(directory_mode) + ): + return None self._remove_tmp_path(shell.tmpdir) return self._ensure_invocation({"changed": False, "dest": dest, "src": source}) diff --git a/tests/unit/python/library/test_file_checksums.py b/tests/unit/python/library/test_file_checksums.py index 0406431847..5c60dec986 100644 --- a/tests/unit/python/library/test_file_checksums.py +++ b/tests/unit/python/library/test_file_checksums.py @@ -61,15 +61,16 @@ class PathStateTests(unittest.TestCase): state, { "isreg": True, + "isdir": False, "mode": "0640", "checksum": hashlib.sha1(b"png").hexdigest(), # noqa: S324 - the checksum Ansible copy compares is SHA-1 }, ) - def test_missing_path_is_not_regular(self): + def test_missing_path_is_neither_regular_nor_a_directory(self): self.assertEqual( self._state(self.root / "absent"), - {"isreg": False, "mode": None, "checksum": None}, + {"isreg": False, "isdir": False, "mode": None, "checksum": None}, ) def test_directory_is_not_regular(self): @@ -77,6 +78,16 @@ class PathStateTests(unittest.TestCase): self.assertFalse(self._state(self.root / "nested")["isreg"]) + def test_directory_reports_its_mode_without_checksum(self): + target = self.root / ".cc-writes" + target.mkdir() + target.chmod(0o1750) + + self.assertEqual( + self._state(target), + {"isreg": False, "isdir": True, "mode": "1750", "checksum": None}, + ) + def test_symlink_is_reported_not_followed(self): target = self.root / "real" target.write_bytes(b"data") @@ -84,6 +95,15 @@ class PathStateTests(unittest.TestCase): self.assertFalse(self._state(self.root / "link")["isreg"]) + def test_symlink_to_a_directory_is_not_a_directory(self): + (self.root / "real").mkdir() + (self.root / "link").symlink_to(self.root / "real") + + self.assertEqual( + self._state(self.root / "link"), + {"isreg": False, "isdir": False, "mode": None, "checksum": None}, + ) + def test_path_below_a_file_is_not_regular(self): (self.root / "file").write_bytes(b"x") diff --git a/tests/unit/python/plugins/action/test_stack_host_copy.py b/tests/unit/python/plugins/action/test_stack_host_copy.py index f7e4744f34..de6440b8b5 100644 --- a/tests/unit/python/plugins/action/test_stack_host_copy.py +++ b/tests/unit/python/plugins/action/test_stack_host_copy.py @@ -174,6 +174,15 @@ def _sha1(data): return hashlib.sha1(data).hexdigest() # noqa: S324 - the checksum Ansible copy compares is SHA-1 +class _Dir: + def __init__(self, mode): + self.mode = mode + + +def _mode_bits(mode): + return mode if isinstance(mode, int) else int(mode, 8) + + class _FakeRemote: def __init__(self, entries): self.entries = dict(entries) @@ -192,28 +201,50 @@ class _FakeRemote: entry = self.entries.get(module_args["path"]) if entry is None: return {"stat": {"exists": False}} - if entry == "dir": + if isinstance(entry, _Dir): return {"stat": {"exists": True, "isdir": True}} return { "stat": {"exists": True, "isdir": False, "checksum": _sha1(entry[0])} } + if module_name == "ansible.legacy.file" and module_args["state"] == "directory": + return self._ensure_directory(module_args["path"], module_args["mode"]) if module_name == "ansible.legacy.file": entry = self.entries[ module_args["dest"] + module_args["_original_basename"] ] - mode = module_args["mode"] - return { - "changed": entry[1] != (mode if isinstance(mode, int) else int(mode, 8)) - } + return {"changed": entry[1] != _mode_bits(module_args["mode"])} if module_name == "ansible.legacy.copy": return {"changed": True} raise AssertionError(f"unexpected module {module_name}") + def _ensure_directory(self, path, mode): + entry = self.entries.get(path) + if entry is None: + return {"changed": True, "path": path} + if not isinstance(entry, _Dir): + return {"failed": True, "msg": f"{path} already exists as a file"} + return { + "changed": mode is not None and entry.mode != _mode_bits(mode), + "path": path, + } + def _state(self, path): entry = self.entries.get(path) - if entry is None or entry == "dir": - return {"isreg": False, "mode": None, "checksum": None} - return {"isreg": True, "mode": f"{entry[1]:04o}", "checksum": _sha1(entry[0])} + if entry is None: + return {"isreg": False, "isdir": False, "mode": None, "checksum": None} + if isinstance(entry, _Dir): + return { + "isreg": False, + "isdir": True, + "mode": f"{entry.mode:04o}", + "checksum": None, + } + return { + "isreg": True, + "isdir": False, + "mode": f"{entry[1]:04o}", + "checksum": _sha1(entry[0]), + } class TestStackHostCopyTree(unittest.TestCase): @@ -298,7 +329,7 @@ class TestStackHostCopyTree(unittest.TestCase): def test_directory_in_place_of_a_file_runs_the_regular_copy(self): entries = self._synced() - entries[self.DEST + "icon.png"] = "dir" + entries[self.DEST + "icon.png"] = _Dir(0o755) remote = _FakeRemote(entries) self._run(remote) @@ -360,19 +391,121 @@ class TestStackHostCopyTree(unittest.TestCase): super_run.assert_called_once() self.assertEqual(remote.calls, []) - def test_empty_source_directory_skips_the_tree_check(self): - (self.src / "empty").mkdir() - remote = _FakeRemote(self._synced()) + LEAVES: ClassVar[tuple[str, ...]] = ( + ".claude", + ".claude/.cc-writes", + "nested/.claude", + "nested/.claude/.cc-writes", + ) + + def _add_empty_directories(self): + for rel in self.LEAVES: + (self.src / rel).mkdir(parents=True, exist_ok=True) + + def _with_leaves(self, mode): + entries = self._synced() + entries.update({self.DEST + rel: _Dir(mode) for rel in self.LEAVES}) + return entries + + def _leaf_paths(self): + return sorted(self.DEST + rel for rel in self.LEAVES) + + @staticmethod + def _directory_calls(remote): + return sorted( + args["path"] + for name, args in zip(remote.calls, remote.args, strict=True) + if name == "ansible.legacy.file" and args["state"] == "directory" + ) + + def test_existing_empty_directories_settle_in_one_remote_call(self): + self._add_empty_directories() + remote = _FakeRemote(self._with_leaves(0o755)) + + result = self._run(remote) + + self.assertFalse(result["changed"]) + self.assertEqual(remote.calls, ["ansible.legacy.file_checksums"]) + self.assertEqual( + sorted(remote.args[0]["paths"]), + sorted([*self._synced(), *self._leaf_paths()]), + ) + + def test_unset_directory_mode_accepts_any_remote_directory_mode(self): + self._add_empty_directories() + remote = _FakeRemote(self._with_leaves(0o700)) + regular = _FakeRemote(self._with_leaves(0o700)) + + settled = self._run(remote) + with patch.object(ActionModule, "_unchanged_tree", return_value=None): + copied = self._run(regular) + + self.assertFalse(settled["changed"]) + self.assertEqual(remote.calls, ["ansible.legacy.file_checksums"]) + self.assertFalse(copied["changed"]) + self.assertEqual(self._directory_calls(regular), self._leaf_paths()) + + def test_requested_directory_mode_settles_in_one_remote_call(self): + self._add_empty_directories() + remote = _FakeRemote(self._with_leaves(0o750)) + + result = self._run(remote, directory_mode="0750") + + self.assertFalse(result["changed"]) + self.assertEqual(remote.calls, ["ansible.legacy.file_checksums"]) + + def test_missing_remote_directory_runs_the_regular_copy(self): + self._add_empty_directories() + entries = self._with_leaves(0o755) + del entries[self.DEST + "nested/.claude/.cc-writes"] + remote = _FakeRemote(entries) + + result = self._run(remote) + + self.assertTrue(result["changed"]) + self.assertEqual(self._directory_calls(remote), self._leaf_paths()) + + def test_directory_mode_drift_runs_the_regular_copy(self): + self._add_empty_directories() + remote = _FakeRemote(self._with_leaves(0o755)) + + result = self._run(remote, directory_mode="0750") + + self.assertTrue(result["changed"]) + self.assertEqual(self._directory_calls(remote), self._leaf_paths()) + + def test_file_in_place_of_a_directory_runs_the_regular_copy(self): + self._add_empty_directories() + entries = self._with_leaves(0o755) + entries[self.DEST + ".claude/.cc-writes"] = (b"", 0o644) + remote = _FakeRemote(entries) + + result = self._run(remote) + + self.assertTrue(result["failed"]) + self.assertIn(self.DEST + ".claude/.cc-writes", self._directory_calls(remote)) + + def test_symbolic_directory_mode_with_empty_directories_skips_the_tree_check(self): + self._add_empty_directories() + remote = _FakeRemote(self._with_leaves(0o755)) with patch( "plugins.action.stack_host_copy.CopyActionModule.run", return_value={"changed": False}, ) as super_run: - self._run(remote) + self._run(remote, directory_mode="u=rwx,g=rx,o=") super_run.assert_called_once() self.assertEqual(remote.calls, []) + def test_symbolic_directory_mode_without_empty_directories_keeps_the_check(self): + remote = _FakeRemote(self._synced()) + + result = self._run(remote, directory_mode="u=rwx,g=rx,o=") + + self.assertFalse(result["changed"]) + self.assertEqual(remote.calls, ["ansible.legacy.file_checksums"]) + def test_integer_mode_is_compared_as_octal(self): remote = _FakeRemote(self._synced())