Merge pull request #30521 from dnephin/cleanup-compose-loader

`cli/compose` cleanup - remove the old loading system

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)):
+	case reflect.TypeOf(types.HealthCheckTest{}):
-		return transformMapStringString(source, target, data)
+		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)
-	}
+	case reflect.TypeOf(types.StringOrNumberList{}):
-	switch target.Kind() {
+		return transformStringOrNumberList(data)
-	case reflect.Struct:
+	case reflect.TypeOf(map[string]*types.ServiceNetworkConfig{}):
-		return transformStruct(source, target, data)
+		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
+				return nil, formatInvalidKeyError(keyPrefix, key)
-				if keyPrefix == "" {
-					location = "at top level"
-				} else {
-					location = fmt.Sprintf("in %s", keyPrefix)
-				}
-				return nil, fmt.Errorf("Non-string key %s: %#v", location, 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 {
+	if len(serviceConfig.EnvFile) > 0 {
-		envFiles := loadStringOrListOfStrings(envFileVal)
-
 		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(
+func transformMapStringString(data interface{}) (interface{}, error) {
-	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) {
 	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 {
+func transformServiceNetworkMap(value interface{}) (interface{}, error) {
-	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) {
 	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 {
+func transformStringList(data interface{}) (interface{}, error) {
-	if list, ok := value.([]interface{}); ok {
+	switch value := data.(type) {
-		result := make([]string, len(list))
+	case string:
-		for i, item := range list {
+		return []string{value}, nil
-			result[i] = fmt.Sprint(item)
+	case []interface{}:
+		return value, nil
+	default:
+		return data, fmt.Errorf("invalid type %T for string list", value)
 	}
-		return result
-	}
-	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) {
+func transformHealthCheckTest(data interface{}) (interface{}, error) {
-	if str, ok := value.(string); ok {
+	switch value := data.(type) {
-		return append([]string{"CMD-SHELL"}, str), nil
+	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)
+	}
 }
 
-func loadSize(value interface{}) (int64, error) {
+func transformSize(value interface{}) (int64, error) {
 	switch value := value.(type) {
 	case int:
 		return int64(value), nil

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{
+			Test:     types.HealthCheckTest([]string{"CMD-SHELL", "echo \"hello world\""}),
-				"CMD-SHELL",
-				"echo \"hello world\"",
-			},
 			Interval: "10s",
 			Timeout:  "1s",
 			Retries:  uint64Ptr(5),

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"`
+	DNS             StringList
-	DNSSearch       []string          `mapstructure:"dns_search" compose:"string_or_list"`
+	DNSSearch       StringList `mapstructure:"dns_search"`
 	DomainName      string     `mapstructure:"domainname"`
-	Entrypoint      []string          `compose:"shell_command"`
+	Entrypoint      ShellCommand
-	Environment     map[string]string `compose:"list_or_dict_equals"`
+	Environment     MappingWithEquals
-	Expose          []string          `compose:"list_of_strings_or_numbers"`
+	EnvFile         StringList `mapstructure:"env_file"`
+	Expose          StringOrNumberList
 	ExternalLinks   []string         `mapstructure:"external_links"`
-	ExtraHosts      map[string]string `mapstructure:"extra_hosts" compose:"list_or_dict_colon"`
+	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"`
+	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,7 +115,7 @@ 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"`
+	Tmpfs           StringList
 	Tty             bool `mapstructure:"tty"`
 	Ulimits         map[string]*UlimitsConfig
 	User            string
@@ -122,6 +123,24 @@ type ServiceConfig struct {
 	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,7 +151,7 @@ type LoggingConfig struct {
 type DeployConfig struct {
 	Mode          string
 	Replicas      *uint64
-	Labels        map[string]string `compose:"list_or_dict_equals"`
+	Labels        MappingWithEquals
 	UpdateConfig  *UpdateConfig `mapstructure:"update_config"`
 	Resources     Resources
 	RestartPolicy *RestartPolicy `mapstructure:"restart_policy"`
@@ -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
 }