Migrate DNS server to dnsv2#382
Conversation
|
That was easier than I thought. Haven't look indepth yet. But for
forwarding you could now just resend Msg.Data without fiddling with it.
Should be less code and less policy in uncloudd
…On Fri, May 29, 2026, 08:56 Zasda Yusuf Mikail ***@***.***> wrote:
Closes #341 <#341>.
Summary
- migrate the embedded machine DNS server from github.com/miekg/dns v1
to codeberg.org/miekg/dns v2
- adapt response construction, handler signatures, forwarding, EDNS
UDP sizing, resolv.conf parsing, and A record construction to the v2 APIs
- add narrow module replacements to keep the existing Caddy dependency
stack compiling with dnsv2 transitive requirements
Tests
- go test -count=1 ./internal/machine/dns
./internal/machine/caddyconfig
- go test $(go list ./... | grep -v '/test/e2e$')\n\nFull go test ./...
was also run; only test/e2e failed because machine readiness checks
could not connect (error reading server preface: EOF).
------------------------------
You can view, comment on, or merge this pull request online at:
#382
Commit Summary
- 6e2dceb
<6e2dceb>
Migrate DNS server to dnsv2
File Changes
(3 files <https://github.com/psviderski/uncloud/pull/382/files>)
- *M* go.mod
<https://github.com/psviderski/uncloud/pull/382/files#diff-33ef32bf6c23acb95f5902d7097b7a1d5128ca061167ec0716715b0b9eeaa5f6>
(60)
- *M* go.sum
<https://github.com/psviderski/uncloud/pull/382/files#diff-3295df7234525439d778f1b282d146a4f1ff6b415248aaac074e8042d9f42d63>
(108)
- *M* internal/machine/dns/server.go
<https://github.com/psviderski/uncloud/pull/382/files#diff-3ac9fdddc2ce3e22855eba1451284516c94e417de9324e2f3ae094969a150407>
(108)
Patch Links:
- https://github.com/psviderski/uncloud/pull/382.patch
- https://github.com/psviderski/uncloud/pull/382.diff
—
Reply to this email directly, view it on GitHub
<#382?email_source=notifications&email_token=AACWIW737GYALFUBZUTN5KT45EYDLA5CNFSNUABEM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UF4ZTONRXGA4TSNJZG6THEZLBONXW5KTTOVRHGY3SNFRGKZFFMV3GK3TUVRTG633UMVZF6Y3MNFRWW>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACWIW2STQ673R34ETEFABT45EYDLAVCNFSM6AAAAACZSAF7AWVHI2DSMVQWIX3LMV43ASLTON2WKOZUGU2DMMZYGA4TCOA>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/AACWIW7WZRZPQC4OA5YCX5D45EYDLA5CNFSNUABEM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UF4ZTONRXGA4TSNJZG6THEZLBONXW5KTTOVRHGY3SNFRGKZFFMV3GK3TUVJTG633UMVZF62LPOM>
and Android
<https://github.com/notifications/mobile/android/AACWIWYYTLEJ3HCW3NYF2NL45EYDLA5CNFSNUABEM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UF4ZTONRXGA4TSNJZG6THEZLBONXW5KTTOVRHGY3SNFRGKZFFMV3GK3TUVZTG633UMVZF6YLOMRZG62LE>.
Download it today!
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
| var lastErr error | ||
| for _, server := range s.upstreamServers { | ||
| resp, _, err := client.Exchange(req, server.String()) | ||
| resp, err := forwardRaw(ctx, req.Data, proto, server.String()) |
There was a problem hiding this comment.
this should be as simple as io.Copy, if it's not are you ran into problem I would like to know, because then the lib. needs to change.
There was a problem hiding this comment.
Why io.Copy instead of dns.Exchange?
| return resp | ||
| } | ||
|
|
||
| func truncate(msg *dns.Msg, maxSize int) { |
There was a problem hiding this comment.
this is probably wrong, as you will returns split up RRsets (which is an unfortunate DNS thing). But apparantly this is difficult enough that this needs a helper. https://codeberg.org/miekg/dns/issues/882
| return strings.TrimSuffix(name, "."+InternalDomain) | ||
| } | ||
|
|
||
| func newResponse(req *dns.Msg, rcode uint16) *dns.Msg { |
There was a problem hiding this comment.
you can, prolly 100% of the time, just the incoming message Reset it and reuse it.
There was a problem hiding this comment.
Yeah, let's make it simpler.
zasdaym
force-pushed
the
fix/dnsv2
branch
7 times, most recently
from
c919abd to
175553f
Compare
|
|
||
| replace github.com/caddyserver/certmagic v0.25.2 => github.com/caddyserver/certmagic v0.21.4 | ||
|
|
||
| replace github.com/caddyserver/zerossl v0.1.5 => github.com/caddyserver/zerossl v0.1.3 |
There was a problem hiding this comment.
@miekg why do we need this Caddy stuff for a DNS library? Does it provide a DNS server now as well?
As we have migrated from the JSON caddy config (https://github.com/psviderski/uncloud/blob/5cc005a4237bf7e5367e4570cce5e5f90748d155/internal/machine/caddyconfig/jsonconfig.go) to Caddyfile generation, we can now drop the caddy/v2 Go dependency which is quite heavy.
But this PR seems to brings some of the caddy packages back.
There was a problem hiding this comment.
honestly don't know - this pr is on my list to review again
(or do it myself first and then see what I come up with)
It should not have to do anything with caddy, as both dns and dnsv2 can be used independently. Even though the deps for dnsv2 are heavier than they should be, as atomdns is also there - on the TODO to rip that out as well. That should prolly by done before including it here as a dep - but as said, I want to look at this more closely.
(there is no rush to merge this btw)
There was a problem hiding this comment.
@psviderski we can remove zerossl but not with certmagic.
There was a problem hiding this comment.
Thank you both for the context. I'll then leave this PR to you guys to decide how we go with this migration
zasdaym
force-pushed
the
fix/dnsv2
branch
2 times, most recently
from
faf9040 to
4f40919
Compare
Signed-off-by: Zasda Mikail <zasdaym@gmail.com>
3 participants
Closes #341.