From 5c56ddf5d6ce5c01b45938db6f18e45dceee06d0 Mon Sep 17 00:00:00 2001
From: Matthew Penner <me@matthewp.io>
Date: Wed, 10 Feb 2021 13:56:48 -0700
Subject: [PATCH] fs: only mkdir and chown if not exists

---
 server/filesystem/filesystem.go | 25 +++++++++++++------------
 server/install.go               |  8 ++++----
 2 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/server/filesystem/filesystem.go b/server/filesystem/filesystem.go
index e63125a..141240e 100644
--- a/server/filesystem/filesystem.go
+++ b/server/filesystem/filesystem.go
@@ -92,13 +92,16 @@ func (fs *Filesystem) Touch(p string, flag int) (*os.File, error) {
 	if !errors.Is(err, os.ErrNotExist) {
 		return nil, errors.Wrap(err, "server/filesystem: touch: failed to open file handle")
 	}
-	// Create the path leading up to the file we're trying to create, setting the final perms
-	// on it as we go.
-	if err := os.MkdirAll(filepath.Dir(cleaned), 0755); err != nil {
-		return nil, errors.Wrap(err, "server/filesystem: touch: failed to create directory tree")
-	}
-	if err := fs.Chown(filepath.Dir(cleaned)); err != nil {
-		return nil, err
+	// Only create and chown the directory if it doesn't exist.
+	if _, err := os.Stat(filepath.Dir(cleaned)); errors.Is(err, os.ErrNotExist) {
+		// Create the path leading up to the file we're trying to create, setting the final perms
+		// on it as we go.
+		if err := os.MkdirAll(filepath.Dir(cleaned), 0755); err != nil {
+			return nil, errors.Wrap(err, "server/filesystem: touch: failed to create directory tree")
+		}
+		if err := fs.Chown(filepath.Dir(cleaned)); err != nil {
+			return nil, err
+		}
 	}
 	o := &fileOpener{}
 	// Try to open the file now that we have created the pathing necessary for it, and then
@@ -154,8 +157,8 @@ func (fs *Filesystem) Writefile(p string, r io.Reader) error {
 		return err
 	}
 
-	// Touch the file and return the handle to it at this point. This will create the file
-	// and any necessary directories as needed.
+	// Touch the file and return the handle to it at this point. This will create the file,
+	// any necessary directories, and set the proper owner of the file.
 	file, err := fs.Touch(cleaned, os.O_RDWR|os.O_CREATE|os.O_TRUNC)
 	if err != nil {
 		return err
@@ -168,8 +171,6 @@ func (fs *Filesystem) Writefile(p string, r io.Reader) error {
 	// Adjust the disk usage to account for the old size and the new size of the file.
 	fs.addDisk(sz - currentSize)
 
-	// Finally, chown the file to ensure the permissions don't end up out-of-whack
-	// if we had just created it.
 	return fs.Chown(cleaned)
 }
 
@@ -249,7 +250,7 @@ func (fs *Filesystem) Chown(path string) error {
 	err = godirwalk.Walk(cleaned, &godirwalk.Options{
 		Unsorted: true,
 		Callback: func(p string, e *godirwalk.Dirent) error {
-			// Do not attempt to chmod a symlink. Go's os.Chown function will affect the symlink
+			// Do not attempt to chown a symlink. Go's os.Chown function will affect the symlink
 			// so if it points to a location outside the data directory the user would be able to
 			// (un)intentionally modify that files permissions.
 			if e.IsSymlink() {
diff --git a/server/install.go b/server/install.go
index fe0fcea..cfde2d7 100644
--- a/server/install.go
+++ b/server/install.go
@@ -48,7 +48,7 @@ func (s *Server) Install(sync bool) error {
 		s.Log().Info("server configured to skip running installation scripts for this egg, not executing process")
 	}
 
-	s.Log().Debug("notifying panel of server install state")
+	s.Log().WithField("was_successful", err == nil).Debug("notifying panel of server install state")
 	if serr := s.SyncInstallState(err == nil); serr != nil {
 		l := s.Log().WithField("was_successful", err == nil)
 
@@ -186,15 +186,15 @@ func (ip *InstallationProcess) Run() error {
 		return err
 	}
 
-	cid, err := ip.Execute()
+	cID, err := ip.Execute()
 	if err != nil {
-		ip.RemoveContainer()
+		_ = ip.RemoveContainer()
 		return err
 	}
 
 	// If this step fails, log a warning but don't exit out of the process. This is completely
 	// internal to the daemon's functionality, and does not affect the status of the server itself.
-	if err := ip.AfterExecute(cid); err != nil {
+	if err := ip.AfterExecute(cID); err != nil {
 		ip.Server.Log().WithField("error", err).Warn("failed to complete after-execute step of installation process")
 	}