From 3562b5bdfad85adb690c9816c517524926672cf6 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 16 Sep 2022 20:24:28 -0700 Subject: [PATCH] envknob, health: support Synology, show parse errors in status Updates #5114 Change-Id: I8ac7a22a511f5a7d0dcb8cac470d4a403aa8c817 Signed-off-by: Brad Fitzpatrick --- cmd/tailscaled/tailscaled.go | 8 ++-- cmd/tailscaled/tailscaled_windows.go | 4 +- envknob/envknob.go | 63 +++++++++++++++++++++++++--- envknob/envknob_windows.go | 27 ------------ health/health.go | 3 ++ 5 files changed, 66 insertions(+), 39 deletions(-) delete mode 100644 envknob/envknob_windows.go diff --git a/cmd/tailscaled/tailscaled.go b/cmd/tailscaled/tailscaled.go index e06380beb..794eb90ac 100644 --- a/cmd/tailscaled/tailscaled.go +++ b/cmd/tailscaled/tailscaled.go @@ -131,11 +131,9 @@ var subCommands = map[string]*func([]string) error{ var beCLI func() // non-nil if CLI is linked in -var diskConfigErr error - func main() { envknob.PanicIfAnyEnvCheckedInInit() - diskConfigErr = envknob.ApplyDiskConfig() + envknob.ApplyDiskConfig() printVersion := false flag.IntVar(&args.verbose, "verbose", 0, "log verbosity level; 0 is default, 1 or higher are increasingly verbose") @@ -313,8 +311,8 @@ func run() error { pol.Shutdown(ctx) }() - if diskConfigErr != nil { - log.Printf("Error reading environment config: %v", diskConfigErr) + if err := envknob.ApplyDiskConfigError(); err != nil { + log.Printf("Error reading environment config: %v", err) } if isWindowsService() { diff --git a/cmd/tailscaled/tailscaled_windows.go b/cmd/tailscaled/tailscaled_windows.go index 2798aa531..7ed82ce0b 100644 --- a/cmd/tailscaled/tailscaled_windows.go +++ b/cmd/tailscaled/tailscaled_windows.go @@ -197,8 +197,8 @@ func beWindowsSubprocess() bool { log.Printf("Program starting: v%v: %#v", version.Long, os.Args) log.Printf("subproc mode: logid=%v", logid) - if diskConfigErr != nil { - log.Printf("Error reading environment config: %v", diskConfigErr) + if err := envknob.ApplyDiskConfigError(); err != nil { + log.Printf("Error reading environment config: %v", err) } go func() { diff --git a/envknob/envknob.go b/envknob/envknob.go index 22527f478..f8ff392bc 100644 --- a/envknob/envknob.go +++ b/envknob/envknob.go @@ -22,6 +22,7 @@ import ( "io" "log" "os" + "path/filepath" "runtime" "sort" "strconv" @@ -30,6 +31,7 @@ import ( "sync/atomic" "tailscale.com/types/opt" + "tailscale.com/version/distro" ) var ( @@ -325,7 +327,10 @@ func PanicIfAnyEnvCheckedInInit() { } } -var platformApplyDiskConfig func() error +var applyDiskConfigErr error + +// ApplyDiskConfigError returns the most recent result of ApplyDiskConfig. +func ApplyDiskConfigError() error { return applyDiskConfigErr } // ApplyDiskConfig returns a platform-specific config file of environment keys/values and // applies them. On Linux and Unix operating systems, it's a no-op and always returns nil. @@ -334,11 +339,59 @@ var platformApplyDiskConfig func() error // It exists primarily for Windows to make it easy to apply environment variables to // a running service in a way similar to modifying /etc/default/tailscaled on Linux. // On Windows, you use %ProgramData%\Tailscale\tailscaled-env.txt instead. -func ApplyDiskConfig() error { - if f := platformApplyDiskConfig; f != nil { - return f() +func ApplyDiskConfig() (err error) { + var f *os.File + defer func() { + if err != nil { + // Stash away our return error for the healthcheck package to use. + applyDiskConfigErr = fmt.Errorf("error parsing %s: %w", f.Name(), err) + } + }() + + // First try the explicitly-provided value for development testing. Not + // useful for users to use on their own. (if they can set this, they can set + // any environment variable anyway) + if name := os.Getenv("TS_DEBUG_ENV_FILE"); name != "" { + f, err = os.Open(name) + if err != nil { + return fmt.Errorf("error opening explicitly configured TS_DEBUG_ENV_FILE: %w", err) + } + defer f.Close() + return applyKeyValueEnv(f) + } + + name := getPlatformEnvFile() + if name == "" { + return nil + } + f, err = os.Open(name) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return err + } + defer f.Close() + return applyKeyValueEnv(f) +} + +// getPlatformEnvFile returns the current platform's path to an optional +// tailscaled-env.txt file. It returns an empty string if none is defined +// for the platform. +func getPlatformEnvFile() string { + switch runtime.GOOS { + case "windows": + return filepath.Join(os.Getenv("ProgramData"), "Tailscale", "tailscaled-env.txt") + case "linux": + if distro.Get() == distro.Synology { + return "/etc/tailscale/tailscaled-env.txt" + } + case "darwin": + // TODO(bradfitz): figure this out. There are three ways to run + // Tailscale on macOS (tailscaled, GUI App Store, GUI System Extension) + // and we should deal with all three. } - return nil + return "" } // applyKeyValueEnv reads key=value lines r and calls Setenv for each. diff --git a/envknob/envknob_windows.go b/envknob/envknob_windows.go deleted file mode 100644 index 2e539cfaf..000000000 --- a/envknob/envknob_windows.go +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright (c) 2022 Tailscale Inc & AUTHORS All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package envknob - -import ( - "os" - "path/filepath" -) - -func init() { - platformApplyDiskConfig = platformApplyDiskConfigWindows -} - -func platformApplyDiskConfigWindows() error { - name := filepath.Join(os.Getenv("ProgramData"), "Tailscale", "tailscaled-env.txt") - f, err := os.Open(name) - if os.IsNotExist(err) { - return nil - } - if err != nil { - return err - } - defer f.Close() - return applyKeyValueEnv(f) -} diff --git a/health/health.go b/health/health.go index 80474857e..a41a28385 100644 --- a/health/health.go +++ b/health/health.go @@ -383,6 +383,9 @@ func overallErrorLocked() error { for _, s := range controlHealth { errs = append(errs, errors.New(s)) } + if err := envknob.ApplyDiskConfigError(); err != nil { + errs = append(errs, err) + } if e := fakeErrForTesting(); len(errs) == 0 && e != "" { return errors.New(e) }