Merge pull request #465 from ndeloof/decode_mapstructure

introduce DecodeMapstructure to allow type to define custom decode logic

Author: ndeloof

loader/interpolate.go

@@ -46,10 +46,6 @@ var interpolateTypeCastMapping = map[tree.Path]interp.Cast{
 	servicePath("deploy", "placement", "max_replicas_per_node"):    toInt,
 	servicePath("healthcheck", "retries"):                          toInt,
 	servicePath("healthcheck", "disable"):                          toBoolean,
-	servicePath("mem_limit"):                                       toUnitBytes,
-	servicePath("mem_reservation"):                                 toUnitBytes,
-	servicePath("memswap_limit"):                                   toUnitBytes,
-	servicePath("mem_swappiness"):                                  toUnitBytes,
 	servicePath("oom_kill_disable"):                                toBoolean,
 	servicePath("oom_score_adj"):                                   toInt64,
 	servicePath("pids_limit"):                                      toInt64,
@@ -58,16 +54,13 @@ var interpolateTypeCastMapping = map[tree.Path]interp.Cast{
 	servicePath("read_only"):                                       toBoolean,
 	servicePath("scale"):                                           toInt,
 	servicePath("secrets", tree.PathMatchList, "mode"):             toInt,
-	servicePath("shm_size"):                                        toUnitBytes,
 	servicePath("stdin_open"):                                      toBoolean,
-	servicePath("stop_grace_period"):                               toDuration,
 	servicePath("tty"):                                             toBoolean,
 	servicePath("ulimits", tree.PathMatchAll):                      toInt,
 	servicePath("ulimits", tree.PathMatchAll, "hard"):              toInt,
 	servicePath("ulimits", tree.PathMatchAll, "soft"):              toInt,
 	servicePath("volumes", tree.PathMatchList, "read_only"):        toBoolean,
 	servicePath("volumes", tree.PathMatchList, "volume", "nocopy"): toBoolean,
-	servicePath("volumes", tree.PathMatchList, "tmpfs", "size"):    toUnitBytes,
 	iPath("networks", tree.PathMatchAll, "external"):               toBoolean,
 	iPath("networks", tree.PathMatchAll, "internal"):               toBoolean,
 	iPath("networks", tree.PathMatchAll, "attachable"):             toBoolean,
@@ -93,14 +86,6 @@ func toInt64(value string) (interface{}, error) {
 	return strconv.ParseInt(value, 10, 64)
 }
 
-func toUnitBytes(value string) (interface{}, error) {
-	return transformSize(value)
-}
-
-func toDuration(value string) (interface{}, error) {
-	return transformStringToDuration(value)
-}
-
 func toFloat(value string) (interface{}, error) {
 	return strconv.ParseFloat(value, 64)
 }

loader/loader.go

@@ -28,15 +28,12 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
-	"time"
 
 	"github.com/compose-spec/compose-go/consts"
 	interp "github.com/compose-spec/compose-go/interpolation"
 	"github.com/compose-spec/compose-go/schema"
 	"github.com/compose-spec/compose-go/template"
 	"github.com/compose-spec/compose-go/types"
-	"github.com/docker/go-units"
-	"github.com/mattn/go-shellwords"
 	"github.com/mitchellh/mapstructure"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -586,7 +583,7 @@ func Transform(source interface{}, target interface{}, additionalTransformers ..
 	config := &mapstructure.DecoderConfig{
 		DecodeHook: mapstructure.ComposeDecodeHookFunc(
 			createTransformHook(additionalTransformers...),
-			mapstructure.StringToTimeDurationHookFunc()),
+			decoderHook),
 		Result:   target,
 		TagName:  "yaml",
 		Metadata: &data,
@@ -610,28 +607,20 @@ type Transformer struct {
 func createTransformHook(additionalTransformers ...Transformer) mapstructure.DecodeHookFuncType {
 	transforms := map[reflect.Type]func(interface{}) (interface{}, error){
 		reflect.TypeOf(types.External{}):                         transformExternal,
-		reflect.TypeOf(types.HealthCheckTest{}):                  transformHealthCheckTest,
-		reflect.TypeOf(types.ShellCommand{}):                     transformShellCommand,
-		reflect.TypeOf(types.StringList{}):                       transformStringList,
-		reflect.TypeOf(types.Options{}):                          transformMapStringString,
+		reflect.TypeOf(types.Options{}):                          transformOptions,
 		reflect.TypeOf(types.UlimitsConfig{}):                    transformUlimits,
-		reflect.TypeOf(types.UnitBytes(0)):                       transformSize,
 		reflect.TypeOf([]types.ServicePortConfig{}):              transformServicePort,
 		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),
 		reflect.TypeOf(types.MappingWithEquals{}):                transformMappingOrListFunc("=", true),
-		reflect.TypeOf(types.Labels{}):                           transformMappingOrListFunc("=", false),
 		reflect.TypeOf(types.MappingWithColon{}):                 transformMappingOrListFunc(":", false),
 		reflect.TypeOf(types.HostsList{}):                        transformMappingOrListFunc(":", false),
 		reflect.TypeOf(types.ServiceVolumeConfig{}):              transformServiceVolumeConfig,
 		reflect.TypeOf(types.BuildConfig{}):                      transformBuildConfig,
-		reflect.TypeOf(types.Duration(0)):                        transformStringToDuration,
 		reflect.TypeOf(types.DependsOnConfig{}):                  transformDependsOnConfig,
 		reflect.TypeOf(types.ExtendsConfig{}):                    transformExtendsConfig,
-		reflect.TypeOf(types.DeviceRequest{}):                    transformServiceDeviceRequest,
 		reflect.TypeOf(types.SSHConfig{}):                        transformSSHConfig,
 		reflect.TypeOf(types.IncludeConfig{}):                    transformIncludeConfig,
 	}
@@ -1031,7 +1020,7 @@ func loadFileObjectConfig(name string, objType string, obj types.FileObjectConfi
 	return obj, nil
 }
 
-var transformMapStringString TransformerFunc = func(data interface{}) (interface{}, error) {
+var transformOptions TransformerFunc = func(data interface{}) (interface{}, error) {
 	switch value := data.(type) {
 	case map[string]interface{}:
 		return toMapStringString(value, false), nil
@@ -1094,35 +1083,6 @@ var transformServicePort TransformerFunc = func(data interface{}) (interface{},
 	}
 }
 
-var transformServiceDeviceRequest TransformerFunc = func(data interface{}) (interface{}, error) {
-	switch value := data.(type) {
-	case map[string]interface{}:
-		count, ok := value["count"]
-		if ok {
-			switch val := count.(type) {
-			case int:
-				return value, nil
-			case string:
-				if strings.ToLower(val) == "all" {
-					value["count"] = -1
-					return value, nil
-				}
-				i, err := strconv.ParseInt(val, 10, 64)
-				if err == nil {
-					value["count"] = i
-					return value, nil
-				}
-				return data, errors.Errorf("invalid string value for 'count' (the only value allowed is 'all' or a number)")
-			default:
-				return data, errors.Errorf("invalid type %T for device count", val)
-			}
-		}
-		return data, nil
-	default:
-		return data, errors.Errorf("invalid type %T for resource reservation", value)
-	}
-}
-
 var transformFileReferenceConfig TransformerFunc = func(data interface{}) (interface{}, error) {
 	switch value := data.(type) {
 	case string:
@@ -1258,26 +1218,6 @@ func ParseShortSSHSyntax(value string) ([]types.SSHKey, error) {
 	return result, nil
 }
 
-var transformStringOrNumberList TransformerFunc = func(value interface{}) (interface{}, error) {
-	list := value.([]interface{})
-	result := make([]string, len(list))
-	for i, item := range list {
-		result[i] = fmt.Sprint(item)
-	}
-	return result, nil
-}
-
-var transformStringList TransformerFunc = func(data interface{}) (interface{}, error) {
-	switch value := data.(type) {
-	case string:
-		return []string{value}, nil
-	case []interface{}:
-		return value, nil
-	default:
-		return data, errors.Errorf("invalid type %T for string list", value)
-	}
-}
-
 func transformMappingOrListFunc(sep string, allowNil bool) TransformerFunc {
 	return func(data interface{}) (interface{}, error) {
 		return transformMappingOrList(data, sep, allowNil)
@@ -1312,52 +1252,6 @@ func transformValueToMapEntry(value string, separator string, allowNil bool) (st
 	}
 }
 
-var transformShellCommand TransformerFunc = func(value interface{}) (interface{}, error) {
-	if str, ok := value.(string); ok {
-		return shellwords.Parse(str)
-	}
-	return value, nil
-}
-
-var transformHealthCheckTest TransformerFunc = func(data interface{}) (interface{}, error) {
-	switch value := data.(type) {
-	case string:
-		return append([]string{"CMD-SHELL"}, value), nil
-	case []interface{}:
-		return value, nil
-	default:
-		return value, errors.Errorf("invalid type %T for healthcheck.test", value)
-	}
-}
-
-var transformSize TransformerFunc = func(value interface{}) (interface{}, error) {
-	switch value := value.(type) {
-	case int:
-		return int64(value), nil
-	case int64, types.UnitBytes:
-		return value, nil
-	case string:
-		return units.RAMInBytes(value)
-	default:
-		return value, errors.Errorf("invalid type for size %T", value)
-	}
-}
-
-var transformStringToDuration TransformerFunc = func(value interface{}) (interface{}, error) {
-	switch value := value.(type) {
-	case string:
-		d, err := time.ParseDuration(value)
-		if err != nil {
-			return value, err
-		}
-		return types.Duration(d), nil
-	case types.Duration:
-		return value, nil
-	default:
-		return value, errors.Errorf("invalid type %T for duration", value)
-	}
-}
-
 func toMapStringString(value map[string]interface{}, allowNil bool) map[string]interface{} {
 	output := make(map[string]interface{})
 	for key, value := range value {

loader/loader_test.go

@@ -2129,9 +2129,9 @@ services:
           devices:
             - driver: nvidia
               capabilities: [gpu]
-              count: somestring
+              count: some_string
 `)
-	assert.ErrorContains(t, err, "invalid string value for 'count' (the only value allowed is 'all' or a number)")
+	assert.ErrorContains(t, err, `invalid value "some_string", the only value allowed is 'all' or a number`)
 }
 
 func TestServicePullPolicy(t *testing.T) {

loader/mapstructure.go

@@ -0,0 +1,53 @@
+/*
+   Copyright 2020 The Compose Specification Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package loader
+
+import "reflect"
+
+// comparable to yaml.Unmarshaler, decoder allow a type to define it's own custom logic to convert value
+// see https://github.com/mitchellh/mapstructure/pull/294
+type decoder interface {
+	DecodeMapstructure(interface{}) error
+}
+
+// see https://github.com/mitchellh/mapstructure/issues/115#issuecomment-735287466
+// adapted to support types derived from built-in types, as DecodeMapstructure would not be able to mutate internal
+// value, so need to invoke DecodeMapstructure defined by pointer to type
+func decoderHook(from reflect.Value, to reflect.Value) (interface{}, error) {
+	// If the destination implements the decoder interface
+	u, ok := to.Interface().(decoder)
+	if !ok {
+		// for non-struct types we need to invoke func (*type) DecodeMapstructure()
+		if to.CanAddr() {
+			pto := to.Addr()
+			u, ok = pto.Interface().(decoder)
+		}
+		if !ok {
+			return from.Interface(), nil
+		}
+	}
+	// If it is nil and a pointer, create and assign the target value first
+	if to.Type().Kind() == reflect.Ptr && to.IsNil() {
+		to.Set(reflect.New(to.Type().Elem()))
+		u = to.Interface().(decoder)
+	}
+	// Call the custom DecodeMapstructure method
+	if err := u.DecodeMapstructure(from.Interface()); err != nil {
+		return to.Interface(), err
+	}
+	return to.Interface(), nil
+}

loader/mapstructure_test.go

@@ -0,0 +1,65 @@
+/*
+   Copyright 2020 The Compose Specification Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package loader
+
+import (
+	"testing"
+
+	"github.com/compose-spec/compose-go/types"
+	"github.com/mitchellh/mapstructure"
+	"gotest.tools/v3/assert"
+)
+
+func TestDecodeMapStructure(t *testing.T) {
+	var target types.ServiceConfig
+	data := mapstructure.Metadata{}
+	config := &mapstructure.DecoderConfig{
+		Result:     &target,
+		TagName:    "yaml",
+		Metadata:   &data,
+		DecodeHook: mapstructure.ComposeDecodeHookFunc(decoderHook),
+	}
+	decoder, err := mapstructure.NewDecoder(config)
+	assert.NilError(t, err)
+	err = decoder.Decode(map[string]interface{}{
+		"mem_limit":         "640k",
+		"command":           "echo hello",
+		"stop_grace_period": "60s",
+		"labels": []interface{}{
+			"FOO=BAR",
+		},
+		"deploy": map[string]interface{}{
+			"labels": map[string]interface{}{
+				"FOO": "BAR",
+				"BAZ": nil,
+				"QIX": 2,
+				"ZOT": true,
+			},
+		},
+	})
+	assert.NilError(t, err)
+	assert.Equal(t, target.MemLimit, types.UnitBytes(640*1024))
+	assert.DeepEqual(t, target.Command, types.ShellCommand{"echo", "hello"})
+	assert.Equal(t, *target.StopGracePeriod, types.Duration(60_000_000_000))
+	assert.DeepEqual(t, target.Labels, types.Labels{"FOO": "BAR"})
+	assert.DeepEqual(t, target.Deploy.Labels, types.Labels{
+		"FOO": "BAR",
+		"BAZ": "",
+		"QIX": "2",
+		"ZOT": "true",
+	})
+}

types/bytes.go

@@ -0,0 +1,42 @@
+/*
+   Copyright 2020 The Compose Specification Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package types
+
+import (
+	"fmt"
+
+	"github.com/docker/go-units"
+)
+
+// UnitBytes is the bytes type
+type UnitBytes int64
+
+// MarshalYAML makes UnitBytes implement yaml.Marshaller
+func (u UnitBytes) MarshalYAML() (interface{}, error) {
+	return fmt.Sprintf("%d", u), nil
+}
+
+// MarshalJSON makes UnitBytes implement json.Marshaler
+func (u UnitBytes) MarshalJSON() ([]byte, error) {
+	return []byte(fmt.Sprintf(`"%d"`, u)), nil
+}
+
+func (u *UnitBytes) DecodeMapstructure(value interface{}) error {
+	v, err := units.RAMInBytes(fmt.Sprint(value))
+	*u = UnitBytes(v)
+	return err
+}