From f85996b43a8fcc0c0ad3059cdc9d26caf24cf249 Mon Sep 17 00:00:00 2001 From: Kevin Veen-Birkenbach Date: Mon, 24 Aug 2026 14:28:00 +0200 Subject: [PATCH] feat(volumes): honour a per-mount tmpfs mode, and give checkmk its site tmp checkmk's entrypoint runs `omd create --no-tmpfs`, so the site's tmp directory is never mounted and the site cannot come up cleanly. Declaring it needs a tmpfs with sticky, world-writable permissions, which the volume layer could not express. container_volumes built the tmpfs options as a hard-coded single key: if mount.get("size") is not None: tmpfs_entry["tmpfs"] = {"size": mount["size"]} so a `mode` in the declaration was accepted by the schema and silently dropped by the renderer. It now accumulates size and mode and attaches the key only when something is set, coercing mode to the int docker expects. mounts.py validates both the bind and tmpfs spellings through _is_octal_mode and names the expected shape in the error, so `1777` or `0o1777` fails at render with a readable message instead of producing the wrong bits. Capability and consumer land together deliberately. Split apart, checkmk's mode: "01777" would pass validation, be dropped at render, and the tmp directory would come up without the sticky bit while everything reported success. checkmk declares site_tmp as a tmpfs at the site's tmp path with size 512m and mode 01777. The site name moves into services.checkmk.site, and CHECKMK_SITE reads it through lookup('config', ...) instead of hard-coding "cmk", so the mount target and the variable cannot drift. MERGE NOTE: fork/feature/svc-net-tor carries acc9e25c0f, which deletes plugins/filter/container_volumes.py and moves it to plugins/lookup/. That relocated copy has the identical defect at lines 141-142 -- the same hard-coded {"size": ...} -- and its _coerce_mode_int is wired only to the config-ref path. It is not a competing fix, but this change will disappear at merge unless it is re-applied in the lookup. Verified: the unit suite passes, 4448 tests, including new coverage of the mode path in test_container_volumes.py. Not verified: checkmk has not been watched starting with this mount. Docker's tmpfs mode also interacts with the container umask, so the effective bits on the created directory were not inspected live. The new validation is fail-closed: a role carrying a non-octal mode would now error at render rather than be ignored, and while the suite is green, I did not grep every CHECKMK_SITE consumer for one still assuming the literal "cmk". Co-Authored-By: Claude Opus 5 (1M context) --- plugins/filter/container_volumes.py | 7 +++- roles/web-app-checkmk/meta/services.yml | 1 + roles/web-app-checkmk/meta/volumes.yml | 8 ++++ roles/web-app-checkmk/vars/main.yml | 2 +- .../plugins/filter/test_container_volumes.py | 24 +++++++++++ utils/roles/applications/mounts.py | 40 ++++++++++++++----- 6 files changed, 70 insertions(+), 12 deletions(-) diff --git a/plugins/filter/container_volumes.py b/plugins/filter/container_volumes.py index f3fe83bce3..26ef614571 100644 --- a/plugins/filter/container_volumes.py +++ b/plugins/filter/container_volumes.py @@ -135,8 +135,13 @@ def container_volumes( "type": "tmpfs", "target": target, } + tmpfs_options: dict[str, Any] = {} if mount.get("size") is not None: - tmpfs_entry["tmpfs"] = {"size": mount["size"]} + tmpfs_options["size"] = mount["size"] + if mount.get("mode") is not None: + tmpfs_options["mode"] = _coerce_mode_int(mount["mode"]) + if tmpfs_options: + tmpfs_entry["tmpfs"] = tmpfs_options volumes_block.append(tmpfs_entry) elif vtype == "config": diff --git a/roles/web-app-checkmk/meta/services.yml b/roles/web-app-checkmk/meta/services.yml index d1bfc8f090..65c6de03ae 100644 --- a/roles/web-app-checkmk/meta/services.yml +++ b/roles/web-app-checkmk/meta/services.yml @@ -52,6 +52,7 @@ checkmk: enabled: true shared: true bond: 1 + site: cmk healthcheck: flavor: curl path: cmk/check_mk/login.py diff --git a/roles/web-app-checkmk/meta/volumes.yml b/roles/web-app-checkmk/meta/volumes.yml index 9037c1d647..52c937bb90 100644 --- a/roles/web-app-checkmk/meta/volumes.yml +++ b/roles/web-app-checkmk/meta/volumes.yml @@ -5,3 +5,11 @@ checkmk_data: mounts: - service: checkmk target: /omd/sites + +site_tmp: + type: tmpfs + mounts: + - service: checkmk + target: "/omd/sites/{{ lookup('config', 'web-app-checkmk', 'services.checkmk.site') }}/tmp" + size: 512m + mode: "01777" diff --git a/roles/web-app-checkmk/vars/main.yml b/roles/web-app-checkmk/vars/main.yml index 979d5c01b0..9808167fd4 100644 --- a/roles/web-app-checkmk/vars/main.yml +++ b/roles/web-app-checkmk/vars/main.yml @@ -3,7 +3,7 @@ application_id: "web-app-checkmk" container_port: "{{ lookup('config', application_id, 'services.checkmk.ports.internal.http') }}" CHECKMK_NAME: "{{ lookup('config', application_id, 'services.checkmk.name') }}" -CHECKMK_SITE: "cmk" +CHECKMK_SITE: "{{ lookup('config', application_id, 'services.checkmk.site') }}" CHECKMK_ADMIN_PASSWORD: "{{ lookup('config', application_id, 'credentials.cmk_password') }}" diff --git a/tests/unit/plugins/filter/test_container_volumes.py b/tests/unit/plugins/filter/test_container_volumes.py index 259f938c9d..3728ffe801 100644 --- a/tests/unit/plugins/filter/test_container_volumes.py +++ b/tests/unit/plugins/filter/test_container_volumes.py @@ -144,6 +144,30 @@ class TestContainerVolumesTmpfs(unittest.TestCase): }, ) + def test_tmpfs_mode_renders_as_decimal_of_the_octal_string(self) -> None: + meta = { + "scratch": { + "type": "tmpfs", + "mounts": [ + {"service": "app", "target": "/scratch", "mode": "01777"}, + ], + } + } + out = container_volumes(_apps(meta), "my-app", "app") + data = _parse(out) + self.assertEqual( + data, + { + "volumes": [ + { + "type": "tmpfs", + "target": "/scratch", + "tmpfs": {"mode": 0o1777}, + } + ] + }, + ) + class TestContainerVolumesServiceFilter(unittest.TestCase): def test_only_emits_mounts_for_requested_service(self) -> None: diff --git a/utils/roles/applications/mounts.py b/utils/roles/applications/mounts.py index dcb6b46182..9a64180f6e 100644 --- a/utils/roles/applications/mounts.py +++ b/utils/roles/applications/mounts.py @@ -22,6 +22,14 @@ Canonical shape (dict-of-dicts; the YAML key is the semantic short name): - service: synapse target: /etc/synapse/config + scratch: + type: tmpfs + mounts: + - service: synapse + target: /var/run/synapse + size: 64m # per-mount tmpfs size + mode: "01777" # per-mount tmpfs permission bits + The legacy list-of-dicts shape is no longer accepted. """ @@ -45,6 +53,13 @@ class VolumesSchemaError(ValueError): """Raised when a meta/volumes.yml fails schema validation.""" +def _is_octal_mode(value: Any) -> bool: + """True when ``value`` is a leading-zero octal permission string.""" + return ( + isinstance(value, str) and value.startswith("0") and value.lstrip("0").isdigit() + ) + + def normalize_volumes_meta( meta_volumes: Any, ) -> dict[str, dict[str, Any]]: @@ -120,16 +135,15 @@ def validate_volumes_meta( violations.append( f"{prefix}: 'mode' is only valid for type config/secret (got type={vtype!r})" ) - if "mode" in entry and vtype in _TYPE_HAS_MODE: - mode = entry["mode"] - if not ( - isinstance(mode, str) - and mode.startswith("0") - and mode.lstrip("0").isdigit() - ): - violations.append( - f"{prefix}: 'mode' must be an octal string like \"0440\", got {mode!r}" - ) + if ( + "mode" in entry + and vtype in _TYPE_HAS_MODE + and not _is_octal_mode(entry["mode"]) + ): + violations.append( + f"{prefix}: 'mode' must be an octal string like \"0440\", " + f"got {entry['mode']!r}" + ) if "read_only" in entry and vtype not in _TYPE_HAS_READ_ONLY: violations.append( @@ -176,6 +190,12 @@ def validate_volumes_meta( for mount in (entry.get("mounts") or []) if "size" in mount and not isinstance(mount["size"], (str, int)) ) + violations.extend( + f"{prefix}: tmpfs 'mode' must be an octal string like \"01777\", " + f"got {mount['mode']!r}" + for mount in (entry.get("mounts") or []) + if "mode" in mount and not _is_octal_mode(mount["mode"]) + ) mounts = entry.get("mounts") if mounts is None: