envknob: add new package for all the strconv.ParseBool(os.Getenv(..))

A new package can also later record/report which knobs are checked and
set. It also makes the code cleaner & easier to grep for env knobs.

Change-Id: Id8a123ab7539f1fadbd27e0cbeac79c2e4f09751
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

wgengine/filter/filter.go

@@ -7,11 +7,11 @@ package filter
 
 import (
 	"fmt"
-	"os"
 	"sync"
 	"time"
 
 	"inet.af/netaddr"
+	"tailscale.com/envknob"
 	"tailscale.com/net/flowtrack"
 	"tailscale.com/net/packet"
 	"tailscale.com/tstime/rate"
@@ -225,7 +225,7 @@ var dropBucket = rate.NewLimiter(rate.Every(5*time.Second), 10)
 //   effectively disable the limits on the log rate by setting the limit
 //   to 1 millisecond. This should capture everything.
 func init() {
-	if os.Getenv("TS_DEBUG_FILTER_RATE_LIMIT_LOGS") != "all" {
+	if envknob.String("TS_DEBUG_FILTER_RATE_LIMIT_LOGS") != "all" {
 		return
 	}
 

wgengine/magicsock/debugknobs.go

@@ -8,8 +8,7 @@
 package magicsock
 
 import (
-	"os"
-	"strconv"
+	"tailscale.com/envknob"
 )
 
 // Various debugging and experimental tweakables, set by environment
@@ -17,25 +16,23 @@ import (
 var (
 	// debugDisco prints verbose logs of active discovery events as
 	// they happen.
-	debugDisco, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_DISCO"))
+	debugDisco = envknob.Bool("TS_DEBUG_DISCO")
 	// debugOmitLocalAddresses removes all local interface addresses
 	// from magicsock's discovered local endpoints. Used in some tests.
-	debugOmitLocalAddresses, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_OMIT_LOCAL_ADDRS"))
+	debugOmitLocalAddresses = envknob.Bool("TS_DEBUG_OMIT_LOCAL_ADDRS")
 	// debugUseDerpRoute temporarily (2020-03-22) controls whether DERP
-	// reverse routing is enabled (Issue 150). It will become always true
-	// later.
-	debugUseDerpRouteEnv = os.Getenv("TS_DEBUG_ENABLE_DERP_ROUTE")
-	debugUseDerpRoute, _ = strconv.ParseBool(debugUseDerpRouteEnv)
+	// reverse routing is enabled (Issue 150).
+	debugUseDerpRoute = envknob.OptBool("TS_DEBUG_ENABLE_DERP_ROUTE")
 	// logDerpVerbose logs all received DERP packets, including their
 	// full payload.
-	logDerpVerbose, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_DERP"))
+	logDerpVerbose = envknob.Bool("TS_DEBUG_DERP")
 	// debugReSTUNStopOnIdle unconditionally enables the "shut down
 	// STUN if magicsock is idle" behavior that normally only triggers
 	// on mobile devices, lowers the shutdown interval, and logs more
 	// verbosely about idle measurements.
-	debugReSTUNStopOnIdle, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_RESTUN_STOP_ON_IDLE"))
+	debugReSTUNStopOnIdle = envknob.Bool("TS_DEBUG_RESTUN_STOP_ON_IDLE")
 	// debugAlwaysDERP disables the use of UDP, forcing all peer communication over DERP.
-	debugAlwaysDERP, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_ALWAYS_USE_DERP"))
+	debugAlwaysDERP = envknob.Bool("TS_DEBUG_ALWAYS_USE_DERP")
 )
 
 // inTest reports whether the running program is a test that set the
@@ -44,7 +41,4 @@ var (
 // Unlike the other debug tweakables above, this one needs to be
 // checked every time at runtime, because tests set this after program
 // startup.
-func inTest() bool {
-	inTest, _ := strconv.ParseBool(os.Getenv("IN_TS_TEST"))
-	return inTest
-}
+func inTest() bool { return envknob.Bool("IN_TS_TEST") }

wgengine/magicsock/magicsock.go

@@ -61,8 +61,8 @@ import (
 // useDerpRoute reports whether magicsock should enable the DERP
 // return path optimization (Issue 150).
 func useDerpRoute() bool {
-	if debugUseDerpRouteEnv != "" {
-		return debugUseDerpRoute
+	if b, ok := debugUseDerpRoute.Get(); ok {
+		return b
 	}
 	ob := controlclient.DERPRouteFlag()
 	if v, ok := ob.Get(); ok {

wgengine/netstack/netstack.go

@@ -34,6 +34,7 @@ import (
 	"inet.af/netstack/tcpip/transport/tcp"
 	"inet.af/netstack/tcpip/transport/udp"
 	"inet.af/netstack/waiter"
+	"tailscale.com/envknob"
 	"tailscale.com/net/packet"
 	"tailscale.com/net/tsaddr"
 	"tailscale.com/net/tsdial"
@@ -49,7 +50,7 @@ import (
 
 const debugPackets = false
 
-var debugNetstack, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_NETSTACK"))
+var debugNetstack = envknob.Bool("TS_DEBUG_NETSTACK")
 
 // Impl contains the state for the netstack implementation,
 // and implements wgengine.FakeImpl to act as a userspace network

wgengine/pendopen.go

@@ -6,9 +6,7 @@ package wgengine
 
 import (
 	"fmt"
-	"os"
 	"runtime"
-	"strconv"
 	"time"
 
 	"tailscale.com/ipn/ipnstate"
@@ -22,17 +20,6 @@ import (
 
 const tcpTimeoutBeforeDebug = 5 * time.Second
 
-// debugConnectFailures reports whether the local node should track
-// outgoing TCP connections and log which ones fail and why.
-func debugConnectFailures() bool {
-	s := os.Getenv("TS_DEBUG_CONNECT_FAILURES")
-	if s == "" {
-		return true
-	}
-	v, _ := strconv.ParseBool(s)
-	return v
-}
-
 type pendingOpenFlow struct {
 	timer *time.Timer // until giving up on the flow
 

wgengine/router/router_linux.go

@@ -22,6 +22,7 @@ import (
 	"golang.org/x/time/rate"
 	"golang.zx2c4.com/wireguard/tun"
 	"inet.af/netaddr"
+	"tailscale.com/envknob"
 	"tailscale.com/net/tsaddr"
 	"tailscale.com/syncs"
 	"tailscale.com/types/logger"
@@ -188,7 +189,7 @@ func useAmbientCaps() bool {
 	return v >= 7
 }
 
-var forceIPCommand, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_USE_IP_COMMAND"))
+var forceIPCommand = envknob.Bool("TS_DEBUG_USE_IP_COMMAND")
 
 // useIPCommand reports whether r should use the "ip" command (or its
 // fake commandRunner for tests) instead of netlink.

wgengine/userspace.go

@@ -11,10 +11,8 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"os"
 	"reflect"
 	"runtime"
-	"strconv"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -25,6 +23,7 @@ import (
 	"golang.zx2c4.com/wireguard/tun"
 	"inet.af/netaddr"
 	"tailscale.com/control/controlclient"
+	"tailscale.com/envknob"
 	"tailscale.com/health"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/net/dns"
@@ -366,7 +365,7 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error)
 	}
 	e.tundev.PreFilterOut = e.handleLocalPackets
 
-	if debugConnectFailures() {
+	if envknob.BoolDefaultTrue("TS_DEBUG_CONNECT_FAILURES") {
 		if e.tundev.PreFilterIn != nil {
 			return nil, errors.New("unexpected PreFilterIn already set")
 		}
@@ -550,10 +549,7 @@ func (e *userspaceEngine) pollResolver() {
 	}
 }
 
-var (
-	debugTrimWireguardEnv = os.Getenv("TS_DEBUG_TRIM_WIREGUARD")
-	debugTrimWireguard, _ = strconv.ParseBool(debugTrimWireguardEnv)
-)
+var debugTrimWireguard = envknob.OptBool("TS_DEBUG_TRIM_WIREGUARD")
 
 // forceFullWireguardConfig reports whether we should give wireguard
 // our full network map, even for inactive peers
@@ -563,8 +559,8 @@ var (
 // and we haven't got enough time testing it.
 func forceFullWireguardConfig(numPeers int) bool {
 	// Did the user explicitly enable trimmming via the environment variable knob?
-	if debugTrimWireguardEnv != "" {
-		return !debugTrimWireguard
+	if b, ok := debugTrimWireguard.Get(); ok {
+		return !b
 	}
 	if opt := controlclient.TrimWGConfig(); opt != "" {
 		return !opt.EqualBool(true)

wgengine/watchdog.go

@@ -6,13 +6,12 @@ package wgengine
 
 import (
 	"log"
-	"os"
 	"runtime/pprof"
-	"strconv"
 	"strings"
 	"time"
 
 	"inet.af/netaddr"
+	"tailscale.com/envknob"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/net/dns"
 	"tailscale.com/net/dns/resolver"
@@ -32,7 +31,7 @@ import (
 //
 // If they do not, the watchdog crashes the process.
 func NewWatchdog(e Engine) Engine {
-	if v, _ := strconv.ParseBool(os.Getenv("TS_DEBUG_DISABLE_WATCHDOG")); v {
+	if envknob.Bool("TS_DEBUG_DISABLE_WATCHDOG") {
 		return e
 	}
 	return &watchdogEngine{