From e40d70cecceb46c7ad6ae8502a29f52867e15a7a Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Mon, 9 Oct 2023 14:08:00 +0200 Subject: [PATCH] new 'no-dereference' mount option Add a new `no-dereference` mount option supported by crun 1.11+ to re-create/copy a symlink if it's the source of a mount. By default the kernel will resolve the symlink on the host and mount the target. As reported in #20098, there are use cases where the symlink structure must be preserved by all means. Fixes: #20098 Fixes: issues.redhat.com/browse/RUN-1935 Signed-off-by: Valentin Rothberg --- docs/source/markdown/options/mount.md | 2 + go.mod | 2 +- go.sum | 4 +- libpod/container_internal_common.go | 6 +- pkg/specgenutil/volumes.go | 2 +- pkg/util/mountOpts.go | 7 +- test/system/060-mount.bats | 81 ++++++++++++++++++- .../containers/common/pkg/parse/parse.go | 7 +- .../containers/common/version/version.go | 2 +- vendor/modules.txt | 2 +- 10 files changed, 105 insertions(+), 10 deletions(-) diff --git a/docs/source/markdown/options/mount.md b/docs/source/markdown/options/mount.md index a44acbf695..9ab02cd97e 100644 --- a/docs/source/markdown/options/mount.md +++ b/docs/source/markdown/options/mount.md @@ -75,6 +75,8 @@ Current supported mount TYPEs are **bind**, **devpts**, **glob**, **image**, **r . U, chown: true or false (default). Change recursively the owner and group of the source volume based on the UID and GID of the container. + . no-dereference: do not dereference symlinks but copy the link source into the mount destination. + Options specific to tmpfs and ramfs: ยท ro, readonly: true or false (default). diff --git a/go.mod b/go.mod index a5c89b23b7..9c0bd374f9 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/containernetworking/cni v1.1.2 github.com/containernetworking/plugins v1.3.0 github.com/containers/buildah v1.33.1 - github.com/containers/common v0.57.0 + github.com/containers/common v0.57.1-0.20231121105603-d54dcfe962d6 github.com/containers/conmon v2.0.20+incompatible github.com/containers/gvisor-tap-vsock v0.7.1 github.com/containers/image/v5 v5.29.0 diff --git a/go.sum b/go.sum index e1dfb3c63f..d2679e2cfe 100644 --- a/go.sum +++ b/go.sum @@ -255,8 +255,8 @@ github.com/containernetworking/plugins v1.3.0 h1:QVNXMT6XloyMUoO2wUOqWTC1hWFV62Q github.com/containernetworking/plugins v1.3.0/go.mod h1:Pc2wcedTQQCVuROOOaLBPPxrEXqqXBFt3cZ+/yVg6l0= github.com/containers/buildah v1.33.1 h1:s+5LaZx+vkOV/BboM6QZbf0Uma/A9W/B1REoUiM3CQo= github.com/containers/buildah v1.33.1/go.mod h1:xEvekGaEeflDV4kxdKcTk0NbTuV4FsbPW4UYReLkHIw= -github.com/containers/common v0.57.0 h1:5O/+6QUBafKK0/zeok9y1rLPukfWgdE0sT4nuzmyAqk= -github.com/containers/common v0.57.0/go.mod h1:t/Z+/sFrapvFMEJe3YnecN49/Tae2wYEQShbEN6SRaU= +github.com/containers/common v0.57.1-0.20231121105603-d54dcfe962d6 h1:4IAcuB9ZYlMIX5rnap2ax9IdFtivbTvUIPh7WgUglvU= +github.com/containers/common v0.57.1-0.20231121105603-d54dcfe962d6/go.mod h1:gUsz+eYo0q1NMwiukwI8E6LAqyYd0DapZ71hKC+MbJw= github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg= github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I= github.com/containers/gvisor-tap-vsock v0.7.1 h1:+Rc+sOPplrkQb/BUXeN0ug8TxjgyrIqo/9P/eNS2A4c= diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index cd19089e2e..81b0fbf337 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -366,7 +366,11 @@ func (c *Container) generateSpec(ctx context.Context) (s *spec.Spec, cleanupFunc if err := c.relabel(m.Source, c.MountLabel(), label.IsShared(o)); err != nil { return nil, nil, err } - + case "no-dereference": + // crun calls the option `copy-symlink`. + // Podman decided for --no-dereference as many + // bin-utils tools (e..g, touch, chown, cp) do. + options = append(options, "copy-symlink") default: options = append(options, o) } diff --git a/pkg/specgenutil/volumes.go b/pkg/specgenutil/volumes.go index b9a85f18b4..4e663f6e73 100644 --- a/pkg/specgenutil/volumes.go +++ b/pkg/specgenutil/volumes.go @@ -355,7 +355,7 @@ func parseMountOptions(mountType string, args []string) (*spec.Mount, error) { default: return nil, fmt.Errorf("%s mount option must be 'private' or 'shared': %w", kv[0], util.ErrBadMntOption) } - case "shared", "rshared", "private", "rprivate", "slave", "rslave", "unbindable", "runbindable", "Z", "z": + case "shared", "rshared", "private", "rprivate", "slave", "rslave", "unbindable", "runbindable", "Z", "z", "no-dereference": mnt.Options = append(mnt.Options, kv[0]) case "src", "source": if mountType == define.TypeTmpfs { diff --git a/pkg/util/mountOpts.go b/pkg/util/mountOpts.go index a65dcfbba9..a35ac77f77 100644 --- a/pkg/util/mountOpts.go +++ b/pkg/util/mountOpts.go @@ -28,7 +28,7 @@ type defaultMountOptions struct { // The sourcePath variable, if not empty, contains a bind mount source. func ProcessOptions(options []string, isTmpfs bool, sourcePath string) ([]string, error) { var ( - foundWrite, foundSize, foundProp, foundMode, foundExec, foundSuid, foundDev, foundCopyUp, foundBind, foundZ, foundU, foundOverlay, foundIdmap, foundCopy, foundNoSwap bool + foundWrite, foundSize, foundProp, foundMode, foundExec, foundSuid, foundDev, foundCopyUp, foundBind, foundZ, foundU, foundOverlay, foundIdmap, foundCopy, foundNoSwap, foundNoDereference bool ) newOptions := make([]string, 0, len(options)) @@ -148,6 +148,11 @@ func ProcessOptions(options []string, isTmpfs bool, sourcePath string) ([]string foundNoSwap = true newOptions = append(newOptions, opt) continue + case "no-dereference": + if foundNoDereference { + return nil, fmt.Errorf("the 'no-dereference' option can only be set once: %w", ErrDupeMntOption) + } + foundNoDereference = true case define.TypeBind, "rbind": if isTmpfs { return nil, fmt.Errorf("the 'bind' and 'rbind' options are not allowed with tmpfs mounts: %w", ErrBadMntOption) diff --git a/test/system/060-mount.bats b/test/system/060-mount.bats index bd4eede000..3bb699421d 100644 --- a/test/system/060-mount.bats +++ b/test/system/060-mount.bats @@ -321,4 +321,83 @@ EOF fi } -# vim: filetype=sh +@test "podman mount no-dereference" { + # Test how bind and glob-mounts behave with respect to relative (rel) and + # absolute (abs) symlinks. + + if [ $(podman_runtime) != "crun" ]; then + # Requires crun >= 1.11.0 + skip "only crun supports the no-dereference (copy-symlink) mount option" + fi + + # One directory for testing relative symlinks, another for absolute ones. + rel_dir=$PODMAN_TMPDIR/rel-dir + abs_dir=$PODMAN_TMPDIR/abs-dir + mkdir $rel_dir $abs_dir + + # Create random values to discrimate data in the rel/abs directory and the + # one from the image. + rel_random_host="rel_on_the_host_$(random_string 15)" + abs_random_host="abs_on_the_host_$(random_string 15)" + random_img="on_the_image_$(random_string 15)" + + # Relative symlink + echo "$rel_random_host" > $rel_dir/data + ln -r -s $rel_dir/data $rel_dir/link + # Absolute symlink + echo "$abs_random_host" > $abs_dir/data + ln -s $abs_dir/data $abs_dir/link + + dockerfile=$PODMAN_TMPDIR/Dockerfile + cat >$dockerfile < /tmp/data +EOF + + img="localhost/preserve:symlinks" + run_podman build -t $img -f $dockerfile + + link_path="/tmp/link" + create_path="/tmp/i/do/not/exist/link" + + tests=" +0 | bind | $rel_dir/link | /tmp/link | | /tmp/link | $rel_random_host | $link_path | bind mount relative symlink: mounts target from the host +0 | bind | $abs_dir/link | /tmp/link | | /tmp/link | $abs_random_host | $link_path | bind mount absolute symlink: mounts target from the host +0 | glob | $rel_dir/lin* | /tmp/ | | /tmp/link | $rel_random_host | $link_path | glob mount relative symlink: mounts target from the host +0 | glob | $abs_dir/lin* | /tmp/ | | /tmp/link | $abs_random_host | $link_path | glob mount absolute symlink: mounts target from the host +0 | glob | $rel_dir/* | /tmp/ | | /tmp/link | $rel_random_host | $link_path | glob mount entire directory: mounts relative target from the host +0 | glob | $abs_dir/* | /tmp/ | | /tmp/link | $abs_random_host | $link_path | glob mount entire directory: mounts absolute target from the host +0 | bind | $rel_dir/link | /tmp/link | ,no-dereference | '/tmp/link' -> 'data' | $random_img | $link_path | no_deref: bind mount relative symlink: points to file on the image +0 | glob | $rel_dir/lin* | /tmp/ | ,no-dereference | '/tmp/link' -> 'data' | $random_img | $link_path | no_deref: glob mount relative symlink: points to file on the image +0 | bind | $rel_dir/ | /tmp/ | ,no-dereference | '/tmp/link' -> 'data' | $rel_random_host | $link_path | no_deref: bind mount the entire directory: preserves symlink automatically +0 | glob | $rel_dir/* | /tmp/ | ,no-dereference | '/tmp/link' -> 'data' | $rel_random_host | $link_path | no_deref: glob mount the entire directory: preserves symlink automatically +1 | bind | $abs_dir/link | /tmp/link | ,no-dereference | '/tmp/link' -> '$abs_dir/data' | cat: can't open '/tmp/link': No such file or directory | $link_path | bind mount *preserved* absolute symlink: now points to a non-existent file on the container +1 | glob | $abs_dir/lin* | /tmp/ | ,no-dereference | '/tmp/link' -> '$abs_dir/data' | cat: can't open '/tmp/link': No such file or directory | $link_path | glob mount *preserved* absolute symlink: now points to a non-existent file on the container +0 | bind | $rel_dir/link | $create_path | | $create_path | $rel_random_host | $create_path | bind mount relative symlink: creates dirs and mounts target from the host +1 | bind | $rel_dir/link | $create_path | ,no-dereference | '$create_path' -> 'data' | cat: can't open '$create_path': No such file or directory | $create_path | no_deref: bind mount relative symlink: creates dirs and mounts target from the host +" + + while read exit_code mount_type mount_src mount_dst mount_opts line_0 line_1 path description; do + if [[ $mount_opts == "''" ]];then + unset mount_opts + fi + run_podman $exit_code run \ + --mount type=$mount_type,src=$mount_src,dst=$mount_dst$mount_opts \ + --rm --privileged $img sh -c "stat -c '%N' $path; cat $path" + assert "${lines[0]}" = "$line_0" "$description" + assert "${lines[1]}" = "$line_1" "$description" + done < <(parse_table "$tests") + + # Make sure that it's presvered across starts and stops + run_podman create --mount type=glob,src=$rel_dir/*,dst=/tmp/,no-dereference --privileged $img sh -c "stat -c '%N' /tmp/link; cat /tmp/link" + cid="$output" + run_podman start -a $cid + assert "${lines[0]}" = "'/tmp/link' -> 'data'" "symlink is preserved" + assert "${lines[1]}" = "$rel_random_host" "glob macthes symlink and host 'data' file" + run_podman start -a $cid + assert "${lines[0]}" = "'/tmp/link' -> 'data'" "symlink is preserved" + assert "${lines[1]}" = "$rel_random_host" "glob macthes symlink and host 'data' file" + run_podman rm -f -t=0 $cid + + run_podman rmi -f $img +} diff --git a/vendor/github.com/containers/common/pkg/parse/parse.go b/vendor/github.com/containers/common/pkg/parse/parse.go index 7629f58421..284751e523 100644 --- a/vendor/github.com/containers/common/pkg/parse/parse.go +++ b/vendor/github.com/containers/common/pkg/parse/parse.go @@ -14,7 +14,7 @@ import ( // ValidateVolumeOpts validates a volume's options func ValidateVolumeOpts(options []string) ([]string, error) { - var foundRootPropagation, foundRWRO, foundLabelChange, bindType, foundExec, foundDev, foundSuid, foundChown, foundUpperDir, foundWorkDir, foundCopy int + var foundRootPropagation, foundRWRO, foundLabelChange, bindType, foundExec, foundDev, foundSuid, foundChown, foundUpperDir, foundWorkDir, foundCopy, foundCopySymlink int finalOpts := make([]string, 0, len(options)) for _, opt := range options { // support advanced options like upperdir=/path, workdir=/path @@ -93,6 +93,11 @@ func ValidateVolumeOpts(options []string) ([]string, error) { if foundCopy > 1 { return nil, fmt.Errorf("invalid options %q, can only specify 1 'copy' or 'nocopy' option", strings.Join(options, ", ")) } + case "no-dereference": + foundCopySymlink++ + if foundCopySymlink > 1 { + return nil, fmt.Errorf("invalid options %q, can only specify 1 'no-dereference' option", strings.Join(options, ", ")) + } default: return nil, fmt.Errorf("invalid option type %q", opt) } diff --git a/vendor/github.com/containers/common/version/version.go b/vendor/github.com/containers/common/version/version.go index 639a2d7206..8773d23d4b 100644 --- a/vendor/github.com/containers/common/version/version.go +++ b/vendor/github.com/containers/common/version/version.go @@ -1,4 +1,4 @@ package version // Version is the version of the build. -const Version = "0.57.0" +const Version = "0.57.1-dev" diff --git a/vendor/modules.txt b/vendor/modules.txt index 1087c98cb3..2bae766e5b 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -167,7 +167,7 @@ github.com/containers/buildah/pkg/sshagent github.com/containers/buildah/pkg/util github.com/containers/buildah/pkg/volumes github.com/containers/buildah/util -# github.com/containers/common v0.57.0 +# github.com/containers/common v0.57.1-0.20231121105603-d54dcfe962d6 ## explicit; go 1.18 github.com/containers/common/internal/attributedstring github.com/containers/common/libimage