[PR #3264] [CLOSED] Tidy up ResolverOpts usage, default trait impls #3693

New issue

Closed

opened 2026-03-16 11:57:30 +03:00 by kerem · 0 comments

kerem commented

2026-03-16 11:57:30 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/hickory-dns/hickory-dns/pull/3264
Author: @cpu
Created: 9/5/2025
Status: ❌ Closed

Base: main ← Head: cpu-tidying-recursor-opts_dev

📝 Commits (5)

f239ec3 config: introduce ConnectionOpts type
15bc67e recursor: extract case randomization to constructor arg
8778471 resolver: split out NameServerOptions from ResolverOpts
02f5982 use NameServerOptions for resolver name servers/pools
c05abfd replace manual Default impl for enums

📊 Changes

15 files changed (+313 additions, -257 deletions)

View changed files

📝 bin/tests/integration/config_tests.rs (+1 -0)
📝 conformance/dns-test/src/implementation.rs (+2 -7)
📝 crates/proto/src/dnssec/rdata/key.rs (+6 -21)
📝 crates/proto/src/op/response_code.rs (+2 -7)
📝 crates/recursor/src/recursor_dns_handle.rs (+26 -31)
📝 crates/recursor/src/recursor_pool.rs (+4 -2)
📝 crates/resolver/src/config.rs (+108 -66)
📝 crates/resolver/src/name_server/connection_provider.rs (+11 -6)
📝 crates/resolver/src/name_server/name_server.rs (+14 -15)
📝 crates/resolver/src/name_server/name_server_pool.rs (+12 -12)
📝 crates/resolver/src/resolver.rs (+2 -3)
📝 crates/resolver/src/system_conf/unix.rs (+14 -3)
📝 crates/server/src/zone_handler/auth_lookup.rs (+4 -14)
📝 tests/integration-tests/src/mock_client.rs (+3 -2)
📝 tests/integration-tests/tests/integration/name_server_pool_tests.rs (+104 -68)

📄 Description

The configuration situation for name servers & name server pools was complicated by the pervasive use of the ResoverOpts struct. This struct has a lot of configuration options, but very few are meaningfully used by these lower layers, resulting in some confusing behaviour (e.g. unit tests setting fields that have no effect on the actual implementation). For instance, name_server_pool_tests.rs tests were setting try_tcp_on_error at a layer where the value was never consulted, and the recursor_dns_handle.rs recursor_opts() func was setting/documenting fields like validate that had no effect.

I suspect there's further room for improvement; in particular, both before & after this branch we're using hard-coded values for NameServerOptions configuration when building the root server pool, and when creating a name server pool for a zone. For now since this is a large diff already I've called this out with a couple of TODO comments while preserving the values used previously.

Along the way I also took this as a chance to tidy up a handful of manual Default impls on enums that could be replaced with derived implementations by marking the correct enum variant as the #[default].

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/hickory-dns/hickory-dns/pull/3264 **Author:** [@cpu](https://github.com/cpu) **Created:** 9/5/2025 **Status:** ❌ Closed **Base:** `main` ← **Head:** `cpu-tidying-recursor-opts_dev` --- ### 📝 Commits (5) - [`f239ec3`](https://github.com/hickory-dns/hickory-dns/commit/f239ec3d1c1a259fdf5b6b9676640455f569365c) config: introduce ConnectionOpts type - [`15bc67e`](https://github.com/hickory-dns/hickory-dns/commit/15bc67e3630547acc3d772e3b62885302383f8af) recursor: extract case randomization to constructor arg - [`8778471`](https://github.com/hickory-dns/hickory-dns/commit/877847139496bddf39aff4b53a920ab2076af408) resolver: split out NameServerOptions from ResolverOpts - [`02f5982`](https://github.com/hickory-dns/hickory-dns/commit/02f5982047cb1a4e57116ca464c37a7f89dd654c) use NameServerOptions for resolver name servers/pools - [`c05abfd`](https://github.com/hickory-dns/hickory-dns/commit/c05abfdcc401ec445df5b237ed00729771f1bb7e) replace manual Default impl for enums ### 📊 Changes **15 files changed** (+313 additions, -257 deletions) <details> <summary>View changed files</summary> 📝 `bin/tests/integration/config_tests.rs` (+1 -0) 📝 `conformance/dns-test/src/implementation.rs` (+2 -7) 📝 `crates/proto/src/dnssec/rdata/key.rs` (+6 -21) 📝 `crates/proto/src/op/response_code.rs` (+2 -7) 📝 `crates/recursor/src/recursor_dns_handle.rs` (+26 -31) 📝 `crates/recursor/src/recursor_pool.rs` (+4 -2) 📝 `crates/resolver/src/config.rs` (+108 -66) 📝 `crates/resolver/src/name_server/connection_provider.rs` (+11 -6) 📝 `crates/resolver/src/name_server/name_server.rs` (+14 -15) 📝 `crates/resolver/src/name_server/name_server_pool.rs` (+12 -12) 📝 `crates/resolver/src/resolver.rs` (+2 -3) 📝 `crates/resolver/src/system_conf/unix.rs` (+14 -3) 📝 `crates/server/src/zone_handler/auth_lookup.rs` (+4 -14) 📝 `tests/integration-tests/src/mock_client.rs` (+3 -2) 📝 `tests/integration-tests/tests/integration/name_server_pool_tests.rs` (+104 -68) </details> ### 📄 Description The configuration situation for name servers & name server pools was complicated by the pervasive use of the `ResoverOpts` struct. This struct has a lot of configuration options, but very few are meaningfully used by these lower layers, resulting in some confusing behaviour (e.g. unit tests setting fields that have no effect on the actual implementation). For instance, `name_server_pool_tests.rs` tests were setting `try_tcp_on_error` at a layer where the value was never consulted, and the `recursor_dns_handle.rs` `recursor_opts()` func was setting/documenting fields like `validate` that had no effect. I suspect there's further room for improvement; in particular, both before & after this branch we're using hard-coded values for `NameServerOptions` configuration when building the root server pool, and when creating a name server pool for a zone. For now since this is a large diff already I've called this out with a couple of TODO comments while preserving the values used previously. Along the way I also took this as a chance to tidy up a handful of manual `Default` impls on enums that could be replaced with derived implementations by marking the correct enum variant as the `#[default]`. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>