Repository: compose-spec/compose-go
Pull request #207 (2021-11-23 09:38:04)

Fix volume trailing slash
Follow-up for #206 with a better fix, including for all possible volumes (service secrets and configs).


Patch:
diff --git a/compatibility/services.go b/compatibility/services.go
index 39bca64..fc68dc2 100644
--- a/compatibility/services.go
+++ b/compatibility/services.go
@@ -169,10 +169,10 @@ func (c *AllowList) CheckConfigs(service *types.ServiceConfig) {
 			c.Unsupported("services.configs")
 			return
 		}
-		for i, s := range service.Secrets {
-			ref := types.FileReferenceConfig(s)
+		for i, conf := range service.Configs {
+			ref := types.FileReferenceConfig(conf)
 			c.CheckFileReference("configs", &ref)
-			service.Secrets[i] = s
+			service.Configs[i] = conf
 		}
 	}
 }
diff --git a/loader/loader.go b/loader/loader.go
index 159e9c7..b55b005 100644
--- a/loader/loader.go
+++ b/loader/loader.go
@@ -145,7 +145,7 @@ func Load(configDetails types.ConfigDetails, options ...func(*Options)) (*types.
 		op(opts)
 	}
 
-	configs := []*types.Config{}
+	var configs []*types.Config
 	for i, file := range configDetails.ConfigFiles {
 		configDict := file.Config
 		if configDict == nil {
@@ -222,14 +222,14 @@ func Load(configDetails types.ConfigDetails, options ...func(*Options)) (*types.
 }
 
 func parseConfig(b []byte, opts *Options) (map[string]interface{}, error) {
-	yaml, err := ParseYAML(b)
+	yml, err := ParseYAML(b)
 	if err != nil {
 		return nil, err
 	}
 	if !opts.SkipInterpolation {
-		return interp.Interpolate(yaml, *opts.Interpolate)
+		return interp.Interpolate(yml, *opts.Interpolate)
 	}
-	return yaml, err
+	return yml, err
 }
 
 func groupXFieldsIntoExtensions(dict map[string]interface{}) map[string]interface{} {
@@ -342,8 +342,8 @@ func createTransformHook(additionalTransformers ...Transformer) mapstructure.Dec
 		reflect.TypeOf(types.UlimitsConfig{}):                    transformUlimits,
 		reflect.TypeOf(types.UnitBytes(0)):                       transformSize,
 		reflect.TypeOf([]types.ServicePortConfig{}):              transformServicePort,
-		reflect.TypeOf(types.ServiceSecretConfig{}):              transformStringSourceMap,
-		reflect.TypeOf(types.ServiceConfigObjConfig{}):           transformStringSourceMap,
+		reflect.TypeOf(types.ServiceSecretConfig{}):              transformFileReferenceConfig,
+		reflect.TypeOf(types.ServiceConfigObjConfig{}):           transformFileReferenceConfig,
 		reflect.TypeOf(types.StringOrNumberList{}):               transformStringOrNumberList,
 		reflect.TypeOf(map[string]*types.ServiceNetworkConfig{}): transformServiceNetworkMap,
 		reflect.TypeOf(types.Mapping{}):                          transformMappingOrListFunc("=", false),
@@ -372,7 +372,7 @@ func createTransformHook(additionalTransformers ...Transformer) mapstructure.Dec
 	}
 }
 
-// keys needs to be converted to strings for jsonschema
+// keys need to be converted to strings for jsonschema
 func convertToStringKeysRecursive(value interface{}, keyPrefix string) (interface{}, error) {
 	if mapping, ok := value.(map[interface{}]interface{}); ok {
 		dict := make(map[string]interface{})
@@ -396,7 +396,7 @@ func convertToStringKeysRecursive(value interface{}, keyPrefix string) (interfac
 		return dict, nil
 	}
 	if list, ok := value.([]interface{}); ok {
-		convertedList := []interface{}{}
+		var convertedList []interface{}
 		for index, entry := range list {
 			newKeyPrefix := fmt.Sprintf("%s[%d]", keyPrefix, index)
 			convertedEntry, err := convertToStringKeysRecursive(entry, newKeyPrefix)
@@ -532,7 +532,7 @@ func LoadService(name string, serviceDict map[string]interface{}, workingDir str
 	}
 
 	for i, volume := range serviceConfig.Volumes {
-		if volume.Type != "bind" {
+		if volume.Type != types.VolumeTypeBind {
 			continue
 		}
 
@@ -552,8 +552,8 @@ func resolveEnvironment(serviceConfig *types.ServiceConfig, workingDir string, l
 	environment := types.MappingWithEquals{}
 
 	if len(serviceConfig.EnvFile) > 0 {
-		for _, file := range serviceConfig.EnvFile {
-			filePath := absPath(workingDir, file)
+		for _, envFile := range serviceConfig.EnvFile {
+			filePath := absPath(workingDir, envFile)
 			file, err := os.Open(filePath)
 			if err != nil {
 				return err
@@ -797,7 +797,7 @@ var transformServicePort TransformerFunc = func(data interface{}) (interface{},
 		// We process the list instead of individual items here.
 		// The reason is that one entry might be mapped to multiple ServicePortConfig.
 		// Therefore we take an input of a list and return an output of a list.
-		ports := []interface{}{}
+		var ports []interface{}
 		for _, entry := range entries {
 			switch value := entry.(type) {
 			case int:
@@ -852,17 +852,27 @@ var transformServiceDeviceRequest TransformerFunc = func(data interface{}) (inte
 	}
 }
 
-var transformStringSourceMap TransformerFunc = func(data interface{}) (interface{}, error) {
+var transformFileReferenceConfig TransformerFunc = func(data interface{}) (interface{}, error) {
 	switch value := data.(type) {
 	case string:
 		return map[string]interface{}{"source": value}, nil
 	case map[string]interface{}:
-		return groupXFieldsIntoExtensions(data.(map[string]interface{})), nil
+		if target, ok := value["target"]; ok {
+			value["target"] = cleanTarget(target.(string))
+		}
+		return groupXFieldsIntoExtensions(value), nil
 	default:
 		return data, errors.Errorf("invalid type %T for secret", value)
 	}
 }
 
+func cleanTarget(target string) string {
+	if target == "" {
+		return ""
+	}
+	return path.Clean(target)
+}
+
 var transformBuildConfig TransformerFunc = func(data interface{}) (interface{}, error) {
 	switch value := data.(type) {
 	case string:
@@ -906,9 +916,15 @@ var transformExtendsConfig TransformerFunc = func(data interface{}) (interface{}
 var transformServiceVolumeConfig TransformerFunc = func(data interface{}) (interface{}, error) {
 	switch value := data.(type) {
 	case string:
-		return ParseVolume(value)
+		volume, err := ParseVolume(value)
+		volume.Target = cleanTarget(volume.Target)
+		return volume, err
 	case map[string]interface{}:
-		return groupXFieldsIntoExtensions(data.(map[string]interface{})), nil
+		data := groupXFieldsIntoExtensions(data.(map[string]interface{}))
+		if target, ok := data["target"]; ok {
+			data["target"] = cleanTarget(target.(string))
+		}
+		return data, nil
 	default:
 		return data, errors.Errorf("invalid type %T for service volume", value)
 	}
@@ -971,7 +987,7 @@ func transformMappingOrList(mappingOrList interface{}, sep string, allowNil bool
 	switch value := mappingOrList.(type) {
 	case map[string]interface{}:
 		return toMapStringString(value, allowNil)
-	case ([]interface{}):
+	case []interface{}:
 		result := make(map[string]interface{})
 		for _, value := range value {
 			parts := strings.SplitN(value.(string), sep, 2)
@@ -1054,7 +1070,7 @@ func toString(value interface{}, allowNil bool) interface{} {
 }
 
 func toStringList(value map[string]interface{}, separator string, allowNil bool) []string {
-	output := []string{}
+	var output []string
 	for key, value := range value {
 		if value == nil && !allowNil {
 			continue
diff --git a/loader/merge.go b/loader/merge.go
index 40405e3..b01c868 100644
--- a/loader/merge.go
+++ b/loader/merge.go
@@ -184,7 +184,7 @@ func toServiceVolumeConfigsMap(s interface{}) (map[interface{}]interface{}, erro
 }
 
 func toServiceSecretConfigsSlice(dst reflect.Value, m map[interface{}]interface{}) error {
-	s := []types.ServiceSecretConfig{}
+	var s []types.ServiceSecretConfig
 	for _, v := range m {
 		s = append(s, v.(types.ServiceSecretConfig))
 	}
@@ -194,7 +194,7 @@ func toServiceSecretConfigsSlice(dst reflect.Value, m map[interface{}]interface{
 }
 
 func toSServiceConfigObjConfigsSlice(dst reflect.Value, m map[interface{}]interface{}) error {
-	s := []types.ServiceConfigObjConfig{}
+	var s []types.ServiceConfigObjConfig
 	for _, v := range m {
 		s = append(s, v.(types.ServiceConfigObjConfig))
 	}
@@ -204,7 +204,7 @@ func toSServiceConfigObjConfigsSlice(dst reflect.Value, m map[interface{}]interf
 }
 
 func toServicePortConfigsSlice(dst reflect.Value, m map[interface{}]interface{}) error {
-	s := []types.ServicePortConfig{}
+	var s []types.ServicePortConfig
 	for _, v := range m {
 		s = append(s, v.(types.ServicePortConfig))
 	}
@@ -225,7 +225,7 @@ func toServicePortConfigsSlice(dst reflect.Value, m map[interface{}]interface{})
 }
 
 func toServiceVolumeConfigsSlice(dst reflect.Value, m map[interface{}]interface{}) error {
-	s := []types.ServiceVolumeConfig{}
+	var s []types.ServiceVolumeConfig
 	for _, v := range m {
 		s = append(s, v.(types.ServiceVolumeConfig))
 	}
@@ -234,7 +234,7 @@ func toServiceVolumeConfigsSlice(dst reflect.Value, m map[interface{}]interface{
 	return nil
 }
 
-type tomapFn func(s interface{}) (map[interface{}]interface{}, error)
+type toMapFn func(s interface{}) (map[interface{}]interface{}, error)
 type writeValueFromMapFn func(reflect.Value, map[interface{}]interface{}) error
 
 func safelyMerge(mergeFn func(dst, src reflect.Value) error) func(dst, src reflect.Value) error {
@@ -250,13 +250,13 @@ func safelyMerge(mergeFn func(dst, src reflect.Value) error) func(dst, src refle
 	}
 }
 
-func mergeSlice(tomap tomapFn, writeValue writeValueFromMapFn) func(dst, src reflect.Value) error {
+func mergeSlice(toMap toMapFn, writeValue writeValueFromMapFn) func(dst, src reflect.Value) error {
 	return func(dst, src reflect.Value) error {
-		dstMap, err := sliceToMap(tomap, dst)
+		dstMap, err := sliceToMap(toMap, dst)
 		if err != nil {
 			return err
 		}
-		srcMap, err := sliceToMap(tomap, src)
+		srcMap, err := sliceToMap(toMap, src)
 		if err != nil {
 			return err
 		}
@@ -267,12 +267,12 @@ func mergeSlice(tomap tomapFn, writeValue writeValueFromMapFn) func(dst, src ref
 	}
 }
 
-func sliceToMap(tomap tomapFn, v reflect.Value) (map[interface{}]interface{}, error) {
+func sliceToMap(toMap toMapFn, v reflect.Value) (map[interface{}]interface{}, error) {
 	// check if valid
 	if !v.IsValid() {
 		return nil, errors.Errorf("invalid value : %+v", v)
 	}
-	return tomap(v.Interface())
+	return toMap(v.Interface())
 }
 
 func mergeLoggingConfig(dst, src reflect.Value) error {
diff --git a/loader/volume.go b/loader/volume.go
index 0162aee..dd0f612 100644
--- a/loader/volume.go
+++ b/loader/volume.go
@@ -17,7 +17,6 @@
 package loader
 
 import (
-	"path"
 	"strings"
 	"unicode"
 	"unicode/utf8"
@@ -57,7 +56,6 @@ func ParseVolume(spec string) (types.ServiceVolumeConfig, error) {
 		}
 	}
 
-	volume.Target = path.Clean(volume.Target)
 	populateType(&volume)
 	return volume, nil
 }
diff --git a/types/project.go b/types/project.go
index 155fad0..6de8e61 100644
--- a/types/project.go
+++ b/types/project.go
@@ -46,7 +46,7 @@ type Project struct {
 
 // ServiceNames return names for all services in this Compose config
 func (p Project) ServiceNames() []string {
-	names := []string{}
+	var names []string
 	for _, s := range p.Services {
 		names = append(names, s.Name)
 	}
@@ -56,7 +56,7 @@ func (p Project) ServiceNames() []string {
 
 // VolumeNames return names for all volumes in this Compose config
 func (p Project) VolumeNames() []string {
-	names := []string{}
+	var names []string
 	for k := range p.Volumes {
 		names = append(names, k)
 	}
@@ -66,7 +66,7 @@ func (p Project) VolumeNames() []string {
 
 // NetworkNames return names for all volumes in this Compose config
 func (p Project) NetworkNames() []string {
-	names := []string{}
+	var names []string
 	for k := range p.Networks {
 		names = append(names, k)
 	}
@@ -76,7 +76,7 @@ func (p Project) NetworkNames() []string {
 
 // SecretNames return names for all secrets in this Compose config
 func (p Project) SecretNames() []string {
-	names := []string{}
+	var names []string
 	for k := range p.Secrets {
 		names = append(names, k)
 	}
@@ -86,7 +86,7 @@ func (p Project) SecretNames() []string {
 
 // ConfigNames return names for all configs in this Compose config
 func (p Project) ConfigNames() []string {
-	names := []string{}
+	var names []string
 	for k := range p.Configs {
 		names = append(names, k)
 	}
@@ -179,12 +179,12 @@ func (p *Project) RelativePath(path string) string {
 }
 
 // HasProfile return true if service has no profile declared or has at least one profile matching
-func (service ServiceConfig) HasProfile(profiles []string) bool {
-	if len(service.Profiles) == 0 {
+func (s ServiceConfig) HasProfile(profiles []string) bool {
+	if len(s.Profiles) == 0 {
 		return true
 	}
 	for _, p := range profiles {
-		for _, sp := range service.Profiles {
+		for _, sp := range s.Profiles {
 			if sp == p {
 				return true
 			}
@@ -327,7 +327,6 @@ func (p *Project) ResolveImages(resolver func(named reference.Named) (digest.Dig
 				if err != nil {
 					return err
 				}
-
 				named, err = reference.WithDigest(named, digest)
 				if err != nil {
 					return err
diff --git a/types/types.go b/types/types.go
index 8250415..c19d0eb 100644
--- a/types/types.go
+++ b/types/types.go
@@ -33,7 +33,7 @@ func (d Duration) String() string {
 	return time.Duration(d).String()
 }
 
-// ConvertDurationPtr converts a typedefined Duration pointer to a time.Duration pointer with the same value.
+// ConvertDurationPtr converts a type defined Duration pointer to a time.Duration pointer with the same value.
 func ConvertDurationPtr(d *Duration) *time.Duration {
 	if d == nil {
 		return nil
@@ -208,7 +208,7 @@ const (
 	PullPolicyNever = "never"
 	//PullPolicyIfNotPresent pull missing images
 	PullPolicyIfNotPresent = "if_not_present"
-	//PullPolicyIfNotPresent pull missing images
+	//PullPolicyMissing pull missing images
 	PullPolicyMissing = "missing"
 	//PullPolicyBuild force building images
 	PullPolicyBuild = "build"
@@ -611,7 +611,7 @@ func ParsePortConfig(value string) ([]ServicePortConfig, error) {
 }
 
 func convertPortToPortConfig(port nat.Port, portBindings map[nat.Port][]nat.PortBinding) ([]ServicePortConfig, error) {
-	portConfigs := []ServicePortConfig{}
+	var portConfigs []ServicePortConfig
 	for _, binding := range portBindings[port] {
 		startHostPort, endHostPort, err := nat.ParsePortRange(binding.HostPort)
 
@@ -647,13 +647,13 @@ type ServiceVolumeConfig struct {
 }
 
 const (
-	// TypeBind is the type for mounting host dir
+	// VolumeTypeBind is the type for mounting host dir
 	VolumeTypeBind = "bind"
-	// TypeVolume is the type for remote storage volumes
+	// VolumeTypeVolume is the type for remote storage volumes
 	VolumeTypeVolume = "volume"
-	// TypeTmpfs is the type for mounting tmpfs
+	// VolumeTypeTmpfs is the type for mounting tmpfs
 	VolumeTypeTmpfs = "tmpfs"
-	// TypeNamedPipe is the type for mounting Windows named pipes
+	// VolumeTypeNamedPipe is the type for mounting Windows named pipes
 	VolumeTypeNamedPipe = "npipe"
 )
 


Test patch:
diff --git a/loader/loader_test.go b/loader/loader_test.go
index ead2e17..8abf0c9 100644
--- a/loader/loader_test.go
+++ b/loader/loader_test.go
@@ -1812,7 +1812,7 @@ services:
 	assert.NilError(t, err)
 }
 
-func TestLoadService(t *testing.T) {
+func TestLoadServiceWithEnvFile(t *testing.T) {
 	file, err := os.CreateTemp("", "test-compose-go")
 	assert.NilError(t, err)
 	defer os.Remove(file.Name())
@@ -1830,3 +1830,33 @@ func TestLoadService(t *testing.T) {
 	assert.NilError(t, err)
 	assert.Equal(t, "YES", *s.Environment["HALLO"])
 }
+
+func TestLoadServiceWithVolumes(t *testing.T) {
+	m := map[string]interface{}{
+		"volumes": []interface{}{
+			"source:/path 1/",
+			map[string]interface{}{
+				"target": "/path 2/",
+			},
+		},
+		"configs": []interface{}{
+			map[string]interface{}{
+				"target": "/path 3/",
+			},
+		},
+		"secrets": []interface{}{
+			map[string]interface{}{
+				"target": "/path 4/",
+			},
+		},
+	}
+	s, err := LoadService("Test Name", m, ".", nil, true)
+	assert.NilError(t, err)
+	assert.Equal(t, len(s.Volumes), 2)
+	assert.Equal(t, "/path 1", s.Volumes[0].Target)
+	assert.Equal(t, "/path 2", s.Volumes[1].Target)
+	assert.Equal(t, len(s.Configs), 1)
+	assert.Equal(t, "/path 3", s.Configs[0].Target)
+	assert.Equal(t, len(s.Secrets), 1)
+	assert.Equal(t, "/path 4", s.Secrets[0].Target)
+}
diff --git a/loader/volume_test.go b/loader/volume_test.go
index cad2bb0..ca41867 100644
--- a/loader/volume_test.go
+++ b/loader/volume_test.go
@@ -34,13 +34,6 @@ func TestParseVolumeAnonymousVolume(t *testing.T) {
 	}
 }
 
-func TestParseVolumeCleanTarget(t *testing.T) {
-	volume, err := ParseVolume("/path/")
-	expected := types.ServiceVolumeConfig{Type: "volume", Target: "/path", Volume: &types.ServiceVolumeVolume{}}
-	assert.NilError(t, err)
-	assert.Check(t, is.DeepEqual(expected, volume))
-}
-
 func TestParseVolumeAnonymousVolumeWindows(t *testing.T) {
 	for _, path := range []string{"C:\\path", "Z:\\path\\foo"} {
 		volume, err := ParseVolume(path)