Fix volume init to always reconcile the host directory's own attributes
resolve_volume_mount() was supposed to initialize a fresh host directory from the image's own content at the mounted container path, but only ever did anything when that image-side directory was non-empty. Two bugs followed: an image declaring an *empty* directory with specific ownership/ permissions (e.g. a data directory owned by a non-root uid/gid) got a host directory with plain create_directories() defaults instead, and even the non-empty case never reconciled the directory's own attributes -- only each copied entry's. Fixed by splitting into two independent steps in the embedded cp script: copy contents only when non-empty (as before), then always reconcile the host directory's own mode/ownership/timestamps/xattrs via `cp -a --attributes-only -T`. -T/--no-target-directory turned out to be required -- caught by direct testing: without it, cp nests the image directory *into* the already-existing host directory instead of reconciling its attributes, which silently produced a spurious nested copy and left the host directory's own attributes untouched. Verified end-to-end against images/gitea.tar's real declared /etc/gitea and /var/lib/gitea volumes under a real rootless mount: the resulting host directories' mode/ownership now match the image's own declared values, with no leftover mounts/layers afterward. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Gv3s5jckJKzh6JkMoi2Akz
This commit is contained in:
@@ -471,20 +471,38 @@ Source layout (all under `src/`):
|
||||
`resolve_volume_mount()`, called once per `-v` occurrence from `run_container()`
|
||||
when running with `-r`. A spec with no `/` is looked up in `config.volumes` by
|
||||
name (error if unknown); one with `/` is treated as a host directory path and
|
||||
`create_directories()`'d if missing. If the resulting host directory is empty and
|
||||
the image already has non-empty content at the given container path, that content
|
||||
is copied in first. Both the existence check and the copy run as a single
|
||||
`sh -c '[ -d ... ] && cp -a ...'` invocation wrapped through
|
||||
`wrap_for_root_namespace()` (`bwrap.h`) — **not** a plain `std::filesystem` check
|
||||
— because a rootless `containers-storage mount`'s content isn't visible to this
|
||||
process at all without `nsenter`, the same constraint `run_bwrap()` itself works
|
||||
around (see the root-vs-rootless paragraph below). `cp -a
|
||||
--preserve=mode,ownership,timestamps,links[,xattr]` does the copy; whether
|
||||
`,xattr` is included is decided by a direct `setxattr()`/`removexattr()` probe on
|
||||
the host directory (no new library dependency — Linux POSIX ACLs are themselves
|
||||
stored as xattrs, so this one probe stands in for both, logging a single
|
||||
`spdlog::warn` if unsupported). A nonzero `cp` exit is only ever a warning, never
|
||||
fatal — often just an ownership-preservation shortfall when not running as root.
|
||||
`create_directories()`'d if missing. If the resulting host directory is empty,
|
||||
`initialize_volume_directory()` reconciles it against the image's own directory
|
||||
at the given container path: if that image directory is non-empty, its contents
|
||||
are copied in first; then, **whether or not there was content to copy**, the
|
||||
host directory's own mode/ownership/timestamps (and xattrs/ACLs where
|
||||
supported) are always set to match the image directory's own — **real bug
|
||||
fixed by the user, not assumed**: an earlier version only ever copied when the
|
||||
image directory was non-empty, so an image declaring an *empty* directory with
|
||||
specific ownership/permissions (e.g. a data directory owned by a non-root
|
||||
uid/gid) got a host directory with default `create_directories()` permissions
|
||||
instead, and even the non-empty case never reconciled the directory's *own*
|
||||
attributes (only each copied entry's). The existence check, content copy, and
|
||||
attribute reconciliation all run as a single `sh -c` invocation wrapped through
|
||||
`wrap_for_root_namespace()` (`bwrap.h`) — **not** a plain `std::filesystem`
|
||||
check — because a rootless `containers-storage mount`'s content isn't visible
|
||||
to this process at all without `nsenter`, the same constraint `run_bwrap()`
|
||||
itself works around (see the root-vs-rootless paragraph below). `cp -a
|
||||
--preserve=mode,ownership,timestamps,links[,xattr] --attributes-only -T` does
|
||||
the attribute-reconciliation step; whether `,xattr` is included is decided by
|
||||
a direct `setxattr()`/`removexattr()` probe on the host directory (no new
|
||||
library dependency — Linux POSIX ACLs are themselves stored as xattrs, so this
|
||||
one probe stands in for both, logging a single `spdlog::warn` if unsupported).
|
||||
`-T`/`--no-target-directory` is required on that second `cp` — confirmed by
|
||||
direct testing: without it, since the host directory already exists, plain
|
||||
`cp SRC DST` copies `SRC` *into* `DST` as a nested `DST/basename(SRC)`
|
||||
subdirectory instead of reconciling `DST`'s own attributes, which is exactly
|
||||
the bug this fix closes. A nonzero `cp` exit is only ever a warning, never
|
||||
fatal — often just an ownership-preservation shortfall when not running as
|
||||
root. Verified end-to-end against `images/gitea.tar`'s real declared
|
||||
`/etc/gitea`/`/var/lib/gitea` volumes under a real rootless mount: the
|
||||
resulting host directories' mode/ownership matched the image's own declared
|
||||
values in both cases, and no mounts/layers were left behind afterward.
|
||||
|
||||
Errors are logged via `spdlog::error`; every external command is also traced at debug
|
||||
level in `run_process()`/`run_process_foreground()` (`src/process.cpp`) — visible via
|
||||
|
||||
Reference in New Issue
Block a user