-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Engine networking updates for moby 29.0 #23215
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
base: moby29
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for docsdocker ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
fbdde7d
to
4309ba9
Compare
However, this is not recommended for most users as it will likely break | ||
container networking. | ||
|
||
### Docker and iptables chains |
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.
Maybe worth mentioning the raw-PREROUTING
chain somewhere (but this may not be the most appropriate section though)?
In `nat` mode, when a port is published to a specific host address, that | ||
port is only accessible via the host interface with that address. So, |
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.
That's not true. If a Docker host has two NICs with addresses 192.168.100.10/24
and 10.0.0.10/24
, IP forwarding is enabled, and a port published on 192.168.100.10
, another host in the 10.0.0.0/24
subnet can access that port if its routing table has an entry for routing 192.168.100.10
via 10.0.0.10
.
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.
Any chance you could offer a suggestion here? I'm not familiar enough with networking 😬
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.
Any chance you could offer a suggestion here? I'm not familiar enough with networking 😬
I can pick this up.
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.
I've rearranged the surrounding paragraphs a bit, and added a "Note" block about direct routing to addresses on other host interfaces.
for example, publishing a port to an address on the loopback interface | ||
means remote hosts cannot access it. |
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.
But this part is true as loopback addresses aren't intended to be routed.
This changes the default binding address to `127.0.0.1` for published container | ||
ports on the default bridge network. | ||
Restart the daemon for this change to take effect. | ||
Alternatively, you can use the `dockerd --ip` flag when starting the daemon. |
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.
This is rendered as a single line (see here), so no need to put extra new lines in the md file.
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.
This is rendered as a single line (see here), so no need to put extra new lines in the md file.
But semantic linefeeds are a thing.
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.
I realize that one of my earlier suggestion is slightly wrong 🙈 Otherwise, LGTM.
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.
@ArthurFlag - thank you for making the updates, I think they're all related to pre-existing text that moved, so maybe a review on my own PR counts (!) ... they LGTM.
@@ -44,8 +44,7 @@ Here are some examples: | |||
> the outside world as well. | |||
> | |||
> If you include the localhost IP address (`127.0.0.1`, or `::1`) with the | |||
> publish flag, only the Docker host and its containers can access the | |||
> published container port. | |||
> publish flag, only the Docker host. |
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.
> publish flag, only the Docker host. | |
> publish flag, only the Docker host can access the published container port. |
This changes the default binding address to `127.0.0.1` for published container | ||
ports on the default bridge network. | ||
Restart the daemon for this change to take effect. | ||
Alternatively, you can use the `dockerd --ip` flag when starting the daemon. |
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.
This is rendered as a single line (see here), so no need to put extra new lines in the md file.
But semantic linefeeds are a thing.
In `nat` mode, when a port is published to a specific host address, that | ||
port is only accessible via the host interface with that address. So, |
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.
Any chance you could offer a suggestion here? I'm not familiar enough with networking 😬
I can pick this up.
@ArthurFlag - shall I squash the review-comment commits to tidy up the history? Not sure what happened in the "path-warnings" test, it says ...
|
No need to squash the history, you can just squash and merge when ready :) Great work 🙇 Oh and yeah, the npm registry is struggling today. These are non-required tests, no worries. |
Signed-off-by: Rob Murray <rob.murray@docker.com>
Signed-off-by: Rob Murray <rob.murray@docker.com>
With nftables on the way - refer to "firewall" instead of "iptables" in the top-level description of packet-filtering-firewalls, move out the iptables specifics, and port-publishing (which applies to both iptables and nftables). Signed-off-by: Rob Murray <rob.murray@docker.com>
Adds engine/network/firewall-nftables.md Signed-off-by: Rob Murray <rob.murray@docker.com>
ea7e170
to
2b123b9
Compare
Thanks Arthur. It's probably marginally worth keeping the original four commits separate, so I squashed the updates into them. I don't have merge-powers here, but I see the target's now branch |
Description
firewall-nftables.md
describing migration to experimental--firewall-backend=nftables
.Reviews