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 <matthew.heon@pm.me>

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.

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)