From a97850f7e22b96466ec2f432f9cc6c3b1e0f64f3 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 14 Apr 2026 00:01:36 +0000 Subject: [PATCH] cmd/derper: fix TestLookupMetric to pass when run alone TestLookupMetric was added in e8d140654 (2023-08-17) without initializing the dnsCache and dnsCacheBytes globals. When run in isolation, handleBootstrapDNS writes a nil body (from the uninitialized dnsCacheBytes), causing getBootstrapDNS to fail decoding an empty response with EOF. Add a setDNSCache test helper that stores the dnsEntryMap, marshals dnsCacheBytes, and registers a t.Cleanup to nil both out, so tests that forget to call it will hit the dnsCache-nil fatal in getBootstrapDNS rather than silently depending on prior test state. Also add AssertNotParallel and a dnsCache-nil fatal check to getBootstrapDNS, the central helper all bootstrap DNS tests flow through, to prevent future tests from running in parallel (they all mutate package-level DNS caches and metrics) and to give a clear error if a test forgets to initialize the DNS caches. Fixes #19388 Change-Id: I8ad454ec6026c71f13ecfa14d25925df5478b908 Signed-off-by: Brad Fitzpatrick Co-authored-by: Avery Pennarun --- cmd/derper/bootstrap_dns_test.go | 34 ++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/cmd/derper/bootstrap_dns_test.go b/cmd/derper/bootstrap_dns_test.go index 5b765f6d3..2055b9751 100644 --- a/cmd/derper/bootstrap_dns_test.go +++ b/cmd/derper/bootstrap_dns_test.go @@ -41,8 +41,28 @@ func (b *bitbucketResponseWriter) Write(p []byte) (int, error) { return len(p), func (b *bitbucketResponseWriter) WriteHeader(statusCode int) {} +// setDNSCache sets the published DNS cache for tests. +func setDNSCache(tb testing.TB, m *dnsEntryMap) { + tb.Helper() + j, err := json.Marshal(m.IPs) + if err != nil { + tb.Fatal(err) + } + tstest.AssertNotParallel(tb) + dnsCache.Store(m) + dnsCacheBytes.Store(j) + tb.Cleanup(func() { + dnsCache.Store(nil) + dnsCacheBytes.Store(nil) + }) +} + func getBootstrapDNS(t *testing.T, q string) map[string][]net.IP { t.Helper() + tstest.AssertNotParallel(t) + if dnsCache.Load() == nil { + t.Fatal("dnsCache not initialized; call setDNSCache before getBootstrapDNS") + } req, _ := http.NewRequest("GET", "https://localhost/bootstrap-dns?q="+url.QueryEscape(q), nil) w := httptest.NewRecorder() handleBootstrapDNS(w, req) @@ -100,7 +120,8 @@ func TestUnpublishedDNS(t *testing.T) { } } -func resetMetrics() { +func resetMetrics(tb testing.TB) { + tstest.AssertNotParallel(tb) publishedDNSHits.Set(0) publishedDNSMisses.Set(0) unpublishedDNSHits.Set(0) @@ -114,8 +135,7 @@ func TestUnpublishedDNSEmptyList(t *testing.T) { pub := &dnsEntryMap{ IPs: map[string][]net.IP{"tailscale.com": {net.IPv4(10, 10, 10, 10)}}, } - dnsCache.Store(pub) - dnsCacheBytes.Store([]byte(`{"tailscale.com":["10.10.10.10"]}`)) + setDNSCache(t, pub) unpublishedDNSCache.Store(&dnsEntryMap{ IPs: map[string][]net.IP{ @@ -131,7 +151,7 @@ func TestUnpublishedDNSEmptyList(t *testing.T) { t.Run("CacheMiss", func(t *testing.T) { // One domain in map but empty, one not in map at all for _, q := range []string{"log.tailscale.com", "login.tailscale.com"} { - resetMetrics() + resetMetrics(t) ips := getBootstrapDNS(t, q) // Expected our public map to be returned on a cache miss @@ -149,7 +169,7 @@ func TestUnpublishedDNSEmptyList(t *testing.T) { // Verify that we do get a valid response and metric. t.Run("CacheHit", func(t *testing.T) { - resetMetrics() + resetMetrics(t) ips := getBootstrapDNS(t, "controlplane.tailscale.com") want := map[string][]net.IP{"controlplane.tailscale.com": {net.IPv4(1, 2, 3, 4)}} if !reflect.DeepEqual(ips, want) { @@ -166,8 +186,10 @@ func TestUnpublishedDNSEmptyList(t *testing.T) { } func TestLookupMetric(t *testing.T) { + setDNSCache(t, &dnsEntryMap{}) + d := []string{"a.io", "b.io", "c.io", "d.io", "e.io", "e.io", "e.io", "a.io"} - resetMetrics() + resetMetrics(t) for _, q := range d { _ = getBootstrapDNS(t, q) }