From 29d9f2b98ebe6808619736e90369de8b6884164e Mon Sep 17 00:00:00 2001 From: Hank Donnay Date: Mon, 18 Mar 2024 10:53:16 -0500 Subject: [PATCH 1/2] libindex: add fallback path for O_TMPFILE not being supported Signed-off-by: Hank Donnay --- libindex/fetcher.go | 6 ----- libindex/tempfile_linux.go | 49 ++++++++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/libindex/fetcher.go b/libindex/fetcher.go index d7783d1b6..93dfbfeda 100644 --- a/libindex/fetcher.go +++ b/libindex/fetcher.go @@ -34,12 +34,6 @@ var ( _ indexer.DescriptionRealizer = (*FetchProxy)(nil) ) -// BUG(hank) On Linux, the [RemoteFetchArena] makes use of the O_TMPFILE flag to -// [open(2)], which requires version 3.11 or newer. There is no fallback for -// older kernels. -// -// [open(2)]: https://man7.org/linux/man-pages/man2/open.2.html - // RemoteFetchArena uses disk space to track fetched layers, removing them once // all users are done with the layers. type RemoteFetchArena struct { diff --git a/libindex/tempfile_linux.go b/libindex/tempfile_linux.go index fc8b8e0f3..0e0928dcb 100644 --- a/libindex/tempfile_linux.go +++ b/libindex/tempfile_linux.go @@ -1,24 +1,65 @@ package libindex import ( + "errors" "fmt" "os" + "sync" "golang.org/x/sys/unix" ) +var tmpMap sync.Map + +func canTmp(dir string) (ok, loaded bool) { + v, loaded := tmpMap.Load(dir) + if v == nil { + return false, loaded + } + return v.(bool), loaded +} + +func setTmp(dir string, ok bool) { + tmpMap.Store(dir, ok) +} + type tempFile struct { *os.File } func openTemp(dir string) (*tempFile, error) { - f, err := os.OpenFile(dir, os.O_WRONLY|unix.O_TMPFILE, 0644) + var f *os.File + var err error + + ok, loaded := canTmp(dir) + switch { + case loaded && ok: + f, err = os.OpenFile(dir, os.O_WRONLY|unix.O_TMPFILE, 0644) + case loaded && !ok: + f, err = os.CreateTemp(dir, "fetcher.*") + case !loaded: + f, err = os.OpenFile(dir, os.O_WRONLY|unix.O_TMPFILE, 0644) + if err == nil || !errors.Is(err, unix.ENOTSUP) { + ok = true + break + } + f, err = os.CreateTemp(dir, "fetcher.*") + default: + panic("unreachable") + } + if !loaded { + setTmp(dir, ok) + } + if !ok && err == nil { + // This is just a best-effort action to keep files from accumulating. + // The correct way is to use the kernel feature for this: the O_TMPFILE flag. + _ = os.Remove(f.Name()) + } + if err != nil { return nil, err } - return &tempFile{ - File: f, - }, nil + return &tempFile{File: f}, nil } func (t *tempFile) Reopen() (*os.File, error) { From fedb9d327aa7a1497502799fc28f23234eb8e186 Mon Sep 17 00:00:00 2001 From: Hank Donnay Date: Tue, 19 Mar 2024 13:15:24 -0500 Subject: [PATCH 2/2] libindex: update and document file placement logic Signed-off-by: Hank Donnay --- libindex/fetcher.go | 27 ++++++++++++++++++++++++++- libindex/tempdir_other.go | 8 ++++++++ libindex/tempdir_unix.go | 18 ++++++++++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 libindex/tempdir_other.go create mode 100644 libindex/tempdir_unix.go diff --git a/libindex/fetcher.go b/libindex/fetcher.go index 93dfbfeda..7c1b2dc39 100644 --- a/libindex/fetcher.go +++ b/libindex/fetcher.go @@ -48,11 +48,36 @@ type RemoteFetchArena struct { } // NewRemoteFetchArena returns an initialized RemoteFetchArena. +// +// If the "root" parameter is "", the advice in [file-hierarchy(7)] and ["Using +// /tmp/ and /var/tmp/ Safely"] is followed. Specifically, "/var/tmp" is used +// unless "TMPDIR" is set in the environment, in which case the contents of that +// variable are interpreted as a path and used. +// +// The RemoteFetchArena attempts to use [O_TMPFILE] and falls back to +// [os.CreateTemp] if that seems to not work. If the filesystem backing "root" +// does not support O_TMPFILE, files may linger in the event of a process +// crashing or an unclean shutdown. Operators should either use a different +// filesystem or arrange for periodic cleanup via [systemd-tmpfiles(8)] or +// similar. +// +// In a containerized environment, operators may need to mount a directory or +// filesystem on "/var/tmp". +// +// On OSX, temporary files are not unlinked from the filesystem upon creation, +// because an equivalent to Linux's "/proc/self/fd" doesn't seem to exist. +// +// On UNIX-unlike systems, none of the above logic takes place. +// +// [file-hierarchy(7)]: https://www.freedesktop.org/software/systemd/man/latest/file-hierarchy.html +// ["Using /tmp/ and /var/tmp/ Safely"]: https://systemd.io/TEMPORARY_DIRECTORIES/ +// [O_TMPFILE]: https://man7.org/linux/man-pages/man2/open.2.html +// [systemd-tmpfiles(8)]: https://www.freedesktop.org/software/systemd/man/latest/systemd-tmpfiles.html func NewRemoteFetchArena(wc *http.Client, root string) *RemoteFetchArena { return &RemoteFetchArena{ wc: wc, sf: &singleflight.Group{}, - root: root, + root: fixTemp(root), } } diff --git a/libindex/tempdir_other.go b/libindex/tempdir_other.go new file mode 100644 index 000000000..83f9622eb --- /dev/null +++ b/libindex/tempdir_other.go @@ -0,0 +1,8 @@ +//go:build !unix + +package libindex + +// FixTemp is a no-op. +func fixTemp(d string) string { + return d +} diff --git a/libindex/tempdir_unix.go b/libindex/tempdir_unix.go new file mode 100644 index 000000000..10d8b7505 --- /dev/null +++ b/libindex/tempdir_unix.go @@ -0,0 +1,18 @@ +package libindex + +import ( + "os" +) + +// FixTemp modifies "dir" according to the documented defaults. +// +// See [NewRemoteFetchArena]. +func fixTemp(dir string) string { + if dir != "" { + return dir + } + if d, ok := os.LookupEnv("TMPDIR"); ok && d != "" { + return d + } + return "/var/tmp" +}