cmk-dev-deploy: replace site files via rename instead of in-place copy

The config deployer wrote onto existing destination files with
shutil.copy2(), which opens them for writing.  Pristine version trees
ship read-only files -- e.g. the 0555 console-script wrappers that
package_wheel generates since e6b62de -- and the version clone only
guarantees writable *directories* (see _ensure_writable_dirs), so a
fresh clone deterministically failed with

  fail  config_deploy  -- [Errno 13] Permission denied:
        '/omd/sites/v300/version/bin/cmk-general-version-infos'

A re-run only appeared to heal it because the concurrently running
wheel_deploy step had meanwhile replaced the file with a writable one.

Honour the documented invariant: copy to a sibling temp file and
os.replace() it over the destination, which only needs the writable
parent directory and is atomic on top.

Change-Id: I0f965fd714c85d4eacf1c6ca31ffc54685e58dbf

Author: englertor

packages/cmk-dev-deploy/cmk/dev_deploy/deployers/config_deployer.py

@@ -81,6 +81,24 @@ def _resolve_src(entry: ConfigFileEntry, repo_root: Path) -> Path:
     return repo_root / entry.src
 
 
+def _replace_file(src: Path, dst: Path, mode: int | None) -> None:
+    """Install ``src`` at ``dst`` without opening the destination for writing.
+
+    Version trees ship read-only files (e.g. 0555 console-script wrappers,
+    0444 bills of materials) and the clone only guarantees writable
+    *directories* (see ``version_clone._ensure_writable_dirs``).  Copying
+    onto the destination would fail with EACCES, so copy to a sibling temp
+    file and rename it over the destination, which only needs the writable
+    parent.
+    """
+    tmp = dst.with_name(dst.name + ".cmk-dev-deploy-tmp")
+    tmp.unlink(missing_ok=True)  # stale temp from an interrupted deploy
+    shutil.copy2(src, tmp)
+    if mode:
+        os.chmod(tmp, mode)
+    os.replace(tmp, dst)
+
+
 def _copy_dir(source: Path, dest: Path, spec: ConfigDeploySpec, repo_root: Path) -> None:
     """Copy a config/data directory to the site using the Bazel-derived file list.
 
@@ -109,13 +127,7 @@ def _copy_dir(source: Path, dest: Path, spec: ConfigDeploySpec, repo_root: Path)
 
             dst = dest / rel
             dst.parent.mkdir(parents=True, exist_ok=True)
-            shutil.copy2(src_path, dst)
-
-            # Apply file mode
-            mode = _resolve_mode(entry.mode, spec.mode, spec.file_chmod)
-            if mode:
-                os.chmod(dst, mode)
-
+            _replace_file(src_path, dst, _resolve_mode(entry.mode, spec.mode, spec.file_chmod))
             expected_files.add(rel)
 
         # Also copy files matching include patterns (dev convenience)
@@ -125,10 +137,7 @@ def _copy_dir(source: Path, dest: Path, spec: ConfigDeploySpec, repo_root: Path)
                     rel = str(match.relative_to(source))
                     dst = dest / rel
                     dst.parent.mkdir(parents=True, exist_ok=True)
-                    shutil.copy2(match, dst)
-                    mode = _resolve_mode("", spec.mode, spec.file_chmod)
-                    if mode:
-                        os.chmod(dst, mode)
+                    _replace_file(match, dst, _resolve_mode("", spec.mode, spec.file_chmod))
                     expected_files.add(rel)
 
         # Delete extra files at dest not present in source.
@@ -170,19 +179,17 @@ def _install_files(source: Path, dest: Path, spec: ConfigDeploySpec, repo_root:
                 continue
             dest.mkdir(parents=True, exist_ok=True)
             dest_file = dest / os.path.basename(entry.dest)
-            shutil.copy2(src_path, dest_file)
-            mode = _resolve_mode(entry.mode, spec.mode, spec.file_chmod)
-            if mode:
-                os.chmod(dest_file, mode)
+            _replace_file(
+                src_path, dest_file, _resolve_mode(entry.mode, spec.mode, spec.file_chmod)
+            )
             count += 1
     else:
         for src_entry in sorted(source.iterdir()):
             if src_entry.is_dir():
                 continue
             assert spec.mode is not None
             dest.mkdir(parents=True, exist_ok=True)
-            shutil.copy2(src_entry, dest / src_entry.name)
-            os.chmod(dest / src_entry.name, spec.mode)
+            _replace_file(src_entry, dest / src_entry.name, spec.mode)
             count += 1
 
     return count
@@ -257,14 +264,12 @@ def _compile_and_deploy_locale(source: Path, dest: Path, spec: ConfigDeploySpec)
         # Install alias file if present
         alias_file = lang_dir / "alias"
         if alias_file.exists():
-            shutil.copy2(alias_file, dest / name / "alias")
-            os.chmod(dest / name / "alias", 0o644)
+            _replace_file(alias_file, dest / name / "alias", 0o644)
 
         # Install compiled multisite.mo if present
         mo_file = lang_dir / "LC_MESSAGES" / "multisite.mo"
         if mo_file.exists():
-            shutil.copy2(mo_file, lc_dest / "multisite.mo")
-            os.chmod(lc_dest / "multisite.mo", 0o644)
+            _replace_file(mo_file, lc_dest / "multisite.mo", 0o644)
             files_installed += 1
 
     return files_installed, po_compiled

packages/cmk-dev-deploy/tests/test_config_deployer.py

@@ -12,7 +12,12 @@
 import pytest
 
 from cmk.dev_deploy.deployers import config_deployer
-from cmk.dev_deploy.deployers.config_deployer import _copy_dir, _install_files, deploy_config
+from cmk.dev_deploy.deployers.config_deployer import (
+    _compile_and_deploy_locale,
+    _copy_dir,
+    _install_files,
+    deploy_config,
+)
 from cmk.dev_deploy.types import (
     ConfigDeploySpec,
     ConfigFileEntry,
@@ -126,6 +131,83 @@ def test_delete_extra_keeps_renamed_file_and_removes_stray(self, tmp_path: Path)
         assert not (site_bin / "check_mk.py").exists()
 
 
+class TestReadOnlyDestination:
+    """Pristine version trees ship read-only files (e.g. 0555 wrappers).
+
+    The clone only guarantees writable parent directories, so deployers
+    must replace files via rename instead of opening them for writing.
+    """
+
+    def test_copy_dir_replaces_read_only_file(self, tmp_path: Path) -> None:
+        repo = tmp_path / "repo"
+        (repo / "bin").mkdir(parents=True)
+        (repo / "bin" / "check_mk.py").write_text("new")
+        site_bin = tmp_path / "site" / "bin"
+        site_bin.mkdir(parents=True)
+        (site_bin / "check_mk").write_text("pristine wrapper")
+        os.chmod(site_bin / "check_mk", 0o555)
+
+        spec = _spec(
+            source_prefix="bin/",
+            site_dest="bin/",
+            files=(ConfigFileEntry(src="bin/check_mk.py", dest="bin/check_mk", mode="0755"),),
+        )
+        _copy_dir(repo / "bin", site_bin, spec, repo)
+
+        assert (site_bin / "check_mk").read_text() == "new"
+        assert os.stat(site_bin / "check_mk").st_mode & 0o777 == 0o755
+
+    def test_install_files_replaces_read_only_file(self, tmp_path: Path) -> None:
+        repo = tmp_path / "repo"
+        (repo / "active_checks").mkdir(parents=True)
+        (repo / "active_checks" / "check_foo.py").write_text("new")
+        dest = tmp_path / "site" / "lib" / "nagios" / "plugins"
+        dest.mkdir(parents=True)
+        (dest / "check_foo").write_text("pristine")
+        os.chmod(dest / "check_foo", 0o555)
+
+        spec = _spec(
+            source_prefix="active_checks/",
+            site_dest="lib/nagios/plugins/",
+            method=DeployMethod.INSTALL_FILES,
+            files=(
+                ConfigFileEntry(
+                    src="active_checks/check_foo.py",
+                    dest="lib/nagios/plugins/check_foo",
+                    mode="0755",
+                ),
+            ),
+        )
+        assert _install_files(repo / "active_checks", dest, spec, repo) == 1
+
+        assert (dest / "check_foo").read_text() == "new"
+        assert os.stat(dest / "check_foo").st_mode & 0o777 == 0o755
+
+    def test_locale_deploy_replaces_read_only_files(self, tmp_path: Path) -> None:
+        source = tmp_path / "locale"
+        (source / "de" / "LC_MESSAGES").mkdir(parents=True)
+        (source / "de" / "alias").write_text("Deutsch")
+        (source / "de" / "LC_MESSAGES" / "multisite.mo").write_bytes(b"new mo")
+        dest = tmp_path / "site" / "locale"
+        (dest / "de" / "LC_MESSAGES").mkdir(parents=True)
+        (dest / "de" / "alias").write_text("old")
+        os.chmod(dest / "de" / "alias", 0o444)
+        (dest / "de" / "LC_MESSAGES" / "multisite.mo").write_bytes(b"old mo")
+        os.chmod(dest / "de" / "LC_MESSAGES" / "multisite.mo", 0o444)
+
+        spec = _spec(
+            source_prefix="locale/",
+            site_dest="share/check_mk/locale/",
+            method=DeployMethod.LOCALE_COMPILE,
+            files=(),
+        )
+        installed, _compiled = _compile_and_deploy_locale(source, dest, spec)
+
+        assert installed == 1
+        assert (dest / "de" / "alias").read_text() == "Deutsch"
+        assert (dest / "de" / "LC_MESSAGES" / "multisite.mo").read_bytes() == b"new mo"
+
+
 def _make_site(tmp_path: Path) -> tuple[SiteInfo, Path]:
     """Simulate an OMD site home next to a writable version clone.