fileutil: WritePayloads atomically for nested paths (#1028)

WritePayloads attempts to cleanup files not written by AtomicWriter by
checking whether or not the file is a symlink.

However, in the case of nested paths, AtomicWriter only symlinks the
first component of the path, so cleanupProviderFiles would erroneously
remove the file.

See: https://github.com/kubernetes-sigs/secrets-store-csi-driver/blob/f851c86011e845f96184b61d53f3a88dcba0f81a/pkg/util/fileutil/atomic_writer.go#L416-L426

pkg/util/fileutil/writer.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"sigs.k8s.io/secrets-store-csi-driver/provider/v1alpha1"
 )
@@ -75,7 +76,10 @@ func WritePayloads(path string, payloads []*v1alpha1.File) error {
 // get updated
 func cleanupProviderFiles(path string, payloads []*v1alpha1.File) error {
 	for i := range payloads {
-		p := filepath.Join(path, payloads[i].GetPath())
+		// AtomicWriter only symlinks the top file or directory
+		firstComponent := strings.Split(payloads[i].GetPath(), string(os.PathSeparator))[0]
+
+		p := filepath.Join(path, firstComponent)
 		info, err := os.Lstat(p)
 		if os.IsNotExist(err) {
 			continue
@@ -87,7 +91,7 @@ func cleanupProviderFiles(path string, payloads []*v1alpha1.File) error {
 		if info.Mode()&os.ModeSymlink != 0 {
 			continue
 		}
-		if err := os.Remove(p); err != nil {
+		if err := os.RemoveAll(p); err != nil {
 			return err
 		}
 	}

pkg/util/fileutil/writer_test.go

@@ -436,6 +436,72 @@ func TestWritePayloads_BackwardCompatible(t *testing.T) {
 	}
 }
 
+func TestCleanupProviderFiles(t *testing.T) {
+	wantFiles := []*v1alpha1.File{
+		{Path: "foo", Contents: []byte("whatever"), Mode: 0600},
+		{Path: "bar/baz", Contents: []byte("whatever"), Mode: 0600},
+	}
+	dontWantFiles := []*v1alpha1.File{
+		{Path: "asdf", Contents: []byte("whatever"), Mode: 0600},
+		{Path: "abc/123", Contents: []byte("whatever"), Mode: 0600},
+	}
+
+	dir := t.TempDir()
+
+	// writes with AtomicWriter
+	w, err := NewAtomicWriter(dir, "")
+	if err != nil {
+		t.Fatalf("creating atomic writer: %s", err)
+	}
+	filesToWriteAtomically := make(map[string]FileProjection, len(wantFiles))
+	for _, f := range wantFiles {
+		filesToWriteAtomically[f.Path] = FileProjection{Data: f.Contents, Mode: f.Mode}
+	}
+	if err := w.Write(filesToWriteAtomically); err != nil {
+		t.Errorf("writing with AtomicWriter: %s", err)
+	}
+
+	// writes without AtomicWriter
+	for _, f := range dontWantFiles {
+		if subdir, _ := filepath.Split(f.Path); subdir != "" {
+			if err := os.MkdirAll(filepath.Join(dir, subdir), 0700); err != nil {
+				t.Errorf("creating subdir: %s", err)
+			}
+		}
+		if err := os.WriteFile(filepath.Join(dir, f.Path), f.Contents, fs.FileMode(f.Mode)); err != nil {
+			t.Errorf("writing files without AtomicWriter: %s", err)
+		}
+	}
+
+	var toCleanup []*v1alpha1.File
+	toCleanup = append(toCleanup, wantFiles...)
+	toCleanup = append(toCleanup, dontWantFiles...)
+	if err := cleanupProviderFiles(dir, toCleanup); err != nil {
+		t.Errorf("cleaning provider files: %s", err)
+	}
+
+	// all files written by AtomicWriter should NOT be cleaned up by cleanupProviderFiles
+	for _, f := range wantFiles {
+		path := filepath.Join(dir, f.Path)
+		if fh, err := os.Open(path); err != nil {
+			t.Errorf("got error opening %q, written by AtomicWriter, want no error: %s", f.Path, err)
+		} else {
+			fh.Close()
+		}
+	}
+
+	// all other files SHOULD be cleaned up by cleanupProviderFiles
+	for _, f := range dontWantFiles {
+		path := filepath.Join(dir, f.Path)
+		if fh, err := os.Open(path); err == nil { // if NO error
+			fh.Close()
+			t.Errorf("got no error opening %q, written without AtomicWriter, want os.IsNotExist", f.Path)
+		} else if !os.IsNotExist(err) {
+			t.Errorf("got unexpected error opening %q, written without AtomicWriter, want os.IsNotExist: %s", f.Path, err)
+		}
+	}
+}
+
 func readPayloads(path string, payloads []*v1alpha1.File) error {
 	for _, p := range payloads {
 		fp := filepath.Join(path, p.Path)