From 3ea665b11338c4eacd746c60d06a7adc8dce3eb0 Mon Sep 17 00:00:00 2001 From: Brendan Creane Date: Tue, 15 Sep 2026 10:24:47 -0600 Subject: [PATCH] tstest/natlab/vmtest: cover openresolv with a second snippet registered (#21003) TestOpenresolvDNS pins the case where Tailscale owns the only resolvconf snippet, so on its own it would also pass if tailscaled never read an OS base config at all. Register a snippet the way a DHCP client would, then toggle accept-dns to force a reapply, and assert its nameserver becomes quad-100's upstream. SetAcceptDNS is the toggle: tailscaled has no reason to re-read the OS config on its own, and this avoids rebooting the guest to change what the OS resolver looks like mid-test. Updates #20825 Updates tailscale/corp#44793 Signed-off-by: Brendan Creane --- tstest/natlab/vmtest/dns_openresolv_test.go | 65 +++++++++++++++++++++ tstest/natlab/vmtest/vmtest.go | 21 +++++++ 2 files changed, 86 insertions(+) diff --git a/tstest/natlab/vmtest/dns_openresolv_test.go b/tstest/natlab/vmtest/dns_openresolv_test.go index dafa76f1c..73640c36b 100644 --- a/tstest/natlab/vmtest/dns_openresolv_test.go +++ b/tstest/natlab/vmtest/dns_openresolv_test.go @@ -38,6 +38,12 @@ // The signature line openresolv writes at the top of resolv.conf. orSignature = "# Generated by resolvconf" + // A name that no route covers and that only vnet's default DNS server + // answers. Resolving it proves the query reached the base config's + // nameservers instead of being answered by quad-100. + orUpstreamOnlyName = "dualstack-web.example.com" + orUpstreamOnlyIP = "5.0.0.100" + // Tailscale's own resolver. tailscaled points resolv.conf at it once it has // configured DNS. orQuad100 = "100.100.100.100" @@ -114,6 +120,65 @@ func TestOpenresolvDNS(t *testing.T) { } } +// TestOpenresolvDNSOtherSnippet is the counterweight to TestOpenresolvDNS. +// When another snippet is present, its nameservers must still become quad-100's +// default upstream. Without this test, the fix for #20825 could return early on +// every host and still look correct. +func TestOpenresolvDNSOtherSnippet(t *testing.T) { + env, node := newOpenresolvEnv(t) + + // Register a second snippet, the way a DHCP client would. It points at + // vnet's default DNS server, the only thing that answers + // orUpstreamOnlyName. + cmd := fmt.Sprintf("printf 'nameserver %s\\n' | resolvconf -a eth0.inet", vnet.FakeDNSIPv4()) + if out, err := env.SSHExec(node, cmd); err != nil { + t.Fatalf("%s: %v (%s)", cmd, err, strings.TrimSpace(out)) + } + + // tailscaled has no reason to re-read the OS config on its own, so force a + // full reapply. This is also how the reporter of #20825 reproduced the bug. + env.SetAcceptDNS(node, false) + env.SetAcceptDNS(node, true) + + // Both snippets are registered. + if out, err := env.SSHExec(node, "resolvconf -i"); err != nil { + t.Errorf("resolvconf -i: %v (%s)", err, strings.TrimSpace(out)) + } else { + // Match whole fields, so "tailscale" does not match a longer snippet + // name. + got := strings.Fields(out) + for _, want := range []string{"eth0.inet", "tailscale"} { + if !slices.Contains(got, want) { + t.Errorf("resolvconf -i = %q, want it to list %q", strings.TrimSpace(out), want) + } + } + } + + // Tailscale still owns resolv.conf, so the OS asks quad-100 rather than the + // other snippet directly. Check that before the lookup at the end of the + // test. Otherwise a successful lookup might only mean libc went straight to + // the other snippet's nameserver during the toggle above. + assertOpenresolvResolvConf(t, env, node, + []string{orSignature, orQuad100}, + []string{vnet.FakeDNSIPv4().String()}) + + assertNoDNSReadWarning(t, env, node) + + // tailscaled must read back the other snippet's nameserver and nothing else. + // An empty config would mean the #20825 early return fired when it should + // not have, and our own snippet would mean the quad-100 filter did not + // (tailscale/tailscale#7816). + if base := openresolvBaseConfig(t, env, node); base != nil { + if want := vnet.FakeDNSIPv4().String(); !slices.Equal(base.Nameservers, []string{want}) { + t.Errorf("OS base config nameservers = %q, want just %s", base.Nameservers, want) + } + } + + // An answer here can only come from quad-100 forwarding to that base config, + // since nothing else on the guest answers this name. + assertResolves(t, env, node, orUpstreamOnlyName, orUpstreamOnlyIP) +} + // assertOpenresolvResolvConf waits for the guest's /etc/resolv.conf to contain // every string in want and none in notWant. func assertOpenresolvResolvConf(t *testing.T, env *vmtest.Env, n *vmtest.Node, want, notWant []string) { diff --git a/tstest/natlab/vmtest/vmtest.go b/tstest/natlab/vmtest/vmtest.go index c484eb63e..70a4b9412 100644 --- a/tstest/natlab/vmtest/vmtest.go +++ b/tstest/natlab/vmtest/vmtest.go @@ -1127,6 +1127,27 @@ func (e *Env) SetAcceptRoutes(n *Node, on bool) { e.t.Logf("[%s] accept-routes=%v", n.name, on) } +// SetAcceptDNS toggles the node's CorpDNS preference (the --accept-dns flag), +// controlling whether it applies the DNS configuration control sends it. +// +// Toggling it off and back on makes tailscaled tear down and reapply its whole +// DNS configuration, including re-reading the OS's own config, without +// rebooting the guest. A test can therefore change the OS resolver state +// mid-run and be sure tailscaled re-reads it. +func (e *Env) SetAcceptDNS(n *Node, on bool) { + e.t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + if _, err := n.agent.EditPrefs(ctx, &ipn.MaskedPrefs{ + Prefs: ipn.Prefs{CorpDNS: on}, + CorpDNSSet: true, + }); err != nil { + e.t.Fatalf("SetAcceptDNS(%s, %v): %v", n.name, on, err) + } + e.t.Logf("[%s] accept-dns=%v", n.name, on) +} + // ApproveRoutes tells the test control server to approve subnet routes // for the given node. The routes should be CIDR strings. func (e *Env) ApproveRoutes(n *Node, routes ...string) {