some initial cleanup to account for drift

Author: DaneEveritt

environment/settings.go

@@ -107,9 +107,9 @@ func (l Limits) AsContainerResources() container.Resources {
 		Memory:            l.BoundedMemoryLimit(),
 		MemoryReservation: l.MemoryLimit * 1024 * 1024,
 		MemorySwap:        l.ConvertedSwap(),
-		BlkioWeight:       l.IoWeight,
-		OomKillDisable:    &l.OOMDisabled,
-		PidsLimit:         &pids,
+		// BlkioWeight:       l.IoWeight,
+		OomKillDisable: &l.OOMDisabled,
+		PidsLimit:      &pids,
 	}
 
 	// If the CPU Limit is not set, don't send any of these fields through. Providing

router/downloader/downloader.go

@@ -199,13 +199,8 @@ func (dl *Download) Execute() error {
 		return errors.New("downloader: got bad response status from endpoint: " + res.Status)
 	}
 
-	// If there is a Content-Length header on this request go ahead and check that we can
-	// even write the whole file before beginning this process. If there is no header present
-	// we'll just have to give it a spin and see how it goes.
-	if res.ContentLength > 0 {
-		if err := dl.server.Filesystem().HasSpaceFor(res.ContentLength); err != nil {
-			return errors.WrapIf(err, "downloader: failed to write file: not enough space")
-		}
+	if res.ContentLength < 1 {
+		return errors.New("downloader: request is missing ContentLength")
 	}
 
 	if dl.req.UseHeader {
@@ -232,8 +227,10 @@ func (dl *Download) Execute() error {
 	p := dl.Path()
 	dl.server.Log().WithField("path", p).Debug("writing remote file to disk")
 
+	// Write the file while tracking the progress, Write will check that the
+	// size of the file won't exceed the disk limit.
 	r := io.TeeReader(res.Body, dl.counter(res.ContentLength))
-	if err := dl.server.Filesystem().Writefile(p, r); err != nil {
+	if err := dl.server.Filesystem().Write(p, r, res.ContentLength, 0o644); err != nil {
 		return errors.WrapIf(err, "downloader: failed to write file to server directory")
 	}
 	return nil

router/router_download.go

@@ -56,6 +56,8 @@ func getDownloadBackup(c *gin.Context) {
 		return
 	}
 
+	// The use of `os` here is safe as backups are not stored within server access
+	// directories, and this path is program-controlled, not user input.
 	f, err := os.Open(b.Path())
 	if err != nil {
 		middleware.CaptureAndAbort(c, err)

router/router_server.go

@@ -225,13 +225,18 @@ func deleteServer(c *gin.Context) {
 	// done in a separate process since failure is not the end of the world and can be
 	// manually cleaned up after the fact.
 	//
-	// In addition, servers with large amounts of files can take some time to finish deleting,
+	// In addition, servers with large numbers of files can take some time to finish deleting,
 	// so we don't want to block the HTTP call while waiting on this.
+	p := s.Filesystem().Path()
 	go func(p string) {
 		if err := os.RemoveAll(p); err != nil {
 			log.WithFields(log.Fields{"path": p, "error": err}).Warn("failed to remove server files during deletion process")
 		}
-	}(s.Filesystem().Path())
+	}(p)
+
+	if err := s.Filesystem().Close(); err != nil {
+		log.WithFields(log.Fields{"server": s.ID(), "error": err}).Warn("failed to close filesystem root")
+	}
 
 	middleware.ExtractManager(c).Remove(func(server *server.Server) bool {
 		return server.ID() == s.ID()

router/router_server_files.go

@@ -30,8 +30,7 @@ import (
 // getServerFileContents returns the contents of a file on the server.
 func getServerFileContents(c *gin.Context) {
 	s := middleware.ExtractServer(c)
-	p := "/" + strings.TrimLeft(c.Query("file"), "/")
-	f, st, err := s.Filesystem().File(p)
+	f, st, err := s.Filesystem().File(c.Query("file"))
 	if err != nil {
 		middleware.CaptureAndAbort(c, err)
 		return
@@ -248,7 +247,7 @@ func postServerWriteFile(c *gin.Context) {
 		return
 	}
 
-	if err := s.Filesystem().Writefile(f, c.Request.Body); err != nil {
+	if err := s.Filesystem().Write(f, c.Request.Body, c.Request.ContentLength, 0o644); err != nil {
 		if filesystem.IsErrorCode(err, filesystem.ErrCodeIsDirectory) {
 			c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{
 				"error": "Cannot write file, name conflicts with an existing directory by the same name.",
@@ -622,7 +621,7 @@ func handleFileUpload(p string, s *server.Server, header *multipart.FileHeader)
 	if err := s.Filesystem().IsIgnored(p); err != nil {
 		return err
 	}
-	if err := s.Filesystem().Writefile(p, file); err != nil {
+	if err := s.Filesystem().Write(p, file, header.Size, 0o644); err != nil {
 		return err
 	}
 	return nil

router/router_transfer.go

@@ -106,8 +106,11 @@ func postTransfers(c *gin.Context) {
 			if !successful && err != nil {
 				// Delete all extracted files.
 				go func(trnsfr *transfer.Transfer) {
-					if err := os.RemoveAll(trnsfr.Server.Filesystem().Path()); err != nil && !os.IsNotExist(err) {
-						trnsfr.Log().WithError(err).Warn("failed to delete local server files")
+					_ = trnsfr.Server.Filesystem().Close()
+					if err := os.RemoveAll(trnsfr.Server.Filesystem().Path()); err != nil {
+						if !errors.Is(err, os.ErrNotExist) {
+							trnsfr.Log().WithError(err).Warn("failed to delete local server files")
+						}
 					}
 				}(trnsfr)
 			}

server/backup.go

@@ -154,17 +154,11 @@ func (s *Server) RestoreBackup(b backup.BackupInterface, reader io.ReadCloser) (
 	err = b.Restore(s.Context(), reader, func(file string, info fs.FileInfo, r io.ReadCloser) error {
 		defer r.Close()
 		s.Events().Publish(DaemonMessageEvent, "(restoring): "+file)
-
-		if err := s.Filesystem().Writefile(file, r); err != nil {
-			return err
-		}
-		if err := s.Filesystem().Chmod(file, info.Mode()); err != nil {
+		if err := s.Filesystem().Write(file, r, info.Size(), info.Mode()); err != nil {
 			return err
 		}
-
 		atime := info.ModTime()
-		mtime := atime
-		return s.Filesystem().Chtimes(file, atime, mtime)
+		return s.Filesystem().Chtimes(file, atime, atime)
 	})
 
 	return errors.WithStackIf(err)

server/filesystem/archive.go

@@ -3,7 +3,6 @@ package filesystem
 import (
 	"archive/tar"
 	"context"
-	"fmt"
 	"io"
 	"io/fs"
 	"os"
@@ -124,6 +123,13 @@ func (a *Archive) Progress() *progress.Progress {
 	return a.p
 }
 
+func (a *Archive) Close() error {
+	if err := a.root.Close(); err != nil {
+		return errors.Wrap(err, "server/filesystem: archive: failed to close root")
+	}
+	return nil
+}
+
 // Create .
 func (a *Archive) Create(ctx context.Context, f *os.File) error {
 	// Select a writer based off of the WriteLimit configuration option. If there is no
@@ -233,7 +239,6 @@ func (a *Archive) addToArchive(p string) error {
 		if err != nil {
 			target = ""
 		}
-		fmt.Println(p, " targeting ", target)
 	}
 
 	// Get the tar FileInfoHeader to add the file to the archive.

server/filesystem/disk_space.go

@@ -1,21 +1,24 @@
 package filesystem
 
 import (
+	fs2 "io/fs"
+	"os"
 	"path/filepath"
-	"strings"
+	"slices"
 	"sync"
 	"sync/atomic"
 	"time"
 
 	"emperror.dev/errors"
 	"github.com/apex/log"
-	"github.com/karrick/godirwalk"
+	"golang.org/x/sys/unix"
 )
 
 type SpaceCheckingOpts struct {
 	AllowStaleResponse bool
 }
 
+// TODO: can this be replaced with some sort of atomic? Like atomic.Pointer?
 type usageLookupTime struct {
 	sync.RWMutex
 	value time.Time
@@ -36,12 +39,13 @@ func (ult *usageLookupTime) Get() time.Time {
 	return ult.value
 }
 
-// Returns the maximum amount of disk space that this Filesystem instance is allowed to use.
+// MaxDisk returns the maximum amount of disk space that this Filesystem
+// instance is allowed to use.
 func (fs *Filesystem) MaxDisk() int64 {
 	return atomic.LoadInt64(&fs.diskLimit)
 }
 
-// Sets the disk space limit for this Filesystem instance.
+// SetDiskLimit sets the disk space limit for this Filesystem instance.
 func (fs *Filesystem) SetDiskLimit(i int64) {
 	atomic.SwapInt64(&fs.diskLimit, i)
 }
@@ -66,7 +70,7 @@ func (fs *Filesystem) HasSpaceErr(allowStaleValue bool) error {
 func (fs *Filesystem) HasSpaceAvailable(allowStaleValue bool) bool {
 	size, err := fs.DiskUsage(allowStaleValue)
 	if err != nil {
-		log.WithField("root", fs.root).WithField("error", err).Warn("failed to determine root fs directory size")
+		log.WithField("root", fs.Path()).WithField("error", err).Warn("failed to determine root fs directory size")
 	}
 
 	// If space is -1 or 0 just return true, means they're allowed unlimited.
@@ -115,7 +119,7 @@ func (fs *Filesystem) DiskUsage(allowStaleValue bool) (int64, error) {
 			// currently performing a lookup, just do the disk usage calculation in the background.
 			go func(fs *Filesystem) {
 				if _, err := fs.updateCachedDiskUsage(); err != nil {
-					log.WithField("root", fs.root).WithField("error", err).Warn("failed to update fs disk usage from within routine")
+					log.WithField("root", fs.rootPath).WithField("error", err).Warn("failed to update fs disk usage from within routine")
 				}
 			}(fs)
 		}
@@ -155,37 +159,55 @@ func (fs *Filesystem) updateCachedDiskUsage() (int64, error) {
 	return size, err
 }
 
-// Determines the directory size of a given location by running parallel tasks to iterate
-// through all of the folders. Returns the size in bytes. This can be a fairly taxing operation
-// on locations with tons of files, so it is recommended that you cache the output.
+// DirectorySize determines the directory size of a given location. Returns the size
+// in bytes. This can be a fairly taxing operation on locations with tons of files,
+// so it is recommended that you cache the output.
 func (fs *Filesystem) DirectorySize(dir string) (int64, error) {
-	dir = strings.TrimLeft(filepath.Clean(dir), "/")
-	if dir != "" {
+	dir = normalize(dir)
+	if dir != "." {
 		if _, err := fs.root.Lstat(dir); err != nil {
 			return 0, err
 		}
 	}
 
+	rt := fs.root
+	if dir != "." {
+		r, err := fs.root.OpenRoot(dir)
+		if err != nil {
+			return 0, errors.Wrap(err, "server/filesystem: directorysize: failed to open root directory")
+		}
+		defer r.Close()
+		rt = r
+	}
+
 	var size int64
-	err := godirwalk.Walk(filepath.Join(fs.rootPath, dir), &godirwalk.Options{
-		Unsorted:            true,
-		FollowSymbolicLinks: false,
-		Callback: func(p string, e *godirwalk.Dirent) error {
-			if !e.ModeType().IsRegular() {
+	var links []uint64
+
+	err := filepath.WalkDir(rt.Name(), func(path string, d fs2.DirEntry, err error) error {
+		if !d.Type().IsRegular() {
+			return nil
+		}
+
+		st, err := d.Info()
+		if err != nil {
+			if errors.Is(err, os.ErrNotExist) {
 				return nil
 			}
+			return err
+		}
 
-			if !e.IsDir() {
-				st, err := fs.root.Lstat(strings.TrimLeft(strings.TrimPrefix(p, fs.rootPath), "/"))
-				if err != nil {
-					return errors.Wrap(err, "server/filesystem: directorysize: failed to stat file")
-				}
-				atomic.AddInt64(&size, st.Size())
+		s := st.Sys().(*unix.Stat_t)
+		if s.Nlink > 1 {
+			// Hard links have the same inode number, don't add them more than once.
+			if slices.Contains(links, s.Ino) {
+				return nil
 			}
+			links = append(links, s.Ino)
+		}
 
-			// todo: don't count hardlinks twice
-			return nil
-		},
+		size += st.Size()
+
+		return nil
 	})
 
 	return size, errors.WrapIf(err, "server/filesystem: directorysize: failed to walk directory")

server/filesystem/filesystem.go

@@ -77,6 +77,14 @@ func (fs *Filesystem) Path() string {
 	return fs.rootPath
 }
 
+// Close closes the underlying os.Root instance for the server.
+func (fs *Filesystem) Close() error {
+	if err := fs.root.Close(); err != nil {
+		return errors.Wrap(err, "server/filesystem: failed to close root")
+	}
+	return nil
+}
+
 // File returns a reader for a file instance as well as the stat information.
 func (fs *Filesystem) File(p string) (*os.File, Stat, error) {
 	p = normalize(p)