From 1eefdba226988149f3191ad071a10aa62c1d3fbf Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Wed, 18 Jan 2017 15:27:02 -0500 Subject: [PATCH] Remove the old loading system from compose config loading The original Compose config loading used the `compose` tag, which was replaced by mapstructure. Some fields were left on the old tag. This commit removes the old tag and uses types and mapstructure. Signed-off-by: Daniel Nephin --- compose/loader/loader.go | 175 +++++++++++----------------------- compose/loader/loader_test.go | 13 +-- compose/types/types.go | 66 ++++++++----- 3 files changed, 106 insertions(+), 148 deletions(-) diff --git a/compose/loader/loader.go b/compose/loader/loader.go index 39f69a03ff..2c92666c51 100644 --- a/compose/loader/loader.go +++ b/compose/loader/loader.go @@ -225,18 +225,28 @@ func transformHook( switch target { case reflect.TypeOf(types.External{}): return transformExternal(data) - case reflect.TypeOf(make(map[string]string, 0)): - return transformMapStringString(source, target, data) + case reflect.TypeOf(types.HealthCheckTest{}): + return transformHealthCheckTest(data) + case reflect.TypeOf(types.ShellCommand{}): + return transformShellCommand(data) + case reflect.TypeOf(types.StringList{}): + return transformStringList(data) + case reflect.TypeOf(map[string]string{}): + return transformMapStringString(data) case reflect.TypeOf(types.UlimitsConfig{}): return transformUlimits(data) case reflect.TypeOf(types.UnitBytes(0)): - return loadSize(data) + return transformSize(data) case reflect.TypeOf(types.ServiceSecretConfig{}): return transformServiceSecret(data) - } - switch target.Kind() { - case reflect.Struct: - return transformStruct(source, target, data) + case reflect.TypeOf(types.StringOrNumberList{}): + return transformStringOrNumberList(data) + case reflect.TypeOf(map[string]*types.ServiceNetworkConfig{}): + return transformServiceNetworkMap(data) + case reflect.TypeOf(types.MappingWithEquals{}): + return transformMappingOrList(data, "="), nil + case reflect.TypeOf(types.MappingWithColon{}): + return transformMappingOrList(data, ":"), nil } return data, nil } @@ -249,13 +259,7 @@ func convertToStringKeysRecursive(value interface{}, keyPrefix string) (interfac for key, entry := range mapping { str, ok := key.(string) if !ok { - var location string - if keyPrefix == "" { - location = "at top level" - } else { - location = fmt.Sprintf("in %s", keyPrefix) - } - return nil, fmt.Errorf("Non-string key %s: %#v", location, key) + return nil, formatInvalidKeyError(keyPrefix, key) } var newKeyPrefix string if keyPrefix == "" { @@ -286,6 +290,16 @@ func convertToStringKeysRecursive(value interface{}, keyPrefix string) (interfac return value, nil } +func formatInvalidKeyError(keyPrefix string, key interface{}) error { + var location string + if keyPrefix == "" { + location = "at top level" + } else { + location = fmt.Sprintf("in %s", keyPrefix) + } + return fmt.Errorf("Non-string key %s: %#v", location, key) +} + func loadServices(servicesDict types.Dict, workingDir string) ([]types.ServiceConfig, error) { var services []types.ServiceConfig @@ -307,7 +321,7 @@ func loadService(name string, serviceDict types.Dict, workingDir string) (*types } serviceConfig.Name = name - if err := resolveEnvironment(serviceConfig, serviceDict, workingDir); err != nil { + if err := resolveEnvironment(serviceConfig, workingDir); err != nil { return nil, err } @@ -318,15 +332,13 @@ func loadService(name string, serviceDict types.Dict, workingDir string) (*types return serviceConfig, nil } -func resolveEnvironment(serviceConfig *types.ServiceConfig, serviceDict types.Dict, workingDir string) error { +func resolveEnvironment(serviceConfig *types.ServiceConfig, workingDir string) error { environment := make(map[string]string) - if envFileVal, ok := serviceDict["env_file"]; ok { - envFiles := loadStringOrListOfStrings(envFileVal) - + if len(serviceConfig.EnvFile) > 0 { var envVars []string - for _, file := range envFiles { + for _, file := range serviceConfig.EnvFile { filePath := absPath(workingDir, file) fileVars, err := opts.ParseEnvFile(filePath) if err != nil { @@ -419,7 +431,6 @@ func loadVolumes(source types.Dict) (map[string]types.VolumeConfig, error) { return volumes, nil } -// TODO: remove duplicate with networks/volumes func loadSecrets(source types.Dict, workingDir string) (map[string]types.SecretConfig, error) { secrets := make(map[string]types.SecretConfig) if err := transform(source, &secrets); err != nil { @@ -444,46 +455,7 @@ func absPath(workingDir string, filepath string) string { return path.Join(workingDir, filepath) } -func transformStruct( - source reflect.Type, - target reflect.Type, - data interface{}, -) (interface{}, error) { - structValue, ok := data.(map[string]interface{}) - if !ok { - // FIXME: this is necessary because of convertToStringKeysRecursive - structValue, ok = data.(types.Dict) - if !ok { - panic(fmt.Sprintf( - "transformStruct called with non-map type: %T, %s", data, data)) - } - } - - var err error - for i := 0; i < target.NumField(); i++ { - field := target.Field(i) - fieldTag := field.Tag.Get("compose") - - yamlName := toYAMLName(field.Name) - value, ok := structValue[yamlName] - if !ok { - continue - } - - structValue[yamlName], err = convertField( - fieldTag, reflect.TypeOf(value), field.Type, value) - if err != nil { - return nil, fmt.Errorf("field %s: %s", yamlName, err.Error()) - } - } - return structValue, nil -} - -func transformMapStringString( - source reflect.Type, - target reflect.Type, - data interface{}, -) (interface{}, error) { +func transformMapStringString(data interface{}) (interface{}, error) { switch value := data.(type) { case map[string]interface{}: return toMapStringString(value), nil @@ -496,37 +468,6 @@ func transformMapStringString( } } -func convertField( - fieldTag string, - source reflect.Type, - target reflect.Type, - data interface{}, -) (interface{}, error) { - switch fieldTag { - case "": - return data, nil - case "healthcheck": - return loadHealthcheck(data) - case "list_or_dict_equals": - return loadMappingOrList(data, "="), nil - case "list_or_dict_colon": - return loadMappingOrList(data, ":"), nil - case "list_or_struct_map": - return loadListOrStructMap(data, target) - case "string_or_list": - return loadStringOrListOfStrings(data), nil - case "list_of_strings_or_numbers": - return loadListOfStringsOrNumbers(data), nil - case "shell_command": - return loadShellCommand(data) - case "size": - return loadSize(data) - case "-": - return nil, nil - } - return data, nil -} - func transformExternal(data interface{}) (interface{}, error) { switch value := data.(type) { case bool: @@ -551,18 +492,9 @@ func transformServiceSecret(data interface{}) (interface{}, error) { default: return data, fmt.Errorf("invalid type %T for external", value) } - } -func toYAMLName(name string) string { - nameParts := fieldNameRegexp.FindAllString(name, -1) - for i, p := range nameParts { - nameParts[i] = strings.ToLower(p) - } - return strings.Join(nameParts, "_") -} - -func loadListOrStructMap(value interface{}, target reflect.Type) (interface{}, error) { +func transformServiceNetworkMap(value interface{}) (interface{}, error) { if list, ok := value.([]interface{}); ok { mapValue := map[interface{}]interface{}{} for _, name := range list { @@ -570,31 +502,30 @@ func loadListOrStructMap(value interface{}, target reflect.Type) (interface{}, e } return mapValue, nil } - return value, nil } -func loadListOfStringsOrNumbers(value interface{}) []string { +func transformStringOrNumberList(value interface{}) (interface{}, error) { list := value.([]interface{}) result := make([]string, len(list)) for i, item := range list { result[i] = fmt.Sprint(item) } - return result + return result, nil } -func loadStringOrListOfStrings(value interface{}) []string { - if list, ok := value.([]interface{}); ok { - result := make([]string, len(list)) - for i, item := range list { - result[i] = fmt.Sprint(item) - } - return result +func transformStringList(data interface{}) (interface{}, error) { + switch value := data.(type) { + case string: + return []string{value}, nil + case []interface{}: + return value, nil + default: + return data, fmt.Errorf("invalid type %T for string list", value) } - return []string{value.(string)} } -func loadMappingOrList(mappingOrList interface{}, sep string) map[string]string { +func transformMappingOrList(mappingOrList interface{}, sep string) map[string]string { if mapping, ok := mappingOrList.(types.Dict); ok { return toMapStringString(mapping) } @@ -613,21 +544,25 @@ func loadMappingOrList(mappingOrList interface{}, sep string) map[string]string panic(fmt.Errorf("expected a map or a slice, got: %#v", mappingOrList)) } -func loadShellCommand(value interface{}) (interface{}, error) { +func transformShellCommand(value interface{}) (interface{}, error) { if str, ok := value.(string); ok { return shellwords.Parse(str) } return value, nil } -func loadHealthcheck(value interface{}) (interface{}, error) { - if str, ok := value.(string); ok { - return append([]string{"CMD-SHELL"}, str), nil +func transformHealthCheckTest(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, fmt.Errorf("invalid type %T for healthcheck.test", value) } - return value, nil } -func loadSize(value interface{}) (int64, error) { +func transformSize(value interface{}) (int64, error) { switch value := value.(type) { case int: return int64(value), nil diff --git a/compose/loader/loader_test.go b/compose/loader/loader_test.go index f7fee89ede..bb5d3ecc06 100644 --- a/compose/loader/loader_test.go +++ b/compose/loader/loader_test.go @@ -394,7 +394,7 @@ services: `) assert.NoError(t, err) - expected := map[string]string{ + expected := types.MappingWithEquals{ "FOO": "1", "BAR": "2", "BAZ": "2.5", @@ -456,7 +456,7 @@ volumes: home := os.Getenv("HOME") - expectedLabels := map[string]string{ + expectedLabels := types.MappingWithEquals{ "home1": home, "home2": home, "nonexistent": "", @@ -621,6 +621,10 @@ func TestFullExample(t *testing.T) { "BAR": "2", "BAZ": "3", }, + EnvFile: []string{ + "./example1.env", + "./example2.env", + }, Expose: []string{"3000", "8000"}, ExternalLinks: []string{ "redis_1", @@ -632,10 +636,7 @@ func TestFullExample(t *testing.T) { "somehost": "162.242.195.82", }, HealthCheck: &types.HealthCheckConfig{ - Test: []string{ - "CMD-SHELL", - "echo \"hello world\"", - }, + Test: types.HealthCheckTest([]string{"CMD-SHELL", "echo \"hello world\""}), Interval: "10s", Timeout: "1s", Retries: uint64Ptr(5), diff --git a/compose/types/types.go b/compose/types/types.go index d70d01ed29..3b9a2b2a08 100644 --- a/compose/types/types.go +++ b/compose/types/types.go @@ -81,31 +81,32 @@ type ServiceConfig struct { CapAdd []string `mapstructure:"cap_add"` CapDrop []string `mapstructure:"cap_drop"` CgroupParent string `mapstructure:"cgroup_parent"` - Command []string `compose:"shell_command"` + Command ShellCommand ContainerName string `mapstructure:"container_name"` DependsOn []string `mapstructure:"depends_on"` Deploy DeployConfig Devices []string - DNS []string `compose:"string_or_list"` - DNSSearch []string `mapstructure:"dns_search" compose:"string_or_list"` - DomainName string `mapstructure:"domainname"` - Entrypoint []string `compose:"shell_command"` - Environment map[string]string `compose:"list_or_dict_equals"` - Expose []string `compose:"list_of_strings_or_numbers"` - ExternalLinks []string `mapstructure:"external_links"` - ExtraHosts map[string]string `mapstructure:"extra_hosts" compose:"list_or_dict_colon"` + DNS StringList + DNSSearch StringList `mapstructure:"dns_search"` + DomainName string `mapstructure:"domainname"` + Entrypoint ShellCommand + Environment MappingWithEquals + EnvFile StringList `mapstructure:"env_file"` + Expose StringOrNumberList + ExternalLinks []string `mapstructure:"external_links"` + ExtraHosts MappingWithColon `mapstructure:"extra_hosts"` Hostname string HealthCheck *HealthCheckConfig Image string Ipc string - Labels map[string]string `compose:"list_or_dict_equals"` + Labels MappingWithEquals Links []string Logging *LoggingConfig - MacAddress string `mapstructure:"mac_address"` - NetworkMode string `mapstructure:"network_mode"` - Networks map[string]*ServiceNetworkConfig `compose:"list_or_struct_map"` + MacAddress string `mapstructure:"mac_address"` + NetworkMode string `mapstructure:"network_mode"` + Networks map[string]*ServiceNetworkConfig Pid string - Ports []string `compose:"list_of_strings_or_numbers"` + Ports StringOrNumberList Privileged bool ReadOnly bool `mapstructure:"read_only"` Restart string @@ -114,14 +115,32 @@ type ServiceConfig struct { StdinOpen bool `mapstructure:"stdin_open"` StopGracePeriod *time.Duration `mapstructure:"stop_grace_period"` StopSignal string `mapstructure:"stop_signal"` - Tmpfs []string `compose:"string_or_list"` - Tty bool `mapstructure:"tty"` + Tmpfs StringList + Tty bool `mapstructure:"tty"` Ulimits map[string]*UlimitsConfig User string Volumes []string WorkingDir string `mapstructure:"working_dir"` } +// ShellCommand is a string or list of string args +type ShellCommand []string + +// StringList is a type for fields that can be a string or list of strings +type StringList []string + +// StringOrNumberList is a type for fields that can be a list of strings or +// numbers +type StringOrNumberList []string + +// MappingWithEquals is a mapping type that can be converted from a list of +// key=value strings +type MappingWithEquals map[string]string + +// MappingWithColon is a mapping type that can be converted from alist of +// 'key: value' strings +type MappingWithColon map[string]string + // LoggingConfig the logging configuration for a service type LoggingConfig struct { Driver string @@ -132,8 +151,8 @@ type LoggingConfig struct { type DeployConfig struct { Mode string Replicas *uint64 - Labels map[string]string `compose:"list_or_dict_equals"` - UpdateConfig *UpdateConfig `mapstructure:"update_config"` + Labels MappingWithEquals + UpdateConfig *UpdateConfig `mapstructure:"update_config"` Resources Resources RestartPolicy *RestartPolicy `mapstructure:"restart_policy"` Placement Placement @@ -141,13 +160,16 @@ type DeployConfig struct { // HealthCheckConfig the healthcheck configuration for a service type HealthCheckConfig struct { - Test []string `compose:"healthcheck"` + Test HealthCheckTest Timeout string Interval string Retries *uint64 Disable bool } +// HealthCheckTest is the command run to test the health of a service +type HealthCheckTest []string + // UpdateConfig the service update configuration type UpdateConfig struct { Parallelism *uint64 @@ -216,7 +238,7 @@ type NetworkConfig struct { Ipam IPAMConfig External External Internal bool - Labels map[string]string `compose:"list_or_dict_equals"` + Labels MappingWithEquals } // IPAMConfig for a network @@ -235,7 +257,7 @@ type VolumeConfig struct { Driver string DriverOpts map[string]string `mapstructure:"driver_opts"` External External - Labels map[string]string `compose:"list_or_dict_equals"` + Labels MappingWithEquals } // External identifies a Volume or Network as a reference to a resource that is @@ -249,5 +271,5 @@ type External struct { type SecretConfig struct { File string External External - Labels map[string]string `compose:"list_or_dict_equals"` + Labels MappingWithEquals }