diff --git a/volume/mounts/linux_parser.go b/volume/mounts/linux_parser.go index 035a24a8d9..22ab2d62cf 100644 --- a/volume/mounts/linux_parser.go +++ b/volume/mounts/linux_parser.go @@ -82,7 +82,10 @@ func (p *linuxParser) validateMountConfigImpl(mnt *mount.Mount, validateBindSour } if validateBindSourceExists { - exists, _, _ := currentFileInfoProvider.fileInfo(mnt.Source) + exists, _, err := currentFileInfoProvider.fileInfo(mnt.Source) + if err != nil { + return &errMountConfig{mnt, err} + } if !exists { return &errMountConfig{mnt, errBindSourceDoesNotExist(mnt.Source)} } diff --git a/volume/mounts/parser_test.go b/volume/mounts/parser_test.go index 27257d62bd..f9b32e5ccf 100644 --- a/volume/mounts/parser_test.go +++ b/volume/mounts/parser_test.go @@ -1,6 +1,7 @@ package mounts // import "github.com/docker/docker/volume/mounts" import ( + "errors" "io/ioutil" "os" "runtime" @@ -8,6 +9,8 @@ import ( "testing" "github.com/docker/docker/api/types/mount" + "gotest.tools/assert" + "gotest.tools/assert/cmp" ) type parseMountRawTestSet struct { @@ -477,4 +480,51 @@ func TestParseMountSpec(t *testing.T) { t.Errorf("Expected mount copy data to match. Expected: '%v', Actual: '%v'", c.expected.CopyData, mp.CopyData) } } + +} + +// always returns the configured error +// this is used to test error handling +type mockFiProviderWithError struct{ err error } + +func (m mockFiProviderWithError) fileInfo(path string) (bool, bool, error) { + return false, false, m.err +} + +// TestParseMountSpecBindWithFileinfoError makes sure that the parser returns +// the error produced by the fileinfo provider. +// +// Some extra context for the future in case of changes and possible wtf are we +// testing this for: +// +// Currently this "fileInfoProvider" returns (bool, bool, error) +// The 1st bool is "does this path exist" +// The 2nd bool is "is this path a dir" +// Then of course the error is an error. +// +// The issue is the parser was ignoring the error and only looking at the +// "does this path exist" boolean, which is always false if there is an error. +// Then the error returned to the caller was a (slightly, maybe) friendlier +// error string than what comes from `os.Stat` +// So ...the caller was always getting an error saying the path doesn't exist +// even if it does exist but got some other error (like a permission error). +// This is confusing to users. +func TestParseMountSpecBindWithFileinfoError(t *testing.T) { + previousProvider := currentFileInfoProvider + defer func() { currentFileInfoProvider = previousProvider }() + + testErr := errors.New("some crazy error") + currentFileInfoProvider = &mockFiProviderWithError{err: testErr} + + p := "/bananas" + if runtime.GOOS == "windows" { + p = `c:\bananas` + } + m := mount.Mount{Type: mount.TypeBind, Source: p, Target: p} + + parser := NewParser(runtime.GOOS) + + _, err := parser.ParseMountSpec(m) + assert.Assert(t, err != nil) + assert.Assert(t, cmp.Contains(err.Error(), "some crazy error")) }