diff --git a/command/cliconfig/config_unix.go b/command/cliconfig/config_unix.go index 36a5939da..5922c17ac 100644 --- a/command/cliconfig/config_unix.go +++ b/command/cliconfig/config_unix.go @@ -50,8 +50,3 @@ func homeDir() (string, error) { return user.HomeDir, nil } - -func replaceFileAtomic(source, destination string) error { - // On Unix systems, a rename is sufficiently atomic. - return os.Rename(source, destination) -} diff --git a/command/cliconfig/config_windows.go b/command/cliconfig/config_windows.go index ff3a10993..526b5d1de 100644 --- a/command/cliconfig/config_windows.go +++ b/command/cliconfig/config_windows.go @@ -3,12 +3,9 @@ package cliconfig import ( - "os" "path/filepath" "syscall" "unsafe" - - "golang.org/x/sys/windows" ) var ( @@ -47,25 +44,3 @@ func homeDir() (string, error) { return syscall.UTF16ToString(b), nil } - -func replaceFileAtomic(source, destination string) error { - // On Windows, renaming one file over another is not atomic and certain - // error conditions can result in having only the source file and nothing - // at the destination file. Instead, we need to call into the MoveFileEx - // Windows API function. - srcPtr, err := syscall.UTF16PtrFromString(source) - if err != nil { - return &os.LinkError{"replace", source, destination, err} - } - destPtr, err := syscall.UTF16PtrFromString(destination) - if err != nil { - return &os.LinkError{"replace", source, destination, err} - } - - flags := uint32(windows.MOVEFILE_REPLACE_EXISTING | windows.MOVEFILE_WRITE_THROUGH) - err = windows.MoveFileEx(srcPtr, destPtr, flags) - if err != nil { - return &os.LinkError{"replace", source, destination, err} - } - return nil -} diff --git a/command/cliconfig/credentials.go b/command/cliconfig/credentials.go index 9d394bf29..0b56191e2 100644 --- a/command/cliconfig/credentials.go +++ b/command/cliconfig/credentials.go @@ -12,9 +12,10 @@ import ( "github.com/zclconf/go-cty/cty" ctyjson "github.com/zclconf/go-cty/cty/json" - "github.com/hashicorp/terraform-svchost" + svchost "github.com/hashicorp/terraform-svchost" svcauth "github.com/hashicorp/terraform-svchost/auth" "github.com/hashicorp/terraform/configs/hcl2shim" + "github.com/hashicorp/terraform/internal/replacefile" pluginDiscovery "github.com/hashicorp/terraform/plugin/discovery" ) @@ -336,7 +337,7 @@ func (s *CredentialsSource) updateLocalHostCredentials(host svchost.Hostname, ne // Temporary file now replaces the original file, as atomically as // possible. (At the very least, we should not end up with a file // containing only a partial JSON object.) - err = replaceFileAtomic(tmpName, filename) + err = replacefile.AtomicRename(tmpName, filename) if err != nil { return fmt.Errorf("failed to replace %s with temporary file %s: %s", filename, tmpName, err) } diff --git a/internal/replacefile/doc.go b/internal/replacefile/doc.go new file mode 100644 index 000000000..8a1fbd993 --- /dev/null +++ b/internal/replacefile/doc.go @@ -0,0 +1,12 @@ +// Package replacefile is a small helper package focused directly at the +// problem of atomically "renaming" one file over another one. +// +// On Unix systems this is the standard behavior of the rename function, but +// the equivalent operation on Windows requires some specific operation flags +// which this package encapsulates. +// +// This package uses conditional compilation to select a different +// implementation for Windows vs. all other platforms. It may therefore +// require further fiddling in future if Terraform is ported to another +// OS that is neither Unix-like nor Windows. +package replacefile diff --git a/internal/replacefile/replacefile_unix.go b/internal/replacefile/replacefile_unix.go new file mode 100644 index 000000000..4bc92b83c --- /dev/null +++ b/internal/replacefile/replacefile_unix.go @@ -0,0 +1,24 @@ +// +build !windows + +package replacefile + +import ( + "os" +) + +// AtomicRename renames from the source path to the destination path, +// atomically replacing any file that might already exist at the destination. +// +// Typically this operation can succeed only if the source and destination +// are within the same physical filesystem, so this function is best reserved +// for cases where the source and destination exist in the same directory and +// only the local filename differs between them. +// +// The Unix implementation of AtomicRename relies on the atomicity of renaming +// that is required by the ISO C standard, which in turn assumes that Go's +// implementation of rename is calling into a system call that preserves that +// guarantee. +func AtomicRename(source, destination string) error { + // On Unix systems, a rename is sufficiently atomic. + return os.Rename(source, destination) +} diff --git a/internal/replacefile/replacefile_windows.go b/internal/replacefile/replacefile_windows.go new file mode 100644 index 000000000..690e57bc2 --- /dev/null +++ b/internal/replacefile/replacefile_windows.go @@ -0,0 +1,40 @@ +// +build windows + +package replacefile + +import ( + "os" + "syscall" + + "golang.org/x/sys/windows" +) + +// AtomicRename renames from the source path to the destination path, +// atomically replacing any file that might already exist at the destination. +// +// Typically this operation can succeed only if the source and destination +// are within the same physical filesystem, so this function is best reserved +// for cases where the source and destination exist in the same directory and +// only the local filename differs between them. +func AtomicRename(source, destination string) error { + // On Windows, renaming one file over another is not atomic and certain + // error conditions can result in having only the source file and nothing + // at the destination file. Instead, we need to call into the MoveFileEx + // Windows API function, setting two flags to opt in to replacing an + // existing file. + srcPtr, err := syscall.UTF16PtrFromString(source) + if err != nil { + return &os.LinkError{"replace", source, destination, err} + } + destPtr, err := syscall.UTF16PtrFromString(destination) + if err != nil { + return &os.LinkError{"replace", source, destination, err} + } + + flags := uint32(windows.MOVEFILE_REPLACE_EXISTING | windows.MOVEFILE_WRITE_THROUGH) + err = windows.MoveFileEx(srcPtr, destPtr, flags) + if err != nil { + return &os.LinkError{"replace", source, destination, err} + } + return nil +} diff --git a/internal/replacefile/writefile.go b/internal/replacefile/writefile.go new file mode 100644 index 000000000..23adde0fa --- /dev/null +++ b/internal/replacefile/writefile.go @@ -0,0 +1,71 @@ +package replacefile + +import ( + "fmt" + "io/ioutil" + "os" + "path/filepath" +) + +// AtomicWriteFile uses a temporary file along with this package's AtomicRename +// function in order to provide a replacement for ioutil.WriteFile that +// writes the given file into place as atomically as the underlying operating +// system can support. +// +// The sense of "atomic" meant by this function is that the file at the +// given filename will either contain the entirety of the previous contents +// or the entirety of the given data array if opened and read at any point +// during the execution of the function. +// +// On some platforms attempting to overwrite a file that has at least one +// open filehandle will produce an error. On other platforms, the overwriting +// will succeed but existing open handles will still refer to the old file, +// even though its directory entry is no longer present. +// +// Although AtomicWriteFile tries its best to avoid leaving behind its +// temporary file on error, some particularly messy error cases may result +// in a leftover temporary file. +func AtomicWriteFile(filename string, data []byte, perm os.FileMode) error { + dir, file := filepath.Split(filename) + f, err := ioutil.TempFile(dir, file) // alongside target file and with a similar name + if err != nil { + return fmt.Errorf("cannot create temporary file to update %s: %s", filename, err) + } + tmpName := f.Name() + moved := false + defer func(f *os.File, name string) { + // Remove the temporary file if it hasn't been moved yet. We're + // ignoring errors here because there's nothing we can do about + // them anyway. + if !moved { + os.Remove(name) + } + }(f, tmpName) + + // We'll try to apply the requested permissions. This may + // not be effective on all platforms, but should at least work on + // Unix-like targets and should be harmless elsewhere. + if err := os.Chmod(tmpName, perm); err != nil { + return fmt.Errorf("cannot set mode for temporary file %s: %s", tmpName, err) + } + + // Write the credentials to the temporary file, then immediately close + // it, whether or not the write succeeds. Note that closing the file here + // is required because on Windows we can't move a file while it's open. + _, err = f.Write(data) + f.Close() + if err != nil { + return fmt.Errorf("cannot write to temporary file %s: %s", tmpName, err) + } + + // Temporary file now replaces the original file, as atomically as + // possible. (At the very least, we should not end up with a file + // containing only a partial JSON object.) + err = AtomicRename(tmpName, filename) + if err != nil { + return fmt.Errorf("failed to replace %s with temporary file %s: %s", filename, tmpName, err) + } + + moved = true + return nil +}