From d3a55915f5b92f406f0a2fa79e69385a8bc13da1 Mon Sep 17 00:00:00 2001 From: Tim Gross Date: Tue, 4 Oct 2022 11:52:12 -0400 Subject: [PATCH] client: defer `nobody` user lookup so Windows doesn't panic (#14790) In #14742 we introduced a cached lookup of the `nobody` user, which is only ever called on Unixish machines. But the initial caching was being done in an `init` block, which meant it was being run on Windows as well. This prevents the Nomad agent from starting on Windows. An alternative fix here would be to have a separate `init` block for Windows and Unix, but this potentially masks incorrect behavior if we accidentally added a call to the `Nobody()` method on Windows later. This way we're forced to handle the error in the caller. --- client/allocdir/fs_unix.go | 11 +++++++---- helper/users/lookup.go | 24 ++++++++++-------------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/client/allocdir/fs_unix.go b/client/allocdir/fs_unix.go index 72ec9a1ea..6f58b20d3 100644 --- a/client/allocdir/fs_unix.go +++ b/client/allocdir/fs_unix.go @@ -40,14 +40,17 @@ func dropDirPermissions(path string, desired os.FileMode) error { return nil } - nobody := users.Nobody() - - uid, err := getUid(&nobody) + nobody, err := users.Nobody() if err != nil { return err } - gid, err := getGid(&nobody) + uid, err := getUid(nobody) + if err != nil { + return err + } + + gid, err := getGid(nobody) if err != nil { return err } diff --git a/helper/users/lookup.go b/helper/users/lookup.go index bac6a6155..87b7817e8 100644 --- a/helper/users/lookup.go +++ b/helper/users/lookup.go @@ -1,33 +1,29 @@ package users import ( - "fmt" "os/user" "sync" ) // lock is used to serialize all user lookup at the process level, because // some NSS implementations are not concurrency safe -var lock *sync.Mutex +var lock sync.Mutex // nobody is a cached copy of the nobody user, which is going to be looked-up // frequently and is unlikely to be modified on the underlying system. -var nobody user.User +var nobody *user.User // Nobody returns User data for the "nobody" user on the system, bypassing the // locking / file read / NSS lookup. -func Nobody() user.User { - // original is immutable via copy by value - return nobody -} - -func init() { - lock = new(sync.Mutex) - u, err := Lookup("nobody") - if err != nil { - panic(fmt.Sprintf("unable to lookup the nobody user: %v", err)) +func Nobody() (*user.User, error) { + lock.Lock() + defer lock.Unlock() + if nobody != nil { + return nobody, nil } - nobody = *u + u, err := user.Lookup("nobody") + nobody = u + return u, err } // Lookup username while holding a global process lock.