Fix a potential file leak

pkg/kubelet/kubeletconfig/util/files/files.go

@@ -63,34 +63,51 @@ func EnsureFile(fs utilfs.Filesystem, path string) error {
 	return file.Close()
 }
 
-// ReplaceFile replaces the contents of the file at `path` with `data` by writing to a tmp file in the same
-// dir as `path` and renaming the tmp file over `path`. The file does not have to exist to use ReplaceFile.
-func ReplaceFile(fs utilfs.Filesystem, path string, data []byte) error {
+// WriteTmpFile creates a temporary file at `path`, writes `data` into it, and fsyncs the file
+func WriteTmpFile(fs utilfs.Filesystem, path string, data []byte) (tmpPath string, retErr error) {
 	dir := filepath.Dir(path)
 	prefix := filepath.Base(path)
 
 	// create the tmp file
 	tmpFile, err := fs.TempFile(dir, prefix)
 	if err != nil {
-		return err
+		return "", err
 	}
+	defer func() {
+		// close the file, return the close error only if there haven't been any other errors
+		if err := tmpFile.Close(); retErr == nil {
+			retErr = err
+		}
+		// if there was an error writing, syncing, or closing, delete the temporary file and return the error
+		if retErr != nil {
+			if err := fs.Remove(tmpPath); err != nil {
+				retErr = fmt.Errorf("attempted to remove temporary file %q after error %v, but failed due to error: %v", path, retErr, err)
+			}
+			tmpPath = ""
+		}
+	}()
 
 	// Name() will be an absolute path when using utilfs.DefaultFS, because ioutil.TempFile passes
 	// an absolute path to os.Open, and we ensure similar behavior in utilfs.FakeFS for testing.
-	tmpPath := tmpFile.Name()
+	tmpPath = tmpFile.Name()
 
 	// write data
 	if _, err := tmpFile.Write(data); err != nil {
-		return err
+		return tmpPath, err
 	}
 	// sync file, to ensure it's written in case a hard reset happens
-	if err := tmpFile.Sync(); err != nil {
-		return err
-	}
-	if err := tmpFile.Close(); err != nil {
+	return tmpPath, tmpFile.Sync()
+}
+
+// ReplaceFile replaces the contents of the file at `path` with `data` by writing to a tmp file in the same
+// dir as `path` and renaming the tmp file over `path`. The file does not have to exist to use ReplaceFile.
+// Note ReplaceFile calls fsync.
+func ReplaceFile(fs utilfs.Filesystem, path string, data []byte) error {
+	// write data to a temporary file
+	tmpPath, err := WriteTmpFile(fs, path, data)
+	if err != nil {
 		return err
 	}
-
 	// rename over existing file
 	if err := fs.Rename(tmpPath, path); err != nil {
 		return err

pkg/util/filesystem/defaultfs.go

@@ -62,6 +62,11 @@ func (DefaultFs) RemoveAll(path string) error {
 	return os.RemoveAll(path)
 }
 
+// Remove via os.RemoveAll
+func (DefaultFs) Remove(name string) error {
+	return os.Remove(name)
+}
+
 // ReadFile via ioutil.ReadFile
 func (DefaultFs) ReadFile(filename string) ([]byte, error) {
 	return ioutil.ReadFile(filename)

pkg/util/filesystem/fakefs.go

@@ -92,6 +92,11 @@ func (fs *fakeFs) RemoveAll(path string) error {
 	return fs.a.RemoveAll(path)
 }
 
+// Remove via afero.RemoveAll
+func (fs *fakeFs) Remove(name string) error {
+	return fs.a.Remove(name)
+}
+
 // fakeFile implements File; for use with fakeFs
 type fakeFile struct {
 	file afero.File

pkg/util/filesystem/filesystem.go

@@ -31,6 +31,7 @@ type Filesystem interface {
 	MkdirAll(path string, perm os.FileMode) error
 	Chtimes(name string, atime time.Time, mtime time.Time) error
 	RemoveAll(path string) error
+	Remove(name string) error
 
 	// from "io/ioutil"
 	ReadFile(filename string) ([]byte, error)