Skip to content

Commit baddfa8

Browse files
dsrinkcursoragent
andcommitted
config path: robustly remove config serials with read-only dirs
Since 2.5.0b1 every config activation snapshots $OMD_ROOT/local into the new helper config serial (helper_config/<serial>/local). The copy is made with shutil.copytree, which preserves permission bits. On sites where the local/ hierarchy is intentionally read-only (e.g. because it is distributed centrally), the snapshot therefore contains directories without write permission. The next cleanup_old_configs run then crashes with PermissionError when removing an outdated serial, because on POSIX a directory must be writable to unlink its children. This aborts config activation. Restore permissions on the directories of the serial's tree and retry when the removal hits a PermissionError. Only directory permissions matter for deletion; symlinks are skipped so no permissions outside the tree can be changed. Use the same robust removal for the stale under-construction serial in create(), which has the same failure mode. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 48d770d commit baddfa8

2 files changed

Lines changed: 47 additions & 2 deletions

File tree

packages/cmk-ccc/cmk/ccc/config_path.py

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,27 @@ def _increment_to_next_serial(base: Path) -> int:
9393
return new_serial
9494

9595

96+
def _rmtree_robust(path: Path) -> None:
97+
"""Remove a tree even if directory write permissions were removed externally.
98+
99+
Some deployments strip write permissions from the distributed site tree.
100+
On POSIX, only the permissions of the *directories* matter for deletion,
101+
so restore them and retry.
102+
"""
103+
try:
104+
shutil.rmtree(path)
105+
except PermissionError:
106+
path.chmod(0o700)
107+
for dirpath, dirnames, _filenames in path.walk():
108+
for dirname in dirnames:
109+
subdir = dirpath / dirname
110+
# Don't follow symlinks: we must not change permissions
111+
# outside the tree. The symlink itself is simply unlinked.
112+
if not subdir.is_symlink():
113+
subdir.chmod(0o700)
114+
shutil.rmtree(path)
115+
116+
96117
def cleanup_old_configs(base: Path) -> None:
97118
root = VersionedConfigPath.make_root_path(base)
98119
if not root.exists():
@@ -123,7 +144,7 @@ def cleanup_old_configs(base: Path) -> None:
123144
count_kept_serials += 1
124145
continue
125146

126-
shutil.rmtree(serial.path)
147+
_rmtree_robust(serial.path)
127148

128149

129150
@contextmanager
@@ -138,7 +159,7 @@ def create(base: Path) -> Iterator[ConfigCreationContext]:
138159

139160
with suppress(FileNotFoundError):
140161
# this should not exist, but we must be robust.
141-
shutil.rmtree(under_construction_path)
162+
_rmtree_robust(under_construction_path)
142163
under_construction_path.mkdir(parents=True, exist_ok=False)
143164

144165
yield ConfigCreationContext(

packages/cmk-ccc/tests/test_config_path.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,30 @@ def test_integration_with_create(self, tmp_path: Path) -> None:
193193
assert (root / "latest").is_symlink()
194194
assert (root / "serial.mk").exists()
195195

196+
def test_removal_of_readonly_serial(self, tmp_path: Path) -> None:
197+
"""Old serials must be removed even if they contain read-only directories.
198+
199+
The `local` snapshot inside a serial dir preserves the permissions of
200+
`$OMD_ROOT/local`, which may be read-only (e.g. distributed setups).
201+
"""
202+
root, _ = self._make_helper_config(tmp_path, serials=[1, 2, 3], latest_serial=3)
203+
local = root / "1" / "local"
204+
plugins = local / "lib" / "plugins"
205+
plugins.mkdir(parents=True)
206+
(plugins / "check").touch()
207+
unreadable = local / "share"
208+
unreadable.mkdir()
209+
(unreadable / "file.mk").touch()
210+
unreadable.chmod(0o000)
211+
for directory in (plugins, local / "lib", local, root / "1"):
212+
directory.chmod(0o555)
213+
214+
cleanup_old_configs(tmp_path)
215+
216+
assert not (root / "1").exists()
217+
assert (root / "2").exists()
218+
assert (root / "3").exists()
219+
196220
def test_proper_sorting(self, tmp_path: Path) -> None:
197221
"""Serial directories must be sorted numerically, not lexically."""
198222
root, _ = self._make_helper_config(tmp_path, serials=[1, 5, 7, 12, 23], latest_serial=23)

0 commit comments

Comments
 (0)