From ed0dfd2ffbfeb0a612623dd783d855f7b413252e Mon Sep 17 00:00:00 2001 From: Seth Hoenig Date: Mon, 17 Apr 2023 12:30:30 -0500 Subject: [PATCH] users: eliminate nobody user memoization (#16904) This PR eliminates code specific to looking up and caching the uid/gid/user.User object associated with the nobody user in an init block. This code existed before adding the generic users cache and was meant to optimize the one search path we knew would happen often. Now that we have the cache, seems reasonable to eliminate this init block and use the cache instead like for any other user. Also fixes a constraint on the podman (and other) drivers, where building without CGO became problematic on some OS like Fedora IoT where the nobody user cannot be found with the pure-Go standard library. Fixes github.com/hashicorp/nomad-driver-podman/issues/228 --- client/allocdir/fs_unix.go | 9 +++-- helper/users/lookup_linux_test.go | 10 ++---- helper/users/lookup_unix.go | 55 ------------------------------- 3 files changed, 9 insertions(+), 65 deletions(-) delete mode 100644 helper/users/lookup_unix.go diff --git a/client/allocdir/fs_unix.go b/client/allocdir/fs_unix.go index db0d6ac0e..b41b78b58 100644 --- a/client/allocdir/fs_unix.go +++ b/client/allocdir/fs_unix.go @@ -43,14 +43,17 @@ func dropDirPermissions(path string, desired os.FileMode) error { return nil } - nobody := users.Nobody() + u, err := users.Lookup("nobody") + if err != nil { + return fmt.Errorf("Unable to find nobody user: %w", err) + } - uid, err := getUid(&nobody) + uid, err := getUid(u) if err != nil { return err } - gid, err := getGid(&nobody) + gid, err := getGid(u) if err != nil { return err } diff --git a/helper/users/lookup_linux_test.go b/helper/users/lookup_linux_test.go index a8fb9fefe..417f7a5fe 100644 --- a/helper/users/lookup_linux_test.go +++ b/helper/users/lookup_linux_test.go @@ -43,12 +43,6 @@ func TestLookup_Linux(t *testing.T) { } } -func TestLookup_NobodyIDs(t *testing.T) { - uid, gid := NobodyIDs() - must.Eq(t, 65534, uid) // ubuntu - must.Eq(t, 65534, gid) // ubuntu -} - func TestWriteFileFor_Linux(t *testing.T) { // This is really how you have to retrieve umask. See `man 2 umask` umask := unix.Umask(0) @@ -93,7 +87,9 @@ func TestSocketFileFor_Linux(t *testing.T) { ln, err := SocketFileFor(logger, path, "nobody") must.NoError(t, err) must.NotNil(t, ln) - defer ln.Close() + t.Cleanup(func() { + _ = ln.Close() + }) stat, err := os.Lstat(path) must.NoError(t, err) diff --git a/helper/users/lookup_unix.go b/helper/users/lookup_unix.go deleted file mode 100644 index efe506e42..000000000 --- a/helper/users/lookup_unix.go +++ /dev/null @@ -1,55 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -//go:build unix - -package users - -import ( - "fmt" - "os/user" - "strconv" -) - -var ( - // 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. - nobody user.User - - // nobodyUID is a cached copy of the int value of the nobody user's UID. - nobodyUID uint32 - - // nobodyGID int is a cached copy of the int value of the nobody users's GID. - nobodyGID uint32 -) - -// Nobody returns User data for the "nobody" user on the system, bypassing the -// locking / file read / NSS lookup. -func Nobody() user.User { - return nobody -} - -// NobodyIDs returns the integer UID and GID of the nobody user. -func NobodyIDs() (uint32, uint32) { - return nobodyUID, nobodyGID -} - -func init() { - u, err := internalLookupUser("nobody") - if err != nil { - panic(fmt.Sprintf("failed to lookup nobody user: %v", err)) - } - nobody = *u - - uid, err := strconv.Atoi(u.Uid) - if err != nil { - panic(fmt.Sprintf("failed to parse nobody UID: %v", err)) - } - - gid, err := strconv.Atoi(u.Gid) - if err != nil { - panic(fmt.Sprintf("failed to parse nobody GID: %v", err)) - } - - nobodyUID, nobodyGID = uint32(uid), uint32(gid) -}