Skip to content
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

[26.0 backport] Enable external DNS for ipvlan-l3, and disable it for macvlan/ipvlan with no parent interface #47705

Merged
merged 4 commits into from Apr 10, 2024

Conversation

robmry
Copy link
Contributor

@robmry robmry commented Apr 10, 2024

- What I did

The internal DNS resolver should only forward requests to external resolvers if the libnetwork.Sandbox served by the resolver has external network access (so, no forwarding for '--internal' networks).

The test for external network access was whether the Sandbox had an Endpoint with a gateway configured.

However, an ipvlan-l3 networks with external network access does not have a gateway, it has a default route bound to an interface.

Also, we document that an ipvlan/macvlan network with no parent interface is equivalent to a '--internal' network. But, in these cases, endpoints were still configured with a gateway (or a default route). So, DNS proxying would be enabled in the internal resolver (and, if the host's resolver was on a localhost address, requests to external resolvers from the host's network namespace would succeed).

Also also, CI hasn't been running most of the ipvlan tests becuase a check that the kernel supports ipvlan has been failing. However, without that check, tests pass (locally, and on the CI test runners). I want the new ipvlan/dns tests to run in CI, and it'd be weird to run only those and not the existing tests. So, removed the kernel-check.

- How I did it

Adjusted the test for enabling DNS proxying to include a check for '--internal' (as a shortcut) and, for non-internal networks, check for a default route as well as a gateway.

Disable configuration of a gateway or a default route for an ipvlan/macvlan Endpoint when no parent interface is specified. (Note that if a parent interface with no external network is supplied as '-o parent=', the gateway/default route will still be set up and external DNS proxying will be enabled. The network must be configured as '--internal' to prevent that from happening.)

Revert the changes from #39358 ... run ipvlan tests without a kernel-check (but not in rootless mode).

- How to verify it

New macvlan/ipvlan integration tests.

- Description for the changelog

- Fix a regression that prevented the internal resolver from forwarding requests from ipvlan-l3 networks to external resolvers.
- Prevent the use of external resolvers in ipvlan/macvlan networks created with no parent interface specified.

Signed-off-by: Rob Murray <rob.murray@docker.com>
The internal DNS resolver should only forward requests to external
resolvers if the libnetwork.Sandbox served by the resolver has external
network access (so, no forwarding for '--internal' networks).

The test for external network access was whether the Sandbox had an
Endpoint with a gateway configured.

However, an ipvlan-l3 networks with external network access does not
have a gateway, it has a default route bound to an interface.

Also, we document that an ipvlan network with no parent interface is
equivalent to a '--internal' network. But, in this case, an ipvlan-l2
network was configured with a gateway. So, DNS proxying would be enabled
in the internal resolver (and, if the host's resolver was on a localhost
address, requests to external resolvers from the host's network
namespace would succeed).

So, this change adjusts the test for enabling DNS proxying to include
a check for '--internal' (as a shortcut) and, for non-internal networks,
checks for a default route as well as a gateway. It also disables
configuration of a gateway or a default route for an ipvlan Endpoint if
no parent interface is specified.

(Note if a parent interface with no external network is supplied as
'-o parent=<dummy>', the gateway/default route will still be set up
and external DNS proxying will be enabled. The network must be
configured as '--internal' to prevent that from happening.)

