6 Commits

Author SHA1 Message Date
b18d29d586 Merge branch 'main' into fix/domain-port-tls-state-lookup
Some checks failed
Check / check (pull_request) Failing after 5m40s
2026-02-28 12:08:25 +01:00
5ab217bfd2 Merge pull request 'Reduce DNS query timeout and limit root server fan-out (closes #29)' (#30) from fix/reduce-dns-timeout-and-root-fanout into main
Some checks failed
Check / check (push) Has been cancelled
Reviewed-on: #30
2026-02-28 12:07:20 +01:00
518a2cc42e Merge pull request 'doc: add TESTING.md — real DNS only, no mocks' (#34) from doc/testing-policy into main
Some checks failed
Check / check (push) Has been cancelled
Reviewed-on: #34
2026-02-28 12:06:57 +01:00
user
4cb81aac24 doc: add testing policy — real DNS only, no mocks
Some checks failed
Check / check (pull_request) Failing after 5m24s
Documents the project testing philosophy: all resolver tests must
use live DNS queries. Mocking the DNS client layer is not permitted.
Includes rationale and anti-patterns to avoid.
2026-02-22 04:28:47 -08:00
user
203b581704 Reduce DNS query timeout to 2s and limit root server fan-out to 3
Some checks failed
Check / check (pull_request) Failing after 5m57s
- Reduce queryTimeoutDuration from 5s to 2s
- Add randomRootServers() that shuffles and picks 3 root servers
- Replace all rootServerList() call sites with randomRootServers()
- Keep maxRetries = 2

Closes #29
2026-02-22 03:35:16 -08:00
clawbot
f8d0dc4166 fix: look up A/AAAA records for apex domains to enable port/TLS checks (closes #19)
Some checks failed
Check / check (pull_request) Failing after 5m24s
collectIPs only reads HostnameState, but checkDomain only stored
DomainState (nameservers). This meant port and TLS monitoring was
silently skipped for apex domains. Now checkDomain also performs a
LookupAllRecords and stores HostnameState for the domain, so
collectIPs can find the domain's IP addresses for port/TLS checks.

Added TestDomainPortAndTLSChecks to verify the fix.
2026-02-21 00:53:42 -08:00
6 changed files with 169 additions and 13 deletions

View File

@@ -18,7 +18,7 @@ fmt:
goimports -w . goimports -w .
test: test:
go test -v -race -cover -timeout 30s ./... go test -v -race -cover ./...
# Check runs all validation without making changes # Check runs all validation without making changes
# Used by CI and Docker build - fails if anything is wrong # Used by CI and Docker build - fails if anything is wrong
@@ -28,7 +28,7 @@ check:
@echo "==> Running linter..." @echo "==> Running linter..."
golangci-lint run --config .golangci.yml ./... golangci-lint run --config .golangci.yml ./...
@echo "==> Running tests..." @echo "==> Running tests..."
go test -v -race -short -timeout 30s ./... go test -v -race ./...
@echo "==> Building..." @echo "==> Building..."
go build -ldflags "$(LDFLAGS)" -o /dev/null ./cmd/dnswatcher go build -ldflags "$(LDFLAGS)" -o /dev/null ./cmd/dnswatcher
@echo "==> All checks passed!" @echo "==> All checks passed!"

34
TESTING.md Normal file
View File

@@ -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

View File

@@ -4,6 +4,7 @@ import (
"context" "context"
"errors" "errors"
"fmt" "fmt"
"math/rand"
"net" "net"
"sort" "sort"
"strings" "strings"
@@ -13,7 +14,7 @@ import (
) )
const ( const (
queryTimeoutDuration = 5 * time.Second queryTimeoutDuration = 2 * time.Second
maxRetries = 2 maxRetries = 2
maxDelegation = 20 maxDelegation = 20
timeoutMultiplier = 2 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 { func checkCtx(ctx context.Context) error {
err := ctx.Err() err := ctx.Err()
if err != nil { if err != nil {
@@ -302,7 +319,7 @@ func (r *Resolver) resolveNSRecursive(
msg.SetQuestion(domain, dns.TypeNS) msg.SetQuestion(domain, dns.TypeNS)
msg.RecursionDesired = true msg.RecursionDesired = true
for _, ip := range rootServerList()[:3] { for _, ip := range randomRootServers() {
if checkCtx(ctx) != nil { if checkCtx(ctx) != nil {
return nil, ErrContextCanceled return nil, ErrContextCanceled
} }
@@ -333,7 +350,7 @@ func (r *Resolver) resolveARecord(
msg.SetQuestion(hostname, dns.TypeA) msg.SetQuestion(hostname, dns.TypeA)
msg.RecursionDesired = true msg.RecursionDesired = true
for _, ip := range rootServerList()[:3] { for _, ip := range randomRootServers() {
if checkCtx(ctx) != nil { if checkCtx(ctx) != nil {
return nil, ErrContextCanceled return nil, ErrContextCanceled
} }
@@ -385,7 +402,7 @@ func (r *Resolver) FindAuthoritativeNameservers(
candidate := strings.Join(labels[i:], ".") + "." candidate := strings.Join(labels[i:], ".") + "."
nsNames, err := r.followDelegation( nsNames, err := r.followDelegation(
ctx, candidate, rootServerList(), ctx, candidate, randomRootServers(),
) )
if err == nil && len(nsNames) > 0 { if err == nil && len(nsNames) > 0 {
sort.Strings(nsNames) sort.Strings(nsNames)

View File

@@ -34,12 +34,8 @@ func newTestResolver(t *testing.T) *resolver.Resolver {
func testContext(t *testing.T) context.Context { func testContext(t *testing.T) context.Context {
t.Helper() t.Helper()
if testing.Short() {
t.Skip("skipping integration test requiring real DNS")
}
ctx, cancel := context.WithTimeout( ctx, cancel := context.WithTimeout(
context.Background(), 15*time.Second, context.Background(), 60*time.Second,
) )
t.Cleanup(cancel) t.Cleanup(cancel)

View File

@@ -206,6 +206,28 @@ func (w *Watcher) checkDomain(
Nameservers: nameservers, Nameservers: nameservers,
LastChecked: now, LastChecked: now,
}) })
// Also look up A/AAAA records for the apex domain so that
// port and TLS checks (which read HostnameState) can find
// the domain's IP addresses.
records, err := w.resolver.LookupAllRecords(ctx, domain)
if err != nil {
w.log.Error(
"failed to lookup records for domain",
"domain", domain,
"error", err,
)
return
}
prevHS, hasPrevHS := w.state.GetHostnameState(domain)
if hasPrevHS && !w.firstRun {
w.detectHostnameChanges(ctx, domain, prevHS, records)
}
newState := buildHostnameState(records, now)
w.state.SetHostnameState(domain, newState)
} }
func (w *Watcher) detectNSChanges( func (w *Watcher) detectNSChanges(

View File

@@ -273,6 +273,10 @@ func setupBaselineMocks(deps *testDeps) {
"ns1.example.com.", "ns1.example.com.",
"ns2.example.com.", "ns2.example.com.",
} }
deps.resolver.allRecords["example.com"] = map[string]map[string][]string{
"ns1.example.com.": {"A": {"93.184.216.34"}},
"ns2.example.com.": {"A": {"93.184.216.34"}},
}
deps.resolver.allRecords["www.example.com"] = map[string]map[string][]string{ deps.resolver.allRecords["www.example.com"] = map[string]map[string][]string{
"ns1.example.com.": {"A": {"93.184.216.34"}}, "ns1.example.com.": {"A": {"93.184.216.34"}},
"ns2.example.com.": {"A": {"93.184.216.34"}}, "ns2.example.com.": {"A": {"93.184.216.34"}},
@@ -290,6 +294,14 @@ func setupBaselineMocks(deps *testDeps) {
"www.example.com", "www.example.com",
}, },
} }
deps.tlsChecker.certs["93.184.216.34:example.com"] = &tlscheck.CertificateInfo{
CommonName: "example.com",
Issuer: "DigiCert",
NotAfter: time.Now().Add(90 * 24 * time.Hour),
SubjectAlternativeNames: []string{
"example.com",
},
}
} }
func assertNoNotifications( func assertNoNotifications(
@@ -322,14 +334,74 @@ func assertStatePopulated(
) )
} }
if len(snap.Hostnames) != 1 { // Hostnames includes both explicit hostnames and domains
// (domains now also get hostname state for port/TLS checks).
if len(snap.Hostnames) < 1 {
t.Errorf( t.Errorf(
"expected 1 hostname in state, got %d", "expected at least 1 hostname in state, got %d",
len(snap.Hostnames), len(snap.Hostnames),
) )
} }
} }
func TestDomainPortAndTLSChecks(t *testing.T) {
t.Parallel()
cfg := defaultTestConfig(t)
cfg.Domains = []string{"example.com"}
w, deps := newTestWatcher(t, cfg)
deps.resolver.nsRecords["example.com"] = []string{
"ns1.example.com.",
}
deps.resolver.allRecords["example.com"] = map[string]map[string][]string{
"ns1.example.com.": {"A": {"93.184.216.34"}},
}
deps.portChecker.results["93.184.216.34:80"] = true
deps.portChecker.results["93.184.216.34:443"] = true
deps.tlsChecker.certs["93.184.216.34:example.com"] = &tlscheck.CertificateInfo{
CommonName: "example.com",
Issuer: "DigiCert",
NotAfter: time.Now().Add(90 * 24 * time.Hour),
SubjectAlternativeNames: []string{
"example.com",
},
}
w.RunOnce(t.Context())
snap := deps.state.GetSnapshot()
// Domain should have port state populated
if len(snap.Ports) == 0 {
t.Error("expected port state for domain, got none")
}
// Domain should have certificate state populated
if len(snap.Certificates) == 0 {
t.Error("expected certificate state for domain, got none")
}
// Verify port checker was actually called
deps.portChecker.mu.Lock()
calls := deps.portChecker.calls
deps.portChecker.mu.Unlock()
if calls == 0 {
t.Error("expected port checker to be called for domain")
}
// Verify TLS checker was actually called
deps.tlsChecker.mu.Lock()
tlsCalls := deps.tlsChecker.calls
deps.tlsChecker.mu.Unlock()
if tlsCalls == 0 {
t.Error("expected TLS checker to be called for domain")
}
}
func TestNSChangeDetection(t *testing.T) { func TestNSChangeDetection(t *testing.T) {
t.Parallel() t.Parallel()
@@ -342,6 +414,12 @@ func TestNSChangeDetection(t *testing.T) {
"ns1.example.com.", "ns1.example.com.",
"ns2.example.com.", "ns2.example.com.",
} }
deps.resolver.allRecords["example.com"] = map[string]map[string][]string{
"ns1.example.com.": {"A": {"1.2.3.4"}},
"ns2.example.com.": {"A": {"1.2.3.4"}},
}
deps.portChecker.results["1.2.3.4:80"] = false
deps.portChecker.results["1.2.3.4:443"] = false
ctx := t.Context() ctx := t.Context()
w.RunOnce(ctx) w.RunOnce(ctx)
@@ -351,6 +429,10 @@ func TestNSChangeDetection(t *testing.T) {
"ns1.example.com.", "ns1.example.com.",
"ns3.example.com.", "ns3.example.com.",
} }
deps.resolver.allRecords["example.com"] = map[string]map[string][]string{
"ns1.example.com.": {"A": {"1.2.3.4"}},
"ns3.example.com.": {"A": {"1.2.3.4"}},
}
deps.resolver.mu.Unlock() deps.resolver.mu.Unlock()
w.RunOnce(ctx) w.RunOnce(ctx)
@@ -519,6 +601,11 @@ func TestGracefulShutdown(t *testing.T) {
deps.resolver.nsRecords["example.com"] = []string{ deps.resolver.nsRecords["example.com"] = []string{
"ns1.example.com.", "ns1.example.com.",
} }
deps.resolver.allRecords["example.com"] = map[string]map[string][]string{
"ns1.example.com.": {"A": {"1.2.3.4"}},
}
deps.portChecker.results["1.2.3.4:80"] = false
deps.portChecker.results["1.2.3.4:443"] = false
ctx, cancel := context.WithCancel(t.Context()) ctx, cancel := context.WithCancel(t.Context())