mirror of
https://github.com/moby/moby.git
synced 2022-11-09 12:21:53 -05:00
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>
This commit is contained in:
parent
12b837e474
commit
ebcef28834
2 changed files with 54 additions and 1 deletions
|
@ -82,7 +82,10 @@ func (p *linuxParser) validateMountConfigImpl(mnt *mount.Mount, validateBindSour
|
||||||
}
|
}
|
||||||
|
|
||||||
if validateBindSourceExists {
|
if validateBindSourceExists {
|
||||||
exists, _, _ := currentFileInfoProvider.fileInfo(mnt.Source)
|
exists, _, err := currentFileInfoProvider.fileInfo(mnt.Source)
|
||||||
|
if err != nil {
|
||||||
|
return &errMountConfig{mnt, err}
|
||||||
|
}
|
||||||
if !exists {
|
if !exists {
|
||||||
return &errMountConfig{mnt, errBindSourceDoesNotExist(mnt.Source)}
|
return &errMountConfig{mnt, errBindSourceDoesNotExist(mnt.Source)}
|
||||||
}
|
}
|
||||||
|
|
|
@ -1,6 +1,7 @@
|
||||||
package mounts // import "github.com/docker/docker/volume/mounts"
|
package mounts // import "github.com/docker/docker/volume/mounts"
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"errors"
|
||||||
"io/ioutil"
|
"io/ioutil"
|
||||||
"os"
|
"os"
|
||||||
"runtime"
|
"runtime"
|
||||||
|
@ -8,6 +9,8 @@ import (
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/docker/docker/api/types/mount"
|
"github.com/docker/docker/api/types/mount"
|
||||||
|
"gotest.tools/assert"
|
||||||
|
"gotest.tools/assert/cmp"
|
||||||
)
|
)
|
||||||
|
|
||||||
type parseMountRawTestSet struct {
|
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)
|
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"))
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue