8000 Fix error handling for bind mount spec parser. · moby/moby@ebcef28 · GitHub
[go: up one dir, main page]
Skip to content

Commit ebcef28

Browse files
cpuguy83cpuguy83
committed
Fix error handling for bind mount spec parser.
Errors were being ignored and always telling the user that the path doesn't exist even if it was some other problem, such as a permission error. Signed-off-by: Brian Goff <cpuguy83@gmail.com>
1 parent 12b837e commit ebcef28

File tree

2 files changed

+54
-1
lines changed

2 files changed

+54
-1
lines changed

‎volume/mounts/linux_parser.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,10 @@ func (p *linuxParser) validateMountConfigImpl(mnt *mount.Mount, validateBindSour
8282
}
8383

8484
if validateBindSourceExists {
85-
exists, _< 8000 /span>, _ := currentFileInfoProvider.fileInfo(mnt.Source)
85+
exists, _, err := currentFileInfoProvider.fileInfo(mnt.Source)
86+
if err != nil {
87+
return &errMountConfig{mnt, err}
88+
}
8689
if !exists {
8790
return &errMountConfig{mnt, errBindSourceDoesNotExist(mnt.Source)}
8891
}

‎volume/mounts/parser_test.go‎

Lines changed: 50 additions & 0 deletions
7E82
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
package mounts // import "github.com/docker/docker/volume/mounts"
22

33
import (
4+
"errors"
45
"io/ioutil"
56
"os"
67
"runtime"
78
"strings"
89
"testing"
910

1011
"github.com/docker/docker/api/types/mount"
12+
"gotest.tools/assert"
13+
"gotest.tools/assert/cmp"
1114
)
1215

1316
type parseMountRawTestSet struct {
@@ -477,4 +480,51 @@ func TestParseMountSpec(t *testing.T) {
477480
t.Errorf("Expected mount copy data to match. Expected: '%v', Actual: '%v'", c.expected.CopyData, mp.CopyData)
478481
}
479482
}
483+
484+
}
485+
486+
// always returns the configured error
487+
// this is used to test error handling
488+
type mockFiProviderWithError struct{ err error }
489+
490+
func (m mockFiProviderWithError) fileInfo(path string) (bool, bool, error) {
491+
return false, false, m.err
492+
}
493+
494+
// TestParseMountSpecBindWithFileinfoError makes sure that the parser returns
495+
// the error produced by the fileinfo provider.
496+
//
497+
// Some extra context for the future in case of changes and possible wtf are we
498+
// testing this for:
499+
//
500+
// Currently this "fileInfoProvider" returns (bool, bool, error)
501+
// The 1st bool is "does this path exist"
502+
// The 2nd bool is "is this path a dir"
503+
// Then of course the error is an error.
504+
//
505+
// The issue is the parser was ignoring the error and only looking at the
506+
// "does this path exist" boolean, which is always false if there is an error.
507+
// Then the error returned to the caller was a (slightly, maybe) friendlier
508+
// error string than what comes from `os.Stat`
509+
// So ...the caller was always getting an error saying the path doesn't exist
510+
// even if it does exist but got some other error (like a permission error).
511+
// This is confusing to users.
512+
func TestParseMountSpecBindWithFileinfoError(t *testing.T) {
513+
previousProvider := currentFileInfoProvider
514+
defer func() { currentFileInfoProvider = previousProvider }()
515+
516+
testErr := errors.New("some crazy error")
517+
currentFileInfoProvider = &mockFiProviderWithError{err: testErr}
518+
519+
p := "/bananas"
520+
if runtime.GOOS == "windows" {
521+
p = `c:\bananas`
522+
}
523+
m := mount.Mount{Type: mount.TypeBind, Source: p, Target: p}
524+
525+
parser := NewParser(runtime.GOOS)
526+
527+
_, err := parser.ParseMountSpec(m)
528+
assert.Assert(t, err != nil)
529+
assert.Assert(t, cmp.Contains(err.Error(), "some crazy error"))
480530
}

0 commit comments

Comments
 (0)
0