Skip to content

feat(dns): embedded DNS server instead of coreDNS #13124

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

Merged
merged 31 commits into from
Apr 7, 2025
Merged

Conversation

lahabana
Copy link
Contributor

@lahabana lahabana commented Mar 17, 2025

Motivation

Add an embedded DNS server that resolves mesh local hostnames.
This feature is disabled by default and needs to be enabled with kuma_dns_proxy_port on DPs in universal to a non 0 value or simply with kuma_runtime_kubernetes_injector_builtin_dns_experimental_proxy =true on CP in Kubernetes (this leverages sidecar injection).

Implementation information

We leveraged the same system as MeshMetric. The DNS map is exposed in the _kuma:dynamicconfig listener, we poll this from the DP at a set frequency and reload the data.

We improved the whole configfetcher to make it reusable across different components. We also setup the endpoint so that it works with etag caching while avoids unnecessary reloading and processing, we can therefore refresh the configs more quickly as well.

For the moment I added a new matrix for e2e test. If this is stable I'll switch the default.

For DNS proxy we'd like to reuse the same configFetcher mechanism
Split it out to its own package and define an api to add handlers

Also add support for the handlers to handle etag to avoid reloading when
the config doesn't change

Signed-off-by: Charly Molter <[email protected]>
Also add support for etag caching

Signed-off-by: Charly Molter <[email protected]>
Signed-off-by: Charly Molter <[email protected]>
@lahabana lahabana added the ci/run-full-matrix PR: Runs all possible e2e test combination (expensive use carefully) label Mar 17, 2025
@lahabana
Copy link
Contributor Author

I've made a few separate commits as this is a bit of a mouthful

Copy link
Contributor

Reviewer Checklist

🔍 Each of these sections need to be checked by the reviewer of the PR 🔍:
If something doesn't apply please check the box and add a justification if the reason is non obvious.

  • Is the PR title satisfactory? Is this part of a larger feature and should be grouped using > Changelog?
  • PR description is clear and complete. It Links to relevant issue as well as docs and UI issues
  • This will not break child repos: it doesn't hardcode values (.e.g "kumahq" as an image registry)
  • IPv6 is taken into account (.e.g: no string concatenation of host port)
  • Tests (Unit test, E2E tests, manual test on universal and k8s)
    • Don't forget ci/ labels to run additional/fewer tests
  • Does this contain a change that needs to be notified to users? In this case, UPGRADE.md should be updated.
  • Does it need to be backported according to the backporting policy? (this GH action will add "backport" label based on these file globs, if you want to prevent it from adding the "backport" label use no-backport-autolabel label)

Signed-off-by: Charly Molter <[email protected]>
@lahabana lahabana changed the title feat(dns): running embedded DNS server instead of coreDNS feat(dns): embedded DNS server instead of coreDNS Mar 18, 2025
@lahabana lahabana marked this pull request as ready for review March 18, 2025 12:49
@lahabana lahabana requested a review from a team as a code owner March 18, 2025 12:49
@lahabana lahabana requested review from slonka and lobkovilya March 18, 2025 12:49
@lahabana lahabana self-assigned this Mar 18, 2025
@bartsmykla bartsmykla self-requested a review March 19, 2025 07:04
Copy link
Contributor

@slonka slonka left a comment

Choose a reason for hiding this comment

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

In general looks promising, a bunch of questions and nitpicks. Also would love to see a simple test checking integration with a DNS client.


  • Is the PR title satisfactory? Is this part of a larger feature and should be grouped using > Changelog?
  • PR description is clear and complete. It Links to relevant issue as well as docs and UI issues
  • This will not break child repos: it doesn't hardcode values (.e.g "kumahq" as an image registry)
  • IPv6 is taken into account (.e.g: no string concatenation of host port)
  • Tests (Unit test, E2E tests, manual test on universal and k8s)
    • Don't forget ci/ labels to run additional/fewer tests
  • Does this contain a change that needs to be notified to users? In this case, UPGRADE.md should be updated.
  • Does it need to be backported according to the backporting policy? (this GH action will add "backport" label based on these file globs, if you want to prevent it from adding the "backport" label use no-backport-autolabel label)

@lahabana
Copy link
Contributor Author

@lobkovilya pushed a new commit which simplifies the handler API as it doesn't deal with shutdowns anymore (that's what components are for)

Signed-off-by: Charly Molter <[email protected]>
Signed-off-by: Charly Molter <[email protected]>
slonka
slonka previously approved these changes Apr 2, 2025
@slonka
Copy link
Contributor

slonka commented Apr 2, 2025

Oh, did not notice that the tests are failing.

Signed-off-by: Charly Molter <[email protected]>
lahabana added 2 commits April 3, 2025 10:32
Signed-off-by: Charly Molter <[email protected]>
Signed-off-by: Charly Molter <[email protected]>
@lahabana lahabana merged commit 725d605 into master Apr 7, 2025
26 checks passed
@lahabana lahabana deleted the experimentDNS branch April 7, 2025 14:23
lobkovilya pushed a commit to lobkovilya/kuma that referenced this pull request Apr 24, 2025
## Motivation

Add an embedded DNS server that resolves mesh local hostnames.
This feature is disabled by default and needs to be enabled with
`kuma_dns_proxy_port ` on DPs in universal to a non 0 value or simply
with `kuma_runtime_kubernetes_injector_builtin_dns_experimental_proxy
=true` on CP in Kubernetes (this leverages sidecar injection).

## Implementation information

We leveraged the same system as MeshMetric. The DNS map is exposed in
the `_kuma:dynamicconfig` listener, we poll this from the DP at a set
frequency and reload the data.

We improved the whole configfetcher to make it reusable across different
components. We also setup the endpoint so that it works with [etag
caching](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/ETag)
while avoids unnecessary reloading and processing, we can therefore
refresh the configs more quickly as well.

For the moment I added a new matrix for e2e test. If this is stable I'll
switch the default.

---------

Signed-off-by: Charly Molter <[email protected]>
Signed-off-by: Ilya Lobkov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/run-full-matrix PR: Runs all possible e2e test combination (expensive use carefully)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants