From dbdf8f6468f7adde9791ee423f66c5d6a774f289 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 30 Oct 2017 01:33:23 +0100 Subject: [PATCH] Preserve sort-order of extra hosts, and allow duplicate entries Extra hosts (`extra_hosts` in compose-file, or `--hosts` in services) adds custom host/ip mappings to the container's `/etc/hosts`. The current implementation used a `map[string]string{}` as intermediate storage, and sorted the results alphabetically when converting to a service-spec. As a result, duplicate hosts were removed, and order of host/ip mappings was not preserved (in case the compose-file used a list instead of a map). According to the **host.conf(5)** man page (http://man7.org/linux/man-pages/man5/host.conf.5.html) multi Valid values are on and off. If set to on, the resolver library will return all valid addresses for a host that appears in the /etc/hosts file, instead of only the first. This is off by default, as it may cause a substantial performance loss at sites with large hosts files. Multiple entries for a host are allowed, and even required for some situations, for example, to add mappings for IPv4 and IPv6 addreses for a host, as illustrated by the example hosts file in the **hosts(5)** man page (http://man7.org/linux/man-pages/man5/hosts.5.html): # The following lines are desirable for IPv4 capable hosts 127.0.0.1 localhost # 127.0.1.1 is often used for the FQDN of the machine 127.0.1.1 thishost.mydomain.org thishost 192.168.1.10 foo.mydomain.org foo 192.168.1.13 bar.mydomain.org bar 146.82.138.7 master.debian.org master 209.237.226.90 www.opensource.org # The following lines are desirable for IPv6 capable hosts ::1 localhost ip6-localhost ip6-loopback ff02::1 ip6-allnodes ff02::2 ip6-allrouters This patch changes the intermediate storage format to use a `[]string`, and only sorts entries if the input format in the compose file is a mapping. If the input format is a list, the original sort-order is preserved. Signed-off-by: Sebastiaan van Stijn --- cli/command/service/update.go | 7 ++-- cli/command/service/update_test.go | 21 +++++++++--- cli/compose/convert/service.go | 13 +++++--- cli/compose/convert/service_test.go | 9 ++++++ cli/compose/loader/loader.go | 29 +++++++++++++++++ cli/compose/loader/loader_test.go | 50 +++++++++++++++++++++++++++-- cli/compose/types/types.go | 7 ++-- 7 files changed, 119 insertions(+), 17 deletions(-) diff --git a/cli/command/service/update.go b/cli/command/service/update.go index 6ee0dfc742..7da2a972a1 100644 --- a/cli/command/service/update.go +++ b/cli/command/service/update.go @@ -868,6 +868,10 @@ func updateReplicas(flags *pflag.FlagSet, serviceMode *swarm.ServiceMode) error return nil } +// updateHosts performs a diff between existing host entries, entries to be +// removed, and entries to be added. Host entries preserve the order in which they +// were added, as the specification mentions that in case multiple entries for a +// host exist, the first entry should be used (by default). func updateHosts(flags *pflag.FlagSet, hosts *[]string) error { // Combine existing Hosts (in swarmkit format) with the host to add (convert to swarmkit format) if flags.Changed(flagHostAdd) { @@ -902,9 +906,6 @@ func updateHosts(flags *pflag.FlagSet, hosts *[]string) error { } } - // Sort so that result is predictable. - sort.Strings(newHosts) - *hosts = newHosts return nil } diff --git a/cli/command/service/update_test.go b/cli/command/service/update_test.go index 50065e4205..32484d0217 100644 --- a/cli/command/service/update_test.go +++ b/cli/command/service/update_test.go @@ -366,12 +366,23 @@ func TestUpdateHosts(t *testing.T) { testutil.ErrorContains(t, flags.Set("host-add", "$example.com$"), `bad format for add-host: "$example.com$"`) hosts := []string{"1.2.3.4 example.com", "4.3.2.1 example.org", "2001:db8:abc8::1 example.net"} + expected := []string{"1.2.3.4 example.com", "4.3.2.1 example.org", "2001:db8:abc8::1 ipv6.net"} - updateHosts(flags, &hosts) - require.Len(t, hosts, 3) - assert.Equal(t, "1.2.3.4 example.com", hosts[0]) - assert.Equal(t, "2001:db8:abc8::1 ipv6.net", hosts[1]) - assert.Equal(t, "4.3.2.1 example.org", hosts[2]) + err := updateHosts(flags, &hosts) + assert.NoError(t, err) + assert.Equal(t, expected, hosts) +} + +func TestUpdateHostsPreservesOrder(t *testing.T) { + flags := newUpdateCommand(nil).Flags() + flags.Set("host-add", "foobar:127.0.0.2") + flags.Set("host-add", "foobar:127.0.0.1") + flags.Set("host-add", "foobar:127.0.0.3") + + hosts := []string{} + err := updateHosts(flags, &hosts) + assert.NoError(t, err) + assert.Equal(t, []string{"127.0.0.2 foobar", "127.0.0.1 foobar", "127.0.0.3 foobar"}, hosts) } func TestUpdatePortsRmWithProtocol(t *testing.T) { diff --git a/cli/compose/convert/service.go b/cli/compose/convert/service.go index c4473ba084..75c6db8ffb 100644 --- a/cli/compose/convert/service.go +++ b/cli/compose/convert/service.go @@ -133,7 +133,7 @@ func Service( Command: service.Entrypoint, Args: service.Command, Hostname: service.Hostname, - Hosts: sortStrings(convertExtraHosts(service.ExtraHosts)), + Hosts: convertExtraHosts(service.ExtraHosts), DNSConfig: dnsConfig, Healthcheck: healthcheck, Env: sortStrings(convertEnvironment(service.Environment)), @@ -365,10 +365,15 @@ func uint32Ptr(value uint32) *uint32 { return &value } -func convertExtraHosts(extraHosts map[string]string) []string { +// convertExtraHosts converts : mappings to SwarmKit notation: +// "IP-address hostname(s)". The original order of mappings is preserved. +func convertExtraHosts(extraHosts composetypes.HostsList) []string { hosts := []string{} - for host, ip := range extraHosts { - hosts = append(hosts, fmt.Sprintf("%s %s", ip, host)) + for _, hostIP := range extraHosts { + if v := strings.SplitN(hostIP, ":", 2); len(v) == 2 { + // Convert to SwarmKit notation: IP-address hostname(s) + hosts = append(hosts, fmt.Sprintf("%s %s", v[1], v[0])) + } } return hosts } diff --git a/cli/compose/convert/service_test.go b/cli/compose/convert/service_test.go index 1945e22d09..42e0a29d0b 100644 --- a/cli/compose/convert/service_test.go +++ b/cli/compose/convert/service_test.go @@ -57,6 +57,15 @@ func TestConvertEnvironment(t *testing.T) { assert.Equal(t, []string{"foo=bar", "key=value"}, env) } +func TestConvertExtraHosts(t *testing.T) { + source := composetypes.HostsList{ + "zulu:127.0.0.2", + "alpha:127.0.0.1", + "zulu:ff02::1", + } + assert.Equal(t, []string{"127.0.0.2 zulu", "127.0.0.1 alpha", "ff02::1 zulu"}, convertExtraHosts(source)) +} + func TestConvertResourcesFull(t *testing.T) { source := composetypes.Resources{ Limits: &composetypes.Resource{ diff --git a/cli/compose/loader/loader.go b/cli/compose/loader/loader.go index 5ab95b21f6..01081a246f 100644 --- a/cli/compose/loader/loader.go +++ b/cli/compose/loader/loader.go @@ -244,6 +244,7 @@ func createTransformHook() mapstructure.DecodeHookFuncType { reflect.TypeOf(types.MappingWithEquals{}): transformMappingOrListFunc("=", true), reflect.TypeOf(types.Labels{}): transformMappingOrListFunc("=", false), reflect.TypeOf(types.MappingWithColon{}): transformMappingOrListFunc(":", false), + reflect.TypeOf(types.HostsList{}): transformListOrMappingFunc(":", false), reflect.TypeOf(types.ServiceVolumeConfig{}): transformServiceVolumeConfig, reflect.TypeOf(types.BuildConfig{}): transformBuildConfig, } @@ -647,6 +648,22 @@ func transformMappingOrListFunc(sep string, allowNil bool) func(interface{}) (in } } +func transformListOrMappingFunc(sep string, allowNil bool) func(interface{}) (interface{}, error) { + return func(data interface{}) (interface{}, error) { + return transformListOrMapping(data, sep, allowNil), nil + } +} + +func transformListOrMapping(listOrMapping interface{}, sep string, allowNil bool) interface{} { + switch value := listOrMapping.(type) { + case map[string]interface{}: + return toStringList(value, sep, allowNil) + case []interface{}: + return listOrMapping + } + panic(errors.Errorf("expected a map or a list, got %T: %#v", listOrMapping, listOrMapping)) +} + func transformMappingOrList(mappingOrList interface{}, sep string, allowNil bool) interface{} { switch value := mappingOrList.(type) { case map[string]interface{}: @@ -749,3 +766,15 @@ func toString(value interface{}, allowNil bool) interface{} { return "" } } + +func toStringList(value map[string]interface{}, separator string, allowNil bool) []string { + output := []string{} + for key, value := range value { + if value == nil && !allowNil { + continue + } + output = append(output, fmt.Sprintf("%s%s%s", key, separator, value)) + } + sort.Strings(output) + return output +} diff --git a/cli/compose/loader/loader_test.go b/cli/compose/loader/loader_test.go index f7e7ff3026..6fcc25534c 100644 --- a/cli/compose/loader/loader_test.go +++ b/cli/compose/loader/loader_test.go @@ -916,9 +916,9 @@ func TestFullExample(t *testing.T) { "project_db_1:mysql", "project_db_1:postgresql", }, - ExtraHosts: map[string]string{ - "otherhost": "50.31.209.229", - "somehost": "162.242.195.82", + ExtraHosts: []string{ + "somehost:162.242.195.82", + "otherhost:50.31.209.229", }, HealthCheck: &types.HealthCheckConfig{ Test: types.HealthCheckTest([]string{"CMD-SHELL", "echo \"hello world\""}), @@ -1362,3 +1362,47 @@ volumes: assert.Len(t, config.Services[0].Volumes, 1) assert.Equal(t, expected, config.Services[0].Volumes[0]) } + +func TestLoadExtraHostsMap(t *testing.T) { + config, err := loadYAML(` +version: "3.2" +services: + web: + image: busybox + extra_hosts: + "zulu": "162.242.195.82" + "alpha": "50.31.209.229" +`) + require.NoError(t, err) + + expected := types.HostsList{ + "alpha:50.31.209.229", + "zulu:162.242.195.82", + } + + require.Len(t, config.Services, 1) + assert.Equal(t, expected, config.Services[0].ExtraHosts) +} + +func TestLoadExtraHostsList(t *testing.T) { + config, err := loadYAML(` +version: "3.2" +services: + web: + image: busybox + extra_hosts: + - "zulu:162.242.195.82" + - "alpha:50.31.209.229" + - "zulu:ff02::1" +`) + require.NoError(t, err) + + expected := types.HostsList{ + "zulu:162.242.195.82", + "alpha:50.31.209.229", + "zulu:ff02::1", + } + + require.Len(t, config.Services, 1) + assert.Equal(t, expected, config.Services[0].ExtraHosts) +} diff --git a/cli/compose/types/types.go b/cli/compose/types/types.go index ec089bdfa0..e44b92c14e 100644 --- a/cli/compose/types/types.go +++ b/cli/compose/types/types.go @@ -97,8 +97,8 @@ type ServiceConfig struct { Environment MappingWithEquals EnvFile StringList `mapstructure:"env_file"` Expose StringOrNumberList - ExternalLinks []string `mapstructure:"external_links"` - ExtraHosts MappingWithColon `mapstructure:"extra_hosts"` + ExternalLinks []string `mapstructure:"external_links"` + ExtraHosts HostsList `mapstructure:"extra_hosts"` Hostname string HealthCheck *HealthCheckConfig Image string @@ -162,6 +162,9 @@ type Labels map[string]string // 'key: value' strings type MappingWithColon map[string]string +// HostsList is a list of colon-separated host-ip mappings +type HostsList []string + // LoggingConfig the logging configuration for a service type LoggingConfig struct { Driver string