Leverage crosspath lib for home / working dir expansion, and detection of absolute path in cross-plat

Signed-off-by: Simon Ferquel <simon.ferquel@docker.com>

Author: simonferquel

cli/command/stack/loader/loader.go

@@ -14,6 +14,7 @@ import (
 	"github.com/docker/cli/cli/compose/loader"
 	"github.com/docker/cli/cli/compose/schema"
 	composetypes "github.com/docker/cli/cli/compose/types"
+	"github.com/docker/docker/pkg/homedir"
 	"github.com/pkg/errors"
 )
 
@@ -97,6 +98,7 @@ func getConfigDetails(composefiles []string, stdin io.Reader) (composetypes.Conf
 	// Take the first file version (2 files can't have different version)
 	details.Version = schema.Version(details.ConfigFiles[0].Config)
 	details.Environment, err = buildEnvironment(os.Environ())
+	details.HomeDir = homedir.Get()
 	return details, err
 }
 

cli/command/stack/loader/loader_test.go

@@ -6,6 +6,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/docker/docker/pkg/homedir"
 	"gotest.tools/assert"
 	is "gotest.tools/assert/cmp"
 	"gotest.tools/fs"
@@ -24,6 +25,7 @@ services:
 	details, err := getConfigDetails([]string{file.Path()}, nil)
 	assert.NilError(t, err)
 	assert.Check(t, is.Equal(filepath.Dir(file.Path()), details.WorkingDir))
+	assert.Check(t, is.Equal(homedir.Get(), details.HomeDir))
 	assert.Assert(t, is.Len(details.ConfigFiles, 1))
 	assert.Check(t, is.Equal("3.0", details.ConfigFiles[0].Config["version"]))
 	assert.Check(t, is.Len(details.Environment, len(os.Environ())))
@@ -41,6 +43,7 @@ services:
 	cwd, err := os.Getwd()
 	assert.NilError(t, err)
 	assert.Check(t, is.Equal(cwd, details.WorkingDir))
+	assert.Check(t, is.Equal(homedir.Get(), details.HomeDir))
 	assert.Assert(t, is.Len(details.ConfigFiles, 1))
 	assert.Check(t, is.Equal("3.0", details.ConfigFiles[0].Config["version"]))
 	assert.Check(t, is.Len(details.Environment, len(os.Environ())))

cli/compose/loader/full-struct_test.go

@@ -5,6 +5,8 @@ import (
 	"path/filepath"
 	"time"
 
+	"github.com/simonferquel/crosspath"
+
 	"github.com/docker/cli/cli/compose/types"
 )
 
