diff --git a/TESTING.md b/TESTING.md new file mode 100644 index 0000000..3dd34c4 --- /dev/null +++ b/TESTING.md @@ -0,0 +1,34 @@ +# Testing Policy + +## DNS Resolution Tests + +All resolver tests **MUST** use live queries against real DNS servers. +No mocking of the DNS client layer is permitted. + +### Rationale + +The resolver performs iterative resolution from root nameservers through +the full delegation chain. Mocked responses cannot faithfully represent +the variety of real-world DNS behavior (truncation, referrals, glue +records, DNSSEC, varied response times, EDNS, etc.). Testing against +real servers ensures the resolver works correctly in production. + +### Constraints + +- Tests hit real DNS infrastructure and require network access +- Test duration depends on network conditions; timeout tuning keeps + the suite within the 30-second target +- Query timeout is calibrated to 3× maximum antipodal RTT (~300ms) + plus processing margin +- Root server fan-out is limited to reduce parallel query load +- Flaky failures from transient network issues are acceptable and + should be investigated as potential resolver bugs, not papered over + with mocks or skip flags + +### What NOT to do + +- **Do not mock `DNSClient`** for resolver tests (the mock constructor + exists for unit-testing other packages that consume the resolver) +- **Do not add `-short` flags** to skip slow tests +- **Do not increase `-timeout`** to hide hanging queries +- **Do not modify linter configuration** to suppress findings diff --git a/internal/resolver/iterative.go b/internal/resolver/iterative.go index 8f41b6d..68ce52f 100644 --- a/internal/resolver/iterative.go +++ b/internal/resolver/iterative.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "math/rand" "net" "sort" "strings" @@ -13,7 +14,7 @@ import ( ) const ( - queryTimeoutDuration = 5 * time.Second + queryTimeoutDuration = 2 * time.Second maxRetries = 2 maxDelegation = 20 timeoutMultiplier = 2 @@ -41,6 +42,22 @@ func rootServerList() []string { } } +const maxRootServers = 3 + +// randomRootServers returns a shuffled subset of root servers. +func randomRootServers() []string { + all := rootServerList() + rand.Shuffle(len(all), func(i, j int) { + all[i], all[j] = all[j], all[i] + }) + + if len(all) > maxRootServers { + return all[:maxRootServers] + } + + return all +} + func checkCtx(ctx context.Context) error { err := ctx.Err() if err != nil { @@ -302,7 +319,7 @@ func (r *Resolver) resolveNSRecursive( msg.SetQuestion(domain, dns.TypeNS) msg.RecursionDesired = true - for _, ip := range rootServerList()[:3] { + for _, ip := range randomRootServers() { if checkCtx(ctx) != nil { return nil, ErrContextCanceled } @@ -333,7 +350,7 @@ func (r *Resolver) resolveARecord( msg.SetQuestion(hostname, dns.TypeA) msg.RecursionDesired = true - for _, ip := range rootServerList()[:3] { + for _, ip := range randomRootServers() { if checkCtx(ctx) != nil { return nil, ErrContextCanceled } @@ -385,7 +402,7 @@ func (r *Resolver) FindAuthoritativeNameservers( candidate := strings.Join(labels[i:], ".") + "." nsNames, err := r.followDelegation( - ctx, candidate, rootServerList(), + ctx, candidate, randomRootServers(), ) if err == nil && len(nsNames) > 0 { sort.Strings(nsNames) diff --git a/internal/state/state.go b/internal/state/state.go index fca7276..dd4561d 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -156,8 +156,8 @@ func (s *State) Load() error { // Save writes the current state to disk atomically. func (s *State) Save() error { - s.mu.RLock() - defer s.mu.RUnlock() + s.mu.Lock() + defer s.mu.Unlock() s.snapshot.LastUpdated = time.Now().UTC()