From 9e78185e372b170922dfdad80cede6b351750bb0 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 27 Oct 2021 17:30:37 +0200 Subject: [PATCH] volumes: be more tolerant and fix infinite loop Make Podman more tolerant when parsing image volumes during container creation and further fix an infinite loop when checking them. Consider `VOLUME ['/etc/foo', '/etc/bar']` in a Containerfile. While it looks correct to the human eye, the single quotes are wrong and yield the two volumes to be `[/etc/foo,` and `/etc/bar]` in Podman and Docker. When running the container, it'll create a directory `bar]` in `/etc` and a directory `[` in `/` with two subdirectories `etc/foo,`. This behavior is surprising to me but how Docker behaves. We may improve on that in the future. Note that the correct way to syntax for volumes in a Containerfile is `VOLUME /A /B /C` or `VOLUME ["/A", "/B", "/C"]`; single quotes are not supported. This change restores this behavior without breaking container creation or ending up in an infinite loop. BZ: https://bugzilla.redhat.com/show_bug.cgi?id=2014149 Signed-off-by: Valentin Rothberg --- libpod/container_path_resolution.go | 2 +- pkg/specgen/generate/storage.go | 3 --- test/system/070-build.bats | 9 +++++++++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/libpod/container_path_resolution.go b/libpod/container_path_resolution.go index bb2ef1a73f..7db23b7833 100644 --- a/libpod/container_path_resolution.go +++ b/libpod/container_path_resolution.go @@ -161,7 +161,7 @@ func isPathOnBindMount(c *Container, containerPath string) bool { if cleanedContainerPath == filepath.Clean(m.Destination) { return true } - for dest := m.Destination; dest != "/"; dest = filepath.Dir(dest) { + for dest := m.Destination; dest != "/" && dest != "."; dest = filepath.Dir(dest) { if cleanedContainerPath == dest { return true } diff --git a/pkg/specgen/generate/storage.go b/pkg/specgen/generate/storage.go index de655ad7d7..0218dce6a3 100644 --- a/pkg/specgen/generate/storage.go +++ b/pkg/specgen/generate/storage.go @@ -214,9 +214,6 @@ func getImageVolumes(ctx context.Context, img *libimage.Image, s *specgen.SpecGe } for volume := range inspect.Config.Volumes { logrus.Debugf("Image has volume at %q", volume) - if err = parse.ValidateVolumeCtrDir(volume); err != nil { - return nil, nil, err - } cleanDest := filepath.Clean(volume) switch mode { case "", "anonymous": diff --git a/test/system/070-build.bats b/test/system/070-build.bats index 4e89e299a7..44387d4fdf 100644 --- a/test/system/070-build.bats +++ b/test/system/070-build.bats @@ -39,6 +39,7 @@ EOF cat >$dockerfile < /$rand_filename +VOLUME ['/etc/foo', '/etc/bar'] EOF run_podman buildx build --load -t build_test --format=docker $tmpdir @@ -47,6 +48,14 @@ EOF run_podman run --rm build_test cat /$rand_filename is "$output" "$rand_content" "reading generated file in image" + # Make sure the volumes are created at surprising yet Docker-compatible + # destinations (see bugzilla.redhat.com/show_bug.cgi?id=2014149). + run_podman run --rm build_test find /[ /etc/bar\] -print + is "$output" "/\[ +/\[/etc +/\[/etc/foo, +/etc/bar]" "weird VOLUME gets converted to directories with brackets and comma" + run_podman rmi -f build_test }