Permit '=' separator and '[ipv6]' in --add-host #2121

robmry · 2023-11-16T10:37:01Z

Fixes ipv6 addresses with square brackets cannot be used with the --add-host option cli#4648
See ipv6 addresses with square brackets cannot be used with the --add-host option cli#4648 (comment)
related: Permit '=' separator and '[ipv6]' in --add-host cli#4663

- What I did

Make it easier to specify IPv6 addresses in the '--add-host' option by permitting 'host=ip' in addition to 'host:ip', and allowing square brackets around the address.

For example:

--add-host=hostname:127.0.0.1
--add-host=hostname:::1
--add-host=hostname=::1
--add-host=hostname:[::1]

- How I did it

Updated the function that converts from --add-host format to buildx's csv format, stripping brackets from the address.

This pull request is equivalent to one in docker/cli, for docker run and legacy builds.

- How to verify it

New unit tests pass. And ...

/work # cat Dockerfile
FROM alpine:latest
RUN ping -c1 something
RUN ping -c1 somethingv6

/work # docker build --add-host something=8.8.8.8 --add-host somethingv6=[::1] . 2>&1 | cat
#0 building with "default" instance using docker driver

#1 [internal] load .dockerignore
#1 transferring context: 2B done
#1 DONE 0.0s

#2 [internal] load build definition from Dockerfile
#2 transferring dockerfile: 104B done
#2 DONE 0.0s

#3 [internal] load metadata for docker.io/library/alpine:latest
#3 DONE 0.9s

#4 [1/3] FROM docker.io/library/alpine:latest@sha256:eece025e432126ce23f223450a0326fbebde39cdf496a85d8c016293fc851978
#4 CACHED

#5 [2/3] RUN ping -c1 something
#5 0.116 PING something (8.8.8.8): 56 data bytes
#5 0.190 64 bytes from 8.8.8.8: seq=0 ttl=62 time=73.792 ms
#5 0.190
#5 0.190 --- something ping statistics ---
#5 0.190 1 packets transmitted, 1 packets received, 0% packet loss
#5 0.190 round-trip min/avg/max = 73.792/73.792/73.792 ms
#5 DONE 0.2s

#6 [3/3] RUN ping -c1 somethingv6
#6 0.205 PING somethingv6 (::1): 56 data bytes
#6 0.205 64 bytes from ::1: seq=0 ttl=64 time=0.028 ms
#6 0.205
#6 0.205 --- somethingv6 ping statistics ---
#6 0.205 1 packets transmitted, 1 packets received, 0% packet loss
#6 0.205 round-trip min/avg/max = 0.028/0.028/0.028 ms
#6 DONE 0.2s

#7 exporting to image
#7 exporting layers done
#7 writing image sha256:ea192cc9965b812a66562225e85b437af3af1d8347daca9d65eff8a6498b679e done
#7 DONE 0.0s

- Description for the changelog

Permit '=' separator and '[ipv6]' in --add-host

crazy-max · 2023-11-16T13:59:03Z

build/utils.go

+		host, ip, ok := strings.Cut(h, "=")
+		if !ok {
+			host, ip, ok = strings.Cut(h, ":")
+		}


Should we update the reference docs in

buildx/commands/build.go

Line 489 in cb37886

flags.StringSliceVar(&options.extraHosts, "add-host", []string{}, `Add a custom host-to-IP mapping (format: "host:ip")`)

?

Guess it should be ... (format: "host=ip") if this format is the preferred one now.

Ah, yes - will do! Thank you.

crazy-max

LGTM

robmry · 2023-11-21T09:02:07Z

The changes for CLI and buildx are done - but docker-compose has an "extra_hosts" option that'll need the same treatment.

robmry · 2023-11-21T14:56:21Z

Updated to use domain ".invalid" in unit tests - as per @thaJeztah's comment on the CLI review, docker/cli#4663 (comment)

thaJeztah

LGTM

thaJeztah · 2023-11-21T16:42:01Z

I added a cherry-pick label for consideration (so that buildx and cli will align on accepted formats)

tonistiigi · 2023-11-22T01:02:50Z

docs/reference/buildx_build.md

@@ -15,7 +15,7 @@ Start a build

 | Name                                                                                                                                                   | Type          | Default   | Description                                                                                         |
 |:-------------------------------------------------------------------------------------------------------------------------------------------------------|:--------------|:----------|:----------------------------------------------------------------------------------------------------|
-| [`--add-host`](https://docs.docker.com/engine/reference/commandline/build/#add-host)                                                                   | `stringSlice` |           | Add a custom host-to-IP mapping (format: `host:ip`)                                                 |
+| [`--add-host`](https://docs.docker.com/engine/reference/commandline/build/#add-host)                                                                   | `stringSlice` |           | Add a custom host-to-IP mapping (format: `host=ip`)                                                 |


I'm not sure if we should just switch the formatting in docs as it takes time for people to upgrade to the latest versions. Doesn't look like the old formatting is deprecated as well.

Yes, I see what you mean - the online docs aren't very version-specific.

This part of the ".md" is generated from the flags.StringSliceVar() text. We could put both in there with an "or", but that may not really help to make it obvious that the : is for backward compatibility - and we probably don't want an essay in there. Earlier comments have been about preferring ("soft-deprecating") the colon separator.

On the other hand, the "--add-host" in the first column here is a link to the cli/build docs, here it is in my branch ...

https://github.com/robmry/buildx/blob/align_add-host_with_cli/docs/reference/buildx_build.md

And the modified version of the page it'll link to is here ...

https://github.com/robmry/cli/blob/4648-clearer_ipv6_in_add-host/docs/reference/commandline/build.md#add-host

It says ...

You can wrap an IPv6 address in square brackets. = and : are both valid separators. Both formats in the following example are valid:

... perhaps we should make it clear there that the : is for backward compatibility?

@dvdksn - any suggestions?

dvdksn · 2023-11-22T10:35:15Z

commands/build.go

@@ -486,7 +486,7 @@ func buildCmd(dockerCli command.Cli, rootOpts *rootOptions, debugConfig *debug.D

 	flags := cmd.Flags()

-	flags.StringSliceVar(&options.extraHosts, "add-host", []string{}, `Add a custom host-to-IP mapping (format: "host:ip")`)
+	flags.StringSliceVar(&options.extraHosts, "add-host", []string{}, `Add a custom host-to-IP mapping (format: "host=ip")`)


We could keep the example as : here, until we officially deprecate that separator. That would give users time to upgrade to later versions which would support both syntaxes.

Ok, will do ...

@crazy-max - this will undo the earlier change for your #2121 (comment) - but I think it makes sense given #2121 (comment) ?

Yes agree let's keep : for now

Fixes docker/cli#4648 Make it easier to specify IPv6 addresses in the '--add-host' option by permitting 'host=ip' in addition to 'host:ip', and allowing square brackets around the address. For example: --add-host=hostname:127.0.0.1 --add-host=hostname:::1 --add-host=hostname=::1 --add-host=hostname=[::1] Signed-off-by: Rob Murray <[email protected]>

robmry force-pushed the align_add-host_with_cli branch from f394488 to 75740c4 Compare November 16, 2023 12:31

crazy-max reviewed Nov 16, 2023

View reviewed changes

crazy-max added the area/cli label Nov 16, 2023

robmry force-pushed the align_add-host_with_cli branch from 75740c4 to f8196c8 Compare November 16, 2023 16:24

crazy-max approved these changes Nov 17, 2023

View reviewed changes

crazy-max requested review from tonistiigi and jsternberg November 17, 2023 14:24

jsternberg approved these changes Nov 20, 2023

View reviewed changes

robmry force-pushed the align_add-host_with_cli branch from f8196c8 to 28cb12d Compare November 21, 2023 14:51

crazy-max approved these changes Nov 21, 2023

View reviewed changes

thaJeztah approved these changes Nov 21, 2023

View reviewed changes

thaJeztah added the needs-cherry-pick/v0.12 label Nov 21, 2023

tonistiigi reviewed Nov 22, 2023

View reviewed changes

dvdksn reviewed Nov 22, 2023

View reviewed changes

robmry force-pushed the align_add-host_with_cli branch from 28cb12d to eeeff1c Compare November 22, 2023 10:53

robmry mentioned this pull request Nov 23, 2023

Permit '=' separator and '[ipv6]' in 'extra_hosts' compose-spec/compose-spec#443

Merged

crazy-max approved these changes Nov 23, 2023

View reviewed changes

dvdksn approved these changes Nov 23, 2023

View reviewed changes

crazy-max merged commit 9368ecb into docker:master Nov 23, 2023
61 checks passed

thaJeztah mentioned this pull request Dec 4, 2023

[v0.12 backport] Permit '=' separator and '[ipv6]' in --add-host #2149

Closed

crazy-max added this to the v0.13.0 milestone Dec 23, 2023

crazy-max removed the needs-cherry-pick/v0.12 label Feb 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Permit '=' separator and '[ipv6]' in --add-host #2121

Permit '=' separator and '[ipv6]' in --add-host #2121

robmry commented Nov 16, 2023 •

edited by thaJeztah

Loading

crazy-max Nov 16, 2023

robmry Nov 16, 2023

crazy-max left a comment

robmry commented Nov 21, 2023

robmry commented Nov 21, 2023

thaJeztah left a comment

thaJeztah commented Nov 21, 2023

tonistiigi Nov 22, 2023

robmry Nov 22, 2023

dvdksn Nov 22, 2023 •

edited

Loading

robmry Nov 22, 2023

crazy-max Nov 22, 2023

Permit '=' separator and '[ipv6]' in --add-host #2121

Permit '=' separator and '[ipv6]' in --add-host #2121

Conversation

robmry commented Nov 16, 2023 • edited by thaJeztah Loading

crazy-max Nov 16, 2023

Choose a reason for hiding this comment

robmry Nov 16, 2023

Choose a reason for hiding this comment

crazy-max left a comment

Choose a reason for hiding this comment

robmry commented Nov 21, 2023

robmry commented Nov 21, 2023

thaJeztah left a comment

Choose a reason for hiding this comment

thaJeztah commented Nov 21, 2023

tonistiigi Nov 22, 2023

Choose a reason for hiding this comment

robmry Nov 22, 2023

Choose a reason for hiding this comment

dvdksn Nov 22, 2023 • edited Loading

Choose a reason for hiding this comment

robmry Nov 22, 2023

Choose a reason for hiding this comment

crazy-max Nov 22, 2023

Choose a reason for hiding this comment

robmry commented Nov 16, 2023 •

edited by thaJeztah

Loading

dvdksn Nov 22, 2023 •

edited

Loading