util/def,cmd/containerboot: add LookupEnv, simplify env parsing (#20277)

Simplifies cmd/containerboot env var parsing. Most of the private helpers did
not earn their abstraction: defaultEnv(name, "") is just os.Getenv(name), and
the rest collapse into cmp.Or and the existing def.Bool. defaultEnv,
defaultEnvs and defaultBool are gone.

Adds def.LookupEnv, the env companion to def.Bool, for the one case that needs
it: TS_KUBE_SECRET, where an explicit "" disables Kubernetes secret storage and
must stay distinct from unset (cmp.Or cannot express that).

Updates #20018

Signed-off-by: Nick Rossi <nrossi0530@gmail.com>

cmd/containerboot/main.go

@@ -154,19 +154,20 @@ import (
 	"tailscale.com/types/logger"
 	"tailscale.com/types/views"
 	"tailscale.com/util/deephash"
+	"tailscale.com/util/def"
 	"tailscale.com/util/dnsname"
 	"tailscale.com/util/linuxfw"
 )
 
 func newNetfilterRunner(logf logger.Logf) (linuxfw.NetfilterRunner, error) {
-	if defaultBool("TS_TEST_FAKE_NETFILTER", false) {
+	if def.Bool(os.Getenv("TS_TEST_FAKE_NETFILTER"), false) {
 		return linuxfw.NewFakeIPTablesRunner(), nil
 	}
 	return linuxfw.New(logf, "")
 }
 
 func getAutoAdvertiseBool() bool {
-	return defaultBool("TS_EXPERIMENTAL_SERVICE_AUTO_ADVERTISEMENT", true)
+	return def.Bool(os.Getenv("TS_EXPERIMENTAL_SERVICE_AUTO_ADVERTISEMENT"), true)
 }
 
 const containerbootWatchMask = ipn.NotifyInitialStatus |

cmd/containerboot/settings.go

@@ -6,6 +6,7 @@
 package main
 
 import (
+	"cmp"
 	"context"
 	"errors"
 	"fmt"
@@ -18,6 +19,7 @@ import (
 
 	"tailscale.com/ipn/conffile"
 	"tailscale.com/kube/kubeclient"
+	"tailscale.com/util/def"
 )
 
 // settings is all the configuration for containerboot.
@@ -89,47 +91,50 @@ type settings struct {
 
 func configFromEnv() (*settings, error) {
 	cfg := &settings{
-		AuthKey:            defaultEnvs([]string{"TS_AUTHKEY", "TS_AUTH_KEY"}, ""),
+		AuthKey:            cmp.Or(os.Getenv("TS_AUTHKEY"), os.Getenv("TS_AUTH_KEY")),
-		ClientID:           defaultEnv("TS_CLIENT_ID", ""),
+		ClientID:           os.Getenv("TS_CLIENT_ID"),
-		ClientSecret:       defaultEnv("TS_CLIENT_SECRET", ""),
+		ClientSecret:       os.Getenv("TS_CLIENT_SECRET"),
-		IDToken:            defaultEnv("TS_ID_TOKEN", ""),
+		IDToken:            os.Getenv("TS_ID_TOKEN"),
-		Audience:           defaultEnv("TS_AUDIENCE", ""),
+		Audience:           os.Getenv("TS_AUDIENCE"),
-		Hostname:           defaultEnv("TS_HOSTNAME", ""),
+		Hostname:           os.Getenv("TS_HOSTNAME"),
 		Routes:             defaultEnvStringPointer("TS_ROUTES"),
-		ServeConfigPath:    defaultEnv("TS_SERVE_CONFIG", ""),
+		ServeConfigPath:    os.Getenv("TS_SERVE_CONFIG"),
-		ProxyTargetIP:      defaultEnv("TS_DEST_IP", ""),
+		ProxyTargetIP:      os.Getenv("TS_DEST_IP"),
-		ProxyTargetDNSName: defaultEnv("TS_EXPERIMENTAL_DEST_DNS_NAME", ""),
+		ProxyTargetDNSName: os.Getenv("TS_EXPERIMENTAL_DEST_DNS_NAME"),
-		TailnetTargetIP:    defaultEnv("TS_TAILNET_TARGET_IP", ""),
+		TailnetTargetIP:    os.Getenv("TS_TAILNET_TARGET_IP"),
-		TailnetTargetFQDN:  defaultEnv("TS_TAILNET_TARGET_FQDN", ""),
+		TailnetTargetFQDN:  os.Getenv("TS_TAILNET_TARGET_FQDN"),
-		DaemonExtraArgs:    defaultEnv("TS_TAILSCALED_EXTRA_ARGS", ""),
+		DaemonExtraArgs:    os.Getenv("TS_TAILSCALED_EXTRA_ARGS"),
-		ExtraArgs:          defaultEnv("TS_EXTRA_ARGS", ""),
+		ExtraArgs:          os.Getenv("TS_EXTRA_ARGS"),
 		InKubernetes:       os.Getenv("KUBERNETES_SERVICE_HOST") != "",
-		UserspaceMode:      defaultBool("TS_USERSPACE", true),
+		UserspaceMode:      def.Bool(os.Getenv("TS_USERSPACE"), true),
-		StateDir:           defaultEnv("TS_STATE_DIR", ""),
+		StateDir:           os.Getenv("TS_STATE_DIR"),
 		AcceptDNS:          defaultEnvBoolPointer("TS_ACCEPT_DNS"),
 		KubeSecret: func() string {
-			if os.Getenv("KUBERNETES_SERVICE_HOST") != "" {
+			if os.Getenv("KUBERNETES_SERVICE_HOST") == "" {
-				return defaultEnv("TS_KUBE_SECRET", "tailscale")
+				return os.Getenv("TS_KUBE_SECRET")
 			}
-			return defaultEnv("TS_KUBE_SECRET", "")
+			// An explicitly empty TS_KUBE_SECRET disables Secret storage, so
+			// unset and empty must stay distinguishable: def.LookupEnv keeps
+			// an explicit "" rather than falling back to the default.
+			return def.LookupEnv("TS_KUBE_SECRET", "tailscale")
 		}(),
-		SOCKSProxyAddr:                        defaultEnv("TS_SOCKS5_SERVER", ""),
+		SOCKSProxyAddr:                        os.Getenv("TS_SOCKS5_SERVER"),
-		HTTPProxyAddr:                         defaultEnv("TS_OUTBOUND_HTTP_PROXY_LISTEN", ""),
+		HTTPProxyAddr:                         os.Getenv("TS_OUTBOUND_HTTP_PROXY_LISTEN"),
-		Socket:                                defaultEnv("TS_SOCKET", "/tmp/tailscaled.sock"),
+		Socket:                                cmp.Or(os.Getenv("TS_SOCKET"), "/tmp/tailscaled.sock"),
-		AuthOnce:                              defaultBool("TS_AUTH_ONCE", false),
+		AuthOnce:                              def.Bool(os.Getenv("TS_AUTH_ONCE"), false),
-		Root:                                  defaultEnv("TS_TEST_ONLY_ROOT", "/"),
+		Root:                                  cmp.Or(os.Getenv("TS_TEST_ONLY_ROOT"), "/"),
 		TailscaledConfigFilePath:              tailscaledConfigFilePath(),
-		AllowProxyingClusterTrafficViaIngress: defaultBool("EXPERIMENTAL_ALLOW_PROXYING_CLUSTER_TRAFFIC_VIA_INGRESS", false),
+		AllowProxyingClusterTrafficViaIngress: def.Bool(os.Getenv("EXPERIMENTAL_ALLOW_PROXYING_CLUSTER_TRAFFIC_VIA_INGRESS"), false),
-		PodIP:                                 defaultEnv("POD_IP", ""),
+		PodIP:                                 os.Getenv("POD_IP"),
-		EnableForwardingOptimizations:         defaultBool("TS_EXPERIMENTAL_ENABLE_FORWARDING_OPTIMIZATIONS", false),
+		EnableForwardingOptimizations:         def.Bool(os.Getenv("TS_EXPERIMENTAL_ENABLE_FORWARDING_OPTIMIZATIONS"), false),
-		HealthCheckAddrPort:                   defaultEnv("TS_HEALTHCHECK_ADDR_PORT", ""),
+		HealthCheckAddrPort:                   os.Getenv("TS_HEALTHCHECK_ADDR_PORT"),
-		LocalAddrPort:                         defaultEnv("TS_LOCAL_ADDR_PORT", "[::]:9002"),
+		LocalAddrPort:                         cmp.Or(os.Getenv("TS_LOCAL_ADDR_PORT"), "[::]:9002"),
-		MetricsEnabled:                        defaultBool("TS_ENABLE_METRICS", false),
+		MetricsEnabled:                        def.Bool(os.Getenv("TS_ENABLE_METRICS"), false),
-		HealthCheckEnabled:                    defaultBool("TS_ENABLE_HEALTH_CHECK", false),
+		HealthCheckEnabled:                    def.Bool(os.Getenv("TS_ENABLE_HEALTH_CHECK"), false),
-		DebugAddrPort:                         defaultEnv("TS_DEBUG_ADDR_PORT", ""),
+		DebugAddrPort:                         os.Getenv("TS_DEBUG_ADDR_PORT"),
-		EgressProxiesCfgPath:                  defaultEnv("TS_EGRESS_PROXIES_CONFIG_PATH", ""),
+		EgressProxiesCfgPath:                  os.Getenv("TS_EGRESS_PROXIES_CONFIG_PATH"),
-		IngressProxiesCfgPath:                 defaultEnv("TS_INGRESS_PROXIES_CONFIG_PATH", ""),
+		IngressProxiesCfgPath:                 os.Getenv("TS_INGRESS_PROXIES_CONFIG_PATH"),
-		PodUID:                                defaultEnv("POD_UID", ""),
+		PodUID:                                os.Getenv("POD_UID"),
 	}
 
 	podIPs, ok := os.LookupEnv("POD_IPS")
@@ -153,7 +158,7 @@ func configFromEnv() (*settings, error) {
 
 	// If cert share is enabled, set the replica as read or write. Only 0th
 	// replica should be able to write.
-	isInCertShareMode := defaultBool("TS_EXPERIMENTAL_CERT_SHARE", false)
+	isInCertShareMode := def.Bool(os.Getenv("TS_EXPERIMENTAL_CERT_SHARE"), false)
 	if isInCertShareMode {
 		cfg.CertShareMode = "ro"
 		podName := os.Getenv("POD_NAME")
@@ -454,15 +459,6 @@ func (cfg *settings) egressSvcsTerminateEPEnabled() bool {
 	return cfg.LocalAddrPort != "" && cfg.EgressProxiesCfgPath != ""
 }
 
-// defaultEnv returns the value of the given envvar name, or defVal if
-// unset.
-func defaultEnv(name, defVal string) string {
-	if v, ok := os.LookupEnv(name); ok {
-		return v
-	}
-	return defVal
-}
-
 // defaultEnvStringPointer returns a pointer to the given envvar value if set, else
 // returns nil. This is useful in cases where we need to distinguish between a
 // variable being set to empty string vs unset.
@@ -484,23 +480,3 @@ func defaultEnvBoolPointer(name string) *bool {
 	}
 	return &ret
 }
-
-func defaultEnvs(names []string, defVal string) string {
-	for _, name := range names {
-		if v, ok := os.LookupEnv(name); ok {
-			return v
-		}
-	}
-	return defVal
-}
-
-// defaultBool returns the boolean value of the given envvar name, or
-// defVal if unset or not a bool.
-func defaultBool(name string, defVal bool) bool {
-	v := os.Getenv(name)
-	ret, err := strconv.ParseBool(v)
-	if err != nil {
-		return defVal
-	}
-	return ret
-}

cmd/containerboot/settings_test.go

@@ -7,6 +7,7 @@ package main
 
 import (
 	"net/netip"
+	"os"
 	"strings"
 	"testing"
 )
@@ -228,6 +229,78 @@ func TestValidateAuthMethods(t *testing.T) {
 	}
 }
 
+func TestConfigFromEnvEmptyDefaults(t *testing.T) {
+	tests := []struct {
+		env  string
+		get  func(*settings) string
+		want string
+	}{
+		{
+			env:  "TS_SOCKET",
+			get:  func(c *settings) string { return c.Socket },
+			want: "/tmp/tailscaled.sock",
+		},
+		{
+			env:  "TS_LOCAL_ADDR_PORT",
+			get:  func(c *settings) string { return c.LocalAddrPort },
+			want: "[::]:9002",
+		},
+		{
+			env:  "TS_TEST_ONLY_ROOT",
+			get:  func(c *settings) string { return c.Root },
+			want: "/",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.env, func(t *testing.T) {
+			t.Setenv(tt.env, "")
+			cfg, err := configFromEnv()
+			if err != nil {
+				t.Fatal(err)
+			}
+			if got := tt.get(cfg); got != tt.want {
+				t.Errorf(`%s set to empty "": got %q, want default %q`, tt.env, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestConfigFromEnvKubeSecret(t *testing.T) {
+	tests := []struct {
+		name         string
+		inKubernetes bool
+		unset        bool
+		value        string
+		want         string
+	}{
+		{name: "in_kubernetes_unset", inKubernetes: true, unset: true, want: "tailscale"},
+		{name: "in_kubernetes_empty", inKubernetes: true, value: "", want: ""},
+		{name: "in_kubernetes_set", inKubernetes: true, value: "custom", want: "custom"},
+		{name: "not_in_kubernetes_unset", inKubernetes: false, unset: true, want: ""},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// t.Setenv registers a t.Cleanup to restore the original value, so
+			// route the unset cases through it rather than a bare os.Unsetenv.
+			t.Setenv("KUBERNETES_SERVICE_HOST", "10.96.0.1")
+			if !tt.inKubernetes {
+				os.Unsetenv("KUBERNETES_SERVICE_HOST")
+			}
+			t.Setenv("TS_KUBE_SECRET", tt.value)
+			if tt.unset {
+				os.Unsetenv("TS_KUBE_SECRET")
+			}
+			cfg, err := configFromEnv()
+			if err != nil {
+				t.Fatal(err)
+			}
+			if cfg.KubeSecret != tt.want {
+				t.Errorf("KubeSecret = %q, want %q", cfg.KubeSecret, tt.want)
+			}
+		})
+	}
+}
+
 func TestHandlesKubeIPV6(t *testing.T) {
 	t.Setenv("TS_LOCAL_ADDR_PORT", "fd7a:115c:a1e0::6c34:352:9002")
 	t.Setenv("POD_IPS", "fd7a:115c:a1e0::6c34:352")

util/def/def.go

@@ -1,10 +1,11 @@
 // Copyright (c) Tailscale Inc & contributors
 // SPDX-License-Identifier: BSD-3-Clause
 
-// Package def parses strings with fallback default values.
+// Package def parses strings and environment variables with fallback default values.
 package def
 
 import (
+	"os"
 	"strconv"
 	"time"
 )
@@ -32,3 +33,13 @@ func Duration(s string, def time.Duration) time.Duration {
 	}
 	return v
 }
+
+// LookupEnv retrieves the value of the environment variable named by the key.
+// If the variable is present in the environment the value (which may be
+// empty) is returned. Otherwise, it returns def.
+func LookupEnv(key, def string) string {
+	if v, ok := os.LookupEnv(key); ok {
+		return v
+	}
+	return def
+}

util/def/def_test.go

@@ -4,6 +4,7 @@
 package def_test
 
 import (
+	"os"
 	"strconv"
 	"testing"
 	"time"
@@ -11,6 +12,32 @@ import (
 	"tailscale.com/util/def"
 )
 
+func TestLookupEnv(t *testing.T) {
+	const key = "TS_DEF_TEST_LOOKUPENV"
+	tests := []struct {
+		name  string
+		unset bool
+		value string
+		def   string
+		want  string
+	}{
+		{name: "unset", unset: true, def: "fallback", want: "fallback"},
+		{name: "set", value: "value", def: "fallback", want: "value"},
+		{name: "empty", value: "", def: "fallback", want: ""},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Setenv(key, tt.value)
+			if tt.unset {
+				os.Unsetenv(key)
+			}
+			if got := def.LookupEnv(key, tt.def); got != tt.want {
+				t.Errorf("LookupEnv(%q, %q) = %q; want %q", key, tt.def, got, tt.want)
+			}
+		})
+	}
+}
+
 func TestBool(t *testing.T) {
 	tests := []struct {
 		name string