diff --git a/pkg/machine/e2e/basic_test.go b/pkg/machine/e2e/basic_test.go index c060479cd9..20912a182d 100644 --- a/pkg/machine/e2e/basic_test.go +++ b/pkg/machine/e2e/basic_test.go @@ -96,6 +96,32 @@ var _ = Describe("run basic podman commands", func() { Expect(build).To(Exit(0)) }) + It("Single character volume mount", func() { + // Get a tmp directory + tDir, err := filepath.Abs(GinkgoT().TempDir()) + Expect(err).ToNot(HaveOccurred()) + name := randomString() + i := new(initMachine).withImage(mb.imagePath).withNow() + + // All other platforms have an implicit mount for the temp area + if isVmtype(define.QemuVirt) { + i.withVolume(tDir) + } + session, err := mb.setName(name).setCmd(i).run() + Expect(err).ToNot(HaveOccurred()) + Expect(session).To(Exit(0)) + + bm := basicMachine{} + + volumeCreate, err := mb.setCmd(bm.withPodmanCommand([]string{"volume", "create", "a"})).run() + Expect(err).ToNot(HaveOccurred()) + Expect(volumeCreate).To(Exit(0)) + + run, err := mb.setCmd(bm.withPodmanCommand([]string{"run", "-v", "a:/test:Z", "quay.io/libpod/alpine_nginx"})).run() + Expect(err).ToNot(HaveOccurred()) + Expect(run).To(Exit(0)) + }) + It("Volume should be virtiofs", func() { // In theory this could run on MacOS too, but we know virtiofs works for that now, // this is just testing linux diff --git a/pkg/specgen/volumes.go b/pkg/specgen/volumes.go index d2c1e54876..f57067c304 100644 --- a/pkg/specgen/volumes.go +++ b/pkg/specgen/volumes.go @@ -203,8 +203,9 @@ func GenVolumeMounts(volumeFlag []string) (map[string]spec.Mount, map[string]*Na return mounts, volumes, overlayVolumes, nil } -// Splits a volume string, accounting for Win drive paths +// SplitVolumeString Splits a volume string, accounting for Win drive paths // when running as a WSL linux guest or Windows client +// Format: [[SOURCE-VOLUME|HOST-DIR:]CONTAINER-DIR[:OPTIONS]] func SplitVolumeString(vol string) []string { parts := strings.Split(vol, ":") if !shouldResolveWinPaths() { @@ -217,6 +218,20 @@ func SplitVolumeString(vol string) []string { n = 4 } + // Determine if the last part is an absolute path (if true, it means we don't have any options such as ro, rw etc.) + lastPartIsPath := strings.HasPrefix(parts[len(parts)-1], "/") + + // Case: Volume or relative host path (e.g., "vol-name:/container" or "./hello:/container") + if lastPartIsPath && len(parts) == 2 { + return parts + } + + // Case: Volume or relative host path with options (e.g., "vol-name:/container:ro" or "./hello:/container:ro") + if !lastPartIsPath && len(parts) == 3 { + return parts + } + + // Case: Windows absolute path (e.g., "C:/Users:/mnt:ro") if hasWinDriveScheme(vol, n) { first := parts[0] + ":" + parts[1] parts = parts[1:] diff --git a/pkg/specgen/volumes_linux_test.go b/pkg/specgen/volumes_linux_test.go new file mode 100644 index 0000000000..65c5ec9ef0 --- /dev/null +++ b/pkg/specgen/volumes_linux_test.go @@ -0,0 +1,72 @@ +package specgen + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSplitVolumeString(t *testing.T) { + tests := []struct { + name string + volume string + expect []string + }{ + // relative host paths + { + name: "relative host path", + volume: "./hello:/container", + expect: []string{"./hello", "/container"}, + }, + { + name: "relative host path with options", + volume: "./hello:/container:ro", + expect: []string{"./hello", "/container", "ro"}, + }, + // absolute host path + { + name: "absolute host path", + volume: "/hello:/container", + expect: []string{"/hello", "/container"}, + }, + { + name: "absolute host path with option", + volume: "/hello:/container:ro", + expect: []string{"/hello", "/container", "ro"}, + }, + { + name: "absolute host path with option", + volume: "/hello:/container:ro", + expect: []string{"/hello", "/container", "ro"}, + }, + // volume source + { + name: "volume without option", + volume: "vol-name:/container", + expect: []string{"vol-name", "/container"}, + }, + { + name: "volume with option", + volume: "vol-name:/container:ro", + expect: []string{"vol-name", "/container", "ro"}, + }, + { + name: "single letter volume without option", + volume: "a:/container", + expect: []string{"a", "/container"}, + }, + { + name: "single letter volume with option", + volume: "a:/container:ro", + expect: []string{"a", "/container", "ro"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + parts := SplitVolumeString(tt.volume) + + assert.Equal(t, tt.expect, parts, tt.name) + }) + } +} diff --git a/pkg/specgen/volumes_windows_test.go b/pkg/specgen/volumes_windows_test.go new file mode 100644 index 0000000000..e59aae29b4 --- /dev/null +++ b/pkg/specgen/volumes_windows_test.go @@ -0,0 +1,77 @@ +package specgen + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestSplitVolumeString(t *testing.T) { + tests := []struct { + name string + volume string + expect []string + }{ + // relative host paths + { + name: "relative host path", + volume: "./hello:/container", + expect: []string{"./hello", "/container"}, + }, + { + name: "relative host path with options", + volume: "./hello:/container:ro", + expect: []string{"./hello", "/container", "ro"}, + }, + // absolute host path + { + name: "absolute host path", + volume: "C:\\hello:/container", + expect: []string{"C:\\hello", "/container"}, + }, + { + name: "absolute host path with option", + volume: "C:\\hello:/container:ro", + expect: []string{"C:\\hello", "/container", "ro"}, + }, + { + name: "absolute host path with option", + volume: "C:\\hello:/container:ro", + expect: []string{"C:\\hello", "/container", "ro"}, + }, + { + name: "absolute extended host path", + volume: `\\?\C:\hello:/container`, + expect: []string{`\\?\C:\hello`, "/container"}, + }, + // volume source + { + name: "volume without option", + volume: "vol-name:/container", + expect: []string{"vol-name", "/container"}, + }, + { + name: "volume with option", + volume: "vol-name:/container:ro", + expect: []string{"vol-name", "/container", "ro"}, + }, + { + name: "single letter volume without option", + volume: "a:/container", + expect: []string{"a", "/container"}, + }, + { + name: "single letter volume with option", + volume: "a:/container:ro", + expect: []string{"a", "/container", "ro"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + parts := SplitVolumeString(tt.volume) + + assert.Equal(t, tt.expect, parts, tt.name) + }) + } +} diff --git a/test/e2e/run_volume_test.go b/test/e2e/run_volume_test.go index b14ca83cc4..7742e655ba 100644 --- a/test/e2e/run_volume_test.go +++ b/test/e2e/run_volume_test.go @@ -145,6 +145,19 @@ var _ = Describe("Podman run with volumes", func() { Expect(session).To(ExitWithError(125, fmt.Sprintf("%s: duplicate mount destination", dest))) }) + It("podman run with single character volume", func() { + // 1. create single character volume + session := podmanTest.Podman([]string{"volume", "create", "a"}) + session.WaitWithDefaultTimeout() + volName := session.OutputToString() + Expect(session).Should(ExitCleanly()) + + // 2. create container with volume + session = podmanTest.Podman([]string{"run", "--volume", volName + ":/data", ALPINE, "sh", "-c", "echo hello world"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + }) + It("podman run with conflict between image volume and user mount succeeds", func() { err = podmanTest.RestoreArtifact(REDIS_IMAGE) Expect(err).ToNot(HaveOccurred())