From 0b290094b581ecab6e071b76682c27e6e1c254f1 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 27 Oct 2024 18:40:05 +0100 Subject: [PATCH 1/6] volume/mounts: windowsParser.ConvertTmpfsOptions don't use runtime.GOOS These parsers can be compiled and used cross-platform, but follow platform- specific semantics. Remove the use of runtime.GOOS, as it would result in confusing errors ("linux does not support tmpfs"). Signed-off-by: Sebastiaan van Stijn --- volume/mounts/windows_parser.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/volume/mounts/windows_parser.go b/volume/mounts/windows_parser.go index 062ede0356..c544039f78 100644 --- a/volume/mounts/windows_parser.go +++ b/volume/mounts/windows_parser.go @@ -6,7 +6,6 @@ import ( "os" "path/filepath" "regexp" - "runtime" "strings" "github.com/docker/docker/api/types/mount" @@ -447,7 +446,7 @@ func (p *windowsParser) DefaultPropagationMode() mount.Propagation { } func (p *windowsParser) ConvertTmpfsOptions(opt *mount.TmpfsOptions, readOnly bool) (string, error) { - return "", fmt.Errorf("%s does not support tmpfs", runtime.GOOS) + return "", errors.New("windows does not support tmpfs") } func (p *windowsParser) DefaultCopyMode() bool { @@ -459,7 +458,7 @@ func (p *windowsParser) IsBackwardCompatible(m *MountPoint) bool { } func (p *windowsParser) ValidateTmpfsMountDestination(dest string) error { - return errors.New("platform does not support tmpfs") + return errors.New("windows does not support tmpfs") } func (p *windowsParser) HasResource(m *MountPoint, absolutePath string) bool { From be7d57367bda7b01ddb0737cd460ae506dbc50cf Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 27 Oct 2024 18:46:18 +0100 Subject: [PATCH 2/6] volumes/mounts: remove backticks from errors These errors used a backtick, which is not commonly used in our errors. Change them to use single quotes instead. Signed-off-by: Sebastiaan van Stijn --- volume/mounts/windows_parser.go | 2 +- volume/mounts/windows_parser_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/volume/mounts/windows_parser.go b/volume/mounts/windows_parser.go index c544039f78..80f1cc6ef5 100644 --- a/volume/mounts/windows_parser.go +++ b/volume/mounts/windows_parser.go @@ -144,7 +144,7 @@ func windowsValidMountMode(mode string) bool { func windowsValidateNotRoot(p string) error { p = strings.ToLower(strings.ReplaceAll(p, `/`, `\`)) if p == "c:" || p == `c:\` { - return fmt.Errorf("destination path cannot be `c:` or `c:\\`: %v", p) + return fmt.Errorf(`destination path (%v) cannot be 'c:' or 'c:\'`, p) } return nil } diff --git a/volume/mounts/windows_parser_test.go b/volume/mounts/windows_parser_test.go index 8490c32e8d..c034954954 100644 --- a/volume/mounts/windows_parser_test.go +++ b/volume/mounts/windows_parser_test.go @@ -44,8 +44,8 @@ func TestWindowsParseMountRaw(t *testing.T) { `..\`: "invalid volume specification: ", `c:\:..\`: "invalid volume specification: ", `c:\:d:\:xyzzy`: "invalid volume specification: ", - `c:`: "cannot be `c:`", - `c:\`: "cannot be `c:`", + `c:`: "cannot be 'c:'", + `c:\`: "cannot be 'c:'", `c:\notexist:d:`: `source path does not exist: c:\notexist`, `c:\windows\system32\ntdll.dll:d:`: `source path must be a directory`, `name<:d:`: `invalid volume specification`, From e141be875297033635214874a7da239c6a062286 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 27 Oct 2024 18:53:36 +0100 Subject: [PATCH 3/6] volumes/mounts: remove backticks from test logs Signed-off-by: Sebastiaan van Stijn --- volume/mounts/lcow_parser_test.go | 6 +++--- volume/mounts/linux_parser_test.go | 6 +++--- volume/mounts/windows_parser_test.go | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/volume/mounts/lcow_parser_test.go b/volume/mounts/lcow_parser_test.go index 03ad2e97c4..78eb710e8e 100644 --- a/volume/mounts/lcow_parser_test.go +++ b/volume/mounts/lcow_parser_test.go @@ -85,16 +85,16 @@ func TestLCOWParseMountRaw(t *testing.T) { for _, path := range valid { if _, err := parser.ParseMountRaw(path, "local"); err != nil { - t.Errorf("ParseMountRaw(`%q`) should succeed: error %q", path, err) + t.Errorf("ParseMountRaw(%q) should succeed: error %q", path, err) } } for path, expectedError := range invalid { if mp, err := parser.ParseMountRaw(path, "local"); err == nil { - t.Errorf("ParseMountRaw(`%q`) should have failed validation. Err '%v' - MP: %v", path, err, mp) + t.Errorf("ParseMountRaw(%q) should have failed validation. Err '%v' - MP: %v", path, err, mp) } else { if !strings.Contains(err.Error(), expectedError) { - t.Errorf("ParseMountRaw(`%q`) error should contain %q, got %v", path, expectedError, err.Error()) + t.Errorf("ParseMountRaw(%q) error should contain %q, got %v", path, expectedError, err.Error()) } } } diff --git a/volume/mounts/linux_parser_test.go b/volume/mounts/linux_parser_test.go index 20c9fbc18b..f9d68da079 100644 --- a/volume/mounts/linux_parser_test.go +++ b/volume/mounts/linux_parser_test.go @@ -83,16 +83,16 @@ func TestLinuxParseMountRaw(t *testing.T) { for _, path := range valid { if _, err := parser.ParseMountRaw(path, "local"); err != nil { - t.Errorf("ParseMountRaw(`%q`) should succeed: error %q", path, err) + t.Errorf("ParseMountRaw(%q) should succeed: error %q", path, err) } } for path, expectedError := range invalid { if mp, err := parser.ParseMountRaw(path, "local"); err == nil { - t.Errorf("ParseMountRaw(`%q`) should have failed validation. Err '%v' - MP: %v", path, err, mp) + t.Errorf("ParseMountRaw(%q) should have failed validation. Err '%v' - MP: %v", path, err, mp) } else { if !strings.Contains(err.Error(), expectedError) { - t.Errorf("ParseMountRaw(`%q`) error should contain %q, got %v", path, expectedError, err.Error()) + t.Errorf("ParseMountRaw(%q) error should contain %q, got %v", path, expectedError, err.Error()) } } } diff --git a/volume/mounts/windows_parser_test.go b/volume/mounts/windows_parser_test.go index c034954954..56641457e4 100644 --- a/volume/mounts/windows_parser_test.go +++ b/volume/mounts/windows_parser_test.go @@ -92,16 +92,16 @@ func TestWindowsParseMountRaw(t *testing.T) { for _, path := range valid { if _, err := parser.ParseMountRaw(path, "local"); err != nil { - t.Errorf("ParseMountRaw(`%q`) should succeed: error %q", path, err) + t.Errorf("ParseMountRaw(%q) should succeed: error %q", path, err) } } for path, expectedError := range invalid { if mp, err := parser.ParseMountRaw(path, "local"); err == nil { - t.Errorf("ParseMountRaw(`%q`) should have failed validation. Err '%v' - MP: %v", path, err, mp) + t.Errorf("ParseMountRaw(%q) should have failed validation. Err '%v' - MP: %v", path, err, mp) } else { if !strings.Contains(err.Error(), expectedError) { - t.Errorf("ParseMountRaw(`%q`) error should contain %q, got %v", path, expectedError, err.Error()) + t.Errorf("ParseMountRaw(%q) error should contain %q, got %v", path, expectedError, err.Error()) } } } From 6b0c4b5216ae752b19d9fd6e793a60b10159a6d0 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 27 Oct 2024 19:07:56 +0100 Subject: [PATCH 4/6] volumes/mounts: don't set "expected" values for fail cases ParseMountRaw returns a nil value on error, so there's nothing to check other than it not returning a value. Signed-off-by: Sebastiaan van Stijn --- volume/mounts/lcow_parser_test.go | 48 ++++++++++------------------ volume/mounts/linux_parser_test.go | 12 +++---- volume/mounts/windows_parser_test.go | 38 ++++++++-------------- 3 files changed, 36 insertions(+), 62 deletions(-) diff --git a/volume/mounts/lcow_parser_test.go b/volume/mounts/lcow_parser_test.go index 78eb710e8e..1bfe7b49ae 100644 --- a/volume/mounts/lcow_parser_test.go +++ b/volume/mounts/lcow_parser_test.go @@ -136,12 +136,9 @@ func TestLCOWParseMountRawSplit(t *testing.T) { expRW: true, }, { - bind: `c:\:/foo:foo`, - driver: "local", - expType: mount.TypeBind, - expDest: `/foo`, - expSource: `c:\`, - fail: true, + bind: `c:\:/foo:foo`, + driver: "local", + fail: true, }, { bind: `name:/foo:rw`, @@ -170,39 +167,27 @@ func TestLCOWParseMountRawSplit(t *testing.T) { expDriver: "local", }, { - bind: `name:/`, - expType: mount.TypeVolume, - expRW: true, - fail: true, + bind: `name:/`, + fail: true, }, { - bind: `driver/name:/`, - expType: mount.TypeVolume, - expRW: true, - fail: true, + bind: `driver/name:/`, + fail: true, }, { - bind: `\\.\pipe\foo:\\.\pipe\bar`, - driver: "local", - expType: mount.TypeNamedPipe, - expDest: `\\.\pipe\bar`, - expSource: `\\.\pipe\foo`, - expRW: true, - fail: true, + bind: `\\.\pipe\foo:\\.\pipe\bar`, + driver: "local", + fail: true, }, { - bind: `\\.\pipe\foo:/data`, - driver: "local", - expType: mount.TypeNamedPipe, - expRW: true, - fail: true, + bind: `\\.\pipe\foo:/data`, + driver: "local", + fail: true, }, { - bind: `c:\foo\bar:\\.\pipe\foo`, - driver: "local", - expType: mount.TypeNamedPipe, - expRW: true, - fail: true, + bind: `c:\foo\bar:\\.\pipe\foo`, + driver: "local", + fail: true, }, } @@ -216,6 +201,7 @@ func TestLCOWParseMountRawSplit(t *testing.T) { t.Run(tc.bind, func(t *testing.T) { m, err := parser.ParseMountRaw(tc.bind, tc.driver) if tc.fail { + assert.Check(t, is.Nil(m)) assert.Check(t, is.ErrorContains(err, ""), "expected an error") return } diff --git a/volume/mounts/linux_parser_test.go b/volume/mounts/linux_parser_test.go index f9d68da079..6660145207 100644 --- a/volume/mounts/linux_parser_test.go +++ b/volume/mounts/linux_parser_test.go @@ -131,9 +131,8 @@ func TestLinuxParseMountRawSplit(t *testing.T) { expRW: true, }, { - bind: "/tmp:/tmp4:foo", - expType: mount.TypeBind, - fail: true, + bind: "/tmp:/tmp4:foo", + fail: true, }, { bind: "name:/named1", @@ -167,10 +166,8 @@ func TestLinuxParseMountRawSplit(t *testing.T) { expRW: true, }, { - bind: "/tmp:tmp", - expType: mount.TypeBind, - expRW: true, - fail: true, + bind: "/tmp:tmp", + fail: true, }, } @@ -184,6 +181,7 @@ func TestLinuxParseMountRawSplit(t *testing.T) { t.Run(tc.bind, func(t *testing.T) { m, err := parser.ParseMountRaw(tc.bind, tc.driver) if tc.fail { + assert.Check(t, is.Nil(m)) assert.Check(t, is.ErrorContains(err, ""), "expected an error") return } diff --git a/volume/mounts/windows_parser_test.go b/volume/mounts/windows_parser_test.go index 56641457e4..fc2bc73b87 100644 --- a/volume/mounts/windows_parser_test.go +++ b/volume/mounts/windows_parser_test.go @@ -151,12 +151,9 @@ func TestWindowsParseMountRawSplit(t *testing.T) { expRW: true, }, { - bind: `c:\:d:\:foo`, - driver: "local", - expType: mount.TypeBind, - expDest: `d:\`, - expSource: `c:\`, - fail: true, + bind: `c:\:d:\:foo`, + driver: "local", + fail: true, }, { bind: `name:d::rw`, @@ -185,16 +182,12 @@ func TestWindowsParseMountRawSplit(t *testing.T) { expDriver: "local", }, { - bind: `name:c:`, - expType: mount.TypeVolume, - expRW: true, - fail: true, + bind: `name:c:`, + fail: true, }, { - bind: `driver/name:c:`, - expType: mount.TypeVolume, - expRW: true, - fail: true, + bind: `driver/name:c:`, + fail: true, }, { bind: `\\.\pipe\foo:\\.\pipe\bar`, @@ -205,18 +198,14 @@ func TestWindowsParseMountRawSplit(t *testing.T) { expRW: true, }, { - bind: `\\.\pipe\foo:c:\foo\bar`, - driver: "local", - expType: mount.TypeNamedPipe, - expRW: true, - fail: true, + bind: `\\.\pipe\foo:c:\foo\bar`, + driver: "local", + fail: true, }, { - bind: `c:\foo\bar:\\.\pipe\foo`, - driver: "local", - expType: mount.TypeNamedPipe, - expRW: true, - fail: true, + bind: `c:\foo\bar:\\.\pipe\foo`, + driver: "local", + fail: true, }, } @@ -230,6 +219,7 @@ func TestWindowsParseMountRawSplit(t *testing.T) { t.Run(tc.bind, func(t *testing.T) { m, err := parser.ParseMountRaw(tc.bind, tc.driver) if tc.fail { + assert.Check(t, is.Nil(m)) assert.Check(t, is.ErrorContains(err, ""), "expected an error") return } From 275609eb37d1602df2d3938f8b399f2ef6b25f77 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 27 Oct 2024 20:40:59 +0100 Subject: [PATCH 5/6] volumes/mounts: test the actual error returned Update the tests to have a match for the actual error string returned. Signed-off-by: Sebastiaan van Stijn --- volume/mounts/lcow_parser_test.go | 22 +++++++++++----------- volume/mounts/linux_parser_test.go | 14 +++++++------- volume/mounts/windows_parser_test.go | 20 ++++++++++---------- 3 files changed, 28 insertions(+), 28 deletions(-) diff --git a/volume/mounts/lcow_parser_test.go b/volume/mounts/lcow_parser_test.go index 1bfe7b49ae..6299d8c186 100644 --- a/volume/mounts/lcow_parser_test.go +++ b/volume/mounts/lcow_parser_test.go @@ -110,7 +110,7 @@ func TestLCOWParseMountRawSplit(t *testing.T) { expName string expDriver string expRW bool - fail bool + expErr string }{ { bind: `c:\:/foo`, @@ -138,7 +138,7 @@ func TestLCOWParseMountRawSplit(t *testing.T) { { bind: `c:\:/foo:foo`, driver: "local", - fail: true, + expErr: `invalid volume specification: 'c:\:/foo:foo'`, }, { bind: `name:/foo:rw`, @@ -167,27 +167,27 @@ func TestLCOWParseMountRawSplit(t *testing.T) { expDriver: "local", }, { - bind: `name:/`, - fail: true, + bind: `name:/`, + expErr: `invalid volume specification: 'name:/': invalid mount config for type "volume": invalid specification: destination can't be '/'`, }, { - bind: `driver/name:/`, - fail: true, + bind: `driver/name:/`, + expErr: `invalid volume specification: 'driver/name:/'`, }, { bind: `\\.\pipe\foo:\\.\pipe\bar`, driver: "local", - fail: true, + expErr: `invalid volume specification: '\\.\pipe\foo:\\.\pipe\bar'`, }, { bind: `\\.\pipe\foo:/data`, driver: "local", - fail: true, + expErr: `invalid volume specification: '\\.\pipe\foo:/data': invalid mount config for type "npipe": Linux containers on Windows do not support named pipe mounts`, }, { bind: `c:\foo\bar:\\.\pipe\foo`, driver: "local", - fail: true, + expErr: `invalid volume specification: 'c:\foo\bar:\\.\pipe\foo'`, }, } @@ -200,9 +200,9 @@ func TestLCOWParseMountRawSplit(t *testing.T) { tc := tc t.Run(tc.bind, func(t *testing.T) { m, err := parser.ParseMountRaw(tc.bind, tc.driver) - if tc.fail { + if tc.expErr != "" { assert.Check(t, is.Nil(m)) - assert.Check(t, is.ErrorContains(err, ""), "expected an error") + assert.Check(t, is.Error(err, tc.expErr)) return } diff --git a/volume/mounts/linux_parser_test.go b/volume/mounts/linux_parser_test.go index 6660145207..a56bf3cc02 100644 --- a/volume/mounts/linux_parser_test.go +++ b/volume/mounts/linux_parser_test.go @@ -108,7 +108,7 @@ func TestLinuxParseMountRawSplit(t *testing.T) { expName string expDriver string expRW bool - fail bool + expErr string }{ { bind: "/tmp:/tmp1", @@ -131,8 +131,8 @@ func TestLinuxParseMountRawSplit(t *testing.T) { expRW: true, }, { - bind: "/tmp:/tmp4:foo", - fail: true, + bind: "/tmp:/tmp4:foo", + expErr: `invalid mode: foo`, }, { bind: "name:/named1", @@ -166,8 +166,8 @@ func TestLinuxParseMountRawSplit(t *testing.T) { expRW: true, }, { - bind: "/tmp:tmp", - fail: true, + bind: "/tmp:tmp", + expErr: `invalid volume specification: '/tmp:tmp': invalid mount config for type "bind": invalid mount path: 'tmp' mount path must be absolute`, }, } @@ -180,9 +180,9 @@ func TestLinuxParseMountRawSplit(t *testing.T) { tc := tc t.Run(tc.bind, func(t *testing.T) { m, err := parser.ParseMountRaw(tc.bind, tc.driver) - if tc.fail { + if tc.expErr != "" { assert.Check(t, is.Nil(m)) - assert.Check(t, is.ErrorContains(err, ""), "expected an error") + assert.Check(t, is.Error(err, tc.expErr)) return } diff --git a/volume/mounts/windows_parser_test.go b/volume/mounts/windows_parser_test.go index fc2bc73b87..0c4674cfdd 100644 --- a/volume/mounts/windows_parser_test.go +++ b/volume/mounts/windows_parser_test.go @@ -117,7 +117,7 @@ func TestWindowsParseMountRawSplit(t *testing.T) { expName string expDriver string expRW bool - fail bool + expErr string }{ { bind: `c:\:d:`, @@ -153,7 +153,7 @@ func TestWindowsParseMountRawSplit(t *testing.T) { { bind: `c:\:d:\:foo`, driver: "local", - fail: true, + expErr: `invalid volume specification: 'c:\:d:\:foo'`, }, { bind: `name:d::rw`, @@ -182,12 +182,12 @@ func TestWindowsParseMountRawSplit(t *testing.T) { expDriver: "local", }, { - bind: `name:c:`, - fail: true, + bind: `name:c:`, + expErr: `invalid volume specification: 'name:c:': invalid mount config for type "volume": destination path (c:) cannot be 'c:' or 'c:\'`, }, { - bind: `driver/name:c:`, - fail: true, + bind: `driver/name:c:`, + expErr: `invalid volume specification: 'driver/name:c:'`, }, { bind: `\\.\pipe\foo:\\.\pipe\bar`, @@ -200,12 +200,12 @@ func TestWindowsParseMountRawSplit(t *testing.T) { { bind: `\\.\pipe\foo:c:\foo\bar`, driver: "local", - fail: true, + expErr: `invalid volume specification: '\\.\pipe\foo:c:\foo\bar': invalid mount config for type "npipe": 'c:\foo\bar' is not a valid pipe path`, }, { bind: `c:\foo\bar:\\.\pipe\foo`, driver: "local", - fail: true, + expErr: `invalid volume specification: 'c:\foo\bar:\\.\pipe\foo': invalid mount config for type "bind": bind source path does not exist: c:\foo\bar`, }, } @@ -218,9 +218,9 @@ func TestWindowsParseMountRawSplit(t *testing.T) { tc := tc t.Run(tc.bind, func(t *testing.T) { m, err := parser.ParseMountRaw(tc.bind, tc.driver) - if tc.fail { + if tc.expErr != "" { assert.Check(t, is.Nil(m)) - assert.Check(t, is.ErrorContains(err, ""), "expected an error") + assert.Check(t, is.Error(err, tc.expErr)) return } From 31880791a437cf1b35039643fc258d9b067eb16e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 27 Oct 2024 21:13:26 +0100 Subject: [PATCH 6/6] volumes/mounts: test the actual MountConfig returned Make sure we're asserting the right fields to be propagated, as there are some fields that shadow top-level fields (by design). Signed-off-by: Sebastiaan van Stijn --- volume/mounts/lcow_parser_test.go | 159 +++++++++++++++------- volume/mounts/linux_parser_test.go | 172 +++++++++++++++++------- volume/mounts/windows_parser_test.go | 194 ++++++++++++++++++--------- 3 files changed, 354 insertions(+), 171 deletions(-) diff --git a/volume/mounts/lcow_parser_test.go b/volume/mounts/lcow_parser_test.go index 6299d8c186..67fab81225 100644 --- a/volume/mounts/lcow_parser_test.go +++ b/volume/mounts/lcow_parser_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/docker/docker/api/types/mount" + "github.com/google/go-cmp/cmp/cmpopts" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" ) @@ -102,38 +103,63 @@ func TestLCOWParseMountRaw(t *testing.T) { func TestLCOWParseMountRawSplit(t *testing.T) { cases := []struct { - bind string - driver string - expType mount.Type - expDest string - expSource string - expName string - expDriver string - expRW bool - expErr string + bind string + driver string + expected *MountPoint + expErr string }{ { - bind: `c:\:/foo`, - driver: "local", - expType: mount.TypeBind, - expDest: `/foo`, - expSource: `c:\`, - expRW: true, + bind: `c:\:/foo`, + driver: "local", + expected: &MountPoint{ + Source: `c:\`, + Destination: "/foo", + RW: true, + Type: mount.TypeBind, + Propagation: "", // Propagation is not set on LCOW. + Spec: mount.Mount{ + Source: `c:\`, + Target: "/foo", + ReadOnly: false, + Type: mount.TypeBind, + }, + }, }, { - bind: `c:\:/foo:ro`, - driver: "local", - expType: mount.TypeBind, - expDest: `/foo`, - expSource: `c:\`, + bind: `c:\:/foo:ro`, + driver: "local", + expected: &MountPoint{ + Source: `c:\`, + Destination: "/foo", + RW: false, + Type: mount.TypeBind, + Mode: "ro", + Propagation: "", // Propagation is not set on LCOW. + Spec: mount.Mount{ + Source: `c:\`, + Target: "/foo", + ReadOnly: true, + Type: mount.TypeBind, + }, + }, }, { - bind: `c:\:/foo:rw`, - driver: "local", - expType: mount.TypeBind, - expDest: `/foo`, - expSource: `c:\`, - expRW: true, + bind: `c:\:/foo:rw`, + driver: "local", + expected: &MountPoint{ + Source: `c:\`, + Destination: "/foo", + RW: true, + Type: mount.TypeBind, + Mode: "rw", + Propagation: "", // Propagation is not set on LCOW. + Spec: mount.Mount{ + Source: `c:\`, + Target: "/foo", + ReadOnly: false, + Type: mount.TypeBind, + }, + }, }, { bind: `c:\:/foo:foo`, @@ -141,30 +167,64 @@ func TestLCOWParseMountRawSplit(t *testing.T) { expErr: `invalid volume specification: 'c:\:/foo:foo'`, }, { - bind: `name:/foo:rw`, - driver: "local", - expType: mount.TypeVolume, - expDest: `/foo`, - expName: `name`, - expDriver: "local", - expRW: true, + bind: `name:/foo:rw`, + driver: "local", + expected: &MountPoint{ + Destination: "/foo", + RW: true, + Name: "name", + Driver: "local", + Type: mount.TypeVolume, + Mode: "rw", + Propagation: "", // Propagation is not set on LCOW. + Spec: mount.Mount{ + Source: `name`, + Target: "/foo", + ReadOnly: false, + Type: mount.TypeVolume, + VolumeOptions: &mount.VolumeOptions{DriverConfig: &mount.Driver{Name: "local"}}, + }, + }, }, { - bind: `name:/foo`, - driver: "local", - expType: mount.TypeVolume, - expDest: `/foo`, - expName: `name`, - expDriver: "local", - expRW: true, + bind: `name:/foo`, + driver: "local", + expected: &MountPoint{ + Destination: "/foo", + RW: true, + Name: "name", + Driver: "local", + Type: mount.TypeVolume, + Mode: "", // FIXME(thaJeztah): why is this different than an explicit "rw" ? + Propagation: "", // Propagation is not set on LCOW. + Spec: mount.Mount{ + Source: `name`, + Target: "/foo", + ReadOnly: false, + Type: mount.TypeVolume, + VolumeOptions: &mount.VolumeOptions{DriverConfig: &mount.Driver{Name: "local"}}, + }, + }, }, { - bind: `name:/foo:ro`, - driver: "local", - expType: mount.TypeVolume, - expDest: `/foo`, - expName: `name`, - expDriver: "local", + bind: `name:/foo:ro`, + driver: "local", + expected: &MountPoint{ + Destination: "/foo", + RW: false, + Name: "name", + Driver: "local", + Type: mount.TypeVolume, + Mode: "ro", + Propagation: "", // Propagation is not set on LCOW. + Spec: mount.Mount{ + Source: `name`, + Target: "/foo", + ReadOnly: true, + Type: mount.TypeVolume, + VolumeOptions: &mount.VolumeOptions{DriverConfig: &mount.Driver{Name: "local"}}, + }, + }, }, { bind: `name:/`, @@ -207,12 +267,7 @@ func TestLCOWParseMountRawSplit(t *testing.T) { } assert.NilError(t, err) - assert.Check(t, is.Equal(m.Destination, tc.expDest)) - assert.Check(t, is.Equal(m.Source, tc.expSource)) - assert.Check(t, is.Equal(m.Name, tc.expName)) - assert.Check(t, is.Equal(m.Driver, tc.expDriver)) - assert.Check(t, is.Equal(m.RW, tc.expRW)) - assert.Check(t, is.Equal(m.Type, tc.expType)) + assert.Check(t, is.DeepEqual(*m, *tc.expected, cmpopts.IgnoreUnexported(MountPoint{}))) }) } } diff --git a/volume/mounts/linux_parser_test.go b/volume/mounts/linux_parser_test.go index a56bf3cc02..660c9dc8a6 100644 --- a/volume/mounts/linux_parser_test.go +++ b/volume/mounts/linux_parser_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/docker/docker/api/types/mount" + "github.com/google/go-cmp/cmp/cmpopts" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" ) @@ -100,70 +101,142 @@ func TestLinuxParseMountRaw(t *testing.T) { func TestLinuxParseMountRawSplit(t *testing.T) { cases := []struct { - bind string - driver string - expType mount.Type - expDest string - expSource string - expName string - expDriver string - expRW bool - expErr string + bind string + driver string + expected *MountPoint + expErr string }{ { - bind: "/tmp:/tmp1", - expType: mount.TypeBind, - expDest: "/tmp1", - expSource: "/tmp", - expRW: true, + bind: "/tmp:/tmp1", + expected: &MountPoint{ + Source: "/tmp", + Destination: "/tmp1", + RW: true, + Type: mount.TypeBind, + Propagation: "rprivate", + Spec: mount.Mount{ + Source: "/tmp", + Target: "/tmp1", + ReadOnly: false, + Type: mount.TypeBind, + }, + }, }, { - bind: "/tmp:/tmp2:ro", - expType: mount.TypeBind, - expDest: "/tmp2", - expSource: "/tmp", + bind: "/tmp:/tmp2:ro", + expected: &MountPoint{ + Source: "/tmp", + Destination: "/tmp2", + RW: false, + Type: mount.TypeBind, + Mode: "ro", + Propagation: "rprivate", + Spec: mount.Mount{ + Source: "/tmp", + Target: "/tmp2", + ReadOnly: true, + Type: mount.TypeBind, + }, + }, }, { - bind: "/tmp:/tmp3:rw", - expType: mount.TypeBind, - expDest: "/tmp3", - expSource: "/tmp", - expRW: true, + bind: "/tmp:/tmp3:rw", + expected: &MountPoint{ + Source: "/tmp", + Destination: "/tmp3", + RW: true, + Type: mount.TypeBind, + Mode: "rw", + Propagation: "rprivate", + Spec: mount.Mount{ + Source: "/tmp", + Target: "/tmp3", + ReadOnly: false, + Type: mount.TypeBind, + }, + }, }, { bind: "/tmp:/tmp4:foo", expErr: `invalid mode: foo`, }, { - bind: "name:/named1", - expType: mount.TypeVolume, - expDest: "/named1", - expName: "name", - expRW: true, + bind: "name:/named1", + expected: &MountPoint{ + Destination: "/named1", + RW: true, + Name: "name", + Type: mount.TypeVolume, + Mode: "", // FIXME(thaJeztah): why is this different than an explicit "rw" ? + Propagation: "", + CopyData: true, + Spec: mount.Mount{ + Source: "name", + Target: "/named1", + ReadOnly: false, + Type: mount.TypeVolume, + }, + }, }, { - bind: "name:/named2", - driver: "external", - expType: mount.TypeVolume, - expDest: "/named2", - expName: "name", - expDriver: "external", - expRW: true, + bind: "name:/named2", + driver: "external", + expected: &MountPoint{ + Destination: "/named2", + RW: true, + Name: "name", + Driver: "external", + Type: mount.TypeVolume, + Mode: "", // FIXME(thaJeztah): why is this different than an explicit "rw" ? + Propagation: "", + CopyData: true, + Spec: mount.Mount{ + Source: "name", + Target: "/named2", + ReadOnly: false, + Type: mount.TypeVolume, + VolumeOptions: &mount.VolumeOptions{DriverConfig: &mount.Driver{Name: "external"}}, + }, + }, }, { - bind: "name:/named3:ro", - driver: "local", - expType: mount.TypeVolume, - expDest: "/named3", - expName: "name", - expDriver: "local", + bind: "name:/named3:ro", + driver: "local", + expected: &MountPoint{ + Destination: "/named3", + RW: false, + Name: "name", + Driver: "local", + Type: mount.TypeVolume, + Mode: "ro", + Propagation: "", + CopyData: true, + Spec: mount.Mount{ + Source: "name", + Target: "/named3", + ReadOnly: true, + Type: mount.TypeVolume, + VolumeOptions: &mount.VolumeOptions{DriverConfig: &mount.Driver{Name: "local"}}, + }, + }, }, { - bind: "local/name:/tmp:rw", - expType: mount.TypeVolume, - expDest: "/tmp", - expName: "local/name", - expRW: true, + bind: "local/name:/tmp:rw", + expected: &MountPoint{ + Destination: "/tmp", + RW: true, + Name: "local/name", + Type: mount.TypeVolume, + Mode: "rw", + Propagation: "", + CopyData: true, + Spec: mount.Mount{ + Source: "local/name", + Target: "/tmp", + ReadOnly: false, + Type: mount.TypeVolume, + }, + }, }, { bind: "/tmp:tmp", @@ -187,12 +260,7 @@ func TestLinuxParseMountRawSplit(t *testing.T) { } assert.NilError(t, err) - assert.Check(t, is.Equal(m.Destination, tc.expDest)) - assert.Check(t, is.Equal(m.Source, tc.expSource)) - assert.Check(t, is.Equal(m.Name, tc.expName)) - assert.Check(t, is.Equal(m.Driver, tc.expDriver)) - assert.Check(t, is.Equal(m.RW, tc.expRW)) - assert.Check(t, is.Equal(m.Type, tc.expType)) + assert.Check(t, is.DeepEqual(*m, *tc.expected, cmpopts.IgnoreUnexported(MountPoint{}))) }) } } diff --git a/volume/mounts/windows_parser_test.go b/volume/mounts/windows_parser_test.go index 0c4674cfdd..8469e235bf 100644 --- a/volume/mounts/windows_parser_test.go +++ b/volume/mounts/windows_parser_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/docker/docker/api/types/mount" + "github.com/google/go-cmp/cmp/cmpopts" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" ) @@ -109,77 +110,136 @@ func TestWindowsParseMountRaw(t *testing.T) { func TestWindowsParseMountRawSplit(t *testing.T) { cases := []struct { - bind string - driver string - expType mount.Type - expDest string - expSource string - expName string - expDriver string - expRW bool - expErr string + bind string + driver string + expected *MountPoint + expErr string }{ { - bind: `c:\:d:`, - driver: "local", - expType: mount.TypeBind, - expDest: `d:`, - expSource: `c:\`, - expRW: true, + bind: `c:\:d:`, + driver: "local", + expected: &MountPoint{ + Source: `c:\`, + Destination: `d:`, + RW: true, + Type: mount.TypeBind, + Spec: mount.Mount{ + Source: `c:\`, + Target: `d:`, + ReadOnly: false, + Type: mount.TypeBind, + }, + }, }, { - bind: `c:\:d:\`, - driver: "local", - expType: mount.TypeBind, - expDest: `d:\`, - expSource: `c:\`, - expRW: true, + bind: `c:\:d:\`, + driver: "local", + expected: &MountPoint{ + Source: `c:\`, + Destination: `d:\`, + RW: true, + Type: mount.TypeBind, + Spec: mount.Mount{ + Source: `c:\`, + Target: `d:\`, + ReadOnly: false, + Type: mount.TypeBind, + }, + }, }, { - bind: `c:\:d:\:ro`, - driver: "local", - expType: mount.TypeBind, - expDest: `d:\`, - expSource: `c:\`, + bind: `c:\:d:\:ro`, + expected: &MountPoint{ + Source: `c:\`, + Destination: `d:\`, + RW: false, + Type: mount.TypeBind, + Mode: "ro", + Spec: mount.Mount{ + Source: `c:\`, + Target: `d:\`, + ReadOnly: true, + Type: mount.TypeBind, + // BindOptions: &mount.BindOptions{}, + }, + }, }, { - bind: `c:\:d:\:rw`, - driver: "local", - expType: mount.TypeBind, - expDest: `d:\`, - expSource: `c:\`, - expRW: true, + bind: `c:\:d:\:rw`, + expected: &MountPoint{ + Source: `c:\`, + Destination: `d:\`, + RW: true, + Type: mount.TypeBind, + Mode: "rw", + Spec: mount.Mount{ + Source: `c:\`, + Target: `d:\`, + ReadOnly: false, + Type: mount.TypeBind, + }, + }, }, { bind: `c:\:d:\:foo`, - driver: "local", expErr: `invalid volume specification: 'c:\:d:\:foo'`, }, { - bind: `name:d::rw`, - driver: "local", - expType: mount.TypeVolume, - expDest: `d:`, - expName: `name`, - expDriver: "local", - expRW: true, + bind: `name:d::rw`, + driver: "local", + expected: &MountPoint{ + Destination: `d:`, + RW: true, + Name: `name`, + Driver: `local`, + Type: mount.TypeVolume, + Mode: `rw`, + Spec: mount.Mount{ + Source: `name`, + Target: `d:`, + ReadOnly: false, + Type: mount.TypeVolume, + VolumeOptions: &mount.VolumeOptions{DriverConfig: &mount.Driver{Name: "local"}}, + }, + }, }, { - bind: `name:d:`, - driver: "local", - expType: mount.TypeVolume, - expDest: `d:`, - expName: `name`, - expDriver: "local", - expRW: true, + bind: `name:d:`, + driver: "local", + expected: &MountPoint{ + Destination: `d:`, + RW: true, + Name: `name`, + Driver: `local`, + Type: mount.TypeVolume, + Mode: ``, // FIXME(thaJeztah): why is this different than an explicit "rw" ? + Spec: mount.Mount{ + Source: `name`, + Target: `d:`, + ReadOnly: false, + Type: mount.TypeVolume, + VolumeOptions: &mount.VolumeOptions{DriverConfig: &mount.Driver{Name: "local"}}, + }, + }, }, { - bind: `name:d::ro`, - driver: "local", - expType: mount.TypeVolume, - expDest: `d:`, - expName: `name`, - expDriver: "local", + bind: `name:d::ro`, + driver: "local", + expected: &MountPoint{ + Destination: `d:`, + RW: false, + Name: `name`, + Driver: `local`, + Type: mount.TypeVolume, + Mode: `ro`, + Spec: mount.Mount{ + Source: `name`, + Target: `d:`, + ReadOnly: true, + Type: mount.TypeVolume, + VolumeOptions: &mount.VolumeOptions{DriverConfig: &mount.Driver{Name: "local"}}, + }, + }, }, { bind: `name:c:`, @@ -190,21 +250,26 @@ func TestWindowsParseMountRawSplit(t *testing.T) { expErr: `invalid volume specification: 'driver/name:c:'`, }, { - bind: `\\.\pipe\foo:\\.\pipe\bar`, - driver: "local", - expType: mount.TypeNamedPipe, - expDest: `\\.\pipe\bar`, - expSource: `\\.\pipe\foo`, - expRW: true, + bind: `\\.\pipe\foo:\\.\pipe\bar`, + expected: &MountPoint{ + Source: `\\.\pipe\foo`, + Destination: `\\.\pipe\bar`, + RW: true, + Type: mount.TypeNamedPipe, + Spec: mount.Mount{ + Source: `\\.\pipe\foo`, + Target: `\\.\pipe\bar`, + ReadOnly: false, + Type: mount.TypeNamedPipe, + }, + }, }, { bind: `\\.\pipe\foo:c:\foo\bar`, - driver: "local", expErr: `invalid volume specification: '\\.\pipe\foo:c:\foo\bar': invalid mount config for type "npipe": 'c:\foo\bar' is not a valid pipe path`, }, { bind: `c:\foo\bar:\\.\pipe\foo`, - driver: "local", expErr: `invalid volume specification: 'c:\foo\bar:\\.\pipe\foo': invalid mount config for type "bind": bind source path does not exist: c:\foo\bar`, }, } @@ -225,12 +290,7 @@ func TestWindowsParseMountRawSplit(t *testing.T) { } assert.NilError(t, err) - assert.Check(t, is.Equal(m.Destination, tc.expDest)) - assert.Check(t, is.Equal(m.Source, tc.expSource)) - assert.Check(t, is.Equal(m.Name, tc.expName)) - assert.Check(t, is.Equal(m.Driver, tc.expDriver)) - assert.Check(t, is.Equal(m.RW, tc.expRW)) - assert.Check(t, is.Equal(m.Type, tc.expType)) + assert.Check(t, is.DeepEqual(*m, *tc.expected, cmpopts.IgnoreUnexported(MountPoint{}))) }) } }