From a9cbc773340025bc9d1ec3fb0b24b80198c95402 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 18 Dec 2016 16:50:32 +0100 Subject: [PATCH 1/2] fix conversion of anonymous volumes in compose-file the `convertVolumeToMount()` function did not take anonymous volumes into account when converting volume specifications to bind-mounts. this resulted in the conversion to try to look up an empty "source" volume, which lead to an error; undefined volume: this patch distinguishes "anonymous" volumes from bind-mounts and named-volumes, and skips further processing if no source is defined (i.e. the volume is "anonymous"). Signed-off-by: Sebastiaan van Stijn Upstream-commit: 34889e579f89e389743c35e935709b204bf45bf4 Component: engine --- components/engine/cli/compose/convert/volume.go | 10 +++++++++- components/engine/cli/compose/convert/volume_test.go | 12 ++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/components/engine/cli/compose/convert/volume.go b/components/engine/cli/compose/convert/volume.go index 4eb5788204..027774bcec 100644 --- a/components/engine/cli/compose/convert/volume.go +++ b/components/engine/cli/compose/convert/volume.go @@ -42,7 +42,15 @@ func convertVolumeToMount(volumeSpec string, stackVolumes volumes, namespace Nam case 1: target = parts[0] default: - return mount.Mount{}, fmt.Errorf("invald volume: %s", volumeSpec) + return mount.Mount{}, fmt.Errorf("invalid volume: %s", volumeSpec) + } + + if source == "" { + // Anonymous volume + return mount.Mount{ + Type: mount.TypeVolume, + Target: target, + }, nil } // TODO: catch Windows paths here diff --git a/components/engine/cli/compose/convert/volume_test.go b/components/engine/cli/compose/convert/volume_test.go index 5e9c042b5f..3ca6ab4a52 100644 --- a/components/engine/cli/compose/convert/volume_test.go +++ b/components/engine/cli/compose/convert/volume_test.go @@ -34,6 +34,18 @@ func TestGetBindOptionsNone(t *testing.T) { assert.Equal(t, opts, (*mount.BindOptions)(nil)) } +func TestConvertVolumeToMountAnonymousVolume(t *testing.T) { + stackVolumes := volumes{} + namespace := NewNamespace("foo") + expected := mount.Mount{ + Type: mount.TypeVolume, + Target: "/foo/bar", + } + mount, err := convertVolumeToMount("/foo/bar", stackVolumes, namespace) + assert.NilError(t, err) + assert.DeepEqual(t, mount, expected) +} + func TestConvertVolumeToMountNamedVolume(t *testing.T) { stackVolumes := volumes{ "normal": composetypes.VolumeConfig{ From 1ad841b29eb4327da57f1296ab2f3e7fd93a2c7f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 19 Dec 2016 01:50:08 +0100 Subject: [PATCH 2/2] Improve validation for volume specs The current validation only checked for the number of elements in the volume-spec, however, did not validate if the elements were empty. Because of this, an empty volume-spec (""), or volume spec only containing separators ("::") would not be invalidated. This adds a simple check for empty elements in the volume-spec, and returns an error if the spec is invalid. A unit-test is also added to verify the behavior. Signed-off-by: Sebastiaan van Stijn Upstream-commit: 120241937e67238fd98b839886751bda70129ba8 Component: engine --- components/engine/cli/compose/convert/volume.go | 8 ++++++-- components/engine/cli/compose/convert/volume_test.go | 9 +++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/components/engine/cli/compose/convert/volume.go b/components/engine/cli/compose/convert/volume.go index 027774bcec..3a7504106a 100644 --- a/components/engine/cli/compose/convert/volume.go +++ b/components/engine/cli/compose/convert/volume.go @@ -31,6 +31,12 @@ func convertVolumeToMount(volumeSpec string, stackVolumes volumes, namespace Nam // 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) + } + } + switch len(parts) { case 3: source = parts[0] @@ -41,8 +47,6 @@ func convertVolumeToMount(volumeSpec string, stackVolumes volumes, namespace Nam target = parts[1] case 1: target = parts[0] - default: - return mount.Mount{}, fmt.Errorf("invalid volume: %s", volumeSpec) } if source == "" { diff --git a/components/engine/cli/compose/convert/volume_test.go b/components/engine/cli/compose/convert/volume_test.go index 3ca6ab4a52..bcbfb08b95 100644 --- a/components/engine/cli/compose/convert/volume_test.go +++ b/components/engine/cli/compose/convert/volume_test.go @@ -46,6 +46,15 @@ func TestConvertVolumeToMountAnonymousVolume(t *testing.T) { assert.DeepEqual(t, mount, expected) } +func TestConvertVolumeToMountInvalidFormat(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) + } +} + func TestConvertVolumeToMountNamedVolume(t *testing.T) { stackVolumes := volumes{ "normal": composetypes.VolumeConfig{