Skip to content

Commit

Permalink
fix: mounting issue with single character volume on windows
Browse files Browse the repository at this point in the history
Signed-off-by: axel7083 <[email protected]>
  • Loading branch information
axel7083 committed Feb 14, 2025
1 parent 3b6c766 commit a593ab2
Show file tree
Hide file tree
Showing 4 changed files with 178 additions and 1 deletion.
17 changes: 16 additions & 1 deletion pkg/specgen/volumes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -217,6 +218,20 @@ func SplitVolumeString(vol string) []string {
n = 4
}

// Determine if the last part is an option (e.g., "ro", "z")
hasOption := !strings.HasPrefix(parts[len(parts)-1], "/")

// Case: Volume or relative host path (e.g., "vol-name:/container" or "./hello:/container")
if !hasOption && len(parts) == 2 {
return parts
}

// Case: Volume or relative host path with options (e.g., "vol-name:/container:ro" or "./hello:/container:ro")
if hasOption && 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:]
Expand Down
72 changes: 72 additions & 0 deletions pkg/specgen/volumes_linux_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
77 changes: 77 additions & 0 deletions pkg/specgen/volumes_windows_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
}
}
13 changes: 13 additions & 0 deletions test/e2e/run_volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down

0 comments on commit a593ab2

Please sign in to comment.