From ac76925ff258223e6b4ea4365db190c8b5444d55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Tue, 23 Jan 2024 12:32:09 +0100 Subject: [PATCH 1/4] volume/local: Make host resolution backwards compatible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 8ae94cafa50a90192a0c0189f54443abac7a9b44 added a DNS resolution of the `device` part of the volume option. The previous way to resolve the passed hostname was to use `addr` option, which was handled by the same code path as the `nfs` mount type. The issue is that `addr` is also an SMB module option handled by kernel and passing a hostname as `addr` produces an invalid argument error. To fix that, restore the old behavior to handle `addr` the same way as before, and only perform the new DNS resolution of `device` if there is no `addr` passed. Signed-off-by: Paweł Gronowski (cherry picked from commit 0d51cf9db82f196af28b7b69139bb01154c4f72a) Signed-off-by: Paweł Gronowski --- volume/local/local_unix.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/volume/local/local_unix.go b/volume/local/local_unix.go index 420e179560..337fdb13c6 100644 --- a/volume/local/local_unix.go +++ b/volume/local/local_unix.go @@ -127,16 +127,20 @@ func (v *localVolume) mount() error { mountDevice := v.opts.MountDevice switch v.opts.MountType { - case "nfs": + case "nfs", "cifs": if addrValue := getAddress(v.opts.MountOpts); addrValue != "" && net.ParseIP(addrValue).To4() == nil { ipAddr, err := net.ResolveIPAddr("ip", addrValue) if err != nil { - return errors.Wrapf(err, "error resolving passed in network volume address") + return errors.Wrap(err, "error resolving passed in network volume address") } mountOpts = strings.Replace(mountOpts, "addr="+addrValue, "addr="+ipAddr.String(), 1) } - case "cifs": - deviceURL, err := url.Parse(v.opts.MountDevice) + + if v.opts.MountType != "cifs" { + break + } + + deviceURL, err := url.Parse(mountDevice) if err != nil { return errors.Wrapf(err, "error parsing mount device url") } From cb77e48229a84f7a4ee9cddcc8725fd0f6a4e07f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Tue, 23 Jan 2024 17:17:53 +0100 Subject: [PATCH 2/4] volume/local: Break early if `addr` was specified MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I made a mistake in the last commit - after resolving the IP from the passed `addr` for CIFS it would still resolve the `device` part. Apply only one name resolution Signed-off-by: Paweł Gronowski (cherry picked from commit df43311f3d47b8f88fa6682be065d1eee5a7b9a9) Signed-off-by: Paweł Gronowski --- volume/local/local_unix.go | 1 + 1 file changed, 1 insertion(+) diff --git a/volume/local/local_unix.go b/volume/local/local_unix.go index 337fdb13c6..734fc5c25b 100644 --- a/volume/local/local_unix.go +++ b/volume/local/local_unix.go @@ -134,6 +134,7 @@ func (v *localVolume) mount() error { return errors.Wrap(err, "error resolving passed in network volume address") } mountOpts = strings.Replace(mountOpts, "addr="+addrValue, "addr="+ipAddr.String(), 1) + break } if v.opts.MountType != "cifs" { From a445aa95e56498e4c99d17f15521aab8fa6690eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Tue, 23 Jan 2024 17:16:44 +0100 Subject: [PATCH 3/4] volume/local: Add tests for parsing nfs/cifs mounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Paweł Gronowski (cherry picked from commit f4beb130b0109c6caa62d24154bd398a5bfadd1c) Signed-off-by: Paweł Gronowski --- volume/local/local_linux_test.go | 82 ++++++++++++++++++++++++++++++++ volume/local/local_unix.go | 36 +++++++++----- 2 files changed, 105 insertions(+), 13 deletions(-) diff --git a/volume/local/local_linux_test.go b/volume/local/local_linux_test.go index 07ebdf77fd..f8ce7215e2 100644 --- a/volume/local/local_linux_test.go +++ b/volume/local/local_linux_test.go @@ -3,6 +3,7 @@ package local // import "github.com/docker/docker/volume/local" import ( + "net" "os" "path/filepath" "strconv" @@ -246,3 +247,84 @@ func TestVolCreateValidation(t *testing.T) { }) } } + +func TestVolMountOpts(t *testing.T) { + tests := []struct { + name string + opts optsConfig + expectedErr string + expectedDevice, expectedOpts string + }{ + { + name: "cifs url with space", + opts: optsConfig{ + MountType: "cifs", + MountDevice: "//1.2.3.4/Program Files", + }, + expectedDevice: "//1.2.3.4/Program Files", + expectedOpts: "", + }, + { + name: "cifs resolve addr", + opts: optsConfig{ + MountType: "cifs", + MountDevice: "//example.com/Program Files", + MountOpts: "addr=example.com", + }, + expectedDevice: "//example.com/Program Files", + expectedOpts: "addr=1.2.3.4", + }, + { + name: "cifs resolve device", + opts: optsConfig{ + MountType: "cifs", + MountDevice: "//example.com/Program Files", + }, + expectedDevice: "//1.2.3.4/Program Files", + }, + { + name: "nfs dont resolve device", + opts: optsConfig{ + MountType: "nfs", + MountDevice: "//example.com/Program Files", + }, + expectedDevice: "//example.com/Program Files", + }, + { + name: "nfs resolve addr", + opts: optsConfig{ + MountType: "nfs", + MountDevice: "//example.com/Program Files", + MountOpts: "addr=example.com", + }, + expectedDevice: "//example.com/Program Files", + expectedOpts: "addr=1.2.3.4", + }, + } + + ip1234 := net.ParseIP("1.2.3.4") + resolveIP := func(network, addr string) (*net.IPAddr, error) { + switch addr { + case "example.com": + return &net.IPAddr{IP: ip1234}, nil + } + + return nil, &net.DNSError{Err: "no such host", Name: addr, IsNotFound: true} + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + dev, opts, err := getMountOptions(&tc.opts, resolveIP) + + if tc.expectedErr != "" { + assert.Check(t, is.ErrorContains(err, tc.expectedErr)) + } else { + assert.Check(t, err) + } + + assert.Check(t, is.Equal(dev, tc.expectedDevice)) + assert.Check(t, is.Equal(opts, tc.expectedOpts)) + }) + } +} diff --git a/volume/local/local_unix.go b/volume/local/local_unix.go index 734fc5c25b..4f3d3b8f3f 100644 --- a/volume/local/local_unix.go +++ b/volume/local/local_unix.go @@ -118,42 +118,52 @@ func (v *localVolume) needsMount() bool { return false } -func (v *localVolume) mount() error { - if v.opts.MountDevice == "" { - return fmt.Errorf("missing device in volume options") +func getMountOptions(opts *optsConfig, resolveIP func(string, string) (*net.IPAddr, error)) (mountDevice string, mountOpts string, _ error) { + if opts.MountDevice == "" { + return "", "", fmt.Errorf("missing device in volume options") } - mountOpts := v.opts.MountOpts - mountDevice := v.opts.MountDevice + mountOpts = opts.MountOpts + mountDevice = opts.MountDevice - switch v.opts.MountType { + switch opts.MountType { case "nfs", "cifs": - if addrValue := getAddress(v.opts.MountOpts); addrValue != "" && net.ParseIP(addrValue).To4() == nil { - ipAddr, err := net.ResolveIPAddr("ip", addrValue) + if addrValue := getAddress(opts.MountOpts); addrValue != "" && net.ParseIP(addrValue).To4() == nil { + ipAddr, err := resolveIP("ip", addrValue) if err != nil { - return errors.Wrap(err, "error resolving passed in network volume address") + return "", "", errors.Wrap(err, "error resolving passed in network volume address") } mountOpts = strings.Replace(mountOpts, "addr="+addrValue, "addr="+ipAddr.String(), 1) break } - if v.opts.MountType != "cifs" { + if opts.MountType != "cifs" { break } deviceURL, err := url.Parse(mountDevice) if err != nil { - return errors.Wrapf(err, "error parsing mount device url") + return "", "", errors.Wrap(err, "error parsing mount device url") } if deviceURL.Host != "" && net.ParseIP(deviceURL.Host) == nil { - ipAddr, err := net.ResolveIPAddr("ip", deviceURL.Host) + ipAddr, err := resolveIP("ip", deviceURL.Host) if err != nil { - return errors.Wrapf(err, "error resolving passed in network volume address") + return "", "", errors.Wrap(err, "error resolving passed in network volume address") } deviceURL.Host = ipAddr.String() mountDevice = deviceURL.String() } } + + return mountDevice, mountOpts, nil +} + +func (v *localVolume) mount() error { + mountDevice, mountOpts, err := getMountOptions(v.opts, net.ResolveIPAddr) + if err != nil { + return err + } + if err := mount.Mount(mountDevice, v.path, v.opts.MountType, mountOpts); err != nil { if password := getPassword(v.opts.MountOpts); password != "" { err = errors.New(strings.Replace(err.Error(), "password="+password, "password=********", 1)) From 3de920a0b1025bf486a756f766ecf42b1487f07d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Gronowski?= Date: Tue, 23 Jan 2024 17:34:49 +0100 Subject: [PATCH 4/4] volume/local: Fix cifs url containing spaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unescapes the URL to avoid passing an URL encoded address to the kernel. Signed-off-by: Paweł Gronowski (cherry picked from commit 250886741bec1151605cccbfa4462ebd97c131c9) Signed-off-by: Paweł Gronowski --- volume/local/local_unix.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/volume/local/local_unix.go b/volume/local/local_unix.go index 4f3d3b8f3f..d265d6e2f6 100644 --- a/volume/local/local_unix.go +++ b/volume/local/local_unix.go @@ -151,7 +151,11 @@ func getMountOptions(opts *optsConfig, resolveIP func(string, string) (*net.IPAd return "", "", errors.Wrap(err, "error resolving passed in network volume address") } deviceURL.Host = ipAddr.String() - mountDevice = deviceURL.String() + dev, err := url.QueryUnescape(deviceURL.String()) + if err != nil { + return "", "", fmt.Errorf("failed to unescape device URL: %q", deviceURL) + } + mountDevice = dev } }