admin/tailscale
1
0 Fork 0
mirror of https://github.com/tailscale/tailscale.git synced 2026-10-23 03:37:19 +09:00
Code Issues Packages Projects Releases Wiki Activity

envknob: add SetenvForTest helper for safe environment variable handling

Browse Source 
Our tests are inconsistent in how they set environment variables and cleanup.
Missing cleanup logic can leak environment variable state across tests and
change the behaviour of subsequent tests.

This recently caused flakes in feature/acme, where `TestGetCertPEMWithValidity`
leaked `TS_CERT_SHARE_MODE` and `TestAsyncRenewalDedup` to fail inconsistently.

To fix the immediate flake and prevent future leaks, introduce a `SetenvForTest`
helper that handles setting and cleaning up environment variables in tests.

Fixes #20902

Change-Id: I9f8ef45ec875d66034cfd0054311bd69b67e2243
Signed-off-by: Alex Chan <alexc@tailscale.com>
This commit is contained in:
Alex Chan authored and Alex Chan committed 2026-08-19 12:22:26 +01:00
1 parent 86c178754d
commit 468241a13f
10 files changed
+40 -37
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

envknob/envknob.go
+21
View File
@@ -35,6 +35,7 @@ import (
"tailscale.com/kube/kubetypes"
"tailscale.com/syncs"
"tailscale.com/types/opt"
"tailscale.com/util/testenv"
"tailscale.com/version"
"tailscale.com/version/distro"
)
@@ -110,6 +111,26 @@ func Setenv(envVar, val string) {
}
}
// SetenvForTest safely changes an environment variable in a test.
//
// It is designed to avoid leaking state between tests. It registers
// a cleanup function to reset the variable once the test completes,
// and panics if you try to set environment variables in parallel tests.
//
// Use this instead of t.Setenv() if your variable will be read by envknob,
// and you need to update envknob's internal caches with the new value.
func SetenvForTest(t testenv.TB, envVar, val string) {
t.Helper()
// This is copied from [tstest.AssertNotParallel], which we can't
// call here because it would create an import cycle.
t.Setenv("ASSERT_NOT_PARALLEL_TEST", "1") // panics if t.Parallel was called
prev := String(envVar)
Setenv(envVar, val)
t.Cleanup(func() { Setenv(envVar, prev) })
}
// String returns the named environment variable, using os.Getenv.
//
// If the variable is non-empty, it's also tracked & logged as being
feature/acme/cert_test.go
+2 -5
View File
@@ -27,7 +27,6 @@ import (
"time"
"github.com/google/go-cmp/cmp"
"tailscale.com/envknob"
"tailscale.com/health"
"tailscale.com/ipn"
"tailscale.com/ipn/ipnlocal"
@@ -744,12 +743,10 @@ func TestGetCertPEMWithValidity(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
tstest.AssertNotParallel(t)
if tt.readOnlyMode {
envknob.Setenv("TS_CERT_SHARE_MODE", "ro")
t.Setenv("TS_CERT_SHARE_MODE", "ro")
} else {
envknob.Setenv("TS_CERT_SHARE_MODE", "")
t.Setenv("TS_CERT_SHARE_MODE", "")
}
os.RemoveAll(certDirPath)
ipn/ipnlocal/state_test.go
+1 -2
View File
@@ -407,8 +407,7 @@ func TestStartShutsDownPreviousControlClient(t *testing.T) {
// predictable, but maybe a bit less thorough. This is more of an overall
// state machine test than a test of the wgengine+magicsock integration.
func TestStateMachine(t *testing.T) {
envknob.Setenv("TAILSCALE_USE_WIP_CODE", "1")
defer envknob.Setenv("TAILSCALE_USE_WIP_CODE", "")
envknob.SetenvForTest(t, "TAILSCALE_USE_WIP_CODE", "1")
c := qt.New(t)
logf := tstest.WhileTestRunningLogger(t)
ipn/ipnserver/server_test.go
+6 -12
View File
@@ -23,7 +23,7 @@ import (
func TestUserConnectDisconnectNonWindows(t *testing.T) {
enableLogging := false
if runtime.GOOS == "windows" {
setGOOSForTest(t, "linux")
envknob.SetenvForTest(t, "TS_DEBUG_FAKE_GOOS", "linux")
}
ctx := context.Background()
@@ -66,7 +66,7 @@ func TestUserConnectDisconnectNonWindows(t *testing.T) {
func TestUserConnectDisconnectOnWindows(t *testing.T) {
enableLogging := false
setGOOSForTest(t, "windows")
envknob.SetenvForTest(t, "TS_DEBUG_FAKE_GOOS", "windows")
ctx := context.Background()
server := lapitest.NewServer(t, lapitest.WithLogging(enableLogging))
@@ -88,7 +88,7 @@ func TestUserConnectDisconnectOnWindows(t *testing.T) {
func TestIPNAlreadyInUseOnWindows(t *testing.T) {
enableLogging := false
setGOOSForTest(t, "windows")
envknob.SetenvForTest(t, "TS_DEBUG_FAKE_GOOS", "windows")
ctx := context.Background()
server := lapitest.NewServer(t, lapitest.WithLogging(enableLogging))
@@ -111,7 +111,7 @@ func TestIPNAlreadyInUseOnWindows(t *testing.T) {
func TestSequentialOSUserSwitchingOnWindows(t *testing.T) {
enableLogging := false
setGOOSForTest(t, "windows")
envknob.SetenvForTest(t, "TS_DEBUG_FAKE_GOOS", "windows")
ctx := context.Background()
server := lapitest.NewServer(t, lapitest.WithLogging(enableLogging))
@@ -140,7 +140,7 @@ func TestSequentialOSUserSwitchingOnWindows(t *testing.T) {
func TestConcurrentOSUserSwitchingOnWindows(t *testing.T) {
enableLogging := false
setGOOSForTest(t, "windows")
envknob.SetenvForTest(t, "TS_DEBUG_FAKE_GOOS", "windows")
ctx := context.Background()
server := lapitest.NewServer(t, lapitest.WithLogging(enableLogging))
@@ -212,7 +212,7 @@ func TestConcurrentOSUserSwitchingOnWindows(t *testing.T) {
func TestBlockWhileIdentityInUse(t *testing.T) {
enableLogging := false
setGOOSForTest(t, "windows")
envknob.SetenvForTest(t, "TS_DEBUG_FAKE_GOOS", "windows")
ctx := context.Background()
server := lapitest.NewServer(t, lapitest.WithLogging(enableLogging))
@@ -312,12 +312,6 @@ func checkError(tb testing.TB, got, want error) {
}
}
func setGOOSForTest(tb testing.TB, goos string) {
tb.Helper()
envknob.Setenv("TS_DEBUG_FAKE_GOOS", goos)
tb.Cleanup(func() { envknob.Setenv("TS_DEBUG_FAKE_GOOS", "") })
}
func pumpIPNBus(watcher *local.IPNBusWatcher) {
for {
_, err := watcher.Next()
ipn/store/kubestore/store_kube_test.go
+2 -3
View File
@@ -375,8 +375,7 @@ func TestWriteTLSCertAndKey(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
// Set POD_NAME for testing selectors
envknob.Setenv("POD_NAME", "ingress-proxies-1")
defer envknob.Setenv("POD_NAME", "")
envknob.SetenvForTest(t, "POD_NAME", "ingress-proxies-1")
secret := tt.initial // track current state
client := &kubeclient.FakeClient{
@@ -772,7 +771,7 @@ func TestNewWithClient(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
envknob.Setenv("TS_CERT_SHARE_MODE", tt.certMode)
envknob.SetenvForTest(t, "TS_CERT_SHARE_MODE", tt.certMode)
t.Setenv("POD_NAME", "ingress-proxies-1")
logtail/logtail_test.go
+1 -1
View File
@@ -70,7 +70,7 @@ type LogtailTestServer struct {
// the default FlushDelay and any other fake timers advance automatically.
func newTestLogtailServer(t *testing.T) (*LogtailTestServer, *Logger) {
// Enable the logtail started message
envknob.Setenv("TS_DEBUG_LOGTAIL", "1")
envknob.SetenvForTest(t, "TS_DEBUG_LOGTAIL", "1")
conf, uploaded := newCaptureServer(t, 0)
conf.Bus = eventbustest.NewBus(t)
wgengine/magicsock/endpoint_test.go
+2 -2
View File
@@ -570,9 +570,9 @@ func Test_endpoint_sendDiscoPingsLocked_neverDirectUDP(t *testing.T) {
t.Run(fmt.Sprintf("neverDirectUDP=%v", neverDirectUDP), func(t *testing.T) {
prev := envknob.String("TS_DEBUG_NEVER_DIRECT_UDP")
if neverDirectUDP {
envknob.Setenv("TS_DEBUG_NEVER_DIRECT_UDP", "true")
envknob.SetenvForTest(t, "TS_DEBUG_NEVER_DIRECT_UDP", "true")
} else {
envknob.Setenv("TS_DEBUG_NEVER_DIRECT_UDP", "")
envknob.SetenvForTest(t, "TS_DEBUG_NEVER_DIRECT_UDP", "")
}
t.Cleanup(func() { envknob.Setenv("TS_DEBUG_NEVER_DIRECT_UDP", prev) })
now := mono.Now()
wgengine/magicsock/magicsock_test.go
+1 -1
View File
@@ -2371,7 +2371,7 @@ func TestSetNetworkMapWithNoPeers(t *testing.T) {
for i := 1; i <= 3; i++ {
v := !debugEnableSilentDisco()
envknob.Setenv("TS_DEBUG_ENABLE_SILENT_DISCO", fmt.Sprint(v))
envknob.SetenvForTest(t, "TS_DEBUG_ENABLE_SILENT_DISCO", fmt.Sprint(v))
c.SetNetworkMap(tailcfg.NodeView{}, nil)
if c.lastFlags.heartbeatDisabled != v {
t.Fatalf("call %d: didn't store netmap", i)
wgengine/netstack/netstack_test.go
+2 -6
View File
@@ -739,9 +739,7 @@ func makeHangDialer(tb testing.TB) (netx.DialFunc, chan struct{}) {
// TestTCPForwardLimits verifies that the limits on the TCP forwarder work in a
// success case (i.e. when we don't hit the limit).
func TestTCPForwardLimits(t *testing.T) {
tstest.AssertNotParallel(t) // calls envknob.Setenv
envknob.Setenv("TS_DEBUG_NETSTACK", "true")
t.Cleanup(func() { envknob.Setenv("TS_DEBUG_NETSTACK", "") })
envknob.SetenvForTest(t, "TS_DEBUG_NETSTACK", "true")
impl := makeNetstack(t, func(impl *Impl) {
impl.ProcessSubnets = true
@@ -818,9 +816,7 @@ func TestTCPForwardLimits(t *testing.T) {
// forwarding works.
func TestTCPForwardLimits_PerClient(t *testing.T) {
clientmetric.ResetForTest(t)
tstest.AssertNotParallel(t) // calls envknob.Setenv
envknob.Setenv("TS_DEBUG_NETSTACK", "true")
t.Cleanup(func() { envknob.Setenv("TS_DEBUG_NETSTACK", "") })
envknob.SetenvForTest(t, "TS_DEBUG_NETSTACK", "true")
// Set our test override limits during this test.
maxInFlightConnectionAttemptsForTest.Store(2)
wgengine/userspace_test.go
+2 -5
View File
@@ -12,7 +12,6 @@ import (
"net/http"
"net/http/httptest"
"net/netip"
"os"
"runtime"
"slices"
"sync"
@@ -266,11 +265,9 @@ func TestUserspaceEnginePeerMTUReconfig(t *testing.T) {
t.Skipf("skipping on %q; peer MTU not supported", runtime.GOOS)
}
defer os.Setenv("TS_DEBUG_ENABLE_PMTUD", os.Getenv("TS_DEBUG_ENABLE_PMTUD"))
envknob.Setenv("TS_DEBUG_ENABLE_PMTUD", "")
envknob.SetenvForTest(t, "TS_DEBUG_ENABLE_PMTUD", "")
// Turn on debugging to help diagnose problems.
defer os.Setenv("TS_DEBUG_PMTUD", os.Getenv("TS_DEBUG_PMTUD"))
envknob.Setenv("TS_DEBUG_PMTUD", "true")
envknob.SetenvForTest(t, "TS_DEBUG_PMTUD", "true")
var knobs controlknobs.Knobs