kubelet: parseResolvConf: Handle "search ." #109441
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 .".
k8s-ci-robot
added
release-note
|
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. |
k8s-ci-robot
added
needs-priority
k8s-ci-robot
added
area/kubelet
sig/node
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
|
/lgtm /assign @thockin |
k8s-ci-robot
added
lgtm
|
/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 |
k8s-ci-robot
added
the
triage/accepted
k8s-ci-robot
added
priority/important-soon
This was referenced Aug 31, 2022
dghubble
added a commit
to poseidon/typhoon
that referenced
this pull request
Aug 31, 2022
* 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
dghubble
added a commit
to poseidon/typhoon
that referenced
this pull request
Aug 31, 2022
* 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
dghubble
added a commit
to dghubble/kubernetes
that referenced
this pull request
Sep 1, 2022
* 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
lucab
pushed a commit
to lucab/kubernetes
that referenced
this pull request
Sep 2, 2022
* 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
elemental-lf
pushed a commit
to elemental-lf/typhoon
that referenced
this pull request
Sep 2, 2022
* 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
danielye11
pushed a commit
to danielye11/kubernetes
that referenced
this pull request
Sep 21, 2022
* 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
Snaipe
pushed a commit
to aristanetworks/monsoon
that referenced
this pull request
Apr 13, 2023
* 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind bug
/sig network
What this PR does / why we need it:
When parsing a
resolv.conffile that hassearch .,parseResolvConfshould accept the "." entry verbatim. Before this change,parseResolvConfunconditionally trimmed the "." suffix, which in the case of "." resulted in a "" entry (that is, the empty string). This empty entry could leadparseResolvConfto produce aresolv.conffile withsearch. Resolvers could fail to parse such aresolv.conffile fromparseResolvConf, thus breaking DNS resolution in pods. After this change,parseResolvConfaccepts aresolv.conffile withsearch .and passes the "." entry through verbatim to produce a validresolv.conffile. 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.confwere 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.: