-
Notifications
You must be signed in to change notification settings - Fork 2k
Skip IPAddr validation for "host-gateway" string #2337
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Skip IPAddr validation for "host-gateway" string #2337
Conversation
opts/hosts.go
Outdated
@@ -6,6 +6,8 @@ import ( | |||
"net/url" | |||
"strconv" | |||
"strings" | |||
|
|||
"github.com/docker/docker/daemon/network" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should move the constant to somewhere in the client/api package; we're now importing the daemon package, which is not really ideal
d4a36c2
to
4688f28
Compare
Relates to - moby/moby 40007 The above PR added support in moby, that detects if a special string "host-gateway" is added to the IP section of --add-host, and if true, replaces it with a special IP value (value of --host-gateway-ip Daemon flag which defaults to the IP of the default bridge). This PR is needed to skip the validation for the above feature Signed-off-by: Arko Dasgupta <arko.dasgupta@docker.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
4688f28
to
67ebcd6
Compare
// hostGatewayName defines a special string which users can append to --add-host | ||
// to add an extra entry in /etc/hosts that maps host.docker.internal to the host IP | ||
// TODO Consider moving the HostGatewayName constant defined in docker at | ||
// github.com/docker/docker/daemon/network/constants.go outside of the "daemon" | ||
// package, so that the CLI can consume it. | ||
hostGatewayName = "host-gateway" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went back to defining a const locally, but adding a TODO to consider improving this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Working on updated documentation for this, but splitting it in "enhancements to be backported" and "for next release" |
@thaJeztah and @silvin-lubecki thanks for taking care of this ! |
@arkodg you're welcome! And sorry for the back-and-forth on the |
@thaJeztah - Will support for |
@jcoutch it's supported by the API / daemon, so would be able to be supported by compose if the validation is removed; can you open a ticket in the docker compose repository? (or perhaps even better; a pull request if you're familiar with Python |
@jcoutch I just tested docker compose against a daemon that supports this feature; docker-compose version
docker-compose version 1.26.0-rc3, build 46118bc5
docker-py version: 4.2.0
CPython version: 3.7.6
OpenSSL version: OpenSSL 1.1.1d 10 Sep 2019 Using this compose file: version: '3.6'
services:
nginx:
image: nginx:alpine
extra_hosts:
- "foo:127.0.0.1"
- "bar:127.0.0.2"
- "host.docker.internal:host-gateway" Trying to run it on a docker 19.03 daemon produces an error (as expected);
On a docker daemon built from master (so with support for the docker-compose up -d
Creating network "extra-hosts_default" with the default driver
Creating extra-hosts_nginx_1 ... done Verifying the container that's created: docker inspect --format='{{json .HostConfig.ExtraHosts}}' extra-hosts_nginx_1 Which produces: [
"foo:127.0.0.1",
"bar:127.0.0.2",
"host.docker.internal:host-gateway"
] And verifying that the magic docker exec extra-hosts_nginx_1 cat /etc/hosts
127.0.0.1 localhost
::1 localhost ip6-localhost ip6-loopback
fe00::0 ip6-localnet
ff00::0 ip6-mcastprefix
ff02::1 ip6-allnodes
ff02::2 ip6-allrouters
127.0.0.1 foo
127.0.0.2 bar
172.18.0.1 host.docker.internal
172.20.0.2 2eb3a4602c9a |
carry of #2293
closes #2293
Relates to - moby/moby#40007 "Support host.docker.internal in dockerd on Linux"
The above PR added support in moby, that detects if
a special string "host-gateway" is added to the IP
section of --add-host, and if true, replaces it with
a special IP value (value of --host-gateway-ip Daemon flag
which defaults to the IP of the default bridge).
This PR is needed to skip the validation for the above
feature