From b91e844014228a488979612031b3bd474d6d9f4b Mon Sep 17 00:00:00 2001 From: Nick Rossi Date: Fri, 17 Jul 2026 20:32:52 -0500 Subject: [PATCH] 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 --- cmd/containerboot/main.go | 5 +- cmd/containerboot/settings.go | 104 +++++++++++------------------ cmd/containerboot/settings_test.go | 73 ++++++++++++++++++++ util/def/def.go | 13 +++- util/def/def_test.go | 27 ++++++++ 5 files changed, 155 insertions(+), 67 deletions(-) diff --git a/cmd/containerboot/main.go b/cmd/containerboot/main.go index 3afddbe3b..04651cd48 100644 --- a/cmd/containerboot/main.go +++ b/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 | diff --git a/cmd/containerboot/settings.go b/cmd/containerboot/settings.go index f695f2e5d..17483194f 100644 --- a/cmd/containerboot/settings.go +++ b/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"}, ""), - ClientID: defaultEnv("TS_CLIENT_ID", ""), - ClientSecret: defaultEnv("TS_CLIENT_SECRET", ""), - IDToken: defaultEnv("TS_ID_TOKEN", ""), - Audience: defaultEnv("TS_AUDIENCE", ""), - Hostname: defaultEnv("TS_HOSTNAME", ""), + AuthKey: cmp.Or(os.Getenv("TS_AUTHKEY"), os.Getenv("TS_AUTH_KEY")), + ClientID: os.Getenv("TS_CLIENT_ID"), + ClientSecret: os.Getenv("TS_CLIENT_SECRET"), + IDToken: os.Getenv("TS_ID_TOKEN"), + Audience: os.Getenv("TS_AUDIENCE"), + Hostname: os.Getenv("TS_HOSTNAME"), Routes: defaultEnvStringPointer("TS_ROUTES"), - ServeConfigPath: defaultEnv("TS_SERVE_CONFIG", ""), - ProxyTargetIP: defaultEnv("TS_DEST_IP", ""), - ProxyTargetDNSName: defaultEnv("TS_EXPERIMENTAL_DEST_DNS_NAME", ""), - TailnetTargetIP: defaultEnv("TS_TAILNET_TARGET_IP", ""), - TailnetTargetFQDN: defaultEnv("TS_TAILNET_TARGET_FQDN", ""), - DaemonExtraArgs: defaultEnv("TS_TAILSCALED_EXTRA_ARGS", ""), - ExtraArgs: defaultEnv("TS_EXTRA_ARGS", ""), + ServeConfigPath: os.Getenv("TS_SERVE_CONFIG"), + ProxyTargetIP: os.Getenv("TS_DEST_IP"), + ProxyTargetDNSName: os.Getenv("TS_EXPERIMENTAL_DEST_DNS_NAME"), + TailnetTargetIP: os.Getenv("TS_TAILNET_TARGET_IP"), + TailnetTargetFQDN: os.Getenv("TS_TAILNET_TARGET_FQDN"), + DaemonExtraArgs: os.Getenv("TS_TAILSCALED_EXTRA_ARGS"), + ExtraArgs: os.Getenv("TS_EXTRA_ARGS"), InKubernetes: os.Getenv("KUBERNETES_SERVICE_HOST") != "", - UserspaceMode: defaultBool("TS_USERSPACE", true), - StateDir: defaultEnv("TS_STATE_DIR", ""), + UserspaceMode: def.Bool(os.Getenv("TS_USERSPACE"), true), + StateDir: os.Getenv("TS_STATE_DIR"), AcceptDNS: defaultEnvBoolPointer("TS_ACCEPT_DNS"), KubeSecret: func() string { - if os.Getenv("KUBERNETES_SERVICE_HOST") != "" { - return defaultEnv("TS_KUBE_SECRET", "tailscale") + if os.Getenv("KUBERNETES_SERVICE_HOST") == "" { + 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", ""), - HTTPProxyAddr: defaultEnv("TS_OUTBOUND_HTTP_PROXY_LISTEN", ""), - Socket: defaultEnv("TS_SOCKET", "/tmp/tailscaled.sock"), - AuthOnce: defaultBool("TS_AUTH_ONCE", false), - Root: defaultEnv("TS_TEST_ONLY_ROOT", "/"), + SOCKSProxyAddr: os.Getenv("TS_SOCKS5_SERVER"), + HTTPProxyAddr: os.Getenv("TS_OUTBOUND_HTTP_PROXY_LISTEN"), + Socket: cmp.Or(os.Getenv("TS_SOCKET"), "/tmp/tailscaled.sock"), + AuthOnce: def.Bool(os.Getenv("TS_AUTH_ONCE"), false), + Root: cmp.Or(os.Getenv("TS_TEST_ONLY_ROOT"), "/"), TailscaledConfigFilePath: tailscaledConfigFilePath(), - AllowProxyingClusterTrafficViaIngress: defaultBool("EXPERIMENTAL_ALLOW_PROXYING_CLUSTER_TRAFFIC_VIA_INGRESS", false), - PodIP: defaultEnv("POD_IP", ""), - EnableForwardingOptimizations: defaultBool("TS_EXPERIMENTAL_ENABLE_FORWARDING_OPTIMIZATIONS", false), - HealthCheckAddrPort: defaultEnv("TS_HEALTHCHECK_ADDR_PORT", ""), - LocalAddrPort: defaultEnv("TS_LOCAL_ADDR_PORT", "[::]:9002"), - MetricsEnabled: defaultBool("TS_ENABLE_METRICS", false), - HealthCheckEnabled: defaultBool("TS_ENABLE_HEALTH_CHECK", false), - DebugAddrPort: defaultEnv("TS_DEBUG_ADDR_PORT", ""), - EgressProxiesCfgPath: defaultEnv("TS_EGRESS_PROXIES_CONFIG_PATH", ""), - IngressProxiesCfgPath: defaultEnv("TS_INGRESS_PROXIES_CONFIG_PATH", ""), - PodUID: defaultEnv("POD_UID", ""), + AllowProxyingClusterTrafficViaIngress: def.Bool(os.Getenv("EXPERIMENTAL_ALLOW_PROXYING_CLUSTER_TRAFFIC_VIA_INGRESS"), false), + PodIP: os.Getenv("POD_IP"), + EnableForwardingOptimizations: def.Bool(os.Getenv("TS_EXPERIMENTAL_ENABLE_FORWARDING_OPTIMIZATIONS"), false), + HealthCheckAddrPort: os.Getenv("TS_HEALTHCHECK_ADDR_PORT"), + LocalAddrPort: cmp.Or(os.Getenv("TS_LOCAL_ADDR_PORT"), "[::]:9002"), + MetricsEnabled: def.Bool(os.Getenv("TS_ENABLE_METRICS"), false), + HealthCheckEnabled: def.Bool(os.Getenv("TS_ENABLE_HEALTH_CHECK"), false), + DebugAddrPort: os.Getenv("TS_DEBUG_ADDR_PORT"), + EgressProxiesCfgPath: os.Getenv("TS_EGRESS_PROXIES_CONFIG_PATH"), + IngressProxiesCfgPath: os.Getenv("TS_INGRESS_PROXIES_CONFIG_PATH"), + 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 -} diff --git a/cmd/containerboot/settings_test.go b/cmd/containerboot/settings_test.go index eca50101b..c2a61b85f 100644 --- a/cmd/containerboot/settings_test.go +++ b/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") diff --git a/util/def/def.go b/util/def/def.go index 1a87d07f4..fb4c8d755 100644 --- a/util/def/def.go +++ b/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 +} diff --git a/util/def/def_test.go b/util/def/def_test.go index 59421dd17..f9e62d6ed 100644 --- a/util/def/def_test.go +++ b/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