From 6a89348bf722e53693a5dbc2943cd6551e440860 Mon Sep 17 00:00:00 2001 From: Seena Fallah Date: Thu, 31 Dec 2020 18:18:45 +0330 Subject: [PATCH] configFile: directly write changes to configFile In some situations, the config.json is volumed from somewhere else and the file is busy, move can't be performed on this file and configFile.Save() will be failed because it's trying to move another file to this file. This will write changes directly to the file. For concurrency issues, it will lock the file using flock and release after write. Signed-off-by: Seena Fallah --- cli/config/configfile/file.go | 37 +++++++++++++++--------------- cli/config/configfile/file_unix.go | 35 ---------------------------- 2 files changed, 19 insertions(+), 53 deletions(-) delete mode 100644 cli/config/configfile/file_unix.go diff --git a/cli/config/configfile/file.go b/cli/config/configfile/file.go index dc9f39eb7e3d..d87b2c9adf5b 100644 --- a/cli/config/configfile/file.go +++ b/cli/config/configfile/file.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "strings" + "syscall" "github.com/docker/cli/cli/config/credentials" "github.com/docker/cli/cli/config/types" @@ -194,37 +195,37 @@ func (configFile *ConfigFile) Save() (retErr error) { if err := os.MkdirAll(dir, 0700); err != nil { return err } - temp, err := ioutil.TempFile(dir, filepath.Base(configFile.Filename)) + + // Handle situation where the configfile is a symlink + cfgFile := configFile.Filename + if f, err := os.Readlink(cfgFile); err == nil { + cfgFile = f + } + + f, err := os.OpenFile(cfgFile, os.O_CREATE|os.O_WRONLY, 0600) if err != nil { return err } defer func() { - temp.Close() - if retErr != nil { - if err := os.Remove(temp.Name()); err != nil { - logrus.WithError(err).WithField("file", temp.Name()).Debug("Error cleaning up temp file") - } + err = syscall.Flock(int(f.Fd()), syscall.LOCK_UN) + if err != nil { + logrus.WithError(err).WithField("file", cfgFile).Error("Unable to unlock config file") } + f.Close() }() - err = configFile.SaveToWriter(temp) + err = syscall.Flock(int(f.Fd()), syscall.LOCK_EX) if err != nil { + logrus.WithError(err).WithField("file", cfgFile).Error("Unable to lock config file") return err } - if err := temp.Close(); err != nil { - return errors.Wrap(err, "error closing temp file") - } - - // Handle situation where the configfile is a symlink - cfgFile := configFile.Filename - if f, err := os.Readlink(cfgFile); err == nil { - cfgFile = f + err = f.Truncate(0) + if err != nil { + return err } - // Try copying the current config file (if any) ownership and permissions - copyFilePermissions(cfgFile, temp.Name()) - return os.Rename(temp.Name(), cfgFile) + return configFile.SaveToWriter(f) } // ParseProxyConfig computes proxy configuration by retrieving the config for the provided host and diff --git a/cli/config/configfile/file_unix.go b/cli/config/configfile/file_unix.go deleted file mode 100644 index 3ca65c6140d6..000000000000 --- a/cli/config/configfile/file_unix.go +++ /dev/null @@ -1,35 +0,0 @@ -// +build !windows - -package configfile - -import ( - "os" - "syscall" -) - -// copyFilePermissions copies file ownership and permissions from "src" to "dst", -// ignoring any error during the process. -func copyFilePermissions(src, dst string) { - var ( - mode os.FileMode = 0600 - uid, gid int - ) - - fi, err := os.Stat(src) - if err != nil { - return - } - if fi.Mode().IsRegular() { - mode = fi.Mode() - } - if err := os.Chmod(dst, mode); err != nil { - return - } - - uid = int(fi.Sys().(*syscall.Stat_t).Uid) - gid = int(fi.Sys().(*syscall.Stat_t).Gid) - - if uid > 0 && gid > 0 { - _ = os.Chown(dst, uid, gid) - } -}