containers / common

Location for shared common files in github.com/containers repos.
Apache License 2.0
193 stars 201 forks source link

libnetwork/resolvconf: filter out ipv6 link local #2233

Closed Luap99 closed 2 weeks ago

Luap99 commented 2 weeks ago

One thing I noticed in the recent aardvark-dns bug[1] that we copy link local nameservers into the container. This makes no sense as the link local address contains a zone (interface name/index) and cannot work without it. However a container by design will have a different interface name/index so the address can never work in the normal case.

Only when we do share the host netns then we should keep it.

[1] https://github.com/containers/aardvark-dns/pull/537

openshift-ci[bot] commented 2 weeks ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Luap99

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/containers/common/blob/main/OWNERS)~~ [Luap99] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
Luap99 commented 2 weeks ago

@mheon PTAL

mheon commented 2 weeks ago

LGTM, though I wonder if it's not time to start thinking about a proper parser for resolv.conf instead of regexes

Luap99 commented 2 weeks ago

yeah I would like that too but that seems a lot work work and this here was a simple enough fix.

rhatdan commented 2 weeks ago

/lgtm