@@ -20,6 +22,7 @@ func fullExampleConfig(workingDir, homeDir string) *types.Config {
 }
 
 func services(workingDir, homeDir string) []types.ServiceConfig {
+	configsPath, _ := crosspath.ParsePathWithDefaults(homeDir + "/configs")
 	return []types.ServiceConfig{
 		{
 			Name: "foo",
@@ -350,7 +353,7 @@ func services(workingDir, homeDir string) []types.ServiceConfig {
 				{Source: "/opt/data", Target: "/var/lib/mysql", Type: "bind"},
 				{Source: workingDir, Target: "/code", Type: "bind"},
 				{Source: filepath.Join(workingDir, "static"), Target: "/var/www/html", Type: "bind"},
-				{Source: homeDir + "/configs", Target: "/etc/configs/", Type: "bind", ReadOnly: true},
+				{Source: configsPath.String(), Target: "/etc/configs/", Type: "bind", ReadOnly: true},
 				{Source: "datavolume", Target: "/var/lib/mysql", Type: "volume"},
 				{Source: filepath.Join(workingDir, "opt"), Target: "/opt", Consistency: "cached", Type: "bind"},
 				{Target: "/opt", Type: "tmpfs", Tmpfs: &types.ServiceVolumeTmpfs{

cli/compose/loader/loader.go

@@ -2,7 +2,6 @@ package loader
 
 import (
 	"fmt"
-	"path"
 	"path/filepath"
 	"reflect"
 	"sort"
@@ -18,6 +17,7 @@ import (
 	shellwords "github.com/mattn/go-shellwords"
 	"github.com/mitchellh/mapstructure"
 	"github.com/pkg/errors"
+	"github.com/simonferquel/crosspath"
 	"github.com/sirupsen/logrus"
 	yaml "gopkg.in/yaml.v2"
 )
@@ -109,7 +109,7 @@ func loadSections(config map[string]interface{}, configDetails types.ConfigDetai
 		{
 			key: "services",
 			fnc: func(config map[string]interface{}) error {
-				cfg.Services, err = LoadServices(config, configDetails.WorkingDir, configDetails.LookupEnv)
+				cfg.Services, err = LoadServices(config, configDetails.WorkingDir, configDetails.HomeDir, configDetails.LookupEnv)
 				return err
 			},
 		},
@@ -334,11 +334,11 @@ func formatInvalidKeyError(keyPrefix string, key interface{}) error {
 
 // LoadServices produces a ServiceConfig map from a compose file Dict
 // the servicesDict is not validated if directly used. Use Load() to enable validation
-func LoadServices(servicesDict map[string]interface{}, workingDir string, lookupEnv template.Mapping) ([]types.ServiceConfig, error) {
+func LoadServices(servicesDict map[string]interface{}, workingDir, homeDir string, lookupEnv template.Mapping) ([]types.ServiceConfig, error) {
 	var services []types.ServiceConfig
 
 	for name, serviceDef := range servicesDict {
-		serviceConfig, err := LoadService(name, serviceDef.(map[string]interface{}), workingDir, lookupEnv)
+		serviceConfig, err := LoadService(name, serviceDef.(map[string]interface{}), workingDir, homeDir, lookupEnv)
 		if err != nil {
 			return nil, err
 		}
@@ -350,7 +350,7 @@ func LoadServices(servicesDict map[string]interface{}, workingDir string, lookup
 
 // LoadService produces a single ServiceConfig from a compose file Dict
 // the serviceDict is not validated if directly used. Use Load() to enable validation
-func LoadService(name string, serviceDict map[string]interface{}, workingDir string, lookupEnv template.Mapping) (*types.ServiceConfig, error) {
+func LoadService(name string, serviceDict map[string]interface{}, workingDir, homeDir string, lookupEnv template.Mapping) (*types.ServiceConfig, error) {
 	serviceConfig := &types.ServiceConfig{}
 	if err := transform(serviceDict, serviceConfig); err != nil {
 		return nil, err
@@ -361,7 +361,7 @@ func LoadService(name string, serviceDict map[string]interface{}, workingDir str
 		return nil, err
 	}
 
-	if err := resolveVolumePaths(serviceConfig.Volumes, workingDir, lookupEnv); err != nil {
+	if err := resolveVolumePaths(serviceConfig.Volumes, workingDir, homeDir); err != nil {
 		return nil, err
 	}
 	return serviceConfig, nil
@@ -402,7 +402,7 @@ func resolveEnvironment(serviceConfig *types.ServiceConfig, workingDir string, l
 	return nil
 }
 
-func resolveVolumePaths(volumes []types.ServiceVolumeConfig, workingDir string, lookupEnv template.Mapping) error {
+func resolveVolumePaths(volumes []types.ServiceVolumeConfig, workingDir, homeDir string) error {
 	for i, volume := range volumes {
 		if volume.Type != "bind" {
 			continue
@@ -412,32 +412,43 @@ func resolveVolumePaths(volumes []types.ServiceVolumeConfig, workingDir string,
 			return errors.New(`invalid mount config for type "bind": field Source must not be empty`)
 		}
 
-		filePath := expandUser(volume.Source, lookupEnv)
-		// Check for a Unix absolute path first, to handle a Windows client
-		// with a Unix daemon. This handles a Windows client connecting to a
-		// Unix daemon. Note that this is not required for Docker for Windows
-		// when specifying a local Windows path, because Docker for Windows
-		// translates the Windows path into a valid path within the VM.
-		if !path.IsAbs(filePath) {
-			filePath = absPath(workingDir, filePath)
+		filePath, err := transformFilePath(volume.Source, workingDir, homeDir)
+		if err != nil {
+			return err
 		}
+
 		volume.Source = filePath
 		volumes[i] = volume
 	}
 	return nil
 }
 
-// TODO: make this more robust
-func expandUser(path string, lookupEnv template.Mapping) string {
-	if strings.HasPrefix(path, "~") {
-		home, ok := lookupEnv("HOME")
-		if !ok {
-			logrus.Warn("cannot expand '~', because the environment lacks HOME")
-			return path
+func transformFilePath(origin string, workingDir, homeDir string) (string, error) {
+	path, err := crosspath.ParsePathWithDefaults(origin)
+	if err != nil {
+		return "", err
+	}
+	switch path.Kind() {
+	case crosspath.Relative:
+		basePath, err := crosspath.ParsePathWithDefaults(workingDir)
+		if err != nil {
+			return "", err
+		}
+		path, err = basePath.Join(path)
+		if err != nil {
+			return "", err
+		}
+	case crosspath.HomeRooted:
+		basePath, err := crosspath.ParsePathWithDefaults(homeDir)
+		if err != nil {
+			return "", err
+		}
+		path, err = basePath.Join(path)
+		if err != nil {
+			return "", err
 		}
-		return strings.Replace(path, "~", home, 1)
 	}
-	return path
+	return path.String(), nil
 }
 
 func transformUlimits(data interface{}) (interface{}, error) {

cli/compose/loader/loader_test.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/docker/cli/cli/compose/types"
+	"github.com/docker/docker/pkg/homedir"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/sirupsen/logrus"
 	"gotest.tools/assert"
@@ -28,6 +29,7 @@ func buildConfigDetails(source map[string]interface{}, env map[string]string) ty
 			{Filename: "filename.yml", Config: source},
 		},
 		Environment: env,
+		HomeDir:     homedir.Get(),
 	}
 }
 
@@ -812,15 +814,14 @@ func TestFullExample(t *testing.T) {
 	bytes, err := ioutil.ReadFile("full-example.yml")
 	assert.NilError(t, err)
 
-	homeDir := "/home/foo"
-	env := map[string]string{"HOME": homeDir, "QUX": "qux_from_environment"}
+	env := map[string]string{"QUX": "qux_from_environment"}
 	config, err := loadYAMLWithEnv(string(bytes), env)
 	assert.NilError(t, err)
 
 	workingDir, err := os.Getwd()
 	assert.NilError(t, err)
 
-	expectedConfig := fullExampleConfig(workingDir, homeDir)
+	expectedConfig := fullExampleConfig(workingDir, homedir.Get())
 
 	assert.Check(t, is.DeepEqual(expectedConfig.Services, config.Services))
 	assert.Check(t, is.DeepEqual(expectedConfig.Networks, config.Networks))
@@ -1285,6 +1286,7 @@ func TestLoadSecretsWarnOnDeprecatedExternalNameVersion35(t *testing.T) {
 	}
 	details := types.ConfigDetails{
 		Version: "3.5",
+		HomeDir: homedir.Get(),
 	}
 	secrets, err := LoadSecrets(source, details)
 	assert.NilError(t, err)
@@ -1395,3 +1397,79 @@ networks:
 	}
 	assert.DeepEqual(t, config, expected, cmpopts.EquateEmpty())
 }
+
+func TestFilePathsCrossPlat(t *testing.T) {
+	cases := []struct {
+		path       string
+		workingDir string
+		expected   string
+		homeDir    string
+	}{
+		{
+			path:       `/var/data`,
+			workingDir: `/working/dir`,
+			expected:   `/var/data`,
+		},
+		{
+			path:       `/var/data`,
+			workingDir: `c:\working\dir`,
+			expected:   `/var/data`,
+		},
+		{
+			path:       `relative/data`,
+			workingDir: `/working/dir`,
+			expected:   `/working/dir/relative/data`,
+		},
+		{
+			path:       `relative/data`,
+			workingDir: `c:\working\dir`,
+			expected:   `c:\working\dir\relative\data`,
+		},
+		{
+			path:       `c:\var\data`,
+			workingDir: `/working/dir`,
+			expected:   `c:\var\data`,
+		},
+		{
+			path:       `c:\var\data`,
+			workingDir: `c:\working\dir`,
+			expected:   `c:\var\data`,
+		},
+		{
+			path:       `relative\data`,
+			workingDir: `/working/dir`,
+			expected:   `/working/dir/relative/data`,
+		},
+		{
+			path:       `relative\data`,
+			workingDir: `c:\working\dir`,
+			expected:   `c:\working\dir\relative\data`,
+		},
+		{
+			path:     `~\homerooted\data`,
+			homeDir:  `c:\users\user`,
+			expected: `c:\users\user\homerooted\data`,
+		},
+		{
+			path:     `~\homerooted\data`,
+			homeDir:  `/home/user`,
+			expected: `/home/user/homerooted/data`,
+		},
+
+		{
+			path:     `~/homerooted/data`,
+			homeDir:  `c:\users\user`,
+			expected: `c:\users\user\homerooted\data`,
+		},
+		{
+			path:     `~/homerooted/data`,
+			homeDir:  `/home/user`,
+			expected: `/home/user/homerooted/data`,
+		},
+	}
+	for _, c := range cases {
+		result, err := transformFilePath(c.path, c.workingDir, c.homeDir)
+		assert.NilError(t, err)
+		assert.Equal(t, c.expected, result)
+	}
+}

cli/compose/types/types.go

@@ -58,6 +58,7 @@ type ConfigFile struct {
 type ConfigDetails struct {
 	Version     string
 	WorkingDir  string
+	HomeDir     string
 	ConfigFiles []ConfigFile
 	Environment map[string]string
 }