All checks were successful
check / check (push) Successful in 1m23s
Rework of the unit at #93 (commit9cb2c2b), against the review at #136 (comment). Review of9cb2c2bfound the quorum assertions could not fail on a class of wrong answer. Each test banned exactly one bad status — _AllReturnOK banned only nxdomain, _NXDomainFromAllNS banned only ok — so resolver.StatusNoData passed both. nodata is a wrong answer, not silence, and answeredCount counted it as answered, so it did not even trigger a retry; with a quorum of 3 of 4 a single wrong nameserver slid through undetected. That is assertion-loosening beyond what the quorum change requires. Tolerance is now a closed allowlist rather than a blocklist of one status. unsanctionedStatuses() reports every per-nameserver result whose status the caller did not explicitly sanction: ok/timeout/error for the all-OK test, nxdomain/timeout/error for the NXDOMAIN test. Silence (timeout, error) is the only thing quorum exists to tolerate; any other status, including one added to the resolver later, fails by name. answeredCount is likewise an allowlist of ok/nxdomain/nodata, so an unknown status counts as silence and can only cause a retry and then a loud failure, never a quiet pass. Two harness tests cover the regression directly: three OK plus one nodata (quorum satisfied, no nxdomain present — the input that used to pass) is now reported as unsanctioned, and an unknown status is neither counted as answered nor tolerated. Verified by re-running the reviewer's probe: queryEachNS patched to force one of google.com's four nameservers to return StatusNoData turns both tests red, naming the offending nameserver and status — --- FAIL: TestQueryAllNameservers_AllReturnOK (1.12s) Should be empty, but was [ns1.google.com.=nodata] every nameserver must answer OK or not answer at all: ns1.google.com.=nodata ns2.google.com.=ok ns3.google.com.=ok ns4.google.com.=ok --- FAIL: TestQueryAllNameservers_NXDomainFromAllNS (1.34s) Should be empty, but was [ns1.google.com.=nodata] every nameserver must report NXDOMAIN or not answer at all: ns1.google.com.=nodata ns2.google.com.=nxdomain ns3.google.com.=nxdomain ns4.google.com.=nxdomain — and green with the probe reverted. Also fixes the review's nit: the per-attempt deadline assertion had no lower bound, so it passed for a deadline far shorter than intended. No production code changed; DNS is still never mocked.
285 lines
6.2 KiB
Go
285 lines
6.2 KiB
Go
package resolver_test
|
|
|
|
import (
|
|
"context"
|
|
"sync"
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/stretchr/testify/assert"
|
|
|
|
"sneak.berlin/go/dnswatcher/internal/resolver"
|
|
)
|
|
|
|
// Tests for the live-DNS harness in livedns_test.go itself. These
|
|
// exercise pure logic and the retry/concurrency plumbing; they
|
|
// perform no DNS resolution of any kind, so they neither mock DNS
|
|
// nor depend on it.
|
|
|
|
// Names for the synthetic status maps below. Nothing is ever queried
|
|
// at them: they are map keys handed to the package's pure counting
|
|
// helpers, not a stand-in for a nameserver.
|
|
const (
|
|
nsExample1 = "ns1.example."
|
|
nsExample2 = "ns2.example."
|
|
nsExample3 = "ns3.example."
|
|
nsExample4 = "ns4.example."
|
|
)
|
|
|
|
func TestLiveQuorumIsStrictMajority(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
cases := map[int]int{
|
|
0: 1,
|
|
1: 1,
|
|
2: 2,
|
|
3: 2,
|
|
4: 3,
|
|
5: 3,
|
|
13: 7,
|
|
}
|
|
|
|
for total, want := range cases {
|
|
assert.Equal(
|
|
t, want, liveQuorum(total),
|
|
"liveQuorum(%d)", total,
|
|
)
|
|
}
|
|
}
|
|
|
|
func TestStatusCountingIgnoresSilentNameservers(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
results := map[string]*resolver.NameserverResponse{
|
|
nsExample1: {
|
|
Nameserver: nsExample1,
|
|
Status: resolver.StatusOK,
|
|
},
|
|
nsExample2: {
|
|
Nameserver: nsExample2,
|
|
Status: resolver.StatusOK,
|
|
},
|
|
nsExample3: {
|
|
Nameserver: nsExample3,
|
|
Status: resolver.StatusTimeout,
|
|
},
|
|
nsExample4: {
|
|
Nameserver: nsExample4,
|
|
Status: resolver.StatusError,
|
|
},
|
|
}
|
|
|
|
assert.Equal(
|
|
t, 2, countStatus(results, resolver.StatusOK),
|
|
)
|
|
assert.Equal(
|
|
t, 0, countStatus(results, resolver.StatusNXDomain),
|
|
)
|
|
|
|
// Two of four answered, which is short of the quorum of
|
|
// three: this is the state that triggers a retry rather
|
|
// than an assertion failure.
|
|
assert.Equal(t, 2, answeredCount(results))
|
|
assert.Less(t, answeredCount(results), liveQuorum(len(results)))
|
|
|
|
assert.Equal(
|
|
t,
|
|
"ns1.example.=ok ns2.example.=ok "+
|
|
"ns3.example.=timeout ns4.example.=error",
|
|
describeStatuses(results),
|
|
)
|
|
}
|
|
|
|
func TestRetryLiveRecoversFromTransientFailure(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
const wantAttempts = 2
|
|
|
|
attempts := 0
|
|
|
|
retryLive(t, "transient", func(_ context.Context) error {
|
|
attempts++
|
|
|
|
if attempts < wantAttempts {
|
|
return errLiveNoAnswer
|
|
}
|
|
|
|
return nil
|
|
})
|
|
|
|
assert.Equal(t, wantAttempts, attempts)
|
|
}
|
|
|
|
func TestRetryLiveGivesEachAttemptADeadline(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
retryLive(t, "deadline", func(ctx context.Context) error {
|
|
deadline, ok := ctx.Deadline()
|
|
assert.True(t, ok, "attempt should carry a deadline")
|
|
|
|
remaining := time.Until(deadline)
|
|
|
|
assert.LessOrEqual(t, remaining, liveAttemptTimeout)
|
|
|
|
// Lower bound too: without one this passes for a
|
|
// deadline far shorter than intended, which would
|
|
// silently turn every live attempt into an instant
|
|
// timeout.
|
|
assert.Greater(t, remaining, liveAttemptTimeout/2)
|
|
|
|
return nil
|
|
})
|
|
}
|
|
|
|
// TestUnsanctionedStatusesRejectsWrongAnswers is the regression test
|
|
// for the defect this allowlist exists to prevent: a minority of
|
|
// nameservers answering WRONGLY while quorum keeps the suite green.
|
|
// nodata is the case that motivated it — it is a wrong answer, not
|
|
// silence, and it was previously banned by neither test.
|
|
func TestUnsanctionedStatusesRejectsWrongAnswers(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
// Four nameservers, three OK and one answering nodata: a
|
|
// quorum of three is satisfied and no NXDOMAIN is present, so
|
|
// the old blocklist assertions both passed on this input.
|
|
results := map[string]*resolver.NameserverResponse{
|
|
nsExample1: {
|
|
Nameserver: nsExample1,
|
|
Status: resolver.StatusOK,
|
|
},
|
|
nsExample2: {
|
|
Nameserver: nsExample2,
|
|
Status: resolver.StatusOK,
|
|
},
|
|
nsExample3: {
|
|
Nameserver: nsExample3,
|
|
Status: resolver.StatusOK,
|
|
},
|
|
nsExample4: {
|
|
Nameserver: nsExample4,
|
|
Status: resolver.StatusNoData,
|
|
},
|
|
}
|
|
|
|
assert.GreaterOrEqual(
|
|
t,
|
|
countStatus(results, resolver.StatusOK),
|
|
liveQuorum(len(results)),
|
|
)
|
|
assert.Zero(t, countStatus(results, resolver.StatusNXDomain))
|
|
|
|
// nodata is an ANSWER, so it never triggers a retry: nothing
|
|
// but the allowlist stands between it and a false green.
|
|
assert.Equal(t, len(results), answeredCount(results))
|
|
|
|
assert.Equal(
|
|
t,
|
|
[]string{nsExample4 + "=nodata"},
|
|
unsanctionedStatuses(
|
|
results,
|
|
resolver.StatusOK,
|
|
resolver.StatusTimeout,
|
|
resolver.StatusError,
|
|
),
|
|
"nodata must be reported as an unsanctioned status",
|
|
)
|
|
}
|
|
|
|
func TestUnsanctionedStatusesToleratesSilenceOnly(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
results := map[string]*resolver.NameserverResponse{
|
|
nsExample1: {
|
|
Nameserver: nsExample1,
|
|
Status: resolver.StatusNXDomain,
|
|
},
|
|
nsExample2: {
|
|
Nameserver: nsExample2,
|
|
Status: resolver.StatusTimeout,
|
|
},
|
|
nsExample3: {
|
|
Nameserver: nsExample3,
|
|
Status: resolver.StatusError,
|
|
},
|
|
}
|
|
|
|
allowed := []string{
|
|
resolver.StatusNXDomain,
|
|
resolver.StatusTimeout,
|
|
resolver.StatusError,
|
|
}
|
|
|
|
assert.Empty(
|
|
t,
|
|
unsanctionedStatuses(results, allowed...),
|
|
"timeout and error are non-answers and are tolerated",
|
|
)
|
|
|
|
// The same silent nameservers do not count towards a quorum.
|
|
assert.Equal(t, 1, answeredCount(results))
|
|
|
|
// An unknown status is treated as silence by answeredCount —
|
|
// so it retries and fails loudly — and is unsanctioned by the
|
|
// allowlist rather than quietly permitted.
|
|
const laterStatus = "some-status-added-later"
|
|
|
|
results[nsExample4] = &resolver.NameserverResponse{
|
|
Nameserver: nsExample4,
|
|
Status: laterStatus,
|
|
}
|
|
|
|
assert.Equal(t, 1, answeredCount(results))
|
|
assert.Equal(
|
|
t,
|
|
[]string{nsExample4 + "=" + laterStatus},
|
|
unsanctionedStatuses(results, allowed...),
|
|
)
|
|
}
|
|
|
|
func TestRunLiveBoundsConcurrency(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
const workers = 24
|
|
|
|
var (
|
|
mu sync.Mutex
|
|
wg sync.WaitGroup
|
|
inFlight int
|
|
maxSeen int
|
|
)
|
|
|
|
wg.Add(workers)
|
|
|
|
for range workers {
|
|
go func() {
|
|
defer wg.Done()
|
|
|
|
_ = runLive(func(_ context.Context) error {
|
|
mu.Lock()
|
|
inFlight++
|
|
|
|
if inFlight > maxSeen {
|
|
maxSeen = inFlight
|
|
}
|
|
mu.Unlock()
|
|
|
|
time.Sleep(time.Millisecond)
|
|
|
|
mu.Lock()
|
|
inFlight--
|
|
mu.Unlock()
|
|
|
|
return nil
|
|
})
|
|
}()
|
|
}
|
|
|
|
wg.Wait()
|
|
|
|
assert.Positive(t, maxSeen)
|
|
assert.LessOrEqual(
|
|
t, maxSeen, liveConcurrency,
|
|
"live queries must stay under the package-wide gate",
|
|
)
|
|
}
|