From 01ee638ccac0d537c841fc830732b58c5627fe73 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 2 Nov 2020 08:33:34 -0800 Subject: [PATCH] Change some os.IsNotExist to errors.Is(err, os.ErrNotExist) for non-os errors. os.IsNotExist doesn't unwrap errors. errors.Is does. The ioutil.ReadFile ones happened to be fine but I changed them so we're consistent with the rule: if the error comes from os, you can use os.IsNotExist, but from any other package, use errors.Is. (errors.Is always would also work, but not worth updating all the code) The motivation here was that we were logging about failure to migrate legacy relay node prefs file on startup, even though the code tried to avoid that. See golang/go#41122 --- cmd/derper/derper.go | 2 +- ipn/local.go | 4 ++-- ipn/prefs_test.go | 15 +++++++++++++++ wgengine/router/dns/direct.go | 3 ++- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/cmd/derper/derper.go b/cmd/derper/derper.go index 9fafe210c..f328932a2 100644 --- a/cmd/derper/derper.go +++ b/cmd/derper/derper.go @@ -63,7 +63,7 @@ func loadConfig() config { } b, err := ioutil.ReadFile(*configPath) switch { - case os.IsNotExist(err): + case errors.Is(err, os.ErrNotExist): return writeNewConfig() case err != nil: log.Fatal(err) diff --git a/ipn/local.go b/ipn/local.go index b7066e1aa..f41f47d98 100644 --- a/ipn/local.go +++ b/ipn/local.go @@ -803,8 +803,8 @@ func (b *LocalBackend) loadStateLocked(key StateKey, prefs *Prefs, legacyPath st if legacyPath != "" { b.prefs, err = LoadPrefs(legacyPath) if err != nil { - if !os.IsNotExist(err) { - b.logf("Failed to load legacy prefs: %v", err) + if !errors.Is(err, os.ErrNotExist) { + b.logf("failed to load legacy prefs: %v", err) } b.prefs = NewPrefs() } else { diff --git a/ipn/prefs_test.go b/ipn/prefs_test.go index 767686cdf..21e8e7b44 100644 --- a/ipn/prefs_test.go +++ b/ipn/prefs_test.go @@ -5,8 +5,12 @@ package ipn import ( + "errors" + "fmt" + "os" "reflect" "testing" + "time" "github.com/tailscale/wireguard-go/wgcfg" "tailscale.com/control/controlclient" @@ -330,3 +334,14 @@ func TestPrefsPretty(t *testing.T) { } } } + +func TestLoadPrefsNotExist(t *testing.T) { + bogusFile := fmt.Sprintf("/tmp/not-exist-%d", time.Now().UnixNano()) + + p, err := LoadPrefs(bogusFile) + if errors.Is(err, os.ErrNotExist) { + // expected. + return + } + t.Fatalf("unexpected prefs=%#v, err=%v", p, err) +} diff --git a/wgengine/router/dns/direct.go b/wgengine/router/dns/direct.go index 62814c5f6..bd1c03b9d 100644 --- a/wgengine/router/dns/direct.go +++ b/wgengine/router/dns/direct.go @@ -9,6 +9,7 @@ package dns import ( "bufio" "bytes" + "errors" "fmt" "io" "io/ioutil" @@ -130,7 +131,7 @@ func (m directManager) Up(config Config) error { contents, err := ioutil.ReadFile(resolvConf) // If the original did not exist, still back up an empty file. // The presence of a backup file is the way we know that Up ran. - if err != nil && !os.IsNotExist(err) { + if err != nil && !errors.Is(err, os.ErrNotExist) { return err } if err := atomicfile.WriteFile(backupConf, contents, 0644); err != nil {