Skip to content

dnscache: use nameservers from cli (remove node/proxy)#48

Merged
k8s-ci-robot merged 3 commits intokubernetes-sigs:mainfrom
aojea:node_proxy_rbac
Feb 3, 2026
Merged

dnscache: use nameservers from cli (remove node/proxy)#48
k8s-ci-robot merged 3 commits intokubernetes-sigs:mainfrom
aojea:node_proxy_rbac

Conversation

@aojea
Copy link
Contributor

@aojea aojea commented Jan 29, 2026

kindnet was using a "smart" hack to obtainer the cluster nameservers for the dnscache functionality, so the nfqueue didn't have to inspect all DNS traffic and just the one associated to the cluster.

However, the node/proxy permissions are risky and users can pass this information via flags, since is not likely going to change and despite is per node, it was always used as a global value.

It also makes this feature disabled by default, since there is some flakiness associated that we never were able to figure out, but it can also be possible it is associated to some kernel bug or behavior associated to the resolvers (happy eyeballs and similar)

Fixes #47

/cc @liggitt

@k8s-ci-robot k8s-ci-robot requested a review from liggitt January 29, 2026 14:02
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: aojea

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 29, 2026
kindnet was using a "smart" hack to obtainer the cluster nameservers
for the dnscache functionality, so the nfqueue didn't have to inspect
all DNS traffic and just the one associated to the cluster.

However, the node/proxy permissions are risky and users can pass this
information via flags, since is not likely going to change and despite
is per node, it was always used as a global value.

It also makes this feature disabled by default, since there is some
flakiness associated that we never were able to figure out, but it can
also be possible it is associated to some kernel bug or behavior
associated to the resolvers (happy eyeballs and similar)
}

d.clusterDomain = kubeletConfig.ClusterDomain
resolvPath := "/etc/resolv.conf"
Copy link
Contributor

Choose a reason for hiding this comment

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

I see the CLI arg for the nameservers, do we not need to be told what resolv.conf to read any more?

Copy link
Contributor Author

@aojea aojea Feb 1, 2026

Choose a reason for hiding this comment

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

this feature needs the DNS nameserver IP to narrow the network filter down and avoid the impact of diverting traffic from kernel to userspace.
The trick I was using is to read the resolv.conf that was going to be populated on the pods, to get the cluster DNS nameserver IP, but this requires to read the kubelet config ... I tried to get as much information as possible from all the sources, but it looks it was just a bad idea and now we can ask admins to configure those ips directly

aojea and others added 2 commits January 31, 2026 23:24
Co-authored-by: Jordan Liggitt <jordan@liggitt.net>
@aojea
Copy link
Contributor Author

aojea commented Feb 1, 2026

@liggitt PTAL

@liggitt
Copy link
Contributor

liggitt commented Feb 3, 2026

/lgtm
/hold in case you want more eyes or testing before merge, feel free to unhold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 3, 2026
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Feb 3, 2026
@aojea
Copy link
Contributor Author

aojea commented Feb 3, 2026

/lgtm /hold in case you want more eyes or testing before merge, feel free to unhold

/hold cancel

This feature is inherited from my own fork, and in hindsight it was not a good idea

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 3, 2026
@k8s-ci-robot k8s-ci-robot merged commit 724a947 into kubernetes-sigs:main Feb 3, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use finegrained kubelet API authorization

3 participants