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 <github@gone.nl>
Upstream-commit: dbdf8f6468
Component: cli

components/cli/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
 }

components/cli/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) {