diff --git a/components/engine/internal/testutil/helpers.go b/components/engine/internal/testutil/helpers.go index adc76418e8..a76056924e 100644 --- a/components/engine/internal/testutil/helpers.go +++ b/components/engine/internal/testutil/helpers.go @@ -7,7 +7,7 @@ import ( // ErrorContains checks that the error is not nil, and contains the expected // substring. -func ErrorContains(t require.TestingT, err error, expectedError string) { - require.Error(t, err) - assert.Contains(t, err.Error(), expectedError) +func ErrorContains(t require.TestingT, err error, expectedError string, msgAndArgs ...interface{}) { + require.Error(t, err, msgAndArgs...) + assert.Contains(t, err.Error(), expectedError, msgAndArgs...) } diff --git a/components/engine/runconfig/config.go b/components/engine/runconfig/config.go index ebb6d3f6b9..3d236deb53 100644 --- a/components/engine/runconfig/config.go +++ b/components/engine/runconfig/config.go @@ -17,20 +17,12 @@ type ContainerDecoder struct{} // DecodeConfig makes ContainerDecoder to implement httputils.ContainerDecoder func (r ContainerDecoder) DecodeConfig(src io.Reader) (*container.Config, *container.HostConfig, *networktypes.NetworkingConfig, error) { - c, hc, nc, err := decodeContainerConfig(src) - if err != nil { - return nil, nil, nil, err - } - return c, hc, nc, nil + return decodeContainerConfig(src) } // DecodeHostConfig makes ContainerDecoder to implement httputils.ContainerDecoder func (r ContainerDecoder) DecodeHostConfig(src io.Reader) (*container.HostConfig, error) { - hc, err := decodeHostConfig(src) - if err != nil { - return nil, err - } - return hc, nil + return decodeHostConfig(src) } // decodeContainerConfig decodes a json encoded config into a ContainerConfigWrapper diff --git a/components/engine/runconfig/config_test.go b/components/engine/runconfig/config_test.go index 231b4509bc..ebd74ea31c 100644 --- a/components/engine/runconfig/config_test.go +++ b/components/engine/runconfig/config_test.go @@ -9,9 +9,14 @@ import ( "strings" "testing" + "os" + "github.com/docker/docker/api/types/container" networktypes "github.com/docker/docker/api/types/network" "github.com/docker/docker/api/types/strslice" + "github.com/gotestyourself/gotestyourself/skip" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type f struct { @@ -137,3 +142,237 @@ func callDecodeContainerConfigIsolation(isolation string) (*container.Config, *c } return decodeContainerConfig(bytes.NewReader(b)) } + +func TestDecodeContainerConfigWithVolumes(t *testing.T) { + var testcases = []decodeConfigTestcase{ + { + doc: "no paths volume", + wrapper: containerWrapperWithVolume(":"), + expectedErr: `invalid volume specification: ':'`, + }, + { + doc: "no paths bind", + wrapper: containerWrapperWithBind(":"), + expectedErr: `invalid volume specification: ':'`, + }, + { + doc: "no paths or mode volume", + wrapper: containerWrapperWithVolume("::"), + expectedErr: `invalid volume specification: '::'`, + }, + { + doc: "no paths or mode bind", + wrapper: containerWrapperWithBind("::"), + expectedErr: `invalid volume specification: '::'`, + }, + } + for _, testcase := range testcases { + t.Run(testcase.doc, runDecodeContainerConfigTestCase(testcase)) + } +} + +func TestDecodeContainerConfigWithVolumesUnix(t *testing.T) { + skip.IfCondition(t, runtime.GOOS == "windows") + + baseErr := `invalid mount config for type "volume": invalid specification: ` + var testcases = []decodeConfigTestcase{ + { + doc: "root to root volume", + wrapper: containerWrapperWithVolume("/:/"), + expectedErr: `invalid volume specification: '/:/'`, + }, + { + doc: "root to root bind", + wrapper: containerWrapperWithBind("/:/"), + expectedErr: `invalid volume specification: '/:/'`, + }, + { + doc: "no destination path volume", + wrapper: containerWrapperWithVolume(`/tmp:`), + expectedErr: ` invalid volume specification: '/tmp:'`, + }, + { + doc: "no destination path bind", + wrapper: containerWrapperWithBind(`/tmp:`), + expectedErr: ` invalid volume specification: '/tmp:'`, + }, + { + doc: "no destination path or mode volume", + wrapper: containerWrapperWithVolume(`/tmp::`), + expectedErr: `invalid mount config for type "bind": field Target must not be empty`, + }, + { + doc: "no destination path or mode bind", + wrapper: containerWrapperWithBind(`/tmp::`), + expectedErr: `invalid mount config for type "bind": field Target must not be empty`, + }, + { + doc: "too many sections volume", + wrapper: containerWrapperWithVolume(`/tmp:/tmp:/tmp:/tmp`), + expectedErr: `invalid volume specification: '/tmp:/tmp:/tmp:/tmp'`, + }, + { + doc: "too many sections bind", + wrapper: containerWrapperWithBind(`/tmp:/tmp:/tmp:/tmp`), + expectedErr: `invalid volume specification: '/tmp:/tmp:/tmp:/tmp'`, + }, + { + doc: "just root volume", + wrapper: containerWrapperWithVolume("/"), + expectedErr: baseErr + `destination can't be '/'`, + }, + { + doc: "just root bind", + wrapper: containerWrapperWithBind("/"), + expectedErr: baseErr + `destination can't be '/'`, + }, + { + doc: "bind mount passed as a volume", + wrapper: containerWrapperWithVolume(`/foo:/bar`), + expectedConfig: &container.Config{ + Volumes: map[string]struct{}{`/foo:/bar`: {}}, + }, + expectedHostConfig: &container.HostConfig{NetworkMode: "default"}, + }, + } + for _, testcase := range testcases { + t.Run(testcase.doc, runDecodeContainerConfigTestCase(testcase)) + } +} + +type decodeConfigTestcase struct { + doc string + wrapper ContainerConfigWrapper + expectedErr string + expectedConfig *container.Config + expectedHostConfig *container.HostConfig + goos string +} + +func runDecodeContainerConfigTestCase(testcase decodeConfigTestcase) func(t *testing.T) { + return func(t *testing.T) { + raw := marshal(t, testcase.wrapper, testcase.doc) + config, hostConfig, _, err := decodeContainerConfig(bytes.NewReader(raw)) + if testcase.expectedErr != "" { + if !assert.Error(t, err) { + return + } + assert.Contains(t, err.Error(), testcase.expectedErr) + return + } + assert.NoError(t, err) + assert.Equal(t, testcase.expectedConfig, config) + assert.Equal(t, testcase.expectedHostConfig, hostConfig) + } +} + +func TestDecodeContainerConfigWithVolumesWindows(t *testing.T) { + skip.IfCondition(t, runtime.GOOS != "windows") + + tmpDir := os.Getenv("TEMP") + systemDrive := os.Getenv("SystemDrive") + var testcases = []decodeConfigTestcase{ + { + doc: "root to root volume", + wrapper: containerWrapperWithVolume(systemDrive + `\:c:\`), + expectedErr: `invalid volume specification: `, + }, + { + doc: "root to root bind", + wrapper: containerWrapperWithBind(systemDrive + `\:c:\`), + expectedErr: `invalid volume specification: `, + }, + { + doc: "no destination path volume", + wrapper: containerWrapperWithVolume(tmpDir + `\:`), + expectedErr: `invalid volume specification: `, + }, + { + doc: "no destination path bind", + wrapper: containerWrapperWithBind(tmpDir + `\:`), + expectedErr: `invalid volume specification: `, + }, + { + doc: "no destination path or mode volume", + wrapper: containerWrapperWithVolume(tmpDir + `\::`), + expectedErr: `invalid volume specification: `, + }, + { + doc: "no destination path or mode bind", + wrapper: containerWrapperWithBind(tmpDir + `\::`), + expectedErr: `invalid volume specification: `, + }, + { + doc: "too many sections volume", + wrapper: containerWrapperWithVolume(tmpDir + ":" + tmpDir + ":" + tmpDir + ":" + tmpDir), + expectedErr: `invalid volume specification: `, + }, + { + doc: "too many sections bind", + wrapper: containerWrapperWithBind(tmpDir + ":" + tmpDir + ":" + tmpDir + ":" + tmpDir), + expectedErr: `invalid volume specification: `, + }, + { + doc: "no drive letter volume", + wrapper: containerWrapperWithVolume(`\tmp`), + expectedErr: `invalid volume specification: `, + }, + { + doc: "no drive letter bind", + wrapper: containerWrapperWithBind(`\tmp`), + expectedErr: `invalid volume specification: `, + }, + { + doc: "root to c-drive volume", + wrapper: containerWrapperWithVolume(systemDrive + `\:c:`), + expectedErr: `invalid volume specification: `, + }, + { + doc: "root to c-drive bind", + wrapper: containerWrapperWithBind(systemDrive + `\:c:`), + expectedErr: `invalid volume specification: `, + }, + { + doc: "container path without driver letter volume", + wrapper: containerWrapperWithVolume(`c:\windows:\somewhere`), + expectedErr: `invalid volume specification: `, + }, + { + doc: "container path without driver letter bind", + wrapper: containerWrapperWithBind(`c:\windows:\somewhere`), + expectedErr: `invalid volume specification: `, + }, + } + + for _, testcase := range testcases { + t.Run(testcase.doc, runDecodeContainerConfigTestCase(testcase)) + } +} + +func marshal(t *testing.T, w ContainerConfigWrapper, doc string) []byte { + b, err := json.Marshal(w) + require.NoError(t, err, "%s: failed to encode config wrapper", doc) + return b +} + +func containerWrapperWithVolume(volume string) ContainerConfigWrapper { + return ContainerConfigWrapper{ + Config: &container.Config{ + Volumes: map[string]struct{}{ + volume: {}, + }, + }, + HostConfig: &container.HostConfig{}, + } +} + +func containerWrapperWithBind(bind string) ContainerConfigWrapper { + return ContainerConfigWrapper{ + Config: &container.Config{ + Volumes: map[string]struct{}{}, + }, + HostConfig: &container.HostConfig{ + Binds: []string{bind}, + }, + } +} diff --git a/components/engine/vendor/github.com/gotestyourself/gotestyourself/skip/skip.go b/components/engine/vendor/github.com/gotestyourself/gotestyourself/skip/skip.go new file mode 100644 index 0000000000..d92110b0ef --- /dev/null +++ b/components/engine/vendor/github.com/gotestyourself/gotestyourself/skip/skip.go @@ -0,0 +1,133 @@ +/*Package skip provides functions for skipping based on a condition. + */ +package skip + +import ( + "bytes" + "fmt" + "go/ast" + "go/format" + "go/parser" + "go/token" + "io/ioutil" + "path" + "reflect" + "runtime" + "strings" + + "github.com/pkg/errors" +) + +type skipT interface { + Skip(args ...interface{}) + Log(args ...interface{}) +} + +// If skips the test if the check function returns true. The skip message will +// contain the name of the check function. Extra message text can be passed as a +// format string with args +func If(t skipT, check func() bool, msgAndArgs ...interface{}) { + if check() { + t.Skip(formatWithCustomMessage( + getFunctionName(check), + formatMessage(msgAndArgs...))) + } +} + +func getFunctionName(function func() bool) string { + funcPath := runtime.FuncForPC(reflect.ValueOf(function).Pointer()).Name() + return strings.SplitN(path.Base(funcPath), ".", 2)[1] +} + +// IfCondition skips the test if the condition is true. The skip message will +// contain the source of the expression passed as the condition. Extra message +// text can be passed as a format string with args. +func IfCondition(t skipT, condition bool, msgAndArgs ...interface{}) { + if !condition { + return + } + source, err := getConditionSource() + if err != nil { + t.Log(err.Error()) + t.Skip(formatMessage(msgAndArgs...)) + } + t.Skip(formatWithCustomMessage(source, formatMessage(msgAndArgs...))) +} + +func getConditionSource() (string, error) { + const callstackIndex = 3 + lines, err := getSourceLine(callstackIndex) + if err != nil { + return "", err + } + + for i := range lines { + source := strings.Join(lines[len(lines)-i-1:], "\n") + node, err := parser.ParseExpr(source) + if err == nil { + return getConditionArgFromAST(node) + } + } + return "", errors.Wrapf(err, "failed to parse source") +} + +// maxContextLines is the maximum number of lines to scan for a complete +// skip.If() statement +const maxContextLines = 10 + +// getSourceLines returns the source line which called skip.If() along with a +// few preceding lines. To properly parse the AST a complete statement is +// required, and that statement may be split across multiple lines, so include +// up to maxContextLines. +func getSourceLine(stackIndex int) ([]string, error) { + _, filename, line, ok := runtime.Caller(stackIndex) + if !ok { + return nil, errors.New("failed to get caller info") + } + + raw, err := ioutil.ReadFile(filename) + if err != nil { + return nil, errors.Wrapf(err, "failed to read source file: %s", filename) + } + + lines := strings.Split(string(raw), "\n") + if len(lines) < line { + return nil, errors.Errorf("file %s does not have line %d", filename, line) + } + firstLine := line - maxContextLines + if firstLine < 0 { + firstLine = 0 + } + return lines[firstLine:line], nil +} + +func getConditionArgFromAST(node ast.Expr) (string, error) { + switch expr := node.(type) { + case *ast.CallExpr: + buf := new(bytes.Buffer) + err := format.Node(buf, token.NewFileSet(), expr.Args[1]) + return buf.String(), err + } + return "", errors.New("unexpected ast") +} + +func formatMessage(msgAndArgs ...interface{}) string { + switch len(msgAndArgs) { + case 0: + return "" + case 1: + return msgAndArgs[0].(string) + default: + return fmt.Sprintf(msgAndArgs[0].(string), msgAndArgs[1:]...) + } +} + +func formatWithCustomMessage(source, custom string) string { + switch { + case custom == "": + return source + case source == "": + return custom + } + return fmt.Sprintf("%s: %s", source, custom) +}