@@ -1,21 +1,19 @@

 package convert

 import (

-	"fmt"

-	"strings"

-

 	"github.com/docker/docker/api/types/mount"

 	composetypes "github.com/docker/docker/cli/compose/types"

+	"github.com/pkg/errors"

 )

 type volumes map[string]composetypes.VolumeConfig

 // Volumes from compose-file types to engine api types

-func Volumes(serviceVolumes []string, stackVolumes volumes, namespace Namespace) ([]mount.Mount, error) {

+func Volumes(serviceVolumes []composetypes.ServiceVolumeConfig, stackVolumes volumes, namespace Namespace) ([]mount.Mount, error) {

 	var mounts []mount.Mount

-	for _, volumeSpec := range serviceVolumes {

-		mount, err := convertVolumeToMount(volumeSpec, stackVolumes, namespace)

+	for _, volumeConfig := range serviceVolumes {

+		mount, err := convertVolumeToMount(volumeConfig, stackVolumes, namespace)

 		if err != nil {

 			return nil, err

 		}

@@ -24,108 +22,65 @@ func Volumes(serviceVolumes []string, stackVolumes volumes, namespace Namespace)

 	return mounts, nil

 }

-func convertVolumeToMount(volumeSpec string, stackVolumes volumes, namespace Namespace) (mount.Mount, error) {

-	var source, target string

-	var mode []string

-

-	// TODO: split Windows path mappings properly

-	parts := strings.SplitN(volumeSpec, ":", 3)

-

-	for _, part := range parts {

-		if strings.TrimSpace(part) == "" {

-			return mount.Mount{}, fmt.Errorf("invalid volume: %s", volumeSpec)

-		}

+func convertVolumeToMount(

+	volume composetypes.ServiceVolumeConfig,

+	stackVolumes volumes,

+	namespace Namespace,

+) (mount.Mount, error) {

+	result := mount.Mount{

+		Type:     mount.Type(volume.Type),

+		Source:   volume.Source,

+		Target:   volume.Target,

+		ReadOnly: volume.ReadOnly,

 	}

-	switch len(parts) {

-	case 3:

-		source = parts[0]

-		target = parts[1]

-		mode = strings.Split(parts[2], ",")

-	case 2:

-		source = parts[0]

-		target = parts[1]

-	case 1:

-		target = parts[0]

+	// Anonymous volumes

+	if volume.Source == "" {

+		return result, nil

 	}

-

-	if source == "" {

-		// Anonymous volume

-		return mount.Mount{

-			Type:   mount.TypeVolume,

-			Target: target,

-		}, nil

+	if volume.Type == "volume" && volume.Bind != nil {

+		return result, errors.New("bind options are incompatible with type volume")

+	}

+	if volume.Type == "bind" && volume.Volume != nil {

+		return result, errors.New("volume options are incompatible with type bind")

 	}

-	// TODO: catch Windows paths here

-	if strings.HasPrefix(source, "/") {

-		return mount.Mount{

-			Type:        mount.TypeBind,

-			Source:      source,

-			Target:      target,

-			ReadOnly:    isReadOnly(mode),

-			BindOptions: getBindOptions(mode),

-		}, nil

+	if volume.Bind != nil {

+		result.BindOptions = &mount.BindOptions{

+			Propagation: mount.Propagation(volume.Bind.Propagation),

+		}

+	}

+	// Binds volumes

+	if volume.Type == "bind" {

+		return result, nil

 	}

-	stackVolume, exists := stackVolumes[source]

+	stackVolume, exists := stackVolumes[volume.Source]

 	if !exists {

-		return mount.Mount{}, fmt.Errorf("undefined volume: %s", source)

+		return result, errors.Errorf("undefined volume: %s", volume.Source)

 	}

-	var volumeOptions *mount.VolumeOptions

-	if stackVolume.External.Name != "" {

-		volumeOptions = &mount.VolumeOptions{

-			NoCopy: isNoCopy(mode),

-		}

-		source = stackVolume.External.Name

-	} else {

-		volumeOptions = &mount.VolumeOptions{

-			Labels: AddStackLabel(namespace, stackVolume.Labels),

-			NoCopy: isNoCopy(mode),

-		}

+	result.Source = namespace.Scope(volume.Source)

+	result.VolumeOptions = &mount.VolumeOptions{}

-		if stackVolume.Driver != "" {

-			volumeOptions.DriverConfig = &mount.Driver{

-				Name:    stackVolume.Driver,

-				Options: stackVolume.DriverOpts,

-			}

-		}

-		source = namespace.Scope(source)

+	if volume.Volume != nil {

+		result.VolumeOptions.NoCopy = volume.Volume.NoCopy

 	}

-	return mount.Mount{

-		Type:          mount.TypeVolume,

-		Source:        source,

-		Target:        target,

-		ReadOnly:      isReadOnly(mode),

-		VolumeOptions: volumeOptions,

-	}, nil

-}

-func modeHas(mode []string, field string) bool {

-	for _, item := range mode {

-		if item == field {

-			return true

-		}

+	// External named volumes

+	if stackVolume.External.External {

+		result.Source = stackVolume.External.Name

+		return result, nil

 	}

-	return false

-}

-func isReadOnly(mode []string) bool {

-	return modeHas(mode, "ro")

-}

-

-func isNoCopy(mode []string) bool {

-	return modeHas(mode, "nocopy")

-}

-

-func getBindOptions(mode []string) *mount.BindOptions {

-	for _, item := range mode {

-		for _, propagation := range mount.Propagations {

-			if mount.Propagation(item) == propagation {

-				return &mount.BindOptions{Propagation: mount.Propagation(item)}

-			}

+	result.VolumeOptions.Labels = AddStackLabel(namespace, stackVolume.Labels)

+	if stackVolume.Driver != "" || stackVolume.DriverOpts != nil {

+		result.VolumeOptions.DriverConfig = &mount.Driver{

+			Name:    stackVolume.Driver,

+			Options: stackVolume.DriverOpts,

 		}

 	}

-	return nil

+

+	// Named volumes

+	return result, nil

 }

@@ -8,51 +8,48 @@ import (

 	"github.com/docker/docker/pkg/testutil/assert"

 )

-func TestIsReadOnly(t *testing.T) {

-	assert.Equal(t, isReadOnly([]string{"foo", "bar", "ro"}), true)

-	assert.Equal(t, isReadOnly([]string{"ro"}), true)

-	assert.Equal(t, isReadOnly([]string{}), false)

-	assert.Equal(t, isReadOnly([]string{"foo", "rw"}), false)

-	assert.Equal(t, isReadOnly([]string{"foo"}), false)

-}

-

-func TestIsNoCopy(t *testing.T) {

-	assert.Equal(t, isNoCopy([]string{"foo", "bar", "nocopy"}), true)

-	assert.Equal(t, isNoCopy([]string{"nocopy"}), true)

-	assert.Equal(t, isNoCopy([]string{}), false)

-	assert.Equal(t, isNoCopy([]string{"foo", "rw"}), false)

-}

-

-func TestGetBindOptions(t *testing.T) {

-	opts := getBindOptions([]string{"slave"})

-	expected := mount.BindOptions{Propagation: mount.PropagationSlave}

-	assert.Equal(t, *opts, expected)

-}

-

-func TestGetBindOptionsNone(t *testing.T) {

-	opts := getBindOptions([]string{"ro"})

-	assert.Equal(t, opts, (*mount.BindOptions)(nil))

-}

-

 func TestConvertVolumeToMountAnonymousVolume(t *testing.T) {

-	stackVolumes := volumes{}

-	namespace := NewNamespace("foo")

+	config := composetypes.ServiceVolumeConfig{

+		Type:   "volume",

+		Target: "/foo/bar",

+	}

 	expected := mount.Mount{

 		Type:   mount.TypeVolume,

 		Target: "/foo/bar",

 	}

-	mount, err := convertVolumeToMount("/foo/bar", stackVolumes, namespace)

+	mount, err := convertVolumeToMount(config, volumes{}, NewNamespace("foo"))

 	assert.NilError(t, err)

 	assert.DeepEqual(t, mount, expected)

 }

-func TestConvertVolumeToMountInvalidFormat(t *testing.T) {

+func TestConvertVolumeToMountConflictingOptionsBind(t *testing.T) {

 	namespace := NewNamespace("foo")

-	invalids := []string{"::", "::cc", ":bb:", "aa::", "aa::cc", "aa:bb:", " : : ", " : :cc", " :bb: ", "aa: : ", "aa: :cc", "aa:bb: "}

-	for _, vol := range invalids {

-		_, err := convertVolumeToMount(vol, volumes{}, namespace)

-		assert.Error(t, err, "invalid volume: "+vol)

+

+	config := composetypes.ServiceVolumeConfig{

+		Type:   "volume",

+		Source: "foo",

+		Target: "/target",

+		Bind: &composetypes.ServiceVolumeBind{

+			Propagation: "slave",

+		},

 	}

+	_, err := convertVolumeToMount(config, volumes{}, namespace)

+	assert.Error(t, err, "bind options are incompatible")

+}

+

+func TestConvertVolumeToMountConflictingOptionsVolume(t *testing.T) {

+	namespace := NewNamespace("foo")

+

+	config := composetypes.ServiceVolumeConfig{

+		Type:   "bind",

+		Source: "/foo",

+		Target: "/target",

+		Volume: &composetypes.ServiceVolumeVolume{

+			NoCopy: true,

+		},

+	}

+	_, err := convertVolumeToMount(config, volumes{}, namespace)

+	assert.Error(t, err, "volume options are incompatible")

 }

 func TestConvertVolumeToMountNamedVolume(t *testing.T) {

@@ -84,9 +81,19 @@ func TestConvertVolumeToMountNamedVolume(t *testing.T) {

 					"opt": "value",

 				},

 			},

+			NoCopy: true,

+		},

+	}

+	config := composetypes.ServiceVolumeConfig{

+		Type:     "volume",

+		Source:   "normal",

+		Target:   "/foo",

+		ReadOnly: true,

+		Volume: &composetypes.ServiceVolumeVolume{

+			NoCopy: true,

 		},

 	}

-	mount, err := convertVolumeToMount("normal:/foo:ro", stackVolumes, namespace)

+	mount, err := convertVolumeToMount(config, stackVolumes, namespace)

 	assert.NilError(t, err)

 	assert.DeepEqual(t, mount, expected)

 }

@@ -109,7 +116,12 @@ func TestConvertVolumeToMountNamedVolumeExternal(t *testing.T) {

 			NoCopy: false,

 		},

 	}

-	mount, err := convertVolumeToMount("outside:/foo", stackVolumes, namespace)

+	config := composetypes.ServiceVolumeConfig{

+		Type:   "volume",

+		Source: "outside",

+		Target: "/foo",

+	}

+	mount, err := convertVolumeToMount(config, stackVolumes, namespace)

 	assert.NilError(t, err)

 	assert.DeepEqual(t, mount, expected)

 }

@@ -132,7 +144,15 @@ func TestConvertVolumeToMountNamedVolumeExternalNoCopy(t *testing.T) {

 			NoCopy: true,

 		},

 	}

-	mount, err := convertVolumeToMount("outside:/foo:nocopy", stackVolumes, namespace)

+	config := composetypes.ServiceVolumeConfig{

+		Type:   "volume",

+		Source: "outside",

+		Target: "/foo",

+		Volume: &composetypes.ServiceVolumeVolume{

+			NoCopy: true,

+		},

+	}

+	mount, err := convertVolumeToMount(config, stackVolumes, namespace)

 	assert.NilError(t, err)

 	assert.DeepEqual(t, mount, expected)

 }

@@ -147,13 +167,26 @@ func TestConvertVolumeToMountBind(t *testing.T) {

 		ReadOnly:    true,

 		BindOptions: &mount.BindOptions{Propagation: mount.PropagationShared},

 	}

-	mount, err := convertVolumeToMount("/bar:/foo:ro,shared", stackVolumes, namespace)

+	config := composetypes.ServiceVolumeConfig{

+		Type:     "bind",

+		Source:   "/bar",

+		Target:   "/foo",

+		ReadOnly: true,

+		Bind:     &composetypes.ServiceVolumeBind{Propagation: "shared"},

+	}

+	mount, err := convertVolumeToMount(config, stackVolumes, namespace)

 	assert.NilError(t, err)

 	assert.DeepEqual(t, mount, expected)

 }

 func TestConvertVolumeToMountVolumeDoesNotExist(t *testing.T) {

 	namespace := NewNamespace("foo")

-	_, err := convertVolumeToMount("unknown:/foo:ro", volumes{}, namespace)

+	config := composetypes.ServiceVolumeConfig{

+		Type:     "volume",

+		Source:   "unknown",

+		Target:   "/foo",

+		ReadOnly: true,

+	}

+	_, err := convertVolumeToMount(config, volumes{}, namespace)

 	assert.Error(t, err, "undefined volume: unknown")

 }

@@ -81,13 +81,18 @@ func toError(result *gojsonschema.Result) error {

 	return err

 }

+const (

+	jsonschemaOneOf = "number_one_of"

+	jsonschemaAnyOf = "number_any_of"

+)

+

 func getDescription(err validationError) string {

 	switch err.parent.Type() {

 	case "invalid_type":

 		if expectedType, ok := err.parent.Details()["expected"].(string); ok {

 			return fmt.Sprintf("must be a %s", humanReadableType(expectedType))

 		}

-	case "number_one_of", "number_any_of":

+	case jsonschemaOneOf, jsonschemaAnyOf:

 		if err.child == nil {

 			return err.parent.Description()

 		}