Merge pull request #34667 from dnephin/add-container-config-decode-tests

Add container config decode tests

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...)
 }

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

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},
+		},
+	}
+}

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)
+}