-
Notifications
You must be signed in to change notification settings - Fork 39.4k
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
kubelet: parseResolvConf: Handle "search ." #109441
kubelet: parseResolvConf: Handle "search ." #109441
Conversation
When parsing a resolv.conf file that has "search .", parseResolvConf should accept the "." entry verbatim. Before this commit, parseResolvConf unconditionally trimmed the "." suffix, which in the case of "." resulted in a "" entry (that is, the empty string). This empty entry could lead parseResolvConf to produce a resolv.conf file with "search ". Resolvers could fail to parse such a resolv.conf file from parseResolvConf, thus breaking DNS resolution in pods. After this commit, parseResolvConf accepts a resolv.conf file with "search ." and passes the "." entry through verbatim to produce a valid resolv.conf file. The "." suffix is still trimmed for any entry that does not solely comprise ".". Follow-up to commit a215a88. * pkg/kubelet/network/dns/dns.go (parseResolvConf): Handle a "." entry in the search path by copying it verbatim. * pkg/kubelet/network/dns/dns_test.go (TestParseResolvConf): Add a test case for "search .".
Hi @Miciah. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test |
/lgtm /assign @thockin |
/lgtm |
/retest |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aojea, dcbw, khenidak, Miciah 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:
Approvers can indicate their approval by writing |
/triage accepted |
* systemd adds "search ." to hosts /run/systemd/resolve/resolv.conf on hosts with a fqdn hostname * Kubelet v1.25 began propagating "search ." from the host node into containers' `/etc/resolv.conf` * musl-based DNS resolvers don't behave correctly when `search .` is used in their `/etc/resolv.conf`. This breaks Alpine images * Adapt the same workaround used by Openshift to strip the "search ." * This only applies to bare-metal Typhoon nodes (where hostnames are set to fqdn's), nodes on cloud platforms aren't affected in the Typhoon configuration Kubernetes tracking issue: kubernetes/kubernetes#112135 Rel: * systemd/systemd#17201 * kubernetes/kubernetes#109441 * coreos/fedora-coreos-tracker#1287 * openshift/okd-machine-os#159
* systemd adds "search ." to hosts /run/systemd/resolve/resolv.conf on hosts with a fqdn hostname * Kubelet v1.25 began propagating "search ." from the host node into containers' `/etc/resolv.conf` * musl-based DNS resolvers don't behave correctly when `search .` is used in their `/etc/resolv.conf`. This breaks Alpine images * Adapt the same workaround used by Openshift to strip the "search ." * This only applies to bare-metal Typhoon nodes (where hostnames are set to fqdn's), nodes on cloud platforms aren't affected in the Typhoon configuration Kubernetes tracking issue: kubernetes/kubernetes#112135 Rel: * systemd/systemd#17201 * kubernetes/kubernetes#109441 * coreos/fedora-coreos-tracker#1287 * openshift/okd-machine-os#159
* Adapt kubernetes#109441 but ensures that `search .` does not get propagated into containers' /etc/resolv.conf. There is no reason to put `.` in a container's search field and it causes issues for musl
* Adapt kubernetes#109441 but ensures that `search .` does not get propagated into containers' /etc/resolv.conf. There is no reason to put `.` in a container's search field and it causes issues for musl
* systemd adds "search ." to hosts /run/systemd/resolve/resolv.conf on hosts with a fqdn hostname * Kubelet v1.25 began propagating "search ." from the host node into containers' `/etc/resolv.conf` * musl-based DNS resolvers don't behave correctly when `search .` is used in their `/etc/resolv.conf`. This breaks Alpine images * Adapt the same workaround used by Openshift to strip the "search ." * This only applies to bare-metal Typhoon nodes (where hostnames are set to fqdn's), nodes on cloud platforms aren't affected in the Typhoon configuration Kubernetes tracking issue: kubernetes/kubernetes#112135 Rel: * systemd/systemd#17201 * kubernetes/kubernetes#109441 * coreos/fedora-coreos-tracker#1287 * openshift/okd-machine-os#159
* Adapt kubernetes#109441 but ensures that `search .` does not get propagated into containers' /etc/resolv.conf. There is no reason to put `.` in a container's search field and it causes issues for musl
* systemd adds "search ." to hosts /run/systemd/resolve/resolv.conf on hosts with a fqdn hostname * Kubelet v1.25 began propagating "search ." from the host node into containers' `/etc/resolv.conf` * musl-based DNS resolvers don't behave correctly when `search .` is used in their `/etc/resolv.conf`. This breaks Alpine images * Adapt the same workaround used by Openshift to strip the "search ." * This only applies to bare-metal Typhoon nodes (where hostnames are set to fqdn's), nodes on cloud platforms aren't affected in the Typhoon configuration Kubernetes tracking issue: kubernetes/kubernetes#112135 Rel: * systemd/systemd#17201 * kubernetes/kubernetes#109441 * coreos/fedora-coreos-tracker#1287 * openshift/okd-machine-os#159
What type of PR is this?
/kind bug
/sig network
What this PR does / why we need it:
When parsing a
resolv.conf
file that hassearch .
,parseResolvConf
should accept the "." entry verbatim. Before this change,parseResolvConf
unconditionally trimmed the "." suffix, which in the case of "." resulted in a "" entry (that is, the empty string). This empty entry could leadparseResolvConf
to produce aresolv.conf
file withsearch
. Resolvers could fail to parse such aresolv.conf
file fromparseResolvConf
, thus breaking DNS resolution in pods. After this change,parseResolvConf
accepts aresolv.conf
file withsearch .
and passes the "." entry through verbatim to produce a validresolv.conf
file. The "." suffix is still trimmed for any entry that does not solely comprise ".".Follow-up to a215a88.
pkg/kubelet/network/dns/dns.go
(parseResolvConf
): Handle a "." entry in the search path by copying it verbatim.pkg/kubelet/network/dns/dns_test.go
(TestParseResolvConf
): Add a test case forsearch .
.Which issue(s) this PR fixes:
Failures related to
search .
inresolv.conf
were reported for OKD running on Fedora in https://bugzilla.redhat.com/show_bug.cgi?id=2063414. The issue can be caused by this systemd change: systemd/systemd#17201Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: