From a98e376593f8fc01b6a53ba4f0ab09f7aba13198 Mon Sep 17 00:00:00 2001 From: Matthew Penner Date: Sun, 12 Jul 2020 12:28:38 -0600 Subject: [PATCH 01/37] Calculate disk usage even if server has 'unlimited' disk space --- server/filesystem.go | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/server/filesystem.go b/server/filesystem.go index 01a7b48..22a4ff7 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -162,17 +162,19 @@ func (fs *Filesystem) ParallelSafePath(paths []string) ([]string, error) { // Because determining the amount of space being used by a server is a taxing operation we // will load it all up into a cache and pull from that as long as the key is not expired. func (fs *Filesystem) HasSpaceAvailable() bool { - var space = fs.Server.Build.DiskSpace - - // If space is -1 or 0 just return true, means they're allowed unlimited. - if space <= 0 { - return true - } + space := fs.Server.Build.DiskSpace // If we have a match in the cache, use that value in the return. No need to perform an expensive // disk operation, even if this is an empty value. if x, exists := fs.Server.Cache.Get("disk_used"); exists { fs.Server.Resources.Disk = x.(int64) + + // This check is here to ensure that true is always returned if the server has unlimited disk space. + // See the end of this method for more information (the other `if space <= 0`). + if space <= 0 { + return true + } + return (x.(int64) / 1000.0 / 1000.0) <= space } @@ -194,6 +196,15 @@ func (fs *Filesystem) HasSpaceAvailable() bool { // been allocated. fs.Server.Resources.Disk = size + // If space is -1 or 0 just return true, means they're allowed unlimited. + // + // Technically we could skip disk space calculation because we don't need to check if the server exceeds it's limit + // but because this method caches the disk usage it would be best to calculate the disk usage and always + // return true. + if space <= 0 { + return true + } + return (size / 1000.0 / 1000.0) <= space } From b64f1897fbe9cd544fb44a62815065318fac7ae0 Mon Sep 17 00:00:00 2001 From: Matthew Penner Date: Wed, 15 Jul 2020 12:28:45 -0600 Subject: [PATCH 02/37] Add endpoint for decompressing archives --- router/router.go | 1 + router/router_server_files.go | 27 +++++++++++++++++++++++++++ server/filesystem.go | 23 +++++++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/router/router.go b/router/router.go index 12d270e..fbc6d8a 100644 --- a/router/router.go +++ b/router/router.go @@ -84,6 +84,7 @@ func Configure() *gin.Engine { files.POST("/create-directory", postServerCreateDirectory) files.POST("/delete", postServerDeleteFiles) files.POST("/compress", postServerCompressFiles) + files.POST("/decompress", postServerDecompressFiles) } backup := server.Group("/backup") diff --git a/router/router_server_files.go b/router/router_server_files.go index d9635e7..5072ef2 100644 --- a/router/router_server_files.go +++ b/router/router_server_files.go @@ -277,3 +277,30 @@ func postServerCompressFiles(c *gin.Context) { Mimetype: "application/tar+gzip", }) } + +func postServerDecompressFiles(c *gin.Context) { + s := GetServer(c.Param("server")) + + var data struct { + RootPath string `json:"root"` + File string `json:"file"` + } + + if err := c.BindJSON(&data); err != nil { + return + } + + if !s.Filesystem.HasSpaceAvailable() { + c.AbortWithStatusJSON(http.StatusConflict, gin.H{ + "error": "This server does not have enough available disk space to decompress an archive.", + }) + return + } + + if err := s.Filesystem.DecompressFile(data.RootPath, data.File); err != nil { + TrackedServerError(err, s).AbortWithServerError(c) + return + } + + c.Status(http.StatusNoContent) +} diff --git a/server/filesystem.go b/server/filesystem.go index 22a4ff7..7db9b07 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -7,6 +7,7 @@ import ( "encoding/json" "fmt" "github.com/gabriel-vasile/mimetype" + "github.com/mholt/archiver/v3" "github.com/pkg/errors" "github.com/pterodactyl/wings/config" "github.com/pterodactyl/wings/server/backup" @@ -742,3 +743,25 @@ func (fs *Filesystem) CompressFiles(dir string, paths []string) (os.FileInfo, er return a.Create(d, context.Background()) } + +// DecompressFile decompresses an archive. +func (fs *Filesystem) DecompressFile(dir, file string) error { + // Clean the directory path where the contents of archive will be extracted to. + safeBasePath, err := fs.SafePath(dir) + if err != nil { + return err + } + + // Clean the full path to the archive which will be unarchived. + safeArchivePath, err := fs.SafePath(filepath.Join(safeBasePath, file)) + if err != nil { + return err + } + + // Decompress the archive using it's extension to determine the algorithm. + if err := archiver.Unarchive(safeArchivePath, safeBasePath); err != nil { + return err + } + + return nil +} From f4c10e5a23dbb190d96f587d0515c98833986023 Mon Sep 17 00:00:00 2001 From: Matthew Penner Date: Wed, 15 Jul 2020 13:11:12 -0600 Subject: [PATCH 03/37] Add some missing error handling, fix a few typos --- server/backup/archiver.go | 17 ++++++++++------- server/filesystem.go | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/server/backup/archiver.go b/server/backup/archiver.go index e2c1ec3..5330e6f 100644 --- a/server/backup/archiver.go +++ b/server/backup/archiver.go @@ -20,9 +20,9 @@ type Archive struct { Files *IncludedFiles } -// Creates an archive at dest with all of the files definied in the included files struct. -func (a *Archive) Create(dest string, ctx context.Context) (os.FileInfo, error) { - f, err := os.OpenFile(dest, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) +// Creates an archive at dst with all of the files defined in the included files struct. +func (a *Archive) Create(dst string, ctx context.Context) (os.FileInfo, error) { + f, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) if err != nil { return nil, err } @@ -66,14 +66,17 @@ func (a *Archive) Create(dest string, ctx context.Context) (os.FileInfo, error) // Attempt to remove the archive if there is an error, report that error to // the logger if it fails. - if rerr := os.Remove(dest); rerr != nil && !os.IsNotExist(rerr) { - log.WithField("location", dest).Warn("failed to delete corrupted backup archive") + if rerr := os.Remove(dst); rerr != nil && !os.IsNotExist(rerr) { + log.WithField("location", dst).Warn("failed to delete corrupted backup archive") } return nil, err } - st, _ := f.Stat() + st, err := f.Stat() + if err != nil { + return nil, err + } return st, nil } @@ -101,7 +104,7 @@ func (a *Archive) addToArchive(p string, s *os.FileInfo, w *tar.Writer) error { a.Lock() defer a.Unlock() - if err = w.WriteHeader(header); err != nil { + if err := w.WriteHeader(header); err != nil { return err } diff --git a/server/filesystem.go b/server/filesystem.go index 7db9b07..50285cc 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -677,7 +677,7 @@ func (fs *Filesystem) GetIncludedFiles(dir string, ignored []string) (*backup.In return inc, nil } -// Compresses all of the files matching the given paths in the specififed directory. This function +// Compresses all of the files matching the given paths in the specified directory. This function // also supports passing nested paths to only compress certain files and folders when working in // a larger directory. This effectively creates a local backup, but rather than ignoring specific // files and folders, it takes an allow-list of files and folders. From ae46add8ef9b671e80bbaf8e3189bf036b5f660e Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Wed, 15 Jul 2020 19:24:13 -0700 Subject: [PATCH 04/37] Remove unnecessary logic --- server/filesystem.go | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/server/filesystem.go b/server/filesystem.go index 50285cc..d20adb0 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -744,24 +744,15 @@ func (fs *Filesystem) CompressFiles(dir string, paths []string) (os.FileInfo, er return a.Create(d, context.Background()) } -// DecompressFile decompresses an archive. -func (fs *Filesystem) DecompressFile(dir, file string) error { - // Clean the directory path where the contents of archive will be extracted to. - safeBasePath, err := fs.SafePath(dir) +// Decompress a file in a given directory by using the archiver tool to infer the file +// type and go from there. +func (fs *Filesystem) DecompressFile(dir string, file string) error { + source, err := fs.SafePath(filepath.Join(dir, file)) if err != nil { - return err + return errors.WithStack(err) } - // Clean the full path to the archive which will be unarchived. - safeArchivePath, err := fs.SafePath(filepath.Join(safeBasePath, file)) - if err != nil { - return err - } + dest := strings.TrimSuffix(source, filepath.Base(source)) - // Decompress the archive using it's extension to determine the algorithm. - if err := archiver.Unarchive(safeArchivePath, safeBasePath); err != nil { - return err - } - - return nil + return archiver.Unarchive(source, dest) } From a635cdd6b288e5fd172947f7161a92356bd13eb2 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Wed, 15 Jul 2020 21:16:08 -0700 Subject: [PATCH 05/37] Support unarching in a better fashion with zip-slip protections and size checking --- router/router_server_files.go | 10 ++- server/filesystem.go | 62 ++++++-------- server/filesystem_unarchive.go | 151 +++++++++++++++++++++++++++++++++ 3 files changed, 183 insertions(+), 40 deletions(-) create mode 100644 server/filesystem_unarchive.go diff --git a/router/router_server_files.go b/router/router_server_files.go index 5072ef2..a7db31e 100644 --- a/router/router_server_files.go +++ b/router/router_server_files.go @@ -290,9 +290,15 @@ func postServerDecompressFiles(c *gin.Context) { return } - if !s.Filesystem.HasSpaceAvailable() { + hasSpace, err := s.Filesystem.SpaceAvailableForDecompression(data.RootPath, data.File) + if err != nil { + TrackedServerError(err, s).AbortWithServerError(c) + return + } + + if !hasSpace { c.AbortWithStatusJSON(http.StatusConflict, gin.H{ - "error": "This server does not have enough available disk space to decompress an archive.", + "error": "This server does not have enough available disk space to decompress this archive.", }) return } diff --git a/server/filesystem.go b/server/filesystem.go index d20adb0..5ea2f16 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -7,7 +7,6 @@ import ( "encoding/json" "fmt" "github.com/gabriel-vasile/mimetype" - "github.com/mholt/archiver/v3" "github.com/pkg/errors" "github.com/pterodactyl/wings/config" "github.com/pterodactyl/wings/server/backup" @@ -165,34 +164,11 @@ func (fs *Filesystem) ParallelSafePath(paths []string) ([]string, error) { func (fs *Filesystem) HasSpaceAvailable() bool { space := fs.Server.Build.DiskSpace - // If we have a match in the cache, use that value in the return. No need to perform an expensive - // disk operation, even if this is an empty value. - if x, exists := fs.Server.Cache.Get("disk_used"); exists { - fs.Server.Resources.Disk = x.(int64) - - // This check is here to ensure that true is always returned if the server has unlimited disk space. - // See the end of this method for more information (the other `if space <= 0`). - if space <= 0 { - return true - } - - return (x.(int64) / 1000.0 / 1000.0) <= space - } - - // If there is no size its either because there is no data (in which case running this function - // will have effectively no impact), or there is nothing in the cache, in which case we need to - // grab the size of their data directory. This is a taxing operation, so we want to store it in - // the cache once we've gotten it. - size, err := fs.DirectorySize("/") + size, err := fs.getCachedDiskUsage() if err != nil { fs.Server.Log().WithField("error", err).Warn("failed to determine root server directory size") } - // Always cache the size, even if there is an error. We want to always return that value - // so that we don't cause an endless loop of determining the disk size if there is a temporary - // error encountered. - fs.Server.Cache.Set("disk_used", size, time.Second*60) - // Determine if their folder size, in bytes, is smaller than the amount of space they've // been allocated. fs.Server.Resources.Disk = size @@ -209,6 +185,29 @@ func (fs *Filesystem) HasSpaceAvailable() bool { return (size / 1000.0 / 1000.0) <= space } +// Internal helper function to allow other parts of the codebase to check the total used disk space +// as needed without overly taxing the system. This will prioritize the value from the cache to avoid +// excessive IO usage. We will only walk the filesystem and determine the size of the directory if there +// is no longer a cached value. +func (fs *Filesystem) getCachedDiskUsage() (int64, error) { + if x, exists := fs.Server.Cache.Get("disk_used"); exists { + return x.(int64), nil + } + + // If there is no size its either because there is no data (in which case running this function + // will have effectively no impact), or there is nothing in the cache, in which case we need to + // grab the size of their data directory. This is a taxing operation, so we want to store it in + // the cache once we've gotten it. + size, err := fs.DirectorySize("/") + + // Always cache the size, even if there is an error. We want to always return that value + // so that we don't cause an endless loop of determining the disk size if there is a temporary + // error encountered. + fs.Server.Cache.Set("disk_used", size, time.Second*60) + + 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. @@ -743,16 +742,3 @@ func (fs *Filesystem) CompressFiles(dir string, paths []string) (os.FileInfo, er return a.Create(d, context.Background()) } - -// Decompress a file in a given directory by using the archiver tool to infer the file -// type and go from there. -func (fs *Filesystem) DecompressFile(dir string, file string) error { - source, err := fs.SafePath(filepath.Join(dir, file)) - if err != nil { - return errors.WithStack(err) - } - - dest := strings.TrimSuffix(source, filepath.Base(source)) - - return archiver.Unarchive(source, dest) -} diff --git a/server/filesystem_unarchive.go b/server/filesystem_unarchive.go new file mode 100644 index 0000000..dc07a82 --- /dev/null +++ b/server/filesystem_unarchive.go @@ -0,0 +1,151 @@ +package server + +import ( + "archive/tar" + "archive/zip" + "compress/gzip" + "context" + "fmt" + "github.com/mholt/archiver/v3" + "github.com/pkg/errors" + "golang.org/x/sync/errgroup" + "io" + "os" + "path/filepath" + "reflect" + "strings" + "sync" + "sync/atomic" +) + +// Look through a given archive and determine if decompressing it would put the server over +// its allocated disk space limit. +func (fs *Filesystem) SpaceAvailableForDecompression(dir string, file string) (bool, error) { + // Don't waste time trying to determine this if we know the server will have the space for + // it since there is no limit. + if fs.Server.Build.DiskSpace <= 0 { + return true, nil + } + + source, err := fs.SafePath(filepath.Join(dir, file)) + if err != nil { + return false, err + } + + wg := new(sync.WaitGroup) + + var dirSize int64 + var cErr error + // Get the cached size in a parallel process so that if it is not cached we are not + // waiting an unnecessary amount of time on this call. + go func() { + wg.Add(1) + defer wg.Done() + + dirSize, cErr = fs.getCachedDiskUsage() + }() + + var size int64 + // In a seperate thread, walk over the archive and figure out just how large the final + // output would be from dearchiving it. + go func() { + wg.Add(1) + defer wg.Done() + + // Walk all of the files and calculate the total decompressed size of this archive. + archiver.Walk(source, func(f archiver.File) error { + atomic.AddInt64(&size, f.Size()) + + return nil + }) + }() + + wg.Wait() + + return ((dirSize + size) / 1000.0 / 1000.0) <= fs.Server.Build.DiskSpace, cErr +} + +// Decompress a file in a given directory by using the archiver tool to infer the file +// type and go from there. This will walk over all of the files within the given archive +// and ensure that there is not a zip-slip attack being attempted by validating that the +// final path is within the server data directory. +func (fs *Filesystem) DecompressFile(dir string, file string) error { + source, err := fs.SafePath(filepath.Join(dir, file)) + if err != nil { + return errors.WithStack(err) + } + + g, ctx := errgroup.WithContext(context.Background()) + + // Walk over all of the files spinning up an additional go-routine for each file we've encountered + // and then extract that file from the archive and write it to the disk. If any part of this process + // encounters an error the entire process will be stopped. + archiver.Walk(source, func(f archiver.File) error { + // Don't waste time with directories, we don't need to create them if they have no contents, and + // we will ensure the directory exists when opening the file for writing anyways. + if f.IsDir() { + return nil + } + + fi := f + g.Go(func() error { + select { + case <-ctx.Done(): + return ctx.Err() + default: + return fs.extractFileFromArchive(fi) + } + }) + + return nil + }) + + // Wait until everything is done, and then return. + if err := g.Wait(); err != nil { + return errors.WithStack(err) + } + + return nil +} + +// Extracts a single file from the archive and writes it to the disk after verifying that it will end +// up in the server data directory. +func (fs *Filesystem) extractFileFromArchive(f archiver.File) error { + var name string + + switch s := f.Sys().(type) { + case *tar.Header: + name = s.Name + case *gzip.Header: + name = s.Name + case *zip.FileHeader: + name = s.Name + default: + return errors.New(fmt.Sprintf("could not parse underlying data source with type %s", reflect.TypeOf(s).String())) + } + + // Guard against a zip-slip attack and prevent writing a file to a destination outside of + // the server root directory. + p, err := fs.SafePath(name) + if err != nil { + return err + } + + // Ensure the directory structure for this file exists before trying to write the file + // to the disk, otherwise we'll have some unexpected fun. + if err := os.MkdirAll(strings.TrimSuffix(p, filepath.Base(p)), 0755); err != nil { + return err + } + + // Open the file and truncate it if it already exists. + o, err := os.OpenFile(p, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644) + if err != nil { + return err + } + + defer o.Close() + + _, cerr := io.Copy(o, f) + + return cerr +} From b2d34cf8e742c4d559eaa01fff8f8071b2e45986 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Wed, 15 Jul 2020 21:35:40 -0700 Subject: [PATCH 06/37] Don't cause a race condition --- server/filesystem_unarchive.go | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/server/filesystem_unarchive.go b/server/filesystem_unarchive.go index dc07a82..7197554 100644 --- a/server/filesystem_unarchive.go +++ b/server/filesystem_unarchive.go @@ -4,11 +4,9 @@ import ( "archive/tar" "archive/zip" "compress/gzip" - "context" "fmt" "github.com/mholt/archiver/v3" "github.com/pkg/errors" - "golang.org/x/sync/errgroup" "io" "os" "path/filepath" @@ -75,37 +73,18 @@ func (fs *Filesystem) DecompressFile(dir string, file string) error { return errors.WithStack(err) } - g, ctx := errgroup.WithContext(context.Background()) - // Walk over all of the files spinning up an additional go-routine for each file we've encountered // and then extract that file from the archive and write it to the disk. If any part of this process // encounters an error the entire process will be stopped. - archiver.Walk(source, func(f archiver.File) error { + return archiver.Walk(source, func(f archiver.File) error { // Don't waste time with directories, we don't need to create them if they have no contents, and // we will ensure the directory exists when opening the file for writing anyways. if f.IsDir() { return nil } - fi := f - g.Go(func() error { - select { - case <-ctx.Done(): - return ctx.Err() - default: - return fs.extractFileFromArchive(fi) - } - }) - - return nil + return fs.extractFileFromArchive(f) }) - - // Wait until everything is done, and then return. - if err := g.Wait(); err != nil { - return errors.WithStack(err) - } - - return nil } // Extracts a single file from the archive and writes it to the disk after verifying that it will end From 7e1b7e7f366b23133e133b5dbba7a224190c9c3a Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Thu, 16 Jul 2020 19:56:53 -0700 Subject: [PATCH 07/37] Prevent race conditions when generating archives --- server/environment_docker.go | 12 ++++++------ server/filesystem.go | 14 ++++++++++++-- server/resources.go | 15 +++++++++++++++ 3 files changed, 33 insertions(+), 8 deletions(-) diff --git a/server/environment_docker.go b/server/environment_docker.go index 3fc30fa..6b9246b 100644 --- a/server/environment_docker.go +++ b/server/environment_docker.go @@ -21,6 +21,7 @@ import ( "path/filepath" "strconv" "strings" + "sync/atomic" "time" ) @@ -510,17 +511,19 @@ func (d *DockerEnvironment) EnableResourcePolling() error { return } + s.Resources.Lock() s.Resources.CpuAbsolute = s.Resources.CalculateAbsoluteCpu(&v.PreCPUStats, &v.CPUStats) s.Resources.Memory = s.Resources.CalculateDockerMemory(v.MemoryStats) s.Resources.MemoryLimit = v.MemoryStats.Limit + s.Resources.Unlock() // Why you ask? This already has the logic for caching disk space in use and then // also handles pushing that value to the resources object automatically. s.Filesystem.HasSpaceAvailable() for _, nw := range v.Networks { - s.Resources.Network.RxBytes += nw.RxBytes - s.Resources.Network.TxBytes += nw.TxBytes + atomic.AddUint64(&s.Resources.Network.RxBytes, nw.RxBytes) + atomic.AddUint64(&s.Resources.Network.TxBytes, nw.TxBytes) } b, _ := json.Marshal(s.Resources) @@ -539,10 +542,7 @@ func (d *DockerEnvironment) DisableResourcePolling() error { err := d.stats.Close() - d.Server.Resources.CpuAbsolute = 0 - d.Server.Resources.Memory = 0 - d.Server.Resources.Network.TxBytes = 0 - d.Server.Resources.Network.RxBytes = 0 + d.Server.Resources.Empty() return errors.WithStack(err) } diff --git a/server/filesystem.go b/server/filesystem.go index 5ea2f16..63d7986 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -29,10 +29,9 @@ import ( var InvalidPathResolution = errors.New("invalid path resolution") type Filesystem struct { - // The server object associated with this Filesystem. Server *Server - Configuration *config.SystemConfiguration + cacheDiskMu sync.Mutex } // Returns the root path that contains all of a server's data. @@ -171,7 +170,9 @@ func (fs *Filesystem) HasSpaceAvailable() bool { // Determine if their folder size, in bytes, is smaller than the amount of space they've // been allocated. + fs.Server.Resources.Lock() fs.Server.Resources.Disk = size + fs.Server.Resources.Unlock() // If space is -1 or 0 just return true, means they're allowed unlimited. // @@ -190,6 +191,15 @@ func (fs *Filesystem) HasSpaceAvailable() bool { // excessive IO usage. We will only walk the filesystem and determine the size of the directory if there // is no longer a cached value. func (fs *Filesystem) getCachedDiskUsage() (int64, error) { + // Obtain an exclusive lock on this process so that we don't unintentionally run it at the same + // time as another running process. Once the lock is available it'll read from the cache for the + // second call rather than hitting the disk in parallel. + // + // This effectively the same speed as running this call in parallel since this cache will return + // instantly on the second call. + fs.cacheDiskMu.Lock() + defer fs.cacheDiskMu.Unlock() + if x, exists := fs.Server.Cache.Get("disk_used"); exists { return x.(int64), nil } diff --git a/server/resources.go b/server/resources.go index 55f5cd1..b42258f 100644 --- a/server/resources.go +++ b/server/resources.go @@ -3,12 +3,15 @@ package server import ( "github.com/docker/docker/api/types" "math" + "sync" ) // Defines the current resource usage for a given server instance. If a server is offline you // should obviously expect memory and CPU usage to be 0. However, disk will always be returned // since that is not dependent on the server being running to collect that data. type ResourceUsage struct { + sync.RWMutex + // The total amount of memory, in bytes, that this server instance is consuming. This is // calculated slightly differently than just using the raw Memory field that the stats // return from the container, so please check the code setting this value for how that @@ -31,6 +34,18 @@ type ResourceUsage struct { } `json:"network"` } +// Resets the usages values to zero, used when a server is stopped to ensure we don't hold +// onto any values incorrectly. +func (ru *ResourceUsage) Empty() { + ru.Lock() + defer ru.Unlock() + + ru.Memory = 0 + ru.CpuAbsolute = 0 + ru.Network.TxBytes = 0 + ru.Network.RxBytes = 0 +} + // The "docker stats" CLI call does not return the same value as the types.MemoryStats.Usage // value which can be rather confusing to people trying to compare panel usage to // their stats output. From f3c8220bd9bad82c3a9626406f4ea1bcf557ed12 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Thu, 16 Jul 2020 21:51:31 -0700 Subject: [PATCH 08/37] Change filewalker implementation to use a pool --- go.mod | 1 + go.sum | 4 ++ router/error.go | 2 +- server/filesystem.go | 65 ++++++++------------- server/filesystem_walker.go | 111 +++++++++++++++++++++++------------- 5 files changed, 102 insertions(+), 81 deletions(-) diff --git a/go.mod b/go.mod index 8f61d89..0de71c2 100644 --- a/go.mod +++ b/go.mod @@ -31,6 +31,7 @@ require ( github.com/docker/go-units v0.3.3 // indirect github.com/fatih/color v1.9.0 github.com/gabriel-vasile/mimetype v0.1.4 + github.com/gammazero/workerpool v0.0.0-20200608033439-1a5ca90a5753 github.com/gbrlsnchs/jwt/v3 v3.0.0-rc.0 github.com/gin-gonic/gin v1.6.3 github.com/golang/protobuf v1.3.5 // indirect diff --git a/go.sum b/go.sum index 889a55f..2ac6433 100644 --- a/go.sum +++ b/go.sum @@ -79,6 +79,10 @@ github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/gabriel-vasile/mimetype v0.1.4 h1:5mcsq3+DXypREUkW+1juhjeKmE/XnWgs+paHMJn7lf8= github.com/gabriel-vasile/mimetype v0.1.4/go.mod h1:kMJbg3SlWZCsj4R73F1WDzbT9AyGCOVmUtIxxwO5pmI= +github.com/gammazero/deque v0.0.0-20200227231300-1e9af0e52b46 h1:iX4+rD9Fjdx8SkmSO/O5WAIX/j79ll3kuqv5VdYt9J8= +github.com/gammazero/deque v0.0.0-20200227231300-1e9af0e52b46/go.mod h1:D90+MBHVc9Sk1lJAbEVgws0eYEurY4mv2TDso3Nxh3w= +github.com/gammazero/workerpool v0.0.0-20200608033439-1a5ca90a5753 h1:oSQ61LxZkz3Z4La0O5cbyVDvLWEfbNgiD43cSPdjPQQ= +github.com/gammazero/workerpool v0.0.0-20200608033439-1a5ca90a5753/go.mod h1:/XWO2YAUUpPi3smDlFBl0vpX0JHwUomDM/oRMwRmnSs= github.com/gbrlsnchs/jwt/v3 v3.0.0-rc.0 h1:7KeiSrO5puFH1+vdAdbpiie2TrNnkvFc/eOQzT60Z2k= github.com/gbrlsnchs/jwt/v3 v3.0.0-rc.0/go.mod h1:D1+3UtCYAJ1os1PI+zhTVEj6Tb+IHJvXjXKz83OstmM= github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk= diff --git a/router/error.go b/router/error.go index 5755b7f..0ad58b8 100644 --- a/router/error.go +++ b/router/error.go @@ -33,7 +33,7 @@ func TrackedError(err error) *RequestError { // generated this server for the purposes of logging. func TrackedServerError(err error, s *server.Server) *RequestError { return &RequestError{ - Err: err, + Err: errors.WithStack(err), Uuid: uuid.Must(uuid.NewRandom()).String(), Message: "", server: s, diff --git a/server/filesystem.go b/server/filesystem.go index 63d7986..0e8c8b0 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -29,9 +29,9 @@ import ( var InvalidPathResolution = errors.New("invalid path resolution") type Filesystem struct { - Server *Server + Server *Server Configuration *config.SystemConfiguration - cacheDiskMu sync.Mutex + cacheDiskMu sync.Mutex } // Returns the root path that contains all of a server's data. @@ -222,17 +222,13 @@ func (fs *Filesystem) getCachedDiskUsage() (int64, error) { // 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. func (fs *Filesystem) DirectorySize(dir string) (int64, error) { - w := fs.NewWalker() - ctx := context.Background() - var size int64 - err := w.Walk(dir, ctx, func(f os.FileInfo, _ string) bool { - // Only increment the size when we're dealing with a file specifically, otherwise - // just continue digging deeper until there are no more directories to iterate over. + err := fs.Walk(dir, func(_ string, f os.FileInfo, err error) error { if !f.IsDir() { atomic.AddInt64(&size, f.Size()) } - return true + + return nil }) return size, err @@ -657,9 +653,6 @@ func (fs *Filesystem) GetIncludedFiles(dir string, ignored []string) (*backup.In return nil, err } - w := fs.NewWalker() - ctx := context.Background() - i, err := ignore.CompileIgnoreLines(ignored...) if err != nil { return nil, err @@ -668,7 +661,8 @@ func (fs *Filesystem) GetIncludedFiles(dir string, ignored []string) (*backup.In // Walk through all of the files and directories on a server. This callback only returns // files found, and will keep walking deeper and deeper into directories. inc := new(backup.IncludedFiles) - if err := w.Walk(cleaned, ctx, func(f os.FileInfo, p string) bool { + + if err := fs.Walk(cleaned, func(p string, f os.FileInfo, err error) error { // Avoid unnecessary parsing if there are no ignored files, nothing will match anyways // so no reason to call the function. if len(ignored) == 0 || !i.MatchesPath(strings.TrimPrefix(p, fs.Path()+"/")) { @@ -678,7 +672,7 @@ func (fs *Filesystem) GetIncludedFiles(dir string, ignored []string) (*backup.In // We can't just abort if the path is technically ignored. It is possible there is a nested // file or folder that should not be excluded, so in this case we need to just keep going // until we get to a final state. - return true + return nil }); err != nil { return nil, err } @@ -709,43 +703,34 @@ func (fs *Filesystem) CompressFiles(dir string, paths []string) (os.FileInfo, er return nil, err } - w := fs.NewWalker() - wg := new(sync.WaitGroup) - inc := new(backup.IncludedFiles) // Iterate over all of the cleaned paths and merge them into a large object of final file // paths to pass into the archiver. As directories are encountered this will drop into them // and look for all of the files. for _, p := range cleaned { - wg.Add(1) + f, err := os.Stat(p) + if err != nil { + fs.Server.Log().WithField("error", err).WithField("path", p).Debug("failed to stat file or directory for compression") + continue + } - go func(pa string) { - defer wg.Done() + if f.IsDir() { + err := fs.Walk(p, func(s string, info os.FileInfo, err error) error { + if !info.IsDir() { + inc.Push(&info, s) + } + + return nil + }) - f, err := os.Stat(pa) if err != nil { - fs.Server.Log().WithField("error", err).WithField("path", pa).Warn("failed to stat file or directory for compression") - return + return nil, err } - - if f.IsDir() { - // Recursively drop into directory and get all of the additional files and directories within - // it that should be included in this backup. - w.Walk(pa, context.Background(), func(info os.FileInfo, s string) bool { - if !info.IsDir() { - inc.Push(&info, s) - } - - return true - }) - } else { - inc.Push(&f, pa) - } - }(p) + } else { + inc.Push(&f, p) + } } - wg.Wait() - a := &backup.Archive{TrimPrefix: fs.Path(), Files: inc} d := path.Join(cleanedRootDir, fmt.Sprintf("archive-%s.tar.gz", strings.ReplaceAll(time.Now().Format(time.RFC3339), ":", ""))) diff --git a/server/filesystem_walker.go b/server/filesystem_walker.go index 0db761a..bb3bf6b 100644 --- a/server/filesystem_walker.go +++ b/server/filesystem_walker.go @@ -1,70 +1,101 @@ package server import ( - "context" - "golang.org/x/sync/errgroup" + "github.com/gammazero/workerpool" "io/ioutil" "os" "path/filepath" + "runtime" + "sync" ) type FileWalker struct { *Filesystem } +type PooledFileWalker struct { + wg sync.WaitGroup + pool *workerpool.WorkerPool + callback filepath.WalkFunc + + Filesystem *Filesystem +} + // Returns a new walker instance. func (fs *Filesystem) NewWalker() *FileWalker { return &FileWalker{fs} } -// Iterate over all of the files and directories within a given directory. When a file is -// found the callback will be called with the file information. If a directory is encountered -// it will be recursively passed back through to this function. -func (fw *FileWalker) Walk(dir string, ctx context.Context, callback func (os.FileInfo, string) bool) error { - cleaned, err := fw.SafePath(dir) +// Creates a new pooled file walker that will concurrently walk over a given directory but limit itself +// to a worker pool as to not completely flood out the system or cause a process crash. +func newPooledWalker(fs *Filesystem) *PooledFileWalker { + return &PooledFileWalker{ + Filesystem: fs, + // Create a worker pool that is the same size as the number of processors available on the + // system. Going much higher doesn't provide much of a performance boost, and is only more + // likely to lead to resource overloading anyways. + pool: workerpool.New(runtime.GOMAXPROCS(0)), + } +} + +// Process a given path by calling the callback function for all of the files and directories within +// the path, and then dropping into any directories that we come across. +func (w *PooledFileWalker) process(path string) error { + defer w.wg.Done() + + p, err := w.Filesystem.SafePath(path) if err != nil { return err } - // Get all of the files from this directory. - files, err := ioutil.ReadDir(cleaned) + files, err := ioutil.ReadDir(p) if err != nil { return err } - // Create an error group that we can use to run processes in parallel while retaining - // the ability to cancel the entire process immediately should any of it fail. - g, ctx := errgroup.WithContext(ctx) - + // Loop over all of the files and directories in the given directory and call the provided + // callback function. If we encounter a directory, push that directory onto the worker queue + // to be processed. for _, f := range files { - if f.IsDir() { - fi := f - p := filepath.Join(cleaned, f.Name()) - // Recursively call this function to continue digging through the directory tree within - // a seperate goroutine. If the context is canceled abort this process. - g.Go(func() error { - select { - case <-ctx.Done(): - return ctx.Err() - default: - // If the callback returns true, go ahead and keep walking deeper. This allows - // us to programatically continue deeper into directories, or stop digging - // if that pathway knows it needs nothing else. - if callback(fi, p) { - return fw.Walk(p, ctx, callback) - } + sp := filepath.Join(p, f.Name()) + i, err := os.Stat(sp) - return nil - } - }) - } else { - // If this isn't a directory, go ahead and pass the file information into the - // callback. We don't care about the response since we won't be stepping into - // anything from here. - callback(f, filepath.Join(cleaned, f.Name())) + if err = w.callback(sp, i, err); err != nil { + if err == filepath.SkipDir { + return nil + } + + return err + } + + if i.IsDir() { + w.push(sp) } } - // Block until all of the routines finish and have returned a value. - return g.Wait() -} \ No newline at end of file + return nil +} + +// Push a new path into the worker pool. +// +// @todo probably helps to handle errors. +func (w *PooledFileWalker) push(path string) { + w.wg.Add(1) + w.pool.Submit(func() { + w.process(path) + }) +} + +// Walks the given directory and executes the callback function for all of the files and directories +// that are encountered. +func (fs *Filesystem) Walk(dir string, callback filepath.WalkFunc) error { + w := newPooledWalker(fs) + w.callback = callback + + w.push(dir) + + w.wg.Wait() + w.pool.StopWait() + + return nil +} From d262c12b43d41eb1ba825509bdd54baee6be83f3 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Thu, 16 Jul 2020 21:53:05 -0700 Subject: [PATCH 09/37] Less confusing waitgroup positioning --- server/filesystem_walker.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/filesystem_walker.go b/server/filesystem_walker.go index bb3bf6b..0c51c07 100644 --- a/server/filesystem_walker.go +++ b/server/filesystem_walker.go @@ -41,8 +41,6 @@ func newPooledWalker(fs *Filesystem) *PooledFileWalker { // Process a given path by calling the callback function for all of the files and directories within // the path, and then dropping into any directories that we come across. func (w *PooledFileWalker) process(path string) error { - defer w.wg.Done() - p, err := w.Filesystem.SafePath(path) if err != nil { return err @@ -82,6 +80,7 @@ func (w *PooledFileWalker) process(path string) error { func (w *PooledFileWalker) push(path string) { w.wg.Add(1) w.pool.Submit(func() { + defer w.wg.Done() w.process(path) }) } From a72d6f37680a5fcbefd08dd5dff33ee40aedb36e Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Thu, 16 Jul 2020 22:01:50 -0700 Subject: [PATCH 10/37] Make the walk processor act the same as Go's walker --- server/filesystem_walker.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/server/filesystem_walker.go b/server/filesystem_walker.go index 0c51c07..2838702 100644 --- a/server/filesystem_walker.go +++ b/server/filesystem_walker.go @@ -58,25 +58,29 @@ func (w *PooledFileWalker) process(path string) error { sp := filepath.Join(p, f.Name()) i, err := os.Stat(sp) - if err = w.callback(sp, i, err); err != nil { - if err == filepath.SkipDir { - return nil - } - + // Call the user-provided callback for this file or directory. If an error is returned that is + // not a SkipDir call, abort the entire process and bubble that error up. + if err = w.callback(sp, i, err); err != nil && err != filepath.SkipDir { return err } - if i.IsDir() { + // If this is a directory, and we didn't get a SkipDir error, continue through by pushing another + // job to the pool to handle it. If we requested a skip, don't do anything just continue on to the + // next item. + if i.IsDir() && err != filepath.SkipDir { w.push(sp) + } else if !i.IsDir() && err == filepath.SkipDir { + // Per the spec for the callback, if we get a SkipDir error but it is returned for an item + // that is _not_ a directory, abort the remaining operations on the directory. + return nil } } return nil } -// Push a new path into the worker pool. -// -// @todo probably helps to handle errors. +// Push a new path into the worker pool and increment the waitgroup so that we do not return too +// early and cause panic's as internal directories attempt to submit to the pool. func (w *PooledFileWalker) push(path string) { w.wg.Add(1) w.pool.Submit(func() { From daf682b9912871a87e46acf45c3ac29025c9c8b4 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Thu, 16 Jul 2020 22:18:47 -0700 Subject: [PATCH 11/37] Handle errors and cancel process when encountered --- server/filesystem_walker.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/server/filesystem_walker.go b/server/filesystem_walker.go index 2838702..a967d6c 100644 --- a/server/filesystem_walker.go +++ b/server/filesystem_walker.go @@ -1,6 +1,7 @@ package server import ( + "context" "github.com/gammazero/workerpool" "io/ioutil" "os" @@ -17,6 +18,10 @@ type PooledFileWalker struct { wg sync.WaitGroup pool *workerpool.WorkerPool callback filepath.WalkFunc + cancel context.CancelFunc + + err error + errOnce sync.Once Filesystem *Filesystem } @@ -85,7 +90,14 @@ func (w *PooledFileWalker) push(path string) { w.wg.Add(1) w.pool.Submit(func() { defer w.wg.Done() - w.process(path) + if err := w.process(path); err != nil { + w.errOnce.Do(func() { + w.err = err + if w.cancel != nil { + w.cancel() + } + }) + } }) } @@ -95,10 +107,17 @@ func (fs *Filesystem) Walk(dir string, callback filepath.WalkFunc) error { w := newPooledWalker(fs) w.callback = callback + _, cancel := context.WithCancel(context.Background()) + w.cancel = cancel + w.push(dir) w.wg.Wait() w.pool.StopWait() + if w.err != nil { + return w.err + } + return nil } From 21303dc517d7b610c3b978340e4aedfeaf256947 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Jul 2020 10:10:34 -0700 Subject: [PATCH 12/37] Address race conditions when booting a server process --- router/websocket/websocket.go | 6 ++++++ server/update.go | 3 +++ 2 files changed, 9 insertions(+) diff --git a/router/websocket/websocket.go b/router/websocket/websocket.go index 7a0507d..dbd9bf8 100644 --- a/router/websocket/websocket.go +++ b/router/websocket/websocket.go @@ -127,6 +127,9 @@ func (h *Handler) TokenValid() error { return errors.New("jwt does not have connect permission") } + h.server.RLock() + defer h.server.RUnlock() + if h.server.Uuid != j.ServerUUID { return errors.New("jwt server uuid mismatch") } @@ -247,6 +250,9 @@ func (h *Handler) HandleInbound(m Message) error { if state == server.ProcessOfflineState { _ = h.server.Filesystem.HasSpaceAvailable() + h.server.Resources.RLock() + defer h.server.Resources.RUnlock() + resources := server.ResourceUsage{ Memory: 0, MemoryLimit: 0, diff --git a/server/update.go b/server/update.go index 7e4a643..465df57 100644 --- a/server/update.go +++ b/server/update.go @@ -33,6 +33,9 @@ func (s *Server) UpdateDataStructure(data []byte, background bool) error { return errors.WithStack(err) } + s.Lock() + defer s.Unlock() + // Don't explode if we're setting CPU limits to 0. Mergo sees that as an empty value // so it won't override the value we've passed through in the API call. However, we can // safely assume that we're passing through valid data structures here. I foresee this From 115131575d2c64bd2085b89841d09dd9e1a025a7 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Jul 2020 10:16:40 -0700 Subject: [PATCH 13/37] Return a 404 when the directory does not exist --- router/router_server_files.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/router/router_server_files.go b/router/router_server_files.go index a7db31e..4559e0c 100644 --- a/router/router_server_files.go +++ b/router/router_server_files.go @@ -78,6 +78,13 @@ func getServerListDirectory(c *gin.Context) { stats, err := s.Filesystem.ListDirectory(d) if err != nil { + if err.Error() == "readdirent: not a directory" { + c.AbortWithStatusJSON(http.StatusNotFound, gin.H{ + "error": "The requested directory does not exist.", + }) + return + } + TrackedServerError(err, s).AbortWithServerError(c) return } From 1b5684e6f8fa1086272e79d41375e6e2167acf35 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Jul 2020 10:47:15 -0700 Subject: [PATCH 14/37] Make sure errors are handled --- server/filesystem.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/server/filesystem.go b/server/filesystem.go index 0e8c8b0..0daea8a 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -224,6 +224,10 @@ func (fs *Filesystem) getCachedDiskUsage() (int64, error) { func (fs *Filesystem) DirectorySize(dir string) (int64, error) { var size int64 err := fs.Walk(dir, func(_ string, f os.FileInfo, err error) error { + if err != nil { + return err + } + if !f.IsDir() { atomic.AddInt64(&size, f.Size()) } @@ -663,6 +667,10 @@ func (fs *Filesystem) GetIncludedFiles(dir string, ignored []string) (*backup.In inc := new(backup.IncludedFiles) if err := fs.Walk(cleaned, func(p string, f os.FileInfo, err error) error { + if err != nil { + return err + } + // Avoid unnecessary parsing if there are no ignored files, nothing will match anyways // so no reason to call the function. if len(ignored) == 0 || !i.MatchesPath(strings.TrimPrefix(p, fs.Path()+"/")) { @@ -716,6 +724,10 @@ func (fs *Filesystem) CompressFiles(dir string, paths []string) (os.FileInfo, er if f.IsDir() { err := fs.Walk(p, func(s string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if !info.IsDir() { inc.Push(&info, s) } From 7c3da842484e4bff9a298059618722f63e4eeed0 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Jul 2020 10:49:28 -0700 Subject: [PATCH 15/37] chown the cleaned location, not the original path --- server/filesystem.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/filesystem.go b/server/filesystem.go index 0daea8a..e6dc4b9 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -314,7 +314,7 @@ func (fs *Filesystem) Writefile(p string, r io.Reader) error { // Finally, chown the file to ensure the permissions don't end up out-of-whack // if we had just created it. - return fs.Chown(p) + return fs.Chown(cleaned) } // Defines the stat struct object. From 6e1844a8c9296bbc584c4d823312470c6903088c Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Jul 2020 10:54:37 -0700 Subject: [PATCH 16/37] Skip over when not exist --- server/filesystem_walker.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/server/filesystem_walker.go b/server/filesystem_walker.go index a967d6c..fc56598 100644 --- a/server/filesystem_walker.go +++ b/server/filesystem_walker.go @@ -63,6 +63,12 @@ func (w *PooledFileWalker) process(path string) error { sp := filepath.Join(p, f.Name()) i, err := os.Stat(sp) + // You might end up getting an error about a file or folder not existing if the given path + // if it is an invalid symlink. We can safely just skip over these files I believe. + if os.IsNotExist(err) { + continue + } + // Call the user-provided callback for this file or directory. If an error is returned that is // not a SkipDir call, abort the entire process and bubble that error up. if err = w.callback(sp, i, err); err != nil && err != filepath.SkipDir { From 4f1b0c67d67e8bf557bcebfe085a266cc73ffa25 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Jul 2020 11:40:38 -0700 Subject: [PATCH 17/37] Address security vulnerabilities allowing certain internal processes to potentiallty escape server data directory --- server/archiver.go | 7 +++++- server/filesystem.go | 47 +++++++++++++++++++++++++++++++++---- server/filesystem_walker.go | 7 ++++-- 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/server/archiver.go b/server/archiver.go index 5c12976..d10dfce 100644 --- a/server/archiver.go +++ b/server/archiver.go @@ -52,7 +52,12 @@ func (a *Archiver) Archive() error { } for _, file := range fileInfo { - files = append(files, filepath.Join(path, file.Name())) + f, err := a.Server.Filesystem.SafeJoin(path, file) + if err != nil { + return err + } + + files = append(files, f) } stat, err := a.Stat() diff --git a/server/filesystem.go b/server/filesystem.go index e6dc4b9..08b4214 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -107,6 +107,27 @@ func (fs *Filesystem) SafePath(p string) (string, error) { return "", InvalidPathResolution } +// Helper function to keep some of the codebase a little cleaner. Returns a "safe" version of the path +// joined with a file. This is important because you cannot just assume that appending a file to a cleaned +// path will result in a cleaned path to that file. For example, imagine you have the following scenario: +// +// my_bad_file -> symlink:/etc/passwd +// +// cleaned := SafePath("../../etc") -> "/" +// filepath.Join(cleaned, my_bad_file) -> "/my_bad_file" +// +// You might think that "/my_bad_file" is fine since it isn't pointing to the original "../../etc/my_bad_file". +// However, this doesn't account for symlinks where the file might be pointing outside of the directory, so +// calling a function such as Chown against it would chown the symlinked location, and not the file within the +// Wings daemon. +func (fs *Filesystem) SafeJoin(dir string, f os.FileInfo) (string, error) { + if f.Mode()&os.ModeSymlink != 0 { + return fs.SafePath(filepath.Join(dir, f.Name())) + } + + return filepath.Join(dir, f.Name()), nil +} + // Executes the fs.SafePath function in parallel against an array of paths. If any of the calls // fails an error will be returned. func (fs *Filesystem) ParallelSafePath(paths []string) ([]string, error) { @@ -456,16 +477,27 @@ func (fs *Filesystem) chownDirectory(path string) error { } for _, f := range files { + // Do not attempt to chmod 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 f.Mode()&os.ModeSymlink != 0 { + continue + } + + p, err := fs.SafeJoin(cleaned, f) + if err != nil { + return err + } + if f.IsDir() { wg.Add(1) go func(p string) { defer wg.Done() fs.chownDirectory(p) - }(filepath.Join(cleaned, f.Name())) + }(p) } else { - // Chown the file. - os.Chown(filepath.Join(cleaned, f.Name()), fs.Configuration.User.Uid, fs.Configuration.User.Gid) + os.Chown(p, fs.Configuration.User.Uid, fs.Configuration.User.Gid) } } @@ -600,7 +632,14 @@ func (fs *Filesystem) ListDirectory(p string) ([]*Stat, error) { var m = "inode/directory" if !f.IsDir() { - m, _, _ = mimetype.DetectFile(filepath.Join(cleaned, f.Name())) + cleanedp, _ := fs.SafeJoin(cleaned, f) + if cleanedp != "" { + m, _, _ = mimetype.DetectFile(filepath.Join(cleaned, f.Name())) + } else { + // Just pass this for an unknown type because the file could not safely be resolved within + // the server data path. + m = "application/octet-stream" + } } out[idx] = &Stat{ diff --git a/server/filesystem_walker.go b/server/filesystem_walker.go index fc56598..4043013 100644 --- a/server/filesystem_walker.go +++ b/server/filesystem_walker.go @@ -60,9 +60,12 @@ func (w *PooledFileWalker) process(path string) error { // callback function. If we encounter a directory, push that directory onto the worker queue // to be processed. for _, f := range files { - sp := filepath.Join(p, f.Name()) - i, err := os.Stat(sp) + sp, err := w.Filesystem.SafeJoin(p, f) + if err != nil { + return err + } + i, err := os.Stat(sp) // You might end up getting an error about a file or folder not existing if the given path // if it is an invalid symlink. We can safely just skip over these files I believe. if os.IsNotExist(err) { From 085a02726b33d8238210c94e533069c23735c05f Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Jul 2020 11:57:50 -0700 Subject: [PATCH 18/37] Handle path resolution errors better in the file walker --- server/filesystem.go | 43 +++++++++++++++++++++++++++++++------ server/filesystem_walker.go | 11 +++++++++- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/server/filesystem.go b/server/filesystem.go index 08b4214..b8d1e64 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -26,7 +26,18 @@ import ( ) // Error returned when there is a bad path provided to one of the FS calls. -var InvalidPathResolution = errors.New("invalid path resolution") +type PathResolutionError struct{} + +// Returns the error response in a string form that can be more easily consumed. +func (pre PathResolutionError) Error() string { + return "invalid path resolution" +} + +func IsPathResolutionError(err error) bool { + _, ok := err.(PathResolutionError) + + return ok +} type Filesystem struct { Server *Server @@ -87,7 +98,7 @@ func (fs *Filesystem) SafePath(p string) (string, error) { // attempt going on, and we should NOT resolve this path for them. if nonExistentPathResolution != "" { if !strings.HasPrefix(nonExistentPathResolution, fs.Path()) { - return "", InvalidPathResolution + return "", PathResolutionError{} } // If the nonExistentPathResolution variable is not empty then the initial path requested @@ -104,7 +115,7 @@ func (fs *Filesystem) SafePath(p string) (string, error) { return p, nil } - return "", InvalidPathResolution + return "", PathResolutionError{} } // Helper function to keep some of the codebase a little cleaner. Returns a "safe" version of the path @@ -246,7 +257,7 @@ func (fs *Filesystem) DirectorySize(dir string) (int64, error) { var size int64 err := fs.Walk(dir, func(_ string, f os.FileInfo, err error) error { if err != nil { - return err + return fs.handleWalkerError(err, f) } if !f.IsDir() { @@ -707,7 +718,7 @@ func (fs *Filesystem) GetIncludedFiles(dir string, ignored []string) (*backup.In if err := fs.Walk(cleaned, func(p string, f os.FileInfo, err error) error { if err != nil { - return err + return fs.handleWalkerError(err, f) } // Avoid unnecessary parsing if there are no ignored files, nothing will match anyways @@ -764,7 +775,7 @@ func (fs *Filesystem) CompressFiles(dir string, paths []string) (os.FileInfo, er if f.IsDir() { err := fs.Walk(p, func(s string, info os.FileInfo, err error) error { if err != nil { - return err + return fs.handleWalkerError(err, info) } if !info.IsDir() { @@ -788,3 +799,23 @@ func (fs *Filesystem) CompressFiles(dir string, paths []string) (os.FileInfo, er return a.Create(d, context.Background()) } + +// Handle errors encountered when walking through directories. +// +// If there is a path resolution error just skip the item entirely. Only return this for a +// directory, otherwise return nil. Returning this error for a file will stop the walking +// for the remainder of the directory. This is assuming an os.FileInfo struct was even returned. +func (fs *Filesystem) handleWalkerError(err error, f os.FileInfo) error { + fmt.Println("handling walker error") + if !IsPathResolutionError(err) { + fmt.Println("not a path res error") + fmt.Println(err) + return err + } + + if f != nil && f.IsDir() { + return filepath.SkipDir + } + + return nil +} diff --git a/server/filesystem_walker.go b/server/filesystem_walker.go index 4043013..7594ffe 100644 --- a/server/filesystem_walker.go +++ b/server/filesystem_walker.go @@ -62,7 +62,16 @@ func (w *PooledFileWalker) process(path string) error { for _, f := range files { sp, err := w.Filesystem.SafeJoin(p, f) if err != nil { - return err + // Let the callback function handle what to do if there is a path resolution error because a + // dangerous path was resolved. If there is an error returned, return from this entire process + // otherwise just skip over this specific file. We don't care if its a file or a directory at + // this point since either way we're skipping it, however, still check for the SkipDir since that + // would be thrown otherwise. + if err = w.callback(sp, f, err); err != nil && err != filepath.SkipDir { + return err + } + + continue } i, err := os.Stat(sp) From f0eeaae747ee2c883f21f143ca49b67f9a7b533f Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Jul 2020 11:58:55 -0700 Subject: [PATCH 19/37] Remove debugging --- server/filesystem.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/server/filesystem.go b/server/filesystem.go index b8d1e64..6414a91 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -806,10 +806,7 @@ func (fs *Filesystem) CompressFiles(dir string, paths []string) (os.FileInfo, er // directory, otherwise return nil. Returning this error for a file will stop the walking // for the remainder of the directory. This is assuming an os.FileInfo struct was even returned. func (fs *Filesystem) handleWalkerError(err error, f os.FileInfo) error { - fmt.Println("handling walker error") if !IsPathResolutionError(err) { - fmt.Println("not a path res error") - fmt.Println(err) return err } From 0b9d923d15f25ba1c234bd5de2480d63d2db31a1 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Jul 2020 12:19:54 -0700 Subject: [PATCH 20/37] Allow the deletion of a file or directory that is a symlink pointing outside the data dir --- server/filesystem.go | 52 ++++++++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/server/filesystem.go b/server/filesystem.go index 6414a91..545fe87 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -59,12 +59,8 @@ func (fs *Filesystem) Path() string { func (fs *Filesystem) SafePath(p string) (string, error) { var nonExistentPathResolution string - // Calling filpath.Clean on the joined directory will resolve it to the absolute path, - // removing any ../ type of resolution arguments, and leaving us with a direct path link. - // - // This will also trim the existing root path off the beginning of the path passed to - // the function since that can get a bit messy. - r := filepath.Clean(filepath.Join(fs.Path(), strings.TrimPrefix(p, fs.Path()))) + // Start with a cleaned up path before checking the more complex bits. + r := fs.unsafeFilePath(p) // At the same time, evaluate the symlink status and determine where this file or folder // is truly pointing to. @@ -82,7 +78,7 @@ func (fs *Filesystem) SafePath(p string) (string, error) { for k := range parts { try = strings.Join(parts[:(len(parts)-k)], "/") - if !strings.HasPrefix(try, fs.Path()) { + if !fs.unsafeIsInDataDirectory(try) { break } @@ -97,7 +93,7 @@ func (fs *Filesystem) SafePath(p string) (string, error) { // If the new path doesn't start with their root directory there is clearly an escape // attempt going on, and we should NOT resolve this path for them. if nonExistentPathResolution != "" { - if !strings.HasPrefix(nonExistentPathResolution, fs.Path()) { + if !fs.unsafeIsInDataDirectory(nonExistentPathResolution) { return "", PathResolutionError{} } @@ -111,13 +107,32 @@ func (fs *Filesystem) SafePath(p string) (string, error) { // If the requested directory from EvalSymlinks begins with the server root directory go // ahead and return it. If not we'll return an error which will block any further action // on the file. - if strings.HasPrefix(p, fs.Path()) { + if fs.unsafeIsInDataDirectory(p) { return p, nil } return "", PathResolutionError{} } +// Generate a path to the file by cleaning it up and appending the root server path to it. This +// DOES NOT gaurantee that the file resolves within the server data directory. You'll want to use +// the fs.unsafeIsInDataDirectory(p) function to confirm. +func (fs *Filesystem) unsafeFilePath(p string) string { + // Calling filpath.Clean on the joined directory will resolve it to the absolute path, + // removing any ../ type of resolution arguments, and leaving us with a direct path link. + // + // This will also trim the existing root path off the beginning of the path passed to + // the function since that can get a bit messy. + return filepath.Clean(filepath.Join(fs.Path(), strings.TrimPrefix(p, fs.Path()))) +} + +// Check that that path string starts with the server data directory path. This function DOES NOT +// validate that the rest of the path does not end up resolving out of this directory, or that the +// targeted file or folder is not a symlink doing the same thing. +func (fs *Filesystem) unsafeIsInDataDirectory(p string) bool { + return strings.HasPrefix(strings.TrimSuffix(p, "/")+"/", strings.TrimSuffix(fs.Path(), "/")+"/") +} + // Helper function to keep some of the codebase a little cleaner. Returns a "safe" version of the path // joined with a file. This is important because you cannot just assume that appending a file to a cleaned // path will result in a cleaned path to that file. For example, imagine you have the following scenario: @@ -600,17 +615,26 @@ func (fs *Filesystem) Copy(p string) error { // Deletes a file or folder from the system. Prevents the user from accidentally // (or maliciously) removing their root server data directory. func (fs *Filesystem) Delete(p string) error { - cleaned, err := fs.SafePath(p) - if err != nil { - return errors.WithStack(err) + // This is one of the few (only?) places in the codebase where we're explictly not using + // the SafePath functionality when working with user provided input. If we did, you would + // not be able to delete a file that is a symlink pointing to a location outside of the data + // directory. + // + // We also want to avoid resolving a symlink that points _within_ the data directory and thus + // deleting the actual source file for the symlink rather than the symlink itself. For these + // purposes just resolve the actual file path using filepath.Join() and confirm that the path + // exists within the data directory. + resolved := fs.unsafeFilePath(p) + if !fs.unsafeIsInDataDirectory(resolved) { + return PathResolutionError{} } // Block any whoopsies. - if cleaned == fs.Path() { + if resolved == fs.Path() { return errors.New("cannot delete root server directory") } - return os.RemoveAll(cleaned) + return os.RemoveAll(resolved) } // Lists the contents of a given directory and returns stat information about each From 8315ff8ae12155f8130e87a0aa9b826a4c07bdca Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Jul 2020 16:03:25 -0700 Subject: [PATCH 21/37] Misc mutex locking things to avoid data races --- router/tokens/websocket.go | 16 +++++++ router/websocket/websocket.go | 5 +-- server/config_parser.go | 3 +- server/environment_docker.go | 79 ++++++++++++++++++++++------------- server/listeners.go | 9 ++-- server/process.go | 10 +++++ server/server.go | 14 ++++++- server/update.go | 2 + 8 files changed, 98 insertions(+), 40 deletions(-) create mode 100644 server/process.go diff --git a/router/tokens/websocket.go b/router/tokens/websocket.go index cc03306..d9d710f 100644 --- a/router/tokens/websocket.go +++ b/router/tokens/websocket.go @@ -4,10 +4,13 @@ import ( "encoding/json" "github.com/gbrlsnchs/jwt/v3" "strings" + "sync" ) type WebsocketPayload struct { jwt.Payload + sync.RWMutex + UserID json.Number `json:"user_id"` ServerUUID string `json:"server_uuid"` Permissions []string `json:"permissions"` @@ -15,11 +18,24 @@ type WebsocketPayload struct { // Returns the JWT payload. func (p *WebsocketPayload) GetPayload() *jwt.Payload { + p.RLock() + defer p.RUnlock() + return &p.Payload } +func (p *WebsocketPayload) GetServerUuid() string { + p.RLock() + defer p.RUnlock() + + return p.ServerUUID +} + // Checks if the given token payload has a permission string. func (p *WebsocketPayload) HasPermission(permission string) bool { + p.RLock() + defer p.RUnlock() + for _, k := range p.Permissions { if k == permission || (!strings.HasPrefix(permission, "admin") && k == "*") { return true diff --git a/router/websocket/websocket.go b/router/websocket/websocket.go index dbd9bf8..3f79117 100644 --- a/router/websocket/websocket.go +++ b/router/websocket/websocket.go @@ -127,10 +127,7 @@ func (h *Handler) TokenValid() error { return errors.New("jwt does not have connect permission") } - h.server.RLock() - defer h.server.RUnlock() - - if h.server.Uuid != j.ServerUUID { + if h.server.Id() != j.GetServerUuid() { return errors.New("jwt server uuid mismatch") } diff --git a/server/config_parser.go b/server/config_parser.go index a0f1f11..5b64099 100644 --- a/server/config_parser.go +++ b/server/config_parser.go @@ -10,7 +10,8 @@ import ( func (s *Server) UpdateConfigurationFiles() { wg := new(sync.WaitGroup) - for _, v := range s.processConfiguration.ConfigurationFiles { + files := s.ProcessConfiguration().ConfigurationFiles + for _, v := range files { wg.Add(1) go func(f parser.ConfigurationFile, server *Server) { diff --git a/server/environment_docker.go b/server/environment_docker.go index 6b9246b..bc17765 100644 --- a/server/environment_docker.go +++ b/server/environment_docker.go @@ -21,6 +21,7 @@ import ( "path/filepath" "strconv" "strings" + "sync" "sync/atomic" "time" ) @@ -44,6 +45,23 @@ type DockerEnvironment struct { // Holds the stats stream used by the polling commands so that we can easily close // it out. stats io.ReadCloser + + sync.RWMutex +} + +// Set if this process is currently attached to the process. +func (d *DockerEnvironment) SetAttached(a bool) { + d.Lock() + d.attached = a + d.Unlock() +} + +// Determine if the this process is currently attached to the container. +func (d *DockerEnvironment) IsAttached() bool { + d.RLock() + defer d.RUnlock() + + return d.attached } // Creates a new base Docker environment. A server must still be attached to it. @@ -72,7 +90,7 @@ func (d *DockerEnvironment) Type() string { // Determines if the container exists in this environment. func (d *DockerEnvironment) Exists() (bool, error) { - _, err := d.Client.ContainerInspect(context.Background(), d.Server.Uuid) + _, err := d.Client.ContainerInspect(context.Background(), d.Server.Id()) if err != nil { // If this error is because the container instance wasn't found via Docker we @@ -96,7 +114,7 @@ func (d *DockerEnvironment) Exists() (bool, error) { // // @see docker/client/errors.go func (d *DockerEnvironment) IsRunning() (bool, error) { - c, err := d.Client.ContainerInspect(context.Background(), d.Server.Uuid) + c, err := d.Client.ContainerInspect(context.Background(), d.Server.Id()) if err != nil { return false, err } @@ -108,7 +126,7 @@ func (d *DockerEnvironment) IsRunning() (bool, error) { // making any changes to the operational state of the container. This allows memory, cpu, // and IO limitations to be adjusted on the fly for individual instances. func (d *DockerEnvironment) InSituUpdate() error { - if _, err := d.Client.ContainerInspect(context.Background(), d.Server.Uuid); err != nil { + if _, err := d.Client.ContainerInspect(context.Background(), d.Server.Id()); err != nil { // If the container doesn't exist for some reason there really isn't anything // we can do to fix that in this process (it doesn't make sense at least). In those // cases just return without doing anything since we still want to save the configuration @@ -130,7 +148,7 @@ func (d *DockerEnvironment) InSituUpdate() error { ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) defer cancel() - if _, err := d.Client.ContainerUpdate(ctx, d.Server.Uuid, u); err != nil { + if _, err := d.Client.ContainerUpdate(ctx, d.Server.Id(), u); err != nil { return errors.WithStack(err) } @@ -156,7 +174,7 @@ func (d *DockerEnvironment) OnBeforeStart() error { // Always destroy and re-create the server container to ensure that synced data from // the Panel is used. - if err := d.Client.ContainerRemove(context.Background(), d.Server.Uuid, types.ContainerRemoveOptions{RemoveVolumes: true}); err != nil { + if err := d.Client.ContainerRemove(context.Background(), d.Server.Id(), types.ContainerRemoveOptions{RemoveVolumes: true}); err != nil { if !client.IsErrNotFound(err) { return err } @@ -204,7 +222,7 @@ func (d *DockerEnvironment) Start() error { return &suspendedError{} } - if c, err := d.Client.ContainerInspect(context.Background(), d.Server.Uuid); err != nil { + if c, err := d.Client.ContainerInspect(context.Background(), d.Server.Id()); err != nil { // Do nothing if the container is not found, we just don't want to continue // to the next block of code here. This check was inlined here to guard againt // a nil-pointer when checking c.State below. @@ -259,7 +277,7 @@ func (d *DockerEnvironment) Start() error { ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) defer cancel() - if err := d.Client.ContainerStart(ctx, d.Server.Uuid, types.ContainerStartOptions{}); err != nil { + if err := d.Client.ContainerStart(ctx, d.Server.Id(), types.ContainerStartOptions{}); err != nil { return errors.WithStack(err) } @@ -272,7 +290,7 @@ func (d *DockerEnvironment) Start() error { // Stops the container that the server is running in. This will allow up to 10 // seconds to pass before a failure occurs. func (d *DockerEnvironment) Stop() error { - stop := d.Server.processConfiguration.Stop + stop := d.Server.ProcessConfiguration().Stop if stop.Type == api.ProcessStopSignal { return d.Terminate(os.Kill) } @@ -284,7 +302,7 @@ func (d *DockerEnvironment) Stop() error { t := time.Second * 10 - return d.Client.ContainerStop(context.Background(), d.Server.Uuid, &t) + return d.Client.ContainerStop(context.Background(), d.Server.Id(), &t) } // Attempts to gracefully stop a server using the defined stop command. If the server @@ -305,7 +323,7 @@ func (d *DockerEnvironment) WaitForStop(seconds int, terminate bool) error { // Block the return of this function until the container as been marked as no // longer running. If this wait does not end by the time seconds have passed, // attempt to terminate the container, or return an error. - ok, errChan := d.Client.ContainerWait(ctx, d.Server.Uuid, container.WaitConditionNotRunning) + ok, errChan := d.Client.ContainerWait(ctx, d.Server.Id(), container.WaitConditionNotRunning) select { case <-ctx.Done(): if ctxErr := ctx.Err(); ctxErr != nil { @@ -327,7 +345,7 @@ func (d *DockerEnvironment) WaitForStop(seconds int, terminate bool) error { // Forcefully terminates the container using the signal passed through. func (d *DockerEnvironment) Terminate(signal os.Signal) error { - c, err := d.Client.ContainerInspect(context.Background(), d.Server.Uuid) + c, err := d.Client.ContainerInspect(context.Background(), d.Server.Id()) if err != nil { return errors.WithStack(err) } @@ -339,7 +357,7 @@ func (d *DockerEnvironment) Terminate(signal os.Signal) error { d.Server.SetState(ProcessStoppingState) return d.Client.ContainerKill( - context.Background(), d.Server.Uuid, strings.TrimSuffix(strings.TrimPrefix(signal.String(), "signal "), "ed"), + context.Background(), d.Server.Id(), strings.TrimSuffix(strings.TrimPrefix(signal.String(), "signal "), "ed"), ) } @@ -349,7 +367,7 @@ func (d *DockerEnvironment) Destroy() error { // Avoid crash detection firing off. d.Server.SetState(ProcessStoppingState) - err := d.Client.ContainerRemove(context.Background(), d.Server.Uuid, types.ContainerRemoveOptions{ + err := d.Client.ContainerRemove(context.Background(), d.Server.Id(), types.ContainerRemoveOptions{ RemoveVolumes: true, RemoveLinks: false, Force: true, @@ -369,7 +387,7 @@ func (d *DockerEnvironment) Destroy() error { // Determine the container exit state and return the exit code and wether or not // the container was killed by the OOM killer. func (d *DockerEnvironment) ExitState() (uint32, bool, error) { - c, err := d.Client.ContainerInspect(context.Background(), d.Server.Uuid) + c, err := d.Client.ContainerInspect(context.Background(), d.Server.Id()) if err != nil { // I'm not entirely sure how this can happen to be honest. I tried deleting a // container _while_ a server was running and wings gracefully saw the crash and @@ -395,7 +413,7 @@ func (d *DockerEnvironment) ExitState() (uint32, bool, error) { // miss important output at the beginning because of the time delay with attaching to the // output. func (d *DockerEnvironment) Attach() error { - if d.attached { + if d.IsAttached() { return nil } @@ -404,7 +422,7 @@ func (d *DockerEnvironment) Attach() error { } var err error - d.stream, err = d.Client.ContainerAttach(context.Background(), d.Server.Uuid, types.ContainerAttachOptions{ + d.stream, err = d.Client.ContainerAttach(context.Background(), d.Server.Id(), types.ContainerAttachOptions{ Stdin: true, Stdout: true, Stderr: true, @@ -419,7 +437,7 @@ func (d *DockerEnvironment) Attach() error { Server: d.Server, } - d.attached = true + d.SetAttached(true) go func() { if err := d.EnableResourcePolling(); err != nil { d.Server.Log().WithField("error", errors.WithStack(err)).Warn("failed to enable resource polling on server") @@ -430,7 +448,7 @@ func (d *DockerEnvironment) Attach() error { defer d.stream.Close() defer func() { d.Server.SetState(ProcessOfflineState) - d.attached = false + d.SetAttached(false) }() io.Copy(console, d.stream.Reader) @@ -448,7 +466,7 @@ func (d *DockerEnvironment) FollowConsoleOutput() error { return errors.WithStack(err) } - return errors.New(fmt.Sprintf("no such container: %s", d.Server.Uuid)) + return errors.New(fmt.Sprintf("no such container: %s", d.Server.Id())) } opts := types.ContainerLogsOptions{ @@ -458,7 +476,7 @@ func (d *DockerEnvironment) FollowConsoleOutput() error { Since: time.Now().Format(time.RFC3339), } - reader, err := d.Client.ContainerLogs(context.Background(), d.Server.Uuid, opts) + reader, err := d.Client.ContainerLogs(context.Background(), d.Server.Id(), opts) go func(r io.ReadCloser) { defer r.Close() @@ -484,7 +502,7 @@ func (d *DockerEnvironment) EnableResourcePolling() error { return errors.New("cannot enable resource polling on a server that is not running") } - stats, err := d.Client.ContainerStats(context.Background(), d.Server.Uuid, true) + stats, err := d.Client.ContainerStats(context.Background(), d.Server.Id(), true) if err != nil { return errors.WithStack(err) } @@ -621,7 +639,7 @@ func (d *DockerEnvironment) Create() error { // If the container already exists don't hit the user with an error, just return // the current information about it which is what we would do when creating the // container anyways. - if _, err := d.Client.ContainerInspect(context.Background(), d.Server.Uuid); err == nil { + if _, err := d.Client.ContainerInspect(context.Background(), d.Server.Id()); err == nil { return nil } else if !client.IsErrNotFound(err) { return errors.WithStack(err) @@ -633,7 +651,7 @@ func (d *DockerEnvironment) Create() error { } conf := &container.Config{ - Hostname: d.Server.Uuid, + Hostname: d.Server.Id(), Domainname: config.Get().Docker.Domainname, User: strconv.Itoa(config.Get().System.User.Uid), AttachStdin: true, @@ -685,16 +703,17 @@ func (d *DockerEnvironment) Create() error { break } - log := log.WithFields(log.Fields{ - "server": d.Server.Uuid, + logger := log.WithFields(log.Fields{ + "server": d.Server.Id(), "source_path": source, "target_path": target, "read_only": m.ReadOnly, }) + if mounted { - log.Debug("attaching mount to server's container") + logger.Debug("attaching mount to server's container") } else { - log.Warn("skipping mount because it isn't allowed") + logger.Warn("skipping mount because it isn't allowed") } } @@ -738,7 +757,7 @@ func (d *DockerEnvironment) Create() error { NetworkMode: container.NetworkMode(config.Get().Docker.Network.Mode), } - if _, err := d.Client.ContainerCreate(context.Background(), conf, hostConf, nil, d.Server.Uuid); err != nil { + if _, err := d.Client.ContainerCreate(context.Background(), conf, hostConf, nil, d.Server.Id()); err != nil { return errors.WithStack(err) } @@ -748,7 +767,7 @@ func (d *DockerEnvironment) Create() error { // Sends the specified command to the stdin of the running container instance. There is no // confirmation that this data is sent successfully, only that it gets pushed into the stdin. func (d *DockerEnvironment) SendCommand(c string) error { - if !d.attached { + if !d.IsAttached() { return errors.New("attempting to send command to non-attached instance") } @@ -760,7 +779,7 @@ func (d *DockerEnvironment) SendCommand(c string) error { // Reads the log file for the server. This does not care if the server is running or not, it will // simply try to read the last X bytes of the file and return them. func (d *DockerEnvironment) Readlog(len int64) ([]string, error) { - j, err := d.Client.ContainerInspect(context.Background(), d.Server.Uuid) + j, err := d.Client.ContainerInspect(context.Background(), d.Server.Id()) if err != nil { return nil, err } diff --git a/server/listeners.go b/server/listeners.go index 73df884..bcb4bf7 100644 --- a/server/listeners.go +++ b/server/listeners.go @@ -27,9 +27,11 @@ func (s *Server) onConsoleOutput(data string) { // If the specific line of output is one that would mark the server as started, // set the server to that state. Only do this if the server is not currently stopped // or stopping. - if s.GetState() == ProcessStartingState && strings.Contains(data, s.processConfiguration.Startup.Done) { + match := s.ProcessConfiguration().Startup.Done + + if s.GetState() == ProcessStartingState && strings.Contains(data, match) { s.Log().WithFields(log.Fields{ - "match": s.processConfiguration.Startup.Done, + "match": match, "against": data, }).Debug("detected server in running state based on console line output") @@ -40,7 +42,8 @@ func (s *Server) onConsoleOutput(data string) { // set the server to be in a stopping state, otherwise crash detection will kick in and // cause the server to unexpectedly restart on the user. if s.IsRunning() { - if s.processConfiguration.Stop.Type == api.ProcessStopCommand && data == s.processConfiguration.Stop.Value { + stop := s.ProcessConfiguration().Stop + if stop.Type == api.ProcessStopCommand && data == stop.Value { s.SetState(ProcessStoppingState) } } diff --git a/server/process.go b/server/process.go new file mode 100644 index 0000000..b8a489d --- /dev/null +++ b/server/process.go @@ -0,0 +1,10 @@ +package server + +import "github.com/pterodactyl/wings/api" + +func (s *Server) ProcessConfiguration() *api.ProcessConfiguration { + s.RLock() + defer s.RUnlock() + + return s.procConfig +} diff --git a/server/server.go b/server/server.go index 7f48148..4a970e7 100644 --- a/server/server.go +++ b/server/server.go @@ -104,7 +104,7 @@ type Server struct { // Defines the process configuration for the server instance. This is dynamically // fetched from the Pterodactyl Server instance each time the server process is // started, and then cached here. - processConfiguration *api.ProcessConfiguration + procConfig *api.ProcessConfiguration // Tracks the installation process for this server and prevents a server from running // two installer processes at the same time. This also allows us to cancel a running @@ -153,6 +153,13 @@ type BuildSettings struct { Threads string `json:"threads"` } +func (s *Server) Id() string { + s.RLock() + defer s.RUnlock() + + return s.Uuid +} + // Converts the CPU limit for a server build into a number that can be better understood // by the Docker environment. If there is no limit set, return -1 which will indicate to // Docker that it has unlimited CPU quota. @@ -366,7 +373,10 @@ func (s *Server) SyncWithConfiguration(cfg *api.ServerConfigurationResponse) err return errors.WithStack(err) } - s.processConfiguration = cfg.ProcessConfiguration + s.Lock() + s.procConfig = cfg.ProcessConfiguration + s.Unlock() + return nil } diff --git a/server/update.go b/server/update.go index 465df57..00d9eed 100644 --- a/server/update.go +++ b/server/update.go @@ -27,12 +27,14 @@ func (s *Server) UpdateDataStructure(data []byte, background bool) error { return errors.New("attempting to merge a data stack with an invalid UUID") } + s.Lock() // Merge the new data object that we have received with the existing server data object // and then save it to the disk so it is persistent. if err := mergo.Merge(s, src, mergo.WithOverride); err != nil { return errors.WithStack(err) } + // Mergo makes that previous lock disappear. Handle that by just re-locking the object. s.Lock() defer s.Unlock() From 6de18f09e553c6f67c9ae4f4a53cf9f1d9622945 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Jul 2020 16:45:10 -0700 Subject: [PATCH 22/37] Don't block websocket from handling another message when a long running proccess is triggered --- router/router_server_ws.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/router/router_server_ws.go b/router/router_server_ws.go index 4f60f93..f630240 100644 --- a/router/router_server_ws.go +++ b/router/router_server_ws.go @@ -51,8 +51,10 @@ func getServerWebsocket(c *gin.Context) { continue } - if err := handler.HandleInbound(j); err != nil { - handler.SendErrorJson(j, err) - } + go func(msg websocket.Message) { + if err := handler.HandleInbound(msg); err != nil { + handler.SendErrorJson(msg, err) + } + }(j) } } From a00288aa64f57e68d9b1057d2b7122aa572dc637 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sat, 18 Jul 2020 16:46:41 -0700 Subject: [PATCH 23/37] Require a lock on the restart process to avoid double restarts causing unexpected behavior --- router/websocket/websocket.go | 9 ++-- server/environment.go | 7 +++ server/environment_docker.go | 81 +++++++++++++++++++++++++++++++++-- server/server.go | 6 +-- 4 files changed, 92 insertions(+), 11 deletions(-) diff --git a/router/websocket/websocket.go b/router/websocket/websocket.go index 3f79117..b321119 100644 --- a/router/websocket/websocket.go +++ b/router/websocket/websocket.go @@ -283,11 +283,14 @@ func (h *Handler) HandleInbound(m Message) error { break case "restart": if h.GetJwt().HasPermission(PermissionSendPowerRestart) { - if err := h.server.Environment.WaitForStop(60, false); err != nil { - return err + // If the server is alreay restarting don't do anything. Perhaps we send back an event + // in the future for this? For now no reason to knowingly trigger an error by trying to + // restart a process already restarting. + if h.server.Environment.IsRestarting() { + return nil } - return h.server.Environment.Start() + return h.server.Environment.Restart() } break case "kill": diff --git a/server/environment.go b/server/environment.go index 4573fdd..ce6eed0 100644 --- a/server/environment.go +++ b/server/environment.go @@ -31,6 +31,13 @@ type Environment interface { // not be returned. Stop() error + // Restart a server instance. If already stopped the process will be started. This function + // will return an error if the server is already performing a restart process as to avoid + // unnecessary double/triple/quad looping issues if multiple people press restart or spam the + // button to restart. + Restart() error + IsRestarting() bool + // Waits for a server instance to stop gracefully. If the server is still detected // as running after seconds, an error will be returned, or the server will be terminated // depending on the value of the second argument. diff --git a/server/environment_docker.go b/server/environment_docker.go index bc17765..27a702e 100644 --- a/server/environment_docker.go +++ b/server/environment_docker.go @@ -16,6 +16,7 @@ import ( "github.com/pkg/errors" "github.com/pterodactyl/wings/api" "github.com/pterodactyl/wings/config" + "golang.org/x/sync/semaphore" "io" "os" "path/filepath" @@ -28,6 +29,8 @@ import ( // Defines the base environment for Docker instances running through Wings. type DockerEnvironment struct { + sync.RWMutex + Server *Server // The Docker client being used for this instance. @@ -46,7 +49,9 @@ type DockerEnvironment struct { // it out. stats io.ReadCloser - sync.RWMutex + // Locks when we're performing a restart to avoid trying to restart a process that is already + // being restarted. + restartSem *semaphore.Weighted } // Set if this process is currently attached to the process. @@ -296,13 +301,83 @@ func (d *DockerEnvironment) Stop() error { } d.Server.SetState(ProcessStoppingState) - if stop.Type == api.ProcessStopCommand { + // Only attempt to send the stop command to the instance if we are actually attached to + // the instance. If we are not for some reason, just send the container stop event. + if d.IsAttached() && stop.Type == api.ProcessStopCommand { return d.SendCommand(stop.Value) } t := time.Second * 10 - return d.Client.ContainerStop(context.Background(), d.Server.Id(), &t) + err := d.Client.ContainerStop(context.Background(), d.Server.Id(), &t) + if err != nil { + // If the container does not exist just mark the process as stopped and return without + // an error. + if client.IsErrNotFound(err) { + d.SetAttached(false) + d.Server.SetState(ProcessOfflineState) + + return nil + } + + return err + } + + return nil +} + +// Try to acquire a lock to restart the server. If one cannot be obtained within 5 seconds return +// an error to the caller. You should ideally be checking IsRestarting() before calling this function +// to avoid unnecessary delays since you can respond immediately from that. +func (d *DockerEnvironment) acquireRestartLock() error { + if d.restartSem == nil { + d.restartSem = semaphore.NewWeighted(1) + } + + ctx, _ := context.WithTimeout(context.Background(), time.Second*5) + + return d.restartSem.Acquire(ctx, 1) +} + +// Restarts the server process by waiting for the process to gracefully stop and then triggering a +// start command. This will return an error if there is already a restart process executing for the +// server. The lock is released when the process is stopped and a start has begun. +func (d *DockerEnvironment) Restart() error { + d.Server.Log().Debug("attempting to acquire restart lock...") + if err := d.acquireRestartLock(); err != nil { + d.Server.Log().Warn("failed to acquire restart lock; already acquired by a different process") + return err + } + + d.Server.Log().Debug("acquired restart lock") + + err := d.WaitForStop(60, false) + if err != nil { + d.restartSem.Release(1) + return err + } + + // Release the restart lock, it is now safe for someone to attempt restarting the server again. + d.restartSem.Release(1) + + // Start the process. + return d.Start() +} + +// Check if the server is currently running the restart process by checking if there is a semaphore +// allocated, and if so, if we can aquire a lock on it. +func (d *DockerEnvironment) IsRestarting() bool { + if d.restartSem == nil { + return false + } + + if d.restartSem.TryAcquire(1) { + d.restartSem.Release(1) + + return false + } + + return true } // Attempts to gracefully stop a server using the defined stop command. If the server diff --git a/server/server.go b/server/server.go index 4a970e7..85eff71 100644 --- a/server/server.go +++ b/server/server.go @@ -411,11 +411,7 @@ func (s *Server) HandlePowerAction(action PowerAction) error { case "start": return s.Environment.Start() case "restart": - if err := s.Environment.WaitForStop(60, false); err != nil { - return err - } - - return s.Environment.Start() + return s.Environment.Restart() case "stop": return s.Environment.Stop() case "kill": From 0cbaad5c729b6177aad356caa67151ac2b6ff49b Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 19 Jul 2020 16:27:55 -0700 Subject: [PATCH 24/37] Completely re-work the server configuration to be separated out better --- cmd/root.go | 10 +- installer/installer.go | 31 ++-- router/router_server.go | 4 +- router/websocket/websocket.go | 14 +- server/allocations.go | 17 ++ server/build_settings.go | 72 +++++++++ server/configuration.go | 84 ++++++++++ server/crash.go | 30 +++- server/environment_docker.go | 60 ++++--- server/filesystem.go | 10 +- server/filesystem_unarchive.go | 4 +- server/loader.go | 119 ++++++++++++++ server/resources.go | 57 ++++++- server/server.go | 287 +++------------------------------ server/state.go | 33 ++-- server/update.go | 72 +++++---- 16 files changed, 495 insertions(+), 409 deletions(-) create mode 100644 server/allocations.go create mode 100644 server/build_settings.go create mode 100644 server/configuration.go create mode 100644 server/loader.go diff --git a/cmd/root.go b/cmd/root.go index 60b5d31..d992e19 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -181,17 +181,13 @@ func rootCmdRun(*cobra.Command, []string) { wg.Add() go func(s *server.Server) { - // Required for tracing purposes. - var err error + defer wg.Done() - defer func() { - s.Log().Trace("ensuring server environment exists").Stop(&err) - wg.Done() - }() + s.Log().Info("ensuring server environment exists") // Create a server environment if none exists currently. This allows us to recover from Docker // being reinstalled on the host system for example. - if err = s.Environment.Create(); err != nil { + if err := s.Environment.Create(); err != nil { s.Log().WithField("error", err).Error("failed to process environment") } diff --git a/installer/installer.go b/installer/installer.go index 37ce0cb..6ca95fd 100644 --- a/installer/installer.go +++ b/installer/installer.go @@ -29,12 +29,10 @@ func New(data []byte) (*Installer, error) { return nil, NewValidationError("service egg provided was not in a valid format") } - s := &server.Server{ + cfg := &server.Configuration{ Uuid: getString(data, "uuid"), Suspended: false, - State: server.ProcessOfflineState, Invocation: getString(data, "invocation"), - EnvVars: make(server.EnvironmentVariables), Build: server.BuildSettings{ MemoryLimit: getInt(data, "build", "memory"), Swap: getInt(data, "build", "swap"), @@ -43,20 +41,18 @@ func New(data []byte) (*Installer, error) { DiskSpace: getInt(data, "build", "disk"), Threads: getString(data, "build", "threads"), }, - Allocations: server.Allocations{ - Mappings: make(map[string][]int), - }, + CrashDetectionEnabled: true, } - s.Allocations.DefaultMapping.Ip = getString(data, "allocations", "default", "ip") - s.Allocations.DefaultMapping.Port = int(getInt(data, "allocations", "default", "port")) + cfg.Allocations.DefaultMapping.Ip = getString(data, "allocations", "default", "ip") + cfg.Allocations.DefaultMapping.Port = int(getInt(data, "allocations", "default", "port")) // Unmarshal the environment variables from the request into the server struct. if b, _, _, err := jsonparser.Get(data, "environment"); err != nil { return nil, errors.WithStack(err) } else { - s.EnvVars = make(server.EnvironmentVariables) - if err := json.Unmarshal(b, &s.EnvVars); err != nil { + cfg.EnvVars = make(server.EnvironmentVariables) + if err := json.Unmarshal(b, &cfg.EnvVars); err != nil { return nil, errors.WithStack(err) } } @@ -65,15 +61,15 @@ func New(data []byte) (*Installer, error) { if b, _, _, err := jsonparser.Get(data, "allocations", "mappings"); err != nil { return nil, errors.WithStack(err) } else { - s.Allocations.Mappings = make(map[string][]int) - if err := json.Unmarshal(b, &s.Allocations.Mappings); err != nil { + cfg.Allocations.Mappings = make(map[string][]int) + if err := json.Unmarshal(b, &cfg.Allocations.Mappings); err != nil { return nil, errors.WithStack(err) } } - s.Container.Image = getString(data, "container", "image") + cfg.Container.Image = getString(data, "container", "image") - c, rerr, err := api.NewRequester().GetServerConfiguration(s.Uuid) + c, rerr, err := api.NewRequester().GetServerConfiguration(cfg.Uuid) if err != nil || rerr != nil { if err != nil { return nil, errors.WithStack(err) @@ -82,15 +78,12 @@ func New(data []byte) (*Installer, error) { return nil, errors.New(rerr.String()) } - // Destroy the temporary server instance. - s = nil - // Create a new server instance using the configuration we wrote to the disk // so that everything gets instantiated correctly on the struct. - s2, err := server.FromConfiguration(c) + s, err := server.FromConfiguration(c) return &Installer{ - server: s2, + server: s, }, err } diff --git a/router/router_server.go b/router/router_server.go index 87c0b67..d8c47a4 100644 --- a/router/router_server.go +++ b/router/router_server.go @@ -64,7 +64,7 @@ func postServerPower(c *gin.Context) { // // We don't really care about any of the other actions at this point, they'll all result // in the process being stopped, which should have happened anyways if the server is suspended. - if (data.Action == "start" || data.Action == "restart") && s.Suspended { + if (data.Action == "start" || data.Action == "restart") && s.IsSuspended() { c.AbortWithStatusJSON(http.StatusBadRequest, gin.H{ "error": "Cannot start or restart a server that is suspended.", }) @@ -162,7 +162,7 @@ func deleteServer(c *gin.Context) { // Immediately suspend the server to prevent a user from attempting // to start it while this process is running. - s.Suspended = true + s.Config().SetSuspended(true) // If the server is currently installing, abort it. if s.IsInstalling() { diff --git a/router/websocket/websocket.go b/router/websocket/websocket.go index b321119..5b0cf3c 100644 --- a/router/websocket/websocket.go +++ b/router/websocket/websocket.go @@ -247,19 +247,7 @@ func (h *Handler) HandleInbound(m Message) error { if state == server.ProcessOfflineState { _ = h.server.Filesystem.HasSpaceAvailable() - h.server.Resources.RLock() - defer h.server.Resources.RUnlock() - - resources := server.ResourceUsage{ - Memory: 0, - MemoryLimit: 0, - CpuAbsolute: 0.0, - Disk: h.server.Resources.Disk, - } - resources.Network.RxBytes = 0 - resources.Network.TxBytes = 0 - - b, _ := json.Marshal(resources) + b, _ := json.Marshal(h.server.Proc()) h.SendJson(&Message{ Event: server.StatsEvent, Args: []string{string(b)}, diff --git a/server/allocations.go b/server/allocations.go new file mode 100644 index 0000000..ac51a0d --- /dev/null +++ b/server/allocations.go @@ -0,0 +1,17 @@ +package server + +// Defines the allocations available for a given server. When using the Docker environment +// driver these correspond to mappings for the container that allow external connections. +type Allocations struct { + // Defines the default allocation that should be used for this server. This is + // what will be used for {SERVER_IP} and {SERVER_PORT} when modifying configuration + // files or the startup arguments for a server. + DefaultMapping struct { + Ip string `json:"ip"` + Port int `json:"port"` + } `json:"default"` + + // Mappings contains all of the ports that should be assigned to a given server + // attached to the IP they correspond to. + Mappings map[string][]int `json:"mappings"` +} \ No newline at end of file diff --git a/server/build_settings.go b/server/build_settings.go new file mode 100644 index 0000000..357aecf --- /dev/null +++ b/server/build_settings.go @@ -0,0 +1,72 @@ +package server + +import "math" + +// The build settings for a given server that impact docker container creation and +// resource limits for a server instance. +type BuildSettings struct { + // The total amount of memory in megabytes that this server is allowed to + // use on the host system. + MemoryLimit int64 `json:"memory_limit"` + + // The amount of additional swap space to be provided to a container instance. + Swap int64 `json:"swap"` + + // The relative weight for IO operations in a container. This is relative to other + // containers on the system and should be a value between 10 and 1000. + IoWeight uint16 `json:"io_weight"` + + // The percentage of CPU that this instance is allowed to consume relative to + // the host. A value of 200% represents complete utilization of two cores. This + // should be a value between 1 and THREAD_COUNT * 100. + CpuLimit int64 `json:"cpu_limit"` + + // The amount of disk space in megabytes that a server is allowed to use. + DiskSpace int64 `json:"disk_space"` + + // Sets which CPU threads can be used by the docker instance. + Threads string `json:"threads"` +} + +func (s *Server) Build() *BuildSettings { + return &s.Config().Build +} + +// Converts the CPU limit for a server build into a number that can be better understood +// by the Docker environment. If there is no limit set, return -1 which will indicate to +// Docker that it has unlimited CPU quota. +func (b *BuildSettings) ConvertedCpuLimit() int64 { + if b.CpuLimit == 0 { + return -1 + } + + return b.CpuLimit * 1000 +} + +// Set the hard limit for memory usage to be 5% more than the amount of memory assigned to +// the server. If the memory limit for the server is < 4G, use 10%, if less than 2G use +// 15%. This avoids unexpected crashes from processes like Java which run over the limit. +func (b *BuildSettings) MemoryOverheadMultiplier() float64 { + if b.MemoryLimit <= 2048 { + return 1.15 + } else if b.MemoryLimit <= 4096 { + return 1.10 + } + + return 1.05 +} + +func (b *BuildSettings) BoundedMemoryLimit() int64 { + return int64(math.Round(float64(b.MemoryLimit) * b.MemoryOverheadMultiplier() * 1_000_000)) +} + +// Returns the amount of swap available as a total in bytes. This is returned as the amount +// of memory available to the server initially, PLUS the amount of additional swap to include +// which is the format used by Docker. +func (b *BuildSettings) ConvertedSwap() int64 { + if b.Swap < 0 { + return -1 + } + + return (b.Swap * 1_000_000) + b.BoundedMemoryLimit() +} diff --git a/server/configuration.go b/server/configuration.go new file mode 100644 index 0000000..3cf39b4 --- /dev/null +++ b/server/configuration.go @@ -0,0 +1,84 @@ +package server + +import ( + "fmt" + "strconv" + "sync" +) + +type EnvironmentVariables map[string]interface{} + +// Ugly hacky function to handle environment variables that get passed through as not-a-string +// from the Panel. Ideally we'd just say only pass strings, but that is a fragile idea and if a +// string wasn't passed through you'd cause a crash or the server to become unavailable. For now +// try to handle the most likely values from the JSON and hope for the best. +func (ev EnvironmentVariables) Get(key string) string { + val, ok := ev[key] + if !ok { + return "" + } + + switch val.(type) { + case int: + return strconv.Itoa(val.(int)) + case int32: + return strconv.FormatInt(val.(int64), 10) + case int64: + return strconv.FormatInt(val.(int64), 10) + case float32: + return fmt.Sprintf("%f", val.(float32)) + case float64: + return fmt.Sprintf("%f", val.(float64)) + case bool: + return strconv.FormatBool(val.(bool)) + } + + return val.(string) +} + +type Configuration struct { + mu sync.RWMutex + + // The unique identifier for the server that should be used when referencing + // it against the Panel API (and internally). This will be used when naming + // docker containers as well as in log output. + Uuid string `json:"uuid"` + + // Whether or not the server is in a suspended state. Suspended servers cannot + // be started or modified except in certain scenarios by an admin user. + Suspended bool `json:"suspended"` + + // The command that should be used when booting up the server instance. + Invocation string `json:"invocation"` + + // An array of environment variables that should be passed along to the running + // server process. + EnvVars EnvironmentVariables `json:"environment"` + + Allocations Allocations `json:"allocations"` + Build BuildSettings `json:"build"` + CrashDetectionEnabled bool `default:"true" json:"enabled" yaml:"enabled"` + Mounts []Mount `json:"mounts"` + Resources ResourceUsage `json:"resources"` + + Container struct { + // Defines the Docker image that will be used for this server + Image string `json:"image,omitempty"` + // If set to true, OOM killer will be disabled on the server's Docker container. + // If not present (nil) we will default to disabling it. + OomDisabled bool `default:"true" json:"oom_disabled"` + } `json:"container,omitempty"` +} + +func (s *Server) Config() *Configuration { + s.cfg.mu.RLock() + defer s.cfg.mu.RUnlock() + + return &s.cfg +} + +func (c *Configuration) SetSuspended(s bool) { + c.mu.Lock() + c.Suspended = s + c.mu.Unlock() +} diff --git a/server/crash.go b/server/crash.go index 649580f..3eb2ba4 100644 --- a/server/crash.go +++ b/server/crash.go @@ -4,18 +4,32 @@ import ( "fmt" "github.com/pkg/errors" "github.com/pterodactyl/wings/config" + "sync" "time" ) -type CrashDetection struct { - // If set to false, the system will not listen for crash detection events that - // can indicate that the server stopped unexpectedly. - Enabled bool `default:"true" json:"enabled" yaml:"enabled"` +type CrashHandler struct { + mu sync.RWMutex // Tracks the time of the last server crash event. lastCrash time.Time } +// Returns the time of the last crash for this server instance. +func (cd *CrashHandler) LastCrashTime() time.Time { + cd.mu.RLock() + defer cd.mu.RUnlock() + + return cd.lastCrash +} + +// Sets the last crash time for a server. +func (cd *CrashHandler) SetLastCrash(t time.Time) { + cd.mu.Lock() + cd.lastCrash = t + cd.mu.Unlock() +} + // Looks at the environment exit state to determine if the process exited cleanly or // if it was the result of an event that we should try to recover from. // @@ -30,8 +44,8 @@ func (s *Server) handleServerCrash() error { // No point in doing anything here if the server isn't currently offline, there // is no reason to do a crash detection event. If the server crash detection is // disabled we want to skip anything after this as well. - if s.GetState() != ProcessOfflineState || !s.CrashDetection.Enabled { - if !s.CrashDetection.Enabled { + if s.GetState() != ProcessOfflineState || !s.Config().CrashDetectionEnabled { + if !s.Config().CrashDetectionEnabled { s.Log().Debug("server triggered crash detection but handler is disabled for server process") s.PublishConsoleOutputFromDaemon("Server detected as crashed; crash detection is disabled for this instance.") @@ -57,7 +71,7 @@ func (s *Server) handleServerCrash() error { s.PublishConsoleOutputFromDaemon(fmt.Sprintf("Exit code: %d", exitCode)) s.PublishConsoleOutputFromDaemon(fmt.Sprintf("Out of memory: %t", oomKilled)) - c := s.CrashDetection.lastCrash + c := s.crasher.LastCrashTime() // If the last crash time was within the last 60 seconds we do not want to perform // an automatic reboot of the process. Return an error that can be handled. if !c.IsZero() && c.Add(time.Second * 60).After(time.Now()) { @@ -66,7 +80,7 @@ func (s *Server) handleServerCrash() error { return &crashTooFrequent{} } - s.CrashDetection.lastCrash = time.Now() + s.crasher.SetLastCrash(time.Now()) return s.Environment.Start() } \ No newline at end of file diff --git a/server/environment_docker.go b/server/environment_docker.go index 27a702e..fd2dda6 100644 --- a/server/environment_docker.go +++ b/server/environment_docker.go @@ -23,7 +23,6 @@ import ( "strconv" "strings" "sync" - "sync/atomic" "time" ) @@ -223,7 +222,7 @@ func (d *DockerEnvironment) Start() error { // Theoretically you'd have the Panel handle all of this logic, but we cannot do that // because we allow the websocket to control the server power state as well, so we'll // need to handle that action in here. - if d.Server.Suspended { + if d.Server.IsSuspended() { return &suspendedError{} } @@ -604,22 +603,16 @@ func (d *DockerEnvironment) EnableResourcePolling() error { return } - s.Resources.Lock() - s.Resources.CpuAbsolute = s.Resources.CalculateAbsoluteCpu(&v.PreCPUStats, &v.CPUStats) - s.Resources.Memory = s.Resources.CalculateDockerMemory(v.MemoryStats) - s.Resources.MemoryLimit = v.MemoryStats.Limit - s.Resources.Unlock() + s.Proc().UpdateFromDocker(v) + for _, nw := range v.Networks { + s.Proc().UpdateNetworkBytes(&nw) + } // Why you ask? This already has the logic for caching disk space in use and then // also handles pushing that value to the resources object automatically. s.Filesystem.HasSpaceAvailable() - for _, nw := range v.Networks { - atomic.AddUint64(&s.Resources.Network.RxBytes, nw.RxBytes) - atomic.AddUint64(&s.Resources.Network.TxBytes, nw.TxBytes) - } - - b, _ := json.Marshal(s.Resources) + b, _ := json.Marshal(s.Proc()) s.Events().Publish(StatsEvent, string(b)) } }(d.Server) @@ -634,12 +627,16 @@ func (d *DockerEnvironment) DisableResourcePolling() error { } err := d.stats.Close() - - d.Server.Resources.Empty() + d.Server.Proc().Empty() return errors.WithStack(err) } +// Returns the image to be used for the instance. +func (d *DockerEnvironment) Image() string { + return d.Server.Config().Container.Image +} + // Pulls the image from Docker. If there is an error while pulling the image from the source // but the image already exists locally, we will report that error to the logger but continue // with the process. @@ -657,7 +654,7 @@ func (d *DockerEnvironment) ensureImageExists() error { ctx, cancel := context.WithTimeout(context.Background(), time.Minute*15) defer cancel() - out, err := d.Client.ImagePull(ctx, d.Server.Container.Image, types.ImagePullOptions{All: false}) + out, err := d.Client.ImagePull(ctx, d.Image(), types.ImagePullOptions{All: false}) if err != nil { images, ierr := d.Client.ImageList(ctx, types.ImageListOptions{}) if ierr != nil { @@ -668,12 +665,12 @@ func (d *DockerEnvironment) ensureImageExists() error { for _, img := range images { for _, t := range img.RepoTags { - if t != d.Server.Container.Image { + if t != d.Image() { continue } d.Server.Log().WithFields(log.Fields{ - "image": d.Server.Container.Image, + "image": d.Image(), "error": errors.New(err.Error()), }).Warn("unable to pull requested image from remote source, however the image exists locally") @@ -687,7 +684,7 @@ func (d *DockerEnvironment) ensureImageExists() error { } defer out.Close() - log.WithField("image", d.Server.Container.Image).Debug("pulling docker image... this could take a bit of time") + log.WithField("image", d.Image()).Debug("pulling docker image... this could take a bit of time") // I'm not sure what the best approach here is, but this will block execution until the image // is done being pulled, which is what we need. @@ -734,12 +731,9 @@ func (d *DockerEnvironment) Create() error { AttachStderr: true, OpenStdin: true, Tty: true, - ExposedPorts: d.exposedPorts(), - - Image: d.Server.Container.Image, - Env: d.Server.GetEnvironmentVariables(), - + Image: d.Image(), + Env: d.Server.GetEnvironmentVariables(), Labels: map[string]string{ "Service": "Pterodactyl", "ContainerType": "server_process", @@ -756,7 +750,7 @@ func (d *DockerEnvironment) Create() error { } var mounted bool - for _, m := range d.Server.Mounts { + for _, m := range d.Server.Config().Mounts { mounted = false source := filepath.Clean(m.Source) target := filepath.Clean(m.Target) @@ -931,7 +925,7 @@ func (d *DockerEnvironment) parseLogToStrings(b []byte) ([]string, error) { func (d *DockerEnvironment) portBindings() nat.PortMap { var out = nat.PortMap{} - for ip, ports := range d.Server.Allocations.Mappings { + for ip, ports := range d.Server.Config().Allocations.Mappings { for _, port := range ports { // Skip over invalid ports. if port < 0 || port > 65535 { @@ -981,14 +975,14 @@ func (d *DockerEnvironment) exposedPorts() nat.PortSet { // the same or higher than the memory limit. func (d *DockerEnvironment) getResourcesForServer() container.Resources { return container.Resources{ - Memory: d.Server.Build.BoundedMemoryLimit(), - MemoryReservation: d.Server.Build.MemoryLimit * 1_000_000, - MemorySwap: d.Server.Build.ConvertedSwap(), - CPUQuota: d.Server.Build.ConvertedCpuLimit(), + Memory: d.Server.Build().BoundedMemoryLimit(), + MemoryReservation: d.Server.Build().MemoryLimit * 1_000_000, + MemorySwap: d.Server.Build().ConvertedSwap(), + CPUQuota: d.Server.Build().ConvertedCpuLimit(), CPUPeriod: 100_000, CPUShares: 1024, - BlkioWeight: d.Server.Build.IoWeight, - OomKillDisable: &d.Server.Container.OomDisabled, - CpusetCpus: d.Server.Build.Threads, + BlkioWeight: d.Server.Build().IoWeight, + OomKillDisable: &d.Server.Config().Container.OomDisabled, + CpusetCpus: d.Server.Build().Threads, } } diff --git a/server/filesystem.go b/server/filesystem.go index 545fe87..b2115cd 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -208,7 +208,7 @@ func (fs *Filesystem) ParallelSafePath(paths []string) ([]string, error) { // Because determining the amount of space being used by a server is a taxing operation we // will load it all up into a cache and pull from that as long as the key is not expired. func (fs *Filesystem) HasSpaceAvailable() bool { - space := fs.Server.Build.DiskSpace + space := fs.Server.Build().DiskSpace size, err := fs.getCachedDiskUsage() if err != nil { @@ -217,9 +217,7 @@ func (fs *Filesystem) HasSpaceAvailable() bool { // Determine if their folder size, in bytes, is smaller than the amount of space they've // been allocated. - fs.Server.Resources.Lock() - fs.Server.Resources.Disk = size - fs.Server.Resources.Unlock() + fs.Server.Proc().SetDisk(size) // If space is -1 or 0 just return true, means they're allowed unlimited. // @@ -247,7 +245,7 @@ func (fs *Filesystem) getCachedDiskUsage() (int64, error) { fs.cacheDiskMu.Lock() defer fs.cacheDiskMu.Unlock() - if x, exists := fs.Server.Cache.Get("disk_used"); exists { + if x, exists := fs.Server.cache.Get("disk_used"); exists { return x.(int64), nil } @@ -260,7 +258,7 @@ func (fs *Filesystem) getCachedDiskUsage() (int64, error) { // Always cache the size, even if there is an error. We want to always return that value // so that we don't cause an endless loop of determining the disk size if there is a temporary // error encountered. - fs.Server.Cache.Set("disk_used", size, time.Second*60) + fs.Server.cache.Set("disk_used", size, time.Second*60) return size, err } diff --git a/server/filesystem_unarchive.go b/server/filesystem_unarchive.go index 7197554..9e40c28 100644 --- a/server/filesystem_unarchive.go +++ b/server/filesystem_unarchive.go @@ -21,7 +21,7 @@ import ( func (fs *Filesystem) SpaceAvailableForDecompression(dir string, file string) (bool, error) { // Don't waste time trying to determine this if we know the server will have the space for // it since there is no limit. - if fs.Server.Build.DiskSpace <= 0 { + if fs.Server.Build().DiskSpace <= 0 { return true, nil } @@ -60,7 +60,7 @@ func (fs *Filesystem) SpaceAvailableForDecompression(dir string, file string) (b wg.Wait() - return ((dirSize + size) / 1000.0 / 1000.0) <= fs.Server.Build.DiskSpace, cErr + return ((dirSize + size) / 1000.0 / 1000.0) <= fs.Server.Build().DiskSpace, cErr } // Decompress a file in a given directory by using the archiver tool to infer the file diff --git a/server/loader.go b/server/loader.go new file mode 100644 index 0000000..55b1a32 --- /dev/null +++ b/server/loader.go @@ -0,0 +1,119 @@ +package server + +import ( + "github.com/apex/log" + "github.com/creasty/defaults" + "github.com/patrickmn/go-cache" + "github.com/pkg/errors" + "github.com/pterodactyl/wings/api" + "github.com/pterodactyl/wings/config" + "github.com/remeh/sizedwaitgroup" + "time" +) + +var servers = NewCollection(nil) + +func GetServers() *Collection { + return servers +} + +// Iterates over a given directory and loads all of the servers listed before returning +// them to the calling function. +func LoadDirectory() error { + if len(servers.items) != 0 { + return errors.New("cannot call LoadDirectory with a non-nil collection") + } + + // We could theoretically use a standard wait group here, however doing + // that introduces the potential to crash the program due to too many + // open files. This wouldn't happen on a small setup, but once the daemon is + // handling many servers you run that risk. + // + // For now just process 10 files at a time, that should be plenty fast to + // read and parse the YAML. We should probably make this configurable down + // the road to help big instances scale better. + wg := sizedwaitgroup.New(10) + + configs, rerr, err := api.NewRequester().GetAllServerConfigurations() + if err != nil || rerr != nil { + if err != nil { + return errors.WithStack(err) + } + + return errors.New(rerr.String()) + } + + states, err := getServerStates() + if err != nil { + return errors.WithStack(err) + } + + for uuid, data := range configs { + wg.Add() + + go func(uuid string, data *api.ServerConfigurationResponse) { + defer wg.Done() + + s, err := FromConfiguration(data) + if err != nil { + log.WithField("server", uuid).WithField("error", err).Error("failed to load server, skipping...") + return + } + + if state, exists := states[s.Id()]; exists { + s.SetState(state) + s.Log().WithField("state", s.GetState()).Debug("loaded server state from cache file") + } + + servers.Add(s) + }(uuid, data) + } + + // Wait until we've processed all of the configuration files in the directory + // before continuing. + wg.Wait() + + return nil +} + +// Initializes a server using a data byte array. This will be marshaled into the +// given struct using a YAML marshaler. This will also configure the given environment +// for a server. +func FromConfiguration(data *api.ServerConfigurationResponse) (*Server, error) { + cfg := Configuration{} + if err := defaults.Set(&cfg); err != nil { + return nil, err + } + + s := new(Server) + s.cfg = cfg + + if err := s.UpdateDataStructure(data.Settings, false); err != nil { + return nil, err + } + + s.AddEventListeners() + + // Right now we only support a Docker based environment, so I'm going to hard code + // this logic in. When we're ready to support other environment we'll need to make + // some modifications here obviously. + if err := NewDockerEnvironment(s); err != nil { + return nil, err + } + + s.cache = cache.New(time.Minute*10, time.Minute*15) + s.Archiver = Archiver{ + Server: s, + } + s.Filesystem = Filesystem{ + Configuration: &config.Get().System, + Server: s, + } + + // Forces the configuration to be synced with the panel. + if err := s.SyncWithConfiguration(data); err != nil { + return nil, err + } + + return s, nil +} diff --git a/server/resources.go b/server/resources.go index b42258f..5ded93c 100644 --- a/server/resources.go +++ b/server/resources.go @@ -4,29 +4,37 @@ import ( "github.com/docker/docker/api/types" "math" "sync" + "sync/atomic" ) // Defines the current resource usage for a given server instance. If a server is offline you // should obviously expect memory and CPU usage to be 0. However, disk will always be returned // since that is not dependent on the server being running to collect that data. type ResourceUsage struct { - sync.RWMutex + mu sync.RWMutex + + // The current server status. + State string `json:"state" default:"offline"` // The total amount of memory, in bytes, that this server instance is consuming. This is // calculated slightly differently than just using the raw Memory field that the stats // return from the container, so please check the code setting this value for how that // is calculated. Memory uint64 `json:"memory_bytes"` + // The total amount of memory this container or resource can use. Inside Docker this is // going to be higher than you'd expect because we're automatically allocating overhead // abilities for the container, so its not going to be a perfect match. MemoryLimit uint64 `json:"memory_limit_bytes"` + // The absolute CPU usage is the amount of CPU used in relation to the entire system and // does not take into account any limits on the server process itself. CpuAbsolute float64 `json:"cpu_absolute"` + // The current disk space being used by the server. This is cached to prevent slow lookup // issues on frequent refreshes. Disk int64 `json:"disk_bytes"` + // Current network transmit in & out for a container. Network struct { RxBytes uint64 `json:"rx_bytes"` @@ -34,11 +42,29 @@ type ResourceUsage struct { } `json:"network"` } +// Returns the resource usage stats for the server instance. If the server is not running, only the +// disk space currently used will be returned. When the server is running all of the other stats will +// be returned. +// +// When a process is stopped all of the stats are zeroed out except for the disk. +func (s *Server) Proc() *ResourceUsage { + s.resources.mu.RLock() + defer s.resources.mu.RUnlock() + + return &s.resources +} + +func (ru *ResourceUsage) setInternalState(state string) { + ru.mu.Lock() + ru.State = state + ru.mu.Unlock() +} + // Resets the usages values to zero, used when a server is stopped to ensure we don't hold // onto any values incorrectly. func (ru *ResourceUsage) Empty() { - ru.Lock() - defer ru.Unlock() + ru.mu.Lock() + defer ru.mu.Unlock() ru.Memory = 0 ru.CpuAbsolute = 0 @@ -46,6 +72,27 @@ func (ru *ResourceUsage) Empty() { ru.Network.RxBytes = 0 } +func (ru *ResourceUsage) SetDisk(i int64) { + ru.mu.Lock() + defer ru.mu.Unlock() + + ru.Disk = i +} + +func (ru *ResourceUsage) UpdateFromDocker(v *types.StatsJSON) { + ru.mu.Lock() + defer ru.mu.Unlock() + + ru.CpuAbsolute = ru.calculateDockerAbsoluteCpu(&v.PreCPUStats, &v.CPUStats) + ru.Memory = ru.calculateDockerMemory(v.MemoryStats) + ru.MemoryLimit = v.MemoryStats.Limit +} + +func (ru *ResourceUsage) UpdateNetworkBytes(nw *types.NetworkStats) { + atomic.AddUint64(&ru.Network.RxBytes, nw.RxBytes) + atomic.AddUint64(&ru.Network.TxBytes, nw.TxBytes) +} + // The "docker stats" CLI call does not return the same value as the types.MemoryStats.Usage // value which can be rather confusing to people trying to compare panel usage to // their stats output. @@ -55,7 +102,7 @@ func (ru *ResourceUsage) Empty() { // correct memory value anyways. // // @see https://github.com/docker/cli/blob/96e1d1d6/cli/command/container/stats_helpers.go#L227-L249 -func (ru *ResourceUsage) CalculateDockerMemory(stats types.MemoryStats) uint64 { +func (ru *ResourceUsage) calculateDockerMemory(stats types.MemoryStats) uint64 { if v, ok := stats.Stats["total_inactive_file"]; ok && v < stats.Usage { return stats.Usage - v } @@ -71,7 +118,7 @@ func (ru *ResourceUsage) CalculateDockerMemory(stats types.MemoryStats) uint64 { // by the defined CPU limits on the container. // // @see https://github.com/docker/cli/blob/aa097cf1aa19099da70930460250797c8920b709/cli/command/container/stats_helpers.go#L166 -func (ru *ResourceUsage) CalculateAbsoluteCpu(pStats *types.CPUStats, stats *types.CPUStats) float64 { +func (ru *ResourceUsage) calculateDockerAbsoluteCpu(pStats *types.CPUStats, stats *types.CPUStats) float64 { // Calculate the change in CPU usage between the current and previous reading. cpuDelta := float64(stats.CPUUsage.TotalUsage) - float64(pStats.CPUUsage.TotalUsage) diff --git a/server/server.go b/server/server.go index 85eff71..df0ae60 100644 --- a/server/server.go +++ b/server/server.go @@ -4,99 +4,42 @@ import ( "context" "fmt" "github.com/apex/log" - "github.com/creasty/defaults" "github.com/patrickmn/go-cache" "github.com/pkg/errors" "github.com/pterodactyl/wings/api" - "github.com/pterodactyl/wings/config" - "github.com/remeh/sizedwaitgroup" "golang.org/x/sync/semaphore" - "math" "os" - "strconv" "strings" "sync" "time" ) -var servers *Collection - -func GetServers() *Collection { - return servers -} - -type EnvironmentVariables map[string]interface{} - -// Ugly hacky function to handle environment variables that get passed through as not-a-string -// from the Panel. Ideally we'd just say only pass strings, but that is a fragile idea and if a -// string wasn't passed through you'd cause a crash or the server to become unavailable. For now -// try to handle the most likely values from the JSON and hope for the best. -func (ev EnvironmentVariables) Get(key string) string { - val, ok := ev[key] - if !ok { - return "" - } - - switch val.(type) { - case int: - return strconv.Itoa(val.(int)) - case int32: - return strconv.FormatInt(val.(int64), 10) - case int64: - return strconv.FormatInt(val.(int64), 10) - case float32: - return fmt.Sprintf("%f", val.(float32)) - case float64: - return fmt.Sprintf("%f", val.(float64)) - case bool: - return strconv.FormatBool(val.(bool)) - } - - return val.(string) -} - // High level definition for a server instance being controlled by Wings. type Server struct { + // Internal mutex used to block actions that need to occur sequentially, such as + // writing the configuration to the disk. + sync.RWMutex + // The unique identifier for the server that should be used when referencing // it against the Panel API (and internally). This will be used when naming // docker containers as well as in log output. - Uuid string `json:"uuid"` + Uuid string `json:"-"` - // Whether or not the server is in a suspended state. Suspended servers cannot - // be started or modified except in certain scenarios by an admin user. - Suspended bool `json:"suspended"` + // Maintains the configuration for the server. This is the data that gets returned by the Panel + // such as build settings and container images. + cfg Configuration - // The power state of the server. - State string `default:"offline" json:"state"` - - // The command that should be used when booting up the server instance. - Invocation string `json:"invocation"` - - // An array of environment variables that should be passed along to the running - // server process. - EnvVars EnvironmentVariables `json:"environment"` - - Allocations Allocations `json:"allocations"` - Build BuildSettings `json:"build"` - CrashDetection CrashDetection `json:"crash_detection"` - Mounts []Mount `json:"mounts"` - Resources ResourceUsage `json:"resources"` + // The crash handler for this server instance. + crasher CrashHandler + resources ResourceUsage Archiver Archiver `json:"-"` Environment Environment `json:"-"` Filesystem Filesystem `json:"-"` - Container struct { - // Defines the Docker image that will be used for this server - Image string `json:"image,omitempty"` - // If set to true, OOM killer will be disabled on the server's Docker container. - // If not present (nil) we will default to disabling it. - OomDisabled bool `default:"true" json:"oom_disabled"` - } `json:"container,omitempty"` - // Server cache used to store frequently requested information in memory and make // certain long operations return faster. For example, FS disk space usage. - Cache *cache.Cache `json:"-"` + cache *cache.Cache // Events emitted by the server instance. emitter *EventBus @@ -111,10 +54,6 @@ type Server struct { // installation process, for example when a server is deleted from the panel while the // installer process is still running. installer InstallerDetails - - // Internal mutex used to block actions that need to occur sequentially, such as - // writing the configuration to the disk. - sync.RWMutex } type InstallerDetails struct { @@ -127,190 +66,9 @@ type InstallerDetails struct { sem *semaphore.Weighted } -// The build settings for a given server that impact docker container creation and -// resource limits for a server instance. -type BuildSettings struct { - // The total amount of memory in megabytes that this server is allowed to - // use on the host system. - MemoryLimit int64 `json:"memory_limit"` - - // The amount of additional swap space to be provided to a container instance. - Swap int64 `json:"swap"` - - // The relative weight for IO operations in a container. This is relative to other - // containers on the system and should be a value between 10 and 1000. - IoWeight uint16 `json:"io_weight"` - - // The percentage of CPU that this instance is allowed to consume relative to - // the host. A value of 200% represents complete utilization of two cores. This - // should be a value between 1 and THREAD_COUNT * 100. - CpuLimit int64 `json:"cpu_limit"` - - // The amount of disk space in megabytes that a server is allowed to use. - DiskSpace int64 `json:"disk_space"` - - // Sets which CPU threads can be used by the docker instance. - Threads string `json:"threads"` -} - +// Returns the UUID for the server instance. func (s *Server) Id() string { - s.RLock() - defer s.RUnlock() - - return s.Uuid -} - -// Converts the CPU limit for a server build into a number that can be better understood -// by the Docker environment. If there is no limit set, return -1 which will indicate to -// Docker that it has unlimited CPU quota. -func (b *BuildSettings) ConvertedCpuLimit() int64 { - if b.CpuLimit == 0 { - return -1 - } - - return b.CpuLimit * 1000 -} - -// Set the hard limit for memory usage to be 5% more than the amount of memory assigned to -// the server. If the memory limit for the server is < 4G, use 10%, if less than 2G use -// 15%. This avoids unexpected crashes from processes like Java which run over the limit. -func (b *BuildSettings) MemoryOverheadMultiplier() float64 { - if b.MemoryLimit <= 2048 { - return 1.15 - } else if b.MemoryLimit <= 4096 { - return 1.10 - } - - return 1.05 -} - -func (b *BuildSettings) BoundedMemoryLimit() int64 { - return int64(math.Round(float64(b.MemoryLimit) * b.MemoryOverheadMultiplier() * 1_000_000)) -} - -// Returns the amount of swap available as a total in bytes. This is returned as the amount -// of memory available to the server initially, PLUS the amount of additional swap to include -// which is the format used by Docker. -func (b *BuildSettings) ConvertedSwap() int64 { - if b.Swap < 0 { - return -1 - } - - return (b.Swap * 1_000_000) + b.BoundedMemoryLimit() -} - -// Defines the allocations available for a given server. When using the Docker environment -// driver these correspond to mappings for the container that allow external connections. -type Allocations struct { - // Defines the default allocation that should be used for this server. This is - // what will be used for {SERVER_IP} and {SERVER_PORT} when modifying configuration - // files or the startup arguments for a server. - DefaultMapping struct { - Ip string `json:"ip"` - Port int `json:"port"` - } `json:"default"` - - // Mappings contains all of the ports that should be assigned to a given server - // attached to the IP they correspond to. - Mappings map[string][]int `json:"mappings"` -} - -// Iterates over a given directory and loads all of the servers listed before returning -// them to the calling function. -func LoadDirectory() error { - // We could theoretically use a standard wait group here, however doing - // that introduces the potential to crash the program due to too many - // open files. This wouldn't happen on a small setup, but once the daemon is - // handling many servers you run that risk. - // - // For now just process 10 files at a time, that should be plenty fast to - // read and parse the YAML. We should probably make this configurable down - // the road to help big instances scale better. - wg := sizedwaitgroup.New(10) - - configs, rerr, err := api.NewRequester().GetAllServerConfigurations() - if err != nil || rerr != nil { - if err != nil { - return errors.WithStack(err) - } - - return errors.New(rerr.String()) - } - - states, err := getServerStates() - if err != nil { - return errors.WithStack(err) - } - - servers = NewCollection(nil) - - for uuid, data := range configs { - wg.Add() - - go func(uuid string, data *api.ServerConfigurationResponse) { - defer wg.Done() - - s, err := FromConfiguration(data) - if err != nil { - log.WithField("server", uuid).WithField("error", err).Error("failed to load server, skipping...") - return - } - - if state, exists := states[s.Uuid]; exists { - s.SetState(state) - s.Log().WithField("state", s.GetState()).Debug("loaded server state from cache file") - } - - servers.Add(s) - }(uuid, data) - } - - // Wait until we've processed all of the configuration files in the directory - // before continuing. - wg.Wait() - - return nil -} - -// Initializes a server using a data byte array. This will be marshaled into the -// given struct using a YAML marshaler. This will also configure the given environment -// for a server. -func FromConfiguration(data *api.ServerConfigurationResponse) (*Server, error) { - s := new(Server) - - if err := defaults.Set(s); err != nil { - return nil, err - } - - if err := s.UpdateDataStructure(data.Settings, false); err != nil { - return nil, err - } - - s.AddEventListeners() - - // Right now we only support a Docker based environment, so I'm going to hard code - // this logic in. When we're ready to support other environment we'll need to make - // some modifications here obviously. - if err := NewDockerEnvironment(s); err != nil { - return nil, err - } - - s.Cache = cache.New(time.Minute*10, time.Minute*15) - s.Archiver = Archiver{ - Server: s, - } - s.Filesystem = Filesystem{ - Configuration: &config.Get().System, - Server: s, - } - s.Resources = ResourceUsage{} - - // Forces the configuration to be synced with the panel. - if err := s.SyncWithConfiguration(data); err != nil { - return nil, err - } - - return s, nil + return s.Config().Uuid } // Returns all of the environment variables that should be assigned to a running @@ -320,21 +78,21 @@ func (s *Server) GetEnvironmentVariables() []string { var out = []string{ fmt.Sprintf("TZ=%s", zone), - fmt.Sprintf("STARTUP=%s", s.Invocation), - fmt.Sprintf("SERVER_MEMORY=%d", s.Build.MemoryLimit), - fmt.Sprintf("SERVER_IP=%s", s.Allocations.DefaultMapping.Ip), - fmt.Sprintf("SERVER_PORT=%d", s.Allocations.DefaultMapping.Port), + fmt.Sprintf("STARTUP=%s", s.Config().Invocation), + fmt.Sprintf("SERVER_MEMORY=%d", s.Build().MemoryLimit), + fmt.Sprintf("SERVER_IP=%s", s.Config().Allocations.DefaultMapping.Ip), + fmt.Sprintf("SERVER_PORT=%d", s.Config().Allocations.DefaultMapping.Port), } eloop: - for k := range s.EnvVars { + for k := range s.Config().EnvVars { for _, e := range out { if strings.HasPrefix(e, strings.ToUpper(k)) { continue eloop } } - out = append(out, fmt.Sprintf("%s=%s", strings.ToUpper(k), s.EnvVars.Get(k))) + out = append(out, fmt.Sprintf("%s=%s", strings.ToUpper(k), s.Config().EnvVars.Get(k))) } return out @@ -420,3 +178,8 @@ func (s *Server) HandlePowerAction(action PowerAction) error { return errors.New("an invalid power action was provided") } } + +// Checks if the server is marked as being suspended or not on the system. +func (s *Server) IsSuspended() bool { + return s.Config().Suspended +} diff --git a/server/state.go b/server/state.go index 033e267..aa24306 100644 --- a/server/state.go +++ b/server/state.go @@ -13,6 +13,13 @@ import ( var stateMutex sync.Mutex +const ( + ProcessOfflineState = "offline" + ProcessStartingState = "starting" + ProcessRunningState = "running" + ProcessStoppingState = "stopping" +) + // Returns the state of the servers. func getServerStates() (map[string]string, error) { // Request a lock after we check if the file exists. @@ -60,13 +67,6 @@ func saveServerStates() error { return nil } -const ( - ProcessOfflineState = "offline" - ProcessStartingState = "starting" - ProcessRunningState = "running" - ProcessStoppingState = "stopping" -) - // Sets the state of the server internally. This function handles crash detection as // well as reporting to event listeners for the server. func (s *Server) SetState(state string) error { @@ -76,16 +76,14 @@ func (s *Server) SetState(state string) error { prevState := s.GetState() - // Obtain a mutex lock and update the current state of the server. - s.Lock() - s.State = state + // Update the currently tracked state for the server. + s.Proc().setInternalState(state) // Emit the event to any listeners that are currently registered. - s.Log().WithField("status", s.State).Debug("saw server status change event") - s.Events().Publish(StatusEvent, s.State) - - // Release the lock as it is no longer needed for the following actions. - s.Unlock() + if prevState != state { + s.Log().WithField("status", s.Proc().State).Debug("saw server status change event") + s.Events().Publish(StatusEvent, s.Proc().State) + } // Persist this change to the disk immediately so that should the Daemon be stopped or // crash we can immediately restore the server state. @@ -128,10 +126,7 @@ func (s *Server) SetState(state string) error { // Returns the current state of the server in a race-safe manner. func (s *Server) GetState() string { - s.RLock() - defer s.RUnlock() - - return s.State + return s.Proc().State } // Determines if the server state is running or not. This is different than the diff --git a/server/update.go b/server/update.go index 00d9eed..750da12 100644 --- a/server/update.go +++ b/server/update.go @@ -15,7 +15,7 @@ import ( // it is up to the specific environment to determine what needs to happen when // that is the case. func (s *Server) UpdateDataStructure(data []byte, background bool) error { - src := new(Server) + src := new(Configuration) if err := json.Unmarshal(data, src); err != nil { return errors.WithStack(err) } @@ -27,26 +27,30 @@ func (s *Server) UpdateDataStructure(data []byte, background bool) error { return errors.New("attempting to merge a data stack with an invalid UUID") } - s.Lock() + // Grab a copy of the configuration to work on. + c := s.Config() + + // Lock the server configuration while we're doing this merge to avoid anything + // trying to overwrite it or make modifications while we're sorting out what we + // need to do. + s.cfg.mu.Lock() + defer s.cfg.mu.Unlock() + // Merge the new data object that we have received with the existing server data object // and then save it to the disk so it is persistent. - if err := mergo.Merge(s, src, mergo.WithOverride); err != nil { + if err := mergo.Merge(c, src, mergo.WithOverride); err != nil { return errors.WithStack(err) } - // Mergo makes that previous lock disappear. Handle that by just re-locking the object. - s.Lock() - defer s.Unlock() - // Don't explode if we're setting CPU limits to 0. Mergo sees that as an empty value // so it won't override the value we've passed through in the API call. However, we can // safely assume that we're passing through valid data structures here. I foresee this // backfiring at some point, but until then... // // We'll go ahead and do this with swap as well. - s.Build.CpuLimit = src.Build.CpuLimit - s.Build.Swap = src.Build.Swap - s.Build.DiskSpace = src.Build.DiskSpace + c.Build.CpuLimit = src.Build.CpuLimit + c.Build.Swap = src.Build.Swap + c.Build.DiskSpace = src.Build.DiskSpace // Mergo can't quite handle this boolean value correctly, so for now we'll just // handle this edge case manually since none of the other data passed through in this @@ -56,7 +60,7 @@ func (s *Server) UpdateDataStructure(data []byte, background bool) error { return errors.WithStack(err) } } else { - s.Container.OomDisabled = v + c.Container.OomDisabled = v } // Mergo also cannot handle this boolean value. @@ -65,25 +69,29 @@ func (s *Server) UpdateDataStructure(data []byte, background bool) error { return errors.WithStack(err) } } else { - s.Suspended = v + c.Suspended = v } // Environment and Mappings should be treated as a full update at all times, never a // true patch, otherwise we can't know what we're passing along. if src.EnvVars != nil && len(src.EnvVars) > 0 { - s.EnvVars = src.EnvVars + c.EnvVars = src.EnvVars } if src.Allocations.Mappings != nil && len(src.Allocations.Mappings) > 0 { - s.Allocations.Mappings = src.Allocations.Mappings + c.Allocations.Mappings = src.Allocations.Mappings } if src.Mounts != nil && len(src.Mounts) > 0 { - s.Mounts = src.Mounts + c.Mounts = src.Mounts } + // Update the configuration once we have a lock on the configuration object. + s.cfg = *c + s.Uuid = c.Uuid + if background { - s.runBackgroundActions() + go s.runBackgroundActions() } return nil @@ -96,24 +104,22 @@ func (s *Server) UpdateDataStructure(data []byte, background bool) error { // These tasks run in independent threads where relevant to speed up any updates // that need to happen. func (s *Server) runBackgroundActions() { - // Update the environment in place, allowing memory and CPU usage to be adjusted - // on the fly without the user needing to reboot (theoretically). - go func(server *Server) { - server.Log().Info("performing server limit modification on-the-fly") - if err := server.Environment.InSituUpdate(); err != nil { - server.Log().WithField("error", err).Warn("failed to perform on-the-fly update of the server environment") - } - }(s) - - // Check if the server is now suspended, and if so and the process is not terminated + // Check if the s is now suspended, and if so and the process is not terminated // yet, do it immediately. - go func(server *Server) { - if server.Suspended && server.GetState() != ProcessOfflineState { - server.Log().Info("server suspended with running process state, terminating now") + if s.IsSuspended() && s.GetState() != ProcessOfflineState { + s.Log().Info("server suspended with running process state, terminating now") - if err := server.Environment.WaitForStop(10, true); err != nil { - server.Log().WithField("error", err).Warn("failed to terminate server environment after suspension") - } + if err := s.Environment.WaitForStop(10, true); err != nil { + s.Log().WithField("error", err).Warn("failed to terminate server environment after suspension") } - }(s) + } + + if !s.IsSuspended() { + // Update the environment in place, allowing memory and CPU usage to be adjusted + // on the fly without the user needing to reboot (theoretically). + s.Log().Info("performing server limit modification on-the-fly") + if err := s.Environment.InSituUpdate(); err != nil { + s.Log().WithField("error", err).Warn("failed to perform on-the-fly update of the server environment") + } + } } From 16467fa7ffe17dcc502b5a35fb88c4a6a8bb2a78 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 19 Jul 2020 17:09:38 -0700 Subject: [PATCH 25/37] Code cleanup --- installer/installer.go | 2 +- server/loader.go | 13 +++++++++---- server/state.go | 4 +++- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/installer/installer.go b/installer/installer.go index 6ca95fd..ddebdf2 100644 --- a/installer/installer.go +++ b/installer/installer.go @@ -80,7 +80,7 @@ func New(data []byte) (*Installer, error) { // Create a new server instance using the configuration we wrote to the disk // so that everything gets instantiated correctly on the struct. - s, err := server.FromConfiguration(c) + s, err := server.FromConfiguration(c, true) return &Installer{ server: s, diff --git a/server/loader.go b/server/loader.go index 55b1a32..a98acba 100644 --- a/server/loader.go +++ b/server/loader.go @@ -43,18 +43,21 @@ func LoadDirectory() error { return errors.New(rerr.String()) } + log.Debug("retrieving cached server states from disk") states, err := getServerStates() if err != nil { return errors.WithStack(err) } + log.WithField("total_configs", len(configs)).Debug("looping over received configurations from API") for uuid, data := range configs { wg.Add() go func(uuid string, data *api.ServerConfigurationResponse) { defer wg.Done() - s, err := FromConfiguration(data) + log.WithField("uuid", uuid).Debug("creating server object from configuration") + s, err := FromConfiguration(data, false) if err != nil { log.WithField("server", uuid).WithField("error", err).Error("failed to load server, skipping...") return @@ -79,7 +82,7 @@ func LoadDirectory() error { // Initializes a server using a data byte array. This will be marshaled into the // given struct using a YAML marshaler. This will also configure the given environment // for a server. -func FromConfiguration(data *api.ServerConfigurationResponse) (*Server, error) { +func FromConfiguration(data *api.ServerConfigurationResponse, sync bool) (*Server, error) { cfg := Configuration{} if err := defaults.Set(&cfg); err != nil { return nil, err @@ -111,8 +114,10 @@ func FromConfiguration(data *api.ServerConfigurationResponse) (*Server, error) { } // Forces the configuration to be synced with the panel. - if err := s.SyncWithConfiguration(data); err != nil { - return nil, err + if sync { + if err := s.SyncWithConfiguration(data); err != nil { + return nil, err + } } return s, nil diff --git a/server/state.go b/server/state.go index aa24306..705d524 100644 --- a/server/state.go +++ b/server/state.go @@ -133,5 +133,7 @@ func (s *Server) GetState() string { // environment state, it is simply the tracked state from this daemon instance, and // not the response from Docker. func (s *Server) IsRunning() bool { - return s.GetState() == ProcessRunningState || s.GetState() == ProcessStartingState + st := s.GetState() + + return st == ProcessRunningState || st == ProcessStartingState } From 21e58b57a1349a40255b0562cad7cbe684ffcbad Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 19 Jul 2020 17:26:53 -0700 Subject: [PATCH 26/37] Whoops, sync correctly --- installer/installer.go | 2 +- server/loader.go | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/installer/installer.go b/installer/installer.go index ddebdf2..6ca95fd 100644 --- a/installer/installer.go +++ b/installer/installer.go @@ -80,7 +80,7 @@ func New(data []byte) (*Installer, error) { // Create a new server instance using the configuration we wrote to the disk // so that everything gets instantiated correctly on the struct. - s, err := server.FromConfiguration(c, true) + s, err := server.FromConfiguration(c) return &Installer{ server: s, diff --git a/server/loader.go b/server/loader.go index a98acba..9299276 100644 --- a/server/loader.go +++ b/server/loader.go @@ -57,7 +57,7 @@ func LoadDirectory() error { defer wg.Done() log.WithField("uuid", uuid).Debug("creating server object from configuration") - s, err := FromConfiguration(data, false) + s, err := FromConfiguration(data) if err != nil { log.WithField("server", uuid).WithField("error", err).Error("failed to load server, skipping...") return @@ -82,7 +82,7 @@ func LoadDirectory() error { // Initializes a server using a data byte array. This will be marshaled into the // given struct using a YAML marshaler. This will also configure the given environment // for a server. -func FromConfiguration(data *api.ServerConfigurationResponse, sync bool) (*Server, error) { +func FromConfiguration(data *api.ServerConfigurationResponse) (*Server, error) { cfg := Configuration{} if err := defaults.Set(&cfg); err != nil { return nil, err @@ -114,10 +114,8 @@ func FromConfiguration(data *api.ServerConfigurationResponse, sync bool) (*Serve } // Forces the configuration to be synced with the panel. - if sync { - if err := s.SyncWithConfiguration(data); err != nil { - return nil, err - } + if err := s.SyncWithConfiguration(data); err != nil { + return nil, err } return s, nil From e28c05ae56c2b92b8a6e43a8b9248f541da1c31a Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 19 Jul 2020 17:46:39 -0700 Subject: [PATCH 27/37] Address some race conditions --- server/configuration.go | 7 +++++++ server/resources.go | 9 +++++++++ server/server.go | 2 +- server/state.go | 2 +- server/update.go | 13 ++++++++++--- 5 files changed, 28 insertions(+), 5 deletions(-) diff --git a/server/configuration.go b/server/configuration.go index 3cf39b4..11ac4bb 100644 --- a/server/configuration.go +++ b/server/configuration.go @@ -77,6 +77,13 @@ func (s *Server) Config() *Configuration { return &s.cfg } +func (c *Configuration) GetUuid() string { + c.mu.RLock() + defer c.mu.RUnlock() + + return c.Uuid +} + func (c *Configuration) SetSuspended(s bool) { c.mu.Lock() c.Suspended = s diff --git a/server/resources.go b/server/resources.go index 5ded93c..f333cb8 100644 --- a/server/resources.go +++ b/server/resources.go @@ -54,6 +54,15 @@ func (s *Server) Proc() *ResourceUsage { return &s.resources } +// Returns the servers current state. +func (ru *ResourceUsage) getInternalState() string { + ru.mu.RLock() + defer ru.mu.RUnlock() + + return ru.State +} + +// Sets the new state for the server. func (ru *ResourceUsage) setInternalState(state string) { ru.mu.Lock() ru.State = state diff --git a/server/server.go b/server/server.go index df0ae60..6e10f1a 100644 --- a/server/server.go +++ b/server/server.go @@ -68,7 +68,7 @@ type InstallerDetails struct { // Returns the UUID for the server instance. func (s *Server) Id() string { - return s.Config().Uuid + return s.Config().GetUuid() } // Returns all of the environment variables that should be assigned to a running diff --git a/server/state.go b/server/state.go index 705d524..5ff4ab9 100644 --- a/server/state.go +++ b/server/state.go @@ -126,7 +126,7 @@ func (s *Server) SetState(state string) error { // Returns the current state of the server in a race-safe manner. func (s *Server) GetState() string { - return s.Proc().State + return s.Proc().getInternalState() } // Determines if the server state is running or not. This is different than the diff --git a/server/update.go b/server/update.go index 750da12..5d9a211 100644 --- a/server/update.go +++ b/server/update.go @@ -28,7 +28,14 @@ func (s *Server) UpdateDataStructure(data []byte, background bool) error { } // Grab a copy of the configuration to work on. - c := s.Config() + c := *s.Config() + + // Lock our copy of the configuration since the defered unlock will end up acting upon this + // new memory address rather than the old one. If we don't lock this, the defered unlock will + // cause a panic when it goes to run. However, since we only update s.cfg at the end, if there + // is an error before that point we'll still properly unlock the original configuration for the + // server. + c.mu.Lock() // Lock the server configuration while we're doing this merge to avoid anything // trying to overwrite it or make modifications while we're sorting out what we @@ -38,7 +45,7 @@ func (s *Server) UpdateDataStructure(data []byte, background bool) error { // Merge the new data object that we have received with the existing server data object // and then save it to the disk so it is persistent. - if err := mergo.Merge(c, src, mergo.WithOverride); err != nil { + if err := mergo.Merge(&c, src, mergo.WithOverride); err != nil { return errors.WithStack(err) } @@ -87,7 +94,7 @@ func (s *Server) UpdateDataStructure(data []byte, background bool) error { } // Update the configuration once we have a lock on the configuration object. - s.cfg = *c + s.cfg = c s.Uuid = c.Uuid if background { From 5079c67aee9ae97de84c95338f0658ddf2739ee2 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 19 Jul 2020 17:50:39 -0700 Subject: [PATCH 28/37] Code cleanup and avoid server race --- server/filesystem.go | 7 +++---- server/loader.go | 2 -- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/server/filesystem.go b/server/filesystem.go index b2115cd..4c1ea69 100644 --- a/server/filesystem.go +++ b/server/filesystem.go @@ -41,13 +41,12 @@ func IsPathResolutionError(err error) bool { type Filesystem struct { Server *Server - Configuration *config.SystemConfiguration cacheDiskMu sync.Mutex } // Returns the root path that contains all of a server's data. func (fs *Filesystem) Path() string { - return filepath.Join(fs.Configuration.Data, fs.Server.Uuid) + return filepath.Join(config.Get().System.Data, fs.Server.Id()) } // Normalizes a directory being passed in to ensure the user is not able to escape @@ -475,7 +474,7 @@ func (fs *Filesystem) Chown(path string) error { if s, err := os.Stat(cleaned); err != nil { return errors.WithStack(err) } else if !s.IsDir() { - return os.Chown(cleaned, fs.Configuration.User.Uid, fs.Configuration.User.Gid) + return os.Chown(cleaned, config.Get().System.User.Uid, config.Get().System.User.Gid) } return fs.chownDirectory(cleaned) @@ -521,7 +520,7 @@ func (fs *Filesystem) chownDirectory(path string) error { fs.chownDirectory(p) }(p) } else { - os.Chown(p, fs.Configuration.User.Uid, fs.Configuration.User.Gid) + os.Chown(p, config.Get().System.User.Uid, config.Get().System.User.Gid) } } diff --git a/server/loader.go b/server/loader.go index 9299276..70a7d30 100644 --- a/server/loader.go +++ b/server/loader.go @@ -6,7 +6,6 @@ import ( "github.com/patrickmn/go-cache" "github.com/pkg/errors" "github.com/pterodactyl/wings/api" - "github.com/pterodactyl/wings/config" "github.com/remeh/sizedwaitgroup" "time" ) @@ -109,7 +108,6 @@ func FromConfiguration(data *api.ServerConfigurationResponse) (*Server, error) { Server: s, } s.Filesystem = Filesystem{ - Configuration: &config.Get().System, Server: s, } From cb850fd81ab9f0af75f63835661816a6baed9eef Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 19 Jul 2020 17:53:41 -0700 Subject: [PATCH 29/37] Update all of the old UUID refs to new --- cmd/root.go | 2 +- installer/installer.go | 2 +- router/middleware.go | 2 +- router/router_server.go | 4 ++-- router/router_transfer.go | 12 ++++++------ server/archiver.go | 2 +- server/install.go | 14 +++++++------- server/server.go | 9 ++------- server/state.go | 2 +- server/update.go | 3 +-- sftp/server.go | 6 +++--- 11 files changed, 26 insertions(+), 32 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index d992e19..ffbf96f 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -169,7 +169,7 @@ func rootCmdRun(*cobra.Command, []string) { // Just for some nice log output. for _, s := range server.GetServers().All() { - log.WithField("server", s.Uuid).Info("loaded configuration for server") + log.WithField("server", s.Id()).Info("loaded configuration for server") } // Create a new WaitGroup that limits us to 4 servers being bootstrapped at a time diff --git a/installer/installer.go b/installer/installer.go index 6ca95fd..92ba655 100644 --- a/installer/installer.go +++ b/installer/installer.go @@ -89,7 +89,7 @@ func New(data []byte) (*Installer, error) { // Returns the UUID associated with this installer instance. func (i *Installer) Uuid() string { - return i.server.Uuid + return i.server.Id() } // Return the server instance. diff --git a/router/middleware.go b/router/middleware.go index 45c50f8..102d1aa 100644 --- a/router/middleware.go +++ b/router/middleware.go @@ -48,7 +48,7 @@ func AuthorizationMiddleware(c *gin.Context) { // Helper function to fetch a server out of the servers collection stored in memory. func GetServer(uuid string) *server.Server { return server.GetServers().Find(func(s *server.Server) bool { - return uuid == s.Uuid + return uuid == s.Id() }) } diff --git a/router/router_server.go b/router/router_server.go index d8c47a4..ac2bf60 100644 --- a/router/router_server.go +++ b/router/router_server.go @@ -200,9 +200,9 @@ func deleteServer(c *gin.Context) { } }(s.Filesystem.Path()) - var uuid = s.Uuid + var uuid = s.Id() server.GetServers().Remove(func(s2 *server.Server) bool { - return s2.Uuid == uuid + return s2.Id() == uuid }) // Deallocate the reference to this server. diff --git a/router/router_transfer.go b/router/router_transfer.go index 0a1778f..cf3bece 100644 --- a/router/router_transfer.go +++ b/router/router_transfer.go @@ -98,33 +98,33 @@ func postServerArchive(c *gin.Context) { start := time.Now() if err := server.Archiver.Archive(); err != nil { - zap.S().Errorw("failed to get archive for server", zap.String("server", s.Uuid), zap.Error(err)) + zap.S().Errorw("failed to get archive for server", zap.String("server", server.Id()), zap.Error(err)) return } zap.S().Debugw( "successfully created archive for server", - zap.String("server", server.Uuid), + zap.String("server", server.Id()), zap.Duration("time", time.Now().Sub(start).Round(time.Microsecond)), ) r := api.NewRequester() - rerr, err := r.SendArchiveStatus(server.Uuid, true) + rerr, err := r.SendArchiveStatus(server.Id(), true) if rerr != nil || err != nil { if err != nil { - zap.S().Errorw("failed to notify panel with archive status", zap.String("server", server.Uuid), zap.Error(err)) + zap.S().Errorw("failed to notify panel with archive status", zap.String("server", server.Id()), zap.Error(err)) return } zap.S().Errorw( "panel returned an error when sending the archive status", - zap.String("server", server.Uuid), + zap.String("server", server.Id()), zap.Error(errors.New(rerr.String())), ) return } - zap.S().Debugw("successfully notified panel about archive status", zap.String("server", server.Uuid)) + zap.S().Debugw("successfully notified panel about archive status", zap.String("server", server.Id())) }(s) c.Status(http.StatusAccepted) diff --git a/server/archiver.go b/server/archiver.go index d10dfce..d177d76 100644 --- a/server/archiver.go +++ b/server/archiver.go @@ -23,7 +23,7 @@ func (a *Archiver) ArchivePath() string { // ArchiveName returns the name of the server's archive. func (a *Archiver) ArchiveName() string { - return a.Server.Uuid + ".tar.gz" + return a.Server.Id() + ".tar.gz" } // Exists returns a boolean based off if the archive exists. diff --git a/server/install.go b/server/install.go index 6a7805e..5974ab9 100644 --- a/server/install.go +++ b/server/install.go @@ -70,7 +70,7 @@ func (s *Server) Reinstall() error { // Internal installation function used to simplify reporting back to the Panel. func (s *Server) internalInstall() error { - script, rerr, err := api.NewRequester().GetInstallationScript(s.Uuid) + script, rerr, err := api.NewRequester().GetInstallationScript(s.Id()) if err != nil || rerr != nil { if err != nil { return err @@ -170,7 +170,7 @@ func (s *Server) AbortInstallation() { // Removes the installer container for the server. func (ip *InstallationProcess) RemoveContainer() { - err := ip.client.ContainerRemove(ip.context, ip.Server.Uuid+"_installer", types.ContainerRemoveOptions{ + err := ip.client.ContainerRemove(ip.context, ip.Server.Id()+"_installer", types.ContainerRemoveOptions{ RemoveVolumes: true, Force: true, }) @@ -313,7 +313,7 @@ func (ip *InstallationProcess) BeforeExecute() (string, error) { Force: true, } - if err := ip.client.ContainerRemove(ip.context, ip.Server.Uuid+"_installer", opts); err != nil { + if err := ip.client.ContainerRemove(ip.context, ip.Server.Id()+"_installer", opts); err != nil { if !client.IsErrNotFound(err) { e = append(e, err) } @@ -333,7 +333,7 @@ func (ip *InstallationProcess) BeforeExecute() (string, error) { // Returns the log path for the installation process. func (ip *InstallationProcess) GetLogPath() string { - return filepath.Join(config.Get().System.GetInstallLogPath(), ip.Server.Uuid+".log") + return filepath.Join(config.Get().System.GetInstallLogPath(), ip.Server.Id()+".log") } // Cleans up after the execution of the installation process. This grabs the logs from the @@ -369,7 +369,7 @@ func (ip *InstallationProcess) AfterExecute(containerId string) error { | | Details | ------------------------------ - Server UUID: {{.Server.Uuid}} + Server UUID: {{.Server.Id()}} Container Image: {{.Script.ContainerImage}} Container Entrypoint: {{.Script.Entrypoint}} @@ -448,7 +448,7 @@ func (ip *InstallationProcess) Execute(installPath string) (string, error) { } ip.Server.Log().WithField("install_script", installPath+"/install.sh").Info("creating install container for server process") - r, err := ip.client.ContainerCreate(ip.context, conf, hostConf, nil, ip.Server.Uuid+"_installer") + r, err := ip.client.ContainerCreate(ip.context, conf, hostConf, nil, ip.Server.Id()+"_installer") if err != nil { return "", errors.WithStack(err) } @@ -516,7 +516,7 @@ func (ip *InstallationProcess) StreamOutput(id string) error { func (s *Server) SyncInstallState(successful bool) error { r := api.NewRequester() - rerr, err := r.SendInstallationStatus(s.Uuid, successful) + rerr, err := r.SendInstallationStatus(s.Id(), successful) if rerr != nil || err != nil { if err != nil { return errors.WithStack(err) diff --git a/server/server.go b/server/server.go index 6e10f1a..e07a340 100644 --- a/server/server.go +++ b/server/server.go @@ -20,11 +20,6 @@ type Server struct { // writing the configuration to the disk. sync.RWMutex - // The unique identifier for the server that should be used when referencing - // it against the Panel API (and internally). This will be used when naming - // docker containers as well as in log output. - Uuid string `json:"-"` - // Maintains the configuration for the server. This is the data that gets returned by the Panel // such as build settings and container images. cfg Configuration @@ -99,7 +94,7 @@ eloop: } func (s *Server) Log() *log.Entry { - return log.WithField("server", s.Uuid) + return log.WithField("server", s.Id()) } // Syncs the state of the server on the Panel with Wings. This ensures that we're always @@ -159,7 +154,7 @@ func (s *Server) CreateEnvironment() error { // Gets the process configuration data for the server. func (s *Server) GetProcessConfiguration() (*api.ServerConfigurationResponse, *api.RequestError, error) { - return api.NewRequester().GetServerConfiguration(s.Uuid) + return api.NewRequester().GetServerConfiguration(s.Id()) } // Helper function that can receieve a power action and then process the diff --git a/server/state.go b/server/state.go index 5ff4ab9..c47c4e6 100644 --- a/server/state.go +++ b/server/state.go @@ -47,7 +47,7 @@ func saveServerStates() error { // Get the states of all servers on the daemon. states := map[string]string{} for _, s := range GetServers().All() { - states[s.Uuid] = s.GetState() + states[s.Id()] = s.GetState() } // Convert the map to a json object. diff --git a/server/update.go b/server/update.go index 5d9a211..7ab63ba 100644 --- a/server/update.go +++ b/server/update.go @@ -23,7 +23,7 @@ func (s *Server) UpdateDataStructure(data []byte, background bool) error { // Don't allow obviously corrupted data to pass through into this function. If the UUID // doesn't match something has gone wrong and the API is attempting to meld this server // instance into a totally different one, which would be bad. - if src.Uuid != "" && s.Uuid != "" && src.Uuid != s.Uuid { + if src.Uuid != "" && s.Id() != "" && src.Uuid != s.Id() { return errors.New("attempting to merge a data stack with an invalid UUID") } @@ -95,7 +95,6 @@ func (s *Server) UpdateDataStructure(data []byte, background bool) error { // Update the configuration once we have a lock on the configuration object. s.cfg = c - s.Uuid = c.Uuid if background { go s.runBackgroundActions() diff --git a/sftp/server.go b/sftp/server.go index 8a9f243..44617a8 100644 --- a/sftp/server.go +++ b/sftp/server.go @@ -49,7 +49,7 @@ func Initialize(config *config.Configuration) error { func validatePath(fs sftp_server.FileSystem, p string) (string, error) { s := server.GetServers().Find(func(server *server.Server) bool { - return server.Uuid == fs.UUID + return server.Id() == fs.UUID }) if s == nil { @@ -61,7 +61,7 @@ func validatePath(fs sftp_server.FileSystem, p string) (string, error) { func validateDiskSpace(fs sftp_server.FileSystem) bool { s := server.GetServers().Find(func(server *server.Server) bool { - return server.Uuid == fs.UUID + return server.Id() == fs.UUID }) if s == nil { @@ -105,7 +105,7 @@ func validateCredentials(c sftp_server.AuthenticationRequest) (*sftp_server.Auth } s := server.GetServers().Find(func(server *server.Server) bool { - return server.Uuid == resp.Server + return server.Id() == resp.Server }) if s == nil { From f567c2c15ccc302b87dd99281c4c3079d4e5648d Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 19 Jul 2020 18:40:01 -0700 Subject: [PATCH 30/37] Use the right files --- .github/workflows/release.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 20fe3b7..9734bd5 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -32,7 +32,6 @@ jobs: sed -n "/^## ${REF:10}/,/^## /{/^## /b;p}" CHANGELOG.md > ./RELEASE_CHANGELOG echo ::set-output name=version_name::`sed -nr "s/^## (${REF:10} .*)$/\1/p" CHANGELOG.md` - - name: Create checksum and add to changelog run: | SUM=`cd build && sha256sum wings_linux_amd64` @@ -48,8 +47,8 @@ jobs: git config --local user.name "Pterodactyl CI" git checkout -b $BRANCH git push -u origin $BRANCH - sed -i "s/ Version = \".*\"/ Version = \"${REF:11}\"/" config/app.php - git add config/app.php + sed -i "s/ Version = \".*\"/ Version = \"${REF:11}\"/" system/const.go + git add system/const.go git commit -m "bump version for release" git push From 4d8f06a3e032d5fe2ee6936b4c596dd630369d8b Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Sun, 19 Jul 2020 19:16:01 -0700 Subject: [PATCH 31/37] Use brute --- .github/workflows/release.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 9734bd5..4360d3f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -23,7 +23,7 @@ jobs: run: go test ./... - name: Compress binary and make it executable - run: upx build/wings_linux_amd64 && chmod +x build/wings_linux_amd64 + run: upx --brute build/wings_linux_amd64 && chmod +x build/wings_linux_amd64 - name: Extract changelog env: From 79ee259874690bf206c2771d6dcd78a49bd50a34 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Wed, 29 Jul 2020 20:34:30 -0700 Subject: [PATCH 32/37] correctly return server resource stats; closes pterodactyl/panel#2183 --- router/router_server.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/router/router_server.go b/router/router_server.go index ac2bf60..6af62d0 100644 --- a/router/router_server.go +++ b/router/router_server.go @@ -13,7 +13,9 @@ import ( // Returns a single server from the collection of servers. func getServer(c *gin.Context) { - c.JSON(http.StatusOK, GetServer(c.Param("server"))) + s := GetServer(c.Param("server")) + + c.JSON(http.StatusOK, s.Proc()) } // Returns the logs for a given server instance. From db0dc179375cf6594b001445b37b16c793982ea3 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Wed, 29 Jul 2020 20:54:15 -0700 Subject: [PATCH 33/37] Fix exception when writing install logs --- server/install.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/install.go b/server/install.go index 5974ab9..0d8fa93 100644 --- a/server/install.go +++ b/server/install.go @@ -369,7 +369,7 @@ func (ip *InstallationProcess) AfterExecute(containerId string) error { | | Details | ------------------------------ - Server UUID: {{.Server.Id()}} + Server UUID: {{.Server.Id}} Container Image: {{.Script.ContainerImage}} Container Entrypoint: {{.Script.Entrypoint}} From 0b761320cce31b58412f8723da100ab1b0a00f17 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Wed, 29 Jul 2020 20:54:26 -0700 Subject: [PATCH 34/37] Fix error handling to be more accurate in the stacks --- loggers/cli/cli.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/loggers/cli/cli.go b/loggers/cli/cli.go index bb673b0..af71c31 100644 --- a/loggers/cli/cli.go +++ b/loggers/cli/cli.go @@ -81,7 +81,7 @@ func (h *Handler) HandleLog(e *log.Entry) error { } func getErrorStack(err error, i bool) errors.StackTrace { - e, ok := errors.Cause(err).(tracer) + e, ok := err.(tracer) if !ok { if i { // Just abort out of this and return a stacktrace leading up to this point. It isn't perfect @@ -89,7 +89,7 @@ func getErrorStack(err error, i bool) errors.StackTrace { return errors.Wrap(err, "failed to generate stacktrace for caught error").(tracer).StackTrace() } - return getErrorStack(errors.New(err.Error()), true) + return getErrorStack(errors.Wrap(err, err.Error()), true) } st := e.StackTrace() From f0d6f67c6b0b400fcde261398944860fda74f2a2 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Wed, 29 Jul 2020 21:39:17 -0700 Subject: [PATCH 35/37] Fix memory leak with websocket not removing unused listeners --- router/websocket/listeners.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/router/websocket/listeners.go b/router/websocket/listeners.go index bf872c6..124a85d 100644 --- a/router/websocket/listeners.go +++ b/router/websocket/listeners.go @@ -52,16 +52,15 @@ func (h *Handler) ListenForServerEvents(ctx context.Context) { h.server.Events().Subscribe(event, eventChannel) } - select { - case <-ctx.Done(): - for _, event := range events { - h.server.Events().Unsubscribe(event, eventChannel) - } + for d := range eventChannel { + select { + case <-ctx.Done(): + for _, event := range events { + h.server.Events().Unsubscribe(event, eventChannel) + } - close(eventChannel) - default: - // Listen for different events emitted by the server and respond to them appropriately. - for d := range eventChannel { + close(eventChannel) + default: h.SendJson(&Message{ Event: d.Topic, Args: []string{d.Data}, From 7f9ec4402abfc94d38d878493e44046f03cabf9e Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Wed, 29 Jul 2020 21:39:27 -0700 Subject: [PATCH 36/37] Add emitters for install started/stopped --- router/websocket/listeners.go | 2 ++ server/events.go | 14 ++++++++------ server/install.go | 7 +++++++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/router/websocket/listeners.go b/router/websocket/listeners.go index 124a85d..b1a99ac 100644 --- a/router/websocket/listeners.go +++ b/router/websocket/listeners.go @@ -43,6 +43,8 @@ func (h *Handler) ListenForServerEvents(ctx context.Context) { server.StatusEvent, server.ConsoleOutputEvent, server.InstallOutputEvent, + server.InstallStartedEvent, + server.InstallCompletedEvent, server.DaemonMessageEvent, server.BackupCompletedEvent, } diff --git a/server/events.go b/server/events.go index 5ea6ceb..425de25 100644 --- a/server/events.go +++ b/server/events.go @@ -9,12 +9,14 @@ import ( // Defines all of the possible output events for a server. // noinspection GoNameStartsWithPackageName const ( - DaemonMessageEvent = "daemon message" - InstallOutputEvent = "install output" - ConsoleOutputEvent = "console output" - StatusEvent = "status" - StatsEvent = "stats" - BackupCompletedEvent = "backup completed" + DaemonMessageEvent = "daemon message" + InstallOutputEvent = "install output" + InstallStartedEvent = "install started" + InstallCompletedEvent = "install completed" + ConsoleOutputEvent = "console output" + StatusEvent = "status" + StatsEvent = "stats" + BackupCompletedEvent = "backup completed" ) type Event struct { diff --git a/server/install.go b/server/install.go index 0d8fa93..54ebcfb 100644 --- a/server/install.go +++ b/server/install.go @@ -36,6 +36,9 @@ func (s *Server) Install(sync bool) error { } } + // Send the start event so the Panel can automatically update. + s.Events().Publish(InstallStartedEvent, "") + err := s.internalInstall() s.Log().Debug("notifying panel of server install state") @@ -52,6 +55,10 @@ func (s *Server) Install(sync bool) error { l.Warn("failed to notify panel of server install state") } + // Push an event to the websocket so we can auto-refresh the information in the panel once + // the install is completed. + s.Events().Publish(InstallCompletedEvent, "") + return err } From 373dbd355e5cb4b1f763647464a98d2e692636d0 Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Wed, 29 Jul 2020 21:56:22 -0700 Subject: [PATCH 37/37] Better handling of subscribers to avoid a slice panic --- server/events.go | 29 ++++++++++++++++++++++++++--- server/server.go | 1 + 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/server/events.go b/server/events.go index 425de25..71da55b 100644 --- a/server/events.go +++ b/server/events.go @@ -32,6 +32,9 @@ type EventBus struct { // Returns the server's emitter instance. func (s *Server) Events() *EventBus { + s.emitterLock.Lock() + defer s.emitterLock.Unlock() + if s.emitter == nil { s.emitter = &EventBus{ subscribers: map[string][]chan Event{}, @@ -85,11 +88,27 @@ func (e *EventBus) Subscribe(topic string, ch chan Event) { e.Lock() defer e.Unlock() - if p, ok := e.subscribers[topic]; ok { - e.subscribers[topic] = append(p, ch) - } else { + p, ok := e.subscribers[topic] + + // If there is nothing currently subscribed to this topic just go ahead and create + // the item and then return. + if !ok { e.subscribers[topic] = append([]chan Event{}, ch) + return } + + // If this topic is already setup, first iterate over the event channels currently in there + // and confirm there is not a match. If there _is_ a match do nothing since that means this + // channel is already being tracked. This avoids registering two identical handlers for the + // same topic, and means the Unsubscribe function can safely assume there will only be a + // single match for an event. + for i := range e.subscribers[topic] { + if ch == e.subscribers[topic][i] { + return + } + } + + e.subscribers[topic] = append(p, ch) } // Unsubscribe a channel from a topic. @@ -101,6 +120,10 @@ func (e *EventBus) Unsubscribe(topic string, ch chan Event) { for i := range e.subscribers[topic] { if ch == e.subscribers[topic][i] { e.subscribers[topic] = append(e.subscribers[topic][:i], e.subscribers[topic][i+1:]...) + // Subscribe enforces a unique event channel for the topic, so we can safely exit + // this loop once matched since there should not be any additional matches after + // this point. + break } } } diff --git a/server/server.go b/server/server.go index e07a340..7530f36 100644 --- a/server/server.go +++ b/server/server.go @@ -19,6 +19,7 @@ type Server struct { // Internal mutex used to block actions that need to occur sequentially, such as // writing the configuration to the disk. sync.RWMutex + emitterLock sync.Mutex // Maintains the configuration for the server. This is the data that gets returned by the Panel // such as build settings and container images.