Fix load order

Signed-off-by: Daniel Nephin <dnephin@docker.com>
2017-10-04 16:51:48 -04:00 · 2017-10-04 16:51:48 -04:00 · af8f563922
parent 18ddec447a
commit af8f563922
5 changed files with 209 additions and 148 deletions
--- a/cli/compose/interpolation/interpolation.go
+++ b/cli/compose/interpolation/interpolation.go
@ -2,7 +2,6 @@ package interpolation

 import (
 	"os"
-
 	"strings"

 	"github.com/docker/cli/cli/compose/template"
@ -11,8 +10,6 @@ import (

 // Options supported by Interpolate
 type Options struct {
-	// SectionName of the configuration section
-	SectionName string
 	// LookupValue from a key
 	LookupValue LookupValue
 	// TypeCastMapping maps key paths to functions to cast to a type
@ -30,8 +27,6 @@ type Cast func(value string) (interface{}, error)

 // Interpolate replaces variables in a string with the values from a mapping
 func Interpolate(config map[string]interface{}, opts Options) (map[string]interface{}, error) {
-	out := map[string]interface{}{}
-
 	if opts.LookupValue == nil {
 		opts.LookupValue = os.LookupEnv
 	}
@ -39,43 +34,12 @@ func Interpolate(config map[string]interface{}, opts Options) (map[string]interf
 		opts.TypeCastMapping = make(map[Path]Cast)
 	}

-	for key, item := range config {
-		if item == nil {
-			out[key] = nil
-			continue
-		}
-		mapItem, ok := item.(map[string]interface{})
-		if !ok {
-			return nil, errors.Errorf("Invalid type for %s : %T instead of %T", key, item, out)
-		}
-		interpolatedItem, err := interpolateSectionItem(NewPath(key), mapItem, opts)
-		if err != nil {
-			return nil, err
-		}
-		out[key] = interpolatedItem
-	}
-
-	return out, nil
-}
-
-func interpolateSectionItem(
-	path Path,
-	item map[string]interface{},
-	opts Options,
-) (map[string]interface{}, error) {
 	out := map[string]interface{}{}

-	for key, value := range item {
-		interpolatedValue, err := recursiveInterpolate(value, path.Next(key), opts)
-		switch err := err.(type) {
-		case nil:
-		case *template.InvalidTemplateError:
-			return nil, errors.Errorf(
-				"Invalid interpolation format for %#v option in %s %#v: %#v. You may need to escape any $ with another $.",
-				key, opts.SectionName, path.root(), err.Template,
-			)
-		default:
-			return nil, errors.Wrapf(err, "error while interpolating %s in %s %s", key, opts.SectionName, path.root())
+	for key, value := range config {
+		interpolatedValue, err := recursiveInterpolate(value, NewPath(key), opts)
+		if err != nil {
+			return out, err
 		}
 		out[key] = interpolatedValue
 	}
@ -89,13 +53,14 @@ func recursiveInterpolate(value interface{}, path Path, opts Options) (interface
 	case string:
 		newValue, err := template.Substitute(value, template.Mapping(opts.LookupValue))
 		if err != nil || newValue == value {
-			return value, err
+			return value, newPathError(path, err)
 		}
 		caster, ok := opts.getCasterForPath(path)
 		if !ok {
 			return newValue, nil
 		}
-		return caster(newValue)
+		casted, err := caster(newValue)
+		return casted, newPathError(path, errors.Wrap(err, "failed to cast to expected type"))

 	case map[string]interface{}:
 		out := map[string]interface{}{}
@ -111,7 +76,7 @@ func recursiveInterpolate(value interface{}, path Path, opts Options) (interface
 	case []interface{}:
 		out := make([]interface{}, len(value))
 		for i, elem := range value {
-			interpolatedElem, err := recursiveInterpolate(elem, path, opts)
+			interpolatedElem, err := recursiveInterpolate(elem, path.Next(PathMatchList), opts)
 			if err != nil {
 				return nil, err
 			}
@ -125,12 +90,28 @@ func recursiveInterpolate(value interface{}, path Path, opts Options) (interface
 	}
 }

+func newPathError(path Path, err error) error {
+	switch err := err.(type) {
+	case nil:
+		return nil
+	case *template.InvalidTemplateError:
+		return errors.Errorf(
+			"invalid interpolation format for %s: %#v. You may need to escape any $ with another $.",
+			path, err.Template)
+	default:
+		return errors.Wrapf(err, "error while interpolating %s", path)
+	}
+}
+
 const pathSeparator = "."

 // PathMatchAll is a token used as part of a Path to match any key at that level
 // in the nested structure
 const PathMatchAll = "*"

+// PathMatchList is a token used as part of a Path to match items in a list
+const PathMatchList = "[]"
+
 // Path is a dotted path of keys to a value in a nested mapping structure. A *
 // section in a path will match any key in the mapping structure.
 type Path string
@ -145,14 +126,6 @@ func (p Path) Next(part string) Path {
 	return Path(string(p) + pathSeparator + part)
 }

-func (p Path) root() string {
-	parts := p.parts()
-	if len(parts) == 0 {
-		return ""
-	}
-	return parts[0]
-}
-
 func (p Path) parts() []string {
 	return strings.Split(string(p), pathSeparator)
 }
--- a/cli/compose/interpolation/interpolation_test.go
+++ b/cli/compose/interpolation/interpolation_test.go
@ -45,10 +45,7 @@ func TestInterpolate(t *testing.T) {
 			},
 		},
 	}
-	result, err := Interpolate(services, Options{
-		SectionName: "service",
-		LookupValue: defaultMapping,
-	})
+	result, err := Interpolate(services, Options{LookupValue: defaultMapping})
 	assert.NoError(t, err)
 	assert.Equal(t, expected, result)
 }
@ -59,11 +56,8 @@ func TestInvalidInterpolation(t *testing.T) {
 			"image": "${",
 		},
 	}
-	_, err := Interpolate(services, Options{
-		SectionName: "service",
-		LookupValue: defaultMapping,
-	})
-	assert.EqualError(t, err, `Invalid interpolation format for "image" option in service "servicea": "${". You may need to escape any $ with another $.`)
+	_, err := Interpolate(services, Options{LookupValue: defaultMapping})
+	assert.EqualError(t, err, `invalid interpolation format for servicea.image: "${". You may need to escape any $ with another $.`)
 }

 func TestInterpolateWithDefaults(t *testing.T) {
@ -95,7 +89,7 @@ func TestInterpolateWithCast(t *testing.T) {
 	}
 	result, err := Interpolate(config, Options{
 		LookupValue:     defaultMapping,
-		TypeCastMapping: map[Path]Cast{NewPath("*", "replicas"): toInt},
+		TypeCastMapping: map[Path]Cast{NewPath(PathMatchAll, "replicas"): toInt},
 	})
 	assert.NoError(t, err)
 	expected := map[string]interface{}{
--- a/cli/compose/loader/interpolate.go
+++ b/cli/compose/loader/interpolate.go
@ -8,46 +8,40 @@ import (
 	"github.com/pkg/errors"
 )

-var interpolateTypeCastMapping = map[string]map[interp.Path]interp.Cast{
-	"services": {
-		iPath("configs", "mode"):                              toInt,
-		iPath("secrets", "mode"):                              toInt,
-		iPath("healthcheck", "retries"):                       toInt,
-		iPath("healthcheck", "disable"):                       toBoolean,
-		iPath("deploy", "replicas"):                           toInt,
-		iPath("deploy", "update_config", "parallelism:"):      toInt,
-		iPath("deploy", "update_config", "max_failure_ratio"): toFloat,
-		iPath("deploy", "restart_policy", "max_attempts"):     toInt,
-		iPath("ports", "target"):                              toInt,
-		iPath("ports", "published"):                           toInt,
-		iPath("ulimits", interp.PathMatchAll):                 toInt,
-		iPath("ulimits", interp.PathMatchAll, "hard"):         toInt,
-		iPath("ulimits", interp.PathMatchAll, "soft"):         toInt,
-		iPath("privileged"):                                   toBoolean,
-		iPath("read_only"):                                    toBoolean,
-		iPath("stdin_open"):                                   toBoolean,
-		iPath("tty"):                                          toBoolean,
-		iPath("volumes", "read_only"):                         toBoolean,
-		iPath("volumes", "volume", "nocopy"):                  toBoolean,
-	},
-	"networks": {
-		iPath("external"):   toBoolean,
-		iPath("internal"):   toBoolean,
-		iPath("attachable"): toBoolean,
-	},
-	"volumes": {
-		iPath("external"): toBoolean,
-	},
-	"secrets": {
-		iPath("external"): toBoolean,
-	},
-	"configs": {
-		iPath("external"): toBoolean,
-	},
+var interpolateTypeCastMapping = map[interp.Path]interp.Cast{
+	servicePath("configs", interp.PathMatchList, "mode"):             toInt,
+	servicePath("secrets", interp.PathMatchList, "mode"):             toInt,
+	servicePath("healthcheck", "retries"):                            toInt,
+	servicePath("healthcheck", "disable"):                            toBoolean,
+	servicePath("deploy", "replicas"):                                toInt,
+	servicePath("deploy", "update_config", "parallelism"):            toInt,
+	servicePath("deploy", "update_config", "max_failure_ratio"):      toFloat,
+	servicePath("deploy", "restart_policy", "max_attempts"):          toInt,
+	servicePath("ports", interp.PathMatchList, "target"):             toInt,
+	servicePath("ports", interp.PathMatchList, "published"):          toInt,
+	servicePath("ulimits", interp.PathMatchAll):                      toInt,
+	servicePath("ulimits", interp.PathMatchAll, "hard"):              toInt,
+	servicePath("ulimits", interp.PathMatchAll, "soft"):              toInt,
+	servicePath("privileged"):                                        toBoolean,
+	servicePath("read_only"):                                         toBoolean,
+	servicePath("stdin_open"):                                        toBoolean,
+	servicePath("tty"):                                               toBoolean,
+	servicePath("volumes", interp.PathMatchList, "read_only"):        toBoolean,
+	servicePath("volumes", interp.PathMatchList, "volume", "nocopy"): toBoolean,
+	iPath("networks", interp.PathMatchAll, "external"):               toBoolean,
+	iPath("networks", interp.PathMatchAll, "internal"):               toBoolean,
+	iPath("networks", interp.PathMatchAll, "attachable"):             toBoolean,
+	iPath("volumes", interp.PathMatchAll, "external"):                toBoolean,
+	iPath("secrets", interp.PathMatchAll, "external"):                toBoolean,
+	iPath("configs", interp.PathMatchAll, "external"):                toBoolean,
 }

 func iPath(parts ...string) interp.Path {
-	return interp.NewPath(append([]string{interp.PathMatchAll}, parts...)...)
+	return interp.NewPath(parts...)
+}
+
+func servicePath(parts ...string) interp.Path {
+	return iPath(append([]string{"services", interp.PathMatchAll}, parts...)...)
 }

 func toInt(value string) (interface{}, error) {
@ -70,26 +64,11 @@ func toBoolean(value string) (interface{}, error) {
 	}
 }

-func interpolateConfig(configDict map[string]interface{}, lookupEnv interp.LookupValue) (map[string]map[string]interface{}, error) {
-	config := make(map[string]map[string]interface{})
-
-	for _, key := range []string{"services", "networks", "volumes", "secrets", "configs"} {
-		section, ok := configDict[key]
-		if !ok {
-			config[key] = make(map[string]interface{})
-			continue
-		}
-		var err error
-		config[key], err = interp.Interpolate(
-			section.(map[string]interface{}),
+func interpolateConfig(configDict map[string]interface{}, lookupEnv interp.LookupValue) (map[string]interface{}, error) {
+	return interp.Interpolate(
+		configDict,
 		interp.Options{
-				SectionName:     key,
 			LookupValue:     lookupEnv,
-				TypeCastMapping: interpolateTypeCastMapping[key],
+			TypeCastMapping: interpolateTypeCastMapping,
 		})
-		if err != nil {
-			return nil, err
-		}
-	}
-	return config, nil
 }
--- a/cli/compose/loader/loader.go
+++ b/cli/compose/loader/loader.go
@ -54,7 +54,8 @@ func Load(configDetails types.ConfigDetails) (*types.Config, error) {
 		return nil, err
 	}

-	config, err := interpolateConfig(configDict, configDetails.LookupEnv)
+	var err error
+	configDict, err = interpolateConfig(configDict, configDetails.LookupEnv)
 	if err != nil {
 		return nil, err
 	}
@ -62,30 +63,7 @@ func Load(configDetails types.ConfigDetails) (*types.Config, error) {
 	if err := schema.Validate(configDict, schema.Version(configDict)); err != nil {
 		return nil, err
 	}
-
-	cfg := types.Config{}
-	cfg.Services, err = LoadServices(config["services"], configDetails.WorkingDir, configDetails.LookupEnv)
-	if err != nil {
-		return nil, err
-	}
-
-	cfg.Networks, err = LoadNetworks(config["networks"])
-	if err != nil {
-		return nil, err
-	}
-
-	cfg.Volumes, err = LoadVolumes(config["volumes"])
-	if err != nil {
-		return nil, err
-	}
-
-	cfg.Secrets, err = LoadSecrets(config["secrets"], configDetails.WorkingDir)
-	if err != nil {
-		return nil, err
-	}
-
-	cfg.Configs, err = LoadConfigObjs(config["configs"], configDetails.WorkingDir)
-	return &cfg, err
+	return loadSections(configDict, configDetails)
 }

 func validateForbidden(configDict map[string]interface{}) error {
@ -100,6 +78,66 @@ func validateForbidden(configDict map[string]interface{}) error {
 	return nil
 }

+func loadSections(config map[string]interface{}, configDetails types.ConfigDetails) (*types.Config, error) {
+	var err error
+	cfg := types.Config{}
+
+	var loaders = []struct {
+		key string
+		fnc func(config map[string]interface{}) error
+	}{
+		{
+			key: "services",
+			fnc: func(config map[string]interface{}) error {
+				cfg.Services, err = LoadServices(config, configDetails.WorkingDir, configDetails.LookupEnv)
+				return err
+			},
+		},
+		{
+			key: "networks",
+			fnc: func(config map[string]interface{}) error {
+				cfg.Networks, err = LoadNetworks(config)
+				return err
+			},
+		},
+		{
+			key: "volumes",
+			fnc: func(config map[string]interface{}) error {
+				cfg.Volumes, err = LoadVolumes(config)
+				return err
+			},
+		},
+		{
+			key: "secrets",
+			fnc: func(config map[string]interface{}) error {
+				cfg.Secrets, err = LoadSecrets(config, configDetails.WorkingDir)
+				return err
+			},
+		},
+		{
+			key: "configs",
+			fnc: func(config map[string]interface{}) error {
+				cfg.Configs, err = LoadConfigObjs(config, configDetails.WorkingDir)
+				return err
+			},
+		},
+	}
+	for _, loader := range loaders {
+		if err := loader.fnc(getSection(config, loader.key)); err != nil {
+			return nil, err
+		}
+	}
+	return &cfg, nil
+}
+
+func getSection(config map[string]interface{}, key string) map[string]interface{} {
+	section, ok := config[key]
+	if !ok {
+		return make(map[string]interface{})
+	}
+	return section.(map[string]interface{})
+}
+
 // GetUnsupportedProperties returns the list of any unsupported properties that are
 // used in the Compose files.
 func GetUnsupportedProperties(configDetails types.ConfigDetails) []string {
--- a/cli/compose/loader/loader_test.go
+++ b/cli/compose/loader/loader_test.go
@ -529,8 +529,9 @@ services:
      - target: $theint
        published: $theint
    ulimits:
-      - $theint
-      - hard: $theint
+      nproc: $theint
+      nofile:
+        hard: $theint
        soft: $theint
    privileged: $thebool
    read_only: $thebool
@ -538,6 +539,7 @@ services:
    tty: $thebool
    volumes:
      - source: data
+        type: volume
        read_only: $thebool
        volume:
          nocopy: $thebool
@ -567,7 +569,78 @@ networks:

 	config, err := Load(buildConfigDetails(dict, env))
 	require.NoError(t, err)
-	expected := &types.Config{}
+	expected := &types.Config{
+		Services: []types.ServiceConfig{
+			{
+				Name: "web",
+				Configs: []types.ServiceConfigObjConfig{
+					{
+						Source: "appconfig",
+						Mode:   uint32Ptr(555),
+					},
+				},
+				Secrets: []types.ServiceSecretConfig{
+					{
+						Source: "super",
+						Mode:   uint32Ptr(555),
+					},
+				},
+				HealthCheck: &types.HealthCheckConfig{
+					Retries: uint64Ptr(555),
+					Disable: true,
+				},
+				Deploy: types.DeployConfig{
+					Replicas: uint64Ptr(555),
+					UpdateConfig: &types.UpdateConfig{
+						Parallelism:     uint64Ptr(555),
+						MaxFailureRatio: 3.14,
+					},
+					RestartPolicy: &types.RestartPolicy{
+						MaxAttempts: uint64Ptr(555),
+					},
+				},
+				Ports: []types.ServicePortConfig{
+					{Target: 555, Mode: "ingress", Protocol: "tcp"},
+					{Target: 34567, Mode: "ingress", Protocol: "tcp"},
+					{Target: 555, Published: 555},
+				},
+				Ulimits: map[string]*types.UlimitsConfig{
+					"nproc":  {Single: 555},
+					"nofile": {Hard: 555, Soft: 555},
+				},
+				Privileged: true,
+				ReadOnly:   true,
+				StdinOpen:  true,
+				Tty:        true,
+				Volumes: []types.ServiceVolumeConfig{
+					{
+						Source:   "data",
+						Type:     "volume",
+						ReadOnly: true,
+						Volume:   &types.ServiceVolumeVolume{NoCopy: true},
+					},
+				},
+				Environment: types.MappingWithEquals{},
+			},
+		},
+		Configs: map[string]types.ConfigObjConfig{
+			"appconfig": {External: types.External{External: true, Name: "appconfig"}},
+		},
+		Secrets: map[string]types.SecretConfig{
+			"super": {External: types.External{External: true, Name: "super"}},
+		},
+		Volumes: map[string]types.VolumeConfig{
+			"data": {External: types.External{External: true, Name: "data"}},
+		},
+		Networks: map[string]types.NetworkConfig{
+			"front": {
+				External:   types.External{External: true, Name: "front"},
+				Internal:   true,
+				Attachable: true,
+			},
+		},
+	}
+
 	assert.Equal(t, expected, config)
 }

@ -748,6 +821,10 @@ func uint64Ptr(value uint64) *uint64 {
 	return &value
 }

+func uint32Ptr(value uint32) *uint32 {
+	return &value
+}
+
 func TestFullExample(t *testing.T) {
 	bytes, err := ioutil.ReadFile("full-example.yml")
 	require.NoError(t, err)