Signed-off-by: Rob Murray <rob.murray@docker.com>
We document that an macvlan network with no parent interface is
equivalent to a '--internal' network. But, in this case, an macvlan
network was still configured with a gateway. So, DNS proxying would
be enabled in the internal resolver (and, if the host's resolver
was on a localhost address, requests to external resolvers from the
host's network namespace would succeed).

This change disables configuration of a gateway for a macvlan Endpoint
if no parent interface is specified.

(Note if a parent interface with no external network is supplied as
'-o parent=<dummy>', the gateway will still be set up. Documentation
will need to be updated to note that '--internal' should be used to
prevent DNS request forwarding in this case.)

Signed-off-by: Rob Murray <rob.murray@docker.com>
This reverts commit a77e147.

The ipvlan integration tests have been skipped in CI because of a check
intended to ensure the kernel has ipvlan support - which failed, but
seems to be unnecessary (probably because kernels have moved on).

Signed-off-by: Rob Murray <rob.murray@docker.com>
@robmry robmry self-assigned this Apr 10, 2024
@robmry robmry added this to the 26.0.1 milestone Apr 10, 2024
@robmry robmry changed the title [26.0 Backport] Enable external DNS for ipvlan-l3, and disable it for macvlan/ipvlan with no parent interface [26.0 backport] Enable external DNS for ipvlan-l3, and disable it for macvlan/ipvlan with no parent interface Apr 10, 2024
@robmry robmry requested a review from akerouanton April 10, 2024 10:58
@robmry robmry marked this pull request as ready for review April 10, 2024 10:58
@robmry robmry requested a review from vvoland April 10, 2024 10:59
Copy link
Member

@akerouanton akerouanton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vvoland vvoland merged commit 60b9add into moby:26.0 Apr 10, 2024
145 of 146 checks passed
renovate bot added a commit to earthly/dind that referenced this pull request Apr 15, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [docker/docker](https://togithub.com/docker/docker) | patch | `26.0.0`
-> `26.0.1` |

---

### Release Notes

<details>
<summary>docker/docker (docker/docker)</summary>

### [`v26.0.1`](https://togithub.com/moby/moby/releases/tag/v26.0.1)

[Compare
Source](https://togithub.com/docker/docker/compare/v26.0.0-rc2...v26.0.1)

#### 26.0.1

For a full list of pull requests and changes in this release, refer to
the relevant GitHub milestones:

- [docker/cli, 26.0.1
milestone](https://togithub.com/docker/cli/issues?q=is%3Aclosed+milestone%3A26.0.1)
- [moby/moby, 26.0.1
milestone](https://togithub.com/moby/moby/issues?q=is%3Aclosed+milestone%3A26.0.1)
- Deprecated and removed features, see [Deprecated
Features](https://togithub.com/docker/cli/blob/v26.0.1/docs/deprecated.md).
- Changes to the Engine API, see [API version
history](https://togithub.com/moby/moby/blob/v26.0.1/docs/api/version-history.md).

##### Bug fixes and enhancements

- Fix a regression that meant network interface specific `--sysctl`
options prevented container startup.
[moby/moby#47646](https://togithub.com/moby/moby/pull/47646)
- Remove erroneous `platform` from image `config` OCI descriptor in
`docker save` output.
[moby/moby#47694](https://togithub.com/moby/moby/pull/47694)
- containerd image store: OCI archives produced by `docker save` will
now have a non-empty `mediaType` field in `index.json`
[moby/moby#47701](https://togithub.com/moby/moby/pull/47701)
- Fix a regression that prevented the internal resolver from forwarding
requests from IPvlan L3 networks to external resolvers.
[moby/moby#47705](https://togithub.com/moby/moby/pull/47705)
- Prevent the use of external resolvers in IPvlan and Macvlan networks
created with no parent interface specified.
[moby/moby#47705](https://togithub.com/moby/moby/pull/47705)

##### Packaging updates

- Update Go runtime to 1.21.9
[moby/moby#47671](https://togithub.com/moby/moby/pull/47671),
[docker/cli#4987](https://togithub.com/docker/cli/pull/4987)
- Update Compose to [v1.26.1
](https://togithub.com/docker/compose/releases/tag/v2.26.1),
[docker/docker-ce-packaging#1009](https://togithub.com/docker/docker-ce-packaging/pull/1009)
- Update containerd to
[v1.7.15](https://togithub.com/containerd/containerd/releases/tag/v1.7.15)
(static binaries only)
[moby/moby#47692](https://togithub.com/moby/moby/pull/47692)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 6am on monday" (UTC), Automerge
- At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/earthly/dind).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yOTMuMCIsInVwZGF0ZWRJblZlciI6IjM3LjI5My4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZSJdfQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
@robmry robmry deleted the backport-26.0/47662_ipvlan_l3_dns branch May 1, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants