From 2de22ebf0d3a0bb5f359b7b50cbb45062762e1e4 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Sun, 6 Aug 2023 15:55:34 -0400 Subject: [PATCH] Ensure volumes-from mounts override image volumes We do not allow volumes and mounts to be placed at the same location in the container, with create-time checks to ensure this does not happen. User-added conflicts cannot be resolved (if the user adds two separate mounts to, say, /myapp, we can't resolve that contradiction and error), but for many other volume sources, we can solve the contradiction ourselves via a priority hierarchy. Image volumes come first, and are overridden by the `--volumes-from` flag, which are overridden by user-added mounts, etc, etc. The problem here is that we were not properly handling volumes-from overriding image volumes. An inherited volume from --volumes-from would supercede an image volume, but an inherited mount would not. Solution is fortunately simple - just clear out the map entry for the other type when adding volumes-from volumes. Makes me wish for Rust sum types - conflict resolution would be a lot simpler if we could use a sum type for volumes and bind mounts and thus have a single map instead of two maps, one for each type. Fixes #19529 Signed-off-by: Matthew Heon --- pkg/specgen/generate/storage.go | 7 +++++++ test/e2e/run_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/pkg/specgen/generate/storage.go b/pkg/specgen/generate/storage.go index 445bc07c6f..faebb8befa 100644 --- a/pkg/specgen/generate/storage.go +++ b/pkg/specgen/generate/storage.go @@ -37,9 +37,16 @@ func finalizeMounts(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Ru // Supersede from --volumes-from. for dest, mount := range volFromMounts { baseMounts[dest] = mount + + // Necessary to ensure that mounts override image volumes + // Ref: https://github.com/containers/podman/issues/19529 + delete(baseVolumes, dest) } for dest, volume := range volFromVolumes { baseVolumes[dest] = volume + + // I don't think this can happen, but best to be safe. + delete(baseMounts, dest) } // Need to make map forms of specgen mounts/volumes. diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index e4eae794ad..4ebcaeef81 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -1168,6 +1168,34 @@ USER mail`, BB) Expect(session.OutputToString()).To(ContainSubstring("data")) }) + It("podman run --volumes-from flag mount conflicts with image volume", func() { + volPathOnHost := filepath.Join(podmanTest.TempDir, "myvol") + err := os.MkdirAll(volPathOnHost, 0755) + Expect(err).ToNot(HaveOccurred()) + + imgName := "testimg" + volPath := "/myvol/mypath" + dockerfile := fmt.Sprintf(`FROM %s +RUN mkdir -p %s +VOLUME %s`, ALPINE, volPath, volPath) + podmanTest.BuildImage(dockerfile, imgName, "false") + + ctr1 := "ctr1" + run1 := podmanTest.Podman([]string{"run", "-d", "-v", fmt.Sprintf("%s:%s:z", volPathOnHost, volPath), "--name", ctr1, ALPINE, "top"}) + run1.WaitWithDefaultTimeout() + Expect(run1).Should(Exit(0)) + + testFile := "testfile1" + ctr1Exec := podmanTest.Podman([]string{"exec", "-t", ctr1, "touch", fmt.Sprintf("%s/%s", volPath, testFile)}) + ctr1Exec.WaitWithDefaultTimeout() + Expect(ctr1Exec).Should(Exit(0)) + + run2 := podmanTest.Podman([]string{"run", "--volumes-from", ctr1, imgName, "ls", volPath}) + run2.WaitWithDefaultTimeout() + Expect(run2).Should(Exit(0)) + Expect(run2.OutputToString()).To(Equal(testFile)) + }) + It("podman run --volumes flag with multiple volumes", func() { vol1 := filepath.Join(podmanTest.TempDir, "vol-test1") err := os.MkdirAll(vol1, 0755)