From f3d08d59aa402444a311701dc231f4a1e83440df Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 2 Jul 2021 13:25:27 +0200 Subject: [PATCH] volume/mounts: move some code to correct location, and minor linting/formatting - Remove the windowsparser.HasResource() override, as it was the same on both Windows and Linux - Move the rxLCOWDestination to the lcowParser code - Move the rwModes variable to a generic (non-platform-specific) file, as it's used both for the windowsParser and the linuxParser - Some minor formatting and linting changes Signed-off-by: Sebastiaan van Stijn --- volume/mounts/lcow_parser.go | 8 ++++++++ volume/mounts/linux_parser.go | 9 +-------- volume/mounts/parser.go | 6 ++++++ volume/mounts/parser_test.go | 10 +++------- volume/mounts/volume_unix.go | 4 ---- volume/mounts/volume_windows.go | 3 --- volume/mounts/windows_parser.go | 17 ++++++++--------- 7 files changed, 26 insertions(+), 31 deletions(-) diff --git a/volume/mounts/lcow_parser.go b/volume/mounts/lcow_parser.go index bafb7b07f8..645a731d93 100644 --- a/volume/mounts/lcow_parser.go +++ b/volume/mounts/lcow_parser.go @@ -7,6 +7,14 @@ import ( "github.com/docker/docker/api/types/mount" ) +// rxLCOWDestination is the regex expression for the mount destination for LCOW +// +// Destination (aka container path): +// - Variation on hostdir but can be a drive followed by colon as well +// - If a path, must be absolute. Can include spaces +// - Drive cannot be c: (explicitly checked in code, not RegEx) +const rxLCOWDestination = `(?P/(?:[^\\/:*?"<>\r\n]+[/]?)*)` + var lcowSpecificValidators mountValidator = func(m *mount.Mount) error { if path.Clean(m.Target) == "/" { return ErrVolumeTargetIsRoot diff --git a/volume/mounts/linux_parser.go b/volume/mounts/linux_parser.go index b5ed4af6a3..8dc9afd34c 100644 --- a/volume/mounts/linux_parser.go +++ b/volume/mounts/linux_parser.go @@ -12,8 +12,7 @@ import ( "github.com/docker/docker/volume" ) -type linuxParser struct { -} +type linuxParser struct{} func linuxSplitRawSpec(raw string) ([]string, error) { if strings.Count(raw, ":") > 2 { @@ -115,12 +114,6 @@ func (p *linuxParser) validateMountConfigImpl(mnt *mount.Mount, validateBindSour return nil } -// read-write modes -var rwModes = map[string]bool{ - "rw": true, - "ro": true, -} - // label modes var linuxLabelModes = map[string]bool{ "Z": true, diff --git a/volume/mounts/parser.go b/volume/mounts/parser.go index 73681750ea..976d276a82 100644 --- a/volume/mounts/parser.go +++ b/volume/mounts/parser.go @@ -18,6 +18,12 @@ const ( // It's used by both LCOW and Linux parsers. var ErrVolumeTargetIsRoot = errors.New("invalid specification: destination can't be '/'") +// read-write modes +var rwModes = map[string]bool{ + "rw": true, + "ro": true, +} + // Parser represents a platform specific parser for mount expressions type Parser interface { ParseMountRaw(raw, volumeDriver string) (*MountPoint, error) diff --git a/volume/mounts/parser_test.go b/volume/mounts/parser_test.go index c879c3b152..ef4919b944 100644 --- a/volume/mounts/parser_test.go +++ b/volume/mounts/parser_test.go @@ -10,7 +10,6 @@ import ( "github.com/docker/docker/api/types/mount" "gotest.tools/v3/assert" - "gotest.tools/v3/assert/cmp" ) type parseMountRawTestSet struct { @@ -300,9 +299,7 @@ func TestParseMountRaw(t *testing.T) { winParser := &windowsParser{} lcowParser := &lcowParser{} tester := func(parser Parser, set parseMountRawTestSet) { - for _, path := range set.valid { - if _, err := parser.ParseMountRaw(path, "local"); err != nil { t.Errorf("ParseMountRaw(`%q`) should succeed: error %q", path, err) } @@ -318,10 +315,10 @@ func TestParseMountRaw(t *testing.T) { } } } + tester(linParser, linuxSet) tester(winParser, windowsSet) tester(lcowParser, lcowSet) - } // testParseMountRaw is a structure used by TestParseMountRawSplit for @@ -397,7 +394,7 @@ func TestParseMountRawSplit(t *testing.T) { } if m == nil || err != nil { - t.Errorf("ParseMountRaw failed for spec '%s', driver '%s', error '%v'", c.bind, c.driver, err.Error()) + t.Errorf("ParseMountRaw failed for spec '%s', driver '%s', error '%v'", c.bind, c.driver, err) continue } @@ -525,6 +522,5 @@ func TestParseMountSpecBindWithFileinfoError(t *testing.T) { parser := NewParser(runtime.GOOS) _, err := parser.ParseMountSpec(m) - assert.Assert(t, err != nil) - assert.Assert(t, cmp.Contains(err.Error(), "some crazy error")) + assert.ErrorContains(t, err, "some crazy error") } diff --git a/volume/mounts/volume_unix.go b/volume/mounts/volume_unix.go index c6d51e0710..9f74c9e7c1 100644 --- a/volume/mounts/volume_unix.go +++ b/volume/mounts/volume_unix.go @@ -12,7 +12,3 @@ func (p *linuxParser) HasResource(m *MountPoint, absolutePath string) bool { relPath, err := filepath.Rel(m.Destination, absolutePath) return err == nil && relPath != ".." && !strings.HasPrefix(relPath, fmt.Sprintf("..%c", filepath.Separator)) } - -func (p *windowsParser) HasResource(m *MountPoint, absolutePath string) bool { - return false -} diff --git a/volume/mounts/volume_windows.go b/volume/mounts/volume_windows.go index 773e7db88a..d91f6cee3a 100644 --- a/volume/mounts/volume_windows.go +++ b/volume/mounts/volume_windows.go @@ -1,8 +1,5 @@ package mounts // import "github.com/docker/docker/volume/mounts" -func (p *windowsParser) HasResource(m *MountPoint, absolutePath string) bool { - return false -} func (p *linuxParser) HasResource(m *MountPoint, absolutePath string) bool { return false } diff --git a/volume/mounts/windows_parser.go b/volume/mounts/windows_parser.go index 8f427d8c50..e0f8f5a7d1 100644 --- a/volume/mounts/windows_parser.go +++ b/volume/mounts/windows_parser.go @@ -12,8 +12,7 @@ import ( "github.com/docker/docker/pkg/stringid" ) -type windowsParser struct { -} +type windowsParser struct{} const ( // Spec should be in the format [source:]destination[:mode] @@ -63,12 +62,6 @@ const ( // rxDestination is the regex expression for the mount destination rxDestination = `(?P((?:\\\\\?\\)?([a-z]):((?:[\\/][^\\/:*?"<>\r\n]+)*[\\/]?))|(` + rxPipe + `))` - rxLCOWDestination = `(?P/(?:[^\\/:*?"<>\r\n]+[/]?)*)` - // Destination (aka container path): - // - Variation on hostdir but can be a drive followed by colon as well - // - If a path, must be absolute. Can include spaces - // - Drive cannot be c: (explicitly checked in code, not RegEx) - // rxMode is the regex expression for the mode of the mount // Mode (optional): // - Hopefully self explanatory in comparison to above regex's. @@ -136,8 +129,10 @@ func windowsValidMountMode(mode string) bool { if mode == "" { return true } + // TODO should windows mounts produce an error if any mode was provided (they're a no-op on windows) return rwModes[strings.ToLower(mode)] } + func windowsValidateNotRoot(p string) error { p = strings.ToLower(strings.Replace(p, `/`, `\`, -1)) if p == "c:" || p == `c:\` { @@ -177,7 +172,7 @@ func (p *windowsParser) ReadWrite(mode string) bool { return strings.ToLower(mode) != "ro" } -// IsVolumeNameValid checks a volume name in a platform specific manner. +// ValidateVolumeName checks a volume name in a platform specific manner. func (p *windowsParser) ValidateVolumeName(name string) error { nameExp := regexp.MustCompile(`^` + rxName + `$`) if !nameExp.MatchString(name) { @@ -454,3 +449,7 @@ func (p *windowsParser) IsBackwardCompatible(m *MountPoint) bool { func (p *windowsParser) ValidateTmpfsMountDestination(dest string) error { return errors.New("Platform does not support tmpfs") } + +func (p *windowsParser) HasResource(m *MountPoint, absolutePath string) bool { + return false +}