From af8f563922bd8751f7b411407449eb6af68b1a55 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 4 Oct 2017 16:51:48 -0400 Subject: [PATCH] Fix load order Signed-off-by: Daniel Nephin --- cli/compose/interpolation/interpolation.go | 75 +++++--------- .../interpolation/interpolation_test.go | 14 +-- cli/compose/loader/interpolate.go | 97 ++++++++----------- cli/compose/loader/loader.go | 88 ++++++++++++----- cli/compose/loader/loader_test.go | 83 +++++++++++++++- 5 files changed, 209 insertions(+), 148 deletions(-) diff --git a/cli/compose/interpolation/interpolation.go b/cli/compose/interpolation/interpolation.go index f05c5602cc..a1e7719860 100644 --- 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) } diff --git a/cli/compose/interpolation/interpolation_test.go b/cli/compose/interpolation/interpolation_test.go index 3248841026..8f5d50db65 100644 --- 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{}{ diff --git a/cli/compose/loader/interpolate.go b/cli/compose/loader/interpolate.go index 151d2034c3..5c3e1b8b0c 100644 --- 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{}), - interp.Options{ - SectionName: key, - LookupValue: lookupEnv, - TypeCastMapping: interpolateTypeCastMapping[key], - }) - if err != nil { - return nil, err - } - } - return config, nil +func interpolateConfig(configDict map[string]interface{}, lookupEnv interp.LookupValue) (map[string]interface{}, error) { + return interp.Interpolate( + configDict, + interp.Options{ + LookupValue: lookupEnv, + TypeCastMapping: interpolateTypeCastMapping, + }) } diff --git a/cli/compose/loader/loader.go b/cli/compose/loader/loader.go index fe1c634347..5ab95b21f6 100644 --- 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 { diff --git a/cli/compose/loader/loader_test.go b/cli/compose/loader/loader_test.go index 0cb43ec816..f7e7ff3026 100644 --- 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)