Merge pull request #47194 from vvoland/volume-cifs-resolve-optout-2

volume/local: Fix CIFS urls with spaces, add tests

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))
+		})
+	}
+}

volume/local/local_unix.go

@@ -118,41 +118,56 @@ 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()
+			dev, err := url.QueryUnescape(deviceURL.String())
+			if err != nil {
+				return "", "", fmt.Errorf("failed to unescape device URL: %q", deviceURL)
+			}
+			mountDevice = dev
 		}
 	}
+
+	